Refactor draft comment state
Do not model `UNSAVED` as part of the draft state. Rename and repurpose
the draft state to `savingState` only.
Don't use the term "unsaved". Use "new" instead. Instead of using
`state` for checking, if a draft is "new", inspect the `client_id` and
`id` properties.
Release-Notes: skip
Change-Id: I9c3bbefb6b2910ef7d7b29f3fc630adcf95bba8d
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 747a982..8a4eb01 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -76,7 +76,7 @@
PatchSetNumber,
CommentThread,
ChangeStates,
- DraftState,
+ SavingState,
} from '../../../types/common';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
@@ -154,7 +154,7 @@
updated: '2018-02-13 22:48:48.018000000' as Timestamp,
message: 'draft',
unresolved: false,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
patch_set: 2 as RevisionPatchSetNum,
},
],
@@ -257,7 +257,7 @@
updated: '2018-02-15 22:48:48.018000000' as Timestamp,
message: 'resolved draft',
unresolved: false,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
patch_set: 2 as RevisionPatchSetNum,
},
],
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
index 0028380..1ed2729 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
@@ -34,7 +34,7 @@
ReviewInputTag,
Timestamp,
UrlEncodedCommentId,
- DraftState,
+ SavingState,
} from '../../../types/common';
import {ChangeMessageDeletedEventDetail} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -776,7 +776,7 @@
message: 'n',
unresolved: false,
path: '/PATCHSET_LEVEL',
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
},
],
patchNum: 1 as RevisionPatchSetNum,
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 cb87f12..c0383fa 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
@@ -71,7 +71,7 @@
queryAndAssert,
} from '../../../utils/common-util';
import {
- createUnsavedPatchsetLevel,
+ createNewPatchsetLevel,
getFirstComment,
isPatchsetLevel,
isUnresolved,
@@ -703,8 +703,8 @@
if (!this.patchsetLevelComment && this.latestPatchNum) {
// TODO: This should rather be done in the comments model. It should
// ensure that a patchset level draft is always present.
- this.getCommentsModel().addUnsavedDraft(
- createUnsavedPatchsetLevel(
+ this.getCommentsModel().addNewDraft(
+ createNewPatchsetLevel(
this.latestPatchNum,
this.patchsetLevelDraftMessage,
!this.patchsetLevelDraftIsResolved
@@ -949,7 +949,8 @@
}
private renderDraftsSection() {
- if (this.computeHideDraftList(this.draftCommentThreads)) return;
+ const threads = this.draftCommentThreads;
+ if (!threads || threads.length === 0) return;
return html`
<section class="draftsContainer">
<div class="includeComments">
@@ -960,17 +961,13 @@
?checked=${this.includeComments}
/>
<label for="includeComments"
- >Publish ${this.computeDraftsTitle(this.draftCommentThreads)}</label
+ >Publish ${this.computeDraftsTitle(threads)}</label
>
</div>
${when(
this.includeComments,
() => html`
- <gr-thread-list
- id="commentList"
- .threads=${this.draftCommentThreads}
- hide-dropdown
- >
+ <gr-thread-list id="commentList" .threads=${threads} hide-dropdown>
</gr-thread-list>
`
)}
@@ -1519,10 +1516,6 @@
});
}
- computeHideDraftList(draftCommentThreads?: CommentThread[]) {
- return !draftCommentThreads || draftCommentThreads.length === 0;
- }
-
computeDraftsTitle(draftCommentThreads?: CommentThread[]) {
const total = draftCommentThreads ? draftCommentThreads.length : 0;
return pluralize(total, 'Draft');
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 126343e..3e20d9a 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
@@ -57,7 +57,6 @@
import {GrAccountList} from '../../shared/gr-account-list/gr-account-list';
import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row';
import {GrLabelScores} from '../gr-label-scores/gr-label-scores';
-import {GrThreadList} from '../gr-thread-list/gr-thread-list';
import {fixture, html, waitUntil, assert} from '@open-wc/testing';
import {accountKey} from '../../../utils/account-util';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -479,9 +478,6 @@
remove_from_attention_set: [],
ignore_automatic_attention_set_rules: true,
});
- assert.isFalse(
- queryAndAssert<GrThreadList>(element, '#commentList').hidden
- );
});
test('modified attention set', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
index 4158f31..a4357bb 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
@@ -34,7 +34,7 @@
RevisionPatchSetNum,
CommentThread,
isDraft,
- DraftState,
+ SavingState,
} from '../../../types/common';
import {query, queryAndAssert} from '../../../utils/common-util';
import {GrAccountLabel} from '../../shared/gr-account-label/gr-account-label';
@@ -76,7 +76,7 @@
updated: '2015-12-01 15:16:15.000000000' as Timestamp,
message: 'draft',
unresolved: true,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
patch_set: '2' as RevisionPatchSetNum,
},
],
@@ -163,7 +163,7 @@
updated: '2015-12-05 15:16:15.000000000' as Timestamp,
message: 'resolved draft',
unresolved: false,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
patch_set: '2' as RevisionPatchSetNum,
},
],
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 511c9ab..2ccae8d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -21,7 +21,7 @@
isNumber,
} from '../../../utils/patch-set-util';
import {
- createUnsaved,
+ createNew,
isInBaseOfPatchRange,
isInRevisionOfPatchRange,
} from '../../../utils/comment-util';
@@ -1141,7 +1141,7 @@
const parentIndex = this.computeParentIndex();
const draft: DraftInfo = {
- ...createUnsaved('', true),
+ ...createNew('', true),
patch_set: patchNum as RevisionPatchSetNum,
side: commentSide,
parent: parentIndex ?? undefined,
@@ -1149,7 +1149,7 @@
line: typeof lineNum === 'number' ? lineNum : undefined,
range,
};
- this.getCommentsModel().addUnsavedDraft(draft);
+ this.getCommentsModel().addNewDraft(draft);
}
private canCommentOnPatchSetNum(patchNum: PatchSetNum) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index 5de86aa..43045f7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -1008,7 +1008,7 @@
setup(async () => {
const commentsModel: CommentsModel = testResolver(commentsModelToken);
- addDraftSpy = sinon.spy(commentsModel, 'addUnsavedDraft');
+ addDraftSpy = sinon.spy(commentsModel, 'addNewDraft');
account = createAccountDetailWithId(1);
element.disconnectedCallback();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 3ec0764..c76f04c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -21,7 +21,7 @@
computeDiffFromContext,
getLastComment,
getFirstComment,
- createUnsavedReply,
+ createNewReply,
NEWLINE_PATTERN,
id,
} from '../../../utils/comment-util';
@@ -38,9 +38,7 @@
CommentRange,
CommentThread,
isDraft,
- isDraftOrUnsaved,
isRobot,
- isUnsaved,
NumericChangeId,
RepoName,
UrlEncodedCommentId,
@@ -50,7 +48,11 @@
import {GrButton} from '../gr-button/gr-button';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffLayer, RenderPreferences} from '../../../api/diff';
-import {assertIsDefined, copyToClipbard} from '../../../utils/common-util';
+import {
+ assert,
+ assertIsDefined,
+ copyToClipbard,
+} from '../../../utils/common-util';
import {fire} from '../../../utils/event-util';
import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
@@ -468,26 +470,26 @@
renderComments() {
assertIsDefined(this.thread, 'thread');
const publishedComments = repeat(
- this.thread.comments.filter(c => !isDraftOrUnsaved(c)),
+ this.thread.comments.filter(c => !isDraft(c)),
comment => comment.id,
comment => this.renderComment(comment)
);
// We are deliberately not including the draft in the repeat directive,
// because we ran into spurious issues with <gr-comment> being destroyed
// and re-created when an unsaved draft transitions to 'saved' state.
- // TODO: Revisit this, because UNSAVED and DRAFT is not so different
+ // TODO: Revisit this, because this transition should not cause issues
// anymore. Just put the draft into the `repeat` directive above and
- // then use `id()` instead of`.id` above.
- const draftComment = this.renderComment(this.getDraftOrUnsaved());
+ // then use `id()` instead of `.id` above.
+ const draftComment = this.renderComment(this.getDraft());
return html`${publishedComments}${draftComment}`;
}
private renderComment(comment?: Comment) {
if (!comment) return nothing;
- const robotButtonDisabled = !this.account || this.isDraftOrUnsaved();
+ const robotButtonDisabled = !this.account || this.isDraft();
const isFirstComment = this.getFirstComment() === comment;
const initiallyCollapsed =
- !isDraftOrUnsaved(comment) &&
+ !isDraft(comment) &&
(this.messageId
? comment.change_message_id !== this.messageId
: !this.unresolved);
@@ -504,18 +506,17 @@
@comment-editing-changed=${(
e: CustomEvent<CommentEditingChangedDetail>
) => {
- if (isDraftOrUnsaved(comment)) this.editing = e.detail.editing;
+ if (isDraft(comment)) this.editing = e.detail.editing;
}}
@comment-unresolved-changed=${(e: ValueChangedEvent<boolean>) => {
- if (isDraftOrUnsaved(comment)) this.unresolved = e.detail.value;
+ if (isDraft(comment)) this.unresolved = e.detail.value;
}}
></gr-comment>
`;
}
renderActions() {
- if (!this.account || this.isDraftOrUnsaved() || this.isRobotComment())
- return;
+ if (!this.account || this.isDraft() || this.isRobotComment()) return;
return html`
<div id="actionsContainer">
<span id="unresolvedLabel">${
@@ -617,13 +618,13 @@
if (changed.has('thread')) {
assertIsDefined(this.thread, 'thread');
assertIsDefined(this.getFirstComment(), 'first comment');
- if (!this.isDraftOrUnsaved()) {
+ if (!this.isDraft()) {
// We can only do this for threads without draft, because otherwise we
// are relying on the <gr-comment> component for the draft to fire
// events about the *dirty* `unresolved` state.
this.unresolved = this.getLastComment()?.unresolved ?? true;
}
- this.hasDraft = this.isDraftOrUnsaved();
+ this.hasDraft = this.isDraft();
this.rootId = id(this.getFirstComment()!);
}
if (changed.has('editing')) {
@@ -651,19 +652,11 @@
return isDraft(this.getLastComment());
}
- private isDraftOrUnsaved(): boolean {
- return this.isDraft() || this.isUnsaved();
- }
-
- private getDraftOrUnsaved(): Comment | undefined {
- if (this.isDraftOrUnsaved()) return this.getLastComment();
+ private getDraft(): Comment | undefined {
+ if (this.isDraft()) return this.getLastComment();
return undefined;
}
- private isUnsaved(): boolean {
- return isUnsaved(this.getLastComment());
- }
-
private isPatchsetLevel() {
return this.thread?.path === SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
}
@@ -800,19 +793,14 @@
const replyingTo = this.getLastComment();
assertIsDefined(this.thread, 'thread');
assertIsDefined(replyingTo, 'the comment that the user wants to reply to');
- if (isDraft(replyingTo)) {
- throw new Error('cannot reply to draft');
- }
- if (isUnsaved(replyingTo)) {
- throw new Error('cannot reply to unsaved comment');
- }
- const unsaved = createUnsavedReply(replyingTo, content, unresolved);
+ assert(!isDraft(replyingTo), 'cannot reply to draft');
+ const newReply = createNewReply(replyingTo, content, unresolved);
if (userWantsToEdit) {
- this.getCommentsModel().addUnsavedDraft(unsaved);
+ this.getCommentsModel().addNewDraft(newReply);
} else {
try {
this.saving = true;
- await this.getCommentsModel().saveDraft(unsaved);
+ await this.getCommentsModel().saveDraft(newReply);
} finally {
this.saving = false;
}
@@ -848,7 +836,7 @@
const author = this.getFirstComment()?.author ?? this.account;
const user = getUserName(undefined, author);
const unresolvedStatus = this.unresolved ? 'Unresolved ' : '';
- const draftStatus = this.isDraftOrUnsaved() ? 'Draft ' : '';
+ const draftStatus = this.isDraft() ? 'Draft ' : '';
return `${unresolvedStatus}${draftStatus}Comment thread by ${user}`;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 8d1f24c..1027a62 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -14,7 +14,7 @@
CommentInfo,
RepoName,
DraftInfo,
- DraftState,
+ SavingState,
} from '../../../types/common';
import {
mockPromise,
@@ -26,7 +26,7 @@
import {
createAccountDetailWithId,
createThread,
- createUnsaved,
+ createNewDraft,
} from '../../../test/test-data-generators';
import {SinonStubbedMember} from 'sinon';
import {fixture, html, assert} from '@open-wc/testing';
@@ -39,14 +39,14 @@
} from '../../../models/comments/comments-model';
import {testResolver} from '../../../test/common-test-setup';
-const c1 = {
+const c1: CommentInfo = {
author: {name: 'Kermit'},
id: 'the-root' as UrlEncodedCommentId,
message: 'start the conversation',
updated: '2021-11-01 10:11:12.000000000' as Timestamp,
};
-const c2 = {
+const c2: CommentInfo = {
author: {name: 'Ms Piggy'},
id: 'the-reply' as UrlEncodedCommentId,
message: 'keep it going',
@@ -54,13 +54,13 @@
in_reply_to: 'the-root' as UrlEncodedCommentId,
};
-const c3 = {
+const c3: DraftInfo = {
author: {name: 'Kermit'},
id: 'the-draft' as UrlEncodedCommentId,
message: 'stop it',
updated: '2021-11-03 10:11:12.000000000' as Timestamp,
in_reply_to: 'the-reply' as UrlEncodedCommentId,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
};
const commentWithContext = {
@@ -128,7 +128,7 @@
});
test('renders unsaved', async () => {
- element.thread = createThread(createUnsaved());
+ element.thread = createThread(createNewDraft());
await element.updateComplete;
assert.shadowDom.equal(
element,
@@ -316,14 +316,14 @@
suite('action button clicks', () => {
let savePromise: MockPromise<DraftInfo>;
let stubSave: SinonStubbedMember<CommentsModel['saveDraft']>;
- let stubAdd: SinonStubbedMember<CommentsModel['addUnsavedDraft']>;
+ let stubAdd: SinonStubbedMember<CommentsModel['addNewDraft']>;
setup(async () => {
savePromise = mockPromise<DraftInfo>();
stubSave = sinon
.stub(testResolver(commentsModelToken), 'saveDraft')
.returns(savePromise);
- stubAdd = sinon.stub(testResolver(commentsModelToken), 'addUnsavedDraft');
+ stubAdd = sinon.stub(testResolver(commentsModelToken), 'addNewDraft');
element.thread = createThread(c1, {...c2, unresolved: true});
await element.updateComplete;
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 ade21a2..d790e8c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -26,13 +26,11 @@
RepoName,
RobotCommentInfo,
Comment,
- isDraftOrUnsaved,
isRobot,
- isUnsaved,
isSaving,
isError,
- isSaved,
isDraft,
+ isNew,
} from '../../../types/common';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import {
@@ -67,7 +65,6 @@
import {createDiffUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {modalStyles} from '../../../styles/gr-modal-styles';
-import {when} from 'lit/directives/when.js';
const FILE = 'FILE';
@@ -494,12 +491,11 @@
override render() {
if (!this.comment) return;
- if (isUnsaved(this.comment) && !this.editing) return;
- this.toggleAttribute('saving', this.isSaving());
- this.toggleAttribute('error', this.isError());
+ this.toggleAttribute('saving', isSaving(this.comment));
+ this.toggleAttribute('error', isError(this.comment));
const classes = {
container: true,
- draft: isDraftOrUnsaved(this.comment),
+ draft: isDraft(this.comment),
};
return html`
<gr-endpoint-decorator name="comment">
@@ -509,10 +505,7 @@
</gr-endpoint-param>
<gr-endpoint-param name="message" .value=${this.messageText}>
</gr-endpoint-param>
- <gr-endpoint-param
- name="isDraft"
- .value=${isDraftOrUnsaved(this.comment)}
- >
+ <gr-endpoint-param name="isDraft" .value=${isDraft(this.comment)}>
</gr-endpoint-param>
<div id="container" class=${classMap(classes)}>
${this.renderHeader()}
@@ -549,17 +542,13 @@
}
private renderAuthor() {
- if (isDraftOrUnsaved(this.comment)) return;
+ if (isDraft(this.comment)) return;
if (isRobot(this.comment)) {
const id = this.comment.robot_id;
return html`<span class="robotName">${id}</span>`;
}
- const classes = {draft: isDraftOrUnsaved(this.comment)};
return html`
- <gr-account-label
- .account=${this.comment?.author ?? this.account}
- class=${classMap(classes)}
- >
+ <gr-account-label .account=${this.comment?.author ?? this.account}>
</gr-account-label>
`;
}
@@ -577,13 +566,13 @@
}
private renderDraftLabel() {
- if (!isDraftOrUnsaved(this.comment)) return;
+ if (!isDraft(this.comment)) return;
let label = 'Draft';
let tooltip =
'This draft is only visible to you. ' +
"To publish drafts, click the 'Reply' or 'Start review' button " +
"at the top of the change or press the 'a' key.";
- if (this.isError()) {
+ if (isError(this.comment)) {
label += ' (Failed to save)';
tooltip = 'Unable to save draft. Please try to save again.';
}
@@ -626,12 +615,7 @@
* a draft. It is an action applied to published comments.
*/
private renderDeleteButton() {
- if (
- !this.isAdmin ||
- isDraftOrUnsaved(this.comment) ||
- isRobot(this.comment)
- )
- return;
+ if (!this.isAdmin || isDraft(this.comment) || isRobot(this.comment)) return;
if (this.collapsed) return;
return html`
<gr-button
@@ -661,29 +645,32 @@
// This should match the condition of `renderPatchset()`.
if (!this.showPatchset) return;
// This should match the condition of `renderDate()`.
- if (!this.comment?.updated || this.collapsed) return;
+ if (this.collapsed) return;
// Render separator, if both are present: patchset AND date.
return html`<span class="separator"></span>`;
}
private renderDate() {
- if (this.isUnsaved() || this.collapsed) return;
+ if (this.collapsed) return;
return html`
<span class="date" tabindex="0" @click=${this.handleAnchorClick}>
- ${when(
- this.isSaved(),
- () => html`
- <gr-date-formatter
- withTooltip
- .dateStr=${this.comment!.updated}
- ></gr-date-formatter>
- `,
- () => (this.comment as DraftInfo).state
- )}
+ ${this.renderDateInner()}
</span>
`;
}
+ private renderDateInner() {
+ if (isError(this.comment)) return 'Error';
+ if (isSaving(this.comment)) return 'Saving';
+ if (isNew(this.comment)) return 'New';
+ return html`
+ <gr-date-formatter
+ withTooltip
+ .dateStr=${this.comment!.updated}
+ ></gr-date-formatter>
+ `;
+ }
+
private renderToggle() {
const icon = this.collapsed ? 'expand_more' : 'expand_less';
const ariaLabel = this.collapsed ? 'Expand' : 'Collapse';
@@ -746,7 +733,7 @@
private renderCopyLinkIcon() {
// Only show the icon when the thread contains a published comment.
- if (!this.comment?.in_reply_to && isDraftOrUnsaved(this.comment)) return;
+ if (!this.comment?.in_reply_to && isDraft(this.comment)) return;
return html`
<gr-icon
icon="link"
@@ -761,7 +748,7 @@
private renderHumanActions() {
if (!this.account || isRobot(this.comment)) return;
- if (this.collapsed || !isDraftOrUnsaved(this.comment)) return;
+ if (this.collapsed || !isDraft(this.comment)) return;
return html`
<div class="actions">
<div class="action resolve">
@@ -781,7 +768,7 @@
}
private renderDraftActions() {
- if (!isDraftOrUnsaved(this.comment)) return;
+ if (!isDraft(this.comment)) return;
return html`
<div class="rightActions">
${this.renderDiscardButton()} ${this.renderEditButton()}
@@ -822,7 +809,7 @@
if (this.editing || this.permanentEditingMode) return;
return html`<gr-button
link
- ?disabled=${this.isSaving() && !this.autoSaving}
+ ?disabled=${isSaving(this.comment) && !this.autoSaving}
class="action discard"
@click=${this.discard}
>Discard</gr-button
@@ -841,7 +828,7 @@
return html`
<gr-button
link
- ?disabled=${this.isSaving() && !this.autoSaving}
+ ?disabled=${isSaving(this.comment) && !this.autoSaving}
class="action cancel"
@click=${this.cancel}
>Cancel</gr-button
@@ -850,7 +837,7 @@
}
private renderSaveButton() {
- if (!this.editing && !this.isError()) return;
+ if (!this.editing) return;
return html`
<gr-button
link
@@ -933,12 +920,20 @@
firstWillUpdate() {
if (this.firstWillUpdateDone) return;
- this.firstWillUpdateDone = true;
- if (this.permanentEditingMode) this.editing = true;
assertIsDefined(this.comment, 'comment');
+ this.firstWillUpdateDone = true;
this.unresolved = this.comment.unresolved ?? true;
- if (isUnsaved(this.comment)) this.editing = true;
- if (isDraftOrUnsaved(this.comment)) {
+ if (this.permanentEditingMode) {
+ this.edit();
+ }
+ if (
+ isDraft(this.comment) &&
+ isNew(this.comment) &&
+ !isSaving(this.comment)
+ ) {
+ this.edit();
+ }
+ if (isDraft(this.comment)) {
this.collapsed = false;
} else {
this.collapsed = !!this.initiallyCollapsed;
@@ -955,6 +950,11 @@
override willUpdate(changed: PropertyValues) {
this.firstWillUpdate();
+ if (changed.has('comment')) {
+ if (isDraft(this.comment) && isError(this.comment)) {
+ this.edit();
+ }
+ }
if (changed.has('editing')) {
this.onEditingChanged();
}
@@ -984,9 +984,7 @@
/** Enter editing mode. */
private edit() {
- if (!isDraftOrUnsaved(this.comment)) {
- throw new Error('Cannot edit published comment.');
- }
+ assert(isDraft(this.comment), 'only drafts are editable');
if (this.editing) return;
this.editing = true;
}
@@ -1047,8 +1045,10 @@
this.collapsed = false;
this.messageText = this.comment?.message ?? '';
this.unresolved = this.comment?.unresolved ?? true;
- this.originalMessage = this.messageText;
- this.originalUnresolved = this.unresolved;
+ if (!isError(this.comment) && !isSaving(this.comment)) {
+ this.originalMessage = this.messageText;
+ this.originalUnresolved = this.unresolved;
+ }
}
// Parent components such as the reply dialog might be interested in whether
@@ -1062,7 +1062,7 @@
// private, but visible for testing
isSaveDisabled() {
assertIsDefined(this.comment, 'comment');
- if (this.isSaving() && !this.autoSaving) return true;
+ if (isSaving(this.comment) && !this.autoSaving) return true;
return !this.messageText?.trimEnd();
}
@@ -1149,18 +1149,16 @@
// private, but visible for testing
cancel() {
assertIsDefined(this.comment, 'comment');
- if (!isDraftOrUnsaved(this.comment)) {
- throw new Error('only unsaved and draft comments are editable');
- }
+ assert(isDraft(this.comment), 'only drafts are editable');
this.messageText = this.originalMessage;
this.unresolved = this.originalUnresolved;
this.save();
}
async autoSave() {
- if (this.isSaving() || this.autoSaving) return;
+ if (isSaving(this.comment) || this.autoSaving) return;
if (!this.editing || !this.comment) return;
- if (!isDraftOrUnsaved(this.comment)) return;
+ assert(isDraft(this.comment), 'only drafts are editable');
const messageToSave = this.messageText.trimEnd();
if (messageToSave === '') return;
if (messageToSave === this.comment.message) return;
@@ -1179,11 +1177,11 @@
}
async save() {
- assert(isDraftOrUnsaved(this.comment), 'can only save drafts');
+ assert(isDraft(this.comment), 'only drafts are editable');
// There is a minimal chance of `isSaving()` being false between iterations
// of the below while loop. But this will be extremely rare and just lead
// to a harmless assertion error. So let's not bother.
- if (this.isSaving() && !this.autoSaving) return;
+ if (isSaving(this.comment) && !this.autoSaving) return;
if (!this.permanentEditingMode) {
this.editing = false;
@@ -1200,6 +1198,7 @@
// No need to make a backend call when nothing has changed.
while (this.somethingToSave()) {
this.comment = await this.rawSave({showToast: true});
+ if (isError(this.comment)) return;
}
}
}
@@ -1207,6 +1206,7 @@
private somethingToSave() {
if (!this.comment) return false;
return (
+ isError(this.comment) ||
this.messageText.trimEnd() !== this.comment?.message ||
this.unresolved !== this.comment.unresolved
);
@@ -1214,8 +1214,8 @@
/** For sharing between save() and autoSave(). */
private rawSave(options: {showToast: boolean}) {
- assert(isDraftOrUnsaved(this.comment), 'can only save drafts');
- assert(!this.isSaving(), 'saving already in progress');
+ assert(isDraft(this.comment), 'only drafts are editable');
+ assert(!isSaving(this.comment), 'saving already in progress');
return this.getCommentsModel().saveDraft(
{
...this.comment,
@@ -1268,23 +1268,6 @@
);
this.closeDeleteCommentModal();
}
-
- isUnsaved() {
- return isUnsaved(this.comment);
- }
-
- isSaved() {
- return !isDraft(this.comment) || isSaved(this.comment);
- }
-
- /** Note that this can also mean that auto-saving is in-progress. */
- isSaving() {
- return isSaving(this.comment);
- }
-
- isError() {
- return isError(this.comment);
- }
}
declare global {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 8299516..59485e2 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -22,7 +22,7 @@
import {
AccountId,
DraftInfo,
- DraftState,
+ SavingState,
EmailAddress,
NumericChangeId,
PatchSetNum,
@@ -33,7 +33,7 @@
createComment,
createDraft,
createRobotComment,
- createUnsaved,
+ createNewDraft,
} from '../../../test/test-data-generators';
import {ReplyToCommentEvent} from '../../../types/events';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
@@ -273,7 +273,7 @@
test('renders draft', async () => {
element.initiallyCollapsed = false;
- (element.comment as DraftInfo).state = DraftState.SAVED;
+ (element.comment as DraftInfo).savingState = SavingState.OK;
await element.updateComplete;
assert.shadowDom.equal(
element,
@@ -353,7 +353,7 @@
test('renders draft in editing mode', async () => {
element.initiallyCollapsed = false;
- (element.comment as DraftInfo).state = DraftState.SAVED;
+ (element.comment as DraftInfo).savingState = SavingState.OK;
element.editing = true;
await element.updateComplete;
assert.shadowDom.equal(
@@ -467,7 +467,7 @@
},
line: 5,
path: 'test',
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
message: 'hello world',
};
element.editing = true;
@@ -492,7 +492,7 @@
},
line: 5,
path: 'test',
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
message: 'hello world',
};
element.editing = true;
@@ -542,7 +542,7 @@
element.changeNum = 42 as NumericChangeId;
element.comment = {
...createComment(),
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
path: '/path/to/file',
line: 5,
};
@@ -565,7 +565,7 @@
await element.updateComplete;
assert.isTrue(element.isSaveDisabled());
- element.comment = {...element.comment, state: DraftState.SAVING};
+ element.comment = {...element.comment, savingState: SavingState.SAVING};
await element.updateComplete;
assert.isTrue(element.isSaveDisabled());
});
@@ -625,22 +625,28 @@
test('save failed', async () => {
sinon.stub(commentsModel, 'saveDraft').returns(
Promise.resolve({
- ...createUnsaved(),
+ ...createNewDraft(),
message: 'something, not important',
- state: DraftState.ERROR,
+ unresolved: true,
+ savingState: SavingState.ERROR,
})
);
- element.comment = createUnsaved();
+ element.comment = createNewDraft({
+ message: '',
+ unresolved: true,
+ });
+ element.unresolved = true;
element.editing = true;
await element.updateComplete;
element.messageText = 'something, not important';
await element.updateComplete;
element.save();
+ assert.isFalse(element.editing);
await waitUntil(() => element.hasAttribute('error'));
- assert.isFalse(element.editing);
+ assert.isTrue(element.editing);
});
test('discard', async () => {
@@ -670,7 +676,7 @@
const saveStub = sinon.stub(commentsModel, 'saveDraft');
element.comment = {
...createComment(),
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
unresolved: false,
};
await element.updateComplete;
@@ -751,7 +757,7 @@
savePromise = mockPromise<DraftInfo>();
saveStub = sinon.stub(commentsModel, 'saveDraft').returns(savePromise);
- element.comment = createUnsaved();
+ element.comment = createNewDraft();
element.editing = true;
await element.updateComplete;
});
@@ -797,14 +803,14 @@
autoSavePromise.resolve({
...element.comment,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
message: 'auto save text',
id: 'exp123' as UrlEncodedCommentId,
updated: '2018-02-13 22:48:48.018000000' as Timestamp,
});
savePromise.resolve({
...element.comment,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
message: 'actual save text',
id: 'exp123' as UrlEncodedCommentId,
updated: '2018-02-13 22:48:49.018000000' as Timestamp,
@@ -829,7 +835,7 @@
},
line: 5,
path: 'test',
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
message: 'hello world',
};
element = await fixture(
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 31d4e2b..026e5e5 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -23,7 +23,7 @@
} from '../../types/common';
import {OpenFixPreviewEventDetail} from '../../types/events';
import {isDefined} from '../../types/types';
-import {PROVIDED_FIX_ID, createUnsaved} from '../../utils/comment-util';
+import {PROVIDED_FIX_ID, createNew} from '../../utils/comment-util';
import {assert, assertIsDefined, assertNever} from '../../utils/common-util';
import {fire} from '../../utils/event-util';
import {CheckResult, CheckRun, RunResult} from './checks-model';
@@ -101,7 +101,7 @@
const pointer = result.codePointers?.[0];
assertIsDefined(pointer, 'codePointer');
return {
- ...createUnsaved(pleaseFixMessage(result), true),
+ ...createNew(pleaseFixMessage(result), true),
path: pointer.path,
patch_set: result.patchset as RevisionPatchSetNum,
side: CommentSide.REVISION,
diff --git a/polygerrit-ui/app/models/comments/comments-model.ts b/polygerrit-ui/app/models/comments/comments-model.ts
index e74647b..2c18a09 100644
--- a/polygerrit-ui/app/models/comments/comments-model.ts
+++ b/polygerrit-ui/app/models/comments/comments-model.ts
@@ -15,14 +15,15 @@
AccountInfo,
DraftInfo,
Comment,
- isDraftOrUnsaved,
- isUnsaved,
- DraftState,
+ SavingState,
isSaving,
isError,
+ isDraft,
+ isNew,
} from '../../types/common';
import {
addPath,
+ createNew,
id,
isDraftThread,
reportingDetails,
@@ -36,7 +37,7 @@
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {ChangeModel} from '../change/change-model';
import {Interaction, Timing} from '../../constants/reporting';
-import {assert, assertIsDefined, uuid} from '../../utils/common-util';
+import {assert, assertIsDefined} from '../../utils/common-util';
import {debounce, DelayedTask} from '../../utils/async-util';
import {pluralize} from '../../utils/string-util';
import {ReportingService} from '../../services/gr-reporting/gr-reporting';
@@ -215,8 +216,8 @@
/** Adds or updates a draft. */
export function setDraft(state: CommentState, draft: DraftInfo): CommentState {
const nextState = {...state};
- if (!draft.path) throw new Error('draft path undefined');
- if (!isDraftOrUnsaved(draft)) throw new Error('draft is not a draft');
+ assert(!!draft.path, 'draft without path');
+ assert(isDraft(draft), 'draft is not a draft');
nextState.drafts = {...nextState.drafts};
const drafts = nextState.drafts;
@@ -236,8 +237,8 @@
draft: DraftInfo
): CommentState {
const nextState = {...state};
- if (!draft.path) throw new Error('draft path undefined');
- if (!isDraftOrUnsaved(draft)) throw new Error('draft is not a draft');
+ assert(!!draft.path, 'draft without path');
+ assert(isDraft(draft), 'draft is not a draft');
nextState.drafts = {...nextState.drafts};
const drafts = nextState.drafts;
@@ -576,25 +577,22 @@
async restoreDraft(draftId: UrlEncodedCommentId) {
const found = this.discardedDrafts?.find(d => id(d) === draftId);
if (!found) throw new Error('discarded draft not found');
- const newDraft = {
+ const newDraft: DraftInfo = {
...found,
- id: undefined,
- updated: undefined,
- state: DraftState.UNSAVED,
- client_id: uuid() as UrlEncodedCommentId,
+ ...createNew(),
};
await this.saveDraft(newDraft);
this.modifyState(s => deleteDiscardedDraft(s, draftId));
}
/**
- * Adds a new unsaved draft without saving it.
+ * Adds a new draft without saving it.
*
- * There is no equivalent `removeUnsavedDraft()` method, because
+ * There is no equivalent `removeNewDraft()` method, because
* `discardDraft()` can be used.
*/
- addUnsavedDraft(draft: DraftInfo) {
- assert(isUnsaved(draft), 'draft must be unsaved');
+ addNewDraft(draft: DraftInfo) {
+ assert(isNew(draft), 'draft must be new');
this.modifyState(s => setDraft(s, draft));
}
@@ -609,10 +607,10 @@
assertIsDefined(this.changeNum, 'change number');
assertIsDefined(draft.patch_set, 'patchset number of comment draft');
assert(!!draft.message?.trim(), 'cannot save empty draft');
- assert(draft.state !== DraftState.SAVING, 'saving already in progress');
+ assert(!isSaving(draft), 'saving already in progress');
// optimistic update
- const draftSaving: DraftInfo = {...draft, state: DraftState.SAVING};
+ const draftSaving: DraftInfo = {...draft, savingState: SavingState.SAVING};
this.modifyState(s => setDraft(s, draftSaving));
// Saving the change number as to make sure that the response is still
@@ -620,7 +618,7 @@
const changeNum = this.changeNum;
this.report(Interaction.SAVE_COMMENT, draft);
if (showToast) this.showStartRequest();
- const timing = isUnsaved(draft) ? Timing.DRAFT_CREATE : Timing.DRAFT_UPDATE;
+ const timing = isNew(draft) ? Timing.DRAFT_CREATE : Timing.DRAFT_UPDATE;
const timer = this.reporting.getTimer(timing);
let savedComment;
@@ -637,16 +635,16 @@
)) as unknown as CommentInfo;
} catch (error) {
if (showToast) this.handleFailedDraftRequest();
- const draftError: DraftInfo = {...draft, state: DraftState.ERROR};
+ const draftError: DraftInfo = {...draft, savingState: SavingState.ERROR};
this.modifyState(s => setDraft(s, draftError));
return draftError;
}
- const draftSaved = {
+ const draftSaved: DraftInfo = {
...draft,
id: savedComment.id,
updated: savedComment.updated,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
};
timer.end({id: draftSaved.id});
if (showToast) this.showEndRequest();
@@ -659,14 +657,13 @@
const draft = this.lookupDraft(draftId);
assertIsDefined(draft, `draft not found by id ${draftId}`);
assertIsDefined(draft.patch_set, 'patchset number of comment draft');
- assert(draft.state !== DraftState.SAVING, 'saving already in progress');
+ assert(!isSaving(draft), 'saving already in progress');
// optimistic update
this.modifyState(s => deleteDraft(s, draft));
// For "unsaved" drafts there is nothing to discard on the server side.
- if (draft.state !== DraftState.UNSAVED) {
- assertIsDefined(draft.id, 'missing id');
+ if (draft.id) {
if (!draft.message?.trim()) throw new Error('empty draft');
// Saving the change number as to make sure that the response is still
// relevant when it comes back. The user maybe have navigated away.
@@ -709,9 +706,7 @@
reason: string
) {
assertIsDefined(comment.patch_set, 'comment.patch_set');
- if (isDraftOrUnsaved(comment)) {
- throw new Error('Admin deletion is only for published comments.');
- }
+ assert(!isDraft(comment), 'Admin deletion is only for published comments.');
const newComment = await this.restApiService.deleteComment(
changeNum,
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index a0a1ebd..a92e932 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -71,7 +71,7 @@
CommentThread,
DraftInfo,
ChangeMessage,
- DraftState,
+ SavingState,
} from '../types/common';
import {
AccountsVisibility,
@@ -92,7 +92,7 @@
import {GetDiffCommentsOutput} from '../services/gr-rest-api/gr-rest-api';
import {CommitInfoWithRequiredCommit} from '../elements/change/gr-change-metadata/gr-change-metadata';
import {WebLinkInfo} from '../types/diff';
-import {createCommentThreads} from '../utils/comment-util';
+import {createCommentThreads, createNew} from '../utils/comment-util';
import {GerritView} from '../services/router/router-model';
import {ChangeComments} from '../elements/diff/gr-comment-api/gr-comment-api';
import {EditRevisionInfo, ParsedChangeInfo} from '../types/types';
@@ -113,7 +113,6 @@
import {RepoDetailView, RepoViewState} from '../models/views/repo';
import {AdminChildView, AdminViewState} from '../models/views/admin';
import {DashboardViewState} from '../models/views/dashboard';
-import {uuid} from '../utils/common-util';
const TEST_DEFAULT_EXPRESSION = 'label:Verified=MAX -label:Verified=MIN';
export const TEST_PROJECT_NAME: RepoName = 'test-project' as RepoName;
@@ -843,19 +842,16 @@
export function createDraft(extra: Partial<CommentInfo> = {}): DraftInfo {
return {
...createComment(),
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
...extra,
};
}
-export function createUnsaved(extra: Partial<CommentInfo> = {}): DraftInfo {
+export function createNewDraft(extra: Partial<CommentInfo> = {}): DraftInfo {
return {
...createComment(),
- state: DraftState.UNSAVED,
- client_id: uuid() as UrlEncodedCommentId,
- id: undefined,
- updated: undefined,
...extra,
+ ...createNew(),
};
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 4688b1f..b148780 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -695,27 +695,19 @@
WIP = 'WIP',
}
-export enum DraftState {
+export enum SavingState {
/**
- * New draft created in this session and not yet saved.
- * Must have a `client_id` set.
- */
- // Possible prior states: -
- // Possible subsequent states: SAVING
- UNSAVED = 'UNSAVED',
- /**
- * Currently saving to the backend.
- */
- // Possible prior states: UNSAVED, SAVED, ERROR
- // Possible subsequent states: SAVED, ERROR
- SAVING = 'SAVING',
- /**
- * Comment saved to the backend.
- * Must have `id` and `updated` set.
+ * Currently not saving. Not yet saved or last saving attempt successful.
*/
// Possible prior states: SAVING
// Possible subsequent states: SAVING
- SAVED = 'SAVED',
+ OK = 'OK',
+ /**
+ * Currently saving to the backend.
+ */
+ // Possible prior states: OK, ERROR
+ // Possible subsequent states: OK, ERROR
+ SAVING = 'SAVING',
/**
* Latest saving attempt failed with an error.
*/
@@ -727,19 +719,25 @@
export type DraftInfo = Omit<CommentInfo, 'id' | 'updated'> & {
// Must be set for all drafts.
// Drafts received from the backend will be modified immediately with
- // `state: SAVED` before allowing them to get into the model.
- state: DraftState;
- // Must be set for UNSAVED drafts. Can be set for other drafts.
+ // `state: OK` before allowing them to get into the model.
+ savingState: SavingState;
+ // Must be set for new drafts created in this session.
// Use the id() utility function for uniquely identifying drafts.
client_id?: UrlEncodedCommentId;
- // Must be set for SAVED drafts. Must not be set for UNSAVED drafts.
+ // Must be set for drafts known to the backend.
// Use the id() utility function for uniquely identifying drafts.
id?: UrlEncodedCommentId;
- // Must be set for SAVED drafts. Can be set for other drafts. Will then just
- // contain the last value of then the draft was previously SAVED.
+ // Set, iff `id` is set. Reflects the time when the draft was last saved to
+ // the backend.
updated?: Timestamp;
};
+export interface NewDraftInfo extends DraftInfo {
+ client_id: UrlEncodedCommentId;
+ id: undefined;
+ updated: undefined;
+}
+
/**
* This is what human, robot and draft comments can agree upon.
*
@@ -760,42 +758,30 @@
export function isDraft<T extends Comment>(
x: T | DraftInfo | undefined
): x is DraftInfo {
- return (
- !!x &&
- (x as DraftInfo).state !== undefined &&
- (x as DraftInfo).state !== DraftState.UNSAVED
- );
-}
-
-export function isUnsaved<T extends Comment>(
- x: T | DraftInfo | undefined
-): x is DraftInfo {
- return !!x && (x as DraftInfo).state === DraftState.UNSAVED;
-}
-
-export function isSaved<T extends Comment>(
- x: T | DraftInfo | undefined
-): x is DraftInfo {
- return !!x && (x as DraftInfo).state === DraftState.SAVED;
+ return !!x && (x as DraftInfo).savingState !== undefined;
}
export function isSaving<T extends Comment>(
x: T | DraftInfo | undefined
-): x is DraftInfo {
- return !!x && (x as DraftInfo).state === DraftState.SAVING;
+): boolean {
+ return !!x && (x as DraftInfo).savingState === SavingState.SAVING;
}
export function isError<T extends Comment>(
x: T | DraftInfo | undefined
-): x is DraftInfo {
- return !!x && (x as DraftInfo).state === DraftState.ERROR;
+): boolean {
+ return !!x && (x as DraftInfo).savingState === SavingState.ERROR;
}
-// TODO: Find a better name for this. Maybe this can become just `isDraft()`.
-export function isDraftOrUnsaved<T extends Comment>(
+/**
+ * A new draft comment is a comment that was created by the user in this session
+ * and has not yet been saved to the backend. Such a comment must have a
+ * `client_id`, but it must not have an `id`.
+ */
+export function isNew<T extends Comment>(
x: T | DraftInfo | undefined
-): x is DraftInfo {
- return isDraft(x) || isUnsaved(x);
+): boolean {
+ return !!x && !!(x as DraftInfo).client_id && !(x as DraftInfo).id;
}
export interface CommentThread {
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index cee7e2c..802d15d 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -23,11 +23,11 @@
ChangeMessage,
isRobot,
isDraft,
- isDraftOrUnsaved,
Comment,
- isUnsaved,
CommentIdToCommentThreadMap,
- DraftState,
+ SavingState,
+ NewDraftInfo,
+ isNew,
} from '../types/common';
import {CommentSide, SpecialFilePath} from '../constants/constants';
import {parseDate} from './date-util';
@@ -63,12 +63,14 @@
* e.g. in `repeat()` directives.
*/
export function id(comment: Comment): UrlEncodedCommentId {
- if (isUnsaved(comment)) {
- assertIsDefined(comment.client_id);
- return comment.client_id;
- }
- if (isDraft(comment) && comment.client_id) {
- return comment.client_id;
+ if (isDraft(comment)) {
+ if (isNew(comment)) {
+ assertIsDefined(comment.client_id);
+ return comment.client_id;
+ }
+ if (comment.client_id) {
+ return comment.client_id;
+ }
}
assertIsDefined(comment.id);
return comment.id;
@@ -76,9 +78,9 @@
export function sortComments<T extends Comment>(comments: T[]): T[] {
return comments.slice(0).sort((c1, c2) => {
- const u1 = isUnsaved(c1);
- const u2 = isUnsaved(c2);
- if (u1 !== u2) return u1 ? 1 : -1;
+ const n1 = isNew(c1);
+ const n2 = isNew(c2);
+ if (n1 !== n2) return n1 ? 1 : -1;
const d1 = isDraft(c1);
const d2 = isDraft(c2);
@@ -97,34 +99,40 @@
});
}
-export function createUnsaved(message?: string, unresolved?: boolean) {
- return {
- message,
- unresolved,
- state: DraftState.UNSAVED,
+export function createNew(
+ message?: string,
+ unresolved?: boolean
+): NewDraftInfo {
+ const newDraft: NewDraftInfo = {
+ savingState: SavingState.OK,
client_id: uuid() as UrlEncodedCommentId,
+ id: undefined,
+ updated: undefined,
};
+ if (message !== undefined) newDraft.message = message;
+ if (unresolved !== undefined) newDraft.unresolved = unresolved;
+ return newDraft;
}
-export function createUnsavedPatchsetLevel(
+export function createNewPatchsetLevel(
patchNum?: PatchSetNumber,
message?: string,
unresolved?: boolean
): DraftInfo {
return {
- ...createUnsaved(message, unresolved),
+ ...createNew(message, unresolved),
patch_set: patchNum,
path: SpecialFilePath.PATCHSET_LEVEL_COMMENTS,
};
}
-export function createUnsavedReply(
+export function createNewReply(
replyingTo: CommentInfo,
message: string,
unresolved: boolean
): DraftInfo {
return {
- ...createUnsaved(message, unresolved),
+ ...createNew(message, unresolved),
path: replyingTo.path,
patch_set: replyingTo.patch_set,
side: replyingTo.side,
@@ -198,7 +206,7 @@
export function getLastPublishedComment(
thread: CommentThread
): CommentInfo | DraftInfo | undefined {
- const publishedComments = thread.comments.filter(c => !isDraftOrUnsaved(c));
+ const publishedComments = thread.comments.filter(c => !isDraft(c));
const len = publishedComments.length;
return publishedComments[len - 1];
}
@@ -428,7 +436,7 @@
const updated: {[path: string]: DraftInfo[]} = {};
for (const filePath of Object.keys(draftsByPath)) {
updated[filePath] = (draftsByPath[filePath] ?? []).map(draft => {
- return {...draft, state: DraftState.SAVED};
+ return {...draft, savingState: SavingState.OK};
});
}
return updated;
@@ -442,7 +450,7 @@
unresolved: comment?.unresolved,
path_length: comment?.path?.length,
line: comment?.range?.start_line ?? comment?.line,
- unsaved: isUnsaved(comment),
+ unsaved: isNew(comment),
};
}
diff --git a/polygerrit-ui/app/utils/comment-util_test.ts b/polygerrit-ui/app/utils/comment-util_test.ts
index 49d387e..ab21c67 100644
--- a/polygerrit-ui/app/utils/comment-util_test.ts
+++ b/polygerrit-ui/app/utils/comment-util_test.ts
@@ -26,7 +26,7 @@
import {
Comment,
DraftInfo,
- DraftState,
+ SavingState,
PARENT,
RevisionPatchSetNum,
Timestamp,
@@ -134,7 +134,7 @@
{
id: 'new_draft' as UrlEncodedCommentId,
message: 'i do not like either of you',
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
updated: '2015-12-20 15:01:20.396000000' as Timestamp,
} as DraftInfo,
{
@@ -159,7 +159,7 @@
suite('createCommentThreads', () => {
test('creates threads from individual comments', () => {
- const comments = [
+ const comments: Comment[] = [
{
id: 'sallys_confession' as UrlEncodedCommentId,
message: 'i like you, jack',
@@ -180,7 +180,7 @@
{
id: 'new_draft' as UrlEncodedCommentId,
message: 'i do not like either of you' as UrlEncodedCommentId,
- state: DraftState.SAVED,
+ savingState: SavingState.OK,
updated: '2015-12-20 15:01:20.396000000' as Timestamp,
patch_set: 1 as RevisionPatchSetNum,
path: 'some/path',
