Code review comment for lp://staging/~julian-edwards/launchpad/ppa-deletion-ui

Great points Paul and Julian.

I'm just taking a look now, as by the time Paul's day starts, mine will be ending. The UI looks great and the redirection seems natural. I've got notes below, but basically I'm happy to ui=me - unless Paul disagrees of course - with the Delete menu item moved as below, and the Edit/Copy/Delete menu items disabled when the PPA is deleted/deleting.

Great stuff!

Details:
When first browsing:

https://launchpad.dev/~cprov/+archive/ppa

I was a bit surprised to see the Delete action as the first presented.

{{{
11:49 < noodles> bigjools: did you intend for the Delete link to be the first presented in the menu? Or is it just because you've listed it alphabetically in the ArchiveIndexActionsMenu?
11:49 < bigjools> noodles: that did cross my mind, I was considering moving it but was waiting for someone to comment first
11:49 < noodles> I'm wondering if it should be the last action (ie. after 'manage_subscribers' on ArchiveIndexActionsMenu.links)
11:49 < bigjools> and you did :)
11:50 < noodles> Great.
}}}

I agree, when deleting the archive, redirecting to the profile and including the blue info box is a good solution. Works well.

At that point though, I'm wasn't sure there was any reason for presenting the link on the profile page allowing me to view the empty PPA, but Julian clarified that - the history is still viewable at:
https://launchpad.dev/~cprov/+archive/ppa/+packages?field.status_filter=

{{{
11:56 < noodles> bigjools: Is there any reason to present the PPA after the deletion has been requested? Why have a disabled-like PPA link on the profile that can be clicked on and viewed in it's empty state with the "This archive has been deleted" message?
11:57 < bigjools> so you can go in and view its history even after deletion
11:58 < bigjools> StevenK: looks good
11:58 < noodles> bigjools: ah, I misunderstood. So it will stay there permanently? OK.
11:58 < bigjools> StevenK: when that's done, you need to run process-accepted to make it do the re-upload between the librarians
11:59 < bigjools> noodles: yes - this is what wgrant wanted, and is the simpler first step before complete obliteration of all archive objects
}}}

which leaves me wondering whether, rather than saying (on the PPA index page), "This archive has been deleted.", we said "This archive has been deleted ([link]View deleted packages[/link])." which would (1) indicate that there is useful information still available and (2) save you from having to know to click on the Packages link and update the filter to be able to see them. See what you both think. (it would be nice if this extra link only appeared on the index page)

Note: you can use view.archive_label instead of 'archive' in your message if you want it to automatically use "PPA" or "archive".

The only other thoughts I had were around the presentation of the menu options, such as "Copy packages", "Delete packages", "Change details" and "Edit dependencies".

I'm assuming we want to disable the last 3 (currently I can click on Change details and "re-enable" my PPA, which then means that the "This PPA has been deleted" msg no longer appears, even though I assume it's still deleted). Note, disabling the links will not stop people from using the edit url if they want to - you may need to ensure that those views redirect back to the PPA with a msg if they are used when the archive is deleted?

Delete packages will never be relevant for a deleted archive.

Copy packages may be possible for a window after the deletion. Is it possible to disable the link only when the librarian files have been deleted? Or otherwise perhaps just disable them as soon as the archive is deleted/deleting.

{{{
12:26 < noodles> bigjools: another question, if the disk repository is blown away, it will still be possible to copy packages from a deleted archive right? (as the files are all in the librarian)?
12:26 < bigjools> noodles: no, they get deleted
12:26 < bigjools> well, eventually
12:27 < noodles> OK, in which case, we should disable to copy packages link right?
12:27 < bigjools> there will be a window where you can copy though
12:27 < bigjools> that should really get disabled when the archive is disabled
12:27 < bigjools> but good point
12:28 < bigjools> should disabling prevent copying?
12:28 < noodles> Why? If an archive is disabled, why shouldn't I...
12:28 < noodles> Yeah, I don't think so.
12:28 < bigjools> or should it be when status != ACTIVE
}}}

Cheers,
Michael