Merge "Add "owner" field to create repo dialog"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
index b2fc64c..11a36b3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
@@ -14,14 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-(function(window, GrDiffBuilderSideBySide) {
+(function(window, GrDiffBuilder) {
'use strict';
// Prevent redefinition.
if (window.GrDiffBuilderBinary) { return; }
- function GrDiffBuilderBinary(diff, comments, prefs, outputEl) {
- GrDiffBuilder.call(this, diff, comments, null, prefs, outputEl);
+ function GrDiffBuilderBinary(diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl) {
+ GrDiffBuilder.call(this, diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl);
}
GrDiffBuilderBinary.prototype = Object.create(GrDiffBuilder.prototype);
@@ -43,4 +45,4 @@
};
window.GrDiffBuilderBinary = GrDiffBuilderBinary;
-})(window, GrDiffBuilderSideBySide);
+})(window, GrDiffBuilder);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index 88ff79b..dce66e1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -22,10 +22,10 @@
const IMAGE_MIME_PATTERN = /^image\/(bmp|gif|jpeg|jpg|png|tiff|webp)$/;
- function GrDiffBuilderImage(diff, comments, createThreadGroupFn, prefs,
- outputEl, baseImage, revisionImage) {
- GrDiffBuilderSideBySide.call(this, diff, comments, createThreadGroupFn,
- prefs, outputEl, []);
+ function GrDiffBuilderImage(diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, baseImage, revisionImage) {
+ GrDiffBuilderSideBySide.call(this, diff, comments, parentIndex, changeNum,
+ path, projectName, prefs, outputEl, []);
this._baseImage = baseImage;
this._revisionImage = revisionImage;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index fafae63..90688fc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -20,10 +20,10 @@
// Prevent redefinition.
if (window.GrDiffBuilderSideBySide) { return; }
- function GrDiffBuilderSideBySide(diff, comments, createThreadGroupFn, prefs,
- outputEl, layers) {
- GrDiffBuilder.call(this, diff, comments, createThreadGroupFn, prefs,
- outputEl, layers);
+ function GrDiffBuilderSideBySide(diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, layers) {
+ GrDiffBuilder.call(this, diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, layers);
}
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
index 9a04b1f..1c9c1b2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
@@ -20,10 +20,10 @@
// Prevent redefinition.
if (window.GrDiffBuilderUnified) { return; }
- function GrDiffBuilderUnified(diff, comments, createThreadGroupFn, prefs,
- outputEl, layers) {
- GrDiffBuilder.call(this, diff, comments, createThreadGroupFn, prefs,
- outputEl, layers);
+ function GrDiffBuilderUnified(diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, layers) {
+ GrDiffBuilder.call(this, diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, layers);
}
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderUnified.prototype.constructor = GrDiffBuilderUnified;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 0997fee..91cd0cf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -113,17 +113,14 @@
isImageDiff: Boolean,
baseImage: Object,
revisionImage: Object,
+ parentIndex: Number,
+ path: String,
projectName: String,
/**
* @type {Defs.LineOfInterest|null}
*/
lineOfInterest: Object,
- /**
- * @type {function(number, booleam, !string)}
- */
- createCommentFn: Function,
-
_builder: Object,
_groups: Array,
_layers: Array,
@@ -305,19 +302,22 @@
}
let builder = null;
- const createFn = this.createCommentFn;
if (this.isImageDiff) {
- builder = new GrDiffBuilderImage(diff, comments, createFn, prefs,
+ builder = new GrDiffBuilderImage(diff, comments, this.parentIndex,
+ this.changeNum, this.path, this.projectName, prefs,
this.diffElement, this.baseImage, this.revisionImage);
} else if (diff.binary) {
// If the diff is binary, but not an image.
- return new GrDiffBuilderBinary(diff, comments, prefs,
+ return new GrDiffBuilderBinary(diff, comments, this.parentIndex,
+ this.changeNum, this.path, this.projectName, prefs,
this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
- builder = new GrDiffBuilderSideBySide(diff, comments, createFn,
+ builder = new GrDiffBuilderSideBySide(diff, comments,
+ this.parentIndex, this.changeNum, this.path, this.projectName,
prefs, this.diffElement, this._layers);
} else if (this.viewMode === DiffViewMode.UNIFIED) {
- builder = new GrDiffBuilderUnified(diff, comments, createFn, prefs,
+ builder = new GrDiffBuilderUnified(diff, comments, this.parentIndex,
+ this.changeNum, this.path, this.projectName, prefs,
this.diffElement, this._layers);
}
if (!builder) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index be53fda..7b568d1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -20,6 +20,60 @@
// Prevent redefinition.
if (window.GrDiffBuilder) { return; }
+ /** @enum {string} */
+ Gerrit.DiffSide = {
+ LEFT: 'left',
+ RIGHT: 'right',
+ BOTH: 'both',
+ };
+
+ /**
+ * @param {!Array<!HTMLElement>} threadEls
+ * @param {!{beforeNumber: (number|string|undefined),
+ * afterNumber: (number|string|undefined)}}
+ * lineInfo
+ * @param {!Gerrit.DiffSide=} side The side (LEFT, RIGHT, BOTH) for
+ * which to return the threads (default: BOTH).
+ * @return {!Array<!HTMLElement>} The thread elements matching the given
+ * location.
+ */
+ Gerrit.filterThreadElsForLocation = function(
+ threadEls, lineInfo, side = Gerrit.DiffSide.BOTH) {
+ function matchesLeftLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.LEFT &&
+ threadEl.getAttribute('line-num') == lineInfo.beforeNumber;
+ }
+ function matchesRightLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.RIGHT &&
+ threadEl.getAttribute('line-num') == lineInfo.afterNumber;
+ }
+ function matchesFileComment(threadEl) {
+ return (side === Gerrit.DiffSide.BOTH ||
+ threadEl.getAttribute('comment-side') == side) &&
+ // line/range comments have 1-based line set, if line is falsy it's
+ // a file comment
+ !threadEl.getAttribute('line-num');
+ }
+
+ // Select the appropriate matchers for the desired side and line
+ // If side is BOTH, we want both the left and right matcher.
+ const matchers = [];
+ if (side !== Gerrit.DiffSide.RIGHT) {
+ matchers.push(matchesLeftLine);
+ }
+ if (side !== Gerrit.DiffSide.LEFT) {
+ matchers.push(matchesRightLine);
+ }
+ if (lineInfo.afterNumber === 'FILE' ||
+ lineInfo.beforeNumber === 'FILE') {
+ matchers.push(matchesFileComment);
+ }
+ return threadEls.filter(threadEl =>
+ matchers.some(matcher => matcher(threadEl)));
+ };
+
/**
* In JS, unicode code points above 0xFFFF occupy two elements of a string.
* For example '𐀏'.length is 2. An occurence of such a code point is called a
@@ -42,11 +96,14 @@
*/
const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
- function GrDiffBuilder(diff, comments, createThreadGroupFn, prefs, outputEl,
- layers) {
+ function GrDiffBuilder(diff, comments, parentIndex, changeNum, path,
+ projectName, prefs, outputEl, layers) {
this._diff = diff;
this._comments = comments;
- this._createThreadGroupFn = createThreadGroupFn;
+ this._parentIndex = parentIndex;
+ this._changeNum = changeNum;
+ this._path = path;
+ this._projectName = projectName;
this._prefs = prefs;
this._outputEl = outputEl;
this.groups = [];
@@ -68,6 +125,11 @@
}
}
+ if (!this._comments) {
+ this._threadEls = [];
+ return;
+ }
+
const allComments = [];
for (const side of [GrDiffBuilder.Side.LEFT, GrDiffBuilder.Side.RIGHT]) {
// This is needed by the threading.
@@ -76,7 +138,12 @@
}
allComments.push(...this._comments[side]);
}
- this._threads = this._createThreads(allComments);
+ const threads = this._createThreads(allComments);
+ this._threadEls = threads.map(thread => {
+ return Gerrit.createThreadElement(
+ thread, this._parentIndex, this._changeNum,
+ this._path, this._projectName);
+ });
}
GrDiffBuilder.GroupType = {
@@ -330,47 +397,6 @@
};
/**
- * @param {!Array<Object>} threads
- * @param {!GrDiffLine} line
- * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which
- * to return the threads (default: BOTH).
- */
- GrDiffBuilder.prototype._filterThreadsForLine = function(
- threads, line, side = GrDiffBuilder.Side.BOTH) {
- function matchesLeftLine(thread) {
- return thread.commentSide == GrDiffBuilder.Side.LEFT &&
- thread.comments[0].line == line.beforeNumber;
- }
- function matchesRightLine(thread) {
- return thread.commentSide == GrDiffBuilder.Side.RIGHT &&
- thread.comments[0].line == line.afterNumber;
- }
- function matchesFileComment(thread) {
- return (side === GrDiffBuilder.Side.BOTH ||
- thread.commentSide == side) &&
- // line/range comments have 1-based line set, if line is falsy it's
- // a file comment
- !thread.comments[0].line;
- }
-
- // Select the appropriate matchers for the desired side and line
- // If side is BOTH, we want both the left and right matcher.
- const matchers = [];
- if (side !== GrDiffBuilder.Side.RIGHT) {
- matchers.push(matchesLeftLine);
- }
- if (side !== GrDiffBuilder.Side.LEFT) {
- matchers.push(matchesRightLine);
- }
- if (line.afterNumber === GrDiffLine.FILE ||
- line.beforeNumber === GrDiffLine.FILE) {
- matchers.push(matchesFileComment);
- }
-
- return threads.filter(thread => matchers.find(matcher => matcher(thread)));
- };
-
- /**
* @param {Array<Object>} comments
*/
GrDiffBuilder.prototype._createThreads = function(comments) {
@@ -400,6 +426,8 @@
commentSide: comment.__commentSide,
patchNum: comment.patch_set,
rootId: comment.id || comment.__draftID,
+ lineNum: comment.line,
+ isOnParent: comment.side === 'PARENT',
};
if (comment.range) {
newThread.range = Object.assign({}, comment.range);
@@ -430,26 +458,6 @@
};
/**
- * Returns whether the comments on the given line are on a (merge) parent.
- *
- * @param {string} firstCommentSide
- * @param {{basePatchNum: number, patchNum: number}} patchRange
- * @param {GrDiffLine} line The line the comments are on.
- * @param {string=} opt_side
- * @return {boolean} True iff the comments on the given line are on a (merge)
- * parent.
- */
- GrDiffBuilder.prototype._determineIsOnParent = function(
- firstCommentSide, patchRange, line, opt_side) {
- return ((line.type === GrDiffLine.Type.REMOVE ||
- opt_side === GrDiffBuilder.Side.LEFT) &&
- (patchRange.basePatchNum === 'PARENT' ||
- Gerrit.PatchSetBehavior.isMergeParent(
- patchRange.basePatchNum))) ||
- firstCommentSide === 'PARENT';
- };
-
- /**
* @param {!GrDiffLine} line
* @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which to return
* the thread group (default: BOTH).
@@ -457,21 +465,17 @@
*/
GrDiffBuilder.prototype._commentThreadGroupForLine = function(
line, side = GrDiffBuilder.Side.BOTH) {
- const threads =
- this._filterThreadsForLine(this._threads, line, side);
- if (!threads || threads.length === 0) {
+ const threadElsForGroup =
+ Gerrit.filterThreadElsForLocation(this._threadEls, line, side);
+ if (!threadElsForGroup || threadElsForGroup.length === 0) {
return null;
}
- const patchRange = this._comments.meta.patchRange;
- const patchNumForNewThread = this._determinePatchNumForNewThreads(
- patchRange, line, side);
- const isOnParent = this._determineIsOnParent(
- threads[0].side, patchRange, line, side);
-
- const threadGroupEl = this._createThreadGroupFn(
- patchNumForNewThread, isOnParent, side);
- threadGroupEl.threads = threads;
+ const threadGroupEl =
+ document.createElement('gr-diff-comment-thread-group');
+ for (const threadEl of threadElsForGroup) {
+ Polymer.dom(threadGroupEl).appendChild(threadEl);
+ }
if (side) {
threadGroupEl.setAttribute('data-side', side);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 80b45a4..bd65703 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -60,9 +60,12 @@
let prefs;
let element;
let builder;
- let createThreadGroupFn;
let sandbox;
const LINE_FEED_HTML = '<span class="style-scope gr-diff br"></span>';
+ const parentIndex = 3;
+ const changeNum = 4;
+ const path = 'some/path.ext';
+ const projectName = 'Awesome Project';
setup(() => {
sandbox = sinon.sandbox.create();
@@ -76,27 +79,86 @@
show_tabs: true,
tab_size: 4,
};
- createThreadGroupFn = sinon.spy(() => ({
- setAttribute: sinon.spy(),
- }));
builder = new GrDiffBuilder(
- {content: []}, {left: [], right: []}, createThreadGroupFn, prefs);
+ {content: []}, {left: [], right: []}, parentIndex, changeNum, path,
+ projectName, prefs);
});
teardown(() => { sandbox.restore(); });
+ test('filterThreadElsForLocation with no threads', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
+
+ const threads = [];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line), []);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.LEFT), []);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.RIGHT), []);
+ });
+
+ test('filterThreadElsForLocation for line comments', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
+
+ const l3 = document.createElement('div');
+ l3.setAttribute('line-num', 3);
+ l3.setAttribute('comment-side', 'left');
+
+ const l5 = document.createElement('div');
+ l5.setAttribute('line-num', 5);
+ l5.setAttribute('comment-side', 'left');
+
+ const r3 = document.createElement('div');
+ r3.setAttribute('line-num', 3);
+ r3.setAttribute('comment-side', 'right');
+
+ const r5 = document.createElement('div');
+ r5.setAttribute('line-num', 5);
+ r5.setAttribute('comment-side', 'right');
+
+ const threadEls = [l3, l5, r3, r5];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
+ [l3, r5]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l3]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r5]);
+ });
+
+ test('filterThreadElsForLocation for file comments', () => {
+ const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
+
+ const l = document.createElement('div');
+ l.setAttribute('comment-side', 'left');
+
+ const r = document.createElement('div');
+ r.setAttribute('comment-side', 'right');
+
+ const threadEls = [l, r];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
+ [l, r]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.BOTH), [l, r]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r]);
+ });
+
test('_createThreads', () => {
const comments = [
{
id: 'sallys_confession',
message: 'i like you, jack',
updated: '2015-12-23 15:00:20.396000000',
+ line: 1,
__commentSide: 'left',
}, {
id: 'jacks_reply',
message: 'i like you, too',
updated: '2015-12-24 15:01:20.396000000',
__commentSide: 'left',
+ line: 1,
in_reply_to: 'sallys_confession',
},
{
@@ -108,48 +170,31 @@
},
];
- const expectedThreadGroups = [
- {
- start_datetime: '2015-12-23 15:00:20.396000000',
- commentSide: 'left',
- comments: [{
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- in_reply_to: 'sallys_confession',
- }],
- patchNum: undefined,
- rootId: 'sallys_confession',
- },
- {
- start_datetime: '2015-12-20 15:01:20.396000000',
- commentSide: 'left',
- comments: [
- {
- id: 'new_draft',
- message: 'i do not like either of you',
- __commentSide: 'left',
- __draft: true,
- updated: '2015-12-20 15:01:20.396000000',
- },
- ],
- patchNum: undefined,
- rootId: 'new_draft',
- },
- ];
+ const actualThreads = builder._createThreads(comments);
- assert.deepEqual(
- builder._createThreads(comments),
- expectedThreadGroups);
+ assert.equal(actualThreads.length, 2);
+
+ assert.equal(
+ actualThreads[0].start_datetime, '2015-12-23 15:00:20.396000000');
+ assert.equal(actualThreads[0].commentSide, 'left');
+ assert.equal(actualThreads[0].comments.length, 2);
+ assert.deepEqual(actualThreads[0].comments[0], comments[0]);
+ assert.deepEqual(actualThreads[0].comments[1], comments[1]);
+ assert.equal(actualThreads[0].patchNum, undefined);
+ assert.equal(actualThreads[0].rootId, 'sallys_confession');
+ assert.equal(actualThreads[0].lineNum, 1);
+
+ assert.equal(
+ actualThreads[1].start_datetime, '2015-12-20 15:01:20.396000000');
+ assert.equal(actualThreads[1].commentSide, 'left');
+ assert.equal(actualThreads[1].comments.length, 1);
+ assert.deepEqual(actualThreads[1].comments[0], comments[2]);
+ assert.equal(actualThreads[1].patchNum, undefined);
+ assert.equal(actualThreads[1].rootId, 'new_draft');
+ assert.equal(actualThreads[1].lineNum, undefined);
});
- test('_createThreads inherits patchNum amd range', () => {
+ test('_createThreads inherits patchNum and range', () => {
const comments = [{
id: 'betsys_confession',
message: 'i like you, jack',
@@ -162,9 +207,10 @@
},
patch_set: 5,
__commentSide: 'left',
+ line: 1,
}];
- expectedThreadGroups = [
+ expectedThreads = [
{
start_datetime: '2015-12-24 15:00:10.396000000',
commentSide: 'left',
@@ -180,6 +226,7 @@
},
patch_set: 5,
__commentSide: 'left',
+ line: 1,
}],
patchNum: 5,
rootId: 'betsys_confession',
@@ -189,30 +236,61 @@
end_line: 1,
end_character: 2,
},
+ lineNum: 1,
+ isOnParent: false,
},
];
assert.deepEqual(
builder._createThreads(comments),
- expectedThreadGroups);
+ expectedThreads);
});
- test('multiple comments at same location but not threaded', () => {
- const comments = [
- {
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- },
- ];
- assert.equal(builder._createThreads(comments).length, 2);
- });
+ test('_createThreads does not thread unrelated comments at same location',
+ () => {
+ const comments = [
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ __commentSide: 'left',
+ },
+ ];
+ assert.equal(builder._createThreads(comments).length, 2);
+ });
+
+ test('_createThreads derives isOnParent using side from first comment',
+ () => {
+ const comments = [
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ // line: 1,
+ // __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ // __commentSide: 'left',
+ // line: 1,
+ in_reply_to: 'sallys_confession',
+ },
+ ];
+
+ assert.equal(builder._createThreads(comments)[0].isOnParent, false);
+
+ comments[0].side = 'REVISION';
+ assert.equal(builder._createThreads(comments)[0].isOnParent, false);
+
+ comments[0].side = 'PARENT';
+ assert.equal(builder._createThreads(comments)[0].isOnParent, true);
+ });
test('_createElement classStr applies all classes', () => {
const node = builder._createElement('div', 'test classes');
@@ -387,87 +465,13 @@
}
});
- test('_filterThreadsForLine with no threads', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const threads = [];
- assert.deepEqual(
- builder._filterThreadsForLine(threads, line), []);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), []);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), []);
- });
-
- test('_filterThreadsForLine for line comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const l3 = {
- comments: [{id: 'l3', line: 3}],
- range: {end_line: 3},
- commentSide: 'left',
- };
- const l5 = {
- comments: [{id: 'l5', line: 5}],
- range: {end_line: 5},
- commentSide: 'left',
- };
- const r3 = {
- comments: [{id: 'r3', line: 3}],
- range: {end_line: 3},
- commentSide: 'right',
- };
- const r5 = {
- comments: [{id: 'r5', line: 5}],
- range: {end_line: 5},
- commentSide: 'right',
- };
-
- const threads = [l3, l5, r3, r5];
- assert.deepEqual(builder._filterThreadsForLine(threads, line),
- [l3, r5]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), [l3]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), [r5]);
- });
-
- test('_filterThreadsForLine for file comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = GrDiffLine.FILE;
- line.afterNumber = GrDiffLine.FILE;
-
- const l = {
- comments: [{id: 'l', line: undefined}],
- commentSide: 'left',
- };
- const r = {
- comments: [{id: 'r', line: undefined}],
- commentSide: 'right',
- };
-
- const threads = [l, r];
- assert.deepEqual(builder._filterThreadsForLine(threads, line),
- [l, r]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.BOTH), [l, r]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), [l]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), [r]);
- });
-
test('comment thread group creation', () => {
const l3 = {id: 'l3', line: 3, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'left'};
+ __commentSide: 'left', side: 'PARENT'};
const l5 = {id: 'l5', line: 5, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'left'};
+ __commentSide: 'left', patch_set: '2', side: 'REVISION'};
const r5 = {id: 'r5', line: 5, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'right'};
+ __commentSide: 'right', patch_set: '3'};
builder = new GrDiffBuilder(
{content: []}, {
@@ -482,59 +486,51 @@
},
left: [l3, l5],
right: [r5],
- }, createThreadGroupFn, prefs);
+ }, parentIndex, changeNum, path, projectName, prefs);
- function threadForComment(c, patchNum) {
- return {
- commentSide: c.__commentSide,
- comments: [c],
- patchNum,
- rootId: c.id,
- start_datetime: c.updated,
- };
- }
-
- function checkThreadGroupProps(threadGroupEl, patchNum, isOnParent,
+ function checkThreadGroupProps(threadGroupEl, patchNum,
comments) {
- assert.equal(createThreadGroupFn.lastCall.args[0], patchNum);
- assert.equal(createThreadGroupFn.lastCall.args[1], isOnParent);
- assert.deepEqual(
- threadGroupEl.threads,
- comments.map(c => threadForComment(c, undefined)));
+ const threadEls = Polymer.dom(threadGroupEl).queryDistributedElements(
+ 'gr-diff-comment-thread');
+ assert.equal(threadEls.length, comments.length);
+ for (let i=0; i < comments.length; i++) {
+ const threadEl = threadEls[i];
+
+ const comment = comments[i];
+ assert.equal(threadEl.patchNum, comment.patch_set);
+ assert.equal(threadEl.commentSide, comment.__commentSide);
+ assert.equal(threadEl.comments.length, 1);
+ assert.equal(threadEl.comments[0], comment);
+ assert.equal(threadEl.rootId, comment.id);
+ }
}
let line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.beforeNumber = 5;
line.afterNumber = 5;
let threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.isTrue(createThreadGroupFn.calledOnce);
- checkThreadGroupProps(threadGroupEl, '3', false, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [l5, r5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- assert.isTrue(createThreadGroupFn.calledTwice);
- checkThreadGroupProps(threadGroupEl, '3', false, [r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [r5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- assert.isTrue(createThreadGroupFn.calledThrice);
- checkThreadGroupProps(threadGroupEl, '3', true, [l5]);
+ checkThreadGroupProps(threadGroupEl, '3', [l5]);
builder._comments.meta.patchRange.basePatchNum = '1';
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 4);
- checkThreadGroupProps(threadGroupEl, '3', false, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [l5, r5]);
threadEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- assert.equal(createThreadGroupFn.callCount, 5);
- checkThreadGroupProps(threadEl, '1', false, [l5]);
+ checkThreadGroupProps(threadEl, '1', [l5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- assert.equal(createThreadGroupFn.callCount, 6);
- checkThreadGroupProps(threadGroupEl, '3', false, [r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [r5]);
builder._comments.meta.patchRange.basePatchNum = 'PARENT';
@@ -542,15 +538,13 @@
line.beforeNumber = 5;
line.afterNumber = 5;
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 7);
- checkThreadGroupProps(threadGroupEl, '3', true, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [l5, r5]);
line = new GrDiffLine(GrDiffLine.Type.ADD);
line.beforeNumber = 3;
line.afterNumber = 5;
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 8);
- checkThreadGroupProps(threadGroupEl, '3', false, [l3, r5]);
+ checkThreadGroupProps(threadGroupEl, '3', [l3, r5]);
});
@@ -1112,7 +1106,8 @@
outputEl = element.queryEffectiveChildren('#diffTable');
sandbox.stub(element, '_getDiffBuilder', () => {
const builder = new GrDiffBuilder(
- {content}, {left: [], right: []}, null, prefs, outputEl);
+ {content}, {left: [], right: []}, parentIndex, changeNum, path,
+ projectName, prefs, outputEl);
sandbox.stub(builder, 'addColumns');
builder.buildSectionElement = function(group) {
const section = document.createElement('stub');
@@ -1230,10 +1225,11 @@
tab_size: 4,
};
- element.render({left: [], right: []}, prefs).then(() => {
- builder = element._builder;
- done();
- });
+ element.render(
+ undefined, prefs).then(() => {
+ builder = element._builder;
+ done();
+ });
});
test('getDiffLength', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html
index 58b7c32..9ecc2b1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html
@@ -31,20 +31,7 @@
margin-top: .2em;
}
</style>
- <template is="dom-repeat" items="[[threads]]" as="thread">
- <gr-diff-comment-thread
- comments="[[thread.comments]]"
- comment-side="[[thread.commentSide]]"
- is-on-parent="[[isOnParent]]"
- parent-index="[[parentIndex]]"
- change-num="[[changeNum]]"
- patch-num="[[thread.patchNum]]"
- root-id="{{thread.rootId}}"
- path="[[path]]"
- project-name="[[projectName]]"
- range="[[thread.range]]"
- on-thread-discard="_handleThreadDiscard"></gr-diff-comment-thread>
- </template>
+ <slot></slot>
</template>
<script src="gr-diff-comment-thread-group.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
index 23d0a58..9eb3fa2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
@@ -17,64 +17,78 @@
(function() {
'use strict';
+ window.Gerrit = window.Gerrit || {};
+
+ // This method will eventually move to gr-diff-host (so that gr-diff and it's
+ // descendants, including gr-diff-comment-thread-group, do not depend on a
+ // specific comment thread implementation, but can instead be used with other
+ // comment widgets). I cannot move it there yet, because it is still called
+ // from this file, and this file cannot depend on gr-diff-host. I decided to
+ // make it a function on the global Gerrit namespace, so that
+ // 1) I can move the call-side in the next change without moving this code,
+ // and thereby reduce the number of moving parts per commit.
+ // 2) To already now cut the ties to the this object - if it was an element
+ // method, I would probably want to use isOnParent etc. from `this`, and
+ // thus be required to change the code when I move it to gr-diff host.
+ // Making it a free function first requires me to catch any references to
+ // `this` and instead pass those in as parameter, which then allows me
+ // to move it later without any other changes, which makes the diff
+ // easier to read.
+ /**
+ * @param {Object} thread
+ * @param {number} parentIndex
+ * @param {number} changeNum
+ * @param {string} path
+ * @param {string} projectName
+ */
+ window.Gerrit.createThreadElement = function(
+ thread, parentIndex, changeNum, path, projectName) {
+ const threadEl = document.createElement('gr-diff-comment-thread');
+ threadEl.comments = thread.comments;
+ threadEl.commentSide = thread.commentSide;
+ threadEl.isOnParent = thread.isOnParent;
+ threadEl.parentIndex = parentIndex;
+ threadEl.changeNum = changeNum;
+ threadEl.patchNum = thread.patchNum;
+ threadEl.lineNum = thread.lineNum;
+ const rootIdChangedListener = changeEvent => {
+ thread.rootId = changeEvent.detail.value;
+ };
+ threadEl.addEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.path = path;
+ threadEl.projectName = projectName;
+ threadEl.range = thread.range;
+ const threadDiscardListener = e => {
+ const threadEl = /** @type {!Node} */ (e.currentTarget);
+ const parent = Polymer.dom(threadEl).parentNode;
+ threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.removeEventListener('thread-discard', threadDiscardListener);
+ Polymer.dom(parent).removeChild(threadEl);
+ };
+ threadEl.addEventListener('thread-discard', threadDiscardListener);
+ return threadEl;
+ };
+
Polymer({
is: 'gr-diff-comment-thread-group',
properties: {
- changeNum: String,
- projectName: String,
- patchForNewThreads: String,
- isOnParent: {
- type: Boolean,
- value: false,
- },
- parentIndex: {
- type: Number,
- value: null,
- },
- threads: {
- type: Array,
- value() { return []; },
- },
},
get threadEls() {
- return Polymer.dom(this.root).querySelectorAll('gr-diff-comment-thread');
+ return Polymer.dom(this).queryDistributedElements(
+ 'gr-diff-comment-thread');
},
/**
- * Adds a new thread. Range is optional because a comment can be
- * added to a line without a range selected.
- *
- * @param {!Object} opt_range
- */
- addNewThread(commentSide, opt_range) {
- this.push('threads', {
- comments: [],
- commentSide,
- patchNum: this.patchForNewThreads,
- range: opt_range,
- });
- },
-
- removeThread(rootId) {
- for (let i = 0; i < this.threads.length; i++) {
- if (this.threads[i].rootId === rootId) {
- this.splice('threads', i, 1);
- return;
- }
- }
- },
-
- /**
- * Fetch the thread group at the given range, or the range-less thread
- * on the line if no range is provided, lineNum, and side.
+ * Fetch the thread element at the given range, or the range-less thread
+ * element on the line if no range is provided, lineNum, and side.
*
* @param {string} side
* @param {!Object=} opt_range
* @return {!Object|undefined}
*/
- getThread(side, opt_range) {
+ getThreadEl(side, opt_range) {
const threads = [].filter.call(this.threadEls,
thread => this._rangesEqual(thread.range, opt_range))
.filter(thread => thread.commentSide === side);
@@ -83,10 +97,6 @@
}
},
- _handleThreadDiscard(e) {
- this.removeThread(e.detail.rootId);
- },
-
/**
* Compare two ranges. Either argument may be falsy, but will only return
* true if both are falsy or if neither are falsy and have the same position
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
index 81181b1..90f50a8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
@@ -51,14 +51,14 @@
sandbox.restore();
});
- test('getThread', () => {
+ test('getThreadEl', () => {
const range = {
start_line: 1,
start_character: 1,
end_line: 1,
end_character: 2,
};
- element.threads = [
+ const threads = [
{
rootId: 'sallys_confession',
commentSide: 'left',
@@ -111,40 +111,37 @@
],
},
];
+ for (const thread of threads) {
+ Polymer.dom(element).appendChild(Gerrit.createThreadElement(
+ thread, 1, '2', 'some/path', 'some_project'));
+ }
flushAsynchronousOperations();
- assert.deepEqual(element.getThread('right').rootId, 'right_side_comment');
- assert.deepEqual(element.getThread('right').comments.length, 1);
- assert.deepEqual(element.getThread('left').rootId, 'sallys_confession');
- assert.deepEqual(element.getThread('left').comments.length, 3);
- assert.deepEqual(element.getThread('left', range).rootId,
+ assert.deepEqual(
+ element.getThreadEl('right').rootId, 'right_side_comment');
+ assert.deepEqual(element.getThreadEl('right').comments.length, 1);
+ assert.deepEqual(element.getThreadEl('left').rootId, 'sallys_confession');
+ assert.deepEqual(element.getThreadEl('left').comments.length, 3);
+ assert.deepEqual(element.getThreadEl('left', range).rootId,
'betsys_confession');
- assert.deepEqual(element.getThread('left', range).comments.length, 1);
+ assert.deepEqual(element.getThreadEl('left', range).comments.length, 1);
});
- test('addNewThread', () => {
- const commentSide = 'left';
- const range = {startLine: 1, endLine: 2, startChar: 3, endChar: 4};
- element.patchForNewThreads = 5;
- element.addNewThread(commentSide, range);
- assert.equal(element.threads.length, 1);
- assert.equal(element.threads[0].comments.length, 0);
- assert.equal(element.threads[0].commentSide, commentSide);
- assert.equal(element.threads[0].patchNum, 5);
- assert.equal(element.threads[0].range.startLine, range.startLine);
- assert.equal(element.threads[0].range.endLine, range.endLine);
- assert.equal(element.threads[0].range.startChar, range.startChar);
- assert.equal(element.threads[0].range.endChar, range.endChar);
- });
-
- test('removeThread', () => {
- element.threads = [
- {rootId: 4711},
- {rootId: 42},
+ test('thread-discard handling', () => {
+ const threads = [
+ {comments: [{id: 4711}]},
+ {comments: [{id: 42}]},
];
- element.removeThread(4711);
- assert.equal(element.threads.length, 1);
- assert.equal(element.threads[0].rootId, 42);
+ for (const thread of threads) {
+ const threadEl = Gerrit.createThreadElement(
+ thread, 1, 2, 'some/path', 'Some Project');
+ Polymer.dom(element).appendChild(threadEl);
+ }
+
+ element.threadEls[0].dispatchEvent(
+ new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
+ assert.equal(element.threadEls.length, 1);
+ assert.equal(element.threadEls[0].rootId, 42);
});
test('_rangesEqual', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 056ab60..68b4616 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -456,8 +456,10 @@
/** @param {CustomEvent} e */
_handleCreateComment(e) {
- const {threadGroupEl, lineNum, side, range} = e.detail;
- const threadEl = this._getOrCreateThread(threadGroupEl, side, range);
+ const {threadGroupEl, lineNum, side, range, isOnParent,
+ patchForNewThreads} = e.detail;
+ const threadEl = this._getOrCreateThread(threadGroupEl, side, range,
+ isOnParent, patchForNewThreads);
threadEl.addOrEditDraft(lineNum, range);
this.$.reporting.recordDraftInteraction();
},
@@ -466,18 +468,28 @@
* Gets or creates a comment thread from a specific thread group.
* May include a range, if the comment is a range comment.
*
- * @param {!Object} threadGroupEl
+ * @param {!Node} threadGroupEl
* @param {string} commentSide
- * @param {!Object=} range
+ * @param {?Object} range
+ * @param {boolean} isOnParent
+ * @param {string} patchForNewThreads
* @return {!Object}
*/
- _getOrCreateThread(threadGroupEl, commentSide, range=undefined) {
- let threadEl = threadGroupEl.getThread(commentSide, range);
+ _getOrCreateThread(threadGroupEl, commentSide, range, isOnParent,
+ patchForNewThreads) {
+ let threadEl = threadGroupEl.getThreadEl(commentSide, range);
if (!threadEl) {
- threadGroupEl.addNewThread(commentSide, range);
- Polymer.dom.flush();
- threadEl = threadGroupEl.getThread(commentSide, range);
+ threadEl = Gerrit.createThreadElement(
+ {
+ comments: [],
+ commentSide,
+ patchNum: patchForNewThreads,
+ range,
+ isOnParent,
+ }, this._parentIndex, this.changeNum,
+ this.path, this.projectName);
+ Polymer.dom(threadGroupEl).appendChild(threadEl);
}
return threadEl;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 24afe87..7117574 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -777,7 +777,17 @@
const commentSide = 'left';
assert.isOk(element._getOrCreateThread(threadGroupEl,
- commentSide));
+ commentSide, undefined, false, '2'));
+
+ let threads = Polymer.dom(threadGroupEl)
+ .queryDistributedElements('gr-diff-comment-thread');
+
+ assert.equal(threads.length, 1);
+ assert.equal(threads[0].commentSide, commentSide);
+ assert.equal(threads[0].range, undefined);
+ assert.equal(threads[0].isOnParent, false);
+ assert.equal(threads[0].patchNum, 2);
+
// Try to fetch a thread with a different range.
range = {
@@ -788,10 +798,16 @@
};
assert.isOk(element._getOrCreateThread(
- threadGroupEl, commentSide, range));
- const threadCount = Polymer.dom(threadGroupEl.root).
- querySelectorAll('gr-diff-comment-thread').length;
- assert.equal(threadCount, 2);
+ threadGroupEl, commentSide, range, true, '3'));
+
+ threads = Polymer.dom(threadGroupEl)
+ .queryDistributedElements('gr-diff-comment-thread');
+
+ assert.equal(threads.length, 2);
+ assert.equal(threads[1].commentSide, commentSide);
+ assert.equal(threads[1].range, range);
+ assert.equal(threads[1].isOnParent, true);
+ assert.equal(threads[1].patchNum, 3);
});
suite('_translateChunksToIgnore', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index adb4dd6..a639d51 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -290,7 +290,8 @@
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
revision-image="[[revisionImage]]"
- create-comment-fn="[[_createThreadGroupFn]]"
+ parentIndex="[[parentIndex]]"
+ path="[[path]]"
line-of-interest="[[lineOfInterest]]">
<table
id="diffTable"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 6016a5a..7e6e9bc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -179,16 +179,6 @@
computed: '_computeNewlineWarning(diff)',
},
- /**
- * @type {function(number, boolean, !string)}
- */
- _createThreadGroupFn: {
- type: Function,
- value() {
- return this._createCommentThreadGroup.bind(this);
- },
- },
-
_diffLength: Number,
},
@@ -361,11 +351,11 @@
const contentEl = contentText.parentElement;
side = side ||
this._getCommentSideByLineAndContent(lineEl, contentEl);
- const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
+ const patchForNewThreads = this._getPatchNumByLineAndContent(
+ lineEl, contentEl);
const isOnParent =
this._getIsParentCommentByLineAndContent(lineEl, contentEl);
- const threadGroupEl = this._getOrCreateThreadGroup(contentEl, patchNum,
- side, isOnParent);
+ const threadGroupEl = this._getOrCreateThreadGroup(contentEl);
this.dispatchEvent(new CustomEvent('create-comment', {
bubbles: true,
detail: {
@@ -373,6 +363,8 @@
lineNum,
side,
range,
+ isOnParent,
+ patchForNewThreads,
},
}));
},
@@ -385,40 +377,18 @@
* Gets or creates a comment thread group for a specific line and side on a
* diff.
* @param {!Object} contentEl
- * @param {number} patchNum
- * @param {string} commentSide
- * @param {boolean} isOnParent
* @return {!Object}
*/
- _getOrCreateThreadGroup(contentEl, patchNum, commentSide, isOnParent) {
+ _getOrCreateThreadGroup(contentEl) {
// Check if thread group exists.
let threadGroupEl = this._getThreadGroupForLine(contentEl);
if (!threadGroupEl) {
- threadGroupEl = this._createCommentThreadGroup(patchNum, isOnParent,
- commentSide);
+ threadGroupEl = document.createElement('gr-diff-comment-thread-group');
contentEl.appendChild(threadGroupEl);
}
return threadGroupEl;
},
- /**
- * @param {number} patchNum
- * @param {boolean} isOnParent
- * @param {!string} commentSide
- * @return {!Object}
- */
- _createCommentThreadGroup(patchNum, isOnParent, commentSide) {
- const threadGroupEl =
- document.createElement('gr-diff-comment-thread-group');
- threadGroupEl.changeNum = this.changeNum;
- threadGroupEl.commentSide = commentSide;
- threadGroupEl.patchForNewThreads = patchNum;
- threadGroupEl.path = this.path;
- threadGroupEl.isOnParent = isOnParent;
- threadGroupEl.projectName = this.projectName;
- threadGroupEl.parentIndex = this._parentIndex;
- return threadGroupEl;
- },
/**
* The value to be used for the patch number of new comments created at the
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 07584c7..193e7eb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -295,9 +295,6 @@
test('thread groups', () => {
const contentEl = document.createElement('div');
- const commentSide = 'left';
- const patchNum = 1;
- const side = 'PARENT';
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
@@ -312,8 +309,7 @@
assert.isNotOk(element._getThreadGroupForLine(contentEl));
// A thread group gets created.
- const threadGroupEl = element._getOrCreateThreadGroup(contentEl,
- patchNum, commentSide, side);
+ const threadGroupEl = element._getOrCreateThreadGroup(contentEl);
assert.isOk(threadGroupEl);
// The new thread group can be fetched.
