Merge "Also render a status link for COMPLETED runs"
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index c8949e6..9ebee9c 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -3698,18 +3698,16 @@
TestRepository<Repo> repo = createProject("repo");
Change change = insert(repo, newChange(repo));
- AssigneeInput ain = new AssigneeInput();
- ain.assignee = user2.toString();
- gApi.changes().id(change.getId().get()).setAssignee(ain);
+ gApi.changes().id(change.getId().get()).addReviewer(user2.toString());
RequestContext adminContext = requestContext.setContext(newRequestContext(user2));
- assertQuery("assignee:self", change);
+ assertQuery("reviewer:self", change);
requestContext.setContext(adminContext);
gApi.accounts().id(user2.get()).setActive(false);
requestContext.setContext(newRequestContext(user2));
- assertQuery("assignee:self", change);
+ assertQuery("reviewer:self", change);
}
@Test
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index fa590a3..fd85117 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -16,7 +16,7 @@
*/
import '../gr-submit-requirement-hovercard/gr-submit-requirement-hovercard';
import {LitElement, css, html} from 'lit';
-import {customElement, property} from 'lit/decorators';
+import {customElement, property, state} from 'lit/decorators';
import {ParsedChangeInfo} from '../../../types/types';
import {
AccountInfo,
@@ -32,7 +32,14 @@
iconForStatus,
} from '../../../utils/label-util';
import {fontStyles} from '../../../styles/gr-font-styles';
-import {charsOnly} from '../../../utils/string-util';
+import {charsOnly, pluralize} from '../../../utils/string-util';
+import {subscribe} from '../../lit/subscription-controller';
+import {
+ allRunsLatestPatchsetLatestAttempt$,
+ CheckRun,
+} from '../../../services/checks/checks-model';
+import {getResultsOf, hasResultsOf} from '../../../services/checks/checks-util';
+import {Category} from '../../../api/checks';
@customElement('gr-submit-requirements')
export class GrSubmitRequirements extends LitElement {
@@ -45,6 +52,9 @@
@property({type: Boolean})
mutable?: boolean;
+ @state()
+ runs: CheckRun[] = [];
+
static override get styles() {
return [
fontStyles,
@@ -95,10 +105,25 @@
td {
padding: var(--spacing-s);
}
+ .votes-cell {
+ display: flex;
+ }
+ .check-error {
+ margin-right: var(--spacing-l);
+ }
+ .check-error iron-icon {
+ color: var(--error-foreground);
+ vertical-align: top;
+ }
`,
];
}
+ constructor() {
+ super();
+ subscribe(this, allRunsLatestPatchsetLatestAttempt$, x => (this.runs = x));
+ }
+
override render() {
const submit_requirements = (this.change?.submit_requirements ?? []).filter(
req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE
@@ -130,7 +155,12 @@
.text="${requirement.name}"
></gr-limited-text>
</td>
- <td>${this.renderVotes(requirement)}</td>
+ <td>
+ <div class="votes-cell">
+ ${this.renderVotes(requirement)}
+ ${this.renderChecks(requirement)}
+ </div>
+ </td>
</tr>`
)}
</tbody>
@@ -199,6 +229,28 @@
);
}
+ renderChecks(requirement: SubmitRequirementResultInfo) {
+ const requirementLabels = extractAssociatedLabels(requirement);
+ const requirementRuns = this.runs
+ .filter(run => hasResultsOf(run, Category.ERROR))
+ .filter(
+ run => run.labelName && requirementLabels.includes(run.labelName)
+ );
+ const runsCount = requirementRuns.reduce(
+ (sum, run) => sum + getResultsOf(run, Category.ERROR).length,
+ 0
+ );
+ if (runsCount > 0) {
+ return html`<span class="check-error"
+ ><iron-icon icon="gr-icons:error"></iron-icon>${pluralize(
+ runsCount,
+ 'error'
+ )}</span
+ >`;
+ }
+ return;
+ }
+
renderTriggerVotes(submitReqs: SubmitRequirementResultInfo[]) {
const labels = this.change?.labels ?? {};
const allLabels = Object.keys(labels);
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 4a6f009..8e70eb9 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -177,8 +177,8 @@
CHANGE_ID_QUERY: /^\/id\/(I[0-9a-f]{40})$/,
- // Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- CHANGE_LEGACY: /^\/c\/(\d+)\/?(((-?\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
+ // Matches /c/<changeNum>/[*][/].
+ CHANGE_LEGACY: /^\/c\/(\d+)\/(.*)$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
// Matches
@@ -210,10 +210,6 @@
// Matches /c/<project>/+/<changeNum>/[<patchNum|edit>]/<path>,edit[#lineNum]
DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)\/(\d+|edit)\/(.+),edit(#\d+)?$/,
- // Matches non-project-relative
- // /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- DIFF_LEGACY: /^\/c\/(\d+)\/((-?\d+|edit)(\.\.(\d+|edit))?)\/(.+)/,
-
// Matches diff routes using @\d+ to specify a file name (whether or not
// the project name is included).
// eslint-disable-next-line max-len
@@ -287,15 +283,6 @@
type QueryStringItem = [string, string]; // [key, value]
-type GenerateUrlLegacyChangeViewParameters = Omit<
- GenerateUrlChangeViewParameters,
- 'project'
->;
-type GenerateUrlLegacyDiffViewParameters = Omit<
- GenerateUrlDiffViewParameters,
- 'project'
->;
-
interface PatchRangeParams {
patchNum?: PatchSetNum;
basePatchNum?: BasePatchSetNum;
@@ -679,37 +666,6 @@
}
/**
- * Given a set of params without a project, gets the project from the rest
- * API project lookup and then sets the app params.
- */
- _normalizeLegacyRouteParams(
- params: Readonly<
- | GenerateUrlLegacyChangeViewParameters
- | GenerateUrlLegacyDiffViewParameters
- >
- ) {
- if (!params.changeNum) {
- return Promise.resolve();
- }
-
- return this.restApiService
- .getFromProjectLookup(params.changeNum)
- .then(project => {
- // Show a 404 and terminate if the lookup request failed. Attempting
- // to redirect after failing to get the project loops infinitely.
- if (!project) {
- this._show404();
- return;
- }
- const updatedParams:
- | GenerateUrlChangeViewParameters
- | GenerateUrlDiffViewParameters = {...params, project};
- this._normalizePatchRangeParams(updatedParams);
- this._redirect(this._generateUrl(updatedParams));
- });
- }
-
- /**
* Normalizes the params object, and determines if the URL needs to be
* modified to fit the proper schema.
*
@@ -1100,8 +1056,6 @@
this._mapRoute(RoutePattern.CHANGE_LEGACY, '_handleChangeLegacyRoute');
- this._mapRoute(RoutePattern.DIFF_LEGACY, '_handleDiffLegacyRoute');
-
this._mapRoute(RoutePattern.AGREEMENTS, '_handleAgreementsRoute', true);
this._mapRoute(
@@ -1666,41 +1620,26 @@
}
_handleChangeLegacyRoute(ctx: PageContextWithQueryMap) {
- // Parameter order is based on the regex group number matched.
- const params: GenerateUrlLegacyChangeViewParameters = {
- changeNum: Number(ctx.params[0]) as NumericChangeId,
- basePatchNum: convertToPatchSetNum(ctx.params[3]) as BasePatchSetNum,
- patchNum: convertToPatchSetNum(ctx.params[5]),
- view: GerritView.CHANGE,
- querystring: ctx.querystring,
- };
-
- this._normalizeLegacyRouteParams(params);
+ const changeNum = Number(ctx.params[0]) as NumericChangeId;
+ if (!changeNum) {
+ this._show404();
+ return;
+ }
+ this.restApiService.getFromProjectLookup(changeNum).then(project => {
+ // Show a 404 and terminate if the lookup request failed. Attempting
+ // to redirect after failing to get the project loops infinitely.
+ if (!project) {
+ this._show404();
+ return;
+ }
+ this._redirect(`/c/${project}/+/${changeNum}/${ctx.params[1]}`);
+ });
}
_handleLegacyLinenum(ctx: PageContextWithQueryMap) {
this._redirect(ctx.path.replace(LEGACY_LINENUM_PATTERN, '#$1'));
}
- _handleDiffLegacyRoute(ctx: PageContextWithQueryMap) {
- // Parameter order is based on the regex group number matched.
- const params: GenerateUrlLegacyDiffViewParameters = {
- changeNum: Number(ctx.params[0]) as NumericChangeId,
- basePatchNum: convertToPatchSetNum(ctx.params[2]) as BasePatchSetNum,
- patchNum: convertToPatchSetNum(ctx.params[4]),
- path: ctx.params[5],
- view: GerritView.DIFF,
- };
-
- const address = this._parseLineAddress(ctx.hash);
- if (address) {
- params.leftSide = address.leftSide;
- params.lineNum = address.lineNum;
- }
-
- this._normalizeLegacyRouteParams(params);
- }
-
_handleDiffEditRoute(ctx: PageContextWithQueryMap) {
// Parameter order is based on the regex group number matched.
const project = ctx.params[0] as RepoName;
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index 4c7855b..b91bf0c 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -192,7 +192,6 @@
'_handleDiffRoute',
'_handleDefaultRoute',
'_handleChangeLegacyRoute',
- '_handleDiffLegacyRoute',
'_handleDocumentationRedirectRoute',
'_handleDocumentationSearchRoute',
'_handleDocumentationSearchRedirectRoute',
@@ -536,66 +535,6 @@
});
suite('param normalization', () => {
- let projectLookupStub;
- let generateUrlStub;
-
- setup(() => {
- projectLookupStub = stubRestApi('getFromProjectLookup');
- generateUrlStub = sinon.stub(element, '_generateUrl');
- });
-
- suite('_normalizeLegacyRouteParams', () => {
- let rangeStub;
- let redirectStub;
- let show404Stub;
-
- setup(() => {
- rangeStub = sinon.stub(element, '_normalizePatchRangeParams')
- .returns(Promise.resolve());
- redirectStub = sinon.stub(element, '_redirect');
- show404Stub = sinon.stub(element, '_show404');
- });
-
- test('w/o changeNum', () => {
- projectLookupStub.returns(Promise.resolve('foo/bar'));
- const params = {};
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isFalse(generateUrlStub.calledOnce);
- assert.isFalse(projectLookupStub.called);
- assert.isFalse(rangeStub.called);
- assert.isFalse(redirectStub.called);
- assert.isFalse(show404Stub.called);
- });
- });
-
- test('w/ changeNum', () => {
- projectLookupStub.returns(Promise.resolve('foo/bar'));
- const params = {changeNum: 1234};
-
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isTrue(generateUrlStub.calledOnce);
- const updatedParams = generateUrlStub.lastCall.args[0];
- assert.isTrue(projectLookupStub.called);
- assert.isTrue(rangeStub.called);
- assert.equal(updatedParams.project, 'foo/bar');
- assert.isTrue(redirectStub.calledOnce);
- assert.isFalse(show404Stub.called);
- });
- });
-
- test('halts on project lookup failure', () => {
- projectLookupStub.returns(Promise.resolve(undefined));
- const params = {changeNum: 1234};
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isFalse(generateUrlStub.calledOnce);
- assert.isTrue(projectLookupStub.called);
- assert.isFalse(rangeStub.called);
- assert.isFalse(redirectStub.called);
- assert.isTrue(show404Stub.calledOnce);
- });
- });
- });
-
suite('_normalizePatchRangeParams', () => {
test('range n..n normalizes to n', () => {
const params = {basePatchNum: 4, patchNum: 4};
@@ -1367,58 +1306,19 @@
assert.isTrue(redirectStub.calledWithExactly('/c/12345'));
});
- test('_handleChangeLegacyRoute', () => {
- const normalizeRouteStub = sinon.stub(element,
- '_normalizeLegacyRouteParams');
+ test('_handleChangeLegacyRoute', async () => {
+ stubRestApi('getFromProjectLookup').returns(Promise.resolve('project'));
const ctx = {
params: [
1234, // 0 Change number
- null, // 1 Unused
- null, // 2 Unused
- 6, // 3 Base patch number
- null, // 4 Unused
- 9, // 5 Patch number
+ 'comment/6789',
],
querystring: '',
};
element._handleChangeLegacyRoute(ctx);
- assert.isTrue(normalizeRouteStub.calledOnce);
- assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
- changeNum: 1234,
- basePatchNum: 6,
- patchNum: 9,
- view: GerritView.CHANGE,
- querystring: '',
- });
- });
-
- test('_handleDiffLegacyRoute', () => {
- const normalizeRouteStub = sinon.stub(element,
- '_normalizeLegacyRouteParams');
- const ctx = {
- params: [
- 1234, // 0 Change number
- null, // 1 Unused
- 3, // 2 Base patch number
- null, // 3 Unused
- 8, // 4 Patch number
- 'foo/bar', // 5 Diff path
- ],
- path: '/c/1234/3..8/foo/bar',
- hash: 'b123',
- };
- element._handleDiffLegacyRoute(ctx);
- assert.isFalse(redirectStub.called);
- assert.isTrue(normalizeRouteStub.calledOnce);
- assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
- changeNum: 1234,
- basePatchNum: 3,
- patchNum: 8,
- view: GerritView.DIFF,
- path: 'foo/bar',
- lineNum: 123,
- leftSide: true,
- });
+ await flush();
+ assert.isTrue(redirectStub.calledWithExactly('/c/project/+/1234' +
+ '/comment/6789'));
});
test('_handleLegacyLinenum w/ @321', () => {