Add checksPatchset to diff URLs
This fixes the issue of check result code pointers resulting in diffs
without the check result showing up in the diff, if the checks result
is not for the latest patchset.
Release-Notes: skip
Google-Bug-Id: b/279812930
Change-Id: I9a496700b3a1556394f9ea2ca905dbc16183e760
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index d778546..b1c4c8e4 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -710,6 +710,7 @@
changeNum: change._number,
repo: change.project,
patchNum: patchset,
+ checksPatchset: patchset,
diffView: {path, lineNum: line},
}),
primary: true,
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 6892188..864f559 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -1446,6 +1446,10 @@
diffView: {path: ctx.params[8]},
};
const queryMap = new URLSearchParams(ctx.querystring);
+ const checksPatchset = Number(queryMap.get('checksPatchset'));
+ if (Number.isInteger(checksPatchset) && checksPatchset > 0) {
+ state.checksPatchset = checksPatchset as PatchSetNumber;
+ }
if (queryMap.has('forceReload')) state.forceReload = true;
const address = this.parseLineAddress(ctx.hash);
if (address) {
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 713d401..127f77b 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -204,25 +204,33 @@
childView: ChangeChildView.DIFF,
});
- const path = `/${encodeURL(state.diffView?.path ?? '')}`;
-
- let suffix = '';
+ let path = `/${encodeURL(state.diffView?.path ?? '')}`;
// TODO: Move creating of comment URLs to a separate function. We are
// "abusing" the `commentId` property, which should only be used for pointing
// to comment in the COMMENTS tab of the OVERVIEW page.
if (state.commentId) {
- suffix += `comment/${state.commentId}/`;
+ path += `comment/${state.commentId}/`;
}
+ let queryParams = '';
+ const params = [];
+ if (state.checksPatchset && state.checksPatchset > 0) {
+ params.push(`checksPatchset=${state.checksPatchset}`);
+ }
+ if (params.length > 0) {
+ queryParams = '?' + params.join('&');
+ }
+
+ let hash = '';
if (state.diffView?.lineNum) {
- suffix += '#';
+ hash += '#';
if (state.diffView?.leftSide) {
- suffix += 'b';
+ hash += 'b';
}
- suffix += state.diffView.lineNum;
+ hash += state.diffView.lineNum;
}
- return `${createChangeUrlCommon(state)}${path}${suffix}`;
+ return `${createChangeUrlCommon(state)}${path}${queryParams}${hash}`;
}
export function createEditUrl(
diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts
index 837e362..8e671d1 100644
--- a/polygerrit-ui/app/models/views/change_test.ts
+++ b/polygerrit-ui/app/models/views/change_test.ts
@@ -6,6 +6,7 @@
import {assert} from '@open-wc/testing';
import {
BasePatchSetNum,
+ PatchSetNumber,
RepoName,
RevisionPatchSetNum,
} from '../../api/rest-api';
@@ -77,59 +78,97 @@
assert.equal(createChangeUrl(state), '/c/x%252B/y%252B/z%252B/w/+/42');
});
- test('createDiffUrl', () => {
- const params: ChangeViewState = {
- ...createDiffViewState(),
- patchNum: 12 as RevisionPatchSetNum,
- diffView: {path: 'x+y/path.cpp'},
- };
- assert.equal(
- createDiffUrl(params),
- '/c/test-project/+/42/12/x%252By/path.cpp'
- );
+ suite('createDiffUrl', () => {
+ let params: ChangeViewState;
+ setup(() => {
+ params = {
+ ...createDiffViewState(),
+ patchNum: 12 as RevisionPatchSetNum,
+ diffView: {path: 'x+y/path.cpp'},
+ };
+ });
- window.CANONICAL_PATH = '/base';
- assert.equal(createDiffUrl(params).substring(0, 5), '/base');
- window.CANONICAL_PATH = undefined;
+ test('CANONICAL_PATH', () => {
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createDiffUrl(params).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+ });
- params.repo = 'test' as RepoName;
- assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp');
+ test('basic', () => {
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/x%252By/path.cpp'
+ );
+ });
- params.basePatchNum = 6 as BasePatchSetNum;
- assert.equal(createDiffUrl(params), '/c/test/+/42/6..12/x%252By/path.cpp');
+ test('repo', () => {
+ params.repo = 'test' as RepoName;
+ assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp');
+ });
- params.diffView = {
- path: 'foo bar/my+file.txt%',
- };
- params.patchNum = 2 as RevisionPatchSetNum;
- delete params.basePatchNum;
- assert.equal(
- createDiffUrl(params),
- '/c/test/+/42/2/foo+bar/my%252Bfile.txt%2525'
- );
+ test('checks patchset', () => {
+ params.checksPatchset = 4 as PatchSetNumber;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/x%252By/path.cpp?checksPatchset=4'
+ );
+ });
- params.diffView = {
- path: 'file.cpp',
- lineNum: 123,
- };
- assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#123');
+ test('base patchset', () => {
+ params.basePatchNum = 6 as BasePatchSetNum;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/6..12/x%252By/path.cpp'
+ );
+ });
- params.diffView = {
- path: 'file.cpp',
- lineNum: 123,
- leftSide: true,
- };
- assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#b123');
- });
+ test('percent', () => {
+ params.diffView = {
+ path: 'foo bar/my+file.txt%',
+ };
+ params.patchNum = 2 as RevisionPatchSetNum;
+ delete params.basePatchNum;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/2/foo+bar/my%252Bfile.txt%2525'
+ );
+ });
- test('diff with repo name encoding', () => {
- const params: ChangeViewState = {
- ...createDiffViewState(),
- patchNum: 12 as RevisionPatchSetNum,
- repo: 'x+/y' as RepoName,
- diffView: {path: 'x+y/path.cpp'},
- };
- assert.equal(createDiffUrl(params), '/c/x%252B/y/+/42/12/x%252By/path.cpp');
+ test('line right', () => {
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/file.cpp#123'
+ );
+ });
+
+ test('line left', () => {
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ leftSide: true,
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/file.cpp#b123'
+ );
+ });
+
+ test('diff with repo name encoding', () => {
+ const params: ChangeViewState = {
+ ...createDiffViewState(),
+ patchNum: 12 as RevisionPatchSetNum,
+ repo: 'x+/y' as RepoName,
+ diffView: {path: 'x+y/path.cpp'},
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/x%252B/y/+/42/12/x%252By/path.cpp'
+ );
+ });
});
test('createEditUrl', () => {
