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

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

Closed
wants to merge 8 commits into from

Conversation

xnyo
Copy link
Member

@xnyo xnyo commented Feb 26, 2024

What is this feature?

Adds a link to Angular dashboard warnings that points to &__feature.autoMigrateOldPanels=true.

It is be displayed only when the dashboard contains panels that can be migrated.

immagine

Requires #84034 to be merged before this PR, in order to set the feature flag from URL in production mode.

Why do we need this feature?

Simplify auto-migrating Angular panels.

Who is this feature for?

Grafana users with Angular panel plugins.

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Need to add more tests

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.

Copy link
Contributor

Hello @xnyo!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@lukasztyrala
Copy link
Member

Hi @xnyo, at DataViz team we are trying to put together a script (guidelines) to use when communicating with customers about the expected deprecations and how to safely test the auto migration. There are few things we need to understand:

  1. Do we know how we will integrate the migration button into the UI and where the banner will be displayed?
  2. Will there be an option to revert the automigration (or will it still be required to save dashboard explicitly after the auto migration)?

cc @nmarrs @leeoniya

@xnyo
Copy link
Member Author

xnyo commented Feb 29, 2024

Hi @lukasztyrala

Do we know how we will integrate the migration button into the UI and where the banner will be displayed?

The link displayed in the screenshot is shown in a warning that's displayed at the top of dashboards that use Angular plugins. This warning is displayed since Grafana 10.4.0. The "Auto-migrate compatible panels" link in particular will be displayed when the dashboard contains at least one of the following plugins:

  • graph
  • table-old
  • grafana-piechart-panel
  • grafana-worldmap-panel
  • singlestat
  • grafana-singlestat-panel

Here's a better screenshot showing it:

image

Will there be an option to revert the automigration (or will it still be required to save dashboard explicitly after the auto migration)?

I don't know the details regarding the auto migration feature, but from my testing the dashboard required a manual save before the migration was committed. So there's no option to revert the auto migration because it has to be saved manually by the user

@leeoniya
Copy link
Contributor

leeoniya commented Feb 29, 2024

So there's no option to revert the auto migration because it has to be saved manually by the user

yes, once it is saved there is no going back, unless you restore old database or if we offer a built-in way to undo db changes (such as retaining old versions somewhere else in the db). but this will only work until angular is removed anyhow.

i think the main issue with the inability to use a url for feature enablement in production environments is that to "undo" an unsaved migration is no longer a browser Back button away. we need to provide a way to clear the appropriate cookie or localStorage variable that we choose to implement instead of simply relying on browser url history.

@sympatheticmoose
Copy link
Contributor

sympatheticmoose commented Feb 29, 2024

yes, once it is saved there is no going back, unless you restore old database or if we offer a built-in way to undo db changes (such as retaining old versions somewhere else in the db). but this will only work until angular is removed anyhow.

Couldn't you just restore the previous dashboard version? Or is the change persisted somewhere else beyond the dashboard.json?
Screenshot 2024-02-29 at 17 33 45

@leeoniya
Copy link
Contributor

leeoniya commented Feb 29, 2024

Couldn't you just restore the previous dashboard version? Or is the change persisted somewhere else beyond the dashboard.json?

the dashboard.json is stored in the db. so if we already offer some way to restore previous dashboard versions, then sure. this is outside my wheelhouse. idk what we offer in terms of backend storage capabilities or how/where we expose this in the UI.

EDIT: yeah, i guess it exists :)

https://grafana.com/docs/grafana/latest/dashboards/build-dashboards/manage-version-history/

@xnyo xnyo changed the title Angular deprecation: Add link to auto-migrate compatible panels to Angular dashboard warning Angular deprecation: Dashboards: Add link to auto-migrate compatible panels to dashboard warning Mar 7, 2024
@xnyo
Copy link
Member Author

xnyo commented Mar 15, 2024

Superseded by #84100

@xnyo xnyo closed this Mar 15, 2024
@grafana-delivery-bot grafana-delivery-bot bot removed this from the 11.0.x milestone Mar 15, 2024
Copy link
Contributor

This PR must be merged before a backport PR will be created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants