Migrate Comments Model to Dependency Injection
Change-Id: I1d255999195e33100a893c96bf92150e79a49885
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 08b42c2..dafa384 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,6 +59,8 @@
import {modifierPressed} from '../../../utils/dom-util';
import {DropdownLink} from '../../shared/gr-dropdown/gr-dropdown';
import {fontStyles} from '../../../styles/gr-font-styles';
+import {commentsModelToken} from '../../../services/comments/comments-model';
+import {resolve} from '../../../services/dependency';
export enum SummaryChipStyles {
INFO = 'info',
@@ -401,14 +403,14 @@
private showAllChips = new Map<RunStatus | Category, boolean>();
- private commentsModel = getAppContext().commentsModel;
+ private getCommentsModel = resolve(this, commentsModelToken);
private userModel = getAppContext().userModel;
private checksModel = getAppContext().checksModel;
- constructor() {
- super();
+ override connectedCallback() {
+ super.connectedCallback();
subscribe(
this,
this.checksModel.allRunsLatestPatchsetLatestAttempt$,
@@ -441,12 +443,12 @@
);
subscribe(
this,
- this.commentsModel.changeComments$,
+ this.getCommentsModel().changeComments$,
x => (this.changeComments = x)
);
subscribe(
this,
- this.commentsModel.threads$,
+ this.getCommentsModel().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 a018edb..5a54599 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
@@ -45,7 +45,6 @@
import '../../checks/gr-checks-tab';
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-change-view_html';
import {
KeyboardShortcutMixin,
@@ -198,6 +197,8 @@
} from '../../../utils/attention-set-util';
import {listen} from '../../../services/shortcuts/shortcuts-service';
import {LoadingStatus} from '../../../services/change/change-model';
+import {commentsModelToken} from '../../../services/comments/comments-model';
+import {resolve, DIPolymerElement} from '../../../services/dependency';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -248,7 +249,7 @@
export type ChangeViewPatchRange = Partial<PatchRange>;
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = KeyboardShortcutMixin(PolymerElement);
+const base = KeyboardShortcutMixin(DIPolymerElement);
@customElement('gr-change-view')
export class GrChangeView extends base {
@@ -610,7 +611,8 @@
private readonly routerModel = getAppContext().routerModel;
- private readonly commentsModel = getAppContext().commentsModel;
+ // Private but used in tests.
+ readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly shortcuts = getAppContext().shortcutsService;
@@ -634,47 +636,6 @@
// visible for testing
routerPatchNum?: PatchSetNum;
- override ready() {
- super.ready();
- this.subscriptions.push(
- this.checksModel.aPluginHasRegistered$.subscribe(b => {
- this._showChecksTab = b;
- })
- );
- this.subscriptions.push(
- this.routerModel.routerView$.subscribe(view => {
- this.isViewCurrent = view === GerritView.CHANGE;
- })
- );
- this.subscriptions.push(
- this.routerModel.routerPatchNum$.subscribe(patchNum => {
- this.routerPatchNum = patchNum;
- })
- );
- this.subscriptions.push(
- this.commentsModel.drafts$.subscribe(drafts => {
- this._diffDrafts = {...drafts};
- })
- );
- this.subscriptions.push(
- this.userModel.preferenceDiffViewMode$.subscribe(diffViewMode => {
- this.diffViewMode = diffViewMode;
- })
- );
- this.subscriptions.push(
- this.commentsModel.changeComments$.subscribe(changeComments => {
- this._changeComments = changeComments;
- })
- );
- this.subscriptions.push(
- this.changeModel.change$.subscribe(change => {
- // The change view is tied to a specific change number, so don't update
- // _change to undefined.
- if (change) this._change = change;
- })
- );
- }
-
constructor() {
super();
this.addEventListener('topic-changed', () => this._handleTopicChanged());
@@ -695,8 +656,49 @@
this.addEventListener('open-reply-dialog', () => this._openReplyDialog());
}
+ private setupSubscriptions() {
+ this.subscriptions.push(
+ this.checksModel.aPluginHasRegistered$.subscribe(b => {
+ this._showChecksTab = b;
+ })
+ );
+ this.subscriptions.push(
+ this.routerModel.routerView$.subscribe(view => {
+ this.isViewCurrent = view === GerritView.CHANGE;
+ })
+ );
+ this.subscriptions.push(
+ this.routerModel.routerPatchNum$.subscribe(patchNum => {
+ this.routerPatchNum = patchNum;
+ })
+ );
+ this.subscriptions.push(
+ this.getCommentsModel().drafts$.subscribe(drafts => {
+ this._diffDrafts = {...drafts};
+ })
+ );
+ this.subscriptions.push(
+ this.userModel.preferenceDiffViewMode$.subscribe(diffViewMode => {
+ this.diffViewMode = diffViewMode;
+ })
+ );
+ this.subscriptions.push(
+ this.getCommentsModel().changeComments$.subscribe(changeComments => {
+ this._changeComments = changeComments;
+ })
+ );
+ this.subscriptions.push(
+ this.changeModel.change$.subscribe(change => {
+ // The change view is tied to a specific change number, so don't update
+ // _change to undefined.
+ if (change) this._change = change;
+ })
+ );
+ }
+
override connectedCallback() {
super.connectedCallback();
+ this.setupSubscriptions();
this._throttledToggleChangeStar = throttleWrap<KeyboardEvent>(_ =>
this._handleToggleChangeStar()
);
@@ -2205,13 +2207,13 @@
const promises = [this.loadAndSetCommitInfo(), this.$.fileList.reload()];
if (patchNumChanged) {
promises.push(
- this.commentsModel.reloadPortedComments(
+ this.getCommentsModel().reloadPortedComments(
this._changeNum,
this._patchRange?.patchNum
)
);
promises.push(
- this.commentsModel.reloadPortedDrafts(
+ this.getCommentsModel().reloadPortedDrafts(
this._changeNum,
this._patchRange?.patchNum
)
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 69cca9f..4ca593a 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
@@ -39,7 +39,6 @@
import {
mockPromise,
queryAndAssert,
- stubComments,
stubRestApi,
stubUsers,
waitQueryAndAssert,
@@ -1362,8 +1361,14 @@
sinon.stub(element, 'loadAndSetCommitInfo');
sinon.stub(element.$.fileList, 'reload');
flush();
- const reloadPortedCommentsStub = stubComments('reloadPortedComments');
- const reloadPortedDraftsStub = stubComments('reloadPortedDrafts');
+ const reloadPortedCommentsStub = sinon.stub(
+ element.getCommentsModel(),
+ 'reloadPortedComments'
+ );
+ const reloadPortedDraftsStub = sinon.stub(
+ element.getCommentsModel(),
+ 'reloadPortedDrafts'
+ );
sinon.stub(element.$.fileList, 'collapseAllDiffs');
const value: AppElementChangeViewParams = {
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 e26a0d5..9bcedc8 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
@@ -31,6 +31,8 @@
import {subscribe} from '../../lit/subscription-controller';
import {getAppContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
+import {commentsModelToken} from '../../../services/comments/comments-model';
+import {resolve} from '../../../services/dependency';
@customElement('gr-confirm-submit-dialog')
export class GrConfirmSubmitDialog extends LitElement {
@@ -61,7 +63,7 @@
@state()
initialised = false;
- private commentsModel = getAppContext().commentsModel;
+ private getCommentsModel = resolve(this, commentsModelToken);
private changeModel = getAppContext().changeModel;
@@ -91,12 +93,12 @@
];
}
- constructor() {
- super();
+ override connectedCallback() {
+ super.connectedCallback();
subscribe(this, this.changeModel.change$, x => (this.change = x));
subscribe(
this,
- this.commentsModel.threads$,
+ this.getCommentsModel().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 b8f41d9..b1e6a1b 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
@@ -87,6 +87,7 @@
import {select} from '../../../utils/observable-util';
import {resolve, DIPolymerElement} from '../../../services/dependency';
import {browserModelToken} from '../../../services/browser/browser-model';
+import {commentsModelToken} from '../../../services/comments/comments-model';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -317,7 +318,7 @@
private readonly userModel = getAppContext().userModel;
- private readonly commentsModel = getAppContext().commentsModel;
+ private readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly getBrowserModel = resolve(this, browserModelToken);
@@ -378,7 +379,7 @@
override connectedCallback() {
super.connectedCallback();
this.subscriptions = [
- this.commentsModel.changeComments$.subscribe(changeComments => {
+ this.getCommentsModel().changeComments$.subscribe(changeComments => {
this.changeComments = changeComments;
}),
this.getBrowserModel().diffViewMode$.subscribe(
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 36d0ce6..8ae0115 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
@@ -21,7 +21,6 @@
import '../gr-message/gr-message';
import '../../../styles/gr-paper-styles';
import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-messages-list_html';
import {
Shortcut,
@@ -51,6 +50,8 @@
FormattedReviewerUpdateInfo,
ParsedChangeInfo,
} from '../../../types/types';
+import {commentsModelToken} from '../../../services/comments/comments-model';
+import {resolve, DIPolymerElement} from '../../../services/dependency';
/**
* The content of the enum is also used in the UI for the button text.
@@ -194,7 +195,7 @@
}
@customElement('gr-messages-list')
-export class GrMessagesList extends PolymerElement {
+export class GrMessagesList extends DIPolymerElement {
static get template() {
return htmlTemplate;
}
@@ -252,7 +253,7 @@
private readonly userModel = getAppContext().userModel;
// Private but used in tests.
- readonly commentsModel = getAppContext().commentsModel;
+ readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly changeModel = getAppContext().changeModel;
@@ -265,7 +266,7 @@
override connectedCallback() {
super.connectedCallback();
this.subscriptions.push(
- this.commentsModel.threads$.subscribe(x => {
+ this.getCommentsModel().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 65c3c74..0a69d8a 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
@@ -139,7 +139,7 @@
// comment API.
commentApiWrapper = basicFixture.instantiate();
element = commentApiWrapper.$.messagesList;
- await element.commentsModel.reloadComments();
+ await element.getCommentsModel().reloadComments();
element.messages = messages;
await flush();
});
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 9cc0dbb..9aafd36 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
@@ -90,6 +90,7 @@
import {DisplayLine, RenderPreferences} from '../../../api/diff';
import {resolve, DIPolymerElement} from '../../../services/dependency';
import {browserModelToken} from '../../../services/browser/browser-model';
+import {commentsModelToken} from '../../../services/comments/comments-model';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -269,7 +270,7 @@
private readonly getBrowserModel = resolve(this, browserModelToken);
- private readonly commentsModel = getAppContext().commentsModel;
+ private readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly reporting = getAppContext().reportingService;
@@ -323,7 +324,7 @@
this._loggedIn = loggedIn;
});
this.subscriptions.push(
- this.commentsModel.changeComments$.subscribe(changeComments => {
+ this.getCommentsModel().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 e1b7b4f..b1cfe55 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
@@ -119,6 +119,7 @@
import {DisplayLine} from '../../../api/diff';
import {GrDownloadDialog} from '../../change/gr-download-dialog/gr-download-dialog';
import {browserModelToken} from '../../../services/browser/browser-model';
+import {commentsModelToken} from '../../../services/comments/comments-model';
import {resolve, DIPolymerElement} from '../../../services/dependency';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
@@ -369,7 +370,7 @@
readonly getBrowserModel = resolve(this, browserModelToken);
// Private but used in tests.
- readonly commentsModel = getAppContext().commentsModel;
+ readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly shortcuts = getAppContext().shortcutsService;
@@ -394,7 +395,7 @@
});
this.subscriptions.push(
- this.commentsModel.changeComments$.subscribe(changeComments => {
+ this.getCommentsModel().changeComments$.subscribe(changeComments => {
this._changeComments = changeComments;
})
);
@@ -1163,7 +1164,7 @@
)
);
}
- promises.push(until(this.commentsModel.commentsLoading$, isFalse));
+ promises.push(until(this.getCommentsModel().commentsLoading$, isFalse));
this.$.diffHost.cancel();
this.$.diffHost.clearDiffContent();
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 aa39f87..b590625 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
@@ -91,7 +91,7 @@
]});
await flush();
- element.commentsModel.setState({
+ element.getCommentsModel().setState({
comments: {},
robotComments: {},
drafts: {},
@@ -145,7 +145,7 @@
});
test('comment url resolves to comment.patch_set vs latest', () => {
- element.commentsModel.setState({
+ element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
{
@@ -219,7 +219,7 @@
test('unchanged diff X vs latest from comment links navigates to base vs X'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- element.commentsModel.setState({
+ element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
{
@@ -272,7 +272,7 @@
test('unchanged diff Base vs latest from comment does not navigate'
, () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- element.commentsModel.setState({
+ element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
{
@@ -353,7 +353,7 @@
});
test('diff toast to go to latest is shown and not base', async () => {
- element.commentsModel.setState({
+ element.getCommentsModel().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 71fee62..b6397b0 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,6 +51,8 @@
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
import {subscribe} from '../../lit/subscription-controller';
+import {commentsModelToken} from '../../../services/comments/comments-model';
+import {resolve} from '../../../services/dependency';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -126,13 +128,13 @@
private readonly reporting: ReportingService =
getAppContext().reportingService;
- private readonly commentsModel = getAppContext().commentsModel;
+ private readonly getCommentsModel = resolve(this, commentsModelToken);
- constructor() {
- super();
+ override connectedCallback() {
+ super.connectedCallback();
subscribe(
this,
- this.commentsModel.changeComments$,
+ this.getCommentsModel().changeComments$,
x => (this.changeComments = x)
);
}
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 7a4b6f2..0f33ffb 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
@@ -70,6 +70,8 @@
import {ShortcutController} from '../../lit/shortcut-controller';
import {ValueChangedEvent} from '../../../types/events';
import {notDeepEqual} from '../../../utils/deep-util';
+import {resolve} from '../../../services/dependency';
+import {commentsModelToken} from '../../../services/comments/comments-model';
const NEWLINE_PATTERN = /\n/g;
@@ -245,7 +247,8 @@
@state()
saving = false;
- private readonly commentsModel = getAppContext().commentsModel;
+ // Private but used in tests.
+ readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly changeModel = getAppContext().changeModel;
@@ -783,7 +786,7 @@
} else {
try {
this.saving = true;
- await this.commentsModel.saveDraft(unsaved);
+ await this.getCommentsModel().saveDraft(unsaved);
} finally {
this.saving = false;
}
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 4615bb7..9e3db21 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
@@ -28,7 +28,6 @@
import {
mockPromise,
queryAndAssert,
- stubComments,
stubRestApi,
waitUntilCalled,
MockPromise,
@@ -234,7 +233,9 @@
setup(async () => {
savePromise = mockPromise<DraftInfo>();
- stub = stubComments('saveDraft').returns(savePromise);
+ stub = sinon
+ .stub(element.getCommentsModel(), 'saveDraft')
+ .returns(savePromise);
element.thread = createThread(c1, {...c2, unresolved: true});
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 19d2ea6..18ef5a9 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -30,6 +30,7 @@
import {getAppContext} from '../../../services/app-context';
import {css, html, LitElement, PropertyValues} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
+import {resolve} from '../../../services/dependency';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {GrTextarea} from '../gr-textarea/gr-textarea';
import {GrOverlay} from '../gr-overlay/gr-overlay';
@@ -55,6 +56,7 @@
import {fire, fireEvent} from '../../../utils/event-util';
import {assertIsDefined} from '../../../utils/common-util';
import {Key, Modifier} from '../../../utils/dom-util';
+import {commentsModelToken} from '../../../services/comments/comments-model';
import {sharedStyles} from '../../../styles/shared-styles';
import {subscribe} from '../../lit/subscription-controller';
import {ShortcutController} from '../../lit/shortcut-controller';
@@ -230,7 +232,8 @@
private readonly changeModel = getAppContext().changeModel;
- private readonly commentsModel = getAppContext().commentsModel;
+ // Private but used in tests.
+ readonly getCommentsModel = resolve(this, commentsModelToken);
private readonly userModel = getAppContext().userModel;
@@ -1092,7 +1095,7 @@
if (messageToSave === '') {
// Don't try to discard UnsavedInfo. Nothing to do then.
if (this.comment.id) {
- await this.commentsModel.discardDraft(this.comment.id);
+ await this.getCommentsModel().discardDraft(this.comment.id);
}
} else {
// No need to make a backend call when nothing has changed.
@@ -1115,7 +1118,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.commentsModel.saveDraft(
+ return this.getCommentsModel().saveDraft(
{
...this.comment,
message,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index a6e0f50..d3a1007 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -24,7 +24,6 @@
query,
pressKey,
listenOnce,
- stubComments,
mockPromise,
waitUntilCalled,
dispatch,
@@ -453,7 +452,9 @@
test('save', async () => {
const savePromise = mockPromise<DraftInfo>();
- const stub = stubComments('saveDraft').returns(savePromise);
+ const stub = sinon
+ .stub(element.getCommentsModel(), 'saveDraft')
+ .returns(savePromise);
element.comment = createDraft();
element.editing = true;
@@ -480,9 +481,9 @@
});
test('save failed', async () => {
- stubComments('saveDraft').returns(
- Promise.reject(new Error('saving failed'))
- );
+ sinon
+ .stub(element.getCommentsModel(), 'saveDraft')
+ .returns(Promise.reject(new Error('saving failed')));
element.comment = createDraft();
element.editing = true;
@@ -500,7 +501,9 @@
test('discard', async () => {
const discardPromise = mockPromise<void>();
- const stub = stubComments('discardDraft').returns(discardPromise);
+ const stub = sinon
+ .stub(element.getCommentsModel(), 'discardDraft')
+ .returns(discardPromise);
element.comment = createDraft();
element.editing = true;
@@ -522,7 +525,7 @@
});
test('resolved comment state indicated by checkbox', async () => {
- const saveStub = stubComments('saveDraft');
+ const saveStub = sinon.stub(element.getCommentsModel(), 'saveDraft');
element.comment = {
...createComment(),
__draft: true,
@@ -546,8 +549,11 @@
});
test('saving empty text calls discard()', async () => {
- const saveStub = stubComments('saveDraft');
- const discardStub = stubComments('discardDraft');
+ const saveStub = sinon.stub(element.getCommentsModel(), 'saveDraft');
+ const discardStub = sinon.stub(
+ element.getCommentsModel(),
+ 'discardDraft'
+ );
element.comment = createDraft();
element.editing = true;
await element.updateComplete;
@@ -619,7 +625,9 @@
setup(async () => {
clock = sinon.useFakeTimers();
savePromise = mockPromise<DraftInfo>();
- saveStub = stubComments('saveDraft').returns(savePromise);
+ saveStub = sinon
+ .stub(element.getCommentsModel(), 'saveDraft')
+ .returns(savePromise);
element.comment = createUnsaved();
element.editing = true;
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 c4658ec..7d31062 100644
--- a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
+++ b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
@@ -78,9 +78,6 @@
changeModel: (_ctx: Partial<AppContext>) => {
throw new Error('changeModel 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 fe325c5..14efe4c 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -27,7 +27,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 {CommentsModel} from './comments/comments-model';
+import {CommentsModel, commentsModelToken} from './comments/comments-model';
import {RouterModel} from './router/router-model';
import {ShortcutsService} from './shortcuts/shortcuts-service';
import {assertIsDefined} from '../utils/common-util';
@@ -63,22 +63,6 @@
assertIsDefined(restApiService, 'restApiService');
return new ChangeModel(routerModel, restApiService);
},
- commentsModel: (ctx: Partial<AppContext>) => {
- const routerModel = ctx.routerModel;
- const changeModel = ctx.changeModel;
- const restApiService = ctx.restApiService;
- const reportingService = ctx.reportingService;
- assertIsDefined(routerModel, 'routerModel');
- assertIsDefined(changeModel, 'changeModel');
- assertIsDefined(restApiService, 'restApiService');
- assertIsDefined(reportingService, 'reportingService');
- return new CommentsModel(
- routerModel,
- changeModel,
- restApiService,
- reportingService
- );
- },
checksModel: (ctx: Partial<AppContext>) => {
const routerModel = ctx.routerModel;
const changeModel = ctx.changeModel;
@@ -121,5 +105,13 @@
const browserModel = new BrowserModel(appContext.userModel!);
dependencies.set(browserModelToken, browserModel);
+ const commentsModel = new CommentsModel(
+ appContext.routerModel,
+ appContext.changeModel,
+ appContext.restApiService,
+ appContext.reportingService
+ );
+ dependencies.set(commentsModelToken, commentsModel);
+
return dependencies;
}
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index b5aadb7..2e02ebf 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -25,7 +25,6 @@
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 {CommentsModel} from './comments/comments-model';
import {RouterModel} from './router/router-model';
import {ShortcutsService} from './shortcuts/shortcuts-service';
import {ConfigModel} from './config/config-model';
@@ -38,7 +37,6 @@
authService: AuthService;
restApiService: RestApiService;
changeModel: ChangeModel;
- 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 a8d47f1..8e860f7 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -39,6 +39,7 @@
import {select} from '../../utils/observable-util';
import {RouterModel} from '../router/router-model';
import {Finalizable} from '../registry';
+import {define} from '../dependency';
import {combineLatest, Subscription} from 'rxjs';
import {fire, fireAlert, fireEvent} from '../../utils/event-util';
import {CURRENT} from '../../utils/patch-set-util';
@@ -220,6 +221,7 @@
return setDiscardedDraft(nextState, discardedDraft);
}
+export const commentsModelToken = define<CommentsModel>('comments-model');
export class CommentsModel implements Finalizable {
private readonly privateState$: BehaviorSubject<CommentState> =
new BehaviorSubject(initialState);
diff --git a/polygerrit-ui/app/services/dependency.ts b/polygerrit-ui/app/services/dependency.ts
index d49f2ef..e7ac242c 100644
--- a/polygerrit-ui/app/services/dependency.ts
+++ b/polygerrit-ui/app/services/dependency.ts
@@ -177,10 +177,10 @@
}
override disconnectedCallback() {
+ super.disconnectedCallback();
for (const c of this.___controllers) {
c.hostDisconnected?.();
}
- super.disconnectedCallback();
}
addController(controller: ReactiveController) {
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index 8714575..19cb790 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -26,6 +26,7 @@
import {
createTestAppContext,
createTestDependencies,
+ Creator,
} from './test-app-context-init';
import {_testOnly_resetPluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
import {_testOnlyResetGrRestApiSharedObjects} from '../elements/shared/gr-rest-api-interface/gr-rest-api-impl';
@@ -114,16 +115,21 @@
function injectDependency<T>(
dependency: DependencyToken<T>,
- service: T & Finalizable
+ creator: Creator<T>
) {
- injectedDependencies.set(dependency, () => service);
- finalizers.push(service);
+ let service: (T & Finalizable) | undefined = undefined;
+ injectedDependencies.set(dependency, () => {
+ if (service) return service;
+ service = creator();
+ finalizers.push(service);
+ return service;
+ });
}
function resolveDependency(evt: DependencyRequestEvent<unknown>) {
- const service = injectedDependencies.get(evt.dependency);
- if (service) {
- evt.callback(service());
+ const provider = injectedDependencies.get(evt.dependency);
+ if (provider) {
+ evt.callback(provider());
} else {
throw new DependencyError(
evt.dependency,
@@ -143,8 +149,8 @@
injectAppContext(appContext);
finalizers.push(appContext);
const dependencies = createTestDependencies(appContext);
- for (const [token, service] of dependencies) {
- injectDependency(token, service);
+ for (const [token, provider] of dependencies) {
+ injectDependency(token, provider);
}
document.addEventListener('request-dependency', resolveDependency);
// The following calls is nessecary to avoid influence of previously executed
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 4e22e44..491a891 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -30,7 +30,10 @@
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 {CommentsModel} from '../services/comments/comments-model';
+import {
+ CommentsModel,
+ commentsModelToken,
+} from '../services/comments/comments-model';
import {RouterModel} from '../services/router/router-model';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
import {ConfigModel} from '../services/config/config-model';
@@ -58,22 +61,6 @@
assertIsDefined(restApiService, 'restApiService');
return new ChangeModel(routerModel, restApiService);
},
- commentsModel: (ctx: Partial<AppContext>) => {
- const routerModel = ctx.routerModel;
- const changeModel = ctx.changeModel;
- const restApiService = ctx.restApiService;
- const reportingService = ctx.reportingService;
- assertIsDefined(routerModel, 'routerModel');
- assertIsDefined(changeModel, 'changeModel');
- assertIsDefined(restApiService, 'restApiService');
- assertIsDefined(reportingService, 'reportingService');
- return new CommentsModel(
- routerModel,
- changeModel,
- restApiService,
- reportingService
- );
- },
checksModel: (ctx: Partial<AppContext>) => {
const routerModel = ctx.routerModel;
const changeModel = ctx.changeModel;
@@ -106,12 +93,27 @@
return create<AppContext>(appRegistry);
}
+export type Creator<T> = () => T & Finalizable;
+
+// Test dependencies are provides as creator functions to ensure that they are
+// not created if a test doesn't depend on them. E.g. don't create a
+// change-model in change-model_test.ts because it creates one in the test
+// after setting up stubs.
export function createTestDependencies(
appContext: AppContext
-): Map<DependencyToken<unknown>, Finalizable> {
+): Map<DependencyToken<unknown>, Creator<unknown>> {
const dependencies = new Map();
- const browserModel = new BrowserModel(appContext.userModel!);
+ const browserModel = () => new BrowserModel(appContext.userModel!);
dependencies.set(browserModelToken, browserModel);
+ const commentsModel = () =>
+ new CommentsModel(
+ appContext.routerModel,
+ appContext.changeModel,
+ appContext.restApiService,
+ appContext.reportingService
+ );
+ dependencies.set(commentsModelToken, commentsModel);
+
return dependencies;
}
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index a40f14a..7652039 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -23,7 +23,6 @@
import {StorageService} from '../services/storage/gr-storage';
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
-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,10 +111,6 @@
return sinon.spy(getAppContext().restApiService, 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) {
return sinon.stub(getAppContext().userModel, method);
}
