Migrate change-model to new pattern
- Rename service to model on appContext
- Merge ...-model.ts and ...-service.ts
- Rename ...Servcie to ...Model
- Move all observables onto ...Model
- Inject ...Model in the models and components that were
directly accessing the observables
Google-Bug-Id: b/206459178
Change-Id: I5e719ba747f8b6d67948f1efee53a9a1bcb05d34
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index e635613..83808af 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -386,7 +386,7 @@
// Accessed in tests
readonly jsAPI = getAppContext().jsApiService;
- private readonly changeService = getAppContext().changeService;
+ private readonly changeModel = getAppContext().changeModel;
@property({type: Object})
change?: ChangeViewChangeInfo;
@@ -1716,7 +1716,7 @@
new Error('Properties change and changeNum must be set.')
);
}
- return this.changeService.fetchChangeUpdates(change).then(result => {
+ return this.changeModel.fetchChangeUpdates(change).then(result => {
if (!result.isLatest) {
this.dispatchEvent(
new CustomEvent<ShowAlertEventDetail>('show-alert', {
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 a0b6bfd..e397117 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
@@ -196,11 +196,7 @@
hasAttention,
} from '../../../utils/attention-set-util';
import {listen} from '../../../services/shortcuts/shortcuts-service';
-import {
- change$,
- changeLoadingStatus$,
- LoadingStatus,
-} from '../../../services/change/change-model';
+import {LoadingStatus} from '../../../services/change/change-model';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -277,15 +273,6 @@
* @event show-auth-required
*/
- // Accessed in tests.
- readonly reporting = getAppContext().reportingService;
-
- readonly jsAPI = getAppContext().jsApiService;
-
- private readonly changeService = getAppContext().changeService;
-
- private readonly checksModel = getAppContext().checksModel;
-
/**
* URL params passed from the router.
*/
@@ -556,15 +543,6 @@
})
resolveWeblinks?: GeneratedWebLink[];
- readonly restApiService = getAppContext().restApiService;
-
- // Private but used in tests.
- readonly userModel = getAppContext().userModel;
-
- private readonly commentsModel = getAppContext().commentsModel;
-
- private readonly shortcuts = getAppContext().shortcutsService;
-
override keyboardShortcuts(): ShortcutListener[] {
return [
listen(Shortcut.SEND_REPLY, _ => {}), // docOnly
@@ -611,6 +589,25 @@
];
}
+ // Accessed in tests.
+ readonly reporting = getAppContext().reportingService;
+
+ readonly jsAPI = getAppContext().jsApiService;
+
+ private readonly checksModel = getAppContext().checksModel;
+
+ readonly restApiService = getAppContext().restApiService;
+
+ // Private but used in tests.
+ readonly userModel = getAppContext().userModel;
+
+ // Private but used in tests.
+ readonly changeModel = getAppContext().changeModel;
+
+ private readonly commentsModel = getAppContext().commentsModel;
+
+ private readonly shortcuts = getAppContext().shortcutsService;
+
private subscriptions: Subscription[] = [];
private replyRefitTask?: DelayedTask;
@@ -655,7 +652,7 @@
})
);
this.subscriptions.push(
- change$.subscribe(change => {
+ 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;
@@ -1877,7 +1874,7 @@
}
const detailCompletes = until(
- changeLoadingStatus$,
+ this.changeModel.changeLoadingStatus$,
status => status === LoadingStatus.LOADED
);
const editCompletes = this._getEdit();
@@ -2326,7 +2323,7 @@
return;
}
const change = this._change;
- this.changeService.fetchChangeUpdates(change).then(result => {
+ this.changeModel.fetchChangeUpdates(change).then(result => {
let toastMessage = null;
if (!result.isLatest) {
toastMessage = ReloadToastMessage.NEWER_REVISION;
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 96e1796..0e74cc6 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
@@ -103,10 +103,7 @@
import {ParsedChangeInfo} from '../../../types/types';
import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
-import {
- LoadingStatus,
- _testOnly_setState as setChangeState,
-} from '../../../services/change/change-model';
+import {LoadingStatus} from '../../../services/change/change-model';
import {FocusTarget, GrReplyDialog} from '../gr-reply-dialog/gr-reply-dialog';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
@@ -1486,7 +1483,7 @@
test('topic is coalesced to null', async () => {
sinon.stub(element, '_changeChanged');
- setChangeState({
+ element.changeModel.setState({
loadingStatus: LoadingStatus.LOADED,
change: {
...createChangeViewChange(),
@@ -1502,7 +1499,7 @@
test('commit sha is populated from getChangeDetail', async () => {
sinon.stub(element, '_changeChanged');
- setChangeState({
+ element.changeModel.setState({
loadingStatus: LoadingStatus.LOADED,
change: {
...createChangeViewChange(),
@@ -1519,7 +1516,7 @@
test('edit is added to change', () => {
sinon.stub(element, '_changeChanged');
const changeRevision = createRevision();
- setChangeState({
+ element.changeModel.setState({
loadingStatus: LoadingStatus.LOADED,
change: {
...createChangeViewChange(),
@@ -1940,7 +1937,7 @@
test('_selectedRevision updates when patchNum is changed', () => {
const revision1: RevisionInfo = createRevision(1);
const revision2: RevisionInfo = createRevision(2);
- setChangeState({
+ element.changeModel.setState({
loadingStatus: LoadingStatus.LOADED,
change: {
...createChangeViewChange(),
@@ -1971,7 +1968,7 @@
const revision1 = createRevision(1);
const revision2 = createRevision(2);
const revision3 = createEditRevision();
- setChangeState({
+ element.changeModel.setState({
loadingStatus: LoadingStatus.LOADED,
change: {
...createChangeViewChange(),
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 6fa11be..c24e054 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
@@ -29,7 +29,6 @@
import {customElement, property, query, state} from 'lit/decorators';
import {fontStyles} from '../../../styles/gr-font-styles';
import {subscribe} from '../../lit/subscription-controller';
-import {change$} from '../../../services/change/change-model';
import {getAppContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
@@ -64,6 +63,8 @@
private commentsModel = getAppContext().commentsModel;
+ private changeModel = getAppContext().changeModel;
+
static override get styles() {
return [
sharedStyles,
@@ -92,7 +93,7 @@
constructor() {
super();
- subscribe(this, change$, x => (this.change = x));
+ subscribe(this, this.changeModel.change$, x => (this.change = x));
subscribe(
this,
this.commentsModel.threads$,
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 baaecc8..36d0ce6 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,11 +51,6 @@
FormattedReviewerUpdateInfo,
ParsedChangeInfo,
} from '../../../types/types';
-import {
- change$,
- changeNum$,
- repo$,
-} from '../../../services/change/change-model';
/**
* The content of the enum is also used in the UI for the button text.
@@ -259,6 +254,8 @@
// Private but used in tests.
readonly commentsModel = getAppContext().commentsModel;
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly reporting = getAppContext().reportingService;
private readonly shortcuts = getAppContext().shortcutsService;
@@ -273,7 +270,7 @@
})
);
this.subscriptions.push(
- change$.subscribe(x => {
+ this.changeModel.change$.subscribe(x => {
this.change = x;
})
);
@@ -283,12 +280,12 @@
})
);
this.subscriptions.push(
- repo$.subscribe(x => {
+ this.changeModel.repo$.subscribe(x => {
this.projectName = x;
})
);
this.subscriptions.push(
- changeNum$.subscribe(x => {
+ this.changeModel.changeNum$.subscribe(x => {
this.changeNum = x;
})
);
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index dca0d65..52a5dac 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -218,7 +218,7 @@
private readonly reporting = getAppContext().reportingService;
- private readonly changeService = getAppContext().changeService;
+ private readonly changeModel = getAppContext().changeModel;
@property({type: Object})
change?: ChangeInfo;
@@ -435,7 +435,7 @@
open(focusTarget?: FocusTarget, quote?: string) {
assertIsDefined(this.change, 'change');
this.knownLatestState = LatestPatchState.CHECKING;
- this.changeService.fetchChangeUpdates(this.change).then(result => {
+ this.changeModel.fetchChangeUpdates(this.change).then(result => {
this.knownLatestState = result.isLatest
? LatestPatchState.LATEST
: LatestPatchState.NOT_LATEST;
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 41e2fc4e..0349b3f 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -43,7 +43,6 @@
import {customElement, property, queryAll, state} from 'lit/decorators';
import {sharedStyles} from '../../../styles/shared-styles';
import {subscribe} from '../../lit/subscription-controller';
-import {change$, changeNum$} from '../../../services/change/change-model';
import {ParsedChangeInfo} from '../../../types/types';
import {repeat} from 'lit/directives/repeat';
import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
@@ -200,12 +199,14 @@
@state()
draftsOnly = false;
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly userModel = getAppContext().userModel;
constructor() {
super();
- subscribe(this, changeNum$, x => (this.changeNum = x));
- subscribe(this, change$, x => (this.change = x));
+ subscribe(this, this.changeModel.changeNum$, x => (this.changeNum = x));
+ subscribe(this, this.changeModel.change$, x => (this.change = x));
subscribe(this, this.userModel.account$, x => (this.account = x));
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 24c06e0..de2099e 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -55,7 +55,6 @@
LabelNameToInfoMap,
PatchSetNumber,
} from '../../types/common';
-import {labels$, latestPatchNum$} from '../../services/change/change-model';
import {getAppContext} from '../../services/app-context';
import {spinnerStyles} from '../../styles/gr-spinner-styles';
import {
@@ -88,11 +87,13 @@
@state()
labels?: LabelNameToInfoMap;
+ private changeModel = getAppContext().changeModel;
+
private checksModel = getAppContext().checksModel;
constructor() {
super();
- subscribe(this, labels$, x => (this.labels = x));
+ subscribe(this, this.changeModel.labels$, x => (this.labels = x));
}
static override get styles() {
@@ -530,7 +531,7 @@
@state()
repoConfig?: ConfigInfo;
- private changeService = getAppContext().changeService;
+ private changeModel = getAppContext().changeModel;
private configModel = getAppContext().configModel;
@@ -618,7 +619,7 @@
const end = pointer?.range?.end_line;
if (start) rangeText += `#${start}`;
if (end && start !== end) rangeText += `-${end}`;
- const change = this.changeService.getChange();
+ const change = this.changeModel.getChange();
assertIsDefined(change);
const path = pointer.path;
const patchset = this.result?.patchset as PatchSetNumber | undefined;
@@ -726,6 +727,8 @@
*/
private isSectionExpandedByUser = new Map<Category, boolean>();
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly checksModel = getAppContext().checksModel;
constructor() {
@@ -745,7 +748,11 @@
this.checksModel.checksSelectedPatchsetNumber$,
x => (this.checksPatchsetNumber = x)
);
- subscribe(this, latestPatchNum$, x => (this.latestPatchsetNumber = x));
+ subscribe(
+ this,
+ this.changeModel.latestPatchNum$,
+ x => (this.latestPatchsetNumber = x)
+ );
subscribe(
this,
this.checksModel.someProvidersAreLoadingSelected$,
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index 16c9bfe..a9c30c5 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -20,7 +20,6 @@
import {CheckResult, CheckRun} from '../../services/checks/checks-model';
import './gr-checks-runs';
import './gr-checks-results';
-import {changeNum$, latestPatchNum$} from '../../services/change/change-model';
import {NumericChangeId, PatchSetNumber} from '../../types/common';
import {ActionTriggeredEvent} from '../../services/checks/checks-util';
import {AttemptSelectedEvent, RunSelectedEvent} from './gr-checks-util';
@@ -62,6 +61,8 @@
number | undefined
>();
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly checksModel = getAppContext().checksModel;
constructor() {
@@ -81,8 +82,12 @@
this.checksModel.checksSelectedPatchsetNumber$,
x => (this.checksPatchsetNumber = x)
);
- subscribe(this, latestPatchNum$, x => (this.latestPatchsetNumber = x));
- subscribe(this, changeNum$, x => (this.changeNum = x));
+ subscribe(
+ this,
+ this.changeModel.latestPatchNum$,
+ x => (this.latestPatchsetNumber = x)
+ );
+ subscribe(this, this.changeModel.changeNum$, x => (this.changeNum = x));
this.addEventListener('action-triggered', (e: ActionTriggeredEvent) =>
this.handleActionTriggered(e.detail.action, e.detail.run)
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 98e4312..e527134 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
@@ -116,13 +116,7 @@
import {filter, take} from 'rxjs/operators';
import {Subscription, combineLatest} from 'rxjs';
import {listen} from '../../../services/shortcuts/shortcuts-service';
-import {
- diffPath$,
- currentPatchNum$,
- change$,
- changeLoadingStatus$,
- LoadingStatus,
-} from '../../../services/change/change-model';
+import {LoadingStatus} from '../../../services/change/change-model';
import {DisplayLine} from '../../../api/diff';
import {GrDownloadDialog} from '../../change/gr-download-dialog/gr-download-dialog';
@@ -364,7 +358,8 @@
// Private but used in tests.
readonly userModel = getAppContext().userModel;
- private readonly changeService = getAppContext().changeService;
+ // Private but used in tests.
+ readonly changeModel = getAppContext().changeModel;
// Private but used in tests.
readonly browserModel = getAppContext().browserModel;
@@ -411,7 +406,7 @@
})
);
this.subscriptions.push(
- change$.subscribe(change => {
+ this.changeModel.change$.subscribe(change => {
// The diff view is tied to a specfic change number, so don't update
// _change to undefined.
if (change) this._change = change;
@@ -423,12 +418,12 @@
// properties since the method will be called anytime a property updates
// but we only want to call this on the initial load.
this.subscriptions.push(
- combineLatest(
- currentPatchNum$,
+ combineLatest([
+ this.changeModel.currentPatchNum$,
routerView$,
- diffPath$,
- this.userModel.diffPreferences$
- )
+ this.changeModel.diffPath$,
+ this.userModel.diffPreferences$,
+ ])
.pipe(
filter(
([currentPatchNum, routerView, path, diffPrefs]) =>
@@ -443,7 +438,9 @@
this.setReviewedStatus(currentPatchNum!, path!, diffPrefs);
})
);
- this.subscriptions.push(diffPath$.subscribe(path => (this._path = path)));
+ this.subscriptions.push(
+ this.changeModel.diffPath$.subscribe(path => (this._path = path))
+ );
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.cursor.replaceDiffs([this.$.diffHost]);
this._onRenderHandler = (_: Event) => {
@@ -1059,7 +1056,7 @@
GerritNav.navigateToChange(this._change);
return;
}
- this.changeService.updatePath(comment.path);
+ this.changeModel.updatePath(comment.path);
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
if (!latestPatchNum) throw new Error('Missing _allPatchSets');
@@ -1069,7 +1066,7 @@
this._focusLineNum = comment.line;
} else {
if (this.params.path) {
- this.changeService.updatePath(this.params.path);
+ this.changeModel.updatePath(this.params.path);
}
if (this.params.patchNum) {
this._patchRange = {
@@ -1135,7 +1132,7 @@
}
this._files = {sortedFileList: [], changeFilesByPath: {}};
- this.changeService.updatePath(undefined);
+ this.changeModel.updatePath(undefined);
this._patchRange = undefined;
this._commitRange = undefined;
this._focusLineNum = undefined;
@@ -1159,10 +1156,14 @@
}
const promises: Promise<unknown>[] = [];
- if (!this._change)
+ if (!this._change) {
promises.push(
- until(changeLoadingStatus$, status => status === LoadingStatus.LOADED)
+ until(
+ this.changeModel.changeLoadingStatus$,
+ status => status === LoadingStatus.LOADED
+ )
);
+ }
promises.push(until(this.commentsModel.commentsLoading$, isFalse));
promises.push(
this._getChangeEdit().then(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 3fad375..4c4361d 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
@@ -31,7 +31,6 @@
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
import {Side} from '../../../api/diff.js';
-import {_testOnly_setState as setChangeModelState} from '../../../services/change/change-model.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -138,10 +137,11 @@
sinon.stub(element.reporting, 'diffViewDisplayed');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.spy(element, '_paramsChanged');
- setChangeModelState({change: {
- ...createChange(),
- revisions: createRevisions(11),
- }});
+ element.changeModel.setState({
+ change: {
+ ...createChange(),
+ revisions: createRevisions(11),
+ }});
});
test('comment url resolves to comment.patch_set vs latest', () => {
@@ -247,10 +247,11 @@
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, '_isFileUnchanged').returns(true);
sinon.spy(element, '_paramsChanged');
- setChangeModelState({change: {
- ...createChange(),
- revisions: createRevisions(11),
- }});
+ element.changeModel.setState({
+ change: {
+ ...createChange(),
+ revisions: createRevisions(11),
+ }});
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
@@ -299,10 +300,11 @@
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, '_isFileUnchanged').returns(true);
sinon.spy(element, '_paramsChanged');
- setChangeModelState({change: {
- ...createChange(),
- revisions: createRevisions(11),
- }});
+ element.changeModel.setState({
+ change: {
+ ...createChange(),
+ revisions: createRevisions(11),
+ }});
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
@@ -380,10 +382,11 @@
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.spy(element, '_paramsChanged');
element._change = undefined;
- setChangeModelState({change: {
- ...createChange(),
- revisions: createRevisions(11),
- }});
+ element.changeModel.setState({
+ change: {
+ ...createChange(),
+ revisions: createRevisions(11),
+ }});
element._patchRange = {
patchNum: 2,
basePatchNum: 1,
@@ -1199,7 +1202,9 @@
manual_review: true,
};
element.userModel.setDiffPreferences(diffPreferences);
- setChangeModelState({change: createChange(), diffPath: '/COMMIT_MSG'});
+ element.changeModel.setState({
+ change: createChange(),
+ diffPath: '/COMMIT_MSG'});
setRouterModelState({
changeNum: TEST_NUMERIC_CHANGE_ID, view: GerritView.DIFF, patchNum: 2}
@@ -1236,7 +1241,8 @@
manual_review: false,
};
element.userModel.setDiffPreferences(diffPreferences);
- setChangeModelState({change: createChange(),
+ element.changeModel.setState({
+ change: createChange(),
diffPath: '/COMMIT_MSG'});
setRouterModelState({
@@ -1261,7 +1267,9 @@
sinon.stub(element.$.diffHost, 'reload');
element.userModel.setDiffPreferences(createDefaultDiffPrefs());
- setChangeModelState({change: createChange(), diffPath: '/COMMIT_MSG'});
+ element.changeModel.setState({
+ change: createChange(),
+ diffPath: '/COMMIT_MSG'});
setRouterModelState({
changeNum: TEST_NUMERIC_CHANGE_ID, view: GerritView.DIFF, patchNum: 2}
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 debd9c0..de8a02d 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
@@ -67,7 +67,6 @@
import {subscribe} from '../../lit/subscription-controller';
import {repeat} from 'lit/directives/repeat';
import {classMap} from 'lit/directives/class-map';
-import {changeNum$, repo$} from '../../../services/change/change-model';
import {ShortcutController} from '../../lit/shortcut-controller';
import {ValueChangedEvent} from '../../../types/events';
@@ -238,6 +237,8 @@
private readonly commentsModel = getAppContext().commentsModel;
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly userModel = getAppContext().userModel;
private readonly shortcuts = new ShortcutController(this);
@@ -246,9 +247,9 @@
constructor() {
super();
- subscribe(this, changeNum$, x => (this.changeNum = x));
+ subscribe(this, this.changeModel.changeNum$, x => (this.changeNum = x));
subscribe(this, this.userModel.account$, x => (this.account = x));
- subscribe(this, repo$, x => (this.repoName = x));
+ subscribe(this, this.changeModel.repo$, x => (this.repoName = x));
subscribe(this, this.userModel.diffPreferences$, x =>
this.syntaxLayer.setEnabled(!!x.syntax_highlighting)
);
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 af2966c..1411c88 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -58,7 +58,6 @@
import {subscribe} from '../../lit/subscription-controller';
import {ShortcutController} from '../../lit/shortcut-controller';
import {classMap} from 'lit/directives/class-map';
-import {changeNum$, repo$} from '../../../services/change/change-model';
import {LineNumber} from '../../../api/diff';
import {CommentSide} from '../../../constants/constants';
import {getRandomInt} from '../../../utils/math-util';
@@ -228,6 +227,8 @@
private readonly reporting = getAppContext().reportingService;
+ private readonly changeModel = getAppContext().changeModel;
+
private readonly commentsModel = getAppContext().commentsModel;
private readonly userModel = getAppContext().userModel;
@@ -263,8 +264,8 @@
this.configModel.repoCommentLinks$,
x => (this.commentLinks = x)
);
- subscribe(this, repo$, x => (this.repoName = x));
- subscribe(this, changeNum$, x => (this.changeNum = x));
+ subscribe(this, this.changeModel.repo$, x => (this.repoName = x));
+ subscribe(this, this.changeModel.changeNum$, x => (this.changeNum = x));
subscribe(
this,
this.autoSaveTrigger$.pipe(debounceTime(AUTO_SAVE_DEBOUNCE_DELAY_MS)),
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 b865a7c..9b3e75d 100644
--- a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
+++ b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
@@ -75,8 +75,8 @@
restApiService: (_ctx: Partial<AppContext>) => {
throw new Error('restApiService is not implemented');
},
- changeService: (_ctx: Partial<AppContext>) => {
- throw new Error('changeService is not implemented');
+ changeModel: (_ctx: Partial<AppContext>) => {
+ throw new Error('changeModel is not implemented');
},
commentsModel: (_ctx: Partial<AppContext>) => {
throw new Error('commentsModel is not implemented');
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index f958429..46d2178 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -21,7 +21,7 @@
import {EventEmitter} from './gr-event-interface/gr-event-interface_impl';
import {Auth} from './gr-auth/gr-auth_impl';
import {GrRestApiServiceImpl} from '../elements/shared/gr-rest-api-interface/gr-rest-api-impl';
-import {ChangeService} from './change/change-service';
+import {ChangeModel} from './change/change-model';
import {ChecksModel} from './checks/checks-model';
import {GrJsApiInterface} from '../elements/shared/gr-js-api-interface/gr-js-api-interface-element';
import {GrStorageService} from './storage/gr-storage_impl';
@@ -53,20 +53,25 @@
assertIsDefined(ctx.flagsService, 'flagsService)');
return new GrRestApiServiceImpl(ctx.authService!, ctx.flagsService!);
},
- changeService: (ctx: Partial<AppContext>) => {
+ changeModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.restApiService, 'restApiService');
- return new ChangeService(ctx.restApiService!);
+ return new ChangeModel(ctx.restApiService!);
},
commentsModel: (ctx: Partial<AppContext>) => {
- const restApi = ctx.restApiService;
+ const changeModel = ctx.changeModel;
+ const restApiService = ctx.restApiService;
const reporting = ctx.reportingService;
- assertIsDefined(restApi, 'restApiService');
+ assertIsDefined(changeModel, 'changeModel');
+ assertIsDefined(restApiService, 'restApiService');
assertIsDefined(reporting, 'reportingService');
- return new CommentsModel(restApi, reporting);
+ return new CommentsModel(changeModel, restApiService, reporting);
},
checksModel: (ctx: Partial<AppContext>) => {
- assertIsDefined(ctx.reportingService, 'reportingService');
- return new ChecksModel(ctx.reportingService!);
+ const changeModel = ctx.changeModel;
+ const reporting = ctx.reportingService;
+ assertIsDefined(changeModel, 'changeModel');
+ assertIsDefined(reporting, 'reportingService');
+ return new ChecksModel(changeModel, reporting);
},
jsApiService: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.reportingService, 'reportingService');
@@ -74,8 +79,11 @@
},
storageService: (_ctx: Partial<AppContext>) => new GrStorageService(),
configModel: (ctx: Partial<AppContext>) => {
- assertIsDefined(ctx.restApiService, 'restApiService');
- return new ConfigModel(ctx.restApiService!);
+ const changeModel = ctx.changeModel;
+ const restApiService = ctx.restApiService;
+ assertIsDefined(changeModel, 'changeModel');
+ assertIsDefined(restApiService, 'restApiService');
+ return new ConfigModel(changeModel, restApiService);
},
userModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.restApiService, 'restApiService');
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index 3439a3d..bea371f 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -20,7 +20,7 @@
import {ReportingService} from './gr-reporting/gr-reporting';
import {AuthService} from './gr-auth/gr-auth';
import {RestApiService} from './gr-rest-api/gr-rest-api';
-import {ChangeService} from './change/change-service';
+import {ChangeModel} from './change/change-model';
import {ChecksModel} from './checks/checks-model';
import {JsApiService} from '../elements/shared/gr-js-api-interface/gr-js-api-types';
import {StorageService} from './storage/gr-storage';
@@ -36,7 +36,7 @@
eventEmitter: EventEmitterService;
authService: AuthService;
restApiService: RestApiService;
- changeService: ChangeService;
+ changeModel: ChangeModel;
commentsModel: CommentsModel;
checksModel: ChecksModel;
jsApiService: JsApiService;
diff --git a/polygerrit-ui/app/services/change/change-model.ts b/polygerrit-ui/app/services/change/change-model.ts
index 2c98320..6a819f8 100644
--- a/polygerrit-ui/app/services/change/change-model.ts
+++ b/polygerrit-ui/app/services/change/change-model.ts
@@ -16,12 +16,21 @@
*/
import {NumericChangeId, PatchSetNum} from '../../types/common';
-import {BehaviorSubject, combineLatest, Observable} from 'rxjs';
+import {
+ combineLatest,
+ from,
+ fromEvent,
+ BehaviorSubject,
+ Observable,
+ Subscription,
+} from 'rxjs';
import {
map,
filter,
withLatestFrom,
distinctUntilChanged,
+ startWith,
+ switchMap,
} from 'rxjs/operators';
import {routerPatchNum$, routerState$} from '../router/router-model';
import {
@@ -30,6 +39,12 @@
} from '../../utils/patch-set-util';
import {ParsedChangeInfo} from '../../types/types';
+import {routerChangeNum$} from '../router/router-model';
+import {ChangeInfo} from '../../types/common';
+import {RestApiService} from '../gr-rest-api/gr-rest-api';
+import {Finalizable} from '../registry';
+import {select} from '../../utils/observable-util';
+
export enum LoadingStatus {
NOT_LOADED = 'NOT_LOADED',
LOADING = 'LOADING',
@@ -58,125 +73,189 @@
loadingStatus: LoadingStatus.NOT_LOADED,
};
-const privateState$ = new BehaviorSubject(initialState);
+export class ChangeModel implements Finalizable {
+ private readonly privateState$ = new BehaviorSubject(initialState);
-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});
-}
+ public readonly changeState$: Observable<ChangeState> =
+ this.privateState$.asObservable();
-export function _testOnly_setState(state: ChangeState) {
- privateState$.next(state);
-}
-
-export function _testOnly_getState() {
- return privateState$.getValue();
-}
-
-// Re-exporting as Observable so that you can only subscribe, but not emit.
-export const changeState$: Observable<ChangeState> = privateState$;
-
-export function updateStateChange(change?: ParsedChangeInfo) {
- const current = privateState$.getValue();
- privateState$.next({
- ...current,
- change,
- loadingStatus:
- change === undefined ? LoadingStatus.NOT_LOADED : LoadingStatus.LOADED,
- });
-}
-
-/**
- * Called when change detail loading is initiated.
- *
- * If the change number matches the current change in the state, then
- * this is a reload. If not, then we not just want to set the state to
- * LOADING instead of RELOADING, but we also want to set the change to
- * undefined right away. Otherwise components could see inconsistent state:
- * a new change number, but an old change.
- */
-export function updateStateLoading(changeNum: NumericChangeId) {
- const current = privateState$.getValue();
- const reloading = current.change?._number === changeNum;
- privateState$.next({
- ...current,
- change: reloading ? current.change : undefined,
- loadingStatus: reloading ? LoadingStatus.RELOADING : LoadingStatus.LOADING,
- });
-}
-
-export function updateStatePath(diffPath?: string) {
- const current = privateState$.getValue();
- privateState$.next({...current, diffPath});
-}
-
-/**
- * If you depend on both, router and change state, then you want to filter out
- * inconsistent state, e.g. router changeNum already updated, change not yet
- * reset to undefined.
- */
-export const changeAndRouterConsistent$ = combineLatest([
- routerState$,
- changeState$,
-]).pipe(
- filter(([routerState, changeState]) => {
- const changeNum = changeState.change?._number;
- const routerChangeNum = routerState.changeNum;
- return changeNum === undefined || changeNum === routerChangeNum;
- }),
- distinctUntilChanged()
-);
-
-export const change$ = changeState$.pipe(
- map(changeState => changeState.change),
- distinctUntilChanged()
-);
-
-export const changeLoadingStatus$ = changeState$.pipe(
- map(changeState => changeState.loadingStatus),
- distinctUntilChanged()
-);
-
-export const diffPath$ = changeState$.pipe(
- map(changeState => changeState?.diffPath),
- distinctUntilChanged()
-);
-
-export const changeNum$ = change$.pipe(
- map(change => change?._number),
- distinctUntilChanged()
-);
-
-export const repo$ = change$.pipe(
- map(change => change?.project),
- distinctUntilChanged()
-);
-
-export const labels$ = change$.pipe(
- map(change => change?.labels),
- distinctUntilChanged()
-);
-
-export const latestPatchNum$ = change$.pipe(
- map(change => computeLatestPatchNum(computeAllPatchSets(change))),
- distinctUntilChanged()
-);
-
-/**
- * Emits the current patchset number. If the route does not define the current
- * patchset num, then this selector waits for the change to be defined and
- * returns the number of the latest patchset.
- *
- * Note that this selector can emit a patchNum without the change being
- * available!
- */
-export const currentPatchNum$: Observable<PatchSetNum | undefined> =
- changeAndRouterConsistent$.pipe(
- withLatestFrom(routerPatchNum$, latestPatchNum$),
- map(
- ([_, routerPatchNum, latestPatchNum]) => routerPatchNum || latestPatchNum
- ),
- distinctUntilChanged()
+ public readonly change$ = select(
+ this.privateState$,
+ changeState => changeState.change
);
+
+ public readonly changeLoadingStatus$ = select(
+ this.privateState$,
+ changeState => changeState.loadingStatus
+ );
+
+ public readonly diffPath$ = select(
+ this.privateState$,
+ changeState => changeState?.diffPath
+ );
+
+ public readonly changeNum$ = select(this.change$, change => change?._number);
+
+ public readonly repo$ = select(this.change$, change => change?.project);
+
+ public readonly labels$ = select(this.change$, change => change?.labels);
+
+ public readonly latestPatchNum$ = select(this.change$, change =>
+ computeLatestPatchNum(computeAllPatchSets(change))
+ );
+
+ /**
+ * Emits the current patchset number. If the route does not define the current
+ * patchset num, then this selector waits for the change to be defined and
+ * returns the number of the latest patchset.
+ *
+ * Note that this selector can emit a patchNum without the change being
+ * available!
+ */
+ public readonly currentPatchNum$: Observable<PatchSetNum | undefined> =
+ /**
+ * If you depend on both, router and change state, then you want to filter
+ * out inconsistent state, e.g. router changeNum already updated, change not
+ * yet reset to undefined.
+ */
+ combineLatest([routerState$, this.changeState$])
+ .pipe(
+ filter(([routerState, changeState]) => {
+ const changeNum = changeState.change?._number;
+ const routerChangeNum = routerState.changeNum;
+ return changeNum === undefined || changeNum === routerChangeNum;
+ }),
+ distinctUntilChanged()
+ )
+ .pipe(
+ withLatestFrom(routerPatchNum$, this.latestPatchNum$),
+ map(([_, routerPatchN, latestPatchN]) => routerPatchN || latestPatchN),
+ distinctUntilChanged()
+ );
+
+ private subscriptions: Subscription[] = [];
+
+ // For usage in `combineLatest` we need `startWith` such that reload$ has an
+ // initial value.
+ private readonly reload$: Observable<unknown> = fromEvent(
+ document,
+ 'reload'
+ ).pipe(startWith(undefined));
+
+ constructor(readonly restApiService: RestApiService) {
+ this.subscriptions = [
+ combineLatest([routerChangeNum$, this.reload$])
+ .pipe(
+ map(([changeNum, _]) => changeNum),
+ switchMap(changeNum => {
+ if (changeNum !== undefined) this.updateStateLoading(changeNum);
+ return from(this.restApiService.getChangeDetail(changeNum));
+ })
+ )
+ .subscribe(change => {
+ // The change service is currently a singleton, so we have to be
+ // careful to avoid situations where the application state is
+ // partially set for the old change where the user is coming from,
+ // and partially for the new change where the user is navigating to.
+ // So setting the change explicitly to undefined when the user
+ // moves away from diff and change pages (changeNum === undefined)
+ // helps with that.
+ this.updateStateChange(change ?? undefined);
+ }),
+ ];
+ }
+
+ finalize() {
+ for (const s of this.subscriptions) {
+ s.unsubscribe();
+ }
+ this.subscriptions = [];
+ }
+
+ // Temporary workaround until path is derived in the model itself.
+ updatePath(diffPath?: string) {
+ const current = this.getState();
+ this.setState({...current, diffPath});
+ }
+
+ /**
+ * Typically you would just subscribe to change$ yourself to get updates. But
+ * sometimes it is nice to also be able to get the current ChangeInfo on
+ * demand. So here it is for your convenience.
+ */
+ getChange() {
+ return this.getState().change;
+ }
+
+ /**
+ * Check whether there is no newer patch than the latest patch that was
+ * available when this change was loaded.
+ *
+ * @return A promise that yields true if the latest patch
+ * has been loaded, and false if a newer patch has been uploaded in the
+ * meantime. The promise is rejected on network error.
+ */
+ fetchChangeUpdates(change: ChangeInfo | ParsedChangeInfo) {
+ const knownLatest = computeLatestPatchNum(computeAllPatchSets(change));
+ return this.restApiService.getChangeDetail(change._number).then(detail => {
+ if (!detail) {
+ const error = new Error('Change detail not found.');
+ return Promise.reject(error);
+ }
+ const actualLatest = computeLatestPatchNum(computeAllPatchSets(detail));
+ if (!actualLatest || !knownLatest) {
+ const error = new Error('Unable to check for latest patchset.');
+ return Promise.reject(error);
+ }
+ return {
+ isLatest: actualLatest <= knownLatest,
+ newStatus: change.status !== detail.status ? detail.status : null,
+ newMessages:
+ (change.messages || []).length < (detail.messages || []).length
+ ? detail.messages![detail.messages!.length - 1]
+ : undefined,
+ };
+ });
+ }
+
+ /**
+ * Called when change detail loading is initiated.
+ *
+ * If the change number matches the current change in the state, then
+ * this is a reload. If not, then we not just want to set the state to
+ * LOADING instead of RELOADING, but we also want to set the change to
+ * undefined right away. Otherwise components could see inconsistent state:
+ * a new change number, but an old change.
+ */
+ private updateStateLoading(changeNum: NumericChangeId) {
+ const current = this.getState();
+ const reloading = current.change?._number === changeNum;
+ this.setState({
+ ...current,
+ change: reloading ? current.change : undefined,
+ loadingStatus: reloading
+ ? LoadingStatus.RELOADING
+ : LoadingStatus.LOADING,
+ });
+ }
+
+ // Private but used in tests.
+ updateStateChange(change?: ParsedChangeInfo) {
+ const current = this.getState();
+ this.setState({
+ ...current,
+ change,
+ loadingStatus:
+ change === undefined ? LoadingStatus.NOT_LOADED : LoadingStatus.LOADED,
+ });
+ }
+
+ getState(): ChangeState {
+ return this.privateState$.getValue();
+ }
+
+ // Private but used in tests
+ setState(state: ChangeState) {
+ this.privateState$.next(state);
+ }
+}
diff --git a/polygerrit-ui/app/services/change/change-services_test.ts b/polygerrit-ui/app/services/change/change-model_test.ts
similarity index 86%
rename from polygerrit-ui/app/services/change/change-services_test.ts
rename to polygerrit-ui/app/services/change/change-model_test.ts
index d05df85a..099a11f 100644
--- a/polygerrit-ui/app/services/change/change-services_test.ts
+++ b/polygerrit-ui/app/services/change/change-model_test.ts
@@ -32,15 +32,15 @@
GerritView,
_testOnly_setState as setRouterState,
} from '../router/router-model';
-import {ChangeState, changeState$, LoadingStatus} from './change-model';
-import {ChangeService} from './change-service';
+import {ChangeState, LoadingStatus} from './change-model';
+import {ChangeModel} from './change-model';
suite('change service tests', () => {
- let changeService: ChangeService;
+ let changeModel: ChangeModel;
let knownChange: ParsedChangeInfo;
const testCompleted = new Subject<void>();
setup(() => {
- changeService = new ChangeService(getAppContext().restApiService);
+ changeModel = new ChangeModel(getAppContext().restApiService);
knownChange = {
...createChange(),
revisions: {
@@ -62,8 +62,8 @@
});
teardown(() => {
- changeService.finalize();
testCompleted.next();
+ changeModel.finalize();
});
test('load a change', async () => {
@@ -72,7 +72,9 @@
let state: ChangeState | undefined = {
loadingStatus: LoadingStatus.NOT_LOADED,
};
- changeState$.pipe(takeUntil(testCompleted)).subscribe(s => (state = s));
+ changeModel.changeState$
+ .pipe(takeUntil(testCompleted))
+ .subscribe(s => (state = s));
await waitUntil(() => state?.loadingStatus === LoadingStatus.NOT_LOADED);
assert.equal(stub.callCount, 0);
@@ -96,7 +98,9 @@
let state: ChangeState | undefined = {
loadingStatus: LoadingStatus.NOT_LOADED,
};
- changeState$.pipe(takeUntil(testCompleted)).subscribe(s => (state = s));
+ changeModel.changeState$
+ .pipe(takeUntil(testCompleted))
+ .subscribe(s => (state = s));
setRouterState({view: GerritView.CHANGE, changeNum: knownChange._number});
promise.resolve(knownChange);
await waitUntil(() => state?.loadingStatus === LoadingStatus.LOADED);
@@ -120,7 +124,9 @@
let state: ChangeState | undefined = {
loadingStatus: LoadingStatus.NOT_LOADED,
};
- changeState$.pipe(takeUntil(testCompleted)).subscribe(s => (state = s));
+ changeModel.changeState$
+ .pipe(takeUntil(testCompleted))
+ .subscribe(s => (state = s));
setRouterState({view: GerritView.CHANGE, changeNum: knownChange._number});
promise.resolve(knownChange);
await waitUntil(() => state?.loadingStatus === LoadingStatus.LOADED);
@@ -150,7 +156,9 @@
let state: ChangeState | undefined = {
loadingStatus: LoadingStatus.NOT_LOADED,
};
- changeState$.pipe(takeUntil(testCompleted)).subscribe(s => (state = s));
+ changeModel.changeState$
+ .pipe(takeUntil(testCompleted))
+ .subscribe(s => (state = s));
setRouterState({view: GerritView.CHANGE, changeNum: knownChange._number});
promise.resolve(knownChange);
await waitUntil(() => state?.loadingStatus === LoadingStatus.LOADED);
@@ -174,15 +182,15 @@
assert.equal(state?.change, knownChange);
});
- test('changeService.fetchChangeUpdates on latest', async () => {
+ test('changeModel.fetchChangeUpdates on latest', async () => {
stubRestApi('getChangeDetail').returns(Promise.resolve(knownChange));
- const result = await changeService.fetchChangeUpdates(knownChange);
+ const result = await changeModel.fetchChangeUpdates(knownChange);
assert.isTrue(result.isLatest);
assert.isNotOk(result.newStatus);
assert.isNotOk(result.newMessages);
});
- test('changeService.fetchChangeUpdates not on latest', async () => {
+ test('changeModel.fetchChangeUpdates not on latest', async () => {
const actualChange = {
...knownChange,
revisions: {
@@ -195,31 +203,31 @@
},
};
stubRestApi('getChangeDetail').returns(Promise.resolve(actualChange));
- const result = await changeService.fetchChangeUpdates(knownChange);
+ const result = await changeModel.fetchChangeUpdates(knownChange);
assert.isFalse(result.isLatest);
assert.isNotOk(result.newStatus);
assert.isNotOk(result.newMessages);
});
- test('changeService.fetchChangeUpdates new status', async () => {
+ test('changeModel.fetchChangeUpdates new status', async () => {
const actualChange = {
...knownChange,
status: ChangeStatus.MERGED,
};
stubRestApi('getChangeDetail').returns(Promise.resolve(actualChange));
- const result = await changeService.fetchChangeUpdates(knownChange);
+ const result = await changeModel.fetchChangeUpdates(knownChange);
assert.isTrue(result.isLatest);
assert.equal(result.newStatus, ChangeStatus.MERGED);
assert.isNotOk(result.newMessages);
});
- test('changeService.fetchChangeUpdates new messages', async () => {
+ test('changeModel.fetchChangeUpdates new messages', async () => {
const actualChange = {
...knownChange,
messages: [{...createChangeMessageInfo(), message: 'blah blah'}],
};
stubRestApi('getChangeDetail').returns(Promise.resolve(actualChange));
- const result = await changeService.fetchChangeUpdates(knownChange);
+ const result = await changeModel.fetchChangeUpdates(knownChange);
assert.isTrue(result.isLatest);
assert.isNotOk(result.newStatus);
assert.deepEqual(result.newMessages, {
diff --git a/polygerrit-ui/app/services/change/change-service.ts b/polygerrit-ui/app/services/change/change-service.ts
deleted file mode 100644
index bcc62b2..0000000
--- a/polygerrit-ui/app/services/change/change-service.ts
+++ /dev/null
@@ -1,126 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 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, from, fromEvent, Observable, Subscription} from 'rxjs';
-import {map, startWith, switchMap} from 'rxjs/operators';
-import {routerChangeNum$} from '../router/router-model';
-import {
- change$,
- updateStateChange,
- updateStateLoading,
- updateStatePath,
-} from './change-model';
-import {ParsedChangeInfo} from '../../types/types';
-import {ChangeInfo} from '../../types/common';
-import {
- computeAllPatchSets,
- computeLatestPatchNum,
-} from '../../utils/patch-set-util';
-import {RestApiService} from '../gr-rest-api/gr-rest-api';
-import {Finalizable} from '../registry';
-
-export class ChangeService implements Finalizable {
- private change?: ParsedChangeInfo;
-
- private readonly subscriptions: Subscription[] = [];
-
- // For usage in `combineLatest` we need `startWith` such that reload$ has an
- // initial value.
- private readonly reload$: Observable<unknown> = fromEvent(
- document,
- 'reload'
- ).pipe(startWith(undefined));
-
- constructor(readonly restApiService: RestApiService) {
- this.subscriptions.push(
- combineLatest([routerChangeNum$, this.reload$])
- .pipe(
- map(([changeNum, _]) => changeNum),
- switchMap(changeNum => {
- if (changeNum !== undefined) updateStateLoading(changeNum);
- return from(this.restApiService.getChangeDetail(changeNum));
- })
- )
- .subscribe(change => {
- // The change service is currently a singleton, so we have to be
- // careful to avoid situations where the application state is
- // partially set for the old change where the user is coming from,
- // and partially for the new change where the user is navigating to.
- // So setting the change explicitly to undefined when the user
- // moves away from diff and change pages (changeNum === undefined)
- // helps with that.
- updateStateChange(change ?? undefined);
- })
- );
- this.subscriptions.push(
- change$.subscribe(change => {
- this.change = change;
- })
- );
- }
-
- finalize() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions.splice(0, this.subscriptions.length);
- }
-
- // Temporary workaround until path is derived in the model itself.
- updatePath(path?: string) {
- updateStatePath(path);
- }
-
- /**
- * Typically you would just subscribe to change$ yourself to get updates. But
- * sometimes it is nice to also be able to get the current ChangeInfo on
- * demand. So here it is for your convenience.
- */
- getChange() {
- return this.change;
- }
-
- /**
- * Check whether there is no newer patch than the latest patch that was
- * available when this change was loaded.
- *
- * @return A promise that yields true if the latest patch
- * has been loaded, and false if a newer patch has been uploaded in the
- * meantime. The promise is rejected on network error.
- */
- fetchChangeUpdates(change: ChangeInfo | ParsedChangeInfo) {
- const knownLatest = computeLatestPatchNum(computeAllPatchSets(change));
- return this.restApiService.getChangeDetail(change._number).then(detail => {
- if (!detail) {
- const error = new Error('Change detail not found.');
- return Promise.reject(error);
- }
- const actualLatest = computeLatestPatchNum(computeAllPatchSets(detail));
- if (!actualLatest || !knownLatest) {
- const error = new Error('Unable to check for latest patchset.');
- return Promise.reject(error);
- }
- return {
- isLatest: actualLatest <= knownLatest,
- newStatus: change.status !== detail.status ? detail.status : null,
- newMessages:
- (change.messages || []).length < (detail.messages || []).length
- ? detail.messages![detail.messages!.length - 1]
- : undefined,
- };
- });
- }
-}
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 6036d07..4024747 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -48,7 +48,7 @@
FetchResponse,
ResponseCode,
} from '../../api/checks';
-import {change$, changeNum$, latestPatchNum$} from '../change/change-model';
+import {ChangeModel} from '../change/change-model';
import {ChangeInfo, NumericChangeId, PatchSetNumber} from '../../types/common';
import {getCurrentRevision} from '../../utils/change-util';
import {getShaByPatchNum} from '../../utils/patch-set-util';
@@ -321,24 +321,28 @@
.filter(r => r !== undefined)
);
- constructor(readonly reporting: ReportingService) {
+ constructor(
+ readonly changeModel: ChangeModel,
+ readonly reporting: ReportingService
+ ) {
this.subscriptions = [
- changeNum$.subscribe(x => (this.changeNum = x)),
+ this.changeModel.changeNum$.subscribe(x => (this.changeNum = x)),
this.checkToPluginMap$.subscribe(map => {
this.checkToPluginMap = map;
}),
- combineLatest([routerPatchNum$, latestPatchNum$]).subscribe(
- ([routerPs, latestPs]) => {
- this.latestPatchNum = latestPs;
- if (latestPs === undefined) {
- this.setPatchset(undefined);
- } else if (typeof routerPs === 'number') {
- this.setPatchset(routerPs);
- } else {
- this.setPatchset(latestPs);
- }
+ combineLatest([
+ routerPatchNum$,
+ this.changeModel.latestPatchNum$,
+ ]).subscribe(([routerPs, latestPs]) => {
+ this.latestPatchNum = latestPs;
+ if (latestPs === undefined) {
+ this.setPatchset(undefined);
+ } else if (typeof routerPs === 'number') {
+ this.setPatchset(routerPs);
+ } else {
+ this.setPatchset(latestPs);
}
- ),
+ }),
];
this.visibilityChangeListener = () => {
this.documentVisibilityChange$.next(undefined);
@@ -634,9 +638,9 @@
// 4. A hidden Gerrit tab becoming visible.
this.subscriptions.push(
combineLatest([
- changeNum$,
+ this.changeModel.changeNum$,
patchset === ChecksPatchset.LATEST
- ? latestPatchNum$
+ ? this.changeModel.latestPatchNum$
: this.checksSelectedPatchsetNumber$,
this.reloadSubjects[pluginName].pipe(throttleTime(1000)),
timer(0, pollIntervalMs),
@@ -645,7 +649,7 @@
.pipe(
takeWhile(_ => !!this.providers[pluginName]),
filter(_ => document.visibilityState !== 'hidden'),
- withLatestFrom(change$),
+ withLatestFrom(this.changeModel.change$),
switchMap(
([[changeNum, patchNum], change]): Observable<FetchResponse> => {
if (!change || !changeNum || !patchNum) return of(this.empty());
diff --git a/polygerrit-ui/app/services/checks/checks-model_test.ts b/polygerrit-ui/app/services/checks/checks-model_test.ts
index 390f6ad..0d46289 100644
--- a/polygerrit-ui/app/services/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/services/checks/checks-model_test.ts
@@ -18,7 +18,7 @@
import './checks-model';
import {ChecksModel, ChecksPatchset, ChecksProviderState} from './checks-model';
import {Category, CheckRun, RunStatus} from '../../api/checks';
-import {grReportingMock} from '../gr-reporting/gr-reporting_mock';
+import {getAppContext} from '../app-context';
const PLUGIN_NAME = 'test-plugin';
@@ -45,7 +45,10 @@
let current: ChecksProviderState;
setup(() => {
- model = new ChecksModel(grReportingMock);
+ model = new ChecksModel(
+ getAppContext().changeModel,
+ getAppContext().reportingService
+ );
model.checksLatest$.subscribe(c => (current = c[PLUGIN_NAME]));
});
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
index ebc6655..d46e08a 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -43,7 +43,7 @@
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 {ChangeModel} from '../change/change-model';
import {Interaction} from '../../constants/reporting';
import {assertIsDefined} from '../../utils/common-util';
import {debounce, DelayedTask} from '../../utils/async-util';
@@ -291,6 +291,7 @@
private discardedDrafts: DraftInfo[] = [];
constructor(
+ readonly changeModel: ChangeModel,
readonly restApiService: RestApiService,
readonly reporting: ReportingService
) {
@@ -301,7 +302,7 @@
this.drafts$.subscribe(x => (this.drafts = x ?? {}))
);
this.subscriptions.push(
- currentPatchNum$.subscribe(x => (this.patchNum = x))
+ this.changeModel.currentPatchNum$.subscribe(x => (this.patchNum = x))
);
this.subscriptions.push(
routerChangeNum$.subscribe(changeNum => {
@@ -311,13 +312,14 @@
})
);
this.subscriptions.push(
- combineLatest([changeNum$, currentPatchNum$]).subscribe(
- ([changeNum, patchNum]) => {
- this.changeNum = changeNum;
- this.patchNum = patchNum;
- this.reloadAllPortedComments();
- }
- )
+ combineLatest([
+ this.changeModel.changeNum$,
+ this.changeModel.currentPatchNum$,
+ ]).subscribe(([changeNum, patchNum]) => {
+ this.changeNum = changeNum;
+ this.patchNum = patchNum;
+ this.reloadAllPortedComments();
+ })
);
this.reloadListener = () => {
this.reloadAllComments();
diff --git a/polygerrit-ui/app/services/comments/comments-model_test.ts b/polygerrit-ui/app/services/comments/comments-model_test.ts
index 93a1d52..f39cad8 100644
--- a/polygerrit-ui/app/services/comments/comments-model_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-model_test.ts
@@ -30,7 +30,6 @@
} 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,
@@ -77,6 +76,7 @@
test('loads comments', async () => {
const model = new CommentsModel(
+ getAppContext().changeModel,
getAppContext().restApiService,
getAppContext().reportingService
);
@@ -103,7 +103,7 @@
);
updateRouterState(GerritView.CHANGE, TEST_NUMERIC_CHANGE_ID);
- updateStateChange(createParsedChange());
+ model.changeModel.updateStateChange(createParsedChange());
await waitUntilCalled(diffCommentsSpy, 'diffCommentsSpy');
await waitUntilCalled(diffRobotCommentsSpy, 'diffRobotCommentsSpy');
diff --git a/polygerrit-ui/app/services/config/config-model.ts b/polygerrit-ui/app/services/config/config-model.ts
index 663935f..91a77b8 100644
--- a/polygerrit-ui/app/services/config/config-model.ts
+++ b/polygerrit-ui/app/services/config/config-model.ts
@@ -19,7 +19,7 @@
import {switchMap} from 'rxjs/operators';
import {Finalizable} from '../registry';
import {RestApiService} from '../gr-rest-api/gr-rest-api';
-import {repo$} from '../change/change-model';
+import {ChangeModel} from '../change/change-model';
import {select} from '../../utils/observable-util';
export interface ConfigState {
@@ -55,12 +55,15 @@
private subscriptions: Subscription[];
- constructor(readonly restApiService: RestApiService) {
+ constructor(
+ readonly changeModel: ChangeModel,
+ readonly restApiService: RestApiService
+ ) {
this.subscriptions = [
from(this.restApiService.getConfig()).subscribe((config?: ServerInfo) => {
this.updateServerConfig(config);
}),
- repo$
+ this.changeModel.repo$
.pipe(
switchMap((repo?: RepoName) => {
if (repo === undefined) return of(undefined);
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index fbc0b48..b8a5e49 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -45,7 +45,6 @@
import {_testOnly_allTasks} from '../utils/async-util';
import {cleanUpStorage} from '../services/storage/gr-storage_mock';
-import {_testOnly_resetState as resetChangeState} from '../services/change/change-model';
import {_testOnly_resetState as resetRouterState} from '../services/router/router-model';
declare global {
@@ -112,7 +111,6 @@
// tests.
initGlobalVariables(appContext);
- resetChangeState();
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 42aa425..01d776e 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -25,7 +25,7 @@
import {GrAuthMock} from '../services/gr-auth/gr-auth_mock';
import {FlagsServiceImplementation} from '../services/flags/flags_impl';
import {EventEmitter} from '../services/gr-event-interface/gr-event-interface_impl';
-import {ChangeService} from '../services/change/change-service';
+import {ChangeModel} from '../services/change/change-model';
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';
@@ -45,18 +45,24 @@
return new GrAuthMock(ctx.eventEmitter);
},
restApiService: (_ctx: Partial<AppContext>) => grRestApiMock,
- changeService: (ctx: Partial<AppContext>) => {
+ changeModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.restApiService, 'restApiService');
- return new ChangeService(ctx.restApiService!);
+ return new ChangeModel(ctx.restApiService!);
},
commentsModel: (ctx: Partial<AppContext>) => {
+ assertIsDefined(ctx.changeModel, 'changeModel');
assertIsDefined(ctx.restApiService, 'restApiService');
assertIsDefined(ctx.reportingService, 'reportingService');
- return new CommentsModel(ctx.restApiService!, ctx.reportingService!);
+ return new CommentsModel(
+ ctx.changeModel!,
+ ctx.restApiService!,
+ ctx.reportingService!
+ );
},
checksModel: (ctx: Partial<AppContext>) => {
+ assertIsDefined(ctx.changeModel, 'changeModel');
assertIsDefined(ctx.reportingService, 'reportingService');
- return new ChecksModel(ctx.reportingService!);
+ return new ChecksModel(ctx.changeModel!, ctx.reportingService!);
},
jsApiService: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.reportingService, 'reportingService');
@@ -64,8 +70,9 @@
},
storageService: (_ctx: Partial<AppContext>) => grStorageMock,
configModel: (ctx: Partial<AppContext>) => {
+ assertIsDefined(ctx.changeModel, 'changeModel');
assertIsDefined(ctx.restApiService, 'restApiService');
- return new ConfigModel(ctx.restApiService!);
+ return new ConfigModel(ctx.changeModel!, ctx.restApiService!);
},
userModel: (ctx: Partial<AppContext>) => {
assertIsDefined(ctx.restApiService, 'restApiService');
