launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-736647 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #736647 in Launchpad itself: "Branch:+new-recipe fails to render for +junk branches"
  https://bugs.launchpad.net/launchpad/+bug/736647

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-736647/+merge/53761

Branch:+new-recipe recently started OOPSing with a TypeError (trying to use a str where Storm wanted a unicode) when a default was introduced for the name field. This is because it takes '+junk' from PersonBranchTarget.name, where the other IBranchTargets have a unicode name derived from the DB. I've fixed PersonBranchTarget.name to be u'+junk' instead.

However, that didn't yield a particularly usable name: the generated default (+junk-daily) is invalid due to the leading +, and +junk probably isn't what they want anyway. So I special cased PersonBranchTarget to use the branch name instead of the target name.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-736647/+merge/53761
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-736647 into lp:launchpad.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-03-02 01:47:19 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-03-17 09:32:35 +0000
@@ -48,6 +48,7 @@
     SimpleTerm,
     SimpleVocabulary,
     )
+from zope.security.proxy import isinstance as zope_isinstance
 
 from canonical.database.constants import UTC_NOW
 from canonical.launchpad import _
@@ -97,6 +98,7 @@
     MINIMAL_RECIPE_TEXT,
     RECIPE_BETA_FLAG,
     )
+from lp.code.model.branchtarget import PersonBranchTarget
 from lp.code.model.sourcepackagerecipe import get_buildable_distroseries_set
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.features import getFeatureFlag
@@ -701,7 +703,12 @@
 
     def _recipe_names(self):
         """A generator of recipe names."""
-        branch_target_name = self.context.target.name.split('/')[-1]
+        # +junk-daily doesn't make a very good recipe name, so use the
+        # branch name in that case.
+        if zope_isinstance(self.context.target, PersonBranchTarget):
+            branch_target_name = self.context.name
+        else:
+            branch_target_name = self.context.target.name.split('/')[-1]
         yield "%s-daily" % branch_target_name
         counter = itertools.count(1)
         while True:

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-03-02 22:38:44 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-03-17 09:32:35 +0000
@@ -224,6 +224,15 @@
             view = create_initialized_view(branch, '+new-recipe')
         self.assertThat('widget-daily', Equals(view.initial_values['name']))
 
+    def test_personal_branch_initial_name(self):
+        # When a personal branch is used, the initial name is the name of the
+        # branch followed by "-daily". +junk-daily is not valid nor
+        # helpful.
+        branch = self.factory.makePersonalBranch(name='widget')
+        with person_logged_in(branch.owner):
+            view = create_initialized_view(branch, '+new-recipe')
+        self.assertThat('widget-daily', Equals(view.initial_values['name']))
+
     def test_initial_name_exists(self):
         # If the initial name exists, a generator is used to find an unused
         # name by appending a numbered suffix on the end.

=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2011-01-31 02:12:30 +0000
+++ lib/lp/code/model/branchtarget.py	2011-03-17 09:32:35 +0000
@@ -198,7 +198,7 @@
 class PersonBranchTarget(_BaseBranchTarget):
     implements(IBranchTarget)
 
-    name = '+junk'
+    name = u'+junk'
     default_stacked_on_branch = None
     default_merge_target = None