Ensure that quoted messages show up in the reply comment
GrReplyDialog had both a `patchsetLevelComment` and
`patchsetLevelDraftMessage` field that represent the same information.
The former is used for what is shown in `gr-comment` but was not always
updated when `patchsetLevelDraftMessage` changed.
Get rid of `patchsetLevelDraftMessage` as this is duplicative.
Additionally, ensure that when gr-comment's comment changes, the
messageText is updated to reflect that.
Google-Bug-Id: b/249567511
Release-Notes: skip
Change-Id: I0245a93d814e9ee12f7a4104cf2b4a817b4eee4a
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index b540c89..22f1325 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -257,9 +257,6 @@
@state() serverConfig?: ServerInfo;
@state()
- patchsetLevelDraftMessage = '';
-
- @state()
filterReviewerSuggestion: (input: Suggestion) => boolean;
@state()
@@ -382,7 +379,7 @@
patchsetLevelDraftIsResolved = true;
@state()
- patchsetLevelComment?: UnsavedInfo | DraftInfo;
+ patchsetLevelComment: UnsavedInfo | DraftInfo = this.createDraft('');
private readonly restApiService: RestApiService =
getAppContext().restApiService;
@@ -673,7 +670,9 @@
subscribe(
this,
() => this.getCommentsModel().patchsetLevelDrafts$,
- x => (this.patchsetLevelComment = x[0])
+ x => {
+ if (x.length > 0) this.patchsetLevelComment = x[0];
+ }
);
subscribe(
this,
@@ -764,7 +763,7 @@
changedProperties.has('mentionedUsersInUnresolvedDrafts') ||
changedProperties.has('includeComments') ||
changedProperties.has('labelsChanged') ||
- changedProperties.has('patchsetLevelDraftMessage') ||
+ changedProperties.has('patchsetLevelComment') ||
changedProperties.has('mentionedCCs')
) {
this.computeNewAttention();
@@ -916,10 +915,11 @@
}
// TODO: move to comment-util
- private createDraft(): UnsavedInfo {
+ // Private but used in tests.
+ createDraft(message: string): UnsavedInfo {
return {
patch_set: this.latestPatchNum,
- message: this.patchsetLevelDraftMessage,
+ message,
unresolved: !this.patchsetLevelDraftIsResolved,
path: SpecialFilePath.PATCHSET_LEVEL_COMMENTS,
__unsaved: true,
@@ -927,8 +927,6 @@
}
private renderPatchsetLevelComment() {
- if (!this.patchsetLevelComment)
- this.patchsetLevelComment = this.createDraft();
return html`
<gr-comment
id="patchsetLevelComment"
@@ -938,7 +936,10 @@
this.patchsetLevelDraftIsResolved = !e.detail.value;
}}
@comment-text-changed=${(e: ValueChangedEvent<string>) => {
- this.patchsetLevelDraftMessage = e.detail.value;
+ const newMessage = e.detail.value;
+ if (this.patchsetLevelComment.message === newMessage) return;
+ this.patchsetLevelComment.message = newMessage;
+ this.requestUpdate('patchsetLevelComment');
}}
.messagePlaceholder=${this.messagePlaceholder}
hide-header
@@ -1268,7 +1269,7 @@
this.focusOn(focusTarget);
if (quote?.length) {
// If a reply quote has been provided, use it.
- this.patchsetLevelDraftMessage = quote;
+ this.patchsetLevelComment = this.createDraft(quote);
}
if (this.restApiService.hasPendingDiffDrafts()) {
this.savingComments = true;
@@ -1281,7 +1282,7 @@
hasDrafts() {
return (
- this.patchsetLevelDraftMessage.length > 0 ||
+ !!this.patchsetLevelComment.message?.length ||
this.draftCommentThreads.length > 0
);
}
@@ -1469,8 +1470,8 @@
return;
}
- this.patchsetLevelDraftMessage = '';
this.includeComments = true;
+ this.patchsetLevelComment = this.createDraft('');
this.dispatchEvent(
new CustomEvent('send', {
composed: true,
@@ -2031,7 +2032,6 @@
computeSendButtonDisabled() {
if (
this.canBeStarted === undefined ||
- this.patchsetLevelDraftMessage === undefined ||
this.reviewersMutated === undefined ||
this.labelsChanged === undefined ||
this.includeComments === undefined ||
@@ -2055,13 +2055,8 @@
const revotingOrNewVote = this.labelsChanged || existingVote;
const hasDrafts =
(this.includeComments && this.draftCommentThreads.length > 0) ||
- this.patchsetLevelDraftMessage.length > 0;
- return (
- !hasDrafts &&
- !this.patchsetLevelDraftMessage.length &&
- !this.reviewersMutated &&
- !revotingOrNewVote
- );
+ !!this.patchsetLevelComment?.message?.length;
+ return !hasDrafts && !this.reviewersMutated && !revotingOrNewVote;
}
computePatchSetWarning() {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index acd1755..cef787a 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -327,7 +327,9 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
element.draftCommentThreads = [createCommentThread([createComment()])];
element.includeComments = true;
@@ -1054,7 +1056,9 @@
});
test('label picker', async () => {
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
element.draftCommentThreads = [createCommentThread([createComment()])];
const saveReviewPromise = interceptSaveReview();
@@ -1075,7 +1079,7 @@
const review = await saveReviewPromise;
await element.updateComplete;
await waitUntil(() => element.disabled === false);
- assert.equal(element.patchsetLevelDraftMessage.length, 0);
+ assert.equal(element.patchsetLevelComment.message?.length, 0);
assert.deepEqual(review, {
drafts: 'PUBLISH_ALL_REVISIONS',
labels: {
@@ -1101,7 +1105,9 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
const saveReviewPromise = interceptSaveReview();
@@ -1616,7 +1622,7 @@
assert.isFalse(element.attentionExpanded);
- element.patchsetLevelDraftMessage = 'a test comment';
+ element.patchsetLevelComment = element.createDraft('a test comment');
await element.updateComplete;
modifyButton.click();
@@ -2066,11 +2072,11 @@
const expectedError = new Error('test');
setup(() => {
- element.patchsetLevelDraftMessage = expectedDraft;
+ element.patchsetLevelComment = element.createDraft(expectedDraft);
});
function assertDialogOpenAndEnabled() {
- assert.strictEqual(expectedDraft, element.patchsetLevelDraftMessage);
+ assert.strictEqual(expectedDraft, element.patchsetLevelComment.message);
assert.isFalse(element.disabled);
}
@@ -2116,7 +2122,7 @@
// Mock canBeStarted
element.canBeStarted = true;
element.draftCommentThreads = [];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2130,7 +2136,7 @@
// Mock everything false
element.canBeStarted = false;
element.draftCommentThreads = [];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2144,7 +2150,7 @@
// Mock nonempty comment draft array; with sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = true;
@@ -2158,7 +2164,7 @@
// Mock nonempty comment draft array; without sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2173,7 +2179,7 @@
// Mock nonempty change message.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = 'test';
+ element.patchsetLevelComment = element.createDraft('test');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2188,7 +2194,7 @@
// Mock reviewers mutated.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = true;
element.labelsChanged = false;
element.includeComments = false;
@@ -2203,7 +2209,7 @@
// Mock labels changed.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2218,7 +2224,7 @@
// Whole dialog is disabled.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2236,7 +2242,7 @@
).all = [account];
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2305,13 +2311,13 @@
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'hello';
- await waitUntil(() => element.patchsetLevelDraftMessage === 'hello');
+ await waitUntil(() => element.patchsetLevelComment.message === 'hello');
assert.isFalse(element.computeSendButtonDisabled());
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'';
- await waitUntil(() => element.patchsetLevelDraftMessage === '');
+ await waitUntil(() => element.patchsetLevelComment.message === '');
assert.isTrue(element.computeSendButtonDisabled());
});
@@ -2327,7 +2333,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelDraftMessage === 'hello world'
+ () => element.patchsetLevelComment.message === 'hello world'
);
const saveReviewPromise = interceptSaveReview();
@@ -2367,7 +2373,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelDraftMessage === 'hello world'
+ () => element.patchsetLevelComment.message === 'hello world'
);
assert.deepEqual(autoSaveStub.callCount, 0);
@@ -2390,7 +2396,7 @@
await waitUntil(() => element.draftCommentThreads.length === 1);
// patchset level draft as a reply is not loaded in patchsetLevel comment
- assert.equal(element.patchsetLevelDraftMessage, '');
+ assert.equal(element.patchsetLevelComment.message, '');
assert.deepEqual(element.draftCommentThreads[0].comments[0], draft);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index ad2daf5..387554a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -968,8 +968,16 @@
override willUpdate(changed: PropertyValues) {
this.firstWillUpdate();
+ if (changed.has('editing') || changed.has('comment')) {
+ this.reflectCommentToInternalFields();
+ }
if (changed.has('editing')) {
- this.onEditingChanged();
+ // Parent components such as the reply dialog might be interested in whether
+ // come of their child components are in editing mode.
+ fire(this, 'comment-editing-changed', {
+ editing: this.editing,
+ path: this.comment?.path ?? '',
+ });
}
if (changed.has('unresolved')) {
// The <gr-comment-thread> component wants to change its color based on
@@ -1047,22 +1055,14 @@
throw new Error('unable to create preview fix event');
}
- private onEditingChanged() {
- if (this.editing) {
- this.collapsed = false;
- this.messageText = this.comment?.message ?? '';
- this.unresolved = this.comment?.unresolved ?? true;
- this.originalMessage = this.messageText;
- this.originalUnresolved = this.unresolved;
- setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
- }
-
- // Parent components such as the reply dialog might be interested in whether
- // come of their child components are in editing mode.
- fire(this, 'comment-editing-changed', {
- editing: this.editing,
- path: this.comment?.path ?? '',
- });
+ private reflectCommentToInternalFields() {
+ if (!this.editing) return;
+ this.collapsed = false;
+ this.messageText = this.comment?.message ?? '';
+ this.unresolved = this.comment?.unresolved ?? true;
+ this.originalMessage = this.messageText;
+ this.originalUnresolved = this.unresolved;
+ setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
}
// private, but visible for testing
