Skip to content

Commit f3c99d7

Browse files
hugohaggmarkhugohaggmark
authored
fix: stop duplicate history entries on load (#849)
* fix: stop duplicate history entries on load * chore: updates after PR feedback
1 parent a416a4b commit f3c99d7

5 files changed

Lines changed: 131 additions & 13 deletions

File tree

‎e2e/navigation.spec.ts‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { expect, test } from '@grafana/plugin-e2e';
22
import { ExplorePage } from './fixtures/explore';
3+
import { getTestIdFromMetric, testIds } from '../src/utils/testIds';
4+
import type { Page } from '@playwright/test';
35

46
test.describe('navigating app', () => {
57
let explorePage: ExplorePage;
6-
8+
79
test.beforeEach(async ({ page }) => {
810
explorePage = new ExplorePage(page);
911
await explorePage.gotoExplorePage();
@@ -19,3 +21,81 @@ test.describe('navigating app', () => {
1921
await explorePage.assertMissingData();
2022
});
2123
});
24+
25+
test.describe('ensure back button works for main actions', () => {
26+
let explorePage: ExplorePage;
27+
28+
test.beforeEach(async ({ page }) => {
29+
explorePage = new ExplorePage(page);
30+
await explorePage.gotoExplorePage();
31+
await explorePage.assertNotLoading();
32+
});
33+
34+
test.afterEach(async () => {
35+
await explorePage.unroute();
36+
});
37+
38+
test('clicking on errors panel, browser back and browser forward should work as expected', async ({ page }) => {
39+
await assertBackAndForwardNavigationWorks(page, 'rate', 'errors');
40+
});
41+
42+
test('clicking on duration panel, browser back and browser forward should work as expected', async ({ page }) => {
43+
await assertBackAndForwardNavigationWorks(page, 'rate', 'duration');
44+
});
45+
});
46+
47+
type MetricType = 'rate' | 'errors' | 'duration';
48+
49+
async function assertBackAndForwardNavigationWorks(page: Page, startMetric: MetricType, switchToMetric: MetricType) {
50+
const explorePage = new ExplorePage(page);
51+
await explorePage.assertNotLoading();
52+
53+
await assertREDPanelRadioVisible(page, startMetric);
54+
await assertREDPanelRadioVisible(page, switchToMetric);
55+
56+
await assertCheckedForREDPanelRadio(page, startMetric);
57+
await assertUnCheckedForREDPanelRadio(page, switchToMetric);
58+
59+
await clickOnREDPanelRadio(page, switchToMetric);
60+
await explorePage.assertNotLoading();
61+
62+
await assertCheckedForREDPanelRadio(page, switchToMetric);
63+
await assertUnCheckedForREDPanelRadio(page, startMetric);
64+
65+
await expect(page.getByTestId(testIds.errorState)).not.toBeVisible();
66+
67+
await page.goBack();
68+
await explorePage.assertNotLoading();
69+
70+
await assertCheckedForREDPanelRadio(page, startMetric);
71+
await assertUnCheckedForREDPanelRadio(page, switchToMetric);
72+
73+
await expect(page.getByTestId(testIds.errorState)).not.toBeVisible();
74+
75+
await page.goForward();
76+
await explorePage.assertNotLoading();
77+
78+
await assertCheckedForREDPanelRadio(page, switchToMetric);
79+
await assertUnCheckedForREDPanelRadio(page, startMetric);
80+
81+
await expect(page.getByTestId(testIds.errorState)).not.toBeVisible();
82+
}
83+
84+
async function assertREDPanelRadioVisible(page: Page, metric: MetricType) {
85+
await expect(page.getByTestId(getTestIdFromMetric(metric))).toBeVisible();
86+
await expect(page.getByTestId(getTestIdFromMetric(metric)).getByRole('radio').first()).toBeVisible();
87+
}
88+
89+
async function assertCheckedForREDPanelRadio(page: Page, metric: MetricType) {
90+
// toBeChecked() is flaky here: `checked` property flickers during re-renders; the attribute is stable.
91+
await expect(page.getByTestId(getTestIdFromMetric(metric)).getByRole('radio').first()).toHaveAttribute('checked');
92+
}
93+
94+
async function assertUnCheckedForREDPanelRadio(page: Page, metric: MetricType) {
95+
// toBeChecked() is flaky here: `checked` property flickers during re-renders; the attribute is stable.
96+
await expect(page.getByTestId(getTestIdFromMetric(metric)).getByRole('radio').first()).not.toHaveAttribute('checked');
97+
}
98+
99+
async function clickOnREDPanelRadio(page: Page, metric: MetricType) {
100+
await page.getByTestId(getTestIdFromMetric(metric)).getByRole('radio').first().click();
101+
}

‎src/components/Explore/TracesByService/MiniREDPanel.tsx‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,19 @@ import { getMetricsTempoQuery } from '../queries/generateMetricsQuery';
2020
import { StepQueryRunner } from '../queries/StepQueryRunner';
2121
import { RadioButtonList, useStyles2 } from '@grafana/ui';
2222
import { css } from '@emotion/css';
23-
import { fieldHasEmptyValues, getOpenTrace, getTraceExplorationScene, getUrlForExploration } from '../../../utils/utils';
23+
import {
24+
fieldHasEmptyValues,
25+
getOpenTrace,
26+
getTraceExplorationScene,
27+
getUrlForExploration,
28+
} from '../../../utils/utils';
2429
import { MINI_PANEL_HEIGHT } from './TracesByServiceScene';
2530
import { buildHistogramQuery } from '../queries/histogram';
2631
import { histogramPanelConfig } from '../panels/histogram';
2732
import { reportAppInteraction, USER_EVENTS_ACTIONS, USER_EVENTS_PAGES } from 'utils/analytics';
2833
import { exemplarsTransformations, removeExemplarsTransformation } from '../../../utils/exemplars';
2934
import { StreamingIndicator } from '../StreamingIndicator';
35+
import { getTestIdFromMetric } from 'utils/testIds';
3036

3137
export interface MiniREDPanelState extends SceneObjectState {
3238
panel?: SceneFlexLayout;
@@ -94,7 +100,11 @@ export class MiniREDPanel extends SceneObjectBase<MiniREDPanelState> {
94100
$data: new StepQueryRunner({
95101
maxDataPoints: this.state.metric === 'duration' ? 24 : 64,
96102
datasource: explorationDS,
97-
queries: [this.state.metric === 'duration' ? buildHistogramQuery() : getMetricsTempoQuery({ metric: this.state.metric, sample: true })],
103+
queries: [
104+
this.state.metric === 'duration'
105+
? buildHistogramQuery()
106+
: getMetricsTempoQuery({ metric: this.state.metric, sample: true }),
107+
],
98108
}),
99109
transformations:
100110
this.state.metric === 'duration' || this.state.embeddedMini
@@ -158,7 +168,11 @@ export class MiniREDPanel extends SceneObjectBase<MiniREDPanelState> {
158168
}
159169

160170
return (
161-
<div className={css([styles.container, styles.clickable])} onClick={() => selectMetric(embeddedMini)}>
171+
<div
172+
className={css([styles.container, styles.clickable])}
173+
onClick={() => selectMetric(embeddedMini)}
174+
data-testid={getTestIdFromMetric(model.state.metric)}
175+
>
162176
{!embeddedMini && (
163177
<div className={styles.headerWrapper}>
164178
<RadioButtonList

‎src/components/Explore/TracesByService/REDPanel.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { exemplarsTransformations, removeExemplarsTransformation } from '../../.
4343
import { locationService } from '@grafana/runtime';
4444
import { TIME_SEEKER_FEATURE_FLAG_KEY, useFlagTracesDrilldownTimeSeeker } from 'featureFlags/featureFlags';
4545
import { reportAppInteraction, USER_EVENTS_ACTIONS, USER_EVENTS_PAGES } from 'utils/analytics';
46+
import { getTestIdFromMetric } from 'utils/testIds';
4647

4748
export interface RateMetricsPanelState extends SceneObjectState {
4849
panel?: SceneFlexLayout;
@@ -323,7 +324,11 @@ export class REDPanel extends SceneObjectBase<RateMetricsPanelState> {
323324
};
324325

325326
return (
326-
<div className={styles.container} onClick={() => selectMetric(embeddedMini)}>
327+
<div
328+
className={styles.container}
329+
onClick={() => selectMetric(embeddedMini)}
330+
data-testid={getTestIdFromMetric(metric)}
331+
>
327332
{!embeddedMini && (
328333
<div className={styles.headerContainer}>
329334
<div className={styles.titleContainer}>

‎src/components/Explore/TracesByService/Tabs/Breakdown/AttributesBreakdownScene.tsx‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,13 @@ export class AttributesBreakdownScene extends SceneObjectBase<AttributesBreakdow
121121

122122
public onChange = (value: string, ignore?: boolean) => {
123123
const variable = getGroupByVariable(this);
124-
if (variable.getValueText() !== value) {
125-
variable.changeValueTo(value, undefined, !ignore);
124+
if (variable.getValueText() === value) {
125+
return;
126+
}
127+
128+
variable.changeValueTo(value, undefined, !ignore);
126129

130+
if (!ignore) {
127131
reportAppInteraction(
128132
USER_EVENTS_PAGES.analyse_traces,
129133
USER_EVENTS_ACTIONS.analyse_traces.breakdown_group_by_changed,
@@ -164,7 +168,7 @@ export class AttributesBreakdownScene extends SceneObjectBase<AttributesBreakdow
164168

165169
useEffect(() => {
166170
if (!groupBy || groupBy === 'All' || groupBy === '') {
167-
model.onChange(favoriteAttributes[0]);
171+
model.onChange(favoriteAttributes[0], true);
168172
}
169173
// eslint-disable-next-line react-hooks/exhaustive-deps
170174
}, [groupBy]);
@@ -247,11 +251,7 @@ function BreakdownCreateAlertModalBridge({
247251
}
248252

249253
return (
250-
<ModalComponent
251-
panel={payload.panel}
252-
range={payload.range}
253-
onDismiss={() => scene.clearBreakdownCreateAlert()}
254-
/>
254+
<ModalComponent panel={payload.panel} range={payload.range} onDismiss={() => scene.clearBreakdownCreateAlert()} />
255255
);
256256
}
257257

‎src/utils/testIds.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
1+
import { VariableValue } from '@grafana/scenes';
2+
13
export const testIds = {
24
emptyState: 'data-testid empty-state',
35
errorState: 'data-testid error-state',
46
loadingState: 'data-testid loading-state',
7+
ratePanel: `data-testid rate-panel`,
8+
errorsPanel: `data-testid errors-panel`,
9+
durationPanel: `data-testid duration-panel`,
510
};
11+
12+
export function getTestIdFromMetric(metric: VariableValue | string): string {
13+
const value = String(metric);
14+
switch (value) {
15+
case 'rate':
16+
return testIds.ratePanel;
17+
case 'errors':
18+
return testIds.errorsPanel;
19+
case 'duration':
20+
return testIds.durationPanel;
21+
}
22+
23+
return 'data-testid unknown-panel';
24+
}

0 commit comments

Comments
 (0)