Fix new change being paired with old comments

This could manifest in old comments being shown in the reply dialog.
But it could also mean that the Change Log was computed based on the
new change with an outdated ChangeComments object.

Change-Id: I6bd9dc8d562d1c3d55d3a8fcf5e0a2ae12497c81
This commit is contained in:
Ben Rohlfs
2020-06-02 13:47:52 +02:00
parent 2815a2f41e
commit 3cf22a7d6d
4 changed files with 31 additions and 16 deletions

View File

@@ -228,7 +228,6 @@ class GrChangeView extends mixinBehaviors( [
type: Boolean,
computed: '_computeCanStartReview(_change)',
},
_comments: Object,
/** @type {?} */
_change: {
type: Object,
@@ -1037,10 +1036,7 @@ class GrChangeView extends mixinBehaviors( [
(value.patchNum !== undefined && value.basePatchNum !== undefined) &&
(this._patchRange.patchNum !== value.patchNum ||
this._patchRange.basePatchNum !== value.basePatchNum);
if (this._changeNum !== value.changeNum) {
this._initialLoadComplete = false;
}
const changeChanged = this._changeNum !== value.changeNum;
const patchRange = {
patchNum: value.patchNum,
@@ -1052,7 +1048,7 @@ class GrChangeView extends mixinBehaviors( [
// If the change has already been loaded and the parameter change is only
// in the patch range, then don't do a full reload.
if (this._initialLoadComplete && patchChanged) {
if (!changeChanged && patchChanged) {
if (patchRange.patchNum == null) {
patchRange.patchNum = this.computeLatestPatchNum(this._allPatchSets);
}
@@ -1062,6 +1058,7 @@ class GrChangeView extends mixinBehaviors( [
return;
}
this._initialLoadComplete = false;
this._changeNum = value.changeNum;
this.$.relatedChanges.clear();
@@ -1686,6 +1683,13 @@ class GrChangeView extends mixinBehaviors( [
* (comments, robot comments, draft comments) is requested.
*/
_reloadComments() {
// We are resetting all comment related properties, because we want to avoid
// a new change being loaded and then paired with outdated comments.
this._changeComments = undefined;
this._commentThreads = undefined;
this._diffDrafts = undefined;
this._draftCommentThreads = undefined;
this._robotCommentThreads = undefined;
return this.$.commentAPI.loadAll(this._changeNum)
.then(comments => this._recomputeComments(comments));
}

View File

@@ -512,6 +512,9 @@ class GrFileList extends mixinBehaviors( [
* @return {string}
*/
_computeCommentsString(changeComments, patchRange, path) {
if ([changeComments, patchRange, path].some(arg => arg === undefined)) {
return '';
}
const unresolvedCount =
changeComments.computeUnresolvedNum({
patchNum: patchRange.basePatchNum,
@@ -551,6 +554,9 @@ class GrFileList extends mixinBehaviors( [
* @return {string}
*/
_computeDraftsString(changeComments, patchRange, path) {
if ([changeComments, patchRange, path].some(arg => arg === undefined)) {
return '';
}
const draftCount =
changeComments.computeDraftCount({
patchNum: patchRange.basePatchNum,
@@ -572,6 +578,9 @@ class GrFileList extends mixinBehaviors( [
* @return {string}
*/
_computeDraftsStringMobile(changeComments, patchRange, path) {
if ([changeComments, patchRange, path].some(arg => arg === undefined)) {
return '';
}
const draftCount =
changeComments.computeDraftCount({
patchNum: patchRange.basePatchNum,
@@ -593,6 +602,9 @@ class GrFileList extends mixinBehaviors( [
* @return {string}
*/
_computeCommentsStringMobile(changeComments, patchRange, path) {
if ([changeComments, patchRange, path].some(arg => arg === undefined)) {
return '';
}
const commentCount =
changeComments.computeCommentCount({
patchNum: patchRange.basePatchNum,

View File

@@ -137,8 +137,8 @@ class GrMessage extends GestureEventListeners(
},
_commentCountText: {
type: Number,
computed: '_computeCommentCountText(comments, message.commentThreads,'
+ ' _isCleanerLogExperimentEnabled)',
computed: '_computeCommentCountText(comments,'
+ ' message.commentThreads.length, _isCleanerLogExperimentEnabled)',
},
_loggedIn: {
type: Boolean,
@@ -202,17 +202,16 @@ class GrMessage extends GestureEventListeners(
}
}
_computeCommentCountText(comments, threads, isCleanerLogExperimentEnabled) {
_computeCommentCountText(
comments, threadsLength, isCleanerLogExperimentEnabled) {
// TODO(taoalpha): clean up after cleaner-changelog experiment launched
if (isCleanerLogExperimentEnabled) {
if (!threads) return undefined;
const count = threads.length;
if (count === 0) {
if (threadsLength === 0) {
return undefined;
} else if (count === 1) {
} else if (threadsLength === 1) {
return '1 comment';
} else {
return `${count} comments`;
return `${threadsLength} comments`;
}
} else {
if (!comments) return undefined;

View File

@@ -643,11 +643,11 @@ class GrReplyDialog extends mixinBehaviors( [
}
_computeHideDraftList(draftCommentThreads) {
return draftCommentThreads.length === 0;
return !draftCommentThreads || draftCommentThreads.length === 0;
}
_computeDraftsTitle(draftCommentThreads) {
const total = draftCommentThreads.length;
const total = draftCommentThreads ? draftCommentThreads.length : 0;
if (total == 0) { return ''; }
if (total == 1) { return '1 Draft'; }
if (total > 1) { return total + ' Drafts'; }