AngularMigration: Allow dashboard by dashboard migration #84100

adela-almasan · 2024-03-07T20:06:55Z

This PR allows to do the Angular migration dashboard by dashboard and removes the confusion from the initial approach - where the feature flag would migrate all dashboards until the next page refresh, so the user could end up with broken dashboards without the ability to see how the dashboard was looking before.

The URL feature flags are treated as strings and are bypassing the previous implementation. I'm expecting to revert to that functionality after we're done with the migrations.

migration_banners.mov

Special notes for your reviewer:

Know issue -> after saving a dashboard the revert banner will persist until refresh. I believe we can fix this in a follow-up, as the user is instructed to save and refresh the dashboard to persist the migration.

Please check that:

It works as expected from a user's perspective.
If this is a pre-GA feature, it is behind a feature toggle.
The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

…rnings

…gration_url

yosiasz · 2024-03-18T10:38:33Z

Both links above take you gete.

nmarrs

Code wise I think this PR is looking great

I am struggling a little bit to test this locally though. I set the angularDeprecationUI feature toggle to true in my custom.ini file but I'm not able to get it to show up without hard coding 😬

Graph old-1710986755154.json

I also noticed that our gdev panel migration dashboard functionality seems to be broken with these changes 🤔

Screen.Recording.2024-03-20.at.7.11.44.PM.mov

public/app/features/dashboard/containers/DashboardPage.tsx

nmarrs · 2024-03-21T02:08:16Z

public/app/features/dashboard/state/getPanelPluginToMigrateTo.ts

+    panel.type === 'graph' &&
+    (shouldMigrateAllAngularPanels ||
+      config.featureToggles.autoMigrateGraphPanel ||
+      isUrlFeatureFlagEnabled('autoMigrateGraphPanel'))


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?

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.

xnyo

Awesome work!! 👏 👏 👏 👏

I have a few comments/questions but the code is looking good!

I tried it locally and it seems to work well! I have also tried @nmarrs dashboard and for me it worked without any issues 🤔

packages/grafana-runtime/src/config.ts

public/app/features/dashboard/state/getPanelPluginToMigrateTo.ts

public/app/features/plugins/angularDeprecation/AngularMigrationNotice.tsx

adela-almasan · 2024-03-21T20:42:09Z

@nmarrs the gdev dashboard is migrating fine for me 🤔

nmarrs · 2024-03-21T20:44:04Z

@nmarrs the gdev dashboard is migrating fine for me 🤔

I will re-review later today - maybe all the things I hard coded for testing was messing things up 😅

public/app/features/plugins/angularDeprecation/AngularMigrationNotice.test.tsx

xnyo

LGTM! Awesome work! 🚀🚀

Works fine for me, but I would wait to see if we can reproduce the issue that @nmarrs was having before merging this

nmarrs

LGTM - thanks for pair reviewing with me @leeoniya / @adela-almasan 😬

We discovered a strange case where the banner isn't appearing in the Devenv - panel migrations dashboard, but the revert banner appears once a migration is applied 🤔

Screen.Recording.2024-03-27.at.1.08.04.PM.mov

Also, just a note - I think that we want to target v11.0.x (and not v11.1.x) for this change - though maybe it doesn't matter either way as post v11 angular support will be disabled by default

Note: to test this + future angular migrations in the future you will need to add
angular_support_enabled = true to your custom.ini file as this PR has disabled angular by default

Test dashboard :)

adela-almasan · 2024-03-27T20:26:25Z

@xnyo I've update the local storage key, to force the banner to show even for those who have dismissed it before - now everybody should see the migrate button. This was the case for @nmarrs's above issue.

(cherry picked from commit e5d1cd8)

