Merge lp:~wallyworld/launchpad/recipe-find-related-branches into lp:launchpad

Proposed by Ian Booth on 2011-01-25
Status: Merged
Approved by: Ian Booth on 2011-02-02
Approved revision: no longer in the source branch.
Merged at revision: 12313
Proposed branch: lp:~wallyworld/launchpad/recipe-find-related-branches
Merge into: lp:launchpad
Diff against target: 969 lines (+732/-6)
8 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+77/-3)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+246/-2)
lib/lp/code/interfaces/branchtarget.py (+30/-1)
lib/lp/code/model/branchtarget.py (+88/-0)
lib/lp/code/model/tests/test_branchtarget.py (+60/-0)
lib/lp/code/templates/sourcepackagerecipe-new.pt (+3/-0)
lib/lp/code/templates/sourcepackagerecipe-related-branches.pt (+75/-0)
lib/lp/testing/factory.py (+153/-0)
To merge this branch: bzr merge lp:~wallyworld/launchpad/recipe-find-related-branches
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve on 2011-02-02
Steve Kowalik (community) code* 2011-01-25 Approve on 2011-02-02
Curtis Hovey (community) ui 2011-01-25 Approve on 2011-02-01
Guilherme Salgado (community) ui* Needs Information on 2011-01-25
Review via email: mp+47367@code.launchpad.net

Commit message

[r=stevenk,thumper][ui=sinzui][bug=670452] Add display of related branches to recipe add and edit pages

Description of the change

This branch was merged before but failed QA and had to be rolled back.
See the original merge proposal for some background to the work done:
https://code.launchpad.net/~wallyworld/launchpad/recipe-find-related-branches/+merge/42097

Corrections have been made to the originally submitted code. The main problem was that the recipe [add|edit] view assumed the context branch was associated with a product. If the branch target was a sourcepackage instead, the view(s) would fail to load. Hence, for example, creating a recipe for a sourcepackage branch failed.

== Implementation ==

The related_series_branches() and related_package_branches() methods of the RecipeRelatedBranchesMixin class have been modified to fix the core issue. If a branch's product attribute is None, then no series branches are returned.

Talking to stevenk, it was decided that in the case of a branch with a target sourcepackage, the related package branches would be determined by using the linked_branches property of the target sourcepackage.

== Screenshot ==

*** ui review required ***

http://people.canonical.com/~ianb/recipe-related-branches1.png

The ui was modified to make the tables used to display the related branches a fixed width, similar to the text area widget. Vertical space was also added between the 2 tables. These changes make the ui look better, but perhaps more work is required?

== Tests ==

Tests were added to cater for the additional recipe creation/edit scenarios, plus some refactoring of what became common code was done.

New tests:

TestSourcePackageRecipeAddView
  test_new_product_branch_with_no_related_branches_recipe
  test_new_package_branch_with_no_linked_branches_recipe
  test_new_sourcepackage_branch_recipe_view_related_branches
  test_new_sourcepackage_branch_recipe_with_related_branches

TestSourcePackageRecipeEditView
  test_edit_product_branch_with_no_related_branches_recipe
  test_edit_sourcepackage_branch_with_no_related_branches_recipe
  test_edit_product_branch_recipe_with_related_branches
  test_edit_sourcepackage_recipe_with_related_branches

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/sourcepackagerecipe.py
  lib/lp/code/browser/tests/test_sourcepackagerecipe.py
  lib/lp/code/templates/sourcepackagerecipe-new.pt
  lib/lp/code/templates/sourcepackagerecipe-related-branches.pt

./lib/lp/code/browser/tests/test_sourcepackagerecipe.py
     552: E501 line too long (80 characters)
     672: E201 whitespace after '('
    1265: E501 line too long (85 characters)
    1295: E501 line too long (85 characters)
    1330: E501 line too long (85 characters)
     552: Line exceeds 78 characters.
    1265: Line exceeds 78 characters.
    1295: Line exceeds 78 characters.
    1317: Line exceeds 78 characters.
    1330: Line exceeds 78 characters.
./lib/lp/code/templates/sourcepackagerecipe-related-branches.pt
       1: unbound prefix

To post a comment you must log in.
Guilherme Salgado (salgado) wrote :

Hi Ian,

This looks like I nice addition, but I wonder if it's necessary to list the branch URLs or if it'd be enough to have just the branch's name (linking to the branch itself) and the series/package name (also linking to the relevant thing). Something like

v Related branches[1]

 ------------------------------------------------------------------------
 | From source packages | Owner |
 ------------------------------------------------------------------------
 | _firefox-4.0_ from the _We're back in the game_ series | TheOwner |

 -----------------------------------------------------------------
 | From product series | Owner |
 -----------------------------------------------------------------
 | _bzr-2.2_ from the _Bazaar 2.2_ series | TheOwner |

