Merge lp:~cjwatson/launchpad/bmp-buglinktarget-ui into lp:launchpad
- bmp-buglinktarget-ui
- Merge into devel
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 18173 | ||||
Proposed branch: | lp:~cjwatson/launchpad/bmp-buglinktarget-ui | ||||
Merge into: | lp:launchpad | ||||
Prerequisite: | lp:~cjwatson/launchpad/bug-bmp-activity | ||||
Diff against target: |
850 lines (+436/-44) 17 files modified
lib/lp/_schema_circular_imports.py (+6/-0) lib/lp/bugs/browser/bugtask.py (+6/-0) lib/lp/bugs/browser/tests/test_bugtask.py (+5/-5) lib/lp/bugs/interfaces/bug.py (+21/-0) lib/lp/bugs/templates/bug-branch.pt (+6/-0) lib/lp/bugs/templates/bugtask-index.pt (+13/-2) lib/lp/code/browser/branchmergeproposal.py (+9/-5) lib/lp/code/browser/configure.zcml (+12/-0) lib/lp/code/browser/tests/test_branchmergeproposal.py (+143/-3) lib/lp/code/interfaces/branchmergeproposal.py (+18/-0) lib/lp/code/javascript/branch.bugspeclinks.js (+15/-4) lib/lp/code/javascript/tests/test_bugspeclinks.js (+38/-12) lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+14/-9) lib/lp/code/templates/branch-macros.pt (+4/-0) lib/lp/code/templates/branchmergeproposal-macros.pt (+66/-0) lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt (+33/-4) lib/lp/code/templates/branchmergeproposal-unlinkbugs.pt (+27/-0) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/bmp-buglinktarget-ui | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Celso Providelo (community) | Approve | ||
Review via email: mp+298368@code.launchpad.net |
Commit message
Add UI and export webservice API for linking merge proposals to bugs.
Description of the change
Add UI and export webservice API for linking merge proposals to bugs.
Bugs display links to MPs where they exist; MPs have their "Related bugs and blueprints" summary section split into "Related bugs" and "Related blueprints" so that the former can more reasonably have a "Link a bug report" option. There's currently no way to add a link from the bug side in the UI, although that could be done later.
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/_schema_circular_imports.py' |
2 | --- lib/lp/_schema_circular_imports.py 2016-06-24 23:35:24 +0000 |
3 | +++ lib/lp/_schema_circular_imports.py 2016-07-29 16:54:16 +0000 |
4 | @@ -283,6 +283,8 @@ |
5 | IBranchMergeProposal, 'votes', ICodeReviewVoteReference) |
6 | patch_collection_return_type( |
7 | IBranchMergeProposal, 'getRelatedBugTasks', IBugTask) |
8 | +patch_plain_parameter_type(IBranchMergeProposal, 'linkBug', 'bug', IBug) |
9 | +patch_plain_parameter_type(IBranchMergeProposal, 'unlinkBug', 'bug', IBug) |
10 | |
11 | patch_collection_return_type(IHasBranches, 'getBranches', IBranch) |
12 | patch_collection_return_type( |
13 | @@ -682,8 +684,12 @@ |
14 | patch_plain_parameter_type( |
15 | IBug, 'getNominations', 'target', IBugTarget) |
16 | patch_collection_property(IBug, 'linked_merge_proposals', IBranchMergeProposal) |
17 | +patch_plain_parameter_type( |
18 | + IBug, 'linkMergeProposal', 'merge_proposal', IBranchMergeProposal) |
19 | patch_choice_parameter_type( |
20 | IBug, 'subscribe', 'level', BugNotificationLevel) |
21 | +patch_plain_parameter_type( |
22 | + IBug, 'unlinkMergeProposal', 'merge_proposal', IBranchMergeProposal) |
23 | |
24 | |
25 | # IFrontPageBugAddForm |
26 | |
27 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
28 | --- lib/lp/bugs/browser/bugtask.py 2016-07-23 10:28:41 +0000 |
29 | +++ lib/lp/bugs/browser/bugtask.py 2016-07-29 16:54:16 +0000 |
30 | @@ -812,6 +812,12 @@ |
31 | eager_load=True)) |
32 | return linked_branches |
33 | |
34 | + @cachedproperty |
35 | + def linked_merge_proposals(self): |
36 | + """Filter out the links to non-visible private MPs.""" |
37 | + return list(self.context.bug.getVisibleLinkedMergeProposals( |
38 | + self.user, eager_load=True)) |
39 | + |
40 | @property |
41 | def days_to_expiration(self): |
42 | """Return the number of days before the bug is expired, or None.""" |
43 | |
44 | === modified file 'lib/lp/bugs/browser/tests/test_bugtask.py' |
45 | --- lib/lp/bugs/browser/tests/test_bugtask.py 2016-07-28 11:10:29 +0000 |
46 | +++ lib/lp/bugs/browser/tests/test_bugtask.py 2016-07-29 16:54:16 +0000 |
47 | @@ -129,7 +129,7 @@ |
48 | 0, 10, login_method=lambda: login(ADMIN_EMAIL)) |
49 | # This may seem large: it is; there is easily another 25% fat in |
50 | # there. |
51 | - self.assertThat(recorder1, HasQueryCount(LessThan(83))) |
52 | + self.assertThat(recorder1, HasQueryCount(LessThan(84))) |
53 | self.assertThat(recorder2, HasQueryCount.byEquality(recorder1)) |
54 | |
55 | def test_rendered_query_counts_constant_with_attachments(self): |
56 | @@ -140,7 +140,7 @@ |
57 | lambda: self.getUserBrowser(url, person), |
58 | lambda: self.factory.makeBugAttachment(bug=task.bug), |
59 | 1, 9, login_method=lambda: login(ADMIN_EMAIL)) |
60 | - self.assertThat(recorder1, HasQueryCount(LessThan(84))) |
61 | + self.assertThat(recorder1, HasQueryCount(LessThan(85))) |
62 | self.assertThat(recorder2, HasQueryCount.byEquality(recorder1)) |
63 | |
64 | def makeLinkedBranchMergeProposal(self, sourcepackage, bug, owner): |
65 | @@ -175,7 +175,7 @@ |
66 | recorder1, recorder2 = record_two_runs( |
67 | lambda: self.getUserBrowser(url, owner), |
68 | make_merge_proposals, 0, 1) |
69 | - self.assertThat(recorder1, HasQueryCount(LessThan(89))) |
70 | + self.assertThat(recorder1, HasQueryCount(LessThan(90))) |
71 | # Ideally this should be much fewer, but this tries to keep a win of |
72 | # removing more than half of these. |
73 | self.assertThat( |
74 | @@ -221,7 +221,7 @@ |
75 | lambda: self.getUserBrowser(url, person), |
76 | lambda: add_activity("description", self.factory.makePerson()), |
77 | 1, 20, login_method=lambda: login(ADMIN_EMAIL)) |
78 | - self.assertThat(recorder1, HasQueryCount(LessThan(84))) |
79 | + self.assertThat(recorder1, HasQueryCount(LessThan(85))) |
80 | self.assertThat(recorder2, HasQueryCount.byEquality(recorder1)) |
81 | |
82 | def test_rendered_query_counts_constant_with_milestones(self): |
83 | @@ -231,7 +231,7 @@ |
84 | |
85 | with celebrity_logged_in('admin'): |
86 | browses_under_limit = BrowsesWithQueryLimit( |
87 | - 84, self.factory.makePerson()) |
88 | + 85, self.factory.makePerson()) |
89 | |
90 | self.assertThat(bug, browses_under_limit) |
91 | |
92 | |
93 | === modified file 'lib/lp/bugs/interfaces/bug.py' |
94 | --- lib/lp/bugs/interfaces/bug.py 2016-07-27 17:59:51 +0000 |
95 | +++ lib/lp/bugs/interfaces/bug.py 2016-07-29 16:54:16 +0000 |
96 | @@ -817,6 +817,27 @@ |
97 | """ |
98 | |
99 | @call_with(user=REQUEST_USER) |
100 | + @operation_parameters( |
101 | + # Really IBranchMergeProposal, patched in _schema_circular_imports.py. |
102 | + merge_proposal=Reference( |
103 | + Interface, title=_('Merge proposal'), required=True)) |
104 | + @export_write_operation() |
105 | + @operation_for_version('devel') |
106 | + def linkMergeProposal(merge_proposal, user, check_permissions=True): |
107 | + """Ensure that this MP is linked to this bug.""" |
108 | + |
109 | + @call_with(user=REQUEST_USER) |
110 | + @operation_parameters( |
111 | + # Really IBranchMergeProposal, patched in _schema_circular_imports.py. |
112 | + merge_proposal=Reference( |
113 | + Interface, title=_('Merge proposal'), required=True)) |
114 | + @export_write_operation() |
115 | + @operation_for_version('devel') |
116 | + def unlinkMergeProposal(merge_proposal, user, check_permissions=True): |
117 | + """Ensure that any links between this bug and the given MP are removed. |
118 | + """ |
119 | + |
120 | + @call_with(user=REQUEST_USER) |
121 | @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True)) |
122 | @export_write_operation() |
123 | def linkCVE(cve, user, check_permissions=True): |
124 | |
125 | === modified file 'lib/lp/bugs/templates/bug-branch.pt' |
126 | --- lib/lp/bugs/templates/bug-branch.pt 2016-02-04 05:32:49 +0000 |
127 | +++ lib/lp/bugs/templates/bug-branch.pt 2016-07-29 16:54:16 +0000 |
128 | @@ -3,6 +3,12 @@ |
129 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
130 | xmlns:i18n="http://xml.zope.org/namespaces/i18n"> |
131 | |
132 | + <tal:comment condition="nothing"> |
133 | + The bug-summary macro in |
134 | + lib/lp/code/templates/branchmergeproposal-macros.pt has similar code for |
135 | + Git. |
136 | + </tal:comment> |
137 | + |
138 | <div class="bug-branch-summary" |
139 | tal:define="bug_branch context; |
140 | branch bug_branch/branch; |
141 | |
142 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' |
143 | --- lib/lp/bugs/templates/bugtask-index.pt 2014-11-29 06:11:18 +0000 |
144 | +++ lib/lp/bugs/templates/bugtask-index.pt 2016-07-29 16:54:16 +0000 |
145 | @@ -206,8 +206,9 @@ |
146 | <div id="bug-branches-container" |
147 | style="float: left"> |
148 | <tal:branches |
149 | - define="linked_branches view/linked_branches" |
150 | - condition="linked_branches"> |
151 | + define="linked_branches view/linked_branches; |
152 | + linked_merge_proposals view/linked_merge_proposals" |
153 | + condition="python: linked_branches or linked_merge_proposals"> |
154 | |
155 | <div id="bug-branches"> |
156 | <h2>Related branches</h2> |
157 | @@ -215,6 +216,16 @@ |
158 | <tal:bug-branches repeat="linked_branch linked_branches"> |
159 | <tal:bug-branch replace="structure linked_branch/@@+bug-branch" /> |
160 | </tal:bug-branches> |
161 | + <tal:bug-mps repeat="linked_merge_proposal linked_merge_proposals"> |
162 | + <tal:comment replace="nothing"> |
163 | + This is for Git-based MPs only at present; Bazaar-based |
164 | + MPs show up under linked_branches instead. |
165 | + </tal:comment> |
166 | + <tal:bug-mp define="proposal linked_merge_proposal; |
167 | + bug context/bug"> |
168 | + <metal:bug-mp use-macro="linked_merge_proposal/@@+bmp-macros/bug-summary" /> |
169 | + </tal:bug-mp> |
170 | + </tal:bug-mps> |
171 | </div> |
172 | </tal:branches> |
173 | </div><!-- bug-branch-container --> |
174 | |
175 | === modified file 'lib/lp/code/browser/branchmergeproposal.py' |
176 | --- lib/lp/code/browser/branchmergeproposal.py 2016-07-13 10:13:30 +0000 |
177 | +++ lib/lp/code/browser/branchmergeproposal.py 2016-07-29 16:54:16 +0000 |
178 | @@ -288,6 +288,10 @@ |
179 | BranchMergeProposalStatus.SUPERSEDED) |
180 | return Link('+resubmit', text, enabled=enabled, icon='edit') |
181 | |
182 | + def link_bug(self): |
183 | + text = 'Link a bug report' |
184 | + return Link('+linkbug', text, icon='add') |
185 | + |
186 | |
187 | class BranchMergeProposalEditMenu(NavigationMenu, |
188 | BranchMergeProposalMenuMixin): |
189 | @@ -313,6 +317,7 @@ |
190 | 'set_commit_message', |
191 | 'set_description', |
192 | 'edit_status', |
193 | + 'link_bug', |
194 | 'merge', |
195 | 'request_review', |
196 | 'resubmit', |
197 | @@ -737,10 +742,9 @@ |
198 | return self.context.preview_diff |
199 | |
200 | @property |
201 | - def has_bug_or_spec(self): |
202 | - """Return whether or not the merge proposal has a linked bug or spec. |
203 | - """ |
204 | - return self.linked_bugtasks or self.spec_links |
205 | + def has_specs(self): |
206 | + """Return whether the merge proposal has any linked specs.""" |
207 | + return bool(self.spec_links) |
208 | |
209 | @property |
210 | def spec_links(self): |
211 | @@ -753,7 +757,7 @@ |
212 | |
213 | @cachedproperty |
214 | def linked_bugtasks(self): |
215 | - """Return BugTasks linked to the source branch.""" |
216 | + """Return BugTasks linked to the MP or the source branch.""" |
217 | return self.context.getRelatedBugTasks(self.user) |
218 | |
219 | @property |
220 | |
221 | === modified file 'lib/lp/code/browser/configure.zcml' |
222 | --- lib/lp/code/browser/configure.zcml 2016-05-14 09:54:32 +0000 |
223 | +++ lib/lp/code/browser/configure.zcml 2016-07-29 16:54:16 +0000 |
224 | @@ -254,6 +254,18 @@ |
225 | permission="launchpad.Edit" |
226 | template="../../app/templates/generic-edit.pt"/> |
227 | <browser:page |
228 | + name="+linkbug" |
229 | + for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
230 | + class="lp.bugs.browser.buglinktarget.BugLinkView" |
231 | + permission="launchpad.AnyPerson" |
232 | + template="../../app/templates/generic-edit.pt"/> |
233 | + <browser:page |
234 | + name="+unlinkbug" |
235 | + for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
236 | + class="lp.bugs.browser.buglinktarget.BugsUnlinkView" |
237 | + permission="launchpad.AnyPerson" |
238 | + template="../templates/branchmergeproposal-unlinkbugs.pt"/> |
239 | + <browser:page |
240 | name="+link-summary" |
241 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
242 | class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView" |
243 | |
244 | === modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py' |
245 | --- lib/lp/code/browser/tests/test_branchmergeproposal.py 2016-07-13 10:13:30 +0000 |
246 | +++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2016-07-29 16:54:16 +0000 |
247 | @@ -1,7 +1,6 @@ |
248 | # Copyright 2009-2016 Canonical Ltd. This software is licensed under the |
249 | # GNU Affero General Public License version 3 (see the file LICENSE). |
250 | |
251 | - |
252 | """Unit tests for BranchMergeProposals.""" |
253 | |
254 | __metaclass__ = type |
255 | @@ -11,6 +10,7 @@ |
256 | timedelta, |
257 | ) |
258 | from difflib import unified_diff |
259 | +import doctest |
260 | import hashlib |
261 | import re |
262 | |
263 | @@ -24,6 +24,7 @@ |
264 | Tag, |
265 | ) |
266 | from testtools.matchers import ( |
267 | + DocTestMatches, |
268 | Equals, |
269 | Is, |
270 | MatchesDict, |
271 | @@ -83,6 +84,7 @@ |
272 | from lp.services.webapp.interfaces import BrowserNotificationLevel |
273 | from lp.services.webapp.servers import LaunchpadTestRequest |
274 | from lp.testing import ( |
275 | + admin_logged_in, |
276 | BrowserTestCase, |
277 | EventRecorder, |
278 | feature_flags, |
279 | @@ -104,6 +106,7 @@ |
280 | from lp.testing.pages import ( |
281 | extract_text, |
282 | find_tag_by_id, |
283 | + find_tags_by_class, |
284 | first_tag_by_class, |
285 | get_feedback_messages, |
286 | ) |
287 | @@ -1398,7 +1401,7 @@ |
288 | return PreviewDiff.create( |
289 | self.bmp, preview_diff_bytes, u'a', u'b', None, u'') |
290 | |
291 | - def test_linked_bugs_excludes_mutual_bugs(self): |
292 | + def test_linked_bugtasks_excludes_mutual_bugs(self): |
293 | """List bugs that are linked to the source only.""" |
294 | bug = self.factory.makeBug() |
295 | self.bmp.source_branch.linkBug(bug, self.bmp.registrant) |
296 | @@ -1406,7 +1409,7 @@ |
297 | view = create_initialized_view(self.bmp, '+index') |
298 | self.assertEqual([], view.linked_bugtasks) |
299 | |
300 | - def test_linked_bugs_excludes_private_bugs(self): |
301 | + def test_linked_bugtasks_excludes_private_bugs(self): |
302 | """List bugs that are linked to the source only.""" |
303 | bug = self.factory.makeBug() |
304 | person = self.factory.makePerson() |
305 | @@ -1418,6 +1421,15 @@ |
306 | view = create_initialized_view(self.bmp, '+index') |
307 | self.assertEqual([bug.default_bugtask], view.linked_bugtasks) |
308 | |
309 | + def test_linked_bugtasks_includes_direct_links(self): |
310 | + # linked_bugtasks includes bugs that are linked directly to the |
311 | + # merge proposal, as is the case for Git-based MPs. |
312 | + bug = self.factory.makeBug() |
313 | + bmp = self.factory.makeBranchMergeProposalForGit(registrant=self.user) |
314 | + bmp.linkBug(bug, bmp.registrant) |
315 | + view = create_initialized_view(bmp, '+index') |
316 | + self.assertEqual([bug.default_bugtask], view.linked_bugtasks) |
317 | + |
318 | def makeRevisionGroups(self): |
319 | review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC) |
320 | bmp = self.factory.makeBranchMergeProposal( |
321 | @@ -2009,6 +2021,134 @@ |
322 | InformationType.USERDATA, branch.owner, verify_policy=False) |
323 | |
324 | |
325 | +class TestBranchMergeProposalLinkBugViewMixin: |
326 | + |
327 | + layer = LaunchpadFunctionalLayer |
328 | + |
329 | + def setUp(self): |
330 | + super(TestBranchMergeProposalLinkBugViewMixin, self).setUp() |
331 | + self.bmp = self._makeBranchMergeProposal() |
332 | + |
333 | + def test_anonymous(self): |
334 | + # The "Link a bug report" link on BranchMergeProposal:+index is |
335 | + # visible to all, but anonymous users will need to log in to use it. |
336 | + self.useFixture(FakeLogger()) |
337 | + browser = self.getViewBrowser( |
338 | + self.bmp, view_name="+index", no_login=True) |
339 | + self.assertRaises( |
340 | + Unauthorized, browser.getLink("Link a bug report").click) |
341 | + |
342 | + def test_logged_in(self): |
343 | + # Any logged-in user can use the "Link a bug report" link. |
344 | + browser = self.getViewBrowser(self.bmp, view_name="+index") |
345 | + browser.getLink("Link a bug report").click() |
346 | + self.assertStartsWith(browser.title, "Link a bug report") |
347 | + |
348 | + def assertBugLinks(self, bugtasks, browser): |
349 | + expected_text = [] |
350 | + for bugtask in bugtasks: |
351 | + expected_text.append( |
352 | + "Bug #%d: %s\n%s\n%s" % ( |
353 | + bugtask.bug.id, bugtask.bug.title, |
354 | + bugtask.importance.title, bugtask.status.title)) |
355 | + self.assertEqual( |
356 | + expected_text, |
357 | + [extract_text(tag) for tag in find_tags_by_class( |
358 | + browser.contents, "bug-mp-summary")]) |
359 | + |
360 | + def test_link(self): |
361 | + # A user can enter a bug number to link from an MP to a bug. |
362 | + bug = self.factory.makeBug() |
363 | + bugtask = bug.default_bugtask |
364 | + browser = self.getViewBrowser(self.bmp, view_name="+linkbug") |
365 | + browser.getControl("Bug ID").value = str(bug.id) |
366 | + browser.getControl("Link").click() |
367 | + with person_logged_in(self.user): |
368 | + self.assertBugLinks([bugtask], browser) |
369 | + |
370 | + def test_same_link_twice(self): |
371 | + # Attempting to link to the same bug twice only creates a single |
372 | + # link. |
373 | + bug = self.factory.makeBug() |
374 | + bugtask = bug.default_bugtask |
375 | + with person_logged_in(self.user): |
376 | + self.bmp.linkBug(bug, self.user) |
377 | + browser = self.getViewBrowser(self.bmp, view_name="+linkbug") |
378 | + browser.getControl("Bug ID").value = str(bug.id) |
379 | + browser.getControl("Link").click() |
380 | + with person_logged_in(self.user): |
381 | + self.assertBugLinks([bugtask], browser) |
382 | + |
383 | + |
384 | +class TestBranchMergeProposalLinkBugViewBzr( |
385 | + TestBranchMergeProposalLinkBugViewMixin, BrowserTestCase): |
386 | + |
387 | + def _makeBranchMergeProposal(self): |
388 | + return self.factory.makeBranchMergeProposal() |
389 | + |
390 | + |
391 | +class TestBranchMergeProposalLinkBugViewGit( |
392 | + TestBranchMergeProposalLinkBugViewMixin, GitHostingClientMixin, |
393 | + BrowserTestCase): |
394 | + |
395 | + def _makeBranchMergeProposal(self): |
396 | + return self.factory.makeBranchMergeProposalForGit() |
397 | + |
398 | + def assertMergeProposalLinks(self, mps, browser): |
399 | + matchers = [] |
400 | + for mp in mps: |
401 | + matchers.append(DocTestMatches( |
402 | + "%s\n...\nfor merging\ninto\n%s\n..." % ( |
403 | + mp.merge_source.identity, mp.merge_target.identity), |
404 | + flags=doctest.ELLIPSIS)) |
405 | + self.assertThat( |
406 | + [extract_text(tag) for tag in find_tags_by_class( |
407 | + browser.contents, "bug-mp-summary")], |
408 | + MatchesListwise(matchers)) |
409 | + |
410 | + def test_bug_page_shows_link(self): |
411 | + # The bug-MP link is shown on the bug page. |
412 | + bug = self.factory.makeBug() |
413 | + title = bug.title |
414 | + with person_logged_in(self.user): |
415 | + self.bmp.linkBug(bug, self.user) |
416 | + browser = self.getViewBrowser(self.bmp) |
417 | + browser.getLink(title).click() |
418 | + with person_logged_in(self.user): |
419 | + self.assertMergeProposalLinks([self.bmp], browser) |
420 | + |
421 | + def test_link_to_private_bug_only_shown_if_visible(self): |
422 | + # The MP page only shows links to private bugs if the user can see |
423 | + # the bugs in question. |
424 | + bug = self.factory.makeBug(information_type=InformationType.USERDATA) |
425 | + with admin_logged_in() as admin: |
426 | + bugtask = bug.default_bugtask |
427 | + self.bmp.linkBug(bug, admin) |
428 | + admin_browser = self.getViewBrowser(self.bmp, user=admin) |
429 | + with admin_logged_in(): |
430 | + self.assertBugLinks([bugtask], admin_browser) |
431 | + user_browser = self.getViewBrowser(self.bmp) |
432 | + self.assertBugLinks([], user_browser) |
433 | + |
434 | + def test_unlink_from_merge_proposal(self): |
435 | + # The MP page has a delete button to unlink the bug. |
436 | + bug = self.factory.makeBug() |
437 | + with person_logged_in(self.user): |
438 | + self.bmp.linkBug(bug, self.user) |
439 | + browser = self.getViewBrowser(self.bmp) |
440 | + browser.getLink(url="+unlinkbug").click() |
441 | + self.assertBugLinks([], browser) |
442 | + |
443 | + def test_unlink_from_bug(self): |
444 | + # The bug page has a delete button to unlink the MP. |
445 | + bug = self.factory.makeBug() |
446 | + with person_logged_in(self.user): |
447 | + self.bmp.linkBug(bug, self.user) |
448 | + browser = self.getViewBrowser(bug) |
449 | + browser.getLink(url="+unlinkbug").click() |
450 | + self.assertMergeProposalLinks([], browser) |
451 | + |
452 | + |
453 | class TestBranchMergeProposalDeleteViewMixin: |
454 | """Test the BranchMergeProposal deletion view.""" |
455 | |
456 | |
457 | === modified file 'lib/lp/code/interfaces/branchmergeproposal.py' |
458 | --- lib/lp/code/interfaces/branchmergeproposal.py 2016-06-24 15:44:48 +0000 |
459 | +++ lib/lp/code/interfaces/branchmergeproposal.py 2016-07-29 16:54:16 +0000 |
460 | @@ -695,6 +695,24 @@ |
461 | :param comments: The comments. |
462 | """ |
463 | |
464 | + @export_write_operation() |
465 | + @operation_parameters( |
466 | + # Really IBug, patched in _schema_circular_imports.py. |
467 | + bug=Reference(schema=Interface)) |
468 | + @call_with(user=REQUEST_USER) |
469 | + @operation_for_version('devel') |
470 | + def linkBug(bug, user=None, check_permissions=True): |
471 | + """Link a bug to this merge proposal.""" |
472 | + |
473 | + @export_write_operation() |
474 | + @operation_parameters( |
475 | + # Really IBug, patched in _schema_circular_imports.py. |
476 | + bug=Reference(schema=Interface)) |
477 | + @call_with(user=REQUEST_USER) |
478 | + @operation_for_version('devel') |
479 | + def unlinkBug(bug, user=None, check_permissions=True): |
480 | + """Unlink a bug from this merge proposal.""" |
481 | + |
482 | |
483 | class IBranchMergeProposal(IBranchMergeProposalPublic, |
484 | IBranchMergeProposalView, IBranchMergeProposalEdit, |
485 | |
486 | === modified file 'lib/lp/code/javascript/branch.bugspeclinks.js' |
487 | --- lib/lp/code/javascript/branch.bugspeclinks.js 2012-08-09 04:56:41 +0000 |
488 | +++ lib/lp/code/javascript/branch.bugspeclinks.js 2016-07-29 16:54:16 +0000 |
489 | @@ -1,4 +1,4 @@ |
490 | -/* Copyright 2012 Canonical Ltd. This software is licensed under the |
491 | +/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the |
492 | * GNU Affero General Public License version 3 (see the file LICENSE). |
493 | * |
494 | * Provide functionality for picking a bug. |
495 | @@ -12,9 +12,20 @@ |
496 | var superclass = Y.lp.bugs.bug_picker.BugPicker; |
497 | |
498 | /* |
499 | - * Extract the best candidate for a bug number from the branch name. |
500 | + * Extract the best candidate for a bug number from the context. |
501 | */ |
502 | -namespace.extract_candidate_bug_id = function(branch_name) { |
503 | +namespace.extract_candidate_bug_id = function(context) { |
504 | + var branch_name; |
505 | + if (context.source_branch !== undefined) { |
506 | + branch_name = context.source_branch.name; |
507 | + } else if (context.source_git_path !== undefined) { |
508 | + branch_name = context.source_git_path; |
509 | + if (branch_name.indexOf('refs/heads/') === 0) { |
510 | + branch_name = branch_name.slice('refs/heads/'.length); |
511 | + } |
512 | + } else { |
513 | + branch_name = context.name; |
514 | + } |
515 | // Extract all the runs of numbers in the branch name and sort by |
516 | // descending length. |
517 | var chunks = branch_name.split(/\D/g).sort(function (a, b) { |
518 | @@ -61,7 +72,7 @@ |
519 | this.after('visibleChange', function() { |
520 | if (this.get('visible')) { |
521 | var guessed_bug_id = |
522 | - namespace.extract_candidate_bug_id(LP.cache.context.name); |
523 | + namespace.extract_candidate_bug_id(LP.cache.context); |
524 | if (Y.Lang.isValue(guessed_bug_id)) { |
525 | this._search_input.set('value', guessed_bug_id); |
526 | // Select the pre-filled bug number (if any) so that it will |
527 | |
528 | === modified file 'lib/lp/code/javascript/tests/test_bugspeclinks.js' |
529 | --- lib/lp/code/javascript/tests/test_bugspeclinks.js 2015-10-22 00:13:43 +0000 |
530 | +++ lib/lp/code/javascript/tests/test_bugspeclinks.js 2016-07-29 16:54:16 +0000 |
531 | @@ -1,4 +1,4 @@ |
532 | -/* Copyright 2012-2012 Canonical Ltd. This software is licensed under the |
533 | +/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the |
534 | * GNU Affero General Public License version 3 (see the file LICENSE). */ |
535 | |
536 | YUI.add('lp.code.branch.bugspeclinks.test', function (Y) { |
537 | @@ -14,48 +14,74 @@ |
538 | test_no_bug_id_present: function() { |
539 | // If nothing that looks like a bug ID is present, null is |
540 | // returned. |
541 | - Y.Assert.isNull(extract_candidate_bug_id('no-id-here')); |
542 | + Y.Assert.isNull(extract_candidate_bug_id({'name': 'no-id-here'})); |
543 | }, |
544 | |
545 | test_short_digit_rund_ignored: function() { |
546 | - Y.Assert.isNull(extract_candidate_bug_id('foo-1234-bar')); |
547 | + Y.Assert.isNull( |
548 | + extract_candidate_bug_id({'name': 'foo-1234-bar'})); |
549 | }, |
550 | |
551 | test_leading_zeros_disqualify_potential_ids: function() { |
552 | // Since bug IDs can't start with zeros, any string of numbers |
553 | // with a leading zero are not considered as a potential ID. |
554 | - Y.Assert.isNull(extract_candidate_bug_id('foo-0123456-bar')); |
555 | + Y.Assert.isNull( |
556 | + extract_candidate_bug_id({'name': 'foo-0123456-bar'})); |
557 | Y.Assert.areEqual( |
558 | - extract_candidate_bug_id('foo-0123456-999999-bar'), '999999'); |
559 | + extract_candidate_bug_id({'name': 'foo-0123456-999999-bar'}), |
560 | + '999999'); |
561 | }, |
562 | |
563 | test_five_digit_bug_ids_are_extracted: function() { |
564 | Y.Assert.areEqual( |
565 | - extract_candidate_bug_id('foo-12345-bar'), '12345'); |
566 | + extract_candidate_bug_id({'name': 'foo-12345-bar'}), '12345'); |
567 | }, |
568 | |
569 | test_six_digit_bug_ids_are_extracted: function() { |
570 | Y.Assert.areEqual( |
571 | - extract_candidate_bug_id('foo-123456-bar'), '123456'); |
572 | + extract_candidate_bug_id({'name': 'foo-123456-bar'}), |
573 | + '123456'); |
574 | }, |
575 | |
576 | test_seven_digit_bug_ids_are_extracted: function() { |
577 | Y.Assert.areEqual( |
578 | - extract_candidate_bug_id('foo-1234567-bar'), '1234567'); |
579 | + extract_candidate_bug_id({'name': 'foo-1234567-bar'}), |
580 | + '1234567'); |
581 | }, |
582 | |
583 | test_eight_digit_bug_ids_are_extracted: function() { |
584 | Y.Assert.areEqual( |
585 | - extract_candidate_bug_id('foo-12345678-bar'), '12345678'); |
586 | + extract_candidate_bug_id({'name': 'foo-12345678-bar'}), |
587 | + '12345678'); |
588 | }, |
589 | |
590 | test_longest_potential_id_is_extracted: function() { |
591 | // Since there may be numbers other than a bug ID in a branch |
592 | // name, we want to extract the longest string of digits. |
593 | Y.Assert.areEqual( |
594 | - extract_candidate_bug_id('bug-123456-take-2'), '123456'); |
595 | - Y.Assert.areEqual( |
596 | - extract_candidate_bug_id('123456-1234567'), '1234567'); |
597 | + extract_candidate_bug_id({'name': 'bug-123456-take-2'}), |
598 | + '123456'); |
599 | + Y.Assert.areEqual( |
600 | + extract_candidate_bug_id({'name': '123456-1234567'}), |
601 | + '1234567'); |
602 | + }, |
603 | + |
604 | + test_merge_proposal_bzr: function() { |
605 | + // If the context is a Bazaar-based merge proposal, bug IDs are |
606 | + // extracted from the source branch. |
607 | + Y.Assert.areEqual( |
608 | + extract_candidate_bug_id( |
609 | + {'source_branch': {'name': 'foo-123456-bar'}}), |
610 | + '123456'); |
611 | + }, |
612 | + |
613 | + test_merge_proposal_git: function() { |
614 | + // If the context is a Git-based merge proposal, bug IDs are |
615 | + // extracted from the source reference path. |
616 | + Y.Assert.areEqual( |
617 | + extract_candidate_bug_id( |
618 | + {'source_git_path': 'refs/heads/foo-123456-bar'}), |
619 | + '123456'); |
620 | } |
621 | |
622 | })); |
623 | |
624 | === modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt' |
625 | --- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2015-06-23 17:30:39 +0000 |
626 | +++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2016-07-29 16:54:16 +0000 |
627 | @@ -88,6 +88,8 @@ |
628 | lp://dev/~name12/gnome-terminal/pushed |
629 | To merge this branch: |
630 | bzr merge lp://dev/~name12/gnome-terminal/klingon |
631 | + Related bugs: |
632 | + Link a bug report |
633 | |
634 | |
635 | Editing a commit message |
636 | @@ -457,23 +459,25 @@ |
637 | in the source branch. |
638 | |
639 | >>> def print_bugs_and_specs(browser): |
640 | - ... links = find_tag_by_id(browser.contents, |
641 | - ... 'related-bugs-and-blueprints') |
642 | - ... if links == None: |
643 | - ... print links |
644 | - ... else: |
645 | - ... print extract_text(links) |
646 | + ... for id in 'related-bugs', 'related-blueprints': |
647 | + ... links = find_tag_by_id(browser.contents, id) |
648 | + ... if links == None: |
649 | + ... print links |
650 | + ... else: |
651 | + ... print extract_text(links) |
652 | |
653 | >>> login('admin@canonical.com') |
654 | >>> bmp = factory.makeBranchMergeProposal() |
655 | >>> bmp_url = canonical_url(bmp) |
656 | >>> logout() |
657 | |
658 | -There shouldn't be a 'Linked bug reports and blueprints' section if there are |
659 | -none |
660 | +If there are no related bugs, the corresponding section should only show a |
661 | +"Link to bug report" link; if there are no related blueprints, there should |
662 | +be no corresponding section. |
663 | |
664 | >>> nopriv_browser.open(bmp_url) |
665 | >>> print_bugs_and_specs(nopriv_browser) |
666 | + Related bugs: Link a bug report |
667 | None |
668 | |
669 | >>> login('admin@canonical.com') |
670 | @@ -485,7 +489,8 @@ |
671 | |
672 | >>> nopriv_browser.open(bmp_url) |
673 | >>> print_bugs_and_specs(nopriv_browser) |
674 | - Related bugs and blueprints: Bug #...: Bug for linking Undecided New |
675 | + Related bugs: Bug #...: Bug for linking Undecided New Link a bug report |
676 | + None |
677 | |
678 | |
679 | Target branch edge cases |
680 | |
681 | === modified file 'lib/lp/code/templates/branch-macros.pt' |
682 | --- lib/lp/code/templates/branch-macros.pt 2016-05-14 09:53:33 +0000 |
683 | +++ lib/lp/code/templates/branch-macros.pt 2016-07-29 16:54:16 +0000 |
684 | @@ -98,6 +98,10 @@ |
685 | branch - the branch that has the bug branch links |
686 | The following variables define what is shown and are optional. |
687 | show_edit - show the edit form |
688 | + |
689 | + The bug-links macro in |
690 | + lib/lp/code/templates/branchmergeproposal-macros.pt has similar code for |
691 | + Git. |
692 | </tal:comment> |
693 | |
694 | <table> |
695 | |
696 | === modified file 'lib/lp/code/templates/branchmergeproposal-macros.pt' |
697 | --- lib/lp/code/templates/branchmergeproposal-macros.pt 2015-05-12 17:14:52 +0000 |
698 | +++ lib/lp/code/templates/branchmergeproposal-macros.pt 2016-07-29 16:54:16 +0000 |
699 | @@ -67,4 +67,70 @@ |
700 | |
701 | </metal:active-reviews> |
702 | |
703 | +<metal:bug-summary define-macro="bug-summary"> |
704 | + |
705 | + <tal:comment condition="nothing"> |
706 | + This macro requires the following defined variables: |
707 | + proposal - the linked merge proposal |
708 | + bug - the linked bug |
709 | + |
710 | + lib/lp/bugs/templates/bug-branch.pt has similar code for Bazaar. |
711 | + </tal:comment> |
712 | + |
713 | + <div class="bug-mp-summary" |
714 | + tal:define="show_edit bug/required:launchpad.Edit" |
715 | + tal:condition="proposal/required:launchpad.View"> |
716 | + <a tal:attributes="href proposal/merge_source/fmt:url" |
717 | + class="sprite branch" |
718 | + tal:content="proposal/merge_source/display_name"/> |
719 | + <a title="Remove link" |
720 | + tal:condition="show_edit" |
721 | + tal:attributes="href string:${proposal/fmt:url}/+unlinkbug?field.bugs=${bug/id}"> |
722 | + <img src="/@@/remove" alt="Remove"/> |
723 | + </a> |
724 | + <div class="reviews"> |
725 | + <tal:merge-fragment |
726 | + tal:replace="structure proposal/@@+summary-fragment"/> |
727 | + </div> |
728 | + </div> |
729 | + |
730 | +</metal:bug-summary> |
731 | + |
732 | +<metal:bug-links define-macro="bug-links"> |
733 | + |
734 | + <tal:comment condition="nothing"> |
735 | + The bug-branch-links macro in lib/lp/code/templates/branch-macros.pt has |
736 | + similar code for Bazaar. |
737 | + </tal:comment> |
738 | + |
739 | + <table tal:condition="view/linked_bugtasks"> |
740 | + <tal:bug-tasks repeat="bugtask view/linked_bugtasks"> |
741 | + <tr tal:condition="bugtask/bug/required:launchpad.View" |
742 | + tal:attributes="id string:buglink-${bugtask/bug/id}" |
743 | + class="bug-mp-summary"> |
744 | + <td tal:content="structure bugtask/fmt:link:bugs" class="first"/> |
745 | + <td> |
746 | + <span tal:content="bugtask/importance/title" |
747 | + tal:attributes="class string:importance${bugtask/importance/name}" |
748 | + >Critical</span> |
749 | + </td> |
750 | + <td> |
751 | + <span tal:content="bugtask/status/title" |
752 | + tal:attributes="class string:status${bugtask/status/name}" |
753 | + >Triaged</span> |
754 | + </td> |
755 | + <td> |
756 | + <a title="Remove link" |
757 | + class="delete-buglink" |
758 | + tal:attributes="href string:+unlinkbug?field.bugs=${bugtask/bug/id}; |
759 | + id string:delete-buglink-${bugtask/bug/id}"> |
760 | + <img src="/@@/remove" alt="Remove"/> |
761 | + </a> |
762 | + </td> |
763 | + </tr> |
764 | + </tal:bug-tasks> |
765 | + </table> |
766 | + |
767 | +</metal:bug-links> |
768 | + |
769 | </tal:root> |
770 | |
771 | === modified file 'lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt' |
772 | --- lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt 2015-08-28 15:03:12 +0000 |
773 | +++ lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt 2016-07-29 16:54:16 +0000 |
774 | @@ -129,11 +129,40 @@ |
775 | <th>To merge this branch:</th> |
776 | <td>bzr merge <span class="branch-url" tal:content="context/source_branch/bzr_identity" /></td> |
777 | </tr> |
778 | - <tr id="related-bugs-and-blueprints" tal:condition="view/has_bug_or_spec" > |
779 | - <th>Related bugs and blueprints:</th> |
780 | + <tr id="related-bugs"> |
781 | + <th>Related bugs:</th> |
782 | + <td> |
783 | + <metal:bug-links use-macro="context/@@+bmp-macros/bug-links"/> |
784 | + <div tal:define="link context_menu/link_bug" |
785 | + tal:condition="link/enabled"> |
786 | + <a id="linkbug" |
787 | + class="sprite add" |
788 | + tal:attributes="href link/url" |
789 | + tal:content="link/text" /> |
790 | + </div> |
791 | + <script type="text/javascript"> |
792 | + LPJS.use('io-base', 'lp.code.branch.bugspeclinks', function(Y) { |
793 | + Y.on('domready', function() { |
794 | + var logged_in = LP.links['me'] !== undefined; |
795 | + |
796 | + if (logged_in) { |
797 | + var config = { |
798 | + picker_activator = '#linkbug' |
799 | + }; |
800 | + var linked_bug_picker = new |
801 | + Y.lp.code.branch.bugspeclinks.LinkedBugPicker(config); |
802 | + linked_bug_picker.render(); |
803 | + linked_bug_picker.hide(); |
804 | + } |
805 | + }); |
806 | + }); |
807 | + </script> |
808 | + </td> |
809 | + </tr> |
810 | + <tr id="related-blueprints" tal:condition="view/has_specs"> |
811 | + <th>Related blueprints:</th> |
812 | <td tal:define="branch context/source_branch"> |
813 | - <metal:bug-branch-links use-macro="branch/@@+macros/bug-branch-links"/> |
814 | - <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/> |
815 | + <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/> |
816 | </td> |
817 | </tr> |
818 | </tbody> |
819 | |
820 | === added file 'lib/lp/code/templates/branchmergeproposal-unlinkbugs.pt' |
821 | --- lib/lp/code/templates/branchmergeproposal-unlinkbugs.pt 1970-01-01 00:00:00 +0000 |
822 | +++ lib/lp/code/templates/branchmergeproposal-unlinkbugs.pt 2016-07-29 16:54:16 +0000 |
823 | @@ -0,0 +1,27 @@ |
824 | +<html |
825 | + xmlns="http://www.w3.org/1999/xhtml" |
826 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
827 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
828 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
829 | + xml:lang="en" |
830 | + lang="en" |
831 | + dir="ltr" |
832 | + metal:use-macro="context/@@unlinkbugs_template/master" |
833 | + i18n:domain="launchpad"> |
834 | + |
835 | + <body> |
836 | + <div class="documentDescription" metal:fill-slot="extra_info"> |
837 | + <div> |
838 | + <a tal:attributes="href context/fmt:url"> |
839 | + <strong> |
840 | + <tal:merge-source replace="context/merge_source/identity"/> |
841 | + </strong> |
842 | + ⇒ |
843 | + <tal:merge-target replace="context/merge_target/identity"/> |
844 | + </a> |
845 | + </div> |
846 | + This will <em>remove</em> the link between the merge proposal and the |
847 | + selected bug reports. |
848 | + </div> |
849 | + </body> |
850 | +</html> |
Thanks, Colin.
It attacks the most exposed side (linking a bug to the context MP), I am happy to start with that and figure out if the other side (linking a MP to a context bug) is a needed.