Add fields required for gr-hovercard-run to RunResult.
After If3ba57173 the RunResult contains only some of the fields from
CheckRun. The gr-hovercard-run is meant to be used with CheckRun and is
therefore missing a lot of fields. In the original change the decision
was made to name copied fields explicitly. We are keeping with that
decision here.
WorstCategory is used to be a computed value, but now we precalculate it
when we construct CheckRun from the CheckRunApi (plugin return value).
We can't calculate the value in runResult, as the operation takes
O(results in the run) and we would end up running it O(results) times.
Propagating run as a field is possible for checks tab. However for the
diff view that would require deeper changes, as diff-view gets results
directly from the model. Which is why we opt for adding the fields to
the RunResult.
Google-Bug-Id: b/290802419
Release-Notes: skip
Change-Id: Idcb908c5aab4347f087fb46c35e7fca421724faa
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index f6a40a2..f6099fa 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -12,12 +12,8 @@
import {subscribe} from '../../lit/subscription-controller';
import {sharedStyles} from '../../../styles/shared-styles';
import {getAppContext} from '../../../services/app-context';
-import {
- CheckResult,
- CheckRun,
- ErrorMessages,
-} from '../../../models/checks/checks-model';
-import {Action, Category, RunStatus} from '../../../api/checks';
+import {CheckRun, ErrorMessages} from '../../../models/checks/checks-model';
+import {Action, Category, CheckResult, RunStatus} from '../../../api/checks';
import {fireShowTab} from '../../../utils/event-util';
import {
compareByWorstCategory,
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 19f96a1..8372b41 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -23,7 +23,6 @@
iconForRun,
PRIMARY_STATUS_ACTIONS,
primaryRunAction,
- worstCategory,
} from '../../models/checks/checks-util';
import {
CheckRun,
@@ -366,7 +365,7 @@
*/
renderAdditionalIcon() {
if (this.run.status !== RunStatus.RUNNING) return nothing;
- const category = worstCategory(this.run);
+ const category = this.run.worstCategory;
if (!category) return nothing;
const icon = iconFor(category);
return html`
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
index 5c17dcd..d1ef25b 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
@@ -7,13 +7,12 @@
import {fontStyles} from '../../styles/gr-font-styles';
import {customElement, property} from 'lit/decorators.js';
import './gr-checks-action';
-import {CheckRun} from '../../models/checks/checks-model';
+import {CheckRun, RunResult} from '../../models/checks/checks-model';
import {
AttemptDetail,
ChecksIcon,
iconFor,
runActions,
- worstCategory,
} from '../../models/checks/checks-util';
import {durationString, fromNow} from '../../utils/date-util';
import {RunStatus} from '../../api/checks';
@@ -28,7 +27,7 @@
@customElement('gr-hovercard-run')
export class GrHovercardRun extends base {
@property({type: Object})
- run?: CheckRun;
+ run?: RunResult | CheckRun;
static override get styles() {
return [
@@ -357,8 +356,7 @@
computeIcon(): ChecksIcon {
if (!this.run) return {name: ''};
- const category = worstCategory(this.run);
- if (category) return iconFor(category);
+ if (this.run.worstCategory) return iconFor(this.run.worstCategory);
return this.run.status === RunStatus.COMPLETED
? iconFor(RunStatus.COMPLETED)
: {name: ''};
diff --git a/polygerrit-ui/app/models/checks/checks-fakes.ts b/polygerrit-ui/app/models/checks/checks-fakes.ts
index d50ddba..1890639 100644
--- a/polygerrit-ui/app/models/checks/checks-fakes.ts
+++ b/polygerrit-ui/app/models/checks/checks-fakes.ts
@@ -26,6 +26,7 @@
isSingleAttempt: true,
isLatestAttempt: true,
attemptDetails: [],
+ worstCategory: Category.ERROR,
results: [
{
internalResultId: 'f0r0',
@@ -94,6 +95,7 @@
isSingleAttempt: true,
isLatestAttempt: true,
attemptDetails: [],
+ worstCategory: Category.ERROR,
results: [
{
internalResultId: 'f1r0',
@@ -228,6 +230,7 @@
callback: () => Promise.resolve({message: 'fake "delete" triggered'}),
},
],
+ worstCategory: Category.INFO,
results: [
{
internalResultId: 'f2r0',
@@ -276,6 +279,7 @@
isSingleAttempt: false,
isLatestAttempt: false,
attemptDetails: [],
+ worstCategory: Category.INFO,
results: [
{
internalResultId: 'f42r0',
@@ -294,6 +298,7 @@
isSingleAttempt: false,
isLatestAttempt: false,
attemptDetails: [],
+ worstCategory: Category.ERROR,
results: [
{
internalResultId: 'f43r0',
@@ -320,6 +325,7 @@
isSingleAttempt: false,
isLatestAttempt: true,
attemptDetails: [],
+ worstCategory: Category.INFO,
results: [
{
internalResultId: 'f44r0',
@@ -380,6 +386,7 @@
isSingleAttempt: false,
isLatestAttempt: false,
attemptDetails: [],
+ worstCategory: Category.ERROR,
results:
attempt % 2 === 0
? [
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 25785e4..612862b 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -9,6 +9,7 @@
createAttemptMap,
LATEST_ATTEMPT,
sortAttemptDetails,
+ worstCategory,
} from './checks-util';
import {assertIsDefined} from '../../utils/common-util';
import {select} from '../../utils/observable-util';
@@ -104,6 +105,12 @@
* List of all attempts for the same check, ordered by attempt number.
*/
attemptDetails: AttemptDetail[];
+
+ /**
+ * The category of the worst check result in the run.
+ */
+ worstCategory?: Category;
+
results?: CheckResult[];
}
@@ -119,7 +126,18 @@
Pick<CheckRun, 'patchset'> &
Pick<CheckRun, 'isLatestAttempt'> &
Pick<CheckRun, 'checkName'> &
- Pick<CheckRun, 'labelName'> & {results?: never};
+ Pick<CheckRun, 'labelName'> &
+ Pick<CheckRun, 'status'> &
+ Pick<CheckRun, 'statusLink'> &
+ Pick<CheckRun, 'statusDescription'> &
+ Pick<CheckRun, 'startedTimestamp'> &
+ Pick<CheckRun, 'scheduledTimestamp'> &
+ Pick<CheckRun, 'finishedTimestamp'> &
+ Pick<CheckRun, 'checkLink'> &
+ Pick<CheckRun, 'checkDescription'> &
+ Pick<CheckRun, 'actions'> &
+ Pick<CheckRun, 'attemptDetails'> &
+ Pick<CheckRun, 'worstCategory'> & {results?: never};
export function runResult(run: CheckRun, result: CheckResult): RunResult {
return {
@@ -129,6 +147,17 @@
isLatestAttempt: run.isLatestAttempt,
checkName: run.checkName,
labelName: run.labelName,
+ status: run.status,
+ statusLink: run.statusLink,
+ statusDescription: run.statusDescription,
+ startedTimestamp: run.startedTimestamp,
+ scheduledTimestamp: run.scheduledTimestamp,
+ finishedTimestamp: run.finishedTimestamp,
+ checkLink: run.checkLink,
+ checkDescription: run.checkDescription,
+ actions: run.actions,
+ attemptDetails: run.attemptDetails,
+ worstCategory: run.worstCategory,
...result,
};
}
@@ -593,6 +622,7 @@
isLatestAttempt: attemptInfo.latestAttempt === (run.attempt ?? 0),
isSingleAttempt: attemptInfo.isSingleAttempt,
attemptDetails: attemptInfo.attempts,
+ worstCategory: worstCategory(run),
results: (run.results ?? []).map((result, i) => {
return {
...result,
@@ -637,7 +667,11 @@
}
return result;
});
- return resultUpdated ? {...run, results} : run;
+ if (resultUpdated) {
+ run = {...run, results};
+ run.worstCategory = worstCategory(run);
+ }
+ return run;
});
if (!runUpdated) return;
pluginState[pluginName] = {
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 86c4b49..a567fb5 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -164,7 +164,7 @@
return r;
}
-export function worstCategory(run: CheckRun) {
+export function worstCategory(run: CheckRunApi) {
if (hasResultsOf(run, Category.ERROR)) return Category.ERROR;
if (hasResultsOf(run, Category.WARNING)) return Category.WARNING;
if (hasResultsOf(run, Category.INFO)) return Category.INFO;
@@ -288,7 +288,7 @@
)[0];
}
-export function runActions(run?: CheckRun): Action[] {
+export function runActions(run?: CheckRun | RunResult): Action[] {
if (!run?.actions) return [];
return run.actions.map(action => toCanonicalAction(action, run.status));
}
@@ -297,7 +297,7 @@
if (run.status !== RunStatus.COMPLETED) {
return iconFor(run.status);
} else {
- const category = worstCategory(run);
+ const category = run.worstCategory;
return category ? iconFor(category) : iconFor(run.status);
}
}
@@ -340,16 +340,16 @@
);
}
-export function hasResultsOf(run: CheckRun, category: Category) {
+export function hasResultsOf(run: CheckRunApi, category: Category) {
return getResultsOf(run, category).length > 0;
}
-export function getResultsOf(run: CheckRun, category: Category) {
+export function getResultsOf(run: CheckRunApi, category: Category) {
return (run.results ?? []).filter(r => r.category === category);
}
export function compareByWorstCategory(a: CheckRun, b: CheckRun) {
- const catComp = catLevel(worstCategory(b)) - catLevel(worstCategory(a));
+ const catComp = catLevel(b.worstCategory) - catLevel(a.worstCategory);
if (catComp !== 0) return catComp;
const statusComp = runLevel(b.status) - runLevel(a.status);
return statusComp;
@@ -490,6 +490,7 @@
isSingleAttempt: false,
isLatestAttempt: false,
attemptDetails: [],
+ worstCategory: worstCategory(run),
results: (run.results ?? []).map(fromApiToInternalResult),
};
}
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 205bda4..38d50d5 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -1129,6 +1129,8 @@
pluginName: 'test-plugin-name',
summary: 'This is the test summary.',
message: 'This is the test message.',
+ status: RunStatus.COMPLETED,
+ attemptDetails: [{attempt: 'latest'}],
};
}