…86330) AngularMigration: Allow dashboard by dashboard migration (#84100) (cherry picked from commit e5d1cd8) Co-authored-by: Adela Almasan <88068998+adela-almasan@users.noreply.github.com>

xnyo and others added 9 commits February 26, 2024 12:55

Add link to auto-migrate panels to Angular dashboard warning

40e3ad1

Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link

dd4ec63

PR review: Use explicitlyControlledMigrationPanels

3ce9740

Fix unused import

5832d00

Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link

61a8b7e

Merge branch 'main' into giuseppe/angular-deprecation/auto-migrate-link

2c7b676

Removed unused import

b43c52d

Fix graph-old not being flagged by Angular deprecation dashboard wa…

4f32084

…rnings

skip overriding migration flags in url

c40f352

grafana-delivery-bot bot added this to the 11.0.x milestone Mar 7, 2024

Merge branch 'giuseppe/angular-deprecation/auto-migrate-link' into mi…

d0e4804

…gration_url

grafana-pr-automation bot added the area/frontend label Mar 7, 2024

adela-almasan added 4 commits March 7, 2024 15:43

revert migration banner

ffc6391

buttons

5776761

Merge branch 'main' into migration_url

9614dc6

links

7c907ec

adela-almasan self-assigned this Mar 11, 2024

adela-almasan changed the title ~~AngularMigration: Url feature flags~~ AngularMigration: Allow dashboard by dashboard migration Mar 11, 2024

adela-almasan added 2 commits March 11, 2024 11:04

update buttons

67711e9

open in same tab

f37597b

xnyo mentioned this pull request Mar 15, 2024

Angular deprecation: Dashboards: Add link to auto-migrate compatible panels to dashboard warning #83394

Closed

3 tasks

adela-almasan added 5 commits March 19, 2024 14:05

Merge branch 'main' into migration_url

c1d208b

update deprecation notice test

f0771ce

update text; basic test

211ec99

Merge branch 'main' into migration_url

7a900b2

disableAngular

e6be361

adela-almasan added area/dashboard no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Mar 20, 2024

nmarrs reviewed Mar 21, 2024

View reviewed changes

xnyo reviewed Mar 21, 2024

View reviewed changes

packages/grafana-runtime/src/config.ts Outdated Show resolved Hide resolved

public/app/features/dashboard/state/getPanelPluginToMigrateTo.ts Show resolved Hide resolved

public/app/features/plugins/angularDeprecation/AngularMigrationNotice.tsx Outdated Show resolved Hide resolved

adela-almasan added 3 commits March 21, 2024 14:13

Merge branch 'main' into migration_url

053e166

remove local storage

d48785f

fix url flags check

2841332

nmarrs reviewed Mar 21, 2024

View reviewed changes

public/app/features/plugins/angularDeprecation/AngularMigrationNotice.test.tsx Outdated Show resolved Hide resolved

fix test

35e7b1c

xnyo approved these changes Mar 25, 2024

View reviewed changes

ashharrison90 modified the milestones: 11.0.x, 11.1.x Mar 26, 2024

adela-almasan requested a review from nmarrs March 27, 2024 18:15

Merge branch 'main' into migration_url

4c2c455

nmarrs approved these changes Mar 27, 2024

View reviewed changes

update key

537f47b

update test

032389f

leeoniya approved these changes Mar 27, 2024

View reviewed changes

adela-almasan merged commit e5d1cd8 into main Mar 27, 2024
18 checks passed

adela-almasan deleted the migration_url branch March 27, 2024 21:24

adela-almasan mentioned this pull request Mar 29, 2024

Epic: DataViz Angular Deprecation #80983

Open

ashharrison90 modified the milestones: 11.1.x, 11.0.0-preview Apr 16, 2024

ashharrison90 added product-approved Pull requests that are approved by product/managers and are allowed to be backported backport v11.0.x Mark PR for automatic backport to v11.0.x and removed no-backport Skip backport of PR labels Apr 16, 2024

grafana-delivery-bot bot pushed a commit that referenced this pull request Apr 16, 2024

AngularMigration: Allow dashboard by dashboard migration (#84100)

b86bd73

(cherry picked from commit e5d1cd8)

grafana-delivery-bot bot mentioned this pull request Apr 16, 2024

[v11.0.x] AngularMigration: Allow dashboard by dashboard migration #86330

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AngularMigration: Allow dashboard by dashboard migration #84100

AngularMigration: Allow dashboard by dashboard migration #84100

adela-almasan commented Mar 7, 2024 •

edited

yosiasz commented Mar 18, 2024 •

edited

nmarrs left a comment •

edited

nmarrs Mar 21, 2024

adela-almasan Mar 21, 2024

xnyo left a comment

adela-almasan commented Mar 21, 2024

nmarrs commented Mar 21, 2024

xnyo left a comment

nmarrs left a comment •

edited

adela-almasan commented Mar 27, 2024

AngularMigration: Allow dashboard by dashboard migration #84100

AngularMigration: Allow dashboard by dashboard migration #84100

Conversation

adela-almasan commented Mar 7, 2024 • edited

yosiasz commented Mar 18, 2024 • edited

nmarrs left a comment • edited

Choose a reason for hiding this comment

nmarrs Mar 21, 2024

Choose a reason for hiding this comment

adela-almasan Mar 21, 2024

Choose a reason for hiding this comment

xnyo left a comment

Choose a reason for hiding this comment

adela-almasan commented Mar 21, 2024

nmarrs commented Mar 21, 2024

xnyo left a comment

Choose a reason for hiding this comment

nmarrs left a comment • edited

Choose a reason for hiding this comment

adela-almasan commented Mar 27, 2024

adela-almasan commented Mar 7, 2024 •

edited

yosiasz commented Mar 18, 2024 •

edited

nmarrs left a comment •

edited

nmarrs left a comment •

edited