Add the selected patchset for Checks as a URL parameter
Since we have a dedicated URL parameter for the checks patchset, we stop
mirroring the files patchset over to the checks patchset. That was only
a one-way sync and likely a bit confusing. It was also a stop-gap, such
that users could point to checks for a specific patchset using the files
patchset.
Release-Notes: skip
Google-Bug-Id: b/235185477
Change-Id: Ief5b2274c0cb8dbe63ede260f3455bee0387a4e6
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 26fa0cb..a4e26fe 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -1246,13 +1246,16 @@
}
private onPatchsetSelected(e: CustomEvent<{value: string}>) {
- const patchset = Number(e.detail.value);
- assert(!isNaN(patchset), 'selected patchset must be a number');
- this.getChecksModel().setPatchset(patchset as PatchSetNumber);
+ let patchset: number | undefined = Number(e.detail.value);
+ assert(Number.isInteger(patchset), `patchset must be integer: ${patchset}`);
+ if (patchset === this.latestPatchsetNumber) patchset = undefined;
+ this.getChecksModel().updateStateSetPatchset(
+ patchset as PatchSetNumber | undefined
+ );
}
private goToLatestPatchset() {
- this.getChecksModel().setPatchset(undefined);
+ this.getChecksModel().updateStateSetPatchset(undefined);
}
private createAttemptDropdownItems() {
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 10c2f58..a35ca40 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -21,6 +21,7 @@
RepoName,
UrlEncodedCommentId,
PARENT,
+ PatchSetNumber,
} from '../../../types/common';
import {AppElement, AppElementParams} from '../../gr-app-types';
import {LocationChangeEventDetail} from '../../../types/events';
@@ -1398,6 +1399,10 @@
const tab = queryMap.get('tab');
if (tab) state.tab = tab;
+ const checksPatchset = Number(queryMap.get('checksPatchset'));
+ if (Number.isInteger(checksPatchset) && checksPatchset > 0) {
+ state.checksPatchset = checksPatchset as PatchSetNumber;
+ }
const filter = queryMap.get('filter');
if (filter) state.filter = filter;
const attempt = stringToAttemptChoice(queryMap.get('attempt'));
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 385aec3..2b92529 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -138,11 +138,6 @@
}
interface ChecksState {
- /**
- * This is the patchset number selected by the user. The *latest* patchset
- * can be picked up from the change model.
- */
- patchsetNumberSelected?: PatchSetNumber;
/** Checks data for the latest patchset. */
pluginStateLatest: {
[name: string]: ChecksProviderState;
@@ -202,9 +197,10 @@
private subscriptions: Subscription[] = [];
+ // TODO: Maybe consider migrating usages to `changeViewModel` directly.
public checksSelectedPatchsetNumber$ = select(
- this.state$,
- state => state.patchsetNumberSelected
+ this.changeViewModel.checksPatchset$,
+ ps => ps
);
public checksSelectedAttemptNumber$ = select(
@@ -219,10 +215,12 @@
public checksLatest$ = select(this.state$, state => state.pluginStateLatest);
- public checksSelected$ = select(this.state$, state =>
- state.patchsetNumberSelected
- ? state.pluginStateSelected
- : state.pluginStateLatest
+ public checksSelected$ = select(
+ combineLatest([this.state$, this.changeViewModel.checksPatchset$]),
+ ([state, ps]) => {
+ const checksPs = ps ? ChecksPatchset.SELECTED : ChecksPatchset.LATEST;
+ return this.getPluginState(state, checksPs);
+ }
);
public aPluginHasRegistered$ = select(
@@ -380,7 +378,6 @@
readonly pluginsModel: PluginsModel
) {
super({
- patchsetNumberSelected: undefined,
pluginStateLatest: {},
pluginStateSelected: {},
});
@@ -399,19 +396,6 @@
this.checkToPluginMap$.subscribe(map => {
this.checkToPluginMap = map;
}),
- combineLatest([
- this.routerModel.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.firstLoadCompleted$
.pipe(
filter(completed => !!completed),
@@ -649,8 +633,10 @@
this.setState(nextState);
}
- updateStateSetPatchset(patchsetNumberSelected?: PatchSetNumber) {
- this.updateState({patchsetNumberSelected});
+ updateStateSetPatchset(num?: PatchSetNumber) {
+ this.changeViewModel.updateState({
+ checksPatchset: num === this.latestPatchNum ? undefined : num,
+ });
}
updateStateSetAttempt(attemptNumberSelected: AttemptChoice) {
@@ -661,10 +647,6 @@
this.changeViewModel.updateState({filter: runFilterRegexp});
}
- setPatchset(num?: PatchSetNumber) {
- this.updateStateSetPatchset(num === this.latestPatchNum ? undefined : num);
- }
-
reload(pluginName: string) {
this.reloadSubjects[pluginName].next();
}
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 4c97f60..826ec66 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -9,6 +9,7 @@
RevisionPatchSetNum,
BasePatchSetNum,
ChangeInfo,
+ PatchSetNumber,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
@@ -25,22 +26,34 @@
export interface ChangeViewState extends ViewState {
view: GerritView.CHANGE;
+
changeNum: NumericChangeId;
project: RepoName;
edit?: boolean;
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
commentId?: UrlEncodedCommentId;
- forceReload?: boolean;
- openReplyDialog?: boolean;
tab?: string;
+
+ /** Checks related view state */
+
+ /** selected patchset for check runs (undefined=latest) */
+ checksPatchset?: PatchSetNumber;
/** regular expression for filtering check runs */
filter?: string;
- /** selected attempt for selected check runs */
+ /** selected attempt for check runs (undefined=latest) */
attempt?: AttemptChoice;
+ /** State properties that trigger one-time actions */
+
+ /** for scrolling a Change Log message into view in gr-change-view */
messageHash?: string;
+ /** for logging where the user came from */
usp?: string;
+ /** triggers all change related data to be reloaded */
+ forceReload?: boolean;
+ /** triggers opening the reply dialog */
+ openReplyDialog?: boolean;
}
/**
@@ -85,6 +98,9 @@
}
let suffix = `${range}`;
const queries = [];
+ if (state.checksPatchset && state.checksPatchset > 0) {
+ queries.push(`checksPatchset=${state.checksPatchset}`);
+ }
if (state.attempt) {
if (state.attempt !== 'latest') queries.push(`attempt=${state.attempt}`);
}
@@ -126,6 +142,11 @@
export class ChangeViewModel extends Model<ChangeViewState | undefined> {
public readonly tab$ = select(this.state$, state => state?.tab);
+ public readonly checksPatchset$ = select(
+ this.state$,
+ state => state?.checksPatchset
+ );
+
public readonly attempt$ = select(this.state$, state => state?.attempt);
public readonly filter$ = select(this.state$, state => state?.filter);
