Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AngularMigration: Allow dashboard by dashboard migration #84100

Merged
merged 30 commits into from Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
40e3ad1
Add link to auto-migrate panels to Angular dashboard warning
xnyo Feb 26, 2024
dd4ec63
Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link
xnyo Mar 4, 2024
3ce9740
PR review: Use explicitlyControlledMigrationPanels
xnyo Mar 7, 2024
5832d00
Fix unused import
xnyo Mar 7, 2024
61a8b7e
Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link
xnyo Mar 7, 2024
2c7b676
Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link
xnyo Mar 7, 2024
b43c52d
Removed unused import
xnyo Mar 7, 2024
4f32084
Fix `graph-old` not being flagged by Angular deprecation dashboard wa…
xnyo Mar 7, 2024
c40f352
skip overriding migration flags in url
adela-almasan Mar 7, 2024
d0e4804
Merge branch 'giuseppe/angular-deprecation/auto-migrate-link' into mi…
adela-almasan Mar 7, 2024
ffc6391
revert migration banner
adela-almasan Mar 7, 2024
5776761
buttons
adela-almasan Mar 7, 2024
9614dc6
Merge branch 'main' into migration_url
adela-almasan Mar 8, 2024
7c907ec
links
adela-almasan Mar 8, 2024
67711e9
update buttons
adela-almasan Mar 11, 2024
f37597b
open in same tab
adela-almasan Mar 11, 2024
c1d208b
Merge branch 'main' into migration_url
adela-almasan Mar 19, 2024
f0771ce
update deprecation notice test
adela-almasan Mar 19, 2024
211ec99
update text; basic test
adela-almasan Mar 19, 2024
7a900b2
Merge branch 'main' into migration_url
adela-almasan Mar 19, 2024
e6be361
disableAngular
adela-almasan Mar 19, 2024
0fd3b36
betterer - for later
adela-almasan Mar 20, 2024
9390f67
Merge remote-tracking branch 'origin' into migration_url
nmarrs Mar 21, 2024
053e166
Merge branch 'main' into migration_url
adela-almasan Mar 21, 2024
d48785f
remove local storage
adela-almasan Mar 21, 2024
2841332
fix url flags check
adela-almasan Mar 21, 2024
35e7b1c
fix test
adela-almasan Mar 21, 2024
4c2c455
Merge branch 'main' into migration_url
adela-almasan Mar 27, 2024
537f47b
update key
adela-almasan Mar 27, 2024
032389f
update test
adela-almasan Mar 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .betterer.results
Expand Up @@ -2762,7 +2762,8 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "1"],
[0, 0, 0, "Do not use any type assertions.", "2"],
[0, 0, 0, "Unexpected any. Specify a different type.", "3"],
[0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "4"]
[0, 0, 0, "Do not use any type assertions.", "4"],
[0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "5"]
],
"public/app/features/dashboard/containers/DashboardPageProxy.tsx:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"]
Expand Down
8 changes: 6 additions & 2 deletions packages/grafana-runtime/src/config.ts
Expand Up @@ -253,7 +253,7 @@ function overrideFeatureTogglesFromUrl(config: GrafanaBootConfig) {

const isLocalDevEnv = config.buildInfo.env === 'development';

const prodUrlAllowedFeatureFlags = new Set([
const migrationFeatureFlags = new Set([
'autoMigrateOldPanels',
'autoMigrateGraphPanel',
'autoMigrateTablePanel',
Expand All @@ -269,7 +269,11 @@ function overrideFeatureTogglesFromUrl(config: GrafanaBootConfig) {
const featureToggles = config.featureToggles as Record<string, boolean>;
const featureName = key.substring(10);

if (!isLocalDevEnv && !prodUrlAllowedFeatureFlags.has(featureName)) {
if (!isLocalDevEnv && !migrationFeatureFlags.has(featureName)) {
return;
}

if (migrationFeatureFlags.has(featureName)) {
adela-almasan marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand Down
52 changes: 51 additions & 1 deletion public/app/features/dashboard/containers/DashboardPage.tsx
Expand Up @@ -17,6 +17,7 @@ import { getNavModel } from 'app/core/selectors/navModel';
import { PanelModel } from 'app/features/dashboard/state';
import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher';
import { AngularDeprecationNotice } from 'app/features/plugins/angularDeprecation/AngularDeprecationNotice';
import { AngularMigrationNotice } from 'app/features/plugins/angularDeprecation/AngularMigrationNotice';
import { getPageNavFromSlug, getRootContentNavModel } from 'app/features/storage/StorageFolderPage';
import { DashboardRoutes, KioskMode, StoreState } from 'app/types';
import { PanelEditEnteredEvent, PanelEditExitedEvent } from 'app/types/events';
Expand All @@ -36,6 +37,7 @@ import { SubMenu } from '../components/SubMenu/SubMenu';
import { DashboardGrid } from '../dashgrid/DashboardGrid';
import { liveTimer } from '../dashgrid/liveTimer';
import { getTimeSrv } from '../services/TimeSrv';
import { explicitlyControlledMigrationPanels, autoMigrateAngular } from '../state/PanelModel';
import { cleanUpDashboardAndVariables } from '../state/actions';
import { initDashboard } from '../state/initDashboard';

Expand Down Expand Up @@ -319,6 +321,50 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
);
}

const migrationFeatureFlags = new Set([
'autoMigrateOldPanels',
'autoMigrateGraphPanel',
'autoMigrateTablePanel',
'autoMigratePiechartPanel',
'autoMigrateWorldmapPanel',
'autoMigrateStatPanel',
'disableAngular',
]);

const isAutoMigrationFlagSet = () => {
const urlParams = new URLSearchParams(window.location.search);
let isFeatureFlagSet = false;

urlParams.forEach((value, key) => {
if (key.startsWith('__feature.')) {
const featureName = key.substring(10);
const toggleState = value === 'true' || value === '';
const featureToggles = config.featureToggles as Record<string, boolean>;

if (featureToggles[featureName]) {
return;
}

if (migrationFeatureFlags.has(featureName) && toggleState) {
isFeatureFlagSet = true;
return;
}
}
});

return isFeatureFlagSet;
};

const dashboardWasAngular = dashboard.panels.some(
(panel) => panel.autoMigrateFrom && autoMigrateAngular[panel.autoMigrateFrom] != null
);

const showDashboardMigrationNotice =
config.featureToggles.angularDeprecationUI &&
dashboardWasAngular &&
isAutoMigrationFlagSet() &&
dashboard.uid !== null;

return (
<>
<Page
Expand Down Expand Up @@ -349,8 +395,12 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
</section>
)}
{config.featureToggles.angularDeprecationUI && dashboard.hasAngularPlugins() && dashboard.uid !== null && (
<AngularDeprecationNotice dashboardUid={dashboard.uid} />
<AngularDeprecationNotice
dashboardUid={dashboard.uid}
showAutoMigrateLink={dashboard.panels.some((x) => explicitlyControlledMigrationPanels.includes(x.type))}
adela-almasan marked this conversation as resolved.
Show resolved Hide resolved
/>
)}
{showDashboardMigrationNotice && <AngularMigrationNotice dashboardUid={dashboard.uid} />}
<DashboardGrid
dashboard={dashboard}
isEditable={!!dashboard.meta.canEdit}
Expand Down
6 changes: 4 additions & 2 deletions public/app/features/dashboard/state/DashboardModel.ts
Expand Up @@ -44,7 +44,7 @@ import { getTimeSrv } from '../services/TimeSrv';
import { mergePanels, PanelMergeInfo } from '../utils/panelMerge';

import { DashboardMigrator } from './DashboardMigrator';
import { PanelModel } from './PanelModel';
import { explicitlyControlledMigrationPanels, PanelModel } from './PanelModel';
import { TimeModel } from './TimeModel';
import { deleteScopeVars, isOnTheSameGridRow } from './utils';

Expand Down Expand Up @@ -1294,8 +1294,10 @@ export class DashboardModel implements TimeModel {
return this.panels.some((panel) => {
// Return false for plugins that are angular but have angular.hideDeprecation = false
// We cannot use panel.plugin.isAngularPlugin() because panel.plugin may not be initialized at this stage.
// We also have to check for old core angular plugins (explicitlyControlledMigrationPanels).
const isAngularPanel =
config.panels[panel.type]?.angular?.detected && !config.panels[panel.type]?.angular?.hideDeprecation;
(config.panels[panel.type]?.angular?.detected || explicitlyControlledMigrationPanels.includes(panel.type)) &&
!config.panels[panel.type]?.angular?.hideDeprecation;
let isAngularDs = false;
if (panel.datasource?.uid) {
isAngularDs = isAngularDatasourcePluginAndNotHidden(panel.datasource?.uid);
Expand Down
Expand Up @@ -7,12 +7,30 @@ export function getPanelPluginToMigrateTo(panel: any, forceMigration?: boolean):
return autoMigrateRemovedPanelPlugins[panel.type];
}

const isUrlFeatureFlagEnabled = (featureName: string) => {
const flag = '__feature.' + featureName;

const urlParams = new URLSearchParams(window.location.search);
const featureFlagValue = urlParams.get(flag);

return featureFlagValue === 'true' || featureFlagValue === '';
};
xnyo marked this conversation as resolved.
Show resolved Hide resolved

// Auto-migrate old angular panels
const shouldMigrateAllAngularPanels =
forceMigration || !config.angularSupportEnabled || config.featureToggles.autoMigrateOldPanels;
forceMigration ||
!config.angularSupportEnabled ||
config.featureToggles.autoMigrateOldPanels ||
isUrlFeatureFlagEnabled('autoMigrateOldPanels') ||
isUrlFeatureFlagEnabled('disableAngular');

// Graph needs special logic as it can be migrated to multiple panels
if (panel.type === 'graph' && (shouldMigrateAllAngularPanels || config.featureToggles.autoMigrateGraphPanel)) {
if (
panel.type === 'graph' &&
(shouldMigrateAllAngularPanels ||
config.featureToggles.autoMigrateGraphPanel ||
isUrlFeatureFlagEnabled('autoMigrateGraphPanel'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this ever happen via the "try migration" button in the banner? Or does that always set the autoMigrateOldPanels feature toggle to true? In which case this code is meant to support the manual case of testing granular panel by panel migration via url query params?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this gets triggered via "try migration", it's not setting/allowing any feature flags and it's just checking the url for the ff and migrates only the current dashboard.

) {
if (panel.xaxis?.mode === 'series') {
return 'barchart';
}
Expand All @@ -28,21 +46,31 @@ export function getPanelPluginToMigrateTo(panel: any, forceMigration?: boolean):
return autoMigrateAngular[panel.type];
}

if (panel.type === 'table-old' && config.featureToggles.autoMigrateTablePanel) {
if (
panel.type === 'table-old' &&
(config.featureToggles.autoMigrateTablePanel || isUrlFeatureFlagEnabled('autoMigrateTablePanel'))
) {
return 'table';
}

if (panel.type === 'grafana-piechart-panel' && config.featureToggles.autoMigratePiechartPanel) {
if (
panel.type === 'grafana-piechart-panel' &&
(config.featureToggles.autoMigratePiechartPanel || isUrlFeatureFlagEnabled('autoMigratePiechartPanel'))
) {
return 'piechart';
}

if (panel.type === 'grafana-worldmap-panel' && config.featureToggles.autoMigrateWorldmapPanel) {
if (
panel.type === 'grafana-worldmap-panel' &&
(config.featureToggles.autoMigrateWorldmapPanel || isUrlFeatureFlagEnabled('autoMigrateWorldmapPanel'))
) {
return 'geomap';
}

if (
(panel.type === 'singlestat' || panel.type === 'grafana-singlestat-panel') &&
config.featureToggles.autoMigrateStatPanel
((panel.type === 'singlestat' || panel.type === 'grafana-singlestat-panel') &&
config.featureToggles.autoMigrateStatPanel) ||
isUrlFeatureFlagEnabled('autoMigrateStatPanel')
) {
return 'stat';
}
Expand Down
Expand Up @@ -63,4 +63,24 @@ describe('AngularDeprecationNotice', () => {
await userEvent.click(closeButton);
expect(reportInteraction).toHaveBeenCalledWith('angular_deprecation_notice_dismissed');
});

describe('auto migrate button', () => {
const autoMigrateText = 'Try migration';

it('should display auto migrate button if showAutoMigrateLink is true', () => {
render(<AngularDeprecationNotice dashboardUid={dsUid} showAutoMigrateLink={true} />);
const autoMigrateButton = screen.getByRole('button', { name: /Try migration/i });
expect(autoMigrateButton).toBeInTheDocument();
});

it('should not display auto migrate button if showAutoMigrateLink is false', () => {
render(<AngularDeprecationNotice dashboardUid={dsUid} showAutoMigrateLink={false} />);
expect(screen.queryByText(autoMigrateText)).not.toBeInTheDocument();
});

it('should not display auto migrate link if showAutoMigrateLink is not provided', () => {
render(<AngularDeprecationNotice dashboardUid={dsUid} />);
expect(screen.queryByText(autoMigrateText)).not.toBeInTheDocument();
});
});
});
@@ -1,7 +1,7 @@
import React from 'react';

import { reportInteraction } from '@grafana/runtime';
import { Alert } from '@grafana/ui';
import { Alert, Button } from '@grafana/ui';
import { LocalStorageValueProvider } from 'app/core/components/LocalStorageValueProvider';

const LOCAL_STORAGE_KEY_PREFIX = 'grafana.angularDeprecation.dashboardNotice.isDismissed';
Expand All @@ -12,9 +12,19 @@ function localStorageKey(dashboardUid: string): string {

export interface Props {
dashboardUid: string;
showAutoMigrateLink?: boolean;
}

export function AngularDeprecationNotice({ dashboardUid }: Props) {
function tryMigration() {
const autoMigrateParam = '__feature.autoMigrateOldPanels';
const url = new URL(window.location.toString());
if (!url.searchParams.has(autoMigrateParam)) {
url.searchParams.append(autoMigrateParam, 'true');
}
window.open(url.toString(), '_self');
}

export function AngularDeprecationNotice({ dashboardUid, showAutoMigrateLink }: Props) {
return (
<LocalStorageValueProvider<boolean> storageKey={localStorageKey(dashboardUid)} defaultValue={false}>
{(isDismissed, onDismiss) => {
Expand All @@ -32,18 +42,21 @@ export function AngularDeprecationNotice({ dashboardUid }: Props) {
}}
>
<div className="markdown-html">
<ul>
<li>
<a
href="https://grafana.com/docs/grafana/latest/developers/angular_deprecation/"
className="external-link"
target="_blank"
rel="noreferrer"
>
Read our deprecation notice and migration advice.
</a>
</li>
</ul>
<a
href="https://grafana.com/docs/grafana/latest/developers/angular_deprecation/"
className="external-link"
target="_blank"
rel="noreferrer"
>
Read our deprecation notice and migration advice.
</a>
<br />

{showAutoMigrateLink && (
<Button fill="outline" size="sm" onClick={tryMigration} style={{ marginTop: 10 }}>
Try migration
</Button>
)}
</div>
</Alert>
</div>
Expand Down
@@ -0,0 +1,81 @@
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';

import { AngularMigrationNotice } from './AngularMigrationNotice';

jest.mock('@grafana/runtime', () => ({
...jest.requireActual('@grafana/runtime'),
reportInteraction: jest.fn(),
}));

function localStorageKey(dsUid: string) {
return `grafana.angularDeprecation.dashboardMigrationNotice.isDismissed.${dsUid}`;
}

describe('AngularMigrationNotice', () => {
const noticeText =
/This dashboard was migrated from Angular. Please make sure everything is behaving as expected and save and refresh this dashboard to persist the migration./i;
const dsUid = 'abc';

afterAll(() => {
jest.resetAllMocks();
});

beforeEach(() => {
jest.clearAllMocks();
window.localStorage.clear();
});

it('should render', () => {
render(<AngularMigrationNotice dashboardUid={dsUid} />);
expect(screen.getByText(noticeText)).toBeInTheDocument();
});

it('should be dismissable', async () => {
render(<AngularMigrationNotice dashboardUid={dsUid} />);
const closeButton = screen.getByRole('button', { name: /Close alert/i });
expect(closeButton).toBeInTheDocument();
await userEvent.click(closeButton);
expect(screen.queryByText(noticeText)).not.toBeInTheDocument();
});

it('should persist dismissed status in localstorage', async () => {
render(<AngularMigrationNotice dashboardUid={dsUid} />);
expect(window.localStorage.getItem(localStorageKey(dsUid))).toBeNull();
const closeButton = screen.getByRole('button', { name: /Close alert/i });
expect(closeButton).toBeInTheDocument();
await userEvent.click(closeButton);
expect(window.localStorage.getItem(localStorageKey(dsUid))).toBe('true');
});

it('should not re-render alert if already dismissed', () => {
window.localStorage.setItem(localStorageKey(dsUid), 'true');
render(<AngularMigrationNotice dashboardUid={dsUid} />);
expect(screen.queryByText(noticeText)).not.toBeInTheDocument();
});

describe('revert migration button', () => {
it('should display the "Revert migration" button', () => {
render(<AngularMigrationNotice dashboardUid={dsUid} />);
const revertMigrationButton = screen.getByRole('button', { name: /Revert migration/i });
expect(revertMigrationButton).toBeInTheDocument();
});

it('should display the "Report issue" button', () => {
render(<AngularMigrationNotice dashboardUid={dsUid} />);
const reportIssueButton = screen.getByRole('button', { name: /Report issue/i });
expect(reportIssueButton).toBeInTheDocument();
});

// it('should not display auto migrate button if showAutoMigrateLink is false', () => {
// render(<AngularDeprecationNotice dashboardUid={dsUid} showAutoMigrateLink={false} />);
// expect(screen.queryByText(autoMigrateText)).not.toBeInTheDocument();
// });
//
// it('should not display auto migrate link if showAutoMigrateLink is not provided', () => {
// render(<AngularDeprecationNotice dashboardUid={dsUid} />);
// expect(screen.queryByText(autoMigrateText)).not.toBeInTheDocument();
// });
adela-almasan marked this conversation as resolved.
Show resolved Hide resolved
});
});