launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/remove-answer-contact-827051 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #827051 in Launchpad itself: "Allow question target owners to remove answer contacts"
  https://bugs.launchpad.net/launchpad/+bug/827051

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/remove-answer-contact-827051/+merge/71632

The canUserAlterAnswerContact method is used to determine whether a logged in user can remove an answer contact from the answer contacts portlet. It is required that question target (eg project) owners be able to remove answer contacts. However, DistributionSourcePackage (which is a question target) has no owner attribute.

== Implementation ==

Add an owner property to SourcePackageQuestionTargetMixin. DistributionSourcePackage extends this class. The owner property returns distribution.owner
Now canUserAlterAnswerContact() use "self.owner" in its code.

== QA ==

Goto https://answers.qastaging.launchpad.net/ubuntu/+source/firefox
Ensure both answer contacts portals load correctly and that if the logged in user is the owner of firefox dsp, they can remove answer contacts.

== Tests ==

Add a new test to lib/lp/answers/tests/test_questiontarget.py
  test_canUserAlterAnswerContact_DistributionSourcePackage_owner
This fills a gap in the test coverage because no tests called canUserAlterAnswerContact() for a dsp.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/interfaces/questiontarget.py
  lib/lp/answers/model/question.py
  lib/lp/answers/tests/test_questiontarget.py
  lib/lp/registry/model/sourcepackage.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/remove-answer-contact-827051/+merge/71632
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/remove-answer-contact-827051 into lp:launchpad.

=== modified file 'lib/lp/answers/interfaces/questiontarget.py'
--- lib/lp/answers/interfaces/questiontarget.py	2011-08-15 22:45:53 +0000
+++ lib/lp/answers/interfaces/questiontarget.py	2011-08-16 02:01:25 +0000
@@ -138,7 +138,7 @@
         """Can the user add or remove the answer contact.
 
         Users can add or remove themselves or one of the teams they
-        administered. Admins can add/remove anyone.
+        administered. Admins and target owners can add/remove anyone.
 
         :param person: The `IPerson` that is or will be an answer contact.
         :param subscribed_by: The `IPerson` making the change.

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2011-08-15 22:45:53 +0000
+++ lib/lp/answers/model/question.py	2011-08-16 02:01:25 +0000
@@ -1351,6 +1351,7 @@
         admins = getUtility(ILaunchpadCelebrities).admin
         if (person == subscribed_by
             or person in subscribed_by.administrated_teams
+            or subscribed_by.inTeam(self.owner)
             or subscribed_by.inTeam(admins)):
             return True
         return False

=== modified file 'lib/lp/answers/tests/test_questiontarget.py'
--- lib/lp/answers/tests/test_questiontarget.py	2011-08-12 04:17:29 +0000
+++ lib/lp/answers/tests/test_questiontarget.py	2011-08-16 02:01:25 +0000
@@ -42,6 +42,13 @@
             self.project.canUserAlterAnswerContact(
                 self.user, self.project.owner))
 
+    def test_canUserAlterAnswerContact_DistributionSourcePackage_owner(self):
+        login_person(self.user)
+        distro = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(distribution=distro)
+        self.assertTrue(
+            dsp.canUserAlterAnswerContact(self.user, distro.owner))
+
     def test_canUserAlterAnswerContact_other_user(self):
         login_person(self.user)
         other_user = self.factory.makePerson()

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2011-07-29 05:33:02 +0000
+++ lib/lp/registry/model/sourcepackage.py	2011-08-16 02:01:25 +0000
@@ -188,6 +188,10 @@
             self.distribution.answer_contacts_with_languages)
         return sorted(answer_contacts, key=attrgetter('displayname'))
 
+    @property
+    def owner(self):
+        return self.distribution.owner
+
 
 class SourcePackage(BugTargetBase, HasBugHeatMixin, HasCodeImportsMixin,
                     HasTranslationImportsMixin, HasTranslationTemplatesMixin,