Merge lp:~stevenk/launchpad/destroy-simplified-branch-ff into lp:launchpad
- destroy-simplified-branch-ff
- Merge into devel
Proposed by
Steve Kowalik
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 16086 |
Proposed branch: | lp:~stevenk/launchpad/destroy-simplified-branch-ff |
Merge into: | lp:launchpad |
Diff against target: |
509 lines (+57/-233) 8 files modified
lib/lp/code/browser/branchlisting.py (+17/-147) lib/lp/code/browser/tests/test_branchlisting.py (+5/-14) lib/lp/code/stories/branches/xx-branch-listings.txt (+1/-1) lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt (+6/-5) lib/lp/code/stories/branches/xx-person-branches.txt (+16/-14) lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt (+1/-1) lib/lp/code/templates/person-codesummary.pt (+11/-45) lib/lp/services/features/flags.py (+0/-6) |
To merge this branch: | bzr merge lp:~stevenk/launchpad/destroy-simplified-branch-ff |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+127630@code.launchpad.net |
Commit message
Description of the change
The code.simplified
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/code/browser/branchlisting.py' |
2 | --- lib/lp/code/browser/branchlisting.py 2012-10-03 03:07:36 +0000 |
3 | +++ lib/lp/code/browser/branchlisting.py 2012-10-04 01:12:22 +0000 |
4 | @@ -71,11 +71,7 @@ |
5 | ) |
6 | from lp.bugs.interfaces.bugbranch import IBugBranchSet |
7 | from lp.code.browser.branch import BranchMirrorMixin |
8 | -from lp.code.browser.branchmergeproposallisting import ( |
9 | - ActiveReviewsView, |
10 | - PersonActiveReviewsView, |
11 | - PersonProductActiveReviewsView, |
12 | - ) |
13 | +from lp.code.browser.branchmergeproposallisting import ActiveReviewsView |
14 | from lp.code.browser.branchmergequeuelisting import HasMergeQueuesMenuMixin |
15 | from lp.code.browser.branchvisibilitypolicy import BranchVisibilityPolicyMixin |
16 | from lp.code.browser.summary import BranchCountSummaryView |
17 | @@ -117,7 +113,6 @@ |
18 | from lp.registry.model.sourcepackage import SourcePackage |
19 | from lp.services.browser_helpers import get_plural_text |
20 | from lp.services.config import config |
21 | -from lp.services.features import getFeatureFlag |
22 | from lp.services.feeds.browser import ( |
23 | FeedsMixin, |
24 | PersonBranchesFeedLink, |
25 | @@ -161,7 +156,7 @@ |
26 | return Badge('/@@/warning', '/@@/warning-large', '', |
27 | 'Branch has errors') |
28 | else: |
29 | - return HasBadgeBase.getBadge(self, badge_name) |
30 | + return super(BranchBadges, self).getBadge(badge_name) |
31 | |
32 | |
33 | class BranchListingItem(BzrIdentityMixin, BranchBadges): |
34 | @@ -560,9 +555,7 @@ |
35 | |
36 | @property |
37 | def initial_values(self): |
38 | - return { |
39 | - 'lifecycle': BranchLifecycleStatusFilter.CURRENT, |
40 | - } |
41 | + return {'lifecycle': BranchLifecycleStatusFilter.CURRENT} |
42 | |
43 | @cachedproperty |
44 | def selected_lifecycle_status(self): |
45 | @@ -856,30 +849,8 @@ |
46 | usedfor = IPerson |
47 | facet = 'branches' |
48 | links = ['registered', 'owned', 'subscribed', |
49 | - 'active_reviews', 'mergequeues', 'source_package_recipes', |
50 | - 'simplified_subscribed', 'simplified_registered', |
51 | - 'simplified_owned', 'simplified_active_reviews'] |
52 | - extra_attributes = [ |
53 | - 'active_review_count', |
54 | - 'owned_branch_count', |
55 | - 'registered_branch_count', |
56 | - 'show_summary', |
57 | - 'subscribed_branch_count', |
58 | - 'mergequeue_count', |
59 | - 'simplified_branches_menu', |
60 | - ] |
61 | - |
62 | - def _getCountCollection(self): |
63 | - """The base collection of branches which should be counted. |
64 | - |
65 | - This collection will be further restricted to, e.g., the |
66 | - branches registered by a particular user for the counts that |
67 | - appear at the top of a branch listing page. |
68 | - |
69 | - This should be overridden in subclasses to restrict to, for |
70 | - example, the set of branches of a particular product. |
71 | - """ |
72 | - return getUtility(IAllBranches).visibleByUser(self.user) |
73 | + 'active_reviews', 'mergequeues', 'source_package_recipes'] |
74 | + extra_attributes = ['mergequeue_count'] |
75 | |
76 | @property |
77 | def person(self): |
78 | @@ -890,139 +861,38 @@ |
79 | """ |
80 | return self.context |
81 | |
82 | - @property |
83 | - def show_summary(self): |
84 | - """Should the template show the summary view with the links.""" |
85 | - |
86 | - if self.simplified_branches_menu: |
87 | - return True |
88 | - else: |
89 | - return ( |
90 | - self.owned_branch_count or |
91 | - self.registered_branch_count or |
92 | - self.subscribed_branch_count or |
93 | - self.active_review_count |
94 | - ) |
95 | - |
96 | - @cachedproperty |
97 | - def simplified_branches_menu(self): |
98 | - return getFeatureFlag('code.simplified_branches_menu.enabled') |
99 | - |
100 | - @cachedproperty |
101 | - def registered_branches_not_empty(self): |
102 | - """False if the number of branches registered by self.person |
103 | - is zero. |
104 | - """ |
105 | - return ( |
106 | - not self._getCountCollection().registeredBy( |
107 | - self.person).is_empty()) |
108 | - |
109 | - def simplified_owned(self): |
110 | - return Link( |
111 | - canonical_url(self.context, rootsite='code'), |
112 | - 'Owned branches') |
113 | - |
114 | - def simplified_registered(self): |
115 | - person_is_individual = (not self.person.is_team) |
116 | - return Link( |
117 | - '+registeredbranches', |
118 | - 'Registered branches', |
119 | - enabled=( |
120 | - person_is_individual and |
121 | - self.registered_branches_not_empty)) |
122 | - |
123 | - def simplified_subscribed(self): |
124 | - return Link( |
125 | - '+subscribedbranches', |
126 | - 'Subscribed branches') |
127 | - |
128 | - def simplified_active_reviews(self): |
129 | - return Link( |
130 | - '+activereviews', |
131 | - 'Active reviews') |
132 | - |
133 | - def source_package_recipes(self): |
134 | - return Link( |
135 | - '+recipes', |
136 | - 'Source package recipes', |
137 | - enabled=IPerson.providedBy(self.context)) |
138 | - |
139 | - @cachedproperty |
140 | - def registered_branch_count(self): |
141 | - """Return the number of branches registered by self.person.""" |
142 | - return self._getCountCollection().registeredBy(self.person).count() |
143 | - |
144 | - @cachedproperty |
145 | - def owned_branch_count(self): |
146 | - """Return the number of branches owned by self.person.""" |
147 | - return self._getCountCollection().ownedBy(self.person).count() |
148 | - |
149 | - @cachedproperty |
150 | - def subscribed_branch_count(self): |
151 | - """Return the number of branches subscribed to by self.person.""" |
152 | - return self._getCountCollection().subscribedBy(self.person).count() |
153 | - |
154 | def owned(self): |
155 | return Link( |
156 | - canonical_url(self.context, rootsite='code'), |
157 | - get_plural_text( |
158 | - self.owned_branch_count, 'owned branch', 'owned branches')) |
159 | + canonical_url(self.context, rootsite='code'), 'Owned branches') |
160 | |
161 | def registered(self): |
162 | - person_is_individual = (not self.person.is_team) |
163 | + enabled = not self.person.is_team |
164 | return Link( |
165 | - '+registeredbranches', |
166 | - get_plural_text( |
167 | - self.registered_branch_count, |
168 | - 'registered branch', 'registered branches'), |
169 | - enabled=person_is_individual) |
170 | + '+registeredbranches', 'Registered branches', enabled=enabled) |
171 | |
172 | def subscribed(self): |
173 | + return Link('+subscribedbranches', 'Subscribed branches') |
174 | + |
175 | + def active_reviews(self): |
176 | + return Link('+activereviews', 'Active reviews') |
177 | + |
178 | + def source_package_recipes(self): |
179 | return Link( |
180 | - '+subscribedbranches', |
181 | - get_plural_text( |
182 | - self.subscribed_branch_count, |
183 | - 'subscribed branch', 'subscribed branches')) |
184 | - |
185 | - @cachedproperty |
186 | - def active_review_count(self): |
187 | - """Return the number of active reviews for self.person's branches.""" |
188 | - active_reviews = PersonActiveReviewsView(self.context, self.request) |
189 | - return active_reviews.getProposals().count() |
190 | - |
191 | - def active_reviews(self): |
192 | - text = get_plural_text( |
193 | - self.active_review_count, |
194 | - 'active review', |
195 | - 'active reviews') |
196 | - return Link('+activereviews', text) |
197 | + '+recipes', 'Source package recipes', |
198 | + enabled=IPerson.providedBy(self.context)) |
199 | |
200 | |
201 | class PersonProductBranchesMenu(PersonBranchesMenu): |
202 | |
203 | usedfor = IPersonProduct |
204 | links = ['registered', 'owned', 'subscribed', 'active_reviews', |
205 | - 'source_package_recipes', |
206 | - 'simplified_subscribed', 'simplified_registered', |
207 | - 'simplified_owned', 'simplified_active_reviews'] |
208 | - |
209 | - def _getCountCollection(self): |
210 | - """See `PersonBranchesMenu`.""" |
211 | - collection = getUtility(IAllBranches).visibleByUser(self.user) |
212 | - return collection.inProduct(self.context.product) |
213 | + 'source_package_recipes'] |
214 | |
215 | @property |
216 | def person(self): |
217 | """See `PersonBranchesMenu`.""" |
218 | return self.context.person |
219 | |
220 | - @cachedproperty |
221 | - def active_review_count(self): |
222 | - """Return the number of active reviews for self.person's branches.""" |
223 | - active_reviews = PersonProductActiveReviewsView( |
224 | - self.context, self.request) |
225 | - return active_reviews.getProposals().count() |
226 | - |
227 | |
228 | class PersonBaseBranchListingView(BranchListingView): |
229 | """Base class used for different person listing views.""" |
230 | |
231 | === modified file 'lib/lp/code/browser/tests/test_branchlisting.py' |
232 | --- lib/lp/code/browser/tests/test_branchlisting.py 2012-09-06 00:01:38 +0000 |
233 | +++ lib/lp/code/browser/tests/test_branchlisting.py 2012-10-04 01:12:22 +0000 |
234 | @@ -1,4 +1,4 @@ |
235 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
236 | +# Copyright 2009-2012 Canonical Ltd. This software is licensed under the |
237 | # GNU Affero General Public License version 3 (see the file LICENSE). |
238 | |
239 | """Tests for branch listing.""" |
240 | @@ -265,10 +265,6 @@ |
241 | self._test_batch_template(self.barney) |
242 | |
243 | |
244 | -SIMPLIFIED_BRANCHES_MENU_FLAG = { |
245 | - 'code.simplified_branches_menu.enabled': 'on'} |
246 | - |
247 | - |
248 | class TestSimplifiedPersonBranchesView(TestCaseWithFactory): |
249 | |
250 | layer = LaunchpadFunctionalLayer |
251 | @@ -294,19 +290,16 @@ |
252 | def get_branch_list_page(self, target=None, page_name='+branches'): |
253 | if target is None: |
254 | target = self.default_target |
255 | - with FeatureFixture(SIMPLIFIED_BRANCHES_MENU_FLAG): |
256 | - with person_logged_in(self.user): |
257 | - return create_initialized_view( |
258 | - target, page_name, rootsite='code', |
259 | - principal=self.user)() |
260 | + with person_logged_in(self.user): |
261 | + return create_initialized_view( |
262 | + target, page_name, rootsite='code', principal=self.user)() |
263 | |
264 | def test_branch_list_h1(self): |
265 | self.makeABranch() |
266 | page = self.get_branch_list_page() |
267 | h1_matcher = soupmatchers.HTMLContains( |
268 | soupmatchers.Tag( |
269 | - 'Title', 'h1', |
270 | - text='Bazaar branches owned by Barney')) |
271 | + 'Title', 'h1', text='Bazaar branches owned by Barney')) |
272 | self.assertThat(page, h1_matcher) |
273 | |
274 | def test_branch_list_empty(self): |
275 | @@ -317,7 +310,6 @@ |
276 | text='There are no branches related to Barney ' |
277 | 'in Launchpad today.')) |
278 | self.assertThat(page, empty_message_matcher) |
279 | - self.assertThat(page, Not(self.registered_branches_matcher)) |
280 | |
281 | def test_branch_list_registered_link(self): |
282 | self.makeABranch() |
283 | @@ -408,7 +400,6 @@ |
284 | text='There are no branches of Bambam owned by Barney ' |
285 | 'in Launchpad today.')) |
286 | self.assertThat(page, empty_message_matcher) |
287 | - self.assertThat(page, Not(self.registered_branches_matcher)) |
288 | |
289 | |
290 | class TestSourcePackageBranchesView(TestCaseWithFactory): |
291 | |
292 | === modified file 'lib/lp/code/stories/branches/xx-branch-listings.txt' |
293 | --- lib/lp/code/stories/branches/xx-branch-listings.txt 2012-07-17 14:29:17 +0000 |
294 | +++ lib/lp/code/stories/branches/xx-branch-listings.txt 2012-10-04 01:12:22 +0000 |
295 | @@ -65,7 +65,7 @@ |
296 | needed, then the table is sortable and no batching navigation links are shown. |
297 | |
298 | >>> browser.open('http://code.launchpad.dev/~name12') |
299 | - >>> browser.getLink('subscribed').click() |
300 | + >>> browser.getLink('Subscribed').click() |
301 | >>> links = find_tag_by_id(browser.contents, 'branch-batch-links') |
302 | >>> links is None |
303 | True |
304 | |
305 | === modified file 'lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt' |
306 | --- lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2012-08-21 11:01:47 +0000 |
307 | +++ lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2012-10-04 01:12:22 +0000 |
308 | @@ -93,10 +93,11 @@ |
309 | |
310 | >>> browser.open('http://code.launchpad.dev/~albert') |
311 | >>> print_tag_with_id(browser.contents, 'portlet-person-codesummary') |
312 | - 1 owned branch |
313 | - 1 registered branch |
314 | - 1 subscribed branch |
315 | - 1 active review |
316 | + Owned branches |
317 | + Registered branches |
318 | + Subscribed branches |
319 | + Active reviews |
320 | + Source package recipes |
321 | |
322 | The person's active reviews also lists all of their currently requested |
323 | reviews. |
324 | @@ -128,7 +129,7 @@ |
325 | Since Albert is in the A-Team, he can do the pending review. |
326 | |
327 | >>> browser.open('http://code.launchpad.dev/~a-team') |
328 | - >>> browser.getLink('active review').click() |
329 | + >>> browser.getLink('Active reviews').click() |
330 | >>> print_tag_with_id(browser.contents, 'proposals') |
331 | Requested reviews I can do |
332 | Branch Merge Proposal Requested By Lines Activity |
333 | |
334 | === modified file 'lib/lp/code/stories/branches/xx-person-branches.txt' |
335 | --- lib/lp/code/stories/branches/xx-person-branches.txt 2012-01-15 13:32:27 +0000 |
336 | +++ lib/lp/code/stories/branches/xx-person-branches.txt 2012-10-04 01:12:22 +0000 |
337 | @@ -64,7 +64,7 @@ |
338 | There is also a link which points to the registered branches page. |
339 | |
340 | >>> browser.open('http://code.launchpad.dev/~name12') |
341 | - >>> browser.getLink('registered').click() |
342 | + >>> browser.getLink('Registered').click() |
343 | >>> print browser.title |
344 | Registered : Code : Sample Person |
345 | >>> table = find_tag_by_id(browser.contents, 'branchtable') |
346 | @@ -81,7 +81,7 @@ |
347 | branches. |
348 | |
349 | >>> browser.open('http://code.launchpad.dev/~name12') |
350 | - >>> browser.getLink('subscribed').click() |
351 | + >>> browser.getLink('Subscribed').click() |
352 | >>> print browser.title |
353 | Subscribed : Code : Sample Person |
354 | >>> table = find_tag_by_id(browser.contents, 'branchtable') |
355 | @@ -112,10 +112,11 @@ |
356 | >>> eric_browser = setupBrowser(auth="Basic eric@example.com:test") |
357 | >>> eric_browser.open('http://code.launchpad.dev/~eric') |
358 | >>> print_tag_with_id(eric_browser.contents, 'portlet-person-codesummary') |
359 | - 1 owned branch |
360 | - 1 registered branch |
361 | - 1 subscribed branch |
362 | - 0 active reviews |
363 | + Owned branches |
364 | + Registered branches |
365 | + Subscribed branches |
366 | + Active reviews |
367 | + Source package recipes |
368 | |
369 | Now we'll create another branch, and unsubscribe the owner from it. |
370 | |
371 | @@ -127,11 +128,11 @@ |
372 | >>> eric_browser.open('http://code.launchpad.dev/~eric') |
373 | >>> print_tag_with_id( |
374 | ... eric_browser.contents, 'portlet-person-codesummary') |
375 | - 2 owned branches |
376 | - 2 registered branches |
377 | - 1 subscribed branch |
378 | - 0 active reviews |
379 | - |
380 | + Owned branches |
381 | + Registered branches |
382 | + Subscribed branches |
383 | + Active reviews |
384 | + Source package recipes |
385 | |
386 | Teams do not show registered branches |
387 | ------------------------------------- |
388 | @@ -142,9 +143,10 @@ |
389 | >>> browser.open('http://code.launchpad.dev/~landscape-developers') |
390 | >>> print_tag_with_id( |
391 | ... browser.contents, 'portlet-person-codesummary') |
392 | - 1 owned branch |
393 | - 2 subscribed branches |
394 | - 0 active reviews |
395 | + Owned branches |
396 | + Subscribed branches |
397 | + Active reviews |
398 | + Source package recipes |
399 | |
400 | >>> browser.getLink('registered').click() |
401 | Traceback (most recent call last): |
402 | |
403 | === modified file 'lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt' |
404 | --- lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt 2010-10-15 16:41:50 +0000 |
405 | +++ lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt 2012-10-04 01:12:22 +0000 |
406 | @@ -39,7 +39,7 @@ |
407 | |
408 | >>> browser.open('http://code.launchpad.dev/~eric/fooix') |
409 | >>> print_tag_with_id(browser.contents, 'portlet-person-codesummary') |
410 | - 2 owned branches ... |
411 | + Owned branches ... |
412 | >>> print_tag_with_id(browser.contents, 'branchtable') |
413 | Name ... |
414 | lp://dev/~eric/fooix/feature ... |
415 | |
416 | === modified file 'lib/lp/code/templates/person-codesummary.pt' |
417 | --- lib/lp/code/templates/person-codesummary.pt 2012-02-21 12:11:11 +0000 |
418 | +++ lib/lp/code/templates/person-codesummary.pt 2012-10-04 01:12:22 +0000 |
419 | @@ -4,62 +4,28 @@ |
420 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
421 | id="portlet-person-codesummary" |
422 | class="portlet" |
423 | - tal:define="menu context/menu:branches; |
424 | - features request/features" |
425 | - tal:condition="menu/show_summary"> |
426 | + tal:define="menu context/menu:branches; features request/features"> |
427 | |
428 | - <table tal:condition="not: menu/simplified_branches_menu"> |
429 | - <tr class="code-links"> |
430 | - <td class="code-count" tal:content="menu/owned_branch_count">100</td> |
431 | - <td tal:content="structure menu/owned/render" |
432 | - /> |
433 | + <table> |
434 | + <tr class="code-links" tal:condition="menu/owned/enabled"> |
435 | + <td tal:content="structure menu/owned/render" /> |
436 | </tr> |
437 | - <tr class="code-links" |
438 | - tal:condition="menu/registered/enabled"> |
439 | - <td class="code-count" tal:content="menu/registered_branch_count">100</td> |
440 | + <tr class="code-links" tal:condition="menu/registered/enabled"> |
441 | <td tal:content="structure menu/registered/render" /> |
442 | </tr> |
443 | - <tr class="code-links"> |
444 | - <td class="code-count" tal:content="menu/subscribed_branch_count">100</td> |
445 | + <tr class="code-links" tal:condition="menu/subscribed/enabled"> |
446 | <td tal:content="structure menu/subscribed/render" /> |
447 | </tr> |
448 | - <tr class="code-links" id="merge-counts"> |
449 | - <td class="code-count" tal:content="menu/active_review_count">5</td> |
450 | + <tr class="code-links" |
451 | + tal:condition="menu/active_reviews/enabled"> |
452 | <td tal:content="structure menu/active_reviews/render" /> |
453 | </tr> |
454 | <tr tal:condition="features/code.branchmergequeue" id="mergequeue-counts"> |
455 | <td class="code-count" tal:content="menu/mergequeue_count">5</td> |
456 | <td tal:condition="menu" |
457 | - tal:content="structure menu/mergequeues/render" |
458 | - /> |
459 | - </tr> |
460 | - </table> |
461 | - |
462 | - <table tal:condition="menu/simplified_branches_menu"> |
463 | - <tr class="code-links" |
464 | - tal:condition="menu/simplified_owned/enabled"> |
465 | - <td tal:content="structure menu/simplified_owned/render" /> |
466 | - </tr> |
467 | - <tr class="code-links" |
468 | - tal:condition="menu/simplified_registered/enabled"> |
469 | - <td tal:content="structure menu/simplified_registered/render" /> |
470 | - </tr> |
471 | - <tr class="code-links" |
472 | - tal:condition="menu/simplified_subscribed/enabled"> |
473 | - <td tal:content="structure menu/simplified_subscribed/render" /> |
474 | - </tr> |
475 | - <tr class="code-links" |
476 | - tal:condition="menu/simplified_active_reviews/enabled"> |
477 | - <td tal:content="structure menu/simplified_active_reviews/render" /> |
478 | - </tr> |
479 | - <tr tal:condition="features/code.branchmergequeue" id="mergequeue-counts"> |
480 | - <td class="code-count" tal:content="menu/mergequeue_count">5</td> |
481 | - <td tal:condition="menu" |
482 | - tal:content="structure menu/mergequeues/render" |
483 | - /> |
484 | - </tr> |
485 | - <tr class="code-links" |
486 | - tal:condition="menu/source_package_recipes/enabled"> |
487 | + tal:content="structure menu/mergequeues/render" /> |
488 | + </tr> |
489 | + <tr class="code-links" tal:condition="menu/source_package_recipes/enabled"> |
490 | <td tal:content="structure menu/source_package_recipes/render" /> |
491 | </tr> |
492 | </table> |
493 | |
494 | === modified file 'lib/lp/services/features/flags.py' |
495 | --- lib/lp/services/features/flags.py 2012-10-02 06:36:44 +0000 |
496 | +++ lib/lp/services/features/flags.py 2012-10-04 01:12:22 +0000 |
497 | @@ -106,12 +106,6 @@ |
498 | '', |
499 | '', |
500 | ''), |
501 | - ('code.simplified_branches_menu.enabled', |
502 | - 'boolean', |
503 | - ('Display a simplified version of the branch menu (omit the counts).'), |
504 | - '', |
505 | - '', |
506 | - ''), |
507 | ('hard_timeout', |
508 | 'float', |
509 | 'Sets the hard request timeout in milliseconds.', |
Thank you.