Merge "Migrate comments-model to new pattern"
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index e8c5916..02d1beb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -59,10 +59,6 @@
import {modifierPressed} from '../../../utils/dom-util';
import {DropdownLink} from '../../shared/gr-dropdown/gr-dropdown';
import {fontStyles} from '../../../styles/gr-font-styles';
-import {
- changeComments$,
- threads$,
-} from '../../../services/comments/comments-model';
export enum SummaryChipStyles {
INFO = 'info',
@@ -405,6 +401,8 @@
private showAllChips = new Map<RunStatus | Category, boolean>();
+ private commentsModel = getAppContext().commentsModel;
+
private userModel = getAppContext().userModel;
private checksModel = getAppContext().checksModel;
@@ -441,8 +439,16 @@
this.checksModel.topLevelActionsLatest$,
x => (this.actions = x)
);
- subscribe(this, changeComments$, x => (this.changeComments = x));
- subscribe(this, threads$, x => (this.commentThreads = x));
+ subscribe(
+ this,
+ this.commentsModel.changeComments$,
+ x => (this.changeComments = x)
+ );
+ subscribe(
+ this,
+ this.commentsModel.threads$,
+ x => (this.commentThreads = x)
+ );
subscribe(this, this.userModel.account$, x => (this.selfAccount = x));
}
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 6ef1e4e..a0b6bfd 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
@@ -191,10 +191,6 @@
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
import {
- changeComments$,
- drafts$,
-} from '../../../services/comments/comments-model';
-import {
getAddedByReason,
getRemovedByReason,
hasAttention,
@@ -565,7 +561,7 @@
// Private but used in tests.
readonly userModel = getAppContext().userModel;
- private readonly commentsService = getAppContext().commentsService;
+ private readonly commentsModel = getAppContext().commentsModel;
private readonly shortcuts = getAppContext().shortcutsService;
@@ -644,7 +640,7 @@
})
);
this.subscriptions.push(
- drafts$.subscribe(drafts => {
+ this.commentsModel.drafts$.subscribe(drafts => {
this._diffDrafts = {...drafts};
})
);
@@ -654,7 +650,7 @@
})
);
this.subscriptions.push(
- changeComments$.subscribe(changeComments => {
+ this.commentsModel.changeComments$.subscribe(changeComments => {
this._changeComments = changeComments;
})
);
@@ -2214,13 +2210,13 @@
const promises = [this._getCommitInfo(), this.$.fileList.reload()];
if (patchNumChanged) {
promises.push(
- this.commentsService.reloadPortedComments(
+ this.commentsModel.reloadPortedComments(
this._changeNum,
this._patchRange?.patchNum
)
);
promises.push(
- this.commentsService.reloadPortedDrafts(
+ this.commentsModel.reloadPortedDrafts(
this._changeNum,
this._patchRange?.patchNum
)
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
index 1778b2b..6fa11be 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
@@ -30,7 +30,7 @@
import {fontStyles} from '../../../styles/gr-font-styles';
import {subscribe} from '../../lit/subscription-controller';
import {change$} from '../../../services/change/change-model';
-import {threads$} from '../../../services/comments/comments-model';
+import {getAppContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
@customElement('gr-confirm-submit-dialog')
@@ -62,6 +62,8 @@
@state()
initialised = false;
+ private commentsModel = getAppContext().commentsModel;
+
static override get styles() {
return [
sharedStyles,
@@ -93,7 +95,7 @@
subscribe(this, change$, x => (this.change = x));
subscribe(
this,
- threads$,
+ this.commentsModel.threads$,
x => (this.unresolvedThreads = x.filter(isUnresolved))
);
}
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 f286098..112077a 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
@@ -83,7 +83,6 @@
import {ParsedChangeInfo, PatchSetFile} from '../../../types/types';
import {Timing} from '../../../constants/reporting';
import {RevisionInfo} from '../../shared/revision-info/revision-info';
-import {changeComments$} from '../../../services/comments/comments-model';
import {listen} from '../../../services/shortcuts/shortcuts-service';
import {select} from '../../../utils/observable-util';
@@ -316,6 +315,8 @@
private readonly userModel = getAppContext().userModel;
+ private readonly commentsModel = getAppContext().commentsModel;
+
private readonly browserModel = getAppContext().browserModel;
private subscriptions: Subscription[] = [];
@@ -375,7 +376,7 @@
override connectedCallback() {
super.connectedCallback();
this.subscriptions = [
- changeComments$.subscribe(changeComments => {
+ this.commentsModel.changeComments$.subscribe(changeComments => {
this.changeComments = changeComments;
}),
this.browserModel.diffViewMode$.subscribe(
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 38875f5..fd6b0d4 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
@@ -1473,8 +1473,7 @@
}
async function renderAndGetNewDiffs(index) {
- const diffs =
- element.root.querySelectorAll('gr-diff-host');
+ const diffs = element.root.querySelectorAll('gr-diff-host');
for (let i = index; i < diffs.length; i++) {
await setupDiff(diffs[i]);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index c3ad0b6..baaecc8 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -51,7 +51,6 @@
FormattedReviewerUpdateInfo,
ParsedChangeInfo,
} from '../../../types/types';
-import {threads$} from '../../../services/comments/comments-model';
import {
change$,
changeNum$,
@@ -257,6 +256,9 @@
private readonly userModel = getAppContext().userModel;
+ // Private but used in tests.
+ readonly commentsModel = getAppContext().commentsModel;
+
private readonly reporting = getAppContext().reportingService;
private readonly shortcuts = getAppContext().shortcutsService;
@@ -266,7 +268,7 @@
override connectedCallback() {
super.connectedCallback();
this.subscriptions.push(
- threads$.subscribe(x => {
+ this.commentsModel.threads$.subscribe(x => {
this.commentThreads = x;
})
);
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 50e1a38..65c3c74 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
@@ -22,7 +22,6 @@
import {MessageTag} from '../../../constants/constants.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {stubRestApi} from '../../../test/test-utils.js';
-import {updateStateComments} from '../../../services/comments/comments-model.js';
createCommentApiMockWithTemplateElement(
'gr-messages-list-comment-mock-api', html`
@@ -129,7 +128,7 @@
};
suite('basic tests', () => {
- setup(() => {
+ setup(async () => {
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
stubRestApi('getDiffComments').returns(Promise.resolve(comments));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
@@ -140,9 +139,9 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.messagesList;
- updateStateComments(comments);
+ await element.commentsModel.reloadComments();
element.messages = messages;
- flush();
+ await flush();
});
test('expand/collapse all', () => {
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 9c62d1a..25a1a00 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
@@ -85,7 +85,6 @@
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 {ChangeComments} from '../gr-comment-api/gr-comment-api';
import {Subscription} from 'rxjs';
import {DisplayLine, RenderPreferences} from '../../../api/diff';
@@ -268,6 +267,8 @@
private readonly browserModel = getAppContext().browserModel;
+ private readonly commentsModel = getAppContext().commentsModel;
+
private readonly reporting = getAppContext().reportingService;
private readonly flags = getAppContext().flagsService;
@@ -320,7 +321,7 @@
this._loggedIn = loggedIn;
});
this.subscriptions.push(
- changeComments$.subscribe(changeComments => {
+ this.commentsModel.changeComments$.subscribe(changeComments => {
this.changeComments = changeComments;
})
);
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 50d6b24..98e4312 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
@@ -113,10 +113,6 @@
import {addGlobalShortcut, Key, toggleClass} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
import {isFalse, throttleWrap, until} from '../../../utils/async-util';
-import {
- changeComments$,
- commentsLoading$,
-} from '../../../services/comments/comments-model';
import {filter, take} from 'rxjs/operators';
import {Subscription, combineLatest} from 'rxjs';
import {listen} from '../../../services/shortcuts/shortcuts-service';
@@ -373,9 +369,8 @@
// Private but used in tests.
readonly browserModel = getAppContext().browserModel;
- // We just want to make sure that CommentsService is instantiated.
- // Otherwise subscribing to the model won't emit any data.
- private readonly _commentsService = getAppContext().commentsService;
+ // Private but used in tests.
+ readonly commentsModel = getAppContext().commentsModel;
private readonly shortcuts = getAppContext().shortcutsService;
@@ -387,11 +382,6 @@
private subscriptions: Subscription[] = [];
- constructor() {
- super();
- this._commentsService;
- }
-
override connectedCallback() {
super.connectedCallback();
this._throttledToggleFileReviewed = throttleWrap(_ =>
@@ -405,7 +395,7 @@
});
this.subscriptions.push(
- changeComments$.subscribe(changeComments => {
+ this.commentsModel.changeComments$.subscribe(changeComments => {
this._changeComments = changeComments;
})
);
@@ -1173,7 +1163,7 @@
promises.push(
until(changeLoadingStatus$, status => status === LoadingStatus.LOADED)
);
- promises.push(until(commentsLoading$, isFalse));
+ promises.push(until(this.commentsModel.commentsLoading$, isFalse));
promises.push(
this._getChangeEdit().then(edit => {
if (edit) {
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 d47a03e..3fad375 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
@@ -32,7 +32,6 @@
import {CursorMoveResult} from '../../../api/core.js';
import {Side} from '../../../api/diff.js';
import {_testOnly_setState as setChangeModelState} from '../../../services/change/change-model.js';
-import {_testOnly_setState as setCommentState} from '../../../services/comments/comments-model.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -93,7 +92,7 @@
]});
await flush();
- setCommentState({
+ element.commentsModel.setState({
comments: {},
robotComments: {},
drafts: {},
@@ -146,7 +145,7 @@
});
test('comment url resolves to comment.patch_set vs latest', () => {
- setCommentState({
+ element.commentsModel.setState({
comments: {
'/COMMIT_MSG': [
{
@@ -220,7 +219,7 @@
test('unchanged diff X vs latest from comment links navigates to base vs X'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- setCommentState({
+ element.commentsModel.setState({
comments: {
'/COMMIT_MSG': [
{
@@ -272,7 +271,7 @@
test('unchanged diff Base vs latest from comment does not navigate'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- setCommentState({
+ element.commentsModel.setState({
comments: {
'/COMMIT_MSG': [
{
@@ -352,7 +351,7 @@
});
test('diff toast to go to latest is shown and not base', async () => {
- setCommentState({
+ element.commentsModel.setState({
comments: {
'/COMMIT_MSG': [
{
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 3a2def0..71fee62 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -51,7 +51,6 @@
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
import {subscribe} from '../../lit/subscription-controller';
-import {changeComments$} from '../../../services/comments/comments-model';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -127,9 +126,15 @@
private readonly reporting: ReportingService =
getAppContext().reportingService;
+ private readonly commentsModel = getAppContext().commentsModel;
+
constructor() {
super();
- subscribe(this, changeComments$, x => (this.changeComments = x));
+ subscribe(
+ this,
+ this.commentsModel.changeComments$,
+ x => (this.changeComments = x)
+ );
}
static override get styles() {
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 1e99b7f..debd9c0 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
@@ -236,7 +236,7 @@
@state()
saving = false;
- private readonly commentsService = getAppContext().commentsService;
+ private readonly commentsModel = getAppContext().commentsModel;
private readonly userModel = getAppContext().userModel;
@@ -769,7 +769,7 @@
} else {
try {
this.saving = true;
- await this.commentsService.saveDraft(unsaved);
+ await this.commentsModel.saveDraft(unsaved);
} finally {
this.saving = false;
}
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 85a3623..af2966c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -228,7 +228,7 @@
private readonly reporting = getAppContext().reportingService;
- private readonly commentsService = getAppContext().commentsService;
+ private readonly commentsModel = getAppContext().commentsModel;
private readonly userModel = getAppContext().userModel;
@@ -1085,7 +1085,7 @@
if (messageToSave === '') {
// Don't try to discard UnsavedInfo. Nothing to do then.
if (this.comment.id) {
- await this.commentsService.discardDraft(this.comment.id);
+ await this.commentsModel.discardDraft(this.comment.id);
}
} else {
// No need to make a backend call when nothing has changed.
@@ -1108,7 +1108,7 @@
/** For sharing between save() and autoSave(). */
private rawSave(message: string, options: {showToast: boolean}) {
if (!isDraftOrUnsaved(this.comment)) throw new Error('not a draft');
- return this.commentsService.saveDraft(
+ return this.commentsModel.saveDraft(
{
...this.comment,
message,
diff --git a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
index 44db793..b865a7c 100644
--- a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
+++ b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
@@ -78,8 +78,8 @@
changeService: (_ctx: Partial<AppContext>) => {
throw new Error('changeService is not implemented');
},
- commentsService: (_ctx: Partial<AppContext>) => {
- throw new Error('commentsService is not implemented');
+ commentsModel: (_ctx: Partial<AppContext>) => {
+ throw new Error('commentsModel is not implemented');
},
checksModel: (_ctx: Partial<AppContext>) => {
throw new Error('checksModel is not implemented');
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index c43ed4e..f958429 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -26,7 +26,7 @@
import {GrJsApiInterface} from '../elements/shared/gr-js-api-interface/gr-js-api-interface-element';
import {GrStorageService} from './storage/gr-storage_impl';
import {UserModel} from './user/user-model';
-import {CommentsService} from './comments/comments-service';
+import {CommentsModel} from './comments/comments-model';
import {ShortcutsService} from './shortcuts/shortcuts-service';
import {BrowserModel} from './browser/browser-model';
import {assertIsDefined} from '../utils/common-util';
@@ -57,12 +57,12 @@
assertIsDefined(ctx.restApiService, 'restApiService');
return new ChangeService(ctx.restApiService!);
},
- commentsService: (ctx: Partial<AppContext>) => {
+ commentsModel: (ctx: Partial<AppContext>) => {
const restApi = ctx.restApiService;
const reporting = ctx.reportingService;
assertIsDefined(restApi, 'restApiService');
assertIsDefined(reporting, 'reportingService');
- return new CommentsService(restApi, reporting);
+ return new CommentsModel(restApi, reporting);
},
checksModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.reportingService, 'reportingService');
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index 4488b12..3439a3d 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -25,7 +25,7 @@
import {JsApiService} from '../elements/shared/gr-js-api-interface/gr-js-api-types';
import {StorageService} from './storage/gr-storage';
import {UserModel} from './user/user-model';
-import {CommentsService} from './comments/comments-service';
+import {CommentsModel} from './comments/comments-model';
import {ShortcutsService} from './shortcuts/shortcuts-service';
import {BrowserModel} from './browser/browser-model';
import {ConfigModel} from './config/config-model';
@@ -37,7 +37,7 @@
authService: AuthService;
restApiService: RestApiService;
changeService: ChangeService;
- commentsService: CommentsService;
+ commentsModel: CommentsModel;
checksModel: ChecksModel;
jsApiService: JsApiService;
storageService: StorageService;
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
index 62aecbc..ebc6655 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -15,19 +15,42 @@
* limitations under the License.
*/
-import {BehaviorSubject, Observable} from 'rxjs';
+import {BehaviorSubject} from 'rxjs';
import {ChangeComments} from '../../elements/diff/gr-comment-api/gr-comment-api';
import {
+ CommentBasics,
CommentInfo,
+ NumericChangeId,
+ PatchSetNum,
+ RevisionId,
+ UrlEncodedCommentId,
PathToCommentsInfoMap,
RobotCommentInfo,
- UrlEncodedCommentId,
} from '../../types/common';
-import {addPath, DraftInfo, isDraft, isUnsaved} from '../../utils/comment-util';
+import {
+ addPath,
+ DraftInfo,
+ isDraft,
+ isUnsaved,
+ reportingDetails,
+ UnsavedInfo,
+} from '../../utils/comment-util';
import {deepEqual} from '../../utils/deep-util';
import {select} from '../../utils/observable-util';
+import {routerChangeNum$} from '../router/router-model';
+import {Finalizable} from '../registry';
+import {combineLatest, Subscription} from 'rxjs';
+import {fire, fireAlert, fireEvent} from '../../utils/event-util';
+import {CURRENT} from '../../utils/patch-set-util';
+import {RestApiService} from '../gr-rest-api/gr-rest-api';
+import {changeNum$, currentPatchNum$} from '../change/change-model';
+import {Interaction} from '../../constants/reporting';
+import {assertIsDefined} from '../../utils/common-util';
+import {debounce, DelayedTask} from '../../utils/async-util';
+import {pluralize} from '../../utils/string-util';
+import {ReportingService} from '../gr-reporting/gr-reporting';
-interface CommentState {
+export interface CommentState {
/** undefined means 'still loading' */
comments?: PathToCommentsInfoMap;
/** undefined means 'still loading' */
@@ -60,134 +83,93 @@
discardedDrafts: [],
};
-const privateState$ = new BehaviorSubject(initialState);
+const TOAST_DEBOUNCE_INTERVAL = 200;
-export function _testOnly_resetState() {
- // We cannot assign a new subject to privateState$, because all the selectors
- // have already subscribed to the original subject. So we have to emit the
- // initial state on the existing subject.
- privateState$.next({...initialState});
+function getSavingMessage(numPending: number, requestFailed?: boolean) {
+ if (requestFailed) {
+ return 'Unable to save draft';
+ }
+ if (numPending === 0) {
+ return 'All changes saved';
+ }
+ return `Saving ${pluralize(numPending, 'draft')}...`;
}
-// Re-exporting as Observable so that you can only subscribe, but not emit.
-export const commentState$: Observable<CommentState> = privateState$;
-
-export function _testOnly_getState() {
- return privateState$.getValue();
-}
-
-export function _testOnly_setState(state: CommentState) {
- privateState$.next(state);
-}
-
-export const commentsLoading$ = select(
- commentState$,
- commentState =>
- commentState.comments === undefined ||
- commentState.robotComments === undefined ||
- commentState.drafts === undefined
-);
-
-export const comments$ = select(
- commentState$,
- commentState => commentState.comments
-);
-
-export const drafts$ = select(
- commentState$,
- commentState => commentState.drafts
-);
-
-export const portedComments$ = select(
- commentState$,
- commentState => commentState.portedComments
-);
-
-export const discardedDrafts$ = select(
- commentState$,
- commentState => commentState.discardedDrafts
-);
-
-// Emits a new value even if only a single draft is changed. Components should
-// aim to subsribe to something more specific.
-export const changeComments$ = select(
- commentState$,
- commentState =>
- new ChangeComments(
- commentState.comments,
- commentState.robotComments,
- commentState.drafts,
- commentState.portedComments,
- commentState.portedDrafts
- )
-);
-
-export const threads$ = select(changeComments$, changeComments =>
- changeComments.getAllThreadsForChange()
-);
-
-export function thread$(id: UrlEncodedCommentId) {
- return select(threads$, threads => threads.find(t => t.rootId === id));
-}
-
-function publishState(state: CommentState) {
- privateState$.next(state);
-}
-
-/** Called when the change number changes. Wipes out all data from the state. */
-export function updateStateReset() {
- publishState({...initialState});
-}
-
-export function updateStateComments(comments?: {
- [path: string]: CommentInfo[];
-}) {
- const nextState = {...privateState$.getValue()};
- if (deepEqual(comments, nextState.comments)) return;
+// Private but used in tests.
+export function setComments(
+ state: CommentState,
+ comments?: {
+ [path: string]: CommentInfo[];
+ }
+): CommentState {
+ const nextState = {...state};
+ if (deepEqual(comments, nextState.comments)) return state;
nextState.comments = addPath(comments) || {};
- publishState(nextState);
+ return nextState;
}
-export function updateStateRobotComments(robotComments?: {
- [path: string]: RobotCommentInfo[];
-}) {
- const nextState = {...privateState$.getValue()};
- if (deepEqual(robotComments, nextState.robotComments)) return;
+// Private but used in tests.
+export function setRobotComments(
+ state: CommentState,
+ robotComments?: {
+ [path: string]: RobotCommentInfo[];
+ }
+): CommentState {
+ if (deepEqual(robotComments, state.robotComments)) return state;
+ const nextState = {...state};
nextState.robotComments = addPath(robotComments) || {};
- publishState(nextState);
+ return nextState;
}
-export function updateStateDrafts(drafts?: {[path: string]: DraftInfo[]}) {
- const nextState = {...privateState$.getValue()};
- if (deepEqual(drafts, nextState.drafts)) return;
+// Private but used in tests.
+export function setDrafts(
+ state: CommentState,
+ drafts?: {[path: string]: DraftInfo[]}
+): CommentState {
+ if (deepEqual(drafts, state.drafts)) return state;
+ const nextState = {...state};
nextState.drafts = addPath(drafts);
- publishState(nextState);
+ return nextState;
}
-export function updateStatePortedComments(
+// Private but used in tests.
+export function setPortedComments(
+ state: CommentState,
portedComments?: PathToCommentsInfoMap
-) {
- const nextState = {...privateState$.getValue()};
- if (deepEqual(portedComments, nextState.portedComments)) return;
+): CommentState {
+ if (deepEqual(portedComments, state.portedComments)) return state;
+ const nextState = {...state};
nextState.portedComments = portedComments || {};
- publishState(nextState);
+ return nextState;
}
-export function updateStatePortedDrafts(portedDrafts?: PathToCommentsInfoMap) {
- const nextState = {...privateState$.getValue()};
- if (deepEqual(portedDrafts, nextState.portedDrafts)) return;
+// Private but used in tests.
+export function setPortedDrafts(
+ state: CommentState,
+ portedDrafts?: PathToCommentsInfoMap
+): CommentState {
+ if (deepEqual(portedDrafts, state.portedDrafts)) return state;
+ const nextState = {...state};
nextState.portedDrafts = portedDrafts || {};
- publishState(nextState);
+ return nextState;
}
-export function updateStateSetDiscardedDraft(draft: DraftInfo) {
- const nextState = {...privateState$.getValue()};
+// Private but used in tests.
+export function setDiscardedDraft(
+ state: CommentState,
+ draft: DraftInfo
+): CommentState {
+ const nextState = {...state};
nextState.discardedDrafts = [...nextState.discardedDrafts, draft];
- publishState(nextState);
+ return nextState;
}
-export function updateStateDeleteDiscardedDraft(draftID?: string) {
- const nextState = {...privateState$.getValue()};
+// Private but used in tests.
+export function deleteDiscardedDraft(
+ state: CommentState,
+ draftID?: string
+): CommentState {
+ const nextState = {...state};
const drafts = [...nextState.discardedDrafts];
const index = drafts.findIndex(d => d.id === draftID);
if (index === -1) {
@@ -195,12 +177,12 @@
}
drafts.splice(index, 1);
nextState.discardedDrafts = drafts;
- publishState(nextState);
+ return nextState;
}
/** Adds or updates a draft. */
-export function updateStateSetDraft(draft: DraftInfo) {
- const nextState = {...privateState$.getValue()};
+export function setDraft(state: CommentState, draft: DraftInfo): CommentState {
+ const nextState = {...state};
if (!draft.path) throw new Error('draft path undefined');
if (!isDraft(draft)) throw new Error('draft is not a draft');
if (isUnsaved(draft)) throw new Error('unsaved drafts dont belong to model');
@@ -215,11 +197,14 @@
} else {
drafts[draft.path].push(draft);
}
- publishState(nextState);
+ return nextState;
}
-export function updateStateDeleteDraft(draft: DraftInfo) {
- const nextState = {...privateState$.getValue()};
+export function deleteDraft(
+ state: CommentState,
+ draft: DraftInfo
+): CommentState {
+ const nextState = {...state};
if (!draft.path) throw new Error('draft path undefined');
if (!isDraft(draft)) throw new Error('draft is not a draft');
if (isUnsaved(draft)) throw new Error('unsaved drafts dont belong to model');
@@ -228,10 +213,331 @@
const index = (drafts[draft.path] || []).findIndex(
d => d.id && d.id === draft.id
);
- if (index === -1) return;
+ if (index === -1) return state;
const discardedDraft = drafts[draft.path][index];
drafts[draft.path] = [...drafts[draft.path]];
drafts[draft.path].splice(index, 1);
- publishState(nextState);
- updateStateSetDiscardedDraft(discardedDraft);
+ return setDiscardedDraft(nextState, discardedDraft);
+}
+
+export class CommentsModel implements Finalizable {
+ private readonly privateState$: BehaviorSubject<CommentState> =
+ new BehaviorSubject(initialState);
+
+ public readonly commentsLoading$ = select(
+ this.privateState$,
+ commentState =>
+ commentState.comments === undefined ||
+ commentState.robotComments === undefined ||
+ commentState.drafts === undefined
+ );
+
+ public readonly comments$ = select(
+ this.privateState$,
+ commentState => commentState.comments
+ );
+
+ public readonly drafts$ = select(
+ this.privateState$,
+ commentState => commentState.drafts
+ );
+
+ public readonly portedComments$ = select(
+ this.privateState$,
+ commentState => commentState.portedComments
+ );
+
+ public readonly discardedDrafts$ = select(
+ this.privateState$,
+ commentState => commentState.discardedDrafts
+ );
+
+ // Emits a new value even if only a single draft is changed. Components should
+ // aim to subsribe to something more specific.
+ public readonly changeComments$ = select(
+ this.privateState$,
+ commentState =>
+ new ChangeComments(
+ commentState.comments,
+ commentState.robotComments,
+ commentState.drafts,
+ commentState.portedComments,
+ commentState.portedDrafts
+ )
+ );
+
+ public readonly threads$ = select(this.changeComments$, changeComments =>
+ changeComments.getAllThreadsForChange()
+ );
+
+ public thread$(id: UrlEncodedCommentId) {
+ return select(this.threads$, threads => threads.find(t => t.rootId === id));
+ }
+
+ private numPendingDraftRequests = 0;
+
+ private changeNum?: NumericChangeId;
+
+ private patchNum?: PatchSetNum;
+
+ private readonly reloadListener: () => void;
+
+ private readonly subscriptions: Subscription[] = [];
+
+ private drafts: {[path: string]: DraftInfo[]} = {};
+
+ private draftToastTask?: DelayedTask;
+
+ private discardedDrafts: DraftInfo[] = [];
+
+ constructor(
+ readonly restApiService: RestApiService,
+ readonly reporting: ReportingService
+ ) {
+ this.subscriptions.push(
+ this.discardedDrafts$.subscribe(x => (this.discardedDrafts = x))
+ );
+ this.subscriptions.push(
+ this.drafts$.subscribe(x => (this.drafts = x ?? {}))
+ );
+ this.subscriptions.push(
+ currentPatchNum$.subscribe(x => (this.patchNum = x))
+ );
+ this.subscriptions.push(
+ routerChangeNum$.subscribe(changeNum => {
+ this.changeNum = changeNum;
+ this.setState({...initialState});
+ this.reloadAllComments();
+ })
+ );
+ this.subscriptions.push(
+ combineLatest([changeNum$, currentPatchNum$]).subscribe(
+ ([changeNum, patchNum]) => {
+ this.changeNum = changeNum;
+ this.patchNum = patchNum;
+ this.reloadAllPortedComments();
+ }
+ )
+ );
+ this.reloadListener = () => {
+ this.reloadAllComments();
+ this.reloadAllPortedComments();
+ };
+ document.addEventListener('reload', this.reloadListener);
+ }
+
+ finalize() {
+ document.removeEventListener('reload', this.reloadListener!);
+ for (const s of this.subscriptions) {
+ s.unsubscribe();
+ }
+ this.subscriptions.splice(0, this.subscriptions.length);
+ }
+
+ // Note that this does *not* reload ported comments.
+ async reloadAllComments() {
+ if (!this.changeNum) return;
+ await Promise.all([
+ this.reloadComments(this.changeNum),
+ this.reloadRobotComments(this.changeNum),
+ this.reloadDrafts(this.changeNum),
+ ]);
+ }
+
+ async reloadAllPortedComments() {
+ if (!this.changeNum) return;
+ if (!this.patchNum) return;
+ await Promise.all([
+ this.reloadPortedComments(this.changeNum, this.patchNum),
+ this.reloadPortedDrafts(this.changeNum, this.patchNum),
+ ]);
+ }
+
+ // visible for testing
+ updateState(reducer: (state: CommentState) => CommentState) {
+ const current = this.privateState$.getValue();
+ this.setState(reducer({...current}));
+ }
+
+ // visible for testing
+ setState(state: CommentState) {
+ this.privateState$.next(state);
+ }
+
+ async reloadComments(changeNum: NumericChangeId): Promise<void> {
+ const comments = await this.restApiService.getDiffComments(changeNum);
+ this.updateState(s => setComments(s, comments));
+ }
+
+ async reloadRobotComments(changeNum: NumericChangeId): Promise<void> {
+ const robotComments = await this.restApiService.getDiffRobotComments(
+ changeNum
+ );
+ this.updateState(s => setRobotComments(s, robotComments));
+ }
+
+ async reloadDrafts(changeNum: NumericChangeId): Promise<void> {
+ const drafts = await this.restApiService.getDiffDrafts(changeNum);
+ this.updateState(s => setDrafts(s, drafts));
+ }
+
+ async reloadPortedComments(
+ changeNum: NumericChangeId,
+ patchNum = CURRENT as RevisionId
+ ): Promise<void> {
+ const portedComments = await this.restApiService.getPortedComments(
+ changeNum,
+ patchNum
+ );
+ this.updateState(s => setPortedComments(s, portedComments));
+ }
+
+ async reloadPortedDrafts(
+ changeNum: NumericChangeId,
+ patchNum = CURRENT as RevisionId
+ ): Promise<void> {
+ const portedDrafts = await this.restApiService.getPortedDrafts(
+ changeNum,
+ patchNum
+ );
+ this.updateState(s => setPortedDrafts(s, portedDrafts));
+ }
+
+ async restoreDraft(id: UrlEncodedCommentId) {
+ const found = this.discardedDrafts?.find(d => d.id === id);
+ if (!found) throw new Error('discarded draft not found');
+ const newDraft = {
+ ...found,
+ id: undefined,
+ updated: undefined,
+ __draft: undefined,
+ __unsaved: true,
+ };
+ await this.saveDraft(newDraft);
+ this.updateState(s => deleteDiscardedDraft(s, id));
+ }
+
+ /**
+ * Saves a new or updates an existing draft.
+ * The model will only be updated when a successful response comes back.
+ */
+ async saveDraft(draft: DraftInfo | UnsavedInfo, showToast = true) {
+ assertIsDefined(this.changeNum, 'change number');
+ assertIsDefined(draft.patch_set, 'patchset number of comment draft');
+ if (!draft.message?.trim()) throw new Error('Cannot save empty draft.');
+
+ // Saving the change number as to make sure that the response is still
+ // relevant when it comes back. The user maybe have navigated away.
+ const changeNum = this.changeNum;
+ this.report(Interaction.SAVE_COMMENT, draft);
+ if (showToast) this.showStartRequest();
+ const result = await this.restApiService.saveDiffDraft(
+ changeNum,
+ draft.patch_set,
+ draft
+ );
+ if (changeNum !== this.changeNum) throw new Error('change changed');
+ if (!result.ok) {
+ if (showToast) this.handleFailedDraftRequest();
+ throw new Error(
+ `Failed to save draft comment: ${JSON.stringify(result)}`
+ );
+ }
+ const obj = await this.restApiService.getResponseObject(result);
+ const savedComment = obj as unknown as CommentInfo;
+ const updatedDraft = {
+ ...draft,
+ id: savedComment.id,
+ updated: savedComment.updated,
+ __draft: true,
+ __unsaved: undefined,
+ };
+ if (showToast) this.showEndRequest();
+ this.updateState(s => setDraft(s, updatedDraft));
+ this.report(Interaction.COMMENT_SAVED, updatedDraft);
+ }
+
+ async discardDraft(draftId: UrlEncodedCommentId) {
+ const draft = this.lookupDraft(draftId);
+ assertIsDefined(this.changeNum, 'change number');
+ assertIsDefined(draft, `draft not found by id ${draftId}`);
+ assertIsDefined(draft.patch_set, 'patchset number of comment draft');
+
+ if (!draft.message?.trim()) throw new Error('saved draft cant be empty');
+ // Saving the change number as to make sure that the response is still
+ // relevant when it comes back. The user maybe have navigated away.
+ const changeNum = this.changeNum;
+ this.report(Interaction.DISCARD_COMMENT, draft);
+ this.showStartRequest();
+ const result = await this.restApiService.deleteDiffDraft(
+ changeNum,
+ draft.patch_set,
+ {id: draft.id}
+ );
+ if (changeNum !== this.changeNum) throw new Error('change changed');
+ if (!result.ok) {
+ this.handleFailedDraftRequest();
+ throw new Error(
+ `Failed to discard draft comment: ${JSON.stringify(result)}`
+ );
+ }
+ this.showEndRequest();
+ this.updateState(s => deleteDraft(s, draft));
+ // We don't store empty discarded drafts and don't need an UNDO then.
+ if (draft.message?.trim()) {
+ fire(document, 'show-alert', {
+ message: 'Draft Discarded',
+ action: 'Undo',
+ callback: () => this.restoreDraft(draft.id),
+ });
+ }
+ this.report(Interaction.COMMENT_DISCARDED, draft);
+ }
+
+ private report(interaction: Interaction, comment: CommentBasics) {
+ const details = reportingDetails(comment);
+ this.reporting.reportInteraction(interaction, details);
+ }
+
+ private showStartRequest() {
+ this.numPendingDraftRequests += 1;
+ this.updateRequestToast();
+ }
+
+ private showEndRequest() {
+ this.numPendingDraftRequests -= 1;
+ this.updateRequestToast();
+ }
+
+ private handleFailedDraftRequest() {
+ this.numPendingDraftRequests -= 1;
+ this.updateRequestToast(/* requestFailed=*/ true);
+ }
+
+ private updateRequestToast(requestFailed?: boolean) {
+ if (this.numPendingDraftRequests === 0 && !requestFailed) {
+ fireEvent(document, 'hide-alert');
+ return;
+ }
+ const message = getSavingMessage(
+ this.numPendingDraftRequests,
+ requestFailed
+ );
+ this.draftToastTask = debounce(
+ this.draftToastTask,
+ () => {
+ // Note: the event is fired on the body rather than this element because
+ // this element may not be attached by the time this executes, in which
+ // case the event would not bubble.
+ fireAlert(document.body, message);
+ },
+ TOAST_DEBOUNCE_INTERVAL
+ );
+ }
+
+ private lookupDraft(id: UrlEncodedCommentId): DraftInfo | undefined {
+ return Object.values(this.drafts)
+ .flat()
+ .find(d => d.id === id);
+ }
}
diff --git a/polygerrit-ui/app/services/comments/comments-model_test.ts b/polygerrit-ui/app/services/comments/comments-model_test.ts
index 30fc7cf..93a1d52 100644
--- a/polygerrit-ui/app/services/comments/comments-model_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-model_test.ts
@@ -19,17 +19,29 @@
import {UrlEncodedCommentId} from '../../types/common';
import {DraftInfo} from '../../utils/comment-util';
import './comments-model';
+import {CommentsModel} from './comments-model';
+import {deleteDraft} from './comments-model';
+import {Subscription} from 'rxjs';
+import '../../test/common-test-setup-karma';
import {
- updateStateDeleteDraft,
- _testOnly_getState,
- _testOnly_setState,
-} from './comments-model';
+ createComment,
+ createParsedChange,
+ TEST_NUMERIC_CHANGE_ID,
+} from '../../test/test-data-generators';
+import {stubRestApi, waitUntil, waitUntilCalled} from '../../test/test-utils';
+import {getAppContext} from '../app-context';
+import {updateStateChange} from '../change/change-model';
+import {
+ GerritView,
+ updateState as updateRouterState,
+} from '../router/router-model';
+import {PathToCommentsInfoMap} from '../../types/common';
suite('comments model tests', () => {
test('updateStateDeleteDraft', () => {
const draft = createDraft();
draft.id = '1' as UrlEncodedCommentId;
- _testOnly_setState({
+ const state = {
comments: {},
robotComments: {},
drafts: {
@@ -38,9 +50,9 @@
portedComments: {},
portedDrafts: {},
discardedDrafts: [],
- });
- updateStateDeleteDraft(draft);
- assert.deepEqual(_testOnly_getState(), {
+ };
+ const output = deleteDraft(state, draft);
+ assert.deepEqual(output, {
comments: {},
robotComments: {},
drafts: {
@@ -52,3 +64,64 @@
});
});
});
+
+suite('change service tests', () => {
+ let subscriptions: Subscription[] = [];
+
+ teardown(() => {
+ for (const s of subscriptions) {
+ s.unsubscribe();
+ }
+ subscriptions = [];
+ });
+
+ test('loads comments', async () => {
+ const model = new CommentsModel(
+ getAppContext().restApiService,
+ getAppContext().reportingService
+ );
+ const diffCommentsSpy = stubRestApi('getDiffComments').returns(
+ Promise.resolve({'foo.c': [createComment()]})
+ );
+ const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
+ Promise.resolve({})
+ );
+ const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
+ Promise.resolve({})
+ );
+ const portedCommentsSpy = stubRestApi('getPortedComments').returns(
+ Promise.resolve({'foo.c': [createComment()]})
+ );
+ const portedDraftsSpy = stubRestApi('getPortedDrafts').returns(
+ Promise.resolve({})
+ );
+ let comments: PathToCommentsInfoMap = {};
+ subscriptions.push(model.comments$.subscribe(c => (comments = c ?? {})));
+ let portedComments: PathToCommentsInfoMap = {};
+ subscriptions.push(
+ model.portedComments$.subscribe(c => (portedComments = c ?? {}))
+ );
+
+ updateRouterState(GerritView.CHANGE, TEST_NUMERIC_CHANGE_ID);
+ updateStateChange(createParsedChange());
+
+ await waitUntilCalled(diffCommentsSpy, 'diffCommentsSpy');
+ await waitUntilCalled(diffRobotCommentsSpy, 'diffRobotCommentsSpy');
+ await waitUntilCalled(diffDraftsSpy, 'diffDraftsSpy');
+ await waitUntilCalled(portedCommentsSpy, 'portedCommentsSpy');
+ await waitUntilCalled(portedDraftsSpy, 'portedDraftsSpy');
+ await waitUntil(
+ () => Object.keys(comments).length > 0,
+ 'comment in model not set'
+ );
+ await waitUntil(
+ () => Object.keys(portedComments).length > 0,
+ 'ported comment in model not set'
+ );
+
+ assert.equal(comments['foo.c'].length, 1);
+ assert.equal(comments['foo.c'][0].id, '12345');
+ assert.equal(portedComments['foo.c'].length, 1);
+ assert.equal(portedComments['foo.c'][0].id, '12345');
+ });
+});
diff --git a/polygerrit-ui/app/services/comments/comments-service.ts b/polygerrit-ui/app/services/comments/comments-service.ts
deleted file mode 100644
index ac32258..0000000
--- a/polygerrit-ui/app/services/comments/comments-service.ts
+++ /dev/null
@@ -1,315 +0,0 @@
-/**
- * @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 {combineLatest, Subscription} from 'rxjs';
-import {
- CommentBasics,
- CommentInfo,
- NumericChangeId,
- PatchSetNum,
- RevisionId,
- UrlEncodedCommentId,
-} from '../../types/common';
-import {
- reportingDetails,
- DraftInfo,
- UnsavedInfo,
-} from '../../utils/comment-util';
-import {fire, fireAlert, fireEvent} from '../../utils/event-util';
-import {CURRENT} from '../../utils/patch-set-util';
-import {RestApiService} from '../gr-rest-api/gr-rest-api';
-import {
- updateStateSetDraft,
- updateStateDeleteDraft,
- updateStateComments,
- updateStateRobotComments,
- updateStateDrafts,
- updateStatePortedComments,
- updateStatePortedDrafts,
- updateStateDeleteDiscardedDraft,
- discardedDrafts$,
- updateStateReset,
- drafts$,
-} from './comments-model';
-import {changeNum$, currentPatchNum$} from '../change/change-model';
-import {Finalizable} from '../registry';
-import {routerChangeNum$} from '../router/router-model';
-import {Interaction} from '../../constants/reporting';
-import {assertIsDefined} from '../../utils/common-util';
-import {debounce, DelayedTask} from '../../utils/async-util';
-import {pluralize} from '../../utils/string-util';
-import {ReportingService} from '../gr-reporting/gr-reporting';
-
-const TOAST_DEBOUNCE_INTERVAL = 200;
-
-function getSavingMessage(numPending: number, requestFailed?: boolean) {
- if (requestFailed) {
- return 'Unable to save draft';
- }
- if (numPending === 0) {
- return 'All changes saved';
- }
- return `Saving ${pluralize(numPending, 'draft')}...`;
-}
-
-export class CommentsService implements Finalizable {
- private numPendingDraftRequests = 0;
-
- private changeNum?: NumericChangeId;
-
- private patchNum?: PatchSetNum;
-
- private readonly reloadListener: () => void;
-
- private readonly subscriptions: Subscription[] = [];
-
- private drafts: {[path: string]: DraftInfo[]} = {};
-
- private draftToastTask?: DelayedTask;
-
- private discardedDrafts: DraftInfo[] = [];
-
- constructor(
- readonly restApiService: RestApiService,
- readonly reporting: ReportingService
- ) {
- this.subscriptions.push(
- discardedDrafts$.subscribe(x => (this.discardedDrafts = x))
- );
- this.subscriptions.push(drafts$.subscribe(x => (this.drafts = x ?? {})));
- this.subscriptions.push(
- currentPatchNum$.subscribe(x => (this.patchNum = x))
- );
- this.subscriptions.push(
- routerChangeNum$.subscribe(changeNum => {
- this.changeNum = changeNum;
- updateStateReset();
- this.reloadAllComments();
- })
- );
- this.subscriptions.push(
- combineLatest([changeNum$, currentPatchNum$]).subscribe(
- ([changeNum, patchNum]) => {
- this.changeNum = changeNum;
- this.patchNum = patchNum;
- this.reloadAllPortedComments();
- }
- )
- );
- this.reloadListener = () => {
- this.reloadAllComments();
- this.reloadAllPortedComments();
- };
- document.addEventListener('reload', this.reloadListener);
- }
-
- finalize() {
- document.removeEventListener('reload', this.reloadListener!);
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions.splice(0, this.subscriptions.length);
- }
-
- // Note that this does *not* reload ported comments.
- reloadAllComments() {
- if (!this.changeNum) return;
- this.reloadComments(this.changeNum);
- this.reloadRobotComments(this.changeNum);
- this.reloadDrafts(this.changeNum);
- }
-
- reloadAllPortedComments() {
- if (!this.changeNum) return;
- if (!this.patchNum) return;
- this.reloadPortedComments(this.changeNum, this.patchNum);
- this.reloadPortedDrafts(this.changeNum, this.patchNum);
- }
-
- reloadComments(changeNum: NumericChangeId): Promise<void> {
- return this.restApiService
- .getDiffComments(changeNum)
- .then(comments => updateStateComments(comments));
- }
-
- reloadRobotComments(changeNum: NumericChangeId): Promise<void> {
- return this.restApiService
- .getDiffRobotComments(changeNum)
- .then(robotComments => updateStateRobotComments(robotComments));
- }
-
- reloadDrafts(changeNum: NumericChangeId): Promise<void> {
- return this.restApiService
- .getDiffDrafts(changeNum)
- .then(drafts => updateStateDrafts(drafts));
- }
-
- reloadPortedComments(
- changeNum: NumericChangeId,
- patchNum = CURRENT as RevisionId
- ): Promise<void> {
- return this.restApiService
- .getPortedComments(changeNum, patchNum)
- .then(portedComments => updateStatePortedComments(portedComments));
- }
-
- reloadPortedDrafts(
- changeNum: NumericChangeId,
- patchNum = CURRENT as RevisionId
- ): Promise<void> {
- return this.restApiService
- .getPortedDrafts(changeNum, patchNum)
- .then(portedDrafts => updateStatePortedDrafts(portedDrafts));
- }
-
- async restoreDraft(id: UrlEncodedCommentId) {
- const found = this.discardedDrafts?.find(d => d.id === id);
- if (!found) throw new Error('discarded draft not found');
- const newDraft = {
- ...found,
- id: undefined,
- updated: undefined,
- __draft: undefined,
- __unsaved: true,
- };
- await this.saveDraft(newDraft);
- updateStateDeleteDiscardedDraft(id);
- }
-
- /**
- * Saves a new or updates an existing draft.
- * The model will only be updated when a successful response comes back.
- */
- async saveDraft(draft: DraftInfo | UnsavedInfo, showToast = true) {
- assertIsDefined(this.changeNum, 'change number');
- assertIsDefined(draft.patch_set, 'patchset number of comment draft');
- if (!draft.message?.trim()) throw new Error('Cannot save empty draft.');
-
- // Saving the change number as to make sure that the response is still
- // relevant when it comes back. The user maybe have navigated away.
- const changeNum = this.changeNum;
- this.report(Interaction.SAVE_COMMENT, draft);
- if (showToast) this.showStartRequest();
- const result = await this.restApiService.saveDiffDraft(
- changeNum,
- draft.patch_set,
- draft
- );
- if (changeNum !== this.changeNum) throw new Error('change changed');
- if (!result.ok) {
- if (showToast) this.handleFailedDraftRequest();
- throw new Error(
- `Failed to save draft comment: ${JSON.stringify(result)}`
- );
- }
- const obj = await this.restApiService.getResponseObject(result);
- const savedComment = obj as unknown as CommentInfo;
- const updatedDraft = {
- ...draft,
- id: savedComment.id,
- updated: savedComment.updated,
- __draft: true,
- __unsaved: undefined,
- };
- if (showToast) this.showEndRequest();
- updateStateSetDraft(updatedDraft);
- this.report(Interaction.COMMENT_SAVED, updatedDraft);
- }
-
- async discardDraft(draftId: UrlEncodedCommentId) {
- const draft = this.lookupDraft(draftId);
- assertIsDefined(this.changeNum, 'change number');
- assertIsDefined(draft, `draft not found by id ${draftId}`);
- assertIsDefined(draft.patch_set, 'patchset number of comment draft');
-
- if (!draft.message?.trim()) throw new Error('saved draft cant be empty');
- // Saving the change number as to make sure that the response is still
- // relevant when it comes back. The user maybe have navigated away.
- const changeNum = this.changeNum;
- this.report(Interaction.DISCARD_COMMENT, draft);
- this.showStartRequest();
- const result = await this.restApiService.deleteDiffDraft(
- changeNum,
- draft.patch_set,
- {id: draft.id}
- );
- if (changeNum !== this.changeNum) throw new Error('change changed');
- if (!result.ok) {
- this.handleFailedDraftRequest();
- throw new Error(
- `Failed to discard draft comment: ${JSON.stringify(result)}`
- );
- }
- this.showEndRequest();
- updateStateDeleteDraft(draft);
- // We don't store empty discarded drafts and don't need an UNDO then.
- if (draft.message?.trim()) {
- fire(document, 'show-alert', {
- message: 'Draft Discarded',
- action: 'Undo',
- callback: () => this.restoreDraft(draft.id),
- });
- }
- this.report(Interaction.COMMENT_DISCARDED, draft);
- }
-
- private report(interaction: Interaction, comment: CommentBasics) {
- const details = reportingDetails(comment);
- this.reporting.reportInteraction(interaction, details);
- }
-
- private showStartRequest() {
- this.numPendingDraftRequests += 1;
- this.updateRequestToast();
- }
-
- private showEndRequest() {
- this.numPendingDraftRequests -= 1;
- this.updateRequestToast();
- }
-
- private handleFailedDraftRequest() {
- this.numPendingDraftRequests -= 1;
- this.updateRequestToast(/* requestFailed=*/ true);
- }
-
- private updateRequestToast(requestFailed?: boolean) {
- if (this.numPendingDraftRequests === 0 && !requestFailed) {
- fireEvent(document, 'hide-alert');
- return;
- }
- const message = getSavingMessage(
- this.numPendingDraftRequests,
- requestFailed
- );
- this.draftToastTask = debounce(
- this.draftToastTask,
- () => {
- // Note: the event is fired on the body rather than this element because
- // this element may not be attached by the time this executes, in which
- // case the event would not bubble.
- fireAlert(document.body, message);
- },
- TOAST_DEBOUNCE_INTERVAL
- );
- }
-
- private lookupDraft(id: UrlEncodedCommentId): DraftInfo | undefined {
- return Object.values(this.drafts)
- .flat()
- .find(d => d.id === id);
- }
-}
diff --git a/polygerrit-ui/app/services/comments/comments-service_test.ts b/polygerrit-ui/app/services/comments/comments-service_test.ts
deleted file mode 100644
index 91ac37c..0000000
--- a/polygerrit-ui/app/services/comments/comments-service_test.ts
+++ /dev/null
@@ -1,94 +0,0 @@
-/**
- * @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 {Subscription} from 'rxjs';
-import '../../test/common-test-setup-karma';
-import {
- createComment,
- createParsedChange,
- TEST_NUMERIC_CHANGE_ID,
-} from '../../test/test-data-generators';
-import {stubRestApi, waitUntil, waitUntilCalled} from '../../test/test-utils';
-import {getAppContext} from '../app-context';
-import {CommentsService} from './comments-service';
-import {updateStateChange} from '../change/change-model';
-import {
- GerritView,
- updateState as updateRouterState,
-} from '../router/router-model';
-import {comments$, portedComments$} from './comments-model';
-import {PathToCommentsInfoMap} from '../../types/common';
-
-suite('change service tests', () => {
- let subscriptions: Subscription[] = [];
-
- teardown(() => {
- for (const s of subscriptions) {
- s.unsubscribe();
- }
- subscriptions = [];
- });
-
- test('loads comments', async () => {
- new CommentsService(
- getAppContext().restApiService,
- getAppContext().reportingService
- );
- const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({'foo.c': [createComment()]})
- );
- const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({})
- );
- const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
- Promise.resolve({})
- );
- const portedCommentsSpy = stubRestApi('getPortedComments').returns(
- Promise.resolve({'foo.c': [createComment()]})
- );
- const portedDraftsSpy = stubRestApi('getPortedDrafts').returns(
- Promise.resolve({})
- );
- let comments: PathToCommentsInfoMap = {};
- subscriptions.push(comments$.subscribe(c => (comments = c ?? {})));
- let portedComments: PathToCommentsInfoMap = {};
- subscriptions.push(
- portedComments$.subscribe(c => (portedComments = c ?? {}))
- );
-
- updateRouterState(GerritView.CHANGE, TEST_NUMERIC_CHANGE_ID);
- updateStateChange(createParsedChange());
-
- await waitUntilCalled(diffCommentsSpy, 'diffCommentsSpy');
- await waitUntilCalled(diffRobotCommentsSpy, 'diffRobotCommentsSpy');
- await waitUntilCalled(diffDraftsSpy, 'diffDraftsSpy');
- await waitUntilCalled(portedCommentsSpy, 'portedCommentsSpy');
- await waitUntilCalled(portedDraftsSpy, 'portedDraftsSpy');
- await waitUntil(
- () => Object.keys(comments).length > 0,
- 'comment in model not set'
- );
- await waitUntil(
- () => Object.keys(portedComments).length > 0,
- 'ported comment in model not set'
- );
-
- assert.equal(comments['foo.c'].length, 1);
- assert.equal(comments['foo.c'][0].id, '12345');
- assert.equal(portedComments['foo.c'].length, 1);
- assert.equal(portedComments['foo.c'][0].id, '12345');
- });
-});
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index fc621a2..fbc0b48 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -46,7 +46,6 @@
import {cleanUpStorage} from '../services/storage/gr-storage_mock';
import {_testOnly_resetState as resetChangeState} from '../services/change/change-model';
-import {_testOnly_resetState as resetCommentsState} from '../services/comments/comments-model';
import {_testOnly_resetState as resetRouterState} from '../services/router/router-model';
declare global {
@@ -114,7 +113,6 @@
initGlobalVariables(appContext);
resetChangeState();
- resetCommentsState();
resetRouterState();
const shortcuts = appContext.shortcutsService;
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 946f813..42aa425 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -29,7 +29,7 @@
import {ChecksModel} from '../services/checks/checks-model';
import {GrJsApiInterface} from '../elements/shared/gr-js-api-interface/gr-js-api-interface-element';
import {UserModel} from '../services/user/user-model';
-import {CommentsService} from '../services/comments/comments-service';
+import {CommentsModel} from '../services/comments/comments-model';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
import {BrowserModel} from '../services/browser/browser-model';
import {ConfigModel} from '../services/config/config-model';
@@ -49,10 +49,10 @@
assertIsDefined(ctx.restApiService, 'restApiService');
return new ChangeService(ctx.restApiService!);
},
- commentsService: (ctx: Partial<AppContext>) => {
+ commentsModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.restApiService, 'restApiService');
assertIsDefined(ctx.reportingService, 'reportingService');
- return new CommentsService(ctx.restApiService!, ctx.reportingService!);
+ return new CommentsModel(ctx.restApiService!, ctx.reportingService!);
},
checksModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.reportingService, 'reportingService');
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 50e08fb..ebeb1db 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -23,7 +23,7 @@
import {StorageService} from '../services/storage/gr-storage';
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
-import {CommentsService} from '../services/comments/comments-service';
+import {CommentsModel} from '../services/comments/comments-model';
import {UserModel} from '../services/user/user-model';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
import {queryAndAssert, query} from '../utils/common-util';
@@ -112,8 +112,8 @@
return sinon.spy(getAppContext().restApiService, method);
}
-export function stubComments<K extends keyof CommentsService>(method: K) {
- return sinon.stub(getAppContext().commentsService, method);
+export function stubComments<K extends keyof CommentsModel>(method: K) {
+ return sinon.stub(getAppContext().commentsModel, method);
}
export function stubUsers<K extends keyof UserModel>(method: K) {
