Code review comment for lp:~edwin-grubbs/launchpad/bug-506965-merge-team-oops

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Edwin.

There are some dangerous changes in the code and the test does is not what is happening in production.

We need to revert the permissions on the view. It must be admin because we do not want every team owner/admin permission to merge their team with *any* other team. Delete restricts the operation to the context team and ~registry.

> Demo and Q/A
> ------------
>
> * Add ~no-priv to the ~registry team.
> * Login as ~no-priv.
> * Open http://launchpad.dev/people/+newteam
> * Add a team with a contact email address.
> * You will need to verify the email address with the token it emails you.
> (On launchpad.dev, you can just look in the logintoken table and paste the
> token into https://launchpad.dev/token/$token)
> * Open http://launchpad.dev/people/
> * Click on "Merge teams"
> * Enter you new team for the first team.
> * Enter "ubuntu-team" for the second team.
> * Click "Merge"
> * When you get a warning message, click "Deactivate members and merge".

This story is wrong. The delete team scenario is a very restricted merge operation that registry administators may perform. Admins can do unrestricted merges.
    * As registry expert.
    * Visit https://edge.launchpad.net/~consulvision
      * This team has an email address in NEW status
      * team email addresses are not shown in the UI
      * only the team owner can access the address.
    * Choose Delete team.
    * Verify the team is gone.

The code change is the the view, so we want a test that verifies the view handles the real conditions. The test should be in lp/registry/tests/peoplemerge-views.txt. You can update the the Delete team section to cover this story
    * Create a team with an email address.
    * deletable_.setContactAddress(None)
    * login_person(registry_experts.teamowner)
    * form = {'field.actions.delete': 'Delete'}
    * view = create_initialized_view(deletable_team, '+delete', form=form)
    * view.errors
      []
    * for notification in view.request.response.notifications:
         print notification.message
      Team deleted.

review: Needs Fixing (code)

« Back to merge proposal