Fix missing comments in diff-view If there is a comment with an invalid line-number, diff view doesn't display some comments. The change: 1) Fixes missing comments in diff-view 2) Shows warning for comments that were posted on invalid line Bug: Issue 12214 Change-Id: I7c9d62b76edcdaf368e9232484586dcc2ce0f666
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 ef22188..5d32e9b 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -815,10 +815,9 @@ // for each line from the start. let lastEl; for (const threadEl of addedThreadEls) { - const lineNumString = threadEl.getAttribute('line-num') || 'FILE'; const commentSide = threadEl.getAttribute('comment-side'); - const lineEl = this.$.diffBuilder.getLineElByNumber( - lineNumString, commentSide); + const lineEl = this._getLineElement(threadEl, + commentSide); const contentText = this.$.diffBuilder.getContentByLineEl(lineEl); const contentEl = contentText.parentElement; const threadGroupEl = this._getOrCreateThreadGroup( @@ -844,6 +843,18 @@ }); } + _getLineElement(threadEl, commentSide) { + const lineNumString = threadEl.getAttribute('line-num') || 'FILE'; + const lineEl = this.$.diffBuilder.getLineElByNumber( + lineNumString, commentSide); + if (lineEl) { + return lineEl; + } + // It is possible to add comment to non-existing line via API + threadEl.invalidLineNumber = true; + return this.$.diffBuilder.getLineElByNumber('FILE', commentSide); + } + _unobserveIncrementalNodes() { if (this._incrementalNodeObserver) { Polymer.dom(this).unobserveNodes(this._incrementalNodeObserver);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html index cce1d19..7bc93b5 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html +++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -16,6 +16,8 @@ --> <link rel="import" href="/bower_components/polymer/polymer.html"> +<link rel="import" href="/bower_components/iron-icon/iron-icon.html"> +<link rel="import" href="../../shared/gr-icons/gr-icons.html"> <link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html"> <link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html"> <link rel="import" href="../../../styles/shared-styles.html"> @@ -78,6 +80,18 @@ margin-left: var(--spacing-m); font-style: italic; } + .invalidLineNumber { + padding: var(--spacing-m); + } + .invalidLineNumberText { + color: var(--error-text-color); + } + .invalidLineNumberIcon { + color: var(--error-text-color); + vertical-align: top; + margin-right: var(--spacing-s); + } + </style> <template is="dom-if" if="[[showFilePath]]"> <div class="pathInfo"> @@ -86,6 +100,11 @@ </div> </template> <div id="container" class$="[[_computeHostClass(unresolved, isRobotComment)]]"> + <template is="dom-if" if="[[invalidLineNumber]]"> + <div class="invalidLineNumber"> + <span class="invalidLineNumberText"><iron-icon icon="gr-icons:error" class="invalidLineNumberIcon"></iron-icon>This comment thread is attached to non-existing line [[lineNum]].</span> + </div> + </template> <template id="commentList" is="dom-repeat" items="[[_orderedComments]]" as="comment"> <gr-comment
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js index d8a56f8..00ff03a 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js +++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -124,6 +124,11 @@ type: Boolean, value: false, }, + /** It is possible to add comment to non-existing line via API */ + invalidLineNumber: { + type: Number, + reflectToAttribute: true, + }, /** Necessary only if showFilePath is true or when used with gr-diff */ lineNum: { type: Number,
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html index 743923b..5ecae00 100644 --- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html +++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -54,6 +54,8 @@ <g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g> <!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full--> <g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g> + <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js --> + <g id="error"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z"></path></g> <!-- This is a custom PolyGerrit SVG --> <g id="side-by-side"><path d="M17.1578947,10.8888889 L2.84210526,10.8888889 C2.37894737,10.8888889 2,11.2888889 2,11.7777778 L2,17.1111111 C2,17.6 2.37894737,18 2.84210526,18 L17.1578947,18 C17.6210526,18 18,17.6 18,17.1111111 L18,11.7777778 C18,11.2888889 17.6210526,10.8888889 17.1578947,10.8888889 Z M17.1578947,2 L2.84210526,2 C2.37894737,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.37894737,9.11111111 2.84210526,9.11111111 L17.1578947,9.11111111 C17.6210526,9.11111111 18,8.71111111 18,8.22222222 L18,2.88888889 C18,2.4 17.6210526,2 17.1578947,2 Z M16.1973628,2 L2.78874238,2 C2.35493407,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.35493407,9.11111111 2.78874238,9.11111111 L16.1973628,9.11111111 C16.6311711,9.11111111 16.9861052,8.71111111 16.9861052,8.22222222 L16.9861052,2.88888889 C16.9861052,2.4 16.6311711,2 16.1973628,2 Z" id="Shape" transform="scale(1.2) translate(10.000000, 10.000000) rotate(-90.000000) translate(-10.000000, -10.000000)"/></g> <!-- This is a custom PolyGerrit SVG -->