Add saved drafts with their ids in the model
In some cases drafts created are also automatically saved(such as when
"Done" is pressed).
Currently we store these drafts without their ID in the model.
In this case further editing the comment results in the code thinking
the edited draft is a new draft and hence saves it as a reply instead
of as an edit to the already saved draft.
Change-Id: If162b13eca66fd36479d845eec09c3f4d937be84
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 b37613a..bc41f98 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
@@ -30,6 +30,7 @@
UIComment,
UIDraft,
UIRobot,
+ DraftInfo,
} from '../../../utils/comment-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {appContext} from '../../../services/app-context';
@@ -564,14 +565,23 @@
if (isEditing) {
reply.__editing = true;
- }
-
- this.commentsService.addDraft(reply);
-
- if (!isEditing) {
+ this.commentsService.addDraft(reply);
+ } else {
assertIsDefined(this.changeNum, 'changeNum');
assertIsDefined(this.patchNum, 'patchNum');
- this.restApiService.saveDiffDraft(this.changeNum, this.patchNum, reply);
+ this.restApiService
+ .saveDiffDraft(this.changeNum, this.patchNum, reply)
+ .then(result => {
+ if (!result.ok) {
+ fireAlert(document, 'Unable to restore draft');
+ return;
+ }
+ this.restApiService.getResponseObject(result).then(obj => {
+ const resComment = obj as unknown as DraftInfo;
+ resComment.patch_set = reply.patch_set;
+ this.commentsService.addDraft(resComment);
+ });
+ });
}
}
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 ecd9731..06d25b3 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
@@ -315,58 +315,64 @@
let element: GrCommentThread;
let addDraftServiceStub: SinonStub;
let saveDiffDraftStub: SinonStub;
+ let comment = {
+ id: '7afa4931_de3d65bd',
+ path: '/path/to/file.txt',
+ line: 5,
+ in_reply_to: 'baf0414d_60047215' as UrlEncodedCommentId,
+ updated: '2015-12-21 02:01:10.850000000',
+ message: 'Done',
+ };
+ const peanutButterComment = {
+ author: {
+ name: 'Mr. Peanutbutter',
+ email: 'tenn1sballchaser@aol.com' as EmailAddress as EmailAddress,
+ },
+ id: 'baf0414d_60047215' as UrlEncodedCommentId,
+ line: 5,
+ in_reply_to: 'baf0414d_60047215' as UrlEncodedCommentId,
+ message: 'is this a crossover episode!?',
+ updated: '2015-12-08 19:48:33.843000000' as Timestamp,
+ path: '/path/to/file.txt',
+ unresolved: true,
+ patch_set: 3 as PatchSetNum,
+ };
+ const mockResponse: Response = {
+ ...new Response(),
+ headers: {} as Headers,
+ redirected: false,
+ status: 200,
+ statusText: '',
+ type: '' as ResponseType,
+ url: '',
+ ok: true,
+ text() {
+ return Promise.resolve(")]}'\n" + JSON.stringify(comment));
+ },
+ };
+ let saveDiffDraftPromiseResolver: (value?: Response) => void;
setup(() => {
addDraftServiceStub = stubComments('addDraft');
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
saveDiffDraftStub = stubRestApi('saveDiffDraft').returns(
- Promise.resolve({
- headers: {} as Headers,
- redirected: false,
- status: 200,
- statusText: '',
- type: '' as ResponseType,
- url: '',
- ok: true,
- text() {
- return Promise.resolve(
- ")]}'\n" +
- JSON.stringify({
- id: '7afa4931_de3d65bd',
- path: '/path/to/file.txt',
- line: 5,
- in_reply_to: 'baf0414d_60047215' as UrlEncodedCommentId,
- updated: '2015-12-21 02:01:10.850000000',
- message: 'Done',
- })
- );
- },
- } as unknown as Response)
+ new Promise<Response>(
+ resolve =>
+ (saveDiffDraftPromiseResolver = resolve as (value?: Response) => void)
+ )
);
stubRestApi('deleteDiffDraft').returns(
- Promise.resolve({ok: true} as unknown as Response)
+ Promise.resolve({...new Response(), ok: true})
);
element = withCommentFixture.instantiate();
element.patchNum = 1 as PatchSetNum;
element.changeNum = 1 as NumericChangeId;
- element.comments = [
- {
- author: {
- name: 'Mr. Peanutbutter',
- email: 'tenn1sballchaser@aol.com' as EmailAddress as EmailAddress,
- },
- id: 'baf0414d_60047215' as UrlEncodedCommentId,
- line: 5,
- message: 'is this a crossover episode!?',
- updated: '2015-12-08 19:48:33.843000000' as Timestamp,
- path: '/path/to/file.txt',
- unresolved: true,
- patch_set: 3 as PatchSetNum,
- },
- ];
+ element.comments = [peanutButterComment];
flush();
});
test('reply', () => {
+ saveDiffDraftPromiseResolver(mockResponse);
+
const commentEl = element.shadowRoot?.querySelector('gr-comment');
const reportStub = stubReporting('recordDraftInteraction');
assert.ok(commentEl);
@@ -385,6 +391,8 @@
});
test('quote reply', () => {
+ saveDiffDraftPromiseResolver(mockResponse);
+
const commentEl = element.shadowRoot?.querySelector('gr-comment');
const reportStub = stubReporting('recordDraftInteraction');
assert.ok(commentEl);
@@ -394,6 +402,10 @@
flush();
const draft = addDraftServiceStub.firstCall.args[0];
+ // the quote reply is not autmatically saved so verify that id is not set
+ assert.isNotOk(draft.id);
+ // verify that the draft returned was not saved
+ assert.isNotOk(saveDiffDraftStub.called);
assert.equal(draft.message, '> is this a crossover episode!?\n\n');
assert.equal(
draft.in_reply_to,
@@ -403,6 +415,7 @@
});
test('quote reply multiline', () => {
+ saveDiffDraftPromiseResolver(mockResponse);
const reportStub = stubReporting('recordDraftInteraction');
element.comments = [
{
@@ -436,6 +449,15 @@
});
test('ack', async () => {
+ saveDiffDraftPromiseResolver(mockResponse);
+ comment = {
+ id: '7afa4931_de3d65bd',
+ path: '/path/to/file.txt',
+ line: 5,
+ in_reply_to: 'baf0414d_60047215' as UrlEncodedCommentId,
+ updated: '2015-12-21 02:01:10.850000000',
+ message: 'Ack',
+ };
const reportStub = stubReporting('recordDraftInteraction');
element.changeNum = 42 as NumericChangeId;
element.patchNum = 1 as PatchSetNum;
@@ -450,11 +472,20 @@
const draft = addDraftServiceStub.firstCall.args[0];
assert.equal(draft.message, 'Ack');
assert.equal(draft.in_reply_to, 'baf0414d_60047215' as UrlEncodedCommentId);
- assert.equal(draft.unresolved, false);
+ assert.isNotOk(draft.unresolved);
assert.isTrue(reportStub.calledOnce);
});
test('done', async () => {
+ saveDiffDraftPromiseResolver(mockResponse);
+ comment = {
+ id: '7afa4931_de3d65bd',
+ path: '/path/to/file.txt',
+ line: 5,
+ in_reply_to: 'baf0414d_60047215' as UrlEncodedCommentId,
+ updated: '2015-12-21 02:01:10.850000000',
+ message: 'Done',
+ };
const reportStub = stubReporting('recordDraftInteraction');
assert.isFalse(saveDiffDraftStub.called);
element.changeNum = 42 as NumericChangeId;
@@ -467,14 +498,18 @@
tap(doneBtn!);
await flush();
const draft = addDraftServiceStub.firstCall.args[0];
+ // Since the reply is automatically saved, verify that draft.id is set in
+ // the model
+ assert.equal(draft.id, '7afa4931_de3d65bd');
assert.equal(draft.message, 'Done');
assert.equal(draft.in_reply_to, 'baf0414d_60047215' as UrlEncodedCommentId);
- assert.isFalse(draft.unresolved);
+ assert.isNotOk(draft.unresolved);
assert.isTrue(reportStub.calledOnce);
assert.isTrue(saveDiffDraftStub.called);
});
test('save', async () => {
+ saveDiffDraftPromiseResolver(mockResponse);
element.changeNum = 42 as NumericChangeId;
element.patchNum = 1 as PatchSetNum;
element.path = '/path/to/file.txt';
@@ -488,13 +523,21 @@
});
test('please fix', async () => {
+ comment = peanutButterComment;
element.changeNum = 42 as NumericChangeId;
element.patchNum = 1 as PatchSetNum;
const commentEl = element.shadowRoot?.querySelector('gr-comment');
assert.ok(commentEl);
const promise = mockPromise();
- commentEl!.addEventListener('create-fix-comment', () => {
- const draft = addDraftServiceStub.firstCall.args[0];
+ commentEl!.addEventListener('create-fix-comment', async () => {
+ assert.isTrue(saveDiffDraftStub.called);
+ assert.isFalse(addDraftServiceStub.called);
+ saveDiffDraftPromiseResolver(mockResponse);
+ // flushing so the saveDiffDraftStub resolves and the draft is returned
+ await flush();
+ assert.isTrue(saveDiffDraftStub.called);
+ assert.isTrue(addDraftServiceStub.called);
+ const draft = saveDiffDraftStub.firstCall.args[2];
assert.equal(
draft.message,
'> is this a crossover episode!?\n\nPlease fix.'
@@ -506,6 +549,8 @@
assert.isTrue(draft.unresolved);
promise.resolve();
});
+ assert.isFalse(saveDiffDraftStub.called);
+ assert.isFalse(addDraftServiceStub.called);
commentEl!.dispatchEvent(
new CustomEvent('create-fix-comment', {
detail: {comment: commentEl!.comment},
@@ -839,6 +884,7 @@
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
stubRestApi('saveDiffDraft').returns(
Promise.resolve({
+ ...new Response(),
ok: true,
text() {
return Promise.resolve(
@@ -853,10 +899,10 @@
})
);
},
- } as unknown as Response)
+ })
);
stubRestApi('deleteDiffDraft').returns(
- Promise.resolve({ok: true} as unknown as Response)
+ Promise.resolve({...new Response(), ok: true})
);
element = withCommentFixture.instantiate();
element.patchNum = 1 as PatchSetNum;
