Merge lp:~sinzui/launchpad/administer-team into lp:launchpad

Proposed by Curtis Hovey on 2012-01-27
Status: Merged
Approved by: Curtis Hovey on 2012-01-30
Approved revision: no longer in the source branch.
Merged at revision: 14735
Proposed branch: lp:~sinzui/launchpad/administer-team
Merge into: lp:launchpad
Diff against target: 0 lines
To merge this branch: bzr merge lp:~sinzui/launchpad/administer-team
Reviewer Review Type Date Requested Status
Benji York (community) code 2012-01-27 Approve on 2012-01-30
Review via email: mp+90530@code.launchpad.net
To post a comment you must log in.
Curtis Hovey (sinzui) wrote :

Add the administer link to the team page.

    Launchpad bug: https://bugs.launchpad.net/bugs/669658
    Pre-implementation: no one

While helping with a question , I realised I could not administer the
team. I could hack the URL with +review to change the team name, but the
link was not shown on the page.

--------------------------------------------------------------------

RULES

    * Add the administer link to the team page.
    * ADDENDUM: It is clear now that the link was omitted from the
      page because launchpad.Moderate is given to team owners and admins,
      as well as Lp admins and registry experts.
      * The link enabled rules need to be very clear about the roles
        being checked.

QA

    * Visit qastaging.launchpad.net/~launchpad
    * Verify there is an Administer link before the delete action.

LINT

    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_team.py
    lib/lp/testing/menu.py

TEST

    ./bin/test -vv -t Menu lp.registry.browser.tests.test_team

IMPLEMENTATION

Added an Administer link to the Team index page. I discovered that team
owners and registry experts saw the same links, then realised that they
both have launchpad.Moderate on a team. While is is harmless for team
owners and admins to see the link and use the page, we want them to
use +edit so I choose to use a checker that checks the roles we know
should have access to +review.
    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_team.py

I discovered that the bad link message was not always returned when the
context object was the reason there was no canonical URL. This fix will
help anyone else debugging specific links menus in the future.
    lib/lp/testing/menu.py

Benji York (benji) wrote :

This change looks good.

You might consider collapsing this if:

    if checker.checkAuthenticated(IPersonRoles(self.user)):
        enabled = True
    else:
        enabled = False

into this:

    enabled = checker.checkAuthenticated(IPersonRoles(self.user))

review: Approve (code)
Curtis Hovey (sinzui) wrote :

thank you for your suggestion. It is definitely the right thing to do.

Preview Diff

Empty