Launchpad itself

Code review comment for lp:~jcsackett/launchpad/projectgroup-branches-652156

  1. projectgroup-branches-652156
  2. Merge into devel
Revision history for this message
j.c.sackett (jcsackett) wrote :

> I think that a project group that doesn't have a single project using LP
> for hosting code is unlikely to have more than a handful projects, so I
> think we could afford these extra SQL queries in the name of a better
> user experience.
>
> That said, it's fine if you want to implement that in a separate branch,
> as I might be wrong.

I can buy your justification; I've added in that feature and attached an incremental.

>> === modified file 'lib/lp/registry/model/projectgroup.py'
>> --- lib/lp/registry/model/projectgroup.py 2010-10-07 13:26:53 +0000
>> +++ lib/lp/registry/model/projectgroup.py 2010-10-13 18:43:40 +0000
>> @@ -222,6 +222,10 @@
>> """See `IProjectGroup`."""
>> return self.translatables().count() != 0
>>
>> + def has_branches(self):
>> + """ See `IProjectGroup`."""
>> + return self.getBranches().count() != 0
>
> Can you use .is_empty() instead of .count() here? It is faster and we
> don't really need the count here.
> (I know this was supposed to be a UI review, but I thought it was worth
> mentioning)

Taken care of.

tmp.diff
1=== modified file 'lib/lp/code/templates/project-branches.pt'
2--- lib/lp/code/templates/project-branches.pt 2010-10-13 17:57:41 +0000
3+++ lib/lp/code/templates/project-branches.pt 2010-10-15 20:13:03 +0000
4@@ -20,8 +20,22 @@
5 <tal:no-branches
6 condition="not:context/has_branches">
7 <div id="no-branchtable">
8- <strong>None of <tal:project replace="context/displayname"/>'s
9- projects are using Launchpad to host their code.</strong>
10+ <p>
11+ <strong>None of <tal:project replace="context/displayname"/>'s
12+ projects are using Launchpad to host their code.</strong>
13+ </p>
14+ <div tal:define="products context/getConfigurableProducts">
15+ <p id="projectgroup-products"
16+ tal:condition="products">
17+ You can set up code hosting for the following projects that
18+ are part of <tal:project replace="context/displayname"/>.
19+ </p>
20+ <ul id="product-list" tal:repeat="product products">
21+ <li>
22+ <a tal:replace="structure product/fmt:link"/>
23+ </li>
24+ </ul>
25+ </div>
26 </div>
27 </tal:no-branches>
28 <tal:has-branches
29
30=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
31--- lib/lp/registry/interfaces/projectgroup.py 2010-10-15 16:23:38 +0000
32+++ lib/lp/registry/interfaces/projectgroup.py 2010-10-15 20:19:13 +0000
33@@ -315,6 +315,9 @@
34 def getProduct(name):
35 """Get a product with name `name`."""
36
37+ def getConfigurableProducts(self):
38+ """Get all products that can be edited by user."""
39+
40 def translatables():
41 """Return an iterator over products that have resources translatables.
42
43
44=== modified file 'lib/lp/registry/model/projectgroup.py'
45--- lib/lp/registry/model/projectgroup.py 2010-10-15 16:23:38 +0000
46+++ lib/lp/registry/model/projectgroup.py 2010-10-15 20:19:26 +0000
47@@ -42,6 +42,7 @@
48 IHasLogo,
49 IHasMugshot,
50 )
51+from canonical.launchpad.webapp.authorization import check_permission
52 from lp.answers.interfaces.faqcollection import IFAQCollection
53 from lp.answers.interfaces.questioncollection import (
54 ISearchableByQuestionOwner,
55@@ -174,6 +175,10 @@
56 def getProduct(self, name):
57 return Product.selectOneBy(project=self, name=name)
58
59+ def getConfigurableProducts(self):
60+ return [product for product in self.products
61+ if check_permission('launchpad.Edit', product)]
62+
63 @property
64 def drivers(self):
65 """See `IHasDrivers`."""

« Back to merge proposal