Move comment thread filtering to top-level
This makes it easier for me to reuse it elsewhere, as I will need later
on.
Change-Id: I0ec8c888aed759bab5deace6d03fa0fe8b73f04f
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 6b58a97..9788b65 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
@@ -346,49 +400,6 @@
};
/**
- * @param {!Array<!HTMLElement>} threadEls
- * @param {!GrDiffLine} lineInfo
- * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which
- * to return the threads (default: BOTH).
- */
- GrDiffBuilder.prototype._filterThreadEls = function(
- threadEls, lineInfo, side = GrDiffBuilder.Side.BOTH) {
- function matchesLeftLine(threadEl) {
- return threadEl.getAttribute('comment-side') ==
- GrDiffBuilder.Side.LEFT &&
- threadEl.getAttribute('line-num') == lineInfo.beforeNumber;
- }
- function matchesRightLine(threadEl) {
- return threadEl.getAttribute('comment-side') ==
- GrDiffBuilder.Side.RIGHT &&
- threadEl.getAttribute('line-num') == lineInfo.afterNumber;
- }
- function matchesFileComment(threadEl) {
- return (side === GrDiffBuilder.Side.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 !== GrDiffBuilder.Side.RIGHT) {
- matchers.push(matchesLeftLine);
- }
- if (side !== GrDiffBuilder.Side.LEFT) {
- matchers.push(matchesRightLine);
- }
- if (lineInfo.afterNumber === GrDiffLine.FILE ||
- lineInfo.beforeNumber === GrDiffLine.FILE) {
- matchers.push(matchesFileComment);
- }
- return threadEls.filter(threadEl =>
- matchers.some(matcher => matcher(threadEl)));
- };
-
- /**
* @param {Array<Object>} comments
*/
GrDiffBuilder.prototype._createThreads = function(comments) {
@@ -477,7 +488,7 @@
GrDiffBuilder.prototype._commentThreadGroupForLine = function(
line, side = GrDiffBuilder.Side.BOTH) {
const threadElsForGroup =
- this._filterThreadEls(this._threadEls, line, side);
+ Gerrit.filterThreadElsForLocation(this._threadEls, line, side);
if (!threadElsForGroup || threadElsForGroup.length === 0) {
return null;
}
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 7cc21c3..1e1e99b 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
@@ -86,6 +86,65 @@
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 = [
{
@@ -376,72 +435,6 @@
}
});
- test('_filterThreadEls with no threads', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const threads = [];
- assert.deepEqual(
- builder._filterThreadEls(threads, line), []);
- assert.deepEqual(builder._filterThreadEls(threads, line,
- GrDiffBuilder.Side.LEFT), []);
- assert.deepEqual(builder._filterThreadEls(threads, line,
- GrDiffBuilder.Side.RIGHT), []);
- });
-
- test('_filterThreadEls for line comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.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(builder._filterThreadEls(threadEls, line),
- [l3, r5]);
- assert.deepEqual(builder._filterThreadEls(threadEls, line,
- GrDiffBuilder.Side.LEFT), [l3]);
- assert.deepEqual(builder._filterThreadEls(threadEls, line,
- GrDiffBuilder.Side.RIGHT), [r5]);
- });
-
- test('_filterThreadEls for file comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = GrDiffLine.FILE;
- line.afterNumber = GrDiffLine.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(builder._filterThreadEls(threadEls, line),
- [l, r]);
- assert.deepEqual(builder._filterThreadEls(threadEls, line,
- GrDiffBuilder.Side.BOTH), [l, r]);
- assert.deepEqual(builder._filterThreadEls(threadEls, line,
- GrDiffBuilder.Side.LEFT), [l]);
- assert.deepEqual(builder._filterThreadEls(threadEls, 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'};
