launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/branch-affiliation into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #857697 in Launchpad itself: "AttributeError: 'NoneType' object has no attribute 'owner'  get affiliation in person picker"
  https://bugs.launchpad.net/launchpad/+bug/857697

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/branch-affiliation/+merge/77018

Personal branches do not have project affiliation.

    Launchpad bug: https://bugs.launchpad.net/bugs/857697
    Pre-implementation: No one

OOPS-2092AQ938 happened when a user wanted to add a subscriber to his
personal branch (+junk).

The affiliation adapter for branches assumes there is always a pillar. This is
not true for junk branches. The methods that want pillars appear to looping
over the getPillars(), The fix may be to return an empty list.

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

RULES

    * Update BranchPillarAffiliation.getPillars() to return an empty list
      if the branch's product and distribution is None.


QA

    * Visit one of your personal branches (+junk)
    * Choose the Add subscriber link
    * Choose the "Choose" link next the the person field.
    * Search for a user or team
    * Verify you do not get an error
    * Choose a user or team


LINT

    lib/lp/registry/model/pillaraffiliation.py
    lib/lp/registry/tests/test_pillaraffiliation.py


IMPLEMENTATION

Updated the return an empty list when the branches project and distribution
is None.
    lib/lp/registry/model/pillaraffiliation.py
    lib/lp/registry/tests/test_pillaraffiliation.py
-- 
https://code.launchpad.net/~sinzui/launchpad/branch-affiliation/+merge/77018
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/branch-affiliation into lp:launchpad.

=== modified file 'lib/lp/registry/model/pillaraffiliation.py'
--- lib/lp/registry/model/pillaraffiliation.py	2011-09-04 23:19:06 +0000
+++ lib/lp/registry/model/pillaraffiliation.py	2011-09-26 16:33:25 +0000
@@ -193,7 +193,11 @@
     """An affiliation adapter for branches."""
 
     def getPillars(self):
-        return [self.context.product or self.context.distribution]
+        pillar = self.context.product or self.context.distribution
+        if pillar is None:
+            # This is a +junk branch.
+            return []
+        return [pillar]
 
     def getBranch(self):
         return self.context

=== modified file 'lib/lp/registry/tests/test_pillaraffiliation.py'
--- lib/lp/registry/tests/test_pillaraffiliation.py	2011-09-04 23:19:06 +0000
+++ lib/lp/registry/tests/test_pillaraffiliation.py	2011-09-26 16:33:25 +0000
@@ -271,6 +271,11 @@
         adapter = IHasAffiliation(branch)
         self.assertEqual([branch.product], adapter.getPillars())
 
+    def test_personal_branches_have_no_pillars(self):
+        branch = self.factory.makeBranch(product=None)
+        adapter = IHasAffiliation(branch)
+        self.assertEqual([], adapter.getPillars())
+
     def test_getBranch(self):
         # The branch is the context.
         branch = self.factory.makeBranch()