opendev/gerrit
Code Issues Proposed changes

Merge "Implement isOnParent as functions rather than an attribute on comments"

Browse Source
This commit is contained in:
Wyatt Allen
2017-03-21 23:23:03 +00:00
committed by Gerrit Code Review
parent b45f894f7f 0727efa3d1
commit 2d6928dc3c
9 changed files with 25 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
View File

@@ -34,11 +34,15 @@
return '/c/' + changeNum + '/' + patchNum + '/' + file;
},
_isOnParent: function(comment) {
return comment.side === 'PARENT';
},
_computeDiffLineURL: function(file, changeNum, patchNum, comment) {
var diffURL = this._computeFileDiffURL(file, changeNum, patchNum);
if (comment.line) {
diffURL += '#';
if (comment.__isOnParent) { diffURL += 'b'; }
if (this._isOnParent(comment)) { diffURL += 'b'; }
diffURL += comment.line;
}
return diffURL;
@@ -51,7 +55,7 @@
},
_computePatchDisplayName: function(comment) {
if (comment.__isOnParent) {
if (this._isOnParent(comment)) {
return 'Base, ';
}
if (comment.patch_set != this.patchNum) {

8
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
View File

@@ -64,14 +64,14 @@ limitations under the License.
});
test('_computeDiffLineURL', function() {
var comment = {line: 123, side: 'REIVISION', patch_set: 10};
var comment = {line: 123, side: 'REVISION', patch_set: 10};
var expected = '/c/<change>/<patch>/<file>#123';
var actual = element._computeDiffLineURL('<file>', '<change>', '<patch>',
comment);
assert.equal(actual, expected);
comment.line = 321;
comment.__isOnParent = true;
comment.side = 'PARENT';
expected = '/c/<change>/<patch>/<file>#b321';
actual = element._computeDiffLineURL('<file>', '<change>', '<patch>',
@@ -79,7 +79,7 @@ limitations under the License.
});
test('_computePatchDisplayName', function() {
var comment = {line: 123, side: 'REIVISION', patch_set: 10};
var comment = {line: 123, side: 'REVISION', patch_set: 10};
element.patchNum = 10;
assert.equal(element._computePatchDisplayName(comment), '');
@@ -87,7 +87,7 @@ limitations under the License.
element.patchNum = 9;
assert.equal(element._computePatchDisplayName(comment), 'PS10, ');
comment.__isOnParent = true;
comment.side = 'PARENT';
assert.equal(element._computePatchDisplayName(comment), 'Base, ');
});
});

2
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
View File

@@ -356,7 +356,7 @@
}
var patchNum = this._comments.meta.patchRange.patchNum;
var isOnParent = comments[0].__isOnParent || false ;
var isOnParent = comments[0].side === 'PARENT' || false;
if (line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) {
if (this._comments.meta.patchRange.basePatchNum === 'PARENT') {

2
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
View File

@@ -59,7 +59,7 @@ limitations under the License.
draft="[[comment.__draft]]"
show-actions="[[_showActions]]"
comment-side="[[comment.__commentSide]]"
is-on-parent="[[isOnParent]]"
side="[[comment.side]]"
project-config="[[projectConfig]]"
on-create-fix-comment="_handleCommentFix"
on-comment-discard="_handleCommentDiscard"></gr-diff-comment>

7
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
View File

@@ -272,7 +272,7 @@
__date: new Date(),
path: this.path,
patchNum: this.patchNum,
__isOnParent: this.__isOnParent,
side: this._getSide(this.isOnParent),
__commentSide: this.commentSide,
};
if (opt_lineNum) {
@@ -289,6 +289,11 @@
return d;
},
_getSide: function(isOnParent) {
if (isOnParent) { return 'PARENT'; }
return 'REVISION';
},
_handleCommentDiscard: function(e) {
var diffCommentEl = Polymer.dom(e).rootTarget;
var comment = diffCommentEl.comment;

12
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
View File

@@ -137,12 +137,13 @@
this.isRobotComment = !!comment.robot_id;
},
isOnParent: function() {
return this.side === 'PARENT';
},
save: function() {
this.comment.message = this._messageText;
// Translate {Boolean} __isOnParent to {String} side for the REST API
// format.
this.comment.side = this.comment.__isOnParent ? 'PARENT' : null;
this.disabled = true;
this._eraseDraftComment();
@@ -153,7 +154,6 @@
return this.$.restAPI.getResponseObject(response).then(function(obj) {
var comment = obj;
comment.__isOnParent = comment.side === 'PARENT';
comment.__draft = true;
// Maintain the ephemeral draft ID for identification by other
// elements.
@@ -407,18 +407,16 @@
},
_saveDraft: function(draft) {
draft.side = draft.__isOnParent ? 'PARENT' : null;
return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft);
},
_deleteDraft: function(draft) {
draft.side = draft.__isOnParent ? 'PARENT' : null;
return this.$.restAPI.deleteDiffDraft(this.changeNum, this.patchNum,
draft);
},
_getPatchNum: function() {
return this.isOnParent ? 'PARENT' : this.patchNum;
return this.isOnParent() ? 'PARENT' : this.patchNum;
},
_loadLocalDraft: function(changeNum, patchNum, comment) {

5
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
View File

@@ -153,10 +153,10 @@ limitations under the License.
});
test('_getPatchNum', function() {
element.isOnParent = true;
element.side = 'PARENT';
element.patchNum = 1;
assert.equal(element._getPatchNum(), 'PARENT');
element.isOnParent = false;
element.side = 'REVISION';
assert.equal(element._getPatchNum(), 1);
});
@@ -508,7 +508,6 @@ limitations under the License.
assert.deepEqual(fireStub.lastCall.args[1], {
comment: {
__commentSide: 'right',
__isOnParent: false,
__draft: true,
__draftID: 'temp_draft_id',
__editing: false,

5
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -781,7 +781,6 @@
function onlyParent(c) { return c.side == PARENT_PATCH_NUM; }
function withoutParent(c) { return c.side != PARENT_PATCH_NUM; }
function setPath(c) { c.path = opt_path; }
function setIsOnParent(c) { c.__isOnParent = true; }
var promises = [];
var comments;
@@ -801,10 +800,6 @@
if (opt_basePatchNum == PARENT_PATCH_NUM) {
baseComments = comments.filter(onlyParent);
baseComments.forEach(setPath);
// Translate {String} side to {Boolean} __isOnParent for readability
// in the code.
baseComments.forEach(setIsOnParent);
}
comments = comments.filter(withoutParent);

1
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
View File

@@ -150,7 +150,6 @@ limitations under the License.
assert.equal(obj.baseComments.length, 1);
assert.deepEqual(obj.baseComments[0], {
side: 'PARENT',
__isOnParent: true,
message: 'how did this work in the first place?',
path: 'sieve.go',
updated: '2017-02-03 22:33:28.000000000',