Merge "Make the js plugin hook and rest API typed"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 8aa3173..410bf42 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1760,7 +1760,7 @@
[
{
"identity": "username:john",
- "email": "john.doe@example.com",
+ "email_address": "john.doe@example.com",
"trusted": true
}
]
@@ -2250,7 +2250,7 @@
|============================
|Field Name ||Description
|`identity` ||The account external id.
-|`email` |optional|The email address for the external id.
+|`email_address` |optional|The email address for the external id.
|`trusted` |not set if `false`|
Whether the external id is trusted.
|`can_delete` |not set if `false`|
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 36fc456..4a81f38 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -118,11 +118,7 @@
"elements/diff/gr-diff-view/gr-diff-view_html.ts",
"elements/diff/gr-diff/gr-diff_html.ts",
"elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts",
- "elements/documentation/gr-documentation-search/gr-documentation-search_html.ts",
- "elements/edit/gr-edit-controls/gr-edit-controls_html.ts",
- "elements/edit/gr-edit-file-controls/gr-edit-file-controls_html.ts",
"elements/gr-app-element_html.ts",
- "elements/settings/gr-cla-view/gr-cla-view_html.ts",
"elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts",
"elements/settings/gr-email-editor/gr-email-editor_html.ts",
"elements/settings/gr-identities/gr-identities_html.ts",
diff --git a/polygerrit-ui/app/api/annotation.ts b/polygerrit-ui/app/api/annotation.ts
index 3bda936..5922e5e 100644
--- a/polygerrit-ui/app/api/annotation.ts
+++ b/polygerrit-ui/app/api/annotation.ts
@@ -28,7 +28,7 @@
basePatchNum?: number,
patchNum?: number,
change?: ChangeInfo
-) => Promise<Array<CoverageRange>>;
+) => Promise<Array<CoverageRange> | undefined>;
export declare interface AnnotationPluginApi {
/**
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index b78d687..39a80e6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -104,7 +104,6 @@
ConfigInfo,
EditInfo,
EditPatchSetNum,
- ElementPropertyDeepChange,
LabelNameToInfoMap,
NumericChangeId,
ParentPatchSetNum,
@@ -131,7 +130,6 @@
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {
CommentThread,
- DraftInfo,
isDraftThread,
isRobot,
isUnresolved,
@@ -164,7 +162,6 @@
ShowAlertEventDetail,
SwitchTabEvent,
TabState,
- ThreadListModifiedEvent,
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrMessagesList} from '../gr-messages-list/gr-messages-list';
@@ -185,6 +182,10 @@
import {Interaction, Timing} from '../../../constants/reporting';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
+import {
+ changeComments$,
+ drafts$,
+} from '../../../services/comments/comments-model';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -419,7 +420,7 @@
@property({
type: String,
- computed: '_computeReplyButtonLabel(_diffDrafts.*, _canStartReview)',
+ computed: '_computeReplyButtonLabel(_diffDrafts, _canStartReview)',
})
_replyButtonLabel = 'Reply';
@@ -549,6 +550,8 @@
restApiService = appContext.restApiService;
+ private readonly commentsService = appContext.commentsService;
+
private replyDialogResizeObserver?: ResizeObserver;
keyboardShortcuts() {
@@ -590,6 +593,14 @@
routerView$.pipe(takeUntil(this.disconnected$)).subscribe(view => {
this.isViewCurrent = view === GerritView.CHANGE;
});
+ drafts$.pipe(takeUntil(this.disconnected$)).subscribe(drafts => {
+ this._diffDrafts = {...drafts};
+ });
+ changeComments$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(changeComments => {
+ this._changeComments = changeComments;
+ });
}
constructor() {
@@ -609,15 +620,6 @@
this._handleShowBackgroundContent()
);
- this.addEventListener('diff-comments-modified', () =>
- this._handleReloadCommentThreads()
- );
-
- this.addEventListener(
- 'thread-list-modified',
- (e: ThreadListModifiedEvent) => this._handleReloadDiffComments(e)
- );
-
this.addEventListener('open-reply-dialog', () => this._openReplyDialog());
}
@@ -665,11 +667,6 @@
})
.then(() => this._initActiveTabs(this.params));
- this.addEventListener('comment-save', e => this._handleCommentSave(e));
- this.addEventListener('comment-refresh', () => this._reloadDrafts());
- this.addEventListener('comment-discard', e =>
- this._handleCommentDiscard(e)
- );
this.addEventListener('change-message-deleted', () => fireReload(this));
this.addEventListener('editable-content-save', e =>
this._handleCommitMessageSave(e)
@@ -1033,30 +1030,6 @@
);
}
- _handleReloadCommentThreads() {
- // Get any new drafts that have been saved in the diff view and show
- // in the comment thread view.
- this._reloadDrafts().then(() => {
- this._commentThreads = this._changeComments?.getAllThreadsForChange();
- flush();
- });
- }
-
- _handleReloadDiffComments(
- e: CustomEvent<{rootId: UrlEncodedCommentId; path: string}>
- ) {
- // Keeps the file list counts updated.
- this._reloadDrafts().then(() => {
- // Get any new drafts that have been saved in the thread view and show
- // in the diff view.
- this.$.fileList.reloadCommentsForThreadWithRootId(
- e.detail.rootId,
- e.detail.path
- );
- flush();
- });
- }
-
_computeTotalCommentCounts(
unresolvedCount: number,
changeComments: ChangeComments
@@ -1075,79 +1048,6 @@
);
}
- _handleCommentSave(e: CustomEvent<{comment: DraftInfo}>) {
- const draft = e.detail.comment;
- if (!draft.__draft || !draft.path) return;
- if (!this._patchRange)
- throw new Error('missing required _patchRange property');
-
- draft.patch_set = draft.patch_set || this._patchRange.patchNum;
-
- // The use of path-based notification helpers (set, push) can’t be used
- // because the paths could contain dots in them. A new object must be
- // created to satisfy Polymer’s dirty checking.
- // https://github.com/Polymer/polymer/issues/3127
- const diffDrafts = {...this._diffDrafts};
- if (!diffDrafts[draft.path]) {
- diffDrafts[draft.path] = [draft];
- this._diffDrafts = diffDrafts;
- return;
- }
- for (let i = 0; i < diffDrafts[draft.path].length; i++) {
- if (diffDrafts[draft.path][i].id === draft.id) {
- diffDrafts[draft.path][i] = draft;
- this._diffDrafts = diffDrafts;
- return;
- }
- }
- diffDrafts[draft.path].push(draft);
- diffDrafts[draft.path].sort(
- (c1, c2) =>
- // No line number means that it’s a file comment. Sort it above the
- // others.
- (c1.line || -1) - (c2.line || -1)
- );
- this._diffDrafts = diffDrafts;
- }
-
- _handleCommentDiscard(e: CustomEvent<{comment: DraftInfo}>) {
- const draft = e.detail.comment;
- if (!draft.__draft || !draft.path) {
- return;
- }
-
- if (!this._diffDrafts || !this._diffDrafts[draft.path]) {
- return;
- }
- let index = -1;
- for (let i = 0; i < this._diffDrafts[draft.path].length; i++) {
- if (this._diffDrafts[draft.path][i].id === draft.id) {
- index = i;
- break;
- }
- }
- if (index === -1) {
- // It may be a draft that hasn’t been added to _diffDrafts since it was
- // never saved.
- return;
- }
-
- if (!this._patchRange)
- throw new Error('missing required _patchRange property');
- draft.patch_set = draft.patch_set || this._patchRange.patchNum;
-
- // The use of path-based notification helpers (set, push) can’t be used
- // because the paths could contain dots in them. A new object must be
- // created to satisfy Polymer’s dirty checking.
- // https://github.com/Polymer/polymer/issues/3127
- const diffDrafts = {...this._diffDrafts};
- diffDrafts[draft.path].splice(index, 1);
- if (diffDrafts[draft.path].length === 0) {
- delete diffDrafts[draft.path];
- }
- this._diffDrafts = diffDrafts;
- }
-
_handleReplyTap(e: MouseEvent) {
e.preventDefault();
this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
@@ -1576,17 +1476,13 @@
}
_computeReplyButtonLabel(
- changeRecord?: ElementPropertyDeepChange<
- GrChangeView,
- '_diffDrafts'
- > | null,
+ drafts?: {[path: string]: UIDraft[]},
canStartReview?: boolean
) {
- if (changeRecord === undefined || canStartReview === undefined) {
+ if (drafts === undefined || canStartReview === undefined) {
return 'Reply';
}
- const drafts = (changeRecord && changeRecord.base) || {};
const draftCount = Object.keys(drafts).reduce(
(count, file) => count + drafts[file].length,
0
@@ -1986,9 +1882,9 @@
// Slice returns a number as a string, convert to an int.
this._lineHeight = Number(lineHeight.slice(0, lineHeight.length - 2));
+ this.changeService.updateChange(change);
this._change = change;
this.computeRevertSubmitted(change);
- this.changeService.updateChange(change);
if (
!this._patchRange ||
!this._patchRange.patchNum ||
@@ -2086,10 +1982,6 @@
});
}
- _reloadDraftsWithCallback(e: CustomEvent<{resolve: () => void}>) {
- return this._reloadDrafts().then(() => e.detail.resolve());
- }
-
/**
* Fetches a new changeComment object, and data for all types of comments
* (comments, robot comments, draft comments) is requested.
@@ -2099,38 +1991,18 @@
// 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;
if (!this._changeNum)
throw new Error('missing required changeNum property');
- return this.$.commentAPI
- .loadAll(this._changeNum, this._patchRange?.patchNum)
- .then(comments => {
- this._recomputeComments(comments);
- });
+ this.commentsService.loadAll(this._changeNum, this._patchRange?.patchNum);
}
- /**
- * Fetches a new changeComment object, but only updated data for drafts is
- * requested.
- *
- * TODO(taoalpha): clean up this and _reloadComments, as single comment
- * can be a thread so it does not make sense to only update drafts
- * without updating threads
- */
- _reloadDrafts() {
- if (!this._changeNum)
- throw new Error('missing required changeNum property');
- return this.$.commentAPI
- .reloadDrafts(this._changeNum)
- .then(comments => this._recomputeComments(comments));
- }
-
- _recomputeComments(comments: ChangeComments) {
+ @observe('_changeComments')
+ changeCommentsChanged(comments?: ChangeComments) {
+ if (!comments) return;
this._changeComments = comments;
- this._diffDrafts = {...this._changeComments.drafts};
this._commentThreads = this._changeComments.getAllThreadsForChange();
this._draftCommentThreads = this._commentThreads
.filter(isDraftThread)
@@ -2201,10 +2073,7 @@
});
allDataPromises.push(projectConfigLoaded);
- // Resolves when change comments have loaded (comments, drafts and robot
- // comments).
- const commentsLoaded = this._reloadComments();
- allDataPromises.push(commentsLoaded);
+ this._reloadComments();
let coreDataPromise;
@@ -2289,6 +2158,7 @@
}
Promise.all(allDataPromises).then(() => {
+ // Loading of commments data is no longer part of this reporting
this.reporting.timeEnd(Timing.CHANGE_DATA);
if (isLocationChange) {
this.reporting.changeFullyLoaded();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 8823377..5b940a8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -554,7 +554,6 @@
file-list-increment="{{_numFilesShown}}"
on-files-shown-changed="_setShownFiles"
on-file-action-tap="_handleFileActionTap"
- on-reload-drafts="_reloadDraftsWithCallback"
observer-target="[[_computeObserverTarget()]]"
>
</gr-file-list>
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 7d47abb..e882920 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
@@ -26,7 +26,6 @@
HttpMethod,
MessageTag,
PrimaryTab,
- SecondaryTab,
} from '../../../constants/constants';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
@@ -71,7 +70,6 @@
CommitInfo,
EditInfo,
EditPatchSetNum,
- ElementPropertyDeepChange,
GitRef,
NumericChangeId,
ParentPatchSetNum,
@@ -94,13 +92,7 @@
import {SinonFakeTimers, SinonStubbedMember} from 'sinon/pkg/sinon-esm';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {CustomKeyboardEvent} from '../../../types/events';
-import {
- CommentThread,
- DraftInfo,
- UIDraft,
- UIRobot,
-} from '../../../utils/comment-util';
-import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
+import {CommentThread, UIRobot} from '../../../utils/comment-util';
import {GerritView} from '../../../services/router/router-model';
import {ParsedChangeInfo} from '../../../types/types';
import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
@@ -865,179 +857,6 @@
});
});
- suite('reloading drafts', () => {
- let reloadStub: SinonStubbedMember<
- typeof element.$.commentAPI.reloadDrafts
- >;
- const drafts: {[path: string]: UIDraft[]} = {
- 'testfile.txt': [
- {
- patch_set: 5 as PatchSetNum,
- id: 'dd2982f5_c01c9e6a' as UrlEncodedCommentId,
- line: 1,
- updated: '2017-11-08 18:47:45.000000000' as Timestamp,
- message: 'test',
- unresolved: true,
- },
- ],
- };
- setup(() => {
- // Fake computeDraftCount as its required for ChangeComments,
- // see gr-comment-api#reloadDrafts.
- reloadStub = sinon.stub(element.$.commentAPI, 'reloadDrafts').returns(
- Promise.resolve({
- drafts,
- getAllThreadsForChange: () => [] as CommentThread[],
- computeDraftCount: () => 1,
- } as ChangeComments)
- );
- element._changeNum = 1 as NumericChangeId;
- });
-
- test('drafts are reloaded when reload-drafts fired', done => {
- element.$.fileList.dispatchEvent(
- new CustomEvent('reload-drafts', {
- detail: {
- resolve: () => {
- assert.isTrue(reloadStub.called);
- assert.deepEqual(element._diffDrafts, drafts);
- done();
- },
- },
- composed: true,
- bubbles: true,
- })
- );
- });
-
- test('drafts are reloaded when comment-refresh fired', () => {
- element.dispatchEvent(
- new CustomEvent('comment-refresh', {
- composed: true,
- bubbles: true,
- })
- );
- assert.isTrue(reloadStub.called);
- });
- });
-
- suite('_recomputeComments', () => {
- setup(() => {
- element._changeNum = TEST_NUMERIC_CHANGE_ID;
- element._change = createChangeViewChange();
- flush();
- // Fake computeDraftCount as its required for ChangeComments,
- // see gr-comment-api#reloadDrafts.
- sinon.stub(element.$.commentAPI, 'reloadDrafts').returns(
- Promise.resolve({
- drafts: {},
- getAllThreadsForChange: () => THREADS,
- computeDraftCount: () => 0,
- } as ChangeComments)
- );
- element._change = createChangeViewChange();
- element._changeNum = element._change._number;
- });
-
- test('draft threads should be a new copy with correct states', done => {
- element.$.fileList.dispatchEvent(
- new CustomEvent('reload-drafts', {
- detail: {
- resolve: () => {
- assert.equal(element._draftCommentThreads!.length, 2);
- assert.equal(
- element._draftCommentThreads![0].rootId,
- THREADS[0].rootId
- );
- assert.notEqual(
- element._draftCommentThreads![0].comments,
- THREADS[0].comments
- );
- assert.notEqual(
- element._draftCommentThreads![0].comments[0],
- THREADS[0].comments[0]
- );
- assert.isTrue(
- element
- ._draftCommentThreads![0].comments.slice(0, 2)
- .every(c => c.collapsed === true)
- );
-
- assert.isTrue(
- element._draftCommentThreads![0].comments[2].collapsed === false
- );
- done();
- },
- },
- composed: true,
- bubbles: true,
- })
- );
- });
- });
-
- test('diff comments modified', () => {
- const reloadThreadsSpy = sinon.spy(element, '_handleReloadCommentThreads');
- return element._reloadComments().then(() => {
- element.dispatchEvent(
- new CustomEvent('diff-comments-modified', {
- composed: true,
- bubbles: true,
- })
- );
- assert.isTrue(reloadThreadsSpy.called);
- });
- });
-
- test('thread list modified', () => {
- const reloadDiffSpy = sinon.spy(element, '_handleReloadDiffComments');
- element._activeTabs = [PrimaryTab.COMMENT_THREADS, SecondaryTab.CHANGE_LOG];
- flush();
-
- return element._reloadComments().then(() => {
- element.threadList!.dispatchEvent(
- new CustomEvent('thread-list-modified', {
- composed: true,
- bubbles: true,
- })
- );
- assert.isTrue(reloadDiffSpy.called);
-
- let draftStub = sinon
- .stub(element._changeComments!, 'computeDraftCount')
- .returns(1);
- assert.equal(
- element._computeTotalCommentCounts(5, element._changeComments!),
- '5 unresolved, 1 draft'
- );
- assert.equal(
- element._computeTotalCommentCounts(0, element._changeComments!),
- '1 draft'
- );
- draftStub.restore();
- draftStub = sinon
- .stub(element._changeComments!, 'computeDraftCount')
- .returns(0);
- assert.equal(
- element._computeTotalCommentCounts(0, element._changeComments!),
- ''
- );
- assert.equal(
- element._computeTotalCommentCounts(1, element._changeComments!),
- '1 unresolved'
- );
- draftStub.restore();
- draftStub = sinon
- .stub(element._changeComments!, 'computeDraftCount')
- .returns(2);
- assert.equal(
- element._computeTotalCommentCounts(1, element._changeComments!),
- '1 unresolved, 2 drafts'
- );
- draftStub.restore();
- });
- });
-
suite('thread list and change log tabs', () => {
setup(() => {
element._changeNum = TEST_NUMERIC_CHANGE_ID;
@@ -1458,66 +1277,18 @@
test('reply button has updated count when there are drafts', () => {
const getLabel = element._computeReplyButtonLabel;
- assert.equal(getLabel(null, false), 'Reply');
- assert.equal(getLabel(null, true), 'Start Review');
+ assert.equal(getLabel(undefined, false), 'Reply');
+ assert.equal(getLabel(undefined, true), 'Reply');
- const changeRecord: ElementPropertyDeepChange<
- GrChangeView,
- '_diffDrafts'
- > = {base: undefined, path: '', value: undefined};
- assert.equal(getLabel(changeRecord, false), 'Reply');
+ let drafts = {};
+ assert.equal(getLabel(drafts, false), 'Reply');
- changeRecord.base = {};
- assert.equal(getLabel(changeRecord, false), 'Reply');
-
- changeRecord.base = {
+ drafts = {
'file1.txt': [{}],
'file2.txt': [{}, {}],
};
- assert.equal(getLabel(changeRecord, false), 'Reply (3)');
- assert.equal(getLabel(changeRecord, true), 'Start Review (3)');
- });
-
- test('comment events properly update diff drafts', () => {
- element._patchRange = {
- basePatchNum: ParentPatchSetNum,
- patchNum: 2 as RevisionPatchSetNum,
- };
- const draft: DraftInfo = {
- __draft: true,
- id: 'id1' as UrlEncodedCommentId,
- path: '/foo/bar.txt',
- message: 'hello',
- };
- element._handleCommentSave(new CustomEvent('', {detail: {comment: draft}}));
- draft.patch_set = 2 as PatchSetNum;
- assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
- draft.patch_set = undefined;
- draft.message = 'hello, there';
- element._handleCommentSave(new CustomEvent('', {detail: {comment: draft}}));
- draft.patch_set = 2 as PatchSetNum;
- assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
- const draft2: DraftInfo = {
- __draft: true,
- id: 'id2' as UrlEncodedCommentId,
- path: '/foo/bar.txt',
- message: 'hola',
- };
- element._handleCommentSave(
- new CustomEvent('', {detail: {comment: draft2}})
- );
- draft2.patch_set = 2 as PatchSetNum;
- assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft, draft2]});
- draft.patch_set = undefined;
- element._handleCommentDiscard(
- new CustomEvent('', {detail: {comment: draft}})
- );
- draft.patch_set = 2 as PatchSetNum;
- assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft2]});
- element._handleCommentDiscard(
- new CustomEvent('', {detail: {comment: draft2}})
- );
- assert.deepEqual(element._diffDrafts, {});
+ assert.equal(getLabel(drafts, false), 'Reply (3)');
+ assert.equal(getLabel(drafts, true), 'Start Review (3)');
});
test('change num change', () => {
@@ -2588,7 +2359,6 @@
};
sinon.stub(element, '_getChangeDetail').returns(Promise.resolve(false));
sinon.stub(element, '_getProjectConfig').returns(Promise.resolve());
- sinon.stub(element, '_reloadComments').returns(Promise.resolve());
sinon.stub(element, '_getMergeability').returns(Promise.resolve());
sinon.stub(element, '_getLatestCommitMessage').returns(Promise.resolve());
sinon
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_html.ts
index 6def4a5..bed9240 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_html.ts
@@ -57,12 +57,7 @@
class="rebaseOption"
hidden$="[[!_displayParentOption(rebaseOnCurrent, hasParent)]]"
>
- <input
- id="rebaseOnParentInput"
- name="rebaseOptions"
- type="radio"
- on-click="_handleRebaseOnParent"
- />
+ <input id="rebaseOnParentInput" name="rebaseOptions" type="radio" />
<label id="rebaseOnParentLabel" for="rebaseOnParentInput">
Rebase on parent change
</label>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 1185342..106fe27 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -17,6 +17,7 @@
import '../../../styles/shared-styles';
import '../../diff/gr-diff-cursor/gr-diff-cursor';
import '../../diff/gr-diff-host/gr-diff-host';
+import '../../diff/gr-comment-api/gr-comment-api';
import '../../diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog';
import '../../edit/gr-edit-file-controls/gr-edit-file-controls';
import '../../shared/gr-button/gr-button';
@@ -67,7 +68,6 @@
FileNameToFileInfoMap,
NumericChangeId,
PatchRange,
- UrlEncodedCommentId,
} from '../../../types/common';
import {DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
@@ -81,8 +81,13 @@
import {Timing} from '../../../constants/reporting';
import {RevisionInfo} from '../../shared/revision-info/revision-info';
import {preferences$} from '../../../services/user/user-model';
+import {
+ changeComments$,
+ drafts$,
+} from '../../../services/comments/comments-model';
import {Subject} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
+import {UIDraft} from '../../../utils/comment-util';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -176,12 +181,6 @@
return htmlTemplate;
}
- /**
- * Fired when a draft refresh should get triggered
- *
- * @event reload-drafts
- */
-
@property({type: Object})
patchRange?: PatchRange;
@@ -316,6 +315,9 @@
@property({type: Array})
_dynamicPrependedContentEndpoints?: string[];
+ @property({type: Object})
+ diffDrafts?: {[path: string]: UIDraft[]} = {};
+
private readonly reporting = appContext.reportingService;
private readonly restApiService = appContext.restApiService;
@@ -370,6 +372,14 @@
/** @override */
connectedCallback() {
super.connectedCallback();
+ drafts$.pipe(takeUntil(this.disconnected$)).subscribe(drafts => {
+ this.diffDrafts = drafts;
+ });
+ changeComments$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(changeComments => {
+ this.changeComments = changeComments;
+ });
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -1470,66 +1480,50 @@
}
}
- return new Promise(resolve => {
- this.dispatchEvent(
- new CustomEvent('reload-drafts', {
- detail: {resolve},
- composed: true,
- bubbles: true,
- })
+ asyncForeach(files, (file, cancel) => {
+ const path = file.path;
+ this._cancelForEachDiff = cancel;
+
+ iter++;
+ console.info('Expanding diff', iter, 'of', initialCount, ':', path);
+ const diffElem = this._findDiffByPath(path, diffElements);
+ if (!diffElem) {
+ this.reporting.error(
+ new Error(`Did not find <gr-diff-host> element for ${path}`)
+ );
+ return Promise.resolve();
+ }
+ if (!this.diffPrefs) {
+ throw new Error('diffPrefs must be set');
+ }
+
+ const promises: Array<Promise<unknown>> = [diffElem.reload()];
+ if (this._loggedIn && !this.diffPrefs.manual_review) {
+ promises.push(this._reviewFile(path, true));
+ }
+ return Promise.all(promises);
+ }).then(() => {
+ this._cancelForEachDiff = undefined;
+ console.info('Finished expanding', initialCount, 'diff(s)');
+ this.reporting.timeEndWithAverage(
+ Timing.FILE_EXPAND_ALL,
+ Timing.FILE_EXPAND_ALL_AVG,
+ initialCount
);
- }).then(() =>
- asyncForeach(files, (file, cancel) => {
- const path = file.path;
- this._cancelForEachDiff = cancel;
+ /* Block diff cursor from auto scrolling after files are done rendering.
+ * This prevents the bug where the screen jumps to the first diff chunk
+ * after files are done being rendered after the user has already begun
+ * scrolling.
+ * This also however results in the fact that the cursor does not auto
+ * focus on the first diff chunk on a small screen. This is however, a use
+ * case we are willing to not support for now.
- iter++;
- console.info('Expanding diff', iter, 'of', initialCount, ':', path);
- const diffElem = this._findDiffByPath(path, diffElements);
- if (!diffElem) {
- this.reporting.error(
- new Error(`Did not find <gr-diff-host> element for ${path}`)
- );
- return Promise.resolve();
- }
- if (!this.changeComments || !this.patchRange || !this.diffPrefs) {
- throw new Error(
- 'changeComments, patchRange and diffPrefs must be set'
- );
- }
-
- diffElem.threads = this.changeComments.getThreadsBySideForFile(
- file,
- this.patchRange
- );
- const promises: Array<Promise<unknown>> = [diffElem.reload()];
- if (this._loggedIn && !this.diffPrefs.manual_review) {
- promises.push(this._reviewFile(path, true));
- }
- return Promise.all(promises);
- }).then(() => {
- this._cancelForEachDiff = undefined;
- console.info('Finished expanding', initialCount, 'diff(s)');
- this.reporting.timeEndWithAverage(
- Timing.FILE_EXPAND_ALL,
- Timing.FILE_EXPAND_ALL_AVG,
- initialCount
- );
- /* Block diff cursor from auto scrolling after files are done rendering.
- * This prevents the bug where the screen jumps to the first diff chunk
- * after files are done being rendered after the user has already begun
- * scrolling.
- * This also however results in the fact that the cursor does not auto
- * focus on the first diff chunk on a small screen. This is however, a use
- * case we are willing to not support for now.
-
- * Using handleDiffUpdate resulted in diffCursor.row being set which
- * prevented the issue of scrolling to top when we expand the second
- * file individually.
- */
- this.diffCursor.reInitAndUpdateStops();
- })
- );
+ * Using handleDiffUpdate resulted in diffCursor.row being set which
+ * prevented the issue of scrolling to top when we expand the second
+ * file individually.
+ */
+ this.diffCursor.reInitAndUpdateStops();
+ });
}
/** Cancel the rendering work of every diff in the list */
@@ -1552,47 +1546,6 @@
return undefined;
}
- /**
- * Reset the comments of a modified thread
- */
- reloadCommentsForThreadWithRootId(rootId: UrlEncodedCommentId, path: string) {
- // Don't bother continuing if we already know that the path that contains
- // the updated comment thread is not expanded.
- if (!this._expandedFiles.some(f => f.path === path)) {
- return;
- }
- const diff = this.diffs.find(d => d.path === path);
-
- if (!diff) {
- throw new Error("Can't find diff by path");
- }
-
- const threadEl = diff.getThreadEls().find(t => t.rootId === rootId);
- if (!threadEl) {
- return;
- }
-
- if (!this.changeComments) {
- throw new Error('changeComments must be set');
- }
-
- const newComments = this.changeComments.getCommentsForThread(rootId);
-
- // If newComments is null, it means that a single draft was
- // removed from a thread in the thread view, and the thread should
- // no longer exist. Remove the existing thread element in the diff
- // view.
- if (!newComments) {
- threadEl.fireRemoveSelf();
- return;
- }
-
- threadEl.comments = newComments.map(c => {
- return {...c};
- });
- flush();
- }
-
_handleEscKey(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
return;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index ee7d04e..c32e3e5 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -49,8 +49,7 @@
const commentApiMock = createCommentApiMockWithTemplateElement(
'gr-file-list-comment-api-mock', html`
<gr-file-list id="fileList"
- change-comments="[[_changeComments]]"
- on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list>
+ change-comments="[[_changeComments]]"></gr-file-list>
<gr-comment-api id="commentAPI"></gr-comment-api>
`);
@@ -67,7 +66,6 @@
let commentApiWrapper;
let saveStub;
- let loadCommentSpy;
suiteSetup(() => {
const kb = TestKeyboardShortcutBinder.push();
@@ -109,14 +107,7 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.fileList;
- loadCommentSpy = sinon.spy(commentApiWrapper.$.commentAPI, 'loadAll');
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- commentApiWrapper.loadComments().then(() => {
- sinon.stub(element.changeComments, 'getPaths').returns({});
- done();
- });
element._loading = false;
element.diffPrefs = {};
element.numFilesShown = 200;
@@ -126,6 +117,7 @@
};
saveStub = sinon.stub(element, '_saveReviewedState').callsFake(
() => Promise.resolve());
+ done();
});
test('correct number of files are shown', () => {
@@ -1008,7 +1000,7 @@
FilesExpandedState.ALL);
});
- test('_renderInOrder', done => {
+ test('_renderInOrder', async () => {
const reviewStub = sinon.stub(element, '_reviewFile');
let callCount = 0;
const diffs = [{
@@ -1038,15 +1030,12 @@
}];
element._renderInOrder([
{path: 'p2'}, {path: 'p1'}, {path: 'p0'},
- ], diffs, 3)
- .then(() => {
- assert.isFalse(reviewStub.called);
- assert.isTrue(loadCommentSpy.called);
- done();
- });
+ ], diffs, 3);
+ await flush();
+ assert.isFalse(reviewStub.called);
});
- test('_renderInOrder logged in', done => {
+ test('_renderInOrder logged in', async () => {
element._loggedIn = true;
const reviewStub = sinon.stub(element, '_reviewFile');
let callCount = 0;
@@ -1080,14 +1069,12 @@
}];
element._renderInOrder([
{path: 'p2'}, {path: 'p1'}, {path: 'p0'},
- ], diffs, 3)
- .then(() => {
- assert.equal(reviewStub.callCount, 3);
- done();
- });
+ ], diffs, 3);
+ await flush();
+ assert.equal(reviewStub.callCount, 3);
});
- test('_renderInOrder respects diffPrefs.manual_review', () => {
+ test('_renderInOrder respects diffPrefs.manual_review', async () => {
element._loggedIn = true;
element.diffPrefs = {manual_review: true};
const reviewStub = sinon.stub(element, '_reviewFile');
@@ -1098,14 +1085,14 @@
reload() { return Promise.resolve(); },
}];
- return element._renderInOrder([{path: 'p'}], diffs, 1).then(() => {
- assert.isFalse(reviewStub.called);
- delete element.diffPrefs.manual_review;
- return element._renderInOrder([{path: 'p'}], diffs, 1).then(() => {
- assert.isTrue(reviewStub.called);
- assert.isTrue(reviewStub.calledWithExactly('p', true));
- });
- });
+ element._renderInOrder([{path: 'p'}], diffs, 1);
+ await flush();
+ assert.isFalse(reviewStub.called);
+ delete element.diffPrefs.manual_review;
+ element._renderInOrder([{path: 'p'}], diffs, 1);
+ await flush();
+ assert.isTrue(reviewStub.called);
+ assert.isTrue(reviewStub.calledWithExactly('p', true));
});
test('_loadingChanged fired from reload in debouncer', async () => {
@@ -1504,14 +1491,12 @@
ignore_whitespace: 'IGNORE_NONE',
};
diff.diff = getMockDiffResponse();
- commentApiWrapper.loadComments().then(() => {
- sinon.stub(element.changeComments, 'getCommentsForPath')
- .withArgs('/COMMIT_MSG', {
- basePatchNum: 'PARENT',
- patchNum: 2,
- })
- .returns(diff.comments);
- });
+ sinon.stub(diff.changeComments, 'getCommentsForPath')
+ .withArgs('/COMMIT_MSG', {
+ basePatchNum: 'PARENT',
+ patchNum: 2,
+ })
+ .returns(diff.comments);
await listenOnce(diff, 'render');
}
@@ -1543,17 +1528,10 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.fileList;
- loadCommentSpy = sinon.spy(commentApiWrapper.$.commentAPI, 'loadAll');
element.diffPrefs = {};
element.change = {_number: 42, project: 'testRepo'};
sinon.stub(element, '_reviewFile');
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- commentApiWrapper.loadComments().then(() => {
- sinon.stub(element.changeComments, 'getPaths').returns({});
- done();
- });
element._loading = false;
element.numFilesShown = 75;
element.selectedIndex = 0;
@@ -1581,6 +1559,7 @@
};
sinon.stub(window, 'fetch').callsFake(() => Promise.resolve());
flush();
+ done();
});
test('cursor with individually opened files', async () => {
@@ -1805,110 +1784,6 @@
.map(row => row.querySelector('gr-edit-file-controls'));
assert.isTrue(editControls[0].classList.contains('invisible'));
});
-
- test('reloadCommentsForThreadWithRootId', async () => {
- // Expand the commit message diff
- MockInteractions.keyUpOn(element, 73, 'shift', 'i');
- const diffs = await renderAndGetNewDiffs(0);
- flush();
-
- // Two comment threads should be generated by renderAndGetNewDiffs
- const threadEls = diffs[0].getThreadEls();
- assert.equal(threadEls.length, 2);
- const threadElsByRootId = new Map(
- threadEls.map(threadEl => [threadEl.rootId, threadEl]));
-
- const thread1 = threadElsByRootId.get('503008e2_0ab203ee');
- assert.equal(thread1.comments.length, 1);
- assert.equal(thread1.comments[0].message, 'a comment');
- assert.equal(thread1.comments[0].line, 10);
-
- const thread2 = threadElsByRootId.get('ecf0b9fa_fe1a5f62');
- assert.equal(thread2.comments.length, 2);
- assert.isTrue(thread2.comments[0].unresolved);
- assert.equal(thread2.comments[0].message, 'another comment');
- assert.equal(thread2.comments[0].line, 20);
-
- const commentStub =
- sinon.stub(element.changeComments, 'getCommentsForThread');
- const commentStubRes1 = [
- {
- patch_set: 2,
- path: '/p',
- id: '503008e2_0ab203ee',
- line: 20,
- updated: '2018-02-08 18:49:18.000000000',
- message: 'edited text',
- unresolved: false,
- },
- ];
- const commentStubRes2 = [
- {
- patch_set: 2,
- path: '/p',
- id: 'ecf0b9fa_fe1a5f62',
- line: 20,
- updated: '2018-02-08 18:49:18.000000000',
- message: 'another comment',
- unresolved: true,
- },
- {
- patch_set: 2,
- path: '/p',
- id: '503008e2_0ab203ee',
- line: 10,
- in_reply_to: 'ecf0b9fa_fe1a5f62',
- updated: '2018-02-14 22:07:43.000000000',
- message: 'response',
- unresolved: true,
- },
- {
- patch_set: 2,
- path: '/p',
- id: '503008e2_0ab203ef',
- line: 20,
- in_reply_to: '503008e2_0ab203ee',
- updated: '2018-02-15 22:07:43.000000000',
- message: 'a third comment in the thread',
- unresolved: true,
- },
- ];
- commentStub.withArgs('503008e2_0ab203ee').returns(
- commentStubRes1);
- commentStub.withArgs('ecf0b9fa_fe1a5f62').returns(
- commentStubRes2);
-
- // Reload comments from the first comment thread, which should have a
- // an updated message and a toggled resolve state.
- element.reloadCommentsForThreadWithRootId('503008e2_0ab203ee',
- '/COMMIT_MSG');
- assert.equal(thread1.comments.length, 1);
- assert.isFalse(thread1.comments[0].unresolved);
- assert.equal(thread1.comments[0].message, 'edited text');
-
- // Reload comments from the second comment thread, which should have a new
- // reply.
- element.reloadCommentsForThreadWithRootId('ecf0b9fa_fe1a5f62',
- '/COMMIT_MSG');
- assert.equal(thread2.comments.length, 3);
-
- const commentStubCount = commentStub.callCount;
- const getThreadsSpy = sinon.spy(diffs[0], 'getThreadEls');
-
- // Should not be getting threads when the file is not expanded.
- element.reloadCommentsForThreadWithRootId('ecf0b9fa_fe1a5f62',
- 'other/file');
- assert.isFalse(getThreadsSpy.called);
- assert.equal(commentStubCount, commentStub.callCount);
-
- // Should be query selecting diffs when the file is expanded.
- // Should not be fetching change comments when the rootId is not found
- // to match.
- element.reloadCommentsForThreadWithRootId('acf0b9fa_fe1a5f62',
- '/COMMIT_MSG');
- assert.isTrue(getThreadsSpy.called);
- assert.equal(commentStubCount, commentStub.callCount);
- });
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
index fd45eec..0939daa 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
@@ -23,6 +23,7 @@
import {MessageTag} from '../../../constants/constants.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {stubRestApi} from '../../../test/test-utils.js';
+import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api.js';
createCommentApiMockWithTemplateElement(
'gr-messages-list-comment-mock-api', html`
@@ -143,11 +144,9 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.messagesList;
+ element.changeComments = new ChangeComments(comments);
element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
+ flush();
});
test('expand/collapse all', () => {
@@ -454,12 +453,9 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.messagesList;
- sinon.spy(commentApiWrapper.$.commentAPI, 'loadAll');
+ element.changeComments = new ChangeComments();
element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
+ flush();
});
test('hide autogenerated button is not hidden', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 36cb053..5e6b076 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -47,7 +47,6 @@
UIComment,
} from '../../../utils/comment-util';
import {pluralize} from '../../../utils/string-util';
-import {fireThreadListModifiedEvent} from '../../../utils/event-util';
import {assertIsDefined, assertNever} from '../../../utils/common-util';
import {CommentTabState} from '../../../types/events';
import {DropdownItem} from '../../shared/gr-dropdown-list/gr-dropdown-list';
@@ -571,10 +570,6 @@
this.removeThread(e.detail.rootId);
}
- _handleCommentsChanged(e: CustomEvent) {
- fireThreadListModifiedEvent(this, e.detail.rootId, e.detail.path);
- }
-
_isOnParent(side?: CommentSide) {
// TODO(TS): That looks like a bug? CommentSide.REVISION will also be
// classified as parent??
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index b5d2b4d..73f3dd3 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -168,7 +168,6 @@
path="[[thread.path]]"
root-id="{{thread.rootId}}"
should-scroll-into-view="[[computeShouldScrollIntoView(thread.comments, scrollCommentId)]]"
- on-thread-changed="_handleCommentsChanged"
on-thread-discard="_handleThreadDiscard"
></gr-comment-thread>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
index 56ed8ce..61f737a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
@@ -651,23 +651,6 @@
assert.equal(getVisibleThreads().length, 2);
});
- test('modification events are consumed and displatched', () => {
- sinon.spy(element, '_handleCommentsChanged');
- const dispatchSpy = sinon.stub();
- element.addEventListener('thread-list-modified', dispatchSpy);
- threadElements[0].dispatchEvent(
- new CustomEvent('thread-changed', {
- detail: {
- rootId: 'ecf0b9fa_fe1a5f62', path: '/COMMIT_MSG'},
- composed: true, bubbles: true,
- }));
- assert.isTrue(element._handleCommentsChanged.called);
- assert.isTrue(dispatchSpy.called);
- assert.equal(dispatchSpy.lastCall.args[0].detail.rootId,
- 'ecf0b9fa_fe1a5f62');
- assert.equal(dispatchSpy.lastCall.args[0].detail.path, '/COMMIT_MSG');
- });
-
suite('hideDropdown', () => {
setup(done => {
element.hideDropdown = true;
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index bf4da14..315fbfb 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -16,13 +16,11 @@
*/
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-comment-api_html';
-import {CURRENT} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {
CommentBasics,
PatchRange,
PatchSetNum,
- PathToRobotCommentsInfoMap,
RobotCommentInfo,
UrlEncodedCommentId,
NumericChangeId,
@@ -44,6 +42,7 @@
isInBaseOfPatchRange,
isInRevisionOfPatchRange,
isPatchsetLevel,
+ addPath,
} from '../../../utils/comment-util';
import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types';
import {appContext} from '../../../services/app-context';
@@ -56,7 +55,7 @@
};
export class ChangeComments {
- private readonly _comments: {[path: string]: CommentInfo[]};
+ private readonly _comments: PathToCommentsInfoMap;
private readonly _robotComments: {[path: string]: RobotCommentInfo[]};
@@ -71,41 +70,19 @@
* elements of that which uses the gr-comment-api.
*/
constructor(
- comments: {[path: string]: CommentInfo[]} | undefined,
+ comments: PathToCommentsInfoMap | undefined,
robotComments: {[path: string]: RobotCommentInfo[]} | undefined,
drafts: {[path: string]: DraftInfo[]} | undefined,
portedComments: PathToCommentsInfoMap | undefined,
portedDrafts: PathToCommentsInfoMap | undefined
) {
- this._comments = this._addPath(comments);
- this._robotComments = this._addPath(robotComments);
- this._drafts = this._addPath(drafts);
+ this._comments = addPath(comments);
+ this._robotComments = addPath(robotComments);
+ this._drafts = addPath(drafts);
this._portedComments = portedComments || {};
this._portedDrafts = portedDrafts || {};
}
- /**
- * Add path info to every comment as CommentInfo returned
- * from server does not have that.
- *
- * TODO(taoalpha): should consider changing BE to send path
- * back within CommentInfo
- */
- _addPath<T>(
- comments: {[path: string]: T[]} = {}
- ): {[path: string]: Array<T & {path: string}>} {
- const updatedComments: {[path: string]: Array<T & {path: string}>} = {};
- for (const filePath of Object.keys(comments)) {
- const allCommentsForPath = comments[filePath] || [];
- if (allCommentsForPath.length) {
- updatedComments[filePath] = allCommentsForPath.map(comment => {
- return {...comment, path: filePath};
- });
- }
- }
- return updatedComments;
- }
-
get drafts() {
return this._drafts;
}
@@ -627,12 +604,6 @@
}
}
-// TODO(TS): move findCommentById out of class
-export const _testOnly_findCommentById =
- ChangeComments.prototype.findCommentById;
-
-export const _testOnly_getCommentsForPath =
- ChangeComments.prototype.getCommentsForPath;
@customElement('gr-comment-api')
export class GrCommentApi extends PolymerElement {
static get template() {
@@ -644,56 +615,11 @@
private readonly restApiService = appContext.restApiService;
- /**
- * Load all comments (with drafts and robot comments) for the given change
- * number. The returned promise resolves when the comments have loaded, but
- * does not yield the comment data.
- */
- loadAll(changeNum: NumericChangeId, patchNum?: PatchSetNum) {
- const revision = patchNum || CURRENT;
- const commentsPromise = [
- this.restApiService.getDiffComments(changeNum),
- this.restApiService.getDiffRobotComments(changeNum),
- this.restApiService.getDiffDrafts(changeNum),
- this.restApiService.getPortedComments(changeNum, revision),
- this.restApiService.getPortedDrafts(changeNum, revision),
- ];
-
- return Promise.all(commentsPromise).then(
- ([comments, robotComments, drafts, portedComments, portedDrafts]) => {
- this._changeComments = new ChangeComments(
- comments,
- // TS 4.0.5 fails without 'as'
- robotComments as PathToRobotCommentsInfoMap | undefined,
- drafts,
- portedComments,
- portedDrafts
- );
- return this._changeComments;
- }
- );
- }
-
- /**
- * Re-initialize _changeComments with a new ChangeComments object, that
- * uses the previous values for comments and robot comments, but fetches
- * updated draft comments.
- */
- reloadDrafts(changeNum: NumericChangeId) {
- if (!this._changeComments) {
- return this.loadAll(changeNum);
- }
- return this.restApiService.getDiffDrafts(changeNum).then(drafts => {
- this._changeComments = this._changeComments!.cloneWithUpdatedDrafts(
- drafts
- );
- return this._changeComments;
- });
- }
+ private readonly commentsService = appContext.commentsService;
reloadPortedComments(changeNum: NumericChangeId, patchNum: PatchSetNum) {
if (!this._changeComments) {
- this.loadAll(changeNum);
+ this.commentsService.loadAll(changeNum);
return Promise.resolve();
}
return Promise.all([
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index fbbe1fb..7e01371 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -34,107 +34,11 @@
element = basicFixture.instantiate();
});
- test('loads logged-out', () => {
- const changeNum = 1234;
-
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [{id: '123', message: 'foo bar', in_reply_to: '321'}],
- }));
- const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({'foo.c': [{id: '321', message: 'done'}]}));
- const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
- Promise.resolve({}));
-
- return element.loadAll(changeNum).then(() => {
- assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
- assert.isOk(element._changeComments._comments);
- assert.isOk(element._changeComments._robotComments);
- assert.deepEqual(element._changeComments._drafts, {});
- });
- });
-
- test('loads logged-in', () => {
- const changeNum = 1234;
-
- const getCommentsStub = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [{id: '123', message: 'foo bar', in_reply_to: '321'}],
- })
- );
- const getRobotCommentsStub = stubRestApi('getDiffRobotComments')
- .returns(Promise.resolve({'foo.c': [{id: '321', message: 'done'}]}));
- const getDraftsStub = stubRestApi('getDiffDrafts')
- .returns(Promise.resolve({'foo.c': [{id: '555', message: 'ack'}]}));
-
- return element.loadAll(changeNum).then(() => {
- assert.isTrue(getCommentsStub.calledWithExactly(changeNum));
- assert.isTrue(getRobotCommentsStub.calledWithExactly(changeNum));
- assert.isTrue(getDraftsStub.calledWithExactly(changeNum));
- assert.isOk(element._changeComments._comments);
- assert.isOk(element._changeComments._robotComments);
- assert.notDeepEqual(element._changeComments._drafts, {});
- });
- });
-
- suite('reloadDrafts', () => {
- let commentStub;
- let robotCommentStub;
- let draftStub;
- setup(() => {
- commentStub = stubRestApi('getDiffComments')
- .returns(Promise.resolve({}));
- robotCommentStub = stubRestApi(
- 'getDiffRobotComments').returns(Promise.resolve({}));
- draftStub = stubRestApi('getDiffDrafts')
- .returns(Promise.resolve({}));
- });
-
- test('without loadAll first', done => {
- assert.isNotOk(element._changeComments);
- sinon.spy(element, 'loadAll');
- element.reloadDrafts().then(() => {
- assert.isTrue(element.loadAll.called);
- assert.isOk(element._changeComments);
- assert.equal(commentStub.callCount, 1);
- assert.equal(robotCommentStub.callCount, 1);
- assert.equal(draftStub.callCount, 1);
- done();
- });
- });
-
- test('with loadAll first', done => {
- assert.isNotOk(element._changeComments);
- element.loadAll()
- .then(() => {
- assert.isOk(element._changeComments);
- assert.equal(commentStub.callCount, 1);
- assert.equal(robotCommentStub.callCount, 1);
- assert.equal(draftStub.callCount, 1);
- return element.reloadDrafts();
- })
- .then(() => {
- assert.isOk(element._changeComments);
- assert.equal(commentStub.callCount, 1);
- assert.equal(robotCommentStub.callCount, 1);
- assert.equal(draftStub.callCount, 2);
- done();
- });
- });
- });
-
suite('_changeComment methods', () => {
- setup(done => {
- const changeNum = 1234;
+ setup(() => {
stubRestApi('getDiffComments').returns(Promise.resolve({}));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
- element.loadAll(changeNum).then(() => {
- done();
- });
});
suite('ported comments', () => {
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 72c7a62..d26e1af 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
@@ -87,6 +87,10 @@
import {DiffContextExpandedEventDetail} from '../gr-diff-builder/gr-diff-builder';
import {TokenHighlightLayer} from '../gr-diff-builder/token-highlight-layer';
import {Timing} from '../../../constants/reporting';
+import {changeComments$} from '../../../services/comments/comments-model';
+import {takeUntil} from 'rxjs/operators';
+import {ChangeComments} from '../gr-comment-api/gr-comment-api';
+import {Subject} from 'rxjs';
const MSG_EMPTY_BLAME = 'No blame information for this diff.';
@@ -145,12 +149,6 @@
* @event show-auth-required
*/
- /**
- * Fired when a comment is saved or discarded
- *
- * @event diff-comments-modified
- */
-
@property({type: Number})
changeNum?: NumericChangeId;
@@ -238,6 +236,9 @@
diff?: DiffInfo;
@property({type: Object})
+ changeComments?: ChangeComments;
+
+ @property({type: Object})
_fetchDiffPromise: Promise<DiffInfo> | null = null;
@property({type: Object})
@@ -272,6 +273,8 @@
private readonly syntaxLayer = new GrSyntaxLayer();
+ disconnected$ = new Subject();
+
constructor() {
super();
this.addEventListener(
@@ -284,12 +287,6 @@
'create-comment',
e => this._handleCreateComment(e)
);
- this.addEventListener('comment-discard', () =>
- this._handleCommentSaveOrDiscard()
- );
- this.addEventListener('comment-save', () =>
- this._handleCommentSaveOrDiscard()
- );
this.addEventListener('render-start', () => this._handleRenderStart());
this.addEventListener('render-content', () => this._handleRenderContent());
this.addEventListener('normalize-range', event =>
@@ -314,10 +311,16 @@
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
+ changeComments$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(changeComments => {
+ this.changeComments = changeComments;
+ });
}
/** @override */
disconnectedCallback() {
+ this.disconnected$.next();
this.clear();
super.disconnectedCallback();
}
@@ -502,6 +505,16 @@
);
}
+ @observe('changeComments', 'patchRange', 'file')
+ computeFileThreads(
+ changeComments?: ChangeComments,
+ patchRange?: PatchRange,
+ file?: PatchSetFile
+ ) {
+ if (!changeComments || !patchRange || !file) return;
+ this.threads = changeComments.getThreadsBySideForFile(file, patchRange);
+ }
+
_getFilesWeblinks(diff: DiffInfo) {
if (!this.projectName || !this.commitRange || !this.path) return {};
return {
@@ -1010,10 +1023,6 @@
: null;
}
- _handleCommentSaveOrDiscard() {
- fireEvent(this, 'diff-comments-modified');
- }
-
_syntaxHighlightingEnabledChanged(_syntaxHighlightingEnabled: boolean) {
this.syntaxLayer.setEnabled(_syntaxHighlightingEnabled);
}
@@ -1168,7 +1177,6 @@
'normalize-range': CustomEvent;
'diff-context-expanded': CustomEvent<DiffContextExpandedEventDetail>;
'create-comment': CustomEvent;
- 'comment-discard': CustomEvent;
'comment-update': CustomEvent;
'comment-save': CustomEvent;
'root-id-changed': CustomEvent;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 54985bb..fe8a1f7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -107,6 +107,9 @@
import {toggleClass, getKeyboardEvent} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
import {throttleWrap} from '../../../utils/async-util';
+import {changeComments$} from '../../../services/comments/comments-model';
+import {takeUntil} from 'rxjs/operators';
+import {Subject} from 'rxjs';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
@@ -332,12 +335,16 @@
private readonly restApiService = appContext.restApiService;
+ private readonly commentsService = appContext.commentsService;
+
_throttledToggleFileReviewed?: EventListener;
_onRenderHandler?: EventListener;
private cursor = new GrDiffCursor();
+ disconnected$ = new Subject();
+
/** @override */
connectedCallback() {
super.connectedCallback();
@@ -347,7 +354,11 @@
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
-
+ changeComments$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(changeComments => {
+ this._changeComments = changeComments;
+ });
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.cursor.replaceDiffs([this.$.diffHost]);
this._onRenderHandler = (_: Event) => {
@@ -358,6 +369,7 @@
/** @override */
disconnectedCallback() {
+ this.disconnected$.next();
this.cursor.dispose();
if (this._onRenderHandler) {
this.$.diffHost.removeEventListener('render', this._onRenderHandler);
@@ -365,6 +377,26 @@
super.disconnectedCallback();
}
+ @observe('_changeComments', '_path', '_patchRange')
+ computeThreads(
+ changeComments?: ChangeComments,
+ path?: string,
+ patchRange?: PatchRange
+ ) {
+ if (
+ changeComments === undefined ||
+ path === undefined ||
+ patchRange === undefined
+ ) {
+ return;
+ }
+ // TODO(dhruvsri): check if basePath should be set here
+ this.$.diffHost.threads = changeComments.getThreadsBySideForFile(
+ {path},
+ patchRange
+ );
+ }
+
_getLoggedIn(): Promise<boolean> {
return this.restApiService.getLoggedIn();
}
@@ -1117,7 +1149,7 @@
);
promises.push(this._getChangeDetail(this._changeNum));
- promises.push(this._loadComments(value.patchNum));
+ this._loadComments(value.patchNum);
promises.push(this._getChangeEdit());
@@ -1130,17 +1162,6 @@
this._initPatchRange();
this._initCommitRange();
- assertIsDefined(this._path, '_path');
- if (!this._changeComments)
- throw new Error('change comments must be defined');
- assertIsDefined(this._patchRange, '_patchRange');
-
- // TODO(dhruvsri): check if basePath should be set here
- this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
- {path: this._path},
- this._patchRange
- );
-
const edit = r[4] as EditInfo | undefined;
if (edit) {
this.set(`_change.revisions.${edit.commit.commit}`, {
@@ -1558,15 +1579,18 @@
_loadComments(patchSet?: PatchSetNum) {
assertIsDefined(this._changeNum, '_changeNum');
- return this.$.commentAPI
- .loadAll(this._changeNum, patchSet)
- .then(comments => {
- this._changeComments = comments;
- });
+ return this.commentsService.loadAll(this._changeNum, patchSet);
}
- @observe('_files.changeFilesByPath', '_path', '_patchRange', '_projectConfig')
+ @observe(
+ '_changeComments',
+ '_files.changeFilesByPath',
+ '_path',
+ '_patchRange',
+ '_projectConfig'
+ )
_recomputeComments(
+ changeComments?: ChangeComments,
files?: {[path: string]: FileInfo},
path?: string,
patchRange?: PatchRange,
@@ -1576,11 +1600,11 @@
if (!path) return;
if (!patchRange) return;
if (!projectConfig) return;
- if (!this._changeComments) return;
+ if (!changeComments) return;
const file = files[path];
if (file && file.old_path) {
- this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
+ this.$.diffHost.threads = changeComments.getThreadsBySideForFile(
{path, basePath: file.old_path},
patchRange
);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index ebcfa18..1960ced 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -21,7 +21,7 @@
import {ChangeStatus} from '../../../constants/constants.js';
import {TestKeyboardShortcutBinder, stubRestApi} from '../../../test/test-utils.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {ChangeComments, _testOnly_findCommentById, _testOnly_getCommentsForPath} from '../gr-comment-api/gr-comment-api.js';
+import {ChangeComments} from '../gr-comment-api/gr-comment-api.js';
import {GerritView} from '../../../services/router/router-model.js';
import {
createChange,
@@ -40,6 +40,7 @@
suite('basic tests', () => {
let element;
let clock;
+ let diffCommentsStub;
suiteSetup(() => {
const kb = TestKeyboardShortcutBinder.push();
@@ -100,7 +101,8 @@
Promise.resolve({}));
stubRestApi('getChangeFiles').returns(Promise.resolve({}));
stubRestApi('saveFileReviewed').returns(Promise.resolve());
- stubRestApi('getDiffComments').returns(Promise.resolve({}));
+ diffCommentsStub = stubRestApi('getDiffComments');
+ diffCommentsStub.returns(Promise.resolve({}));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
@@ -114,32 +116,21 @@
patchNum: 77,
basePatchNum: 'PARENT',
};
- sinon.stub(element.$.commentAPI, 'loadAll').returns(Promise.resolve({
- _comments: {'/COMMIT_MSG': [
- {
- ...createComment(),
- id: 'c1',
- line: 10,
- patch_set: 2,
- path: '/COMMIT_MSG',
- }, {
- ...createComment(),
- id: 'c3',
- line: 10,
- patch_set: 'PARENT',
- path: '/COMMIT_MSG',
- },
- ]},
- computeCommentThreadCount: () => {},
- computeCommentsString: () => '',
- computeUnresolvedNum: () => {},
- getPaths: () => {},
- getThreadsBySideForFile: () => [],
- getCommentsForPath: _testOnly_getCommentsForPath,
- findCommentById: _testOnly_findCommentById,
-
- }));
- await element._loadComments();
+ element._changeComments = new ChangeComments({'/COMMIT_MSG': [
+ {
+ ...createComment(),
+ id: 'c1',
+ line: 10,
+ patch_set: 2,
+ path: '/COMMIT_MSG',
+ }, {
+ ...createComment(),
+ id: 'c3',
+ line: 10,
+ patch_set: 'PARENT',
+ path: '/COMMIT_MSG',
+ },
+ ]});
await flush();
});
@@ -186,6 +177,22 @@
});
test('comment url resolves to comment.patch_set vs latest', () => {
+ diffCommentsStub.returns(Promise.resolve({
+ '/COMMIT_MSG': [
+ {
+ ...createComment(),
+ id: 'c1',
+ line: 10,
+ patch_set: 2,
+ path: '/COMMIT_MSG',
+ }, {
+ ...createComment(),
+ id: 'c3',
+ line: 10,
+ patch_set: 'PARENT',
+ path: '/COMMIT_MSG',
+ },
+ ]}));
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
@@ -236,6 +243,22 @@
test('unchanged diff X vs latest from comment links navigates to base vs X'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ diffCommentsStub.returns(Promise.resolve({
+ '/COMMIT_MSG': [
+ {
+ ...createComment(),
+ id: 'c1',
+ line: 10,
+ patch_set: 2,
+ path: '/COMMIT_MSG',
+ }, {
+ ...createComment(),
+ id: 'c3',
+ line: 10,
+ patch_set: 'PARENT',
+ path: '/COMMIT_MSG',
+ },
+ ]}));
sinon.stub(element.reporting, 'diffViewDisplayed');
sinon.stub(element, '_loadBlame');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
@@ -265,6 +288,22 @@
test('unchanged diff Base vs latest from comment does not navigate'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ diffCommentsStub.returns(Promise.resolve({
+ '/COMMIT_MSG': [
+ {
+ ...createComment(),
+ id: 'c1',
+ line: 10,
+ patch_set: 2,
+ path: '/COMMIT_MSG',
+ }, {
+ ...createComment(),
+ id: 'c3',
+ line: 10,
+ patch_set: 'PARENT',
+ path: '/COMMIT_MSG',
+ },
+ ]}));
sinon.stub(element.reporting, 'diffViewDisplayed');
sinon.stub(element, '_loadBlame');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
@@ -322,6 +361,22 @@
});
test('diff toast to go to latest is shown and not base', async () => {
+ diffCommentsStub.returns(Promise.resolve({
+ '/COMMIT_MSG': [
+ {
+ ...createComment(),
+ id: 'c1',
+ line: 10,
+ patch_set: 2,
+ path: '/COMMIT_MSG',
+ }, {
+ ...createComment(),
+ id: 'c3',
+ line: 10,
+ patch_set: 'PARENT',
+ path: '/COMMIT_MSG',
+ },
+ ]}));
sinon.stub(element.reporting, 'diffViewDisplayed');
sinon.stub(element, '_loadBlame');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
index 9af7e05..89b8b4a 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
@@ -63,7 +63,7 @@
// Stub methods on the changeComments object after changeComments has
// been initialized.
- return commentApiWrapper.loadComments();
+ element.changeComments = new ChangeComments();
});
test('enabled/disabled options', () => {
@@ -217,11 +217,10 @@
// Should be recomputed for each available patch
sinon.stub(element, '_computeBaseDropdownContent');
assert.equal(element._computeBaseDropdownContent.callCount, 0);
- commentApiWrapper.loadComments().then()
- .then(() => {
- assert.equal(element._computeBaseDropdownContent.callCount, 1);
- done();
- });
+ element.changeComments = new ChangeComments();
+ flush();
+ assert.equal(element._computeBaseDropdownContent.callCount, 1);
+ done();
});
test('_computePatchDropdownContent called when basePatchNum updates', () => {
@@ -248,33 +247,6 @@
assert.equal(element._computePatchDropdownContent.callCount, 1);
});
- test('_computePatchDropdownContent called when comments update', done => {
- element.revisions = [
- {commit: {parents: []}},
- {commit: {parents: []}},
- {commit: {parents: []}},
- {commit: {parents: []}},
- ];
- element.revisionInfo = getInfo(element.revisions);
- element.availablePatches = [
- {num: 1, sha: '1'},
- {num: 2, sha: '2'},
- {num: 3, sha: '3'},
- {num: 'edit', sha: '4'},
- ];
- element.patchNum = 2;
- element.basePatchNum = 'PARENT';
- flush();
-
- // Should be recomputed for each available patch
- sinon.stub(element, '_computePatchDropdownContent');
- assert.equal(element._computePatchDropdownContent.callCount, 0);
- commentApiWrapper.loadComments().then()
- .then(() => {
- done();
- });
- });
-
test('_computePatchDropdownContent', () => {
const availablePatches = [
{num: 'edit', sha: '1'},
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
index 7e4edd6..f125bfa 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
@@ -48,7 +48,7 @@
_loading = true;
@property({type: String})
- _filter = '';
+ _filter?: string;
private readonly restApiService = appContext.restApiService;
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts
index dd1faeb..95ce1ec 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts
@@ -25,7 +25,6 @@
</style>
<gr-list-view
filter="[[_filter]]"
- items="false"
offset="0"
loading="[[_loading]]"
path="/Documentation"
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.js b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
similarity index 68%
rename from polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.js
rename to polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
index f5e47ca..9c196c7 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.js
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -15,15 +15,18 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-documentation-search.js';
-import {page} from '../../../utils/page-wrapper-utils.js';
-import 'lodash/lodash.js';
-import {stubRestApi} from '../../../test/test-utils.js';
+import '../../../test/common-test-setup-karma';
+import './gr-documentation-search';
+import {GrDocumentationSearch} from './gr-documentation-search';
+import {page} from '../../../utils/page-wrapper-utils';
+import 'lodash/lodash';
+import {stubRestApi} from '../../../test/test-utils';
+import {ListViewParams} from '../../../mixins/gr-list-view-mixin/gr-list-view-mixin';
+import {DocResult} from '../../../types/common';
const basicFixture = fixtureFromElement('gr-documentation-search');
-let counter;
+let counter: number;
const documentationGenerator = () => {
return {
title: `Gerrit Code Review - REST API Developers Notes${++counter}`,
@@ -32,10 +35,10 @@
};
suite('gr-documentation-search tests', () => {
- let element;
- let documentationSearches;
+ let element: GrDocumentationSearch;
+ let documentationSearches: DocResult[];
- let value;
+ let value: ListViewParams;
setup(() => {
sinon.stub(page, 'show');
@@ -47,16 +50,23 @@
setup(done => {
documentationSearches = _.times(26, documentationGenerator);
stubRestApi('getDocumentationSearches').returns(
- Promise.resolve(documentationSearches));
- element._paramsChanged(value).then(() => { flush(done); });
+ Promise.resolve(documentationSearches)
+ );
+ element._paramsChanged(value).then(() => {
+ flush(done);
+ });
});
test('test for test repo in the list', done => {
flush(() => {
- assert.equal(element._documentationSearches[0].title,
- 'Gerrit Code Review - REST API Developers Notes1');
- assert.equal(element._documentationSearches[0].url,
- 'Documentation/dev-rest-api.html');
+ assert.equal(
+ element._documentationSearches![0].title,
+ 'Gerrit Code Review - REST API Developers Notes1'
+ );
+ assert.equal(
+ element._documentationSearches![0].url,
+ 'Documentation/dev-rest-api.html'
+ );
done();
});
});
@@ -65,12 +75,12 @@
suite('filter', () => {
setup(() => {
documentationSearches = _.times(25, documentationGenerator);
- _.times(1, documentationSearches);
});
test('_paramsChanged', async () => {
const stub = stubRestApi('getDocumentationSearches').returns(
- Promise.resolve(documentationSearches));
+ Promise.resolve(documentationSearches)
+ );
const value = {filter: 'test'};
await element._paramsChanged(value);
assert.isTrue(stub.lastCall.calledWithExactly('test'));
@@ -84,7 +94,6 @@
assert.equal(getComputedStyle(element.$.loading).display, 'block');
element._loading = false;
- element._repos = _.times(25, documentationGenerator);
flush();
assert.equal(element.computeLoadingClass(element._loading), '');
@@ -92,4 +101,3 @@
});
});
});
-
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index e61d0b8..4163f9a 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -180,21 +180,19 @@
}
}
- _closeDialog(dialog?: GrDialog, clearInputs = false) {
+ _closeDialog(dialog?: GrDialog) {
if (!dialog) return;
- if (clearInputs) {
- // Dialog may have autocompletes and plain inputs -- as these have
- // different properties representing their bound text, it is easier to
- // just make two separate queries.
- dialog.querySelectorAll('gr-autocomplete').forEach(input => {
- input.text = '';
- });
+ // Dialog may have autocompletes and plain inputs -- as these have
+ // different properties representing their bound text, it is easier to
+ // just make two separate queries.
+ dialog.querySelectorAll('gr-autocomplete').forEach(input => {
+ input.text = '';
+ });
- dialog.querySelectorAll('iron-input').forEach(input => {
- input.bindValue = '';
- });
- }
+ dialog.querySelectorAll('iron-input').forEach(input => {
+ input.bindValue = '';
+ });
dialog.classList.toggle('invisible', true);
return this.$.overlay.close();
@@ -211,12 +209,12 @@
this.patchNum
);
GerritNav.navigateToRelativeUrl(url);
- this._closeDialog(this._getDialogFromEvent(e), true);
+ this._closeDialog(this._getDialogFromEvent(e));
}
_handleUploadConfirm(path: string, fileData: string) {
if (!this.change || !path || !fileData) {
- this._closeDialog(this.$.openDialog, true);
+ this._closeDialog(this.$.openDialog);
return Promise.resolve();
}
return this.restApiService
@@ -225,7 +223,7 @@
if (!res || !res.ok) {
return;
}
- this._closeDialog(this.$.openDialog, true);
+ this._closeDialog(this.$.openDialog);
GerritNav.navigateToChange(this.change);
});
}
@@ -240,7 +238,7 @@
if (!res || !res.ok) {
return;
}
- this._closeDialog(dialog, true);
+ this._closeDialog(dialog);
GerritNav.navigateToChange(this.change);
});
}
@@ -253,7 +251,7 @@
if (!res || !res.ok) {
return;
}
- this._closeDialog(dialog, true);
+ this._closeDialog(dialog);
GerritNav.navigateToChange(this.change);
});
}
@@ -266,7 +264,7 @@
if (!res || !res.ok) {
return;
}
- this._closeDialog(dialog, true);
+ this._closeDialog(dialog);
GerritNav.navigateToChange(this.change);
});
}
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index d9f5cd7..a0b5392 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -117,7 +117,7 @@
assert.isFalse(editDiffStub.called);
assert.isFalse(navStub.called);
assert.isTrue(closeDialogSpy.called);
- assert.equal(element._path, 'src/test.cpp');
+ assert.equal(element._path, '');
});
});
});
@@ -202,7 +202,7 @@
);
assert.isFalse(navStub.called);
assert.isTrue(closeDialogSpy.called);
- assert.equal(element._path, 'src/test.cpp');
+ assert.equal(element._path, '');
});
});
});
@@ -296,8 +296,8 @@
);
assert.isFalse(navStub.called);
assert.isTrue(closeDialogSpy.called);
- assert.equal(element._path, 'src/test.cpp');
- assert.equal(element._newPath, 'src/test.newPath');
+ assert.equal(element._path, '');
+ assert.equal(element._newPath, '');
});
});
});
@@ -365,7 +365,7 @@
);
assert.isFalse(navStub.called);
assert.isTrue(closeDialogSpy.called);
- assert.equal(element._path, 'src/test.cpp');
+ assert.equal(element._path, '');
});
});
});
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.js b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.js
deleted file mode 100644
index 180a3a4..0000000
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.js
+++ /dev/null
@@ -1,92 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import '../gr-edit-constants.js';
-import './gr-edit-file-controls.js';
-import {GrEditConstants} from '../gr-edit-constants.js';
-
-const basicFixture = fixtureFromElement('gr-edit-file-controls');
-
-suite('gr-edit-file-controls tests', () => {
- let element;
-
- let fileActionHandler;
-
- setup(() => {
- element = basicFixture.instantiate();
- fileActionHandler = sinon.stub();
- element.addEventListener('file-action-tap', fileActionHandler);
- });
-
- test('open tap emits event', () => {
- const actions = element.$.actions;
- element.filePath = 'foo';
- actions._open();
- flush();
-
- MockInteractions.tap(actions.shadowRoot
- .querySelector('li [data-id="open"]'));
- assert.isTrue(fileActionHandler.called);
- assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
- {action: GrEditConstants.Actions.OPEN.id, path: 'foo'});
- });
-
- test('delete tap emits event', () => {
- const actions = element.$.actions;
- element.filePath = 'foo';
- actions._open();
- flush();
-
- MockInteractions.tap(actions.shadowRoot
- .querySelector('li [data-id="delete"]'));
- assert.isTrue(fileActionHandler.called);
- assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
- {action: GrEditConstants.Actions.DELETE.id, path: 'foo'});
- });
-
- test('restore tap emits event', () => {
- const actions = element.$.actions;
- element.filePath = 'foo';
- actions._open();
- flush();
-
- MockInteractions.tap(actions.shadowRoot
- .querySelector('li [data-id="restore"]'));
- assert.isTrue(fileActionHandler.called);
- assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
- {action: GrEditConstants.Actions.RESTORE.id, path: 'foo'});
- });
-
- test('rename tap emits event', () => {
- const actions = element.$.actions;
- element.filePath = 'foo';
- actions._open();
- flush();
-
- MockInteractions.tap(actions.shadowRoot
- .querySelector('li [data-id="rename"]'));
- assert.isTrue(fileActionHandler.called);
- assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
- {action: GrEditConstants.Actions.RENAME.id, path: 'foo'});
- });
-
- test('computed properties', () => {
- assert.equal(element._allFileActions.length, 4);
- });
-});
-
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.ts
new file mode 100644
index 0000000..a432ebf
--- /dev/null
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.ts
@@ -0,0 +1,102 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../../test/common-test-setup-karma';
+import './gr-edit-file-controls';
+import {GrEditFileControls} from './gr-edit-file-controls';
+import {GrEditConstants} from '../gr-edit-constants';
+import {queryAndAssert} from '../../../test/test-utils';
+import {GrDropdown} from '../../shared/gr-dropdown/gr-dropdown';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+
+const basicFixture = fixtureFromElement('gr-edit-file-controls');
+
+suite('gr-edit-file-controls tests', () => {
+ let element: GrEditFileControls;
+
+ let fileActionHandler: sinon.SinonStub;
+
+ setup(() => {
+ element = basicFixture.instantiate();
+ fileActionHandler = sinon.stub();
+ element.addEventListener('file-action-tap', fileActionHandler);
+ });
+
+ test('open tap emits event', () => {
+ const actions = queryAndAssert<GrDropdown>(element, '#actions');
+ element.filePath = 'foo';
+ actions._open();
+ flush();
+
+ const row = queryAndAssert(actions, 'li [data-id="open"]');
+ MockInteractions.tap(row);
+ assert.isTrue(fileActionHandler.called);
+ assert.deepEqual(fileActionHandler.lastCall.args[0].detail, {
+ action: GrEditConstants.Actions.OPEN.id,
+ path: 'foo',
+ });
+ });
+
+ test('delete tap emits event', () => {
+ const actions = queryAndAssert<GrDropdown>(element, '#actions');
+ element.filePath = 'foo';
+ actions._open();
+ flush();
+
+ const row = queryAndAssert(actions, 'li [data-id="delete"]');
+ MockInteractions.tap(row);
+ assert.isTrue(fileActionHandler.called);
+ assert.deepEqual(fileActionHandler.lastCall.args[0].detail, {
+ action: GrEditConstants.Actions.DELETE.id,
+ path: 'foo',
+ });
+ });
+
+ test('restore tap emits event', () => {
+ const actions = queryAndAssert<GrDropdown>(element, '#actions');
+ element.filePath = 'foo';
+ actions._open();
+ flush();
+
+ const row = queryAndAssert(actions, 'li [data-id="restore"]');
+ MockInteractions.tap(row);
+ assert.isTrue(fileActionHandler.called);
+ assert.deepEqual(fileActionHandler.lastCall.args[0].detail, {
+ action: GrEditConstants.Actions.RESTORE.id,
+ path: 'foo',
+ });
+ });
+
+ test('rename tap emits event', () => {
+ const actions = queryAndAssert<GrDropdown>(element, '#actions');
+ element.filePath = 'foo';
+ actions._open();
+ flush();
+
+ const row = queryAndAssert(actions, 'li [data-id="rename"]');
+ MockInteractions.tap(row);
+ assert.isTrue(fileActionHandler.called);
+ assert.deepEqual(fileActionHandler.lastCall.args[0].detail, {
+ action: GrEditConstants.Actions.RENAME.id,
+ path: 'foo',
+ });
+ });
+
+ test('computed properties', () => {
+ assert.equal(element._allFileActions.length, 4);
+ });
+});
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
index b0b7ab8..b724e72 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
@@ -168,7 +168,7 @@
_hideAgreements(
item: ContributorAgreementInfo,
- groups: GroupInfo[],
+ groups?: GroupInfo[],
signedAgreements?: ContributorAgreementInfo[]
) {
return this._disableAgreements(item, groups, signedAgreements)
@@ -176,18 +176,18 @@
: 'hide';
}
- _disableAgreementsText(text: string) {
- return text.toLowerCase() === 'i agree' ? false : true;
+ _disableAgreementsText(text?: string) {
+ return text?.toLowerCase() === 'i agree' ? false : true;
}
// This checks for auto_verify_group,
// if specified it returns 'hideAgreementsTextBox' which
// then hides the text box and submit button.
_computeHideAgreementClass(
- name: string,
+ name?: string,
contributorAgreements?: ContributorAgreementInfo[]
) {
- if (!contributorAgreements) return '';
+ if (!name || !contributorAgreements) return '';
return contributorAgreements.some(
(contributorAgreement: ContributorAgreementInfo) =>
name === contributorAgreement.name &&
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
index 887382e..4800e5b 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
@@ -104,12 +104,7 @@
bind-value="{{_agreementsText}}"
placeholder="Enter 'I agree' here"
>
- <input
- id="input-agreements"
- is="iron-input"
- bind-value="{{_agreementsText}}"
- placeholder="Enter 'I agree' here"
- />
+ <input id="input-agreements" placeholder="Enter 'I agree' here" />
</iron-input>
<gr-button
on-click="_handleSaveAgreements"
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 5e8cedd..2ad3be6 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
@@ -92,12 +92,6 @@
*/
/**
- * Fired when a comment in the thread is permanently modified.
- *
- * @event thread-changed
- */
-
- /**
* gr-comment-thread exposes the following attributes that allow a
* diff widget like gr-diff to show the thread in the right location:
*
@@ -718,7 +712,6 @@
if (this.comments.length === 0) {
this.fireRemoveSelf();
}
- this._handleCommentSavedOrDiscarded();
// Check to see if there are any other open comments getting edited and
// set the local storage value to its message value.
@@ -738,15 +731,6 @@
}
}
- _handleCommentSavedOrDiscarded() {
- this.dispatchEvent(
- new CustomEvent('thread-changed', {
- detail: {rootId: this.rootId, path: this.path},
- bubbles: false,
- })
- );
- }
-
_handleCommentUpdate(e: CustomEvent) {
const comment = e.detail.comment;
const index = this._indexOf(comment, this.comments);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index 247e8e4..5c01467 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -165,7 +165,6 @@
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
on-comment-discard="_handleCommentDiscard"
- on-comment-save="_handleCommentSavedOrDiscarded"
on-copy-comment-link="handleCopyLink"
></gr-comment>
</template>
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 31908fd..82a455e 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
@@ -495,21 +495,10 @@
const commentEl = element.shadowRoot?.querySelector('gr-comment');
assert.ok(commentEl);
- const saveOrDiscardStub = sinon.stub();
- element.addEventListener('thread-changed', saveOrDiscardStub);
element.shadowRoot?.querySelector('gr-comment')?._fireSave();
flush(() => {
- assert.isTrue(saveOrDiscardStub.called);
- assert.equal(
- saveOrDiscardStub.lastCall.args[0].detail.rootId,
- 'baf0414d_60047215'
- );
assert.equal(element.rootId, 'baf0414d_60047215' as UrlEncodedCommentId);
- assert.equal(
- saveOrDiscardStub.lastCall.args[0].detail.path,
- '/path/to/file.txt'
- );
done();
});
});
@@ -556,22 +545,11 @@
);
flush();
- const saveOrDiscardStub = sinon.stub();
- element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl = element.root?.querySelectorAll('gr-comment')[1];
assert.ok(draftEl);
draftEl!.addEventListener('comment-discard', () => {
const drafts = element.comments.filter(c => isDraft(c));
assert.equal(drafts.length, 0);
- assert.isTrue(saveOrDiscardStub.called);
- assert.equal(
- saveOrDiscardStub.lastCall.args[0].detail.rootId,
- element.rootId
- );
- assert.equal(
- saveOrDiscardStub.lastCall.args[0].detail.path,
- element.path
- );
done();
});
draftEl!.dispatchEvent(
@@ -593,18 +571,10 @@
const rootId = element.rootId;
assert.isOk(rootId);
- const saveOrDiscardStub = sinon.stub();
- element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl = element.root?.querySelectorAll('gr-comment')[0];
assert.ok(draftEl);
draftEl!.addEventListener('comment-discard', () => {
assert.equal(element.comments.length, 0);
- assert.isTrue(saveOrDiscardStub.called);
- assert.equal(saveOrDiscardStub.lastCall.args[0].detail.rootId, rootId);
- assert.equal(
- saveOrDiscardStub.lastCall.args[0].detail.path,
- element.path
- );
done();
});
draftEl!.dispatchEvent(
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 5dea745..1303e48 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -273,6 +273,8 @@
private readonly reporting = appContext.reportingService;
+ private readonly commentsService = appContext.commentsService;
+
private fireUpdateTask?: DelayedTask;
private storeTask?: DelayedTask;
@@ -486,6 +488,7 @@
if (this.comment?.__draftID) {
resComment.__draftID = this.comment.__draftID;
}
+ if (!resComment.patch_set) resComment.patch_set = this.patchNum;
this.comment = resComment;
this._fireSave();
return obj;
@@ -542,6 +545,7 @@
}
_fireSave() {
+ if (this.comment) this.commentsService.addDraft(this.comment);
this.dispatchEvent(
new CustomEvent('comment-save', {
detail: this._getEventPayload(),
@@ -743,6 +747,7 @@
}
_fireDiscard() {
+ if (this.comment) this.commentsService.deleteDraft(this.comment);
this.fireUpdateTask?.cancel();
this.dispatchEvent(
new CustomEvent('comment-discard', {
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 4f750dd..f74962a 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -26,6 +26,7 @@
import {GrStorageService} from './storage/gr-storage_impl';
import {ConfigService} from './config/config-service';
import {UserService} from './user/user-service';
+import {CommentsService} from './comments/comments-service';
type ServiceName = keyof AppContext;
type ServiceCreator<T> = () => T;
@@ -74,6 +75,7 @@
authService: () => new Auth(appContext.eventEmitter),
restApiService: () => new GrRestApiInterface(appContext.authService),
changeService: () => new ChangeService(),
+ commentsService: () => new CommentsService(appContext.restApiService),
checksService: () => new ChecksService(appContext.reportingService),
jsApiService: () => new GrJsApiInterface(),
storageService: () => new GrStorageService(),
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index de74f2a..161378d 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -25,6 +25,7 @@
import {StorageService} from './storage/gr-storage';
import {ConfigService} from './config/config-service';
import {UserService} from './user/user-service';
+import {CommentsService} from './comments/comments-service';
export interface AppContext {
flagsService: FlagsService;
@@ -33,6 +34,7 @@
authService: AuthService;
restApiService: RestApiService;
changeService: ChangeService;
+ commentsService: CommentsService;
checksService: ChecksService;
jsApiService: JsApiService;
storageService: StorageService;
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
new file mode 100644
index 0000000..e4a23a3
--- /dev/null
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -0,0 +1,135 @@
+/**
+ * @license
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {BehaviorSubject, Observable} from 'rxjs';
+import {distinctUntilChanged, map} from 'rxjs/operators';
+import {ChangeComments} from '../../elements/diff/gr-comment-api/gr-comment-api';
+import {
+ CommentInfo,
+ PathToCommentsInfoMap,
+ RobotCommentInfo,
+} from '../../types/common';
+import {addPath, DraftInfo} from '../../utils/comment-util';
+
+interface CommentState {
+ comments: PathToCommentsInfoMap;
+ robotComments: {[path: string]: RobotCommentInfo[]};
+ drafts: {[path: string]: DraftInfo[]};
+ portedComments: PathToCommentsInfoMap;
+ portedDrafts: PathToCommentsInfoMap;
+}
+
+const initialState: CommentState = {
+ comments: {},
+ robotComments: {},
+ drafts: {},
+ portedComments: {},
+ portedDrafts: {},
+};
+
+const privateState$ = new BehaviorSubject(initialState);
+
+// Re-exporting as Observable so that you can only subscribe, but not emit.
+export const commentState$: Observable<CommentState> = privateState$;
+
+export const drafts$ = commentState$.pipe(
+ map(commentState => commentState.drafts),
+ distinctUntilChanged()
+);
+
+// Emits a new value even if only a single draft is changed. Components should
+// aim to subsribe to something more specific.
+export const changeComments$ = commentState$.pipe(
+ map(
+ commentState =>
+ new ChangeComments(
+ commentState.comments,
+ commentState.robotComments,
+ commentState.drafts,
+ commentState.portedComments,
+ commentState.portedDrafts
+ )
+ ),
+ distinctUntilChanged()
+);
+
+export function updateStateComments(comments?: {
+ [path: string]: CommentInfo[];
+}) {
+ const nextState = {...privateState$.getValue()};
+ nextState.comments = addPath(comments) || {};
+ privateState$.next(nextState);
+}
+
+export function updateStateRobotComments(robotComments?: {
+ [path: string]: RobotCommentInfo[];
+}) {
+ const nextState = {...privateState$.getValue()};
+ nextState.robotComments = addPath(robotComments) || {};
+ privateState$.next(nextState);
+}
+
+export function updateStateDrafts(drafts?: {[path: string]: DraftInfo[]}) {
+ const nextState = {...privateState$.getValue()};
+ nextState.drafts = addPath(drafts) || {};
+ privateState$.next(nextState);
+}
+
+export function updateStatePortedComments(
+ portedComments?: PathToCommentsInfoMap
+) {
+ const nextState = {...privateState$.getValue()};
+ nextState.portedComments = portedComments || {};
+ privateState$.next(nextState);
+}
+
+export function updateStatePortedDrafts(portedDrafts?: PathToCommentsInfoMap) {
+ const nextState = {...privateState$.getValue()};
+ nextState.portedDrafts = portedDrafts || {};
+ privateState$.next(nextState);
+}
+
+export function updateStateAddDraft(draft: DraftInfo) {
+ const nextState = {...privateState$.getValue()};
+ if (!draft.path) throw new Error('draft path undefined');
+ nextState.drafts = {...nextState.drafts};
+ const drafts = nextState.drafts;
+ if (!drafts[draft.path]) drafts[draft.path] = [] as DraftInfo[];
+ else drafts[draft.path] = [...drafts[draft.path]];
+ const index = drafts[draft.path].findIndex(
+ d => d.__draftID === draft.__draftID || d.id === draft.id
+ );
+ if (index !== -1) {
+ drafts[draft.path][index] = draft;
+ } else {
+ drafts[draft.path].push(draft);
+ }
+ privateState$.next(nextState);
+}
+
+export function updateStateDeleteDraft(draft: DraftInfo) {
+ const nextState = {...privateState$.getValue()};
+ if (!draft.path) throw new Error('draft path undefined');
+ nextState.drafts = {...nextState.drafts};
+ const drafts = nextState.drafts;
+ const index = drafts[draft.path].findIndex(
+ d => d.__draftID === draft.__draftID || d.id === draft.id
+ );
+ if (index === -1) return;
+ drafts[draft.path] = [...drafts[draft.path]].splice(index, 1);
+ privateState$.next(nextState);
+}
diff --git a/polygerrit-ui/app/services/comments/comments-service.ts b/polygerrit-ui/app/services/comments/comments-service.ts
new file mode 100644
index 0000000..05dfc4c
--- /dev/null
+++ b/polygerrit-ui/app/services/comments/comments-service.ts
@@ -0,0 +1,68 @@
+/**
+ * @license
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {NumericChangeId, RevisionId} from '../../types/common';
+import {DraftInfo} from '../../utils/comment-util';
+import {CURRENT} from '../../utils/patch-set-util';
+import {RestApiService} from '../gr-rest-api/gr-rest-api';
+import {
+ updateStateAddDraft,
+ updateStateDeleteDraft,
+ updateStateComments,
+ updateStateRobotComments,
+ updateStateDrafts,
+ updateStatePortedComments,
+ updateStatePortedDrafts,
+} from './comments-model';
+
+export class CommentsService {
+ constructor(readonly restApiService: RestApiService) {}
+
+ /**
+ * Load all comments (with drafts and robot comments) for the given change
+ * number. The returned promise resolves when the comments have loaded, but
+ * does not yield the comment data.
+ */
+ // TODO(dhruvsri): listen to changeNum changes or reload event to update
+ // automatically
+ loadAll(changeNum: NumericChangeId, patchNum = CURRENT as RevisionId) {
+ const revision = patchNum;
+ this.restApiService
+ .getDiffComments(changeNum)
+ .then(comments => updateStateComments(comments));
+ this.restApiService
+ .getDiffRobotComments(changeNum)
+ .then(robotComments => updateStateRobotComments(robotComments));
+ this.restApiService
+ .getDiffDrafts(changeNum)
+ .then(drafts => updateStateDrafts(drafts));
+ this.restApiService
+ .getPortedComments(changeNum, revision)
+ .then(portedComments => updateStatePortedComments(portedComments));
+ this.restApiService
+ .getPortedDrafts(changeNum, revision)
+ .then(portedDrafts => updateStatePortedDrafts(portedDrafts));
+ }
+
+ addDraft(draft: DraftInfo) {
+ updateStateAddDraft(draft);
+ }
+
+ deleteDraft(draft: DraftInfo) {
+ updateStateDeleteDraft(draft);
+ }
+}
diff --git a/polygerrit-ui/app/services/comments/comments-service_test.ts b/polygerrit-ui/app/services/comments/comments-service_test.ts
new file mode 100644
index 0000000..604b5c4
--- /dev/null
+++ b/polygerrit-ui/app/services/comments/comments-service_test.ts
@@ -0,0 +1,126 @@
+/**
+ * @license
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../test/common-test-setup-karma';
+import {
+ createComment,
+ createFixSuggestionInfo,
+} from '../../test/test-data-generators';
+import {stubRestApi} from '../../test/test-utils';
+import {
+ NumericChangeId,
+ RobotId,
+ RobotRunId,
+ Timestamp,
+ UrlEncodedCommentId,
+} from '../../types/common';
+import {appContext} from '../app-context';
+import {CommentsService} from './comments-service';
+
+suite('change service tests', () => {
+ let commentsService: CommentsService;
+
+ test('loads logged-out', () => {
+ const changeNum = 1234 as NumericChangeId;
+ commentsService = new CommentsService(appContext.restApiService);
+ stubRestApi('getLoggedIn').returns(Promise.resolve(false));
+ const diffCommentsSpy = stubRestApi('getDiffComments').returns(
+ Promise.resolve({
+ 'foo.c': [
+ {
+ ...createComment(),
+ id: '123' as UrlEncodedCommentId,
+ message: 'Done',
+ updated: '2017-02-08 16:40:49' as Timestamp,
+ },
+ ],
+ })
+ );
+ const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
+ Promise.resolve({
+ 'foo.c': [
+ {
+ ...createComment(),
+ id: '321' as UrlEncodedCommentId,
+ message: 'Done',
+ updated: '2017-02-08 16:40:49' as Timestamp,
+ robot_id: 'robot_1' as RobotId,
+ robot_run_id: 'run_1' as RobotRunId,
+ properties: {},
+ fix_suggestions: [
+ createFixSuggestionInfo('fix_1'),
+ createFixSuggestionInfo('fix_2'),
+ ],
+ },
+ ],
+ })
+ );
+ const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
+ Promise.resolve({})
+ );
+
+ commentsService.loadAll(changeNum);
+ assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
+ assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
+ assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
+ });
+
+ test('loads logged-in', () => {
+ const changeNum = 1234 as NumericChangeId;
+
+ stubRestApi('getLoggedIn').returns(Promise.resolve(true));
+ const diffCommentsSpy = stubRestApi('getDiffComments').returns(
+ Promise.resolve({
+ 'foo.c': [
+ {
+ ...createComment(),
+ id: '123' as UrlEncodedCommentId,
+ message: 'Done',
+ updated: '2017-02-08 16:40:49' as Timestamp,
+ },
+ ],
+ })
+ );
+ const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
+ Promise.resolve({
+ 'foo.c': [
+ {
+ ...createComment(),
+ id: '321' as UrlEncodedCommentId,
+ message: 'Done',
+ updated: '2017-02-08 16:40:49' as Timestamp,
+ robot_id: 'robot_1' as RobotId,
+ robot_run_id: 'run_1' as RobotRunId,
+ properties: {},
+ fix_suggestions: [
+ createFixSuggestionInfo('fix_1'),
+ createFixSuggestionInfo('fix_2'),
+ ],
+ },
+ ],
+ })
+ );
+ const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
+ Promise.resolve({})
+ );
+
+ commentsService.loadAll(changeNum);
+ assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
+ assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
+ assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
+ });
+});
diff --git a/polygerrit-ui/app/test/mocks/comment-api.js b/polygerrit-ui/app/test/mocks/comment-api.js
index 526aabe..fc4599d 100644
--- a/polygerrit-ui/app/test/mocks/comment-api.js
+++ b/polygerrit-ui/app/test/mocks/comment-api.js
@@ -28,27 +28,6 @@
_changeComments: Object,
};
}
-
- loadComments() {
- return this._reloadComments();
- }
-
- /**
- * For the purposes of the mock, _reloadDrafts is not included because its
- * response is the same type as reloadComments, just makes less API
- * requests. Since this is for test purposes/mocked data anyway, keep this
- * file simpler by just using _reloadComments here instead.
- */
- _reloadDraftsWithCallback(e) {
- return this._reloadComments().then(() => e.detail.resolve());
- }
-
- _reloadComments() {
- return this.$.commentAPI.loadAll(this._changeNum)
- .then(comments => {
- this._changeComments = this.$.commentAPI._changeComments;
- });
- }
}
/**
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index cc163c8..6e37697 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import {EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {PatchSetNum, UrlEncodedCommentId} from './common';
+import {PatchSetNum} from './common';
import {UIComment} from '../utils/comment-util';
import {FetchRequest} from './types';
import {LineNumberEventDetail, MovedLinkClickedEventDetail} from '../api/diff';
@@ -23,9 +23,11 @@
import {ChangeMessage} from '../elements/change/gr-message/gr-message';
export enum EventType {
+ CHANGE = 'change',
CHANGED = 'changed',
CHANGE_MESSAGE_DELETED = 'change-message-deleted',
DIALOG_CHANGE = 'dialog-change',
+ DROP = 'drop',
EDITABLE_CONTENT_SAVE = 'editable-content-save',
GR_RPC_LOG = 'gr-rpc-log',
LOCATION_CHANGE = 'location-change',
@@ -45,19 +47,25 @@
SHOW_ERROR = 'show-error',
SHOW_PRIMARY_TAB = 'show-primary-tab',
SHOW_SECONDARY_TAB = 'show-secondary-tab',
- THREAD_LIST_MODIFIED = 'thread-list-modified',
+ TAP_ITEM = 'tap-item',
TITLE_CHANGE = 'title-change',
}
declare global {
interface HTMLElementEventMap {
/* prettier-ignore */
+ 'change': ChangeEvent;
+ /* prettier-ignore */
'changed': ChangedEvent;
'change-message-deleted': ChangeMessageDeletedEvent;
'dialog-change': DialogChangeEvent;
+ /* prettier-ignore */
+ 'drop': DropEvent;
'editable-content-save': EditableContentSaveEvent;
'location-change': LocationChangeEvent;
'iron-announce': IronAnnounceEvent;
+ /* prettier-ignore */
+ 'keypress': KeypressEvent;
'line-mouse-enter': LineNumberEvent;
'line-mouse-leave': LineNumberEvent;
'line-cursor-moved-in': LineNumberEvent;
@@ -74,7 +82,7 @@
'show-error': ShowErrorEvent;
'show-primary-tab': SwitchTabEvent;
'show-secondary-tab': SwitchTabEvent;
- 'thread-list-modified': ThreadListModifiedEvent;
+ 'tap-item': TapItemEvent;
'title-change': TitleChangeEvent;
}
}
@@ -92,6 +100,8 @@
}
}
+export type ChangeEvent = InputEvent;
+
export type ChangedEvent = CustomEvent<string>;
export interface ChangeMessageDeletedEventDetail {
@@ -107,6 +117,8 @@
}
export type DialogChangeEvent = CustomEvent<DialogChangeEventDetail>;
+export type DropEvent = DragEvent;
+
export interface EditableContentSaveEventDetail {
content: string;
}
@@ -125,6 +137,8 @@
}
export type IronAnnounceEvent = CustomEvent<IronAnnounceEventDetail>;
+export type KeypressEvent = InputEvent;
+
export interface LocationChangeEventDetail {
hash: string;
pathname: string;
@@ -217,11 +231,7 @@
}
export type SwitchTabEvent = CustomEvent<SwitchTabEventDetail>;
-export interface ThreadListModifiedDetail {
- rootId: UrlEncodedCommentId;
- path: string;
-}
-export type ThreadListModifiedEvent = CustomEvent<ThreadListModifiedDetail>;
+export type TapItemEvent = CustomEvent;
export interface TitleChangeEventDetail {
title: string;
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 83a9174..3e85b48 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -361,3 +361,25 @@
if (isDraft(comment)) return comment.__draftID;
throw new Error('Missing id in root comment.');
}
+
+/**
+ * Add path info to every comment as CommentInfo returned
+ * from server does not have that.
+ *
+ * TODO(taoalpha): should consider changing BE to send path
+ * back within CommentInfo
+ */
+export function addPath<T>(
+ comments: {[path: string]: T[]} = {}
+): {[path: string]: Array<T & {path: string}>} {
+ const updatedComments: {[path: string]: Array<T & {path: string}>} = {};
+ for (const filePath of Object.keys(comments)) {
+ const allCommentsForPath = comments[filePath] || [];
+ if (allCommentsForPath.length) {
+ updatedComments[filePath] = allCommentsForPath.map(comment => {
+ return {...comment, path: filePath};
+ });
+ }
+ }
+ return updatedComments;
+}
diff --git a/polygerrit-ui/app/utils/event-util.ts b/polygerrit-ui/app/utils/event-util.ts
index 4ba27bc..c356893 100644
--- a/polygerrit-ui/app/utils/event-util.ts
+++ b/polygerrit-ui/app/utils/event-util.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
-import {UrlEncodedCommentId} from '../types/common';
import {FetchRequest} from '../types/types';
import {
DialogChangeEventDetail,
@@ -108,17 +107,6 @@
fire(target, EventType.IRON_ANNOUNCE, {text});
}
-export function fireThreadListModifiedEvent(
- target: EventTarget,
- rootId: UrlEncodedCommentId,
- path: string
-) {
- fire(target, EventType.THREAD_LIST_MODIFIED, {
- rootId,
- path,
- });
-}
-
export function fireShowPrimaryTab(
target: EventTarget,
tab: string,
