Merge "Fix scrolling comments into view"
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index 8395d12..fa9a820 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -89,7 +89,10 @@
.map(query -> query.replaceAll("\\$\\{user}", "self"))
.collect(toImmutableList());
public static final ImmutableSet<ListChangesOption> DASHBOARD_OPTIONS =
- ImmutableSet.of(ListChangesOption.LABELS, ListChangesOption.DETAILED_ACCOUNTS);
+ ImmutableSet.of(
+ ListChangesOption.LABELS,
+ ListChangesOption.DETAILED_ACCOUNTS,
+ ListChangesOption.SUBMIT_REQUIREMENTS);
public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS =
ImmutableSet.of(
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 03886a9..4a17b5c 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -901,7 +901,7 @@
Config rc, String section, String subsection, String varName, boolean useRange) {
Permission.Builder perm = Permission.builder(varName);
loadPermissionRules(rc, section, subsection, varName, perm, useRange);
- return ImmutableList.copyOf(perm.build().getRules());
+ return perm.build().getRules();
}
private void loadPermissionRules(
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
index 402bb51..df836e0 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -14,13 +14,13 @@
package com.google.gerrit.server.project;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
-import java.util.Map;
public interface SubmitRequirementsEvaluator {
/**
@@ -31,7 +31,7 @@
* @param includeLegacy if set to true, evaluate legacy {@link
* com.google.gerrit.entities.SubmitRecord}s and convert them to submit requirements.
*/
- Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ ImmutableMap<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
ChangeData cd, boolean includeLegacy);
/** Evaluate a single {@link SubmitRequirement} using change data. */
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 64c9a4c..16645a5 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -82,7 +82,7 @@
}
@Override
- public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ public ImmutableMap<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
ChangeData cd, boolean includeLegacy) {
Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements = getRequirements(cd);
Map<SubmitRequirement, SubmitRequirementResult> result = projectConfigRequirements;
diff --git a/plugins/gitiles b/plugins/gitiles
index 97ce60f..a0709a4 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 97ce60f8bb4dbf40dde79cf56db6425c384dabcf
+Subproject commit a0709a402ee1d4fe3921fd81e575ec48a053cc9f
diff --git a/polygerrit-ui/app/api/checks.ts b/polygerrit-ui/app/api/checks.ts
index d1c320f..17b0ba0 100644
--- a/polygerrit-ui/app/api/checks.ts
+++ b/polygerrit-ui/app/api/checks.ts
@@ -97,6 +97,11 @@
actions?: Action[];
/**
+ * Shown prominently in the change summary below the run chips.
+ */
+ summaryMessage?: string;
+
+ /**
* Top-level links that are not associated with a specific run or result.
* Will be shown as icons in the header of the Checks tab.
*/
diff --git a/polygerrit-ui/app/api/reporting.ts b/polygerrit-ui/app/api/reporting.ts
index c3655bb..40474e1 100644
--- a/polygerrit-ui/app/api/reporting.ts
+++ b/polygerrit-ui/app/api/reporting.ts
@@ -18,6 +18,27 @@
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type EventDetails = any;
+export enum Deduping {
+ /**
+ * Only report the event once per session, even if the event details are
+ * different.
+ */
+ EVENT_ONCE_PER_SESSION = 'EVENT_ONCE_PER_SESSION',
+ /**
+ * Only report the event once per change, even if the event details are
+ * different.
+ */
+ EVENT_ONCE_PER_CHANGE = 'EVENT_ONCE_PER_CHANGE',
+ /** Only report these exact event details once per session. */
+ DETAILS_ONCE_PER_SESSION = 'DETAILS_ONCE_PER_SESSION',
+ /** Only report these exact event details once per change. */
+ DETAILS_ONCE_PER_CHANGE = 'DETAILS_ONCE_PER_CHANGE',
+}
+export declare interface ReportingOptions {
+ /** Set this, if you don't want to report *every* time. */
+ deduping?: Deduping;
+}
+
export declare interface ReportingPluginApi {
reportInteraction(eventName: string, details?: EventDetails): void;
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts
index 78fffe3..52747b3 100644
--- a/polygerrit-ui/app/constants/reporting.ts
+++ b/polygerrit-ui/app/constants/reporting.ts
@@ -108,4 +108,5 @@
COMMENT_SAVED = 'comment-saved',
DISCARD_COMMENT = 'discard-comment',
COMMENT_DISCARDED = 'comment-discarded',
+ CHECKS_TAB_RENDERED = 'checks-tab-rendered',
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
index 84cf6d9..b4fa7cc 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
@@ -75,10 +75,10 @@
></gr-user-header>
<h1 class="assistive-tech-only">Dashboard</h1>
<gr-change-list
- show-star=""
+ showstar=""
account="[[account]]"
preferences="[[preferences]]"
- selected-index="[[_selectedChangeIndex]]"
+ selectedindex="[[_selectedChangeIndex]]"
sections="[[_results]]"
on-selected-index-changed="_handleSelectedIndexChanged"
on-toggle-star="_handleToggleStar"
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 c90359e..3298e53 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
@@ -405,6 +405,9 @@
@state()
actions: Action[] = [];
+ @state()
+ messages: string[] = [];
+
private showAllChips = new Map<RunStatus | Category, boolean>();
private getCommentsModel = resolve(this, commentsModelToken);
@@ -447,6 +450,11 @@
);
subscribe(
this,
+ this.checksModel.topLevelMessagesLatest$,
+ x => (this.messages = x)
+ );
+ subscribe(
+ this,
this.getCommentsModel().changeComments$,
x => (this.changeComments = x)
);
@@ -556,10 +564,18 @@
.actions #moreMessage {
display: none;
}
+ .summaryMessage {
+ line-height: var(--line-height-normal);
+ color: var(--primary-text-color);
+ }
`,
];
}
+ private renderSummaryMessage() {
+ return this.messages.map(m => html`<div class="summaryMessage">${m}</div>`);
+ }
+
private renderActions() {
const actions = this.actions ?? [];
const summaryActions = actions.filter(a => a.summary).slice(0, 2);
@@ -794,7 +810,8 @@
class="loadingSpin"
?hidden="${!this.someProvidersAreLoading}"
></span>
- ${this.renderErrorMessages()}${this.renderChecksLogin()}${this.renderActions()}
+ ${this.renderErrorMessages()} ${this.renderChecksLogin()}
+ ${this.renderSummaryMessage()} ${this.renderActions()}
</div>
</td>
</tr>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 18eda12..0f19016 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -2152,29 +2152,28 @@
if (isLocationChange) {
this._editingCommitMessage = false;
- const relatedChangesLoaded = coreDataPromise.then(() => {
- let relatedChangesPromise:
- | Promise<RelatedChangesInfo | undefined>
- | undefined;
- const patchNum = this._computeLatestPatchNum(this._allPatchSets);
- if (this._change && patchNum) {
- relatedChangesPromise = this.restApiService
- .getRelatedChanges(this._change._number, patchNum)
- .then(response => {
- if (this._change && response) {
- this.hasParent = this._calculateHasParent(
- this._change.change_id,
- response.changes
- );
- }
- return response;
- });
- }
- // TODO: use returned Promise
- this.getRelatedChangesList()?.reload(relatedChangesPromise);
- });
- allDataPromises.push(relatedChangesLoaded);
}
+ const relatedChangesLoaded = coreDataPromise.then(() => {
+ let relatedChangesPromise:
+ | Promise<RelatedChangesInfo | undefined>
+ | undefined;
+ const patchNum = this._computeLatestPatchNum(this._allPatchSets);
+ if (this._change && patchNum) {
+ relatedChangesPromise = this.restApiService
+ .getRelatedChanges(this._change._number, patchNum)
+ .then(response => {
+ if (this._change && response) {
+ this.hasParent = this._calculateHasParent(
+ this._change.change_id,
+ response.changes
+ );
+ }
+ return response;
+ });
+ }
+ return this.getRelatedChangesList()?.reload(relatedChangesPromise);
+ });
+ allDataPromises.push(relatedChangesLoaded);
Promise.all(allDataPromises).then(() => {
// Loading of commments data is no longer part of this reporting
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 4ca593a..c432276 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1448,15 +1448,27 @@
assert.isTrue(recreateSpy.calledOnce);
});
- test('related changes are not updated after other action', async () => {
- sinon.stub(element, 'loadData').callsFake(() => Promise.resolve());
+ test('related changes are updated when loadData is called', async () => {
await flush();
const relatedChanges = element.shadowRoot!.querySelector(
'#relatedChanges'
) as GrRelatedChangesList;
- sinon.stub(relatedChanges, 'reload');
+ const reloadStub = sinon.stub(relatedChanges, 'reload');
+ stubRestApi('getMergeable').returns(
+ Promise.resolve({...createMergeable(), mergeable: true})
+ );
+
+ element.params = createAppElementChangeViewParams();
+ element.changeModel.setState({
+ loadingStatus: LoadingStatus.LOADED,
+ change: {
+ ...createChangeViewChange(),
+ },
+ });
+
await element.loadData(true);
assert.isFalse(navigateToChangeStub.called);
+ assert.isTrue(reloadStub.called);
});
test('_computeCopyTextForTitle', () => {
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 4294e3f..38430b0 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
@@ -171,17 +171,17 @@
</tr>
</thead>
<tbody>
- ${submit_requirements.map(requirement =>
- this.renderRequirement(requirement)
+ ${submit_requirements.map((requirement, index) =>
+ this.renderRequirement(requirement, index)
)}
</tbody>
</table>
${this.disableHovercards
? ''
: submit_requirements.map(
- requirement => html`
+ (requirement, index) => html`
<gr-submit-requirement-hovercard
- for="requirement-${charsOnly(requirement.name)}"
+ for="requirement-${index}-${charsOnly(requirement.name)}"
.requirement="${requirement}"
.change="${this.change}"
.account="${this.account}"
@@ -192,9 +192,9 @@
${this.renderTriggerVotes()}`;
}
- renderRequirement(requirement: SubmitRequirementResultInfo) {
+ renderRequirement(requirement: SubmitRequirementResultInfo, index: number) {
return html`
- <tr id="requirement-${charsOnly(requirement.name)}">
+ <tr id="requirement-${index}-${charsOnly(requirement.name)}">
<td>${this.renderStatus(requirement.status)}</td>
<td class="name">
<gr-limited-text
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
index 5a094ef..323c70f 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
@@ -85,7 +85,7 @@
</tr>
</thead>
<tbody>
- <tr id="requirement-Verified">
+ <tr id="requirement-0-Verified">
<td>
<iron-icon
aria-label="satisfied"
@@ -111,7 +111,7 @@
</tr>
</tbody>
</table>
- <gr-submit-requirement-hovercard for="requirement-Verified">
+ <gr-submit-requirement-hovercard for="requirement-0-Verified">
</gr-submit-requirement-hovercard>
`);
});
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 4929b7c..f4bbc5d 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -676,6 +676,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -683,6 +684,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -690,6 +692,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -697,6 +700,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -704,6 +708,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -711,6 +716,7 @@
[],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
}
@@ -721,6 +727,7 @@
[fakeRun0],
fakeActions,
fakeLinks,
+ 'ETA: 1 min',
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -728,6 +735,7 @@
[fakeRun1],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -735,6 +743,7 @@
[fakeRun2],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -742,6 +751,7 @@
[fakeRun3],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -749,6 +759,7 @@
fakeRun4Att,
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
this.checksModel.updateStateSetResults(
@@ -756,6 +767,7 @@
[fakeRun5],
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
}
@@ -764,7 +776,8 @@
plugin: string,
runs: CheckRun[],
actions: Action[] = [],
- links: Link[] = []
+ links: Link[] = [],
+ summaryMessage: string | undefined = undefined
) {
const newRuns = this.runs.includes(runs[0]) ? [] : runs;
this.checksModel.updateStateSetResults(
@@ -772,6 +785,7 @@
newRuns,
actions,
links,
+ summaryMessage,
ChecksPatchset.LATEST
);
}
@@ -843,7 +857,13 @@
<gr-button
link
@click="${() =>
- this.toggle('f0', [fakeRun0], fakeActions, fakeLinks)}"
+ this.toggle(
+ 'f0',
+ [fakeRun0],
+ fakeActions,
+ fakeLinks,
+ 'ETA: 1 min'
+ )}"
>0</gr-button
>
<gr-button link @click="${() => this.toggle('f1', [fakeRun1])}"
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index a9c30c5..0ffde6f 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -26,6 +26,8 @@
import {ChecksTabState} from '../../types/events';
import {getAppContext} from '../../services/app-context';
import {subscribe} from '../lit/subscription-controller';
+import {Deduping} from '../../api/reporting';
+import {Interaction} from '../../constants/reporting';
/**
* The "Checks" tab on the Gerrit change page. Gets its data from plugins that
@@ -65,6 +67,8 @@
private readonly checksModel = getAppContext().checksModel;
+ private readonly reporting = getAppContext().reportingService;
+
constructor() {
super();
subscribe(
@@ -113,6 +117,11 @@
}
override render() {
+ this.reporting.reportInteraction(
+ Interaction.CHECKS_TAB_RENDERED,
+ this.tabState,
+ {deduping: Deduping.DETAILS_ONCE_PER_CHANGE}
+ );
return html`
<div class="container">
<gr-checks-runs
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 2bdee51..e58e594 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -122,6 +122,7 @@
errorMessage?: string;
/** Presence of loginCallback implicitly means that the provider is in NOT_LOGGED_IN state. */
loginCallback?: () => void;
+ summaryMessage?: string;
runs: CheckRun[];
actions: Action[];
links: Link[];
@@ -239,6 +240,13 @@
)
);
+ public topLevelMessagesLatest$ = select(this.checksLatest$, state => {
+ const messages = Object.values(state).map(
+ providerState => providerState.summaryMessage
+ );
+ return messages.filter(m => m !== undefined) as string[];
+ });
+
public topLevelActionsSelected$ = select(this.checksSelected$, state =>
Object.values(state).reduce(
(allActions: Action[], providerState: ChecksProviderState) => [
@@ -444,6 +452,7 @@
runs: CheckRunApi[],
actions: Action[] = [],
links: Link[] = [],
+ summaryMessage: string | undefined,
patchset: ChecksPatchset
) {
const attemptMap = createAttemptMap(runs);
@@ -483,6 +492,7 @@
}),
actions: [...actions],
links: [...links],
+ summaryMessage,
};
this.subject$.next(nextState);
}
@@ -701,6 +711,7 @@
response.runs ?? [],
response.actions ?? [],
response.links ?? [],
+ response.summaryMessage,
patchset
);
break;
diff --git a/polygerrit-ui/app/services/checks/checks-model_test.ts b/polygerrit-ui/app/services/checks/checks-model_test.ts
index 1bb0f8a..c2588fe 100644
--- a/polygerrit-ui/app/services/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/services/checks/checks-model_test.ts
@@ -120,6 +120,7 @@
RUNS,
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
assert.isFalse(current.loading);
@@ -132,6 +133,7 @@
RUNS,
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
assert.isFalse(current.loading);
@@ -144,6 +146,7 @@
RUNS,
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
assert.lengthOf(current.runs, 1);
@@ -156,6 +159,7 @@
RUNS,
[],
[],
+ undefined,
ChecksPatchset.LATEST
);
assert.equal(
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 518716b..0da8b4c 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -16,7 +16,7 @@
*/
import {Finalizable} from '../registry';
import {NumericChangeId} from '../../types/common';
-import {EventDetails} from '../../api/reporting';
+import {EventDetails, ReportingOptions} from '../../api/reporting';
import {PluginApi} from '../../api/plugin';
import {
Execution,
@@ -113,7 +113,8 @@
): void;
reportInteraction(
eventName: string | Interaction,
- details?: EventDetails
+ details?: EventDetails,
+ options?: ReportingOptions
): void;
reportErrorDialog(message: string): void;
setRepoName(repoName: string): void;
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index a01e9db..bf10da9 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -18,7 +18,7 @@
import {EventValue, ReportingService, Timer} from './gr-reporting';
import {hasOwnProperty} from '../../utils/common-util';
import {NumericChangeId} from '../../types/common';
-import {EventDetails} from '../../api/reporting';
+import {Deduping, EventDetails, ReportingOptions} from '../../api/reporting';
import {PluginApi} from '../../api/plugin';
import {Finalizable} from '../registry';
import {
@@ -285,10 +285,10 @@
private slowRpcList: SlowRpcCall[] = [];
/**
- * Keeps track of which ids were already reported to have been executed.
- * Execution ids should only be reported once per session.
+ * Keeps track of which ids were already reported for events that should only
+ * be reported once per session.
*/
- private executionReported = new Set<string>();
+ private reportedIds = new Set<string>();
public readonly hiddenDurationTimer = new HiddenDurationTimer();
@@ -815,7 +815,43 @@
);
}
- reportInteraction(eventName: string | Interaction, details: EventDetails) {
+ /**
+ * Returns true when the event was deduped and thus should not be reported.
+ */
+ _dedup(
+ eventName: string | Interaction,
+ details: EventDetails,
+ deduping?: Deduping
+ ): boolean {
+ if (!deduping) return false;
+ let id = '';
+ switch (deduping) {
+ case Deduping.DETAILS_ONCE_PER_CHANGE:
+ id = `${eventName}-${this.reportChangeId}-${JSON.stringify(details)}`;
+ break;
+ case Deduping.DETAILS_ONCE_PER_SESSION:
+ id = `${eventName}-${JSON.stringify(details)}`;
+ break;
+ case Deduping.EVENT_ONCE_PER_CHANGE:
+ id = `${eventName}-${this.reportChangeId}`;
+ break;
+ case Deduping.EVENT_ONCE_PER_SESSION:
+ id = `${eventName}`;
+ break;
+ default:
+ throw new Error(`Invalid 'deduping' option '${deduping}'.`);
+ }
+ if (this.reportedIds.has(id)) return true;
+ this.reportedIds.add(id);
+ return false;
+ }
+
+ reportInteraction(
+ eventName: string | Interaction,
+ details: EventDetails,
+ options?: ReportingOptions
+ ) {
+ if (this._dedup(eventName, details, options?.deduping)) return;
this.reporter(
INTERACTION.TYPE,
INTERACTION.CATEGORY.DEFAULT,
@@ -827,9 +863,7 @@
}
reportExecution(name: Execution, details?: EventDetails) {
- const id = `${name}${JSON.stringify(details)}`;
- if (this.executionReported.has(id)) return;
- this.executionReported.add(id);
+ if (this._dedup(name, details, Deduping.DETAILS_ONCE_PER_SESSION)) return;
this.reporter(
LIFECYCLE.TYPE,
LIFECYCLE.CATEGORY.EXECUTION,
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.js b/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.js
index 8068dc00..990a5c8 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.js
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.js
@@ -18,6 +18,7 @@
import '../../test/common-test-setup-karma.js';
import {GrReporting, DEFAULT_STARTUP_TIMERS, initErrorReporter} from './gr-reporting_impl.js';
import {getAppContext} from '../app-context.js';
+import {Deduping} from '../../api/reporting.js';
suite('gr-reporting tests', () => {
let service;
@@ -347,6 +348,44 @@
assert.equal(dispatchStub.getCall(2).args[0].detail.eventStart, 42);
});
+ test('dedup', () => {
+ assert.isFalse(service._dedup('a', undefined, undefined));
+ assert.isFalse(service._dedup('a', undefined, undefined));
+
+ let deduping = Deduping.EVENT_ONCE_PER_SESSION;
+ assert.isFalse(service._dedup('b', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('b', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('b', {x: 'bar'}, deduping));
+
+ deduping = Deduping.DETAILS_ONCE_PER_SESSION;
+ assert.isFalse(service._dedup('c', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('c', {x: 'foo'}, deduping));
+ assert.isFalse(service._dedup('c', {x: 'bar'}, deduping));
+ assert.isTrue(service._dedup('c', {x: 'bar'}, deduping));
+
+ deduping = Deduping.EVENT_ONCE_PER_CHANGE;
+ service.setChangeId(1);
+ assert.isFalse(service._dedup('d', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('d', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('d', {x: 'bar'}, deduping));
+ service.setChangeId(2);
+ assert.isFalse(service._dedup('d', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('d', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('d', {x: 'bar'}, deduping));
+
+ deduping = Deduping.DETAILS_ONCE_PER_CHANGE;
+ service.setChangeId(1);
+ assert.isFalse(service._dedup('e', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('e', {x: 'foo'}, deduping));
+ assert.isFalse(service._dedup('e', {x: 'bar'}, deduping));
+ assert.isTrue(service._dedup('e', {x: 'bar'}, deduping));
+ service.setChangeId(2);
+ assert.isFalse(service._dedup('e', {x: 'foo'}, deduping));
+ assert.isTrue(service._dedup('e', {x: 'foo'}, deduping));
+ assert.isFalse(service._dedup('e', {x: 'bar'}, deduping));
+ assert.isTrue(service._dedup('e', {x: 'bar'}, deduping));
+ });
+
suite('plugins', () => {
setup(() => {
service.reporter.restore();
