Code review comment for lp:~jpds/launchpad/fix_361650

Hi Jonothan, thanks for yet another long-awaited feature :)

I'd really like to get Curtis to look at this one, although I've got a few comments.

The first question that came to my mind was, why is this another separate form (currently there is "Change details", "Review", and now "Set/Unset as country mirror".

I personally wonder whether the 'Review' form could accommodate this (perhaps renamed with something that implied official and/or country-mirror status?), so that both items could be presented and dealt with there on the one form. This would also alleviate the need for the changing menu item name ('Set/Unset'). But Curtis might have other ideas.

Also, when editing the mirror after setting it as a country mirror, the validation is great (for country/content-type), but afaik it shouldn't redirect back to the +index page, but instead +country-mirror form itself?

IRC snippet:

<noodles775> jpds: what's the reasoning behind not doing this as an admin-only field displayed on the normal edit form?
<jpds> noodles775: Do you mean the +review form?
<noodles775> jpds: no, I meant the +edit form (but i'm logged in as an admin, maybe that's not presented for mirror-admins...). Checking now.
<jpds> noodles775: I wanted to make it a little obvious by having the separate button for it.
<jpds> noodles775: And mirror registrants shouldn't be able to see the checkbox themselves.