Merge lp:~adeuring/launchpad/bug-479331 into lp:launchpad

This branch fixes (sort of) bug 479331: It's not possible to set someone else as bug supervisor.

While the bug description says that anybody should be able to appoint the bug supervisor, it seems for now better to keep the policy of the structural bug subscription which is autmatically performed for the new supervisor: People may only subscribe themselves and teams they administer.

Firstly, the policy for structural bug subscription is reasonable: people who are not aware of the consequences of becoming a bug supervisor might feel being spammed by the possibly huge amount of bug mails they suddenly receive.

Secondly, the option to simply allow anybody to appoint a bug supervisor would mean to implement another exception to the subscription permission check in StructuralSubscriptionTargetMixin.userCanAlterSubscription(), which is already slightly too complicated for my taste, with the special handling of IDistributionSourcePackage targets.

So I only added a check if the person which tries to appoint the bug supervisor can actually do this: userCanAlterSubscription() is now called in BugSupervisorEditView.validate(). This involved to rename the method _userCanAlterSubscription(), to ad it to the interface class and to change two ZCML files.

The problem that most people cannot appoint others as bug supervisors was not caught in the tests because the persons used in the tests were all Launchpad administrators, which are allowed to make structural subscriptions for others. So I had to change the "test persons", and I had to change the teams they can subscribe. (BTW, our test data has way too many LP administrators: I needed some time to find persons who are team admins but not LP admins.)

The changed and added tests quickly re-appoint several persons and team as bug supervisors. When a new bug supervisor is appointed, the structural of the old supervisor is not automatically removed. The set of people subscribed are shown in other page tests, so these sets changed too: I did not bother add anything to remove the subscription.

test:./bin/test -v -f lp stories.initial-bug-contacts

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/database/structuralsubscription.py
  lib/canonical/launchpad/interfaces/structuralsubscription.py
  lib/lp/bugs/browser/bugsupervisor.py
  lib/lp/bugs/stories/initial-bug-contacts/05-set-distribution-bugcontact.txt
  lib/lp/bugs/stories/initial-bug-contacts/10-set-upstream-bugcontact.txt
  lib/lp/bugs/stories/initial-bug-contacts/20-file-upstream-bug.txt
  lib/lp/bugs/stories/initial-bug-contacts/25-file-distribution-bug.txt
  lib/lp/registry/configure.zcml

== Pylint notices ==

lib/canonical/launchpad/interfaces/structuralsubscription.py
    22: [F0401] Unable to import 'lazr.enum' (No module named enum)
    29: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    34: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)

Hi Abel,

Nice branch. Couple of minor issues to deal with - nothing major - and the
error message change that we discussed on IRC. Other than that, this is
good to go.

> === modified file 'lib/lp/bugs/browser/bugsupervisor.py'
> --- lib/lp/bugs/browser/bugsupervisor.py 2009-09-09 13:37:24 +0000
> +++ lib/lp/bugs/browser/bugsupervisor.py 2009-11-12 12:30:31 +0000
> @@ -11,7 +11,6 @@
> from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
> from canonical.launchpad.webapp import (
> action, canonical_url, LaunchpadEditFormView)
> -from canonical.launchpad.webapp.authorization import check_permission
> from canonical.launchpad.webapp.menu import structured
>
>
> @@ -91,19 +90,31 @@
>
> supervisor = data['bug_supervisor']
>
> - if (supervisor is not None and supervisor.isTeam() and
> - supervisor not in self.user.getAdministratedTeams() and not
> - check_permission('launchpad.Admin', self.user)):
> - error = structured(
> - "You cannot set %(team)s as the bug supervisor for "
> - "%(target)s because you are not an administrator of that "
> - "team.<br />If you believe that %(team)s should be the bug"
> - " supervisor for %(target)s, please notify one of the "
> - "<a href=\"%(url)s\">%(team)s administrators</a>.",
> - team=supervisor.displayname,
> - target=self.context.displayname,
> - url=(canonical_url(supervisor, rootsite='mainsite') +
> - '/+members'))
> + # Making a person the bug supervisor implies subscribing him
> + # to all bug mail. Ensure that the current user can indeed
> + # do this.
> + if (supervisor is not None and
> + not self.context.userCanAlterSubscription(supervisor, self.user)):
> + if supervisor.isTeam():
> + error = structured(
> + "You cannot set %(team)s as the bug supervisor for "
> + "%(target)s because you are not an administrator of that "
> + "team.<br />If you believe that %(team)s should be the "
> + "bug supervisor for %(target)s, please notify one of the "
> + "<a href=\"%(url)s\">%(team)s administrators</a>.",
> + team=supervisor.displayname,
> + target=self.context.displayname,
> + url=(canonical_url(supervisor, rootsite='mainsite') +
> + '/+members'))
> + self.setFieldError('bug_supervisor', error)
> + else:
> + error = structured(
> + "You cannot set another person as the bug supervisor for "
> + "%(target)s.<br/> If you believe that %(person)s should "
> + "be the bug supervisor for %(target)s, please ask this "
> + "person to make itself the bug supervisor.",

As we discussed on IRC, this error message needs to be changed, since
people who aren't project admins or members of admin teams can't set
themselves as bug supervisor.

> + ...