launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21468
[Merge] lp:~cjwatson/launchpad/fix-mp-extract-candidate-bug-id into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-mp-extract-candidate-bug-id into lp:launchpad.
Commit message:
Fix extract_candidate_bug_id for merge proposals.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/fix-mp-extract-candidate-bug-id/+merge/322531
Nested objects aren't expanded in the client-side JSON cache, but just rendered as foo_link, so the previous approach didn't work. Instead, just precalculate the branch name and have the JS code rely on that.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-mp-extract-candidate-bug-id into lp:launchpad.
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2016-07-29 16:13:36 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2017-04-13 14:17:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Views, navigation and actions for BranchMergeProposals."""
@@ -605,6 +605,7 @@
def initialize(self):
super(BranchMergeProposalView, self).initialize()
cache = IJSONRequestCache(self.request)
+ cache.objects['branch_name'] = self.context.merge_source.name
if IBranch.providedBy(self.context.merge_source):
cache.objects.update({
'branch_diff_link':
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2016-09-07 11:12:58 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2017-04-13 14:17:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for BranchMergeProposals."""
@@ -24,6 +24,7 @@
Tag,
)
from testtools.matchers import (
+ ContainsDict,
DocTestMatches,
Equals,
Is,
@@ -1593,6 +1594,32 @@
"%.7s...\nby\nExample Person <person@xxxxxxxxxxx>\n"
"on 2015-01-01" % sha1, extract_text(tag))
+ def test_client_cache_bzr(self):
+ # For Bazaar, the client cache contains the branch name and a
+ # loggerhead-based diff link.
+ bmp = self.factory.makeBranchMergeProposal()
+ view = create_initialized_view(bmp, '+index')
+ cache = IJSONRequestCache(view.request)
+ self.assertThat(cache.objects, ContainsDict({
+ 'branch_name': Equals(bmp.source_branch.name),
+ 'branch_diff_link': Equals(
+ 'https://code.launchpad.dev/+loggerhead/%s/diff/' %
+ bmp.source_branch.unique_name),
+ }))
+
+ def test_client_cache_git(self):
+ # For Git, the client cache contains the ref name and a webapp-based
+ # diff link.
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ view = create_initialized_view(bmp, '+index')
+ cache = IJSONRequestCache(view.request)
+ self.assertThat(cache.objects, ContainsDict({
+ 'branch_name': Equals(bmp.source_git_ref.name),
+ 'branch_diff_link': Equals(
+ 'http://code.launchpad.dev/%s/+diff/' %
+ bmp.source_git_repository.unique_name),
+ }))
+
class TestBranchMergeProposalBrowserView(BrowserTestCase):
=== modified file 'lib/lp/code/javascript/branch.bugspeclinks.js'
--- lib/lp/code/javascript/branch.bugspeclinks.js 2016-06-25 08:05:06 +0000
+++ lib/lp/code/javascript/branch.bugspeclinks.js 2017-04-13 14:17:39 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the
+/* Copyright 2012-2017 Canonical Ltd. This software is licensed under the
* GNU Affero General Public License version 3 (see the file LICENSE).
*
* Provide functionality for picking a bug.
@@ -12,19 +12,14 @@
var superclass = Y.lp.bugs.bug_picker.BugPicker;
/*
- * Extract the best candidate for a bug number from the context.
+ * Extract the best candidate for a bug number from the cache.
*/
-namespace.extract_candidate_bug_id = function(context) {
+namespace.extract_candidate_bug_id = function(cache) {
var branch_name;
- if (context.source_branch !== undefined) {
- branch_name = context.source_branch.name;
- } else if (context.source_git_path !== undefined) {
- branch_name = context.source_git_path;
- if (branch_name.indexOf('refs/heads/') === 0) {
- branch_name = branch_name.slice('refs/heads/'.length);
- }
+ if (cache.branch_name !== undefined) {
+ branch_name = cache.branch_name;
} else {
- branch_name = context.name;
+ branch_name = cache.context.name;
}
// Extract all the runs of numbers in the branch name and sort by
// descending length.
@@ -72,7 +67,7 @@
this.after('visibleChange', function() {
if (this.get('visible')) {
var guessed_bug_id =
- namespace.extract_candidate_bug_id(LP.cache.context);
+ namespace.extract_candidate_bug_id(LP.cache);
if (Y.Lang.isValue(guessed_bug_id)) {
this._search_input.set('value', guessed_bug_id);
// Select the pre-filled bug number (if any) so that it will
=== modified file 'lib/lp/code/javascript/tests/test_bugspeclinks.js'
--- lib/lp/code/javascript/tests/test_bugspeclinks.js 2016-06-25 08:05:06 +0000
+++ lib/lp/code/javascript/tests/test_bugspeclinks.js 2017-04-13 14:17:39 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the
+/* Copyright 2012-2017 Canonical Ltd. This software is licensed under the
* GNU Affero General Public License version 3 (see the file LICENSE). */
YUI.add('lp.code.branch.bugspeclinks.test', function (Y) {
@@ -14,44 +14,51 @@
test_no_bug_id_present: function() {
// If nothing that looks like a bug ID is present, null is
// returned.
- Y.Assert.isNull(extract_candidate_bug_id({'name': 'no-id-here'}));
+ Y.Assert.isNull(extract_candidate_bug_id(
+ {'context': {'name': 'no-id-here'}}));
},
test_short_digit_rund_ignored: function() {
- Y.Assert.isNull(
- extract_candidate_bug_id({'name': 'foo-1234-bar'}));
+ Y.Assert.isNull(extract_candidate_bug_id(
+ {'context': {'name': 'foo-1234-bar'}}));
},
test_leading_zeros_disqualify_potential_ids: function() {
// Since bug IDs can't start with zeros, any string of numbers
// with a leading zero are not considered as a potential ID.
- Y.Assert.isNull(
- extract_candidate_bug_id({'name': 'foo-0123456-bar'}));
+ Y.Assert.isNull(extract_candidate_bug_id(
+ {'context': {'name': 'foo-0123456-bar'}}));
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'foo-0123456-999999-bar'}),
+ extract_candidate_bug_id(
+ {'context': {'name': 'foo-0123456-999999-bar'}}),
'999999');
},
test_five_digit_bug_ids_are_extracted: function() {
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'foo-12345-bar'}), '12345');
+ extract_candidate_bug_id(
+ {'context': {'name': 'foo-12345-bar'}}),
+ '12345');
},
test_six_digit_bug_ids_are_extracted: function() {
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'foo-123456-bar'}),
+ extract_candidate_bug_id(
+ {'context': {'name': 'foo-123456-bar'}}),
'123456');
},
test_seven_digit_bug_ids_are_extracted: function() {
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'foo-1234567-bar'}),
+ extract_candidate_bug_id(
+ {'context': {'name': 'foo-1234567-bar'}}),
'1234567');
},
test_eight_digit_bug_ids_are_extracted: function() {
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'foo-12345678-bar'}),
+ extract_candidate_bug_id(
+ {'context': {'name': 'foo-12345678-bar'}}),
'12345678');
},
@@ -59,28 +66,20 @@
// Since there may be numbers other than a bug ID in a branch
// name, we want to extract the longest string of digits.
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': 'bug-123456-take-2'}),
+ extract_candidate_bug_id(
+ {'context': {'name': 'bug-123456-take-2'}}),
'123456');
Y.Assert.areEqual(
- extract_candidate_bug_id({'name': '123456-1234567'}),
+ extract_candidate_bug_id(
+ {'context': {'name': '123456-1234567'}}),
'1234567');
},
- test_merge_proposal_bzr: function() {
- // If the context is a Bazaar-based merge proposal, bug IDs are
- // extracted from the source branch.
- Y.Assert.areEqual(
- extract_candidate_bug_id(
- {'source_branch': {'name': 'foo-123456-bar'}}),
- '123456');
- },
-
- test_merge_proposal_git: function() {
- // If the context is a Git-based merge proposal, bug IDs are
- // extracted from the source reference path.
- Y.Assert.areEqual(
- extract_candidate_bug_id(
- {'source_git_path': 'refs/heads/foo-123456-bar'}),
+ test_merge_proposal: function() {
+ // If the context is a merge proposal, bug IDs are extracted
+ // from the source branch name.
+ Y.Assert.areEqual(
+ extract_candidate_bug_id({'branch_name': 'foo-123456-bar'}),
'123456');
}
Follow ups
