Update URL generation in gr-comment-list
The gr-message-list component and its children need to the project
config as well as the project name for linkification and URL
construction respectively. With this change, instead of taking the
project config, the message list takes the project name and uses it for
both purposes.
Bug: Issue 6446
Change-Id: I551b0f0cbb74b71e10c35a613b525aa281b3f740
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index cb304d8..b1cbe3e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -523,7 +523,7 @@
messages="[[_change.messages]]"
reviewer-updates="[[_change.reviewer_updates]]"
comments="[[_comments]]"
- project-config="[[_projectConfig]]"
+ project-name="[[_change.project]]"
show-reply-buttons="[[_loggedIn]]"
on-reply="_handleMessageReply"></gr-messages-list>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
index 9094933..2b7b0fd 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
@@ -18,6 +18,7 @@
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-formatted-text/gr-formatted-text.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -76,7 +77,7 @@
class="message"
no-trailing-margin
content="[[comment.message]]"
- config="[[projectConfig.commentlinks]]"></gr-formatted-text>
+ config="[[commentLinks]]"></gr-formatted-text>
</div>
</template>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
index d8cdb02..703f386 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
@@ -30,7 +30,8 @@
changeNum: Number,
comments: Object,
patchNum: Number,
- projectConfig: Object,
+ commentLinks: Object,
+ projectName: String,
},
_computeFilesFromComments(comments) {
@@ -39,8 +40,8 @@
},
_computeFileDiffURL(file, changeNum, patchNum) {
- return this.getBaseUrl() + '/c/' + changeNum + '/' + patchNum + '/' +
- this.encodeURL(file);
+ return Gerrit.Nav.getUrlForDiffById(this.changeNum, this.projectName,
+ file, patchNum);
},
_computeFileDisplayName(path) {
@@ -57,13 +58,8 @@
},
_computeDiffLineURL(file, changeNum, patchNum, comment) {
- let diffURL = this._computeFileDiffURL(file, changeNum, patchNum);
- if (comment.line) {
- diffURL += '#';
- if (this._isOnParent(comment)) { diffURL += 'b'; }
- diffURL += comment.line;
- }
- return diffURL;
+ return Gerrit.Nav.getUrlForDiffById(this.changeNum, this.projectName,
+ file, patchNum, null, comment.line, this._isOnParent(comment));
},
_computeCommentsForFile(comments, file) {
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
index d25664e..ed1ece6 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
@@ -60,12 +60,6 @@
assert.deepEqual(element._computeFilesFromComments(null), []);
});
- test('_computeFileDiffURL', () => {
- const expected = '/c/change/patch/file';
- const actual = element._computeFileDiffURL('file', 'change', 'patch');
- assert.equal(actual, expected);
- });
-
test('_computeFileDisplayName', () => {
assert.equal(element._computeFileDisplayName('/COMMIT_MSG'),
'Commit message');
@@ -75,27 +69,6 @@
'/foo/bar/baz');
});
- test('_computeDiffLineURL', () => {
- const comment = {line: 123, side: 'REVISION', patch_set: 10};
- let expected = '/c/change/patch/file#123';
- let actual = element._computeDiffLineURL('file', 'change', 'patch',
- comment);
- assert.equal(actual, expected);
-
- comment.line = 321;
- comment.side = 'PARENT';
-
- expected = '/c/change/patch/file#b321';
- actual = element._computeDiffLineURL('file', 'change', 'patch', comment);
- });
-
- test('_computeDiffLineURL encoding', () => {
- const comment = {line: 123, side: 'REVISION', patch_set: 10};
- const expected = '/c/123/2/x%252By.md#123';
- const actual = element._computeDiffLineURL('x+y.md', '123', '2', comment);
- assert.equal(actual, expected);
- });
-
test('_computePatchDisplayName', () => {
const comment = {line: 123, side: 'REVISION', patch_set: 10};
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index d034f16..78f59db 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -152,12 +152,13 @@
<gr-formatted-text
class="message hideOnCollapsed"
content="[[message.message]]"
- config="[[projectConfig.commentlinks]]"></gr-formatted-text>
+ config="[[_commentLinks]]"></gr-formatted-text>
<gr-comment-list
comments="[[comments]]"
change-num="[[changeNum]]"
patch-num="[[message._revision_number]]"
- project-config="[[projectConfig]]"></gr-comment-list>
+ project-name="[[projectName]]"
+ comment-links="[[_commentLinks]]"></gr-comment-list>
</div>
</template>
<template is="dom-if" if="[[_computeIsReviewerUpdate(message)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 3433201..2593869 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -74,7 +74,11 @@
type: Boolean,
computed: '_computeShowReplyButton(message, _loggedIn)',
},
- projectConfig: Object,
+ projectName: {
+ type: String,
+ observer: '_projectNameChanged',
+ },
+ _commentLinks: Object,
// Computed property needed to trigger Polymer value observing.
_expanded: {
type: Object,
@@ -240,5 +244,11 @@
return this.getAnonymousName(this.config);
},
+
+ _projectNameChanged(name) {
+ this.$.restAPI.getProjectConfig(name).then(config => {
+ this._commentLinks = config.commentlinks;
+ });
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index 95ca60e..2eed88b 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -95,7 +95,7 @@
message="[[message]]"
comments="[[_computeCommentsForMessage(comments, message)]]"
hide-automated="[[_hideAutomated]]"
- project-config="[[projectConfig]]"
+ project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-scroll-to="_handleScrollTo"
data-message-id$="[[message.id]]"></gr-message>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index ebdb4dc..58e56a4 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -36,7 +36,7 @@
value() { return []; },
},
comments: Object,
- projectConfig: Object,
+ projectName: String,
showReplyButtons: {
type: Boolean,
value: false,
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
index dd9709d..3fc2c22 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
@@ -43,6 +43,10 @@
// - `basePatchNum`, optional, Number, the patch for the left-hand-side
// of the diff. If `basePatchNum` is provided, then `patchNum` must
// also be provided.
+ // - `lineNum`, optional, Number, the line number to be selected on load.
+ // - `leftSide`, optional, Boolean, if a `lineNum` is provided, a value
+ // of true selects the line from base of the patch range. False by
+ // default.
window.Gerrit = window.Gerrit || {};
@@ -166,9 +170,9 @@
/**
* @param {!Object} change The change object.
- * @param {number} opt_patchNum
- * @param {number|string} opt_basePatchNum The string 'PARENT' can be used
- * for none.
+ * @param {number=} opt_patchNum
+ * @param {number|string=} opt_basePatchNum The string 'PARENT' can be
+ * used for none.
* @return {string}
*/
getUrlForChange(change, opt_patchNum, opt_basePatchNum) {
@@ -187,9 +191,9 @@
/**
* @param {!Object} change The change object.
- * @param {number} opt_patchNum
- * @param {number|string} opt_basePatchNum The string 'PARENT' can be used
- * for none.
+ * @param {number=} opt_patchNum
+ * @param {number|string=} opt_basePatchNum The string 'PARENT' can be
+ * used for none.
* @return {string}
*/
navigateToChange(change, opt_patchNum, opt_basePatchNum) {
@@ -199,13 +203,30 @@
/**
* @param {!Object} change The change object.
- * @param {!String} path The file path.
- * @param {number} opt_patchNum
- * @param {number|string} opt_basePatchNum The string 'PARENT' can be used
- * for none.
+ * @param {!string} path The file path.
+ * @param {number=} opt_patchNum
+ * @param {number|string=} opt_basePatchNum The string 'PARENT' can be
+ * used for none.
* @return {string}
*/
getUrlForDiff(change, path, opt_patchNum, opt_basePatchNum) {
+ return this.getUrlForDiffById(change._number, change.project, path,
+ opt_patchNum, opt_basePatchNum);
+ },
+
+ /**
+ * @param {!number} change The change object.
+ * @param {!string} projectName The name of the project.
+ * @param {!string} path The file path.
+ * @param {number=} opt_patchNum
+ * @param {number|string=} opt_basePatchNum The string 'PARENT' can be
+ * used for none.
+ * @param {number=} opt_lineNum
+ * @param {boolean=} opt_leftSide
+ * @return {string}
+ */
+ getUrlForDiffById(changeNum, projectName, path, opt_patchNum,
+ opt_basePatchNum, opt_lineNum, opt_leftSide) {
if (opt_basePatchNum === PARENT_PATCHNUM) {
opt_basePatchNum = undefined;
}
@@ -213,19 +234,22 @@
this._checkPatchRange(opt_patchNum, opt_basePatchNum);
return this._getUrlFor({
view: Gerrit.Nav.View.DIFF,
- changeNum: change._number,
+ changeNum,
+ projectName,
path,
patchNum: opt_patchNum,
basePatchNum: opt_basePatchNum,
+ lineNum: opt_lineNum,
+ leftSide: opt_leftSide,
});
},
/**
* @param {!Object} change The change object.
- * @param {!String} path The file path.
- * @param {number} opt_patchNum
- * @param {number|string} opt_basePatchNum The string 'PARENT' can be used
- * for none.
+ * @param {!string} path The file path.
+ * @param {number=} opt_patchNum
+ * @param {number|string=} opt_basePatchNum The string 'PARENT' can be
+ * used for none.
*/
navigateToDiff(change, path, opt_patchNum, opt_basePatchNum) {
this._navigate(this.getUrlForDiff(change, path, opt_patchNum,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index f290dd2..f393445 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -580,6 +580,12 @@
if (range.length) { range = '/' + range; }
url = `/c/${params.changeNum}${range}/${encode(params.path, true)}`;
+
+ if (params.lineNum) {
+ url += '#';
+ if (params.leftSide) { url += 'b'; }
+ url += params.lineNum;
+ }
} else {
throw new Error('Can\'t generate');
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index ff1a0ca..92d0fde 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -93,6 +93,13 @@
delete params.basePatchNum;
assert.equal(element._generateUrl(params),
'/c/42/2/foo+bar/my%252Bfile.txt%2525');
+
+ params.path = 'file.cpp';
+ params.lineNum = 123;
+ assert.equal(element._generateUrl(params), '/c/42/2/file.cpp#123');
+
+ params.leftSide = true;
+ assert.equal(element._generateUrl(params), '/c/42/2/file.cpp#b123');
});
});
});
