Fix how the change gets updated in the change-model and -service
A reload from the change-view would not trigger an update of the
Observables in the change-model, so the Checks tab would be stuck
with an old patchset when the user Shift-R reloads.
For now we let the change-view fully control which change is current.
That will have to be fixed/updated later.
Change-Id: Id84cd38548a06d43c88d3c1a69c4203e7e7a248d
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 3d7646f..87a7308 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
@@ -274,11 +274,13 @@
* @event show-auth-required
*/
- reporting = appContext.reportingService;
+ private readonly reporting = appContext.reportingService;
- flagsService = appContext.flagsService;
+ private readonly flagsService = appContext.flagsService;
- readonly jsAPI = appContext.jsApiService;
+ private readonly jsAPI = appContext.jsApiService;
+
+ private readonly changeService = appContext.changeService;
/**
* URL params passed from the router.
@@ -2022,6 +2024,7 @@
this._lineHeight = Number(lineHeight.slice(0, lineHeight.length - 2));
this._change = change;
+ this.changeService.updateChange(change);
if (
!this._patchRange ||
!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 99e5356..ae446cd 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
@@ -105,6 +105,7 @@
import {GerritView} from '../../../services/router/router-model';
import {ParsedChangeInfo} from '../../../types/types';
import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
+import {appContext} from '../../../services/app-context';
const pluginApi = _testOnly_initGerritPluginApi();
const fixture = fixtureFromElement('gr-change-view');
@@ -2766,7 +2767,7 @@
element._change = {...change};
element._patchRange = {patchNum: 4 as PatchSetNum};
element._mergeable = true;
- const showStub = sinon.stub(element.jsAPI, 'handleEvent');
+ const showStub = sinon.stub(appContext.jsApiService, 'handleEvent');
element._sendShowChangeEvent();
assert.isTrue(showStub.calledOnce);
assert.equal(showStub.lastCall.args[0], EventType.SHOW_CHANGE);
@@ -2968,11 +2969,11 @@
test("don't report changeDisplayed on reply", done => {
const changeDisplayStub = sinon.stub(
- element.reporting,
+ appContext.reportingService,
'changeDisplayed'
);
const changeFullyLoadedStub = sinon.stub(
- element.reporting,
+ appContext.reportingService,
'changeFullyLoaded'
);
element._handleReplySent();
@@ -2985,11 +2986,11 @@
test('report changeDisplayed on _paramsChanged', done => {
const changeDisplayStub = sinon.stub(
- element.reporting,
+ appContext.reportingService,
'changeDisplayed'
);
const changeFullyLoadedStub = sinon.stub(
- element.reporting,
+ appContext.reportingService,
'changeFullyLoaded'
);
element._paramsChanged({
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 3a76112..676ef7b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -308,15 +308,6 @@
private readonly restApiService = appContext.restApiService;
- private readonly changeService = appContext.changeService;
-
- constructor() {
- super();
- // TODO: This is just an artificical dependdency such that the service is
- // instantiated and its observables subscribed. Remove this later.
- this.changeService.dontDoAnything();
- }
-
start() {
if (!this._app) {
return;
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 09e0724..0369ccf 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -70,7 +70,7 @@
eventEmitter: () => new EventEmitter(),
authService: () => new Auth(appContext.eventEmitter),
restApiService: () => new GrRestApiInterface(appContext.authService),
- changeService: () => new ChangeService(appContext.restApiService),
+ changeService: () => new ChangeService(),
checksService: () => new ChecksService(),
jsApiService: () => new GrJsApiInterface(),
});
diff --git a/polygerrit-ui/app/services/change/change-service.ts b/polygerrit-ui/app/services/change/change-service.ts
index 6a9a5e9..4524813 100644
--- a/polygerrit-ui/app/services/change/change-service.ts
+++ b/polygerrit-ui/app/services/change/change-service.ts
@@ -14,28 +14,33 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
import {routerChangeNum$} from '../router/router-model';
import {updateState} from './change-model';
-import {RestApiService} from '../gr-rest-api/gr-rest-api';
-import {switchMap, tap} from 'rxjs/operators';
-import {of, from} from 'rxjs';
+import {tap} from 'rxjs/operators';
+import {ParsedChangeInfo} from '../../types/types';
export class ChangeService {
+ // TODO: In the future we will want to make restApiService.getChangeDetail()
+ // calls from a switchMap() here. For now just make sure to invalidate the
+ // change when no changeNum is set.
private routerChangeNumEffect = routerChangeNum$.pipe(
- switchMap(changeNum => {
- if (!changeNum) return of(undefined);
- return from(this.restApiService.getChangeDetail(changeNum));
- }),
- tap(change => {
- updateState(change ?? undefined);
+ tap(changeNum => {
+ if (!changeNum) updateState(undefined);
})
);
- constructor(private readonly restApiService: RestApiService) {
+ constructor() {
this.routerChangeNumEffect.subscribe();
}
- // TODO: Remove.
- dontDoAnything() {}
+ /**
+ * This is a temporary indirection between change-view, which currently
+ * manages what the current change is, and the change-model, which will
+ * become the source of truth in the future. We will extract a substantial
+ * amount of code from change-view and move it into this change-service. This
+ * will take some time ...
+ */
+ updateChange(change: ParsedChangeInfo) {
+ updateState(change);
+ }
}