That makes (what seems to me to be) the important information more prominent, avoids the repetition, provides a link to the series/packages themselves and allows us to use similar table headings for both tables (having one table list the owner and the other just the source package seemed a bit weird to me, but maybe there's a reason for doing so?).

Also, do you really need the fixed width on the table? Shouldn't the surrounding boxes of the launchpad-form limit the width of the table? Or is this because you want them to have the exact same width as the recipe text area?

[1] notice the different capitalization, as recommended by https://dev.launchpad.net/UserInterfaceWording

review: Needs Information (ui*)
Curtis Hovey (sinzui) wrote :

The column width issue is the same one that the milestone and project packages pages had to solve. User really do need the columns to be the same size and aligned to scan the page. But as Salgado notes, the columns do not have the same content. This is a bit like the related packages problem (see https://launchpad.net/~wgrant/+related-software for packages and ppa). This data issue might not be solvable now, but it would be nice to have the same column headings.

I think Salgado has an interesting point about showing the URL
I suspect such a change will frustrate users who copy the branch URL from the listing. But I wonder. Why is the owner missing from the product URLs?

So I think Salgado is asking the right question. I would rephrase it as "Why isn't the data symmetric?"

Guilherme Salgado (salgado) wrote :

On Tue, 2011-01-25 at 20:07 +0000, Curtis Hovey wrote:
> The column width issue is the same one that the milestone and project
> packages pages had to solve. User really do need the columns to be the
> same size and aligned to scan the page. But as Salgado notes, the
> columns do not have the same content. This is a bit like the related
> packages problem (see https://launchpad.net/~wgrant/+related-software
> for packages and ppa). This data issue might not be solvable now, but
> it would be nice to have the same column headings.
>
> I think Salgado has an interesting point about showing the URL
> I suspect such a change will frustrate users who copy the branch URL
> from the listing. But I wonder. Why is the owner missing from the
> product URLs?

I can see why people would copy the branch URL if they wanted to use
that in the recipe but I was under the impression the main use case here
was just to show stuff that the user might like to know rather than to
aid them in crafting the recipe. That was my understanding after
reading https://bugs.launchpad.net/launchpad/+bug/670452/comments/2 but
maybe I misunderstood or the main use case is to aid people writing
recipes?

Ian Booth (wallyworld) wrote :

> Hi Ian,
>
> This looks like I nice addition, but I wonder if it's necessary to list the
> branch URLs or if it'd be enough to have just the branch's name (linking to
> the branch itself) and the series/package name (also linking to the relevant
> thing). Something like
>
> v Related branches[1]
>
> ------------------------------------------------------------------------
> | From source packages | Owner |
> ------------------------------------------------------------------------
> | _firefox-4.0_ from the _We're back in the game_ series | TheOwner |
>
> -----------------------------------------------------------------
> | From product series | Owner |
> -----------------------------------------------------------------
> | _bzr-2.2_ from the _Bazaar 2.2_ series | TheOwner |
>
> That makes (what seems to me to be) the important information more prominent,
> avoids the repetition, provides a link to the series/packages themselves and
> allows us to use similar table headings for both tables (having one table list
> the owner and the other just the source package seemed a bit weird to me, but
> maybe there's a reason for doing so?).
>
> Also, do you really need the fixed width on the table? Shouldn't the
> surrounding boxes of the launchpad-form limit the width of the table? Or is
> this because you want them to have the exact same width as the recipe text
> area?
>
> [1] notice the different capitalization, as recommended by
> https://dev.launchpad.net/UserInterfaceWording

Thanks for such a detailed comment - really appreciated. The reason for showing the branch urls is to allow the user to explicitly see what the urls are and so to allow simple copy and paste into the recipe - I believe this to be one of the key drivers for this work; when creating a recipe, it's the most error prone bit and the info is not easily intuited unless it is displayed. The branch name is clear from reading the url hence I didn't see the need to show it.

The reason for showing the owner of the product series branches is that it is not obvious from the url who the owner is. The source package column in the source package branches table is to allow easy navigation to the source package home page. These choice were based on implementation discussions when the work was commenced, however I am more than happy to show whatever information is deemed necessary. If we want to continue showing the urls for the reason mentioned above, it's tricky showing too many columns given the urls are so long and they look terrible when wrapped.

Before I added the fixed table width, the tables expanded out to the right hand edge of the browser window and looked terrible - I'm not sure why the form didn't limit the width. Having the tables a similar width to the other main recipe form widgets (the description text area in particular) seems to look good to me but that's only an opinion.

I'll fix the capitalization.

Ian Booth (wallyworld) wrote :

> The column width issue is the same one that the milestone and project packages
> pages had to solve. User really do need the columns to be the same size and
> aligned to scan the page. But as Salgado notes, the columns do not have the
> same content. This is a bit like the related packages problem (see
> https://launchpad.net/~wgrant/+related-software for packages and ppa). This
> data issue might not be solvable now, but it would be nice to have the same
> column headings.
>

Do we want the same styling to be used here as on the related software page listed above? That would have been my preferred way of doing it - bold table heading text etc. I just used the CCS class "listing" for the table style because I thought that was the correct thing to do. It would be good to change the look of my page to match the one referenced.

> I think Salgado has an interesting point about showing the URL
> I suspect such a change will frustrate users who copy the branch URL from the
> listing. But I wonder. Why is the owner missing from the product URLs?
>
> So I think Salgado is asking the right question. I would rephrase it as "Why
> isn't the data symmetric?"

I agree it would be better to have the same column headings etc. The main reason the two tables have different headings is that the branch urls are so long and I didn't want them to wrap and so was limited in how much space could be devoted to adding other columns. Initial implementation discussion resulting in the key info being chosen for display for each branch type and although I could have displayed owner for source package branches (for example), decided not to in order to limit the required table width.

When I didn't limit the table width, the tables came out very wide - see the screenshot from the original implementation: http://people.canonical.com/~ianb/recipe-related-branches.png
The screenshot lacks the vertical space between the tables, and the columns are each very wide and so the tables could easily accommodate more columns with extra info being added, but I think it the ui looks terrible given the tables are so much wider than the other data entry widgets on the recipe form??

If we are happy to have the tables wider, I could look to make the data in each symmetric....

Guilherme Salgado (salgado) wrote :

I think it's good to have the links to the SourcePackage, but we should also have them for the ProductSeries. Maybe that could replace the owner column? Or is the owner really important for some reason?

I thought the text area widgets were constrained by the width of the surrounding elements, but I was wrong; zope's TextAreaWidget uses, by default, a 60-cols width. I'd also prefer if the tables had the same width as the other form elements, but I'm not sure using a fixed width on the table is a good idea because the widget's width might change but most importantly because I don't think the fixed-width table will behave the same way as the textare widget when the window is resized to a smaller size than what's needed to fit the textarea.

Ian Booth (wallyworld) wrote :

> I think it's good to have the links to the SourcePackage, but we should also
> have them for the ProductSeries. Maybe that could replace the owner column?
> Or is the owner really important for some reason?
>

I've added links to the product series instead of owner. I've also made the ui look better by including <h2> headings for each table. These headings and the table headers etc are all styled with the "code" domain css. Hence the headings appear in gold etc. New screenshot: http://people.canonical.com/~ianb/recipe-related-branches2.png

The ordering for the product series branches is based on the sorted_active_series_list property provided by the SortSeriesMixin. This provides only active series with the most recent first and the development focus at the top of the list.

I realised when doing these changes that I was exposing private branches and others the users had no right to see. So I fixed the implementation and added a couple of extra tests.

> I thought the text area widgets were constrained by the width of the
> surrounding elements, but I was wrong; zope's TextAreaWidget uses, by default,
> a 60-cols width. I'd also prefer if the tables had the same width as the
> other form elements, but I'm not sure using a fixed width on the table is a
> good idea because the widget's width might change but most importantly because
> I don't think the fixed-width table will behave the same way as the textare
> widget when the window is resized to a smaller size than what's needed to fit
> the textarea.

Before doing the fixed width tables, I did check the behaviour of the widgets' layouts when the browser window was resized and it all behaved as expected - the text area widget and tables retained their width and a simple horizontal scroll bar was added to the browser window.

What I've done now is to make the <fieldset> element which contains everything for the related branches widget a fixed size and remove the width styling from the tables. The resize behaviour is the same as described above but I think it's a cleaner implementation.

Guilherme Salgado (salgado) wrote :

On Thu, 2011-01-27 at 12:57 +0000, Ian Booth wrote:
[...]
> > I thought the text area widgets were constrained by the width of the
> > surrounding elements, but I was wrong; zope's TextAreaWidget uses, by default,
> > a 60-cols width. I'd also prefer if the tables had the same width as the
> > other form elements, but I'm not sure using a fixed width on the table is a
> > good idea because the widget's width might change but most importantly because
> > I don't think the fixed-width table will behave the same way as the textare
> > widget when the window is resized to a smaller size than what's needed to fit
> > the textarea.
>
> Before doing the fixed width tables, I did check the behaviour of the
> widgets' layouts when the browser window was resized and it all
> behaved as expected - the text area widget and tables retained their
> width and a simple horizontal scroll bar was added to the browser
> window.

I thought the number of columns on a textarea wouldn't prevent the
browser from resizing it when the window is resized. That's the
behaviour I see with the comment box (which specifies cols=60) on a
merge-proposal page, but I tested here on a html file I crafted and the
browser didn't resize my textarea. Maybe Curtis will know why we're
seeing a different behaviour on the merge-proposal page.

>
> What I've done now is to make the <fieldset> element which contains
> everything for the related branches widget a fixed size and remove the
> width styling from the tables. The resize behaviour is the same as
> described above but I think it's a cleaner implementation.

Do you really think we should go with the fixed width, even with all its
drawbacks?

Ian Booth (wallyworld) wrote :

>
>>
>> What I've done now is to make the <fieldset> element which contains
>> everything for the related branches widget a fixed size and remove the
>> width styling from the tables. The resize behaviour is the same as
>> described above but I think it's a cleaner implementation.
>
> Do you really think we should go with the fixed width, even with all its
> drawbacks?
>

Without the fixed width, the tables expand right out to the edge of the
browser window, and on a screen with the browser maximised for example,
it looks really bad (to my eye) when all the form elements are around
the same width (approx size of text area widget) and just the related
branches tables are really wide. In my view, it's better visually to try
and maintain a similar width for all the form widgets and a consistent
resize behaviour ie if the text area widget is fixed, the the tables
(via the width styling on the fieldset element) should be too. When the
tables are really wide without the fixed width, the first column expands
way out and there's a lot of empty space and your eye is forced to scan
all the way to the right to see the 2nd table column. Do you agree with
this view? I'm not an css expert so I'd love to hear about any issues
with using the current fixed width approach that outweigh the positive
benefits outlined above.

Guilherme Salgado (salgado) wrote :

On Thu, 2011-01-27 at 14:28 +0000, Ian Booth wrote:
> >
> >>
> >> What I've done now is to make the <fieldset> element which contains
> >> everything for the related branches widget a fixed size and remove the
> >> width styling from the tables. The resize behaviour is the same as
> >> described above but I think it's a cleaner implementation.
> >
> > Do you really think we should go with the fixed width, even with all its
> > drawbacks?
> >
>
> Without the fixed width, the tables expand right out to the edge of the
> browser window, and on a screen with the browser maximised for example,
> it looks really bad (to my eye) when all the form elements are around
> the same width (approx size of text area widget) and just the related
> branches tables are really wide. In my view, it's better visually to try
> and maintain a similar width for all the form widgets and a consistent
> resize behaviour ie if the text area widget is fixed, the the tables
> (via the width styling on the fieldset element) should be too. When the
> tables are really wide without the fixed width, the first column expands
> way out and there's a lot of empty space and your eye is forced to scan
> all the way to the right to see the 2nd table column. Do you agree with
> this view? I'm not an css expert so I'd love to hear about any issues
> with using the current fixed width approach that outweigh the positive
> benefits outlined above.

Well, the main issue is that there's no way to make sure the table's
width is kept in sync with the widget's sync. You have a good point
that it will look really bad on wide windows, so I think you're right
that it's better to use the fixed width for now.

Tim Penhey (thumper) wrote :

The implementation of the methods need to be put into the BranchTarget implementation classes.

review: Needs Fixing
Ian Booth (wallyworld) wrote :

Code changes made to move core model functionality to the IBranchTarget implementation. The View calls through to this class to get the required data. Separate tests written for IBranchTarget. The ordering of related branches is such the those for the most recent product series or distro series appear first. The sorting was done using sorted_version_numbers()

The related package branch listing now shows the distro series instead of the source package. I would appreciate a careful review of the algorithm used to load the related branches (especially the related package branches) to ensure everything is correct.

One question - I don't like it how the related branches tables are inside the form but I think it looks weird if they are moved outside the form and hence appear below the Update Recipe button. I'll do whatever is deemed best.

Ian Booth (wallyworld) wrote :

I added functionality to limit the number of related branches displayed to 5 in each category. See screenshot for example of how it looks:

http://people.canonical.com/~ianb/recipe-related-branches3.png

The branches shown are the most recent according to the sorted_version_numbers() function:
- for related series, branches, the ordering is applied to the product series, and the devel focus is always shown
- for package branches, the ordering is applied to the distro series

Tim Penhey (thumper) wrote :

On Mon, 31 Jan 2011 15:18:53 you wrote:
> I added functionality to limit the number of related branches displayed to
> 5 in each category. See screenshot for example of how it looks:
>
> http://people.canonical.com/~ianb/recipe-related-branches3.png
>
> The branches shown are the most recent according to the
> sorted_version_numbers() function: - for related series, branches, the
> ordering is applied to the product series, and the devel focus is always
> shown - for package branches, the ordering is applied to the distro series

I don't think it is beneficial to have the text: "Top 5 most recent
branches...".

Ian Booth (wallyworld) wrote :

I think that if we limit the results, the user should be informed, lest
they get confused as to why a branch they may have been expecting to be
there is not. But that's just my opinion as a general ui priciple. If
you feel it's better to remove the text, that's no problem. Or would a
rewording of the text be better?

On 31/01/11 12:28, Tim Penhey wrote:
> On Mon, 31 Jan 2011 15:18:53 you wrote:
>> I added functionality to limit the number of related branches displayed to
>> 5 in each category. See screenshot for example of how it looks:
>>
>> http://people.canonical.com/~ianb/recipe-related-branches3.png
>>
>> The branches shown are the most recent according to the
>> sorted_version_numbers() function: - for related series, branches, the
>> ordering is applied to the product series, and the devel focus is always
>> shown - for package branches, the ordering is applied to the distro series
>
> I don't think it is beneficial to have the text: "Top 5 most recent
> branches...".
>

Curtis Hovey (sinzui) wrote :

This is good to land.

review: Approve (ui)
Steve Kowalik (stevenk) wrote :

This looks okay to me, aside from a few blank lines that look to have crept -- however, if make lint disagrees with me, then I'll let that slide.

Why are the imports for lp.testing.factory right under the docstring rather than being ordered with the rest of them?

I'd suggest running utilites/format-imports over the files you changed.

review: Approve (code*)
Ian Booth (wallyworld) wrote :

On 02/02/11 10:43, Steve Kowalik wrote:
> Review: Approve code*
> This looks okay to me, aside from a few blank lines that look to have crept -- however, if make lint disagrees with me, then I'll let that slide.
>

Lint is happy - just complains about lines too long which were already
there, related to string literals in some tests.

> Why are the imports for lp.testing.factory right under the docstring rather than being ordered with the rest of them?
>

Cause I need my eyesight checked. Fixed.

> I'd suggest running utilites/format-imports over the files you changed.

Tim Penhey (thumper) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
2--- lib/lp/code/browser/sourcepackagerecipe.py 2011-01-28 01:15:05 +0000
3+++ lib/lp/code/browser/sourcepackagerecipe.py 2011-02-02 01:11:03 +0000
4@@ -14,7 +14,6 @@
5 'SourcePackageRecipeView',
6 ]
7
8-
9 from bzrlib.plugins.builder.recipe import (
10 ForbiddenInstructionError,
11 RecipeParseError,
12@@ -24,6 +23,9 @@
13 from lazr.lifecycle.snapshot import Snapshot
14 from lazr.restful.interface import use_template
15 from storm.locals import Store
16+from z3c.ptcompat import ViewPageTemplateFile
17+from zope.app.form.browser.widget import Widget
18+from zope.app.form.interfaces import IView
19 from zope.component import getUtility
20 from zope.event import notify
21 from zope.formlib import form
22@@ -33,6 +35,7 @@
23 providedBy,
24 )
25 from zope.schema import (
26+ Field,
27 Choice,
28 List,
29 Text,
30@@ -79,12 +82,14 @@
31 PrivateBranchRecipe,
32 TooNewRecipeFormat,
33 )
34+from lp.code.interfaces.branchtarget import IBranchTarget
35 from lp.code.interfaces.sourcepackagerecipe import (
36 ISourcePackageRecipe,
37 ISourcePackageRecipeSource,
38 MINIMAL_RECIPE_TEXT,
39 )
40 from lp.registry.interfaces.pocket import PackagePublishingPocket
41+from lp.services.propertycache import cachedproperty
42 from lp.soyuz.model.archive import Archive
43
44
45@@ -394,7 +399,67 @@
46 self.setFieldError('recipe_text', str(error))
47
48
49-class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
50+class RelatedBranchesWidget(Widget):
51+ """A widget to render the related branches for a recipe."""
52+ implements(IView)
53+
54+ __call__ = ViewPageTemplateFile(
55+ '../templates/sourcepackagerecipe-related-branches.pt')
56+
57+ related_package_branch_info = []
58+ related_series_branch_info = []
59+
60+ def hasInput(self):
61+ return True
62+
63+ def setRenderedValue(self, value):
64+ self.related_package_branch_info = (
65+ value['related_package_branch_info'])
66+ self.related_series_branch_info = value['related_series_branch_info']
67+
68+
69+class RecipeRelatedBranchesMixin(LaunchpadFormView):
70+ """A class to find related branches for a recipe's base branch."""
71+
72+ custom_widget('related-branches', RelatedBranchesWidget)
73+
74+ def extendFields(self):
75+ """See `LaunchpadFormView`.
76+
77+ Adds a related branches field to the form.
78+ """
79+ self.form_fields += form.Fields(Field(__name__='related-branches'))
80+ self.form_fields['related-branches'].custom_widget = (
81+ self.custom_widgets['related-branches'])
82+ self.widget_errors['related-branches'] = ''
83+
84+ def setUpWidgets(self, context=None):
85+ # Adds a new related branches widget.
86+ super(RecipeRelatedBranchesMixin, self).setUpWidgets(context)
87+ self.widgets['related-branches'].display_label = False
88+ self.widgets['related-branches'].setRenderedValue(dict(
89+ related_package_branch_info=self.related_package_branch_info,
90+ related_series_branch_info=self.related_series_branch_info))
91+
92+ @cachedproperty
93+ def related_series_branch_info(self):
94+ branch_to_check = self.getBranch()
95+ return IBranchTarget(
96+ branch_to_check.target).getRelatedSeriesBranchInfo(
97+ branch_to_check,
98+ limit_results=5)
99+
100+ @cachedproperty
101+ def related_package_branch_info(self):
102+ branch_to_check = self.getBranch()
103+ return IBranchTarget(
104+ branch_to_check.target).getRelatedPackageBranchInfo(
105+ branch_to_check,
106+ limit_results=5)
107+
108+
109+class SourcePackageRecipeAddView(RecipeRelatedBranchesMixin,
110+ RecipeTextValidatorMixin, LaunchpadFormView):
111 """View for creating Source Package Recipes."""
112
113 title = label = 'Create a new source package recipe'
114@@ -424,6 +489,10 @@
115 # all confused when we want to create a new PPA.
116 archive_widget._displayItemForMissingValue = False
117
118+ def getBranch(self):
119+ """The branch on which the recipe is built."""
120+ return self.context
121+
122 @property
123 def initial_values(self):
124 return {
125@@ -495,10 +564,15 @@
126 self.setFieldError('ppa_name', error)
127
128
129-class SourcePackageRecipeEditView(RecipeTextValidatorMixin,
130+class SourcePackageRecipeEditView(RecipeRelatedBranchesMixin,
131+ RecipeTextValidatorMixin,
132 LaunchpadEditFormView):
133 """View for editing Source Package Recipes."""
134
135+ def getBranch(self):
136+ """The branch on which the recipe is built."""
137+ return self.context.base_branch
138+
139 @property
140 def title(self):
141 return 'Edit %s source package recipe' % self.context.name
142
143=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
144--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2011-01-21 16:52:57 +0000
145+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2011-02-02 01:11:03 +0000
146@@ -7,6 +7,7 @@
147 __metaclass__ = type
148
149
150+from BeautifulSoup import BeautifulSoup
151 from datetime import (
152 datetime,
153 timedelta,
154@@ -29,6 +30,7 @@
155 get_radio_button_text_for_field,
156 )
157 from canonical.launchpad.webapp import canonical_url
158+from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
159 from canonical.testing.layers import (
160 DatabaseFunctionalLayer,
161 LaunchpadFunctionalLayer,
162@@ -103,6 +105,81 @@
163 ' Secret Squirrel changes.', branches=[cake_branch],
164 daily_build_archive=self.ppa)
165
166+ def checkRelatedBranches(self, related_series_branch_info,
167+ related_package_branch_info, browser_contents):
168+ """Check that the browser contents contain the correct branch info."""
169+ login(ANONYMOUS)
170+ soup = BeautifulSoup(browser_contents)
171+
172+ # The related branches collapsible section needs to be there.
173+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
174+ self.assertIsNot(related_branches, None)
175+
176+ # Check the related package branches.
177+ root_url = canonical_url(
178+ getUtility(ILaunchpadRoot), rootsite='code')
179+ root_url = root_url.rstrip('/')
180+ branch_table = soup.find(
181+ 'table', {'id': 'related-package-branches-listing'})
182+ if not related_package_branch_info:
183+ self.assertIs(branch_table, None)
184+ else:
185+ rows = branch_table.tbody.findAll('tr')
186+
187+ package_branches_info = []
188+ for row in rows:
189+ branch_links = row.findAll('a')
190+ self.assertEqual(2, len(branch_links))
191+ package_branches_info.append(
192+ '%s%s' % (root_url, branch_links[0]['href']))
193+ package_branches_info.append(branch_links[0].renderContents())
194+ package_branches_info.append(
195+ '%s%s' % (root_url, branch_links[1]['href']))
196+ package_branches_info.append(branch_links[1].renderContents())
197+ expected_branch_info = []
198+ for branch_info in related_package_branch_info:
199+ branch = branch_info[0]
200+ distro_series = branch_info[1]
201+ expected_branch_info.append(
202+ canonical_url(branch, rootsite='code'))
203+ expected_branch_info.append(branch.displayname)
204+ expected_branch_info.append(
205+ canonical_url(distro_series, rootsite='code'))
206+ expected_branch_info.append(distro_series.name)
207+ self.assertEqual(package_branches_info, expected_branch_info)
208+
209+ # Check the related series branches.
210+ branch_table = soup.find(
211+ 'table', {'id': 'related-series-branches-listing'})
212+ if not related_series_branch_info:
213+ self.assertIs(branch_table, None)
214+ else:
215+ rows = branch_table.tbody.findAll('tr')
216+
217+ series_branches_info = []
218+ for row in rows:
219+ branch_links = row.findAll('a')
220+ self.assertEqual(2, len(branch_links))
221+ series_branches_info.append(
222+ '%s%s' % (root_url, branch_links[0]['href']))
223+ series_branches_info.append(branch_links[0].renderContents())
224+ series_branches_info.append(branch_links[1]['href'])
225+ series_branches_info.append(branch_links[1].renderContents())
226+ expected_branch_info = []
227+ for branch_info in related_series_branch_info:
228+ branch = branch_info[0]
229+ product_series = branch_info[1]
230+ expected_branch_info.append(
231+ canonical_url(branch,
232+ rootsite='code',
233+ path_only_if_possible=True))
234+ expected_branch_info.append(branch.displayname)
235+ expected_branch_info.append(
236+ canonical_url(product_series,
237+ path_only_if_possible=True))
238+ expected_branch_info.append(product_series.name)
239+ self.assertEqual(expected_branch_info, series_branches_info)
240+
241
242 def get_message_text(browser, index):
243 """Return the text of a message, specified by index."""
244@@ -353,8 +430,8 @@
245 browser.getControl('Automatically build each day').click()
246 browser.getControl('Create Recipe').click()
247 self.assertEqual(
248- extract_text(find_tags_by_class(browser.contents, 'message')[2]),
249- 'You must specify at least one series for daily builds.')
250+ 'You must specify at least one series for daily builds.',
251+ get_message_text(browser, 2))
252
253 def test_create_recipe_bad_base_branch(self):
254 # If a user tries to create source package recipe with a bad base
255@@ -431,6 +508,81 @@
256 get_message_text(browser, 2),
257 'Recipe may not refer to private branch: %s' % bzr_identity)
258
259+ def _test_new_recipe_with_no_related_branches(self, branch):
260+ # The Related Branches section should not appear if there are no
261+ # related branches.
262+ # A new recipe can be created from the branch page.
263+ browser = self.getUserBrowser(
264+ canonical_url(branch, view_name='+new-recipe'), user=self.chef)
265+ # There shouldn't be a related-branches section if there are no
266+ # related branches..
267+ soup = BeautifulSoup(browser.contents)
268+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
269+ self.assertIs(related_branches, None)
270+
271+ def test_new_product_branch_with_no_related_branches_recipe(self):
272+ # We can create a new recipe off a product branch.
273+ branch = self.factory.makeBranch()
274+ self._test_new_recipe_with_no_related_branches(branch)
275+
276+ def test_new_package_branch_with_no_linked_branches_recipe(self):
277+ # We can create a new recipe off a sourcepackage branch where the
278+ # sourcepackage has no linked branches.
279+ branch = self.factory.makePackageBranch()
280+ self._test_new_recipe_with_no_related_branches(branch)
281+
282+ def test_new_recipe_with_package_branches(self):
283+ # The series branches table should not appear if there are none.
284+ (branch, related_series_branch_info, related_package_branches) = (
285+ self.factory.makeRelatedBranches(with_series_branches=False))
286+ browser = self.getUserBrowser(
287+ canonical_url(branch, view_name='+new-recipe'), user=self.chef)
288+ soup = BeautifulSoup(browser.contents)
289+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
290+ self.assertIsNot(related_branches, None)
291+ related_branches = soup.find(
292+ 'div', {'id': 'related-package-branches'})
293+ self.assertIsNot(related_branches, None)
294+ related_branches = soup.find(
295+ 'div', {'id': 'related-series-branches'})
296+ self.assertIs(related_branches, None)
297+
298+ def test_new_recipe_with_series_branches(self):
299+ # The package branches table should not appear if there are none.
300+ (branch, related_series_branch_info, related_package_branches) = (
301+ self.factory.makeRelatedBranches(with_package_branches=False))
302+ browser = self.getUserBrowser(
303+ canonical_url(branch, view_name='+new-recipe'), user=self.chef)
304+ soup = BeautifulSoup(browser.contents)
305+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
306+ self.assertIsNot(related_branches, None)
307+ related_branches = soup.find(
308+ 'div', {'id': 'related-series-branches'})
309+ self.assertIsNot(related_branches, None)
310+ related_branches = soup.find(
311+ 'div', {'id': 'related-package-branches'})
312+ self.assertIs(related_branches, None)
313+
314+ def test_new_product_branch_recipe_with_related_branches(self):
315+ # The related branches should be rendered correctly on the page.
316+ (branch, related_series_branch_info,
317+ related_package_branch_info) = self.factory.makeRelatedBranches()
318+ browser = self.getUserBrowser(
319+ canonical_url(branch, view_name='+new-recipe'), user=self.chef)
320+ self.checkRelatedBranches(
321+ related_series_branch_info, related_package_branch_info,
322+ browser.contents)
323+
324+ def test_new_sourcepackage_branch_recipe_with_related_branches(self):
325+ # The related branches should be rendered correctly on the page.
326+ reference_branch= self.factory.makePackageBranch()
327+ (branch, ignore, related_package_branch_info) = (
328+ self.factory.makeRelatedBranches(reference_branch))
329+ browser = self.getUserBrowser(
330+ canonical_url(branch, view_name='+new-recipe'), user=self.chef)
331+ self.checkRelatedBranches(
332+ set(), related_package_branch_info, browser.contents)
333+
334 def test_ppa_selector_not_shown_if_user_has_no_ppas(self):
335 # If the user creating a recipe has no existing PPAs, the selector
336 # isn't shown, but the field to enter a new PPA name is.
337@@ -834,6 +986,98 @@
338 get_message_text(browser, 1),
339 'Recipe may not refer to private branch: %s' % bzr_identity)
340
341+ def _test_edit_recipe_with_no_related_branches(self, recipe):
342+ # The Related Branches section should not appear if there are no
343+ # related branches.
344+ browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
345+ browser.getLink('Edit recipe').click()
346+ # There shouldn't be a related-branches section if there are no
347+ # related branches.
348+ soup = BeautifulSoup(browser.contents)
349+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
350+ self.assertIs(related_branches, None)
351+
352+ def test_edit_product_branch_with_no_related_branches_recipe(self):
353+ # The Related Branches section should not appear if there are no
354+ # related branches.
355+ base_branch = self.factory.makeBranch()
356+ recipe = self.factory.makeSourcePackageRecipe(
357+ owner=self.chef, branches=[base_branch])
358+ self._test_edit_recipe_with_no_related_branches(recipe)
359+
360+ def test_edit_sourcepackage_branch_with_no_related_branches_recipe(self):
361+ # The Related Branches section should not appear if there are no
362+ # related branches.
363+ base_branch = self.factory.makePackageBranch()
364+ recipe = self.factory.makeSourcePackageRecipe(
365+ owner=self.chef, branches=[base_branch])
366+ self._test_edit_recipe_with_no_related_branches(recipe)
367+
368+ def test_edit_recipe_with_package_branches(self):
369+ # The series branches table should not appear if there are none.
370+ with person_logged_in(self.chef):
371+ recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
372+ self.factory.makeRelatedBranches(
373+ reference_branch=recipe.base_branch,
374+ with_series_branches=False)
375+ browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
376+ browser.getLink('Edit recipe').click()
377+ soup = BeautifulSoup(browser.contents)
378+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
379+ self.assertIsNot(related_branches, None)
380+ related_branches = soup.find(
381+ 'div', {'id': 'related-package-branches'})
382+ self.assertIsNot(related_branches, None)
383+ related_branches = soup.find(
384+ 'div', {'id': 'related-series-branches'})
385+ self.assertIs(related_branches, None)
386+
387+ def test_edit_recipe_with_series_branches(self):
388+ # The package branches table should not appear if there are none.
389+ with person_logged_in(self.chef):
390+ recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
391+ self.factory.makeRelatedBranches(
392+ reference_branch=recipe.base_branch,
393+ with_package_branches=False)
394+ browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
395+ browser.getLink('Edit recipe').click()
396+ soup = BeautifulSoup(browser.contents)
397+ related_branches = soup.find('fieldset', {'id': 'related-branches'})
398+ self.assertIsNot(related_branches, None)
399+ related_branches = soup.find(
400+ 'div', {'id': 'related-series-branches'})
401+ self.assertIsNot(related_branches, None)
402+ related_branches = soup.find(
403+ 'div', {'id': 'related-package-branches'})
404+ self.assertIs(related_branches, None)
405+
406+ def test_edit_product_branch_recipe_with_related_branches(self):
407+ # The related branches should be rendered correctly on the page.
408+ with person_logged_in(self.chef):
409+ recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
410+ (branch, related_series_branch_info,
411+ related_package_branch_info) = (
412+ self.factory.makeRelatedBranches(
413+ reference_branch=recipe.base_branch))
414+ browser = self.getUserBrowser(
415+ canonical_url(recipe, view_name='+edit'), user=self.chef)
416+ self.checkRelatedBranches(
417+ related_series_branch_info, related_package_branch_info,
418+ browser.contents)
419+
420+ def test_edit_sourcepackage_branch_recipe_with_related_branches(self):
421+ # The related branches should be rendered correctly on the page.
422+ with person_logged_in(self.chef):
423+ reference_branch= self.factory.makePackageBranch()
424+ recipe = self.factory.makeSourcePackageRecipe(
425+ owner=self.chef, branches=[reference_branch])
426+ (branch, ignore, related_package_branch_info) = (
427+ self.factory.makeRelatedBranches(reference_branch))
428+ browser = self.getUserBrowser(
429+ canonical_url(recipe, view_name='+edit'), user=self.chef)
430+ self.checkRelatedBranches(
431+ set(), related_package_branch_info, browser.contents)
432+
433
434 class TestSourcePackageRecipeView(TestCaseForRecipe):
435
436
437=== modified file 'lib/lp/code/interfaces/branchtarget.py'
438--- lib/lp/code/interfaces/branchtarget.py 2010-08-20 20:31:18 +0000
439+++ lib/lp/code/interfaces/branchtarget.py 2011-02-02 01:11:03 +0000
440@@ -26,7 +26,6 @@
441 from zope.security.interfaces import Unauthorized
442
443 from canonical.launchpad import _
444-from canonical.launchpad.webapp.interfaces import IPrimaryContext
445 from lp.code.enums import BranchType
446
447
448@@ -143,3 +142,33 @@
449 :returns: an `ICodeImport`.
450 :raises AssertionError: if supports_code_imports is False.
451 """
452+
453+ def getRelatedSeriesBranchInfo(parent_branch, limit_results=None):
454+ """Find development branch info related to this parent branch.
455+
456+ The result is a list of tuples:
457+ (branch, product_series)
458+ where:
459+ branch: the related branch.
460+ product_series: the product series associated with the branch.
461+
462+ The development focus is first in the list.
463+
464+ :param parent_branch: `IBranch` we are finding related branches for.
465+ :param limit_results: if not None, limit the number of results to the
466+ specified value.
467+ """
468+
469+ def getRelatedPackageBranchInfo(parent_branch, limit_results=None):
470+ """Find package branch info related to this parent branch.
471+
472+ The result is a list of tuples:
473+ (branch, distro_series)
474+ where:
475+ branch: the related branch.
476+ distro_series: the distro series associated with the branch.
477+
478+ :param parent_branch: `IBranch` we are finding related branches for.
479+ :param limit_results: if not None, limit the number of results to the
480+ specified value.
481+ """
482
483=== modified file 'lib/lp/code/model/branchtarget.py'
484--- lib/lp/code/model/branchtarget.py 2010-08-20 20:31:18 +0000
485+++ lib/lp/code/model/branchtarget.py 2011-02-02 01:11:03 +0000
486@@ -11,18 +11,25 @@
487 'ProductBranchTarget',
488 ]
489
490+from operator import attrgetter
491+
492 from zope.component import getUtility
493 from zope.interface import implements
494 from zope.security.proxy import isinstance as zope_isinstance
495
496+from canonical.launchpad.webapp.authorization import check_permission
497 from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
498+from canonical.launchpad.webapp.sorting import sorted_version_numbers
499+from lp.code.errors import NoLinkedBranch
500 from lp.code.interfaces.branchcollection import IAllBranches
501 from lp.code.interfaces.branchtarget import (
502 check_default_stacked_on,
503 IBranchTarget,
504 )
505 from lp.code.interfaces.codeimport import ICodeImportSet
506+from lp.code.interfaces.linkedbranch import get_linked_to_branch
507 from lp.registry.interfaces.pocket import PackagePublishingPocket
508+from lp.registry.interfaces.series import SeriesStatus
509
510
511 def branch_to_target(branch):
512@@ -45,6 +52,14 @@
513 registrant, self, branch_name, rcs_type, url=url,
514 cvs_root=cvs_root, cvs_module=cvs_module, owner=owner)
515
516+ def getRelatedSeriesBranchInfo(self, parent_branch, limit_results=None):
517+ """See `IBranchTarget`."""
518+ return []
519+
520+ def getRelatedPackageBranchInfo(self, parent_branch, limit_results=None):
521+ """See `IBranchTarget`."""
522+ return []
523+
524
525 class PackageBranchTarget(_BaseBranchTarget):
526 implements(IBranchTarget)
527@@ -159,6 +174,26 @@
528 branch.distroseries = self.sourcepackage.distroseries
529 branch.sourcepackagename = self.sourcepackage.sourcepackagename
530
531+ def getRelatedPackageBranchInfo(self, parent_branch, limit_results=None):
532+ """See `IBranchTarget`."""
533+ result = []
534+ linked_branches = self.sourcepackage.linkedBranches()
535+ distroseries = self.sourcepackage.distroseries
536+ result.extend(
537+ [(branch, distroseries)
538+ for branch in linked_branches.values()
539+ if (check_permission('launchpad.View', branch) and
540+ branch != parent_branch and
541+ distroseries.status != SeriesStatus.OBSOLETE)])
542+
543+ result = sorted_version_numbers(result, key=lambda branch_info: (
544+ getattr(branch_info[1], 'name')))
545+
546+ if limit_results is not None:
547+ # We only want the most recent branches
548+ result = result[:limit_results]
549+ return result
550+
551
552 class PersonBranchTarget(_BaseBranchTarget):
553 implements(IBranchTarget)
554@@ -338,6 +373,58 @@
555 branch.distroseries = None
556 branch.sourcepackagename = None
557
558+ def getRelatedSeriesBranchInfo(self, parent_branch, limit_results=None):
559+ """See `IBranchTarget`."""
560+ sorted_series = []
561+ for series in self.product.series:
562+ if (series.status != SeriesStatus.OBSOLETE
563+ and series != self.product.development_focus):
564+ sorted_series.append(series)
565+ # Now sort the list by name with newer versions before older.
566+ sorted_series = sorted_version_numbers(sorted_series,
567+ key=attrgetter('name'))
568+ # Add the development focus first.
569+ sorted_series.insert(0, self.product.development_focus)
570+
571+ result = []
572+ for series in sorted_series:
573+ try:
574+ branch = get_linked_to_branch(series).branch
575+ if (branch not in result and branch != parent_branch and
576+ check_permission('launchpad.View', branch)):
577+ result.append((branch, series))
578+ except NoLinkedBranch:
579+ # If there's no branch for a particular series, we don't care.
580+ pass
581+
582+ if limit_results is not None:
583+ # We only want the most recent branches
584+ result = result[:limit_results]
585+ return result
586+
587+ def getRelatedPackageBranchInfo(self, parent_branch, limit_results=None):
588+ """See `IBranchTarget`."""
589+ result = []
590+ for distrosourcepackage in self.product.distrosourcepackages:
591+ try:
592+ branch = get_linked_to_branch(distrosourcepackage).branch
593+ series = distrosourcepackage.distribution.currentseries
594+ if (branch != parent_branch and series is not None and
595+ check_permission('launchpad.View', branch)):
596+ result.append((branch, series))
597+ except NoLinkedBranch:
598+ # If there's no branch for a particular source package,
599+ # we don't care.
600+ pass
601+
602+ result = sorted_version_numbers(result, key=lambda branch_info: (
603+ getattr(branch_info[1], 'name')))
604+
605+ if limit_results is not None:
606+ # We only want the most recent branches
607+ result = result[:limit_results]
608+ return result
609+
610
611 def get_canonical_url_data_for_target(branch_target):
612 """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
613@@ -348,6 +435,7 @@
614 """The Product itself is the branch target given a ProductSeries."""
615 return ProductBranchTarget(product_series.product)
616
617+
618 def distribution_sourcepackage_to_branch_target(distro_sourcepackage):
619 """The development version of the distro sourcepackage is the target."""
620 dev_version = distro_sourcepackage.development_version
621
622=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
623--- lib/lp/code/model/tests/test_branchtarget.py 2010-10-04 19:50:45 +0000
624+++ lib/lp/code/model/tests/test_branchtarget.py 2011-02-02 01:11:03 +0000
625@@ -221,6 +221,30 @@
626 self.assertEqual(owner, code_import.branch.owner)
627 self.assertEqual(self.target, code_import.branch.target)
628
629+ def test_related_branches(self):
630+ (branch, related_series_branch_info,
631+ related_package_branches) = (
632+ self.factory.makeRelatedBranchesForSourcePackage(
633+ sourcepackage=self.original))
634+ self.assertEqual(
635+ related_series_branch_info,
636+ self.target.getRelatedSeriesBranchInfo(branch))
637+ self.assertEqual(
638+ related_package_branches,
639+ self.target.getRelatedPackageBranchInfo(branch))
640+
641+ def test_related_branches_with_private_branch(self):
642+ (branch, related_series_branch_info,
643+ related_package_branches) = (
644+ self.factory.makeRelatedBranchesForSourcePackage(
645+ sourcepackage=self.original, with_private_branches=True))
646+ self.assertEqual(
647+ related_series_branch_info,
648+ self.target.getRelatedSeriesBranchInfo(branch))
649+ self.assertEqual(
650+ related_package_branches,
651+ self.target.getRelatedPackageBranchInfo(branch))
652+
653
654 class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
655
656@@ -452,6 +476,42 @@
657 self.assertEqual(owner, code_import.branch.owner)
658 self.assertEqual(self.target, code_import.branch.target)
659
660+ def test_related_branches(self):
661+ (branch, related_series_branch_info,
662+ related_package_branches) = (
663+ self.factory.makeRelatedBranchesForProduct(
664+ product=self.original))
665+ self.assertEqual(
666+ related_series_branch_info,
667+ self.target.getRelatedSeriesBranchInfo(branch))
668+ self.assertEqual(
669+ related_package_branches,
670+ self.target.getRelatedPackageBranchInfo(branch))
671+
672+ def test_related_branches_with_private_branch(self):
673+ (branch, related_series_branch_info,
674+ related_package_branches) = (
675+ self.factory.makeRelatedBranchesForProduct(
676+ product=self.original, with_private_branches=True))
677+ self.assertEqual(
678+ related_series_branch_info,
679+ self.target.getRelatedSeriesBranchInfo(branch))
680+ self.assertEqual(
681+ related_package_branches,
682+ self.target.getRelatedPackageBranchInfo(branch))
683+
684+ def test_related_branches_with_limit(self):
685+ (branch, related_series_branch_info,
686+ related_package_branches) = (
687+ self.factory.makeRelatedBranchesForProduct(
688+ product=self.original))
689+ self.assertEqual(
690+ related_series_branch_info[:2],
691+ self.target.getRelatedSeriesBranchInfo(branch, 2))
692+ self.assertEqual(
693+ related_package_branches[:2],
694+ self.target.getRelatedPackageBranchInfo(branch, 2))
695+
696
697 class TestCheckDefaultStackedOnBranch(TestCaseWithFactory):
698 """Only certain branches are allowed to be default stacked-on branches."""
699
700=== modified file 'lib/lp/code/templates/sourcepackagerecipe-new.pt'
701--- lib/lp/code/templates/sourcepackagerecipe-new.pt 2010-12-22 02:01:36 +0000
702+++ lib/lp/code/templates/sourcepackagerecipe-new.pt 2011-02-02 01:11:03 +0000
703@@ -104,6 +104,9 @@
704 <tal:widget define="widget nocall:view/widgets/recipe_text">
705 <metal:block use-macro="context/@@launchpad_form/widget_row" />
706 </tal:widget>
707+ <tal:widget define="widget nocall:view/widgets/related-branches">
708+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
709+ </tal:widget>
710
711 </table>
712 </metal:formbody>
713
714=== added file 'lib/lp/code/templates/sourcepackagerecipe-related-branches.pt'
715--- lib/lp/code/templates/sourcepackagerecipe-related-branches.pt 1970-01-01 00:00:00 +0000
716+++ lib/lp/code/templates/sourcepackagerecipe-related-branches.pt 2011-02-02 01:11:03 +0000
717@@ -0,0 +1,75 @@
718+<fieldset id="related-branches"
719+ class="collapsible collapsed" style="width: 60em"
720+ tal:define="seriesBranches view/related_series_branch_info;
721+ packageBranches view/related_package_branch_info"
722+ tal:condition="python: seriesBranches or packageBranches">
723+ <legend>Related Branches</legend>
724+ <table>
725+
726+ <div tal:condition="packageBranches" id="related-package-branches">
727+ <h2>Source package branches</h2>
728+ <table class="listing" id="related-package-branches-listing">
729+ <thead>
730+ <tr>
731+ <th>Branch URL</th>
732+ <th>Distro series</th>
733+ </tr>
734+ </thead>
735+ <tbody>
736+ <tr tal:repeat="branch_info packageBranches">
737+ <tal:defines define="branch python:branch_info[0];
738+ distro_series python:branch_info[1];">
739+ <td width="80%">
740+ <a href="#"
741+ tal:content="branch/displayname"
742+ tal:attributes="href branch/fmt:url">
743+ source package branch
744+ </a>
745+ </td>
746+ <td>
747+ <a href="#"
748+ tal:attributes="href distro_series/fmt:url"
749+ tal:content="distro_series/name">
750+ distro series
751+ </a>
752+ </td>
753+ </tal:defines>
754+ </tr>
755+ </tbody>
756+ </table>
757+ </div>
758+
759+ <div tal:condition="seriesBranches" id="related-series-branches">
760+ <h2 style="margin-top: 1.5em;">Product series branches</h2>
761+ <table class="listing" id="related-series-branches-listing">
762+ <thead>
763+ <tr>
764+ <th>Branch URL</th>
765+ <th>Product series</th>
766+ </tr>
767+ </thead>
768+ <tbody>
769+ <tr tal:repeat="branch_info seriesBranches">
770+ <tal:defines define="branch python:branch_info[0];
771+ product_series python:branch_info[1];">
772+ <td width="80%">
773+ <a href="#"
774+ tal:content="branch/displayname"
775+ tal:attributes="href branch/fmt:url">
776+ product series branch
777+ </a>
778+ </td>
779+ <td>
780+ <a href="#"
781+ tal:content="product_series/name"
782+ tal:attributes="href product_series/fmt:url">
783+ product series
784+ </a>
785+ </td>
786+ </tal:defines>
787+ </tr>
788+ </tbody>
789+ </table>
790+ </div>
791+ </table>
792+</fieldset>
793
794=== modified file 'lib/lp/testing/factory.py'
795--- lib/lp/testing/factory.py 2011-01-31 18:00:50 +0000
796+++ lib/lp/testing/factory.py 2011-02-02 01:11:03 +0000
797@@ -100,6 +100,7 @@
798 MAIN_STORE,
799 OAuthPermission,
800 )
801+from canonical.launchpad.webapp.sorting import sorted_version_numbers
802 from lp.app.enums import ServiceUsage
803 from lp.archiveuploader.dscfile import DSCFile
804 from lp.archiveuploader.uploadpolicy import BuildDaemonUploadPolicy
805@@ -147,6 +148,7 @@
806 from lp.code.interfaces.codeimportevent import ICodeImportEventSet
807 from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
808 from lp.code.interfaces.codeimportresult import ICodeImportResultSet
809+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
810 from lp.code.interfaces.revision import IRevisionSet
811 from lp.code.interfaces.sourcepackagerecipe import (
812 ISourcePackageRecipeSource,
813@@ -1148,6 +1150,157 @@
814 name, owner, registrant, description, configuration, branches)
815 return queue
816
817+ def makeRelatedBranchesForSourcePackage(self, sourcepackage=None,
818+ **kwargs):
819+ """Create some branches associated with a sourcepackage."""
820+
821+ reference_branch = self.makePackageBranch(sourcepackage=sourcepackage)
822+ return self.makeRelatedBranches(
823+ reference_branch=reference_branch, **kwargs)
824+
825+ def makeRelatedBranchesForProduct(self, product=None, **kwargs):
826+ """Create some branches associated with a product."""
827+
828+ reference_branch = self.makeProductBranch(product=product)
829+ return self.makeRelatedBranches(
830+ reference_branch=reference_branch, **kwargs)
831+
832+ def makeRelatedBranches(self, reference_branch=None,
833+ with_series_branches=True,
834+ with_package_branches=True,
835+ with_private_branches=False):
836+ """Create some branches associated with a reference branch.
837+ The other branches are:
838+ - series branches: a set of branches associated with product
839+ series of the same product as the reference branch.
840+ - package branches: a set of branches associated with packagesource
841+ entities of the same product as the reference branch or the same
842+ sourcepackage depending on what type of branch it is.
843+
844+ If no reference branch is supplied, create one.
845+
846+ Returns: a tuple consisting of
847+ (reference_branch, related_series_branches, related_package_branches)
848+
849+ """
850+ related_series_branch_info = []
851+ related_package_branch_info = []
852+ # Make the base_branch if required and find the product if one exists.
853+ naked_product = None
854+ if reference_branch is None:
855+ naked_product = removeSecurityProxy(self.makeProduct())
856+ # Create the 'source' branch ie the base branch of a recipe.
857+ reference_branch = self.makeProductBranch(
858+ name="reference_branch",
859+ product=naked_product)
860+ elif reference_branch.product is not None:
861+ naked_product = removeSecurityProxy(reference_branch.product)
862+
863+ related_branch_owner = self.makePerson()
864+ # Only branches related to products have related series branches.
865+ if with_series_branches and naked_product is not None:
866+ series_branch_info = []
867+ # Add some product series
868+ def makeSeriesBranch(name, is_private=False):
869+ branch = self.makeBranch(
870+ name=name,
871+ product=naked_product, owner=related_branch_owner,
872+ private=is_private)
873+ series = self.makeProductSeries(
874+ product=naked_product, branch=branch)
875+ return branch, series
876+ for x in range(4):
877+ is_private = x == 0 and with_private_branches
878+ (branch, series) = makeSeriesBranch(
879+ name=("series_branch_%s" % x), is_private=is_private)
880+ if not is_private:
881+ series_branch_info.append((branch, series))
882+
883+ # Sort them
884+ related_series_branch_info = sorted_version_numbers(
885+ series_branch_info, key=lambda branch_info: (
886+ getattr(branch_info[1], 'name')))
887+
888+ # Add a development branch at the start of the list.
889+ naked_product.development_focus.name = 'trunk'
890+ devel_branch = self.makeProductBranch(
891+ product=naked_product, name='trunk_branch',
892+ owner=related_branch_owner)
893+ linked_branch = ICanHasLinkedBranch(naked_product)
894+ linked_branch.setBranch(devel_branch)
895+ related_series_branch_info.insert(0,
896+ (devel_branch, naked_product.development_focus))
897+
898+ if with_package_branches:
899+ # Create related package branches if the base_branch is
900+ # associated with a product.
901+ if naked_product is not None:
902+
903+ def makePackageBranch(name, is_private=False):
904+ distro = self.makeDistribution()
905+ distroseries = self.makeDistroSeries(
906+ distribution=distro)
907+ sourcepackagename = self.makeSourcePackageName()
908+
909+ suitesourcepackage = self.makeSuiteSourcePackage(
910+ sourcepackagename=sourcepackagename,
911+ distroseries=distroseries,
912+ pocket=PackagePublishingPocket.RELEASE)
913+ naked_sourcepackage = removeSecurityProxy(
914+ suitesourcepackage)
915+
916+ branch = self.makePackageBranch(
917+ name=name, owner=related_branch_owner,
918+ sourcepackagename=sourcepackagename,
919+ distroseries=distroseries, private=is_private)
920+ linked_branch = ICanHasLinkedBranch(naked_sourcepackage)
921+ with celebrity_logged_in('admin'):
922+ linked_branch.setBranch(branch, related_branch_owner)
923+
924+ series = self.makeProductSeries(product=naked_product)
925+ self.makePackagingLink(
926+ distroseries=distroseries, productseries=series,
927+ sourcepackagename=sourcepackagename)
928+ return branch, distroseries
929+
930+ for x in range(5):
931+ is_private = x == 0 and with_private_branches
932+ branch, distroseries = makePackageBranch(
933+ name=("product_package_branch_%s" % x),
934+ is_private=is_private)
935+ if not is_private:
936+ related_package_branch_info.append(
937+ (branch, distroseries))
938+
939+ # Create related package branches if the base_branch is
940+ # associated with a sourcepackage.
941+ if reference_branch.sourcepackage is not None:
942+ distroseries = reference_branch.sourcepackage.distroseries
943+ for pocket in [
944+ PackagePublishingPocket.RELEASE,
945+ PackagePublishingPocket.UPDATES,
946+ ]:
947+ branch = self.makePackageBranch(
948+ name="package_branch_%s" % pocket.name,
949+ distroseries=distroseries)
950+ with celebrity_logged_in('admin'):
951+ reference_branch.sourcepackage.setBranch(
952+ pocket, branch,
953+ related_branch_owner)
954+
955+ related_package_branch_info.append(
956+ (branch, distroseries))
957+
958+ related_package_branch_info = sorted_version_numbers(
959+ related_package_branch_info, key=lambda branch_info: (
960+ getattr(branch_info[1], 'name')))
961+
962+
963+ return (
964+ reference_branch,
965+ related_series_branch_info,
966+ related_package_branch_info)
967+
968 def enableDefaultStackingForProduct(self, product, branch=None):
969 """Give 'product' a default stacked-on branch.
970