Merge "ProjectLevelConfig: Cache parsed config and avoid reparsing"
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/ApprovalInference.java
index 40bf249..d77427a 100644
--- a/java/com/google/gerrit/server/ApprovalInference.java
+++ b/java/com/google/gerrit/server/ApprovalInference.java
@@ -25,6 +25,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.LabelTypes;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
@@ -368,11 +369,12 @@
"change kind for patch set %d of change %d against prior patch set %s is %s",
ps.id().get(), ps.id().changeId().get(), priorPatchSet.getValue().id().changeId(), kind);
PatchList patchList = null;
+ LabelTypes labelTypes = project.getLabelTypes();
for (PatchSetApproval psa : priorApprovals) {
if (resultByUser.contains(psa.label(), psa.accountId())) {
continue;
}
- LabelType type = project.getLabelTypes().byLabel(psa.labelId());
+ LabelType type = labelTypes.byLabel(psa.labelId());
// Only compute patchList if there is a relevant label, since this is expensive.
if (patchList == null && type != null && type.isCopyAllScoresIfListOfFilesDidNotChange()) {
patchList = getPatchList(project, ps, priorPatchSet);
diff --git a/polygerrit-ui/app/api/checks.ts b/polygerrit-ui/app/api/checks.ts
index 799d1f7..4a5ef7e 100644
--- a/polygerrit-ui/app/api/checks.ts
+++ b/polygerrit-ui/app/api/checks.ts
@@ -247,7 +247,7 @@
checkName: string | undefined,
/** Identical to 'name' property of Action entity. */
actionName: string
-) => Promise<ActionResult>;
+) => Promise<ActionResult> | undefined;
export interface ActionResult {
/** An empty errorMessage means success. */
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index ad4f2ae..e5386f4 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -28,6 +28,7 @@
allActions$,
allResults$,
allRuns$,
+ someProvidersAreLoading$,
} from '../../services/checks/checks-model';
import './gr-checks-runs';
import './gr-checks-results';
@@ -41,6 +42,10 @@
import {checkRequiredProperty} from '../../utils/common-util';
import {RunSelectedEvent} from './gr-checks-runs';
import {ChecksTabState} from '../../types/events';
+import {fireAlert} from '../../utils/event-util';
+import {appContext} from '../../services/app-context';
+import {from, timer} from 'rxjs';
+import {takeUntil} from 'rxjs/operators';
/**
* The "Checks" tab on the Gerrit change page. Gets its data from plugins that
@@ -64,9 +69,14 @@
@property()
changeNum: NumericChangeId | undefined = undefined;
+ @property()
+ someProvidersAreLoading = false;
+
@internalProperty()
selectedRuns: string[] = [];
+ private readonly checksService = appContext.checksService;
+
constructor() {
super();
this.subscribe('runs', allRuns$);
@@ -74,6 +84,7 @@
this.subscribe('results', allResults$);
this.subscribe('currentPatchNum', currentPatchNum$);
this.subscribe('changeNum', changeNum$);
+ this.subscribe('someProvidersAreLoading', someProvidersAreLoading$);
this.addEventListener('action-triggered', (e: ActionTriggeredEvent) =>
this.handleActionTriggered(e.detail.action, e.detail.run)
@@ -131,6 +142,7 @@
},
]}"
></gr-dropdown-list>
+ <span ?hidden="${!this.someProvidersAreLoading}">Loading...</span>
</div>
<div class="right">
${this.actions.map(this.renderAction)}
@@ -170,10 +182,7 @@
handleActionTriggered(action: Action, run?: CheckRun) {
if (!this.changeNum) return;
if (!this.currentPatchNum) return;
- // TODO(brohlfs): The callback is supposed to be returning a promise.
- // A toast should be displayed until the promise completes. And then the
- // data should be updated.
- action.callback(
+ const promise = action.callback(
this.changeNum,
this.currentPatchNum as number,
run?.attempt,
@@ -181,6 +190,25 @@
run?.checkName,
action.name
);
+ // Plugins *should* return a promise, but you never know ...
+ if (promise?.then) {
+ const prefix = `Triggering action '${action.name}'`;
+ fireAlert(this, `${prefix} ...`);
+ from(promise)
+ // If the action takes longer than 5 seconds, then most likely the
+ // user is either not interested or the result not relevant anymore.
+ .pipe(takeUntil(timer(5000)))
+ .subscribe(result => {
+ if (result.errorMessage) {
+ fireAlert(this, `${prefix} failed with ${result.errorMessage}.`);
+ } else {
+ fireAlert(this, `${prefix} successful.`);
+ this.checksService.reloadForCheck(run?.checkName);
+ }
+ });
+ } else {
+ fireAlert(this, `Action '${action.name}' triggered.`);
+ }
}
handleRunSelected(e: RunSelectedEvent) {
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 28aca73..d8dc688 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -87,6 +87,18 @@
})
);
+export const checkToPluginMap$ = checksState$.pipe(
+ map(state => {
+ const map = new Map<string, string>();
+ for (const [pluginName, providerState] of Object.entries(state)) {
+ for (const run of providerState.runs) {
+ map.set(run.checkName, pluginName);
+ }
+ }
+ return map;
+ })
+);
+
export const allResults$ = checksState$.pipe(
map(state => {
return Object.values(state)
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts
index b11eada..d2a4775 100644
--- a/polygerrit-ui/app/services/checks/checks-service.ts
+++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -32,6 +32,7 @@
import {change$, currentPatchNum$} from '../change/change-model';
import {
updateStateSetLoading,
+ checkToPluginMap$,
updateStateSetProvider,
updateStateSetResults,
} from './checks-model';
@@ -51,6 +52,12 @@
private changeAndPatchNum$ = change$.pipe(withLatestFrom(currentPatchNum$));
+ private checkToPluginMap = new Map<string, string>();
+
+ constructor() {
+ checkToPluginMap$.subscribe(x => (this.checkToPluginMap = x));
+ }
+
reload(pluginName: string) {
this.reloadSubjects[pluginName].next();
}
@@ -59,6 +66,12 @@
Object.keys(this.providers).forEach(key => this.reload(key));
}
+ reloadForCheck(checkName?: string) {
+ if (!checkName) return;
+ const plugin = this.checkToPluginMap.get(checkName);
+ if (plugin) this.reload(plugin);
+ }
+
register(
pluginName: string,
provider: ChecksProvider,
