PanelContext: Add functionality to update data from panel #66993

torkelo · 2023-04-20T17:29:24Z

The DataGrid panel PR wants to update data, but to do that it needs to switch the query to Grafanas built in data source, and update the queries. First approach was to emit events that was subscribed to by QueryGroup. That felt a bit hacky. Second version is using getDashboardSrv().getCurrent() to find the panel and update it's queries via updateQueries method. This will not work in other context when panel is rendered via PanelRenderer or in new scenes powered dashboards.

this PR explores two different ways to accomplish this

PanelContext.onUpdateQueries

Overall pretty simple. Problems

Panels do not have access to the current (so strange to have an update function but no way to access current). Could provide it via optional PanelProps (or context)
queries: DataQuery[] array is not really enough, miss the "main datasource" in case of mixed data sources. But we could detect if the data sources have different data sources and set the data source to mixed otherwise set it to to data source of the first query.

PanelContext.onUpdateDAta

This attacks the problem in a different way. What does the panel actually want to do? Update the data! How can it be done? well that's up to the PanelContext to figure out :)

ryantxu

Between the two... I think the direct query approach makes more sense

ryantxu · 2023-04-21T05:06:34Z

packages/grafana-ui/src/components/PanelChrome/PanelContext.ts

+  /**
+   * Some panels could be interested in updating the queries.
+   * This optional and not always available in all contexts.
+   */


This one is odd since a panel viz does not know the queries

True, but in the datagrid scenario it can create new snapshot queries

Ignore above message. I think the onUpdateData is all datagrid needs.

@ryantxu we could add queries to PanelProps

ryantxu · 2023-04-21T05:08:52Z

packages/grafana-ui/src/components/PanelChrome/PanelContext.ts

+
+  /**
+   * Optional, only some contexts support this. This can fail / be cancelled. Which is why it returns a promise.
+   */


And this one is odd given that can only translate to a single query type/ so not sure the helper wrapper is a good approach

@ryantxu what do you mean, only translate to single query?

We send a DataFrame array (via panel data ) so the update is an array. How the context updates the data is up to the context

mdvictor · 2023-04-21T08:19:54Z

Why would we need 2 handlers? I think onUpdateData will be enough for Datagrid, because it will always generate snapshot queries. Do you have any future cases in mind?

torkelo · 2023-04-21T08:28:53Z

@mdvictor i added two approaches to solve the same problem to get feedback on both (instead of opening two PRs)

mdvictor · 2023-04-21T08:40:13Z

Taking into consideration what @ryantxu mentioned about panels not knowing about queries, I'm more inclined to go with onUpdateData

torkelo · 2023-04-21T12:08:21Z

packages/grafana-ui/src/components/PanelChrome/PanelContext.ts

+  /**
+   * Optional, only some contexts support this. This can fail / be cancelled. Which is why it returns a promise.
+   */
+  onUpdateData?: (frames: DataFrame[]) => Promise<boolean>;


I feel like the update that removes your queries needs a ConfirmModal as it's a destructive action where the user looses all his queries. Question who is responsible for that confirm modal? the panel or the context implementation?

I think the context should own confirming the destructive action, since leaving it to panels means that panel devs might not always enforce it.

mdvictor · 2023-04-24T13:14:10Z

@torkelo I added a confirmation modal for onUpdateData. wdyt?

torkelo · 2023-04-24T17:56:18Z

public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx

+  changeToSnapshotData = (frames: DataFrame[]) => {
+    const snapshot: DataFrameJSON[] = frames.map((f) => dataFrameToJSON(f));
+
+    const query: GrafanaQuery = {
+      refId: 'A',
+      queryType: GrafanaQueryType.Snapshot,
+      snapshot,
+      datasource: { uid: GRAFANA_DATASOURCE_NAME },
+    };
+
+    this.props.panel.updateQueries({
+      dataSource: { uid: GRAFANA_DATASOURCE_NAME },
+      queries: [query],
+    });
+
+    this.props.panel.refresh();
+
+    this.closeConfirmationModal();
+  };
+


@mdvictor

Can we move this to a function/file in the grafana datasource folder?

And I think that function could handle showing the confirm modal. You can show a confirm modal from anywhere with simply this

appEvents.publish( new ShowConfirmModalEvent({ title: 'Delete', text: 'Are you sure you want to permanently delete your starred query?', yesText: 'Delete', icon: 'trash-alt', onConfirm: () => performDelete(query.id), }) );

Don't want to add to much stuff to PanelStateWrapper that is too large already :)

@mdvictor what does happen in the data grid panel if you cancel the update? Is the edit change reverted in the panel (without knowing it was cancelled).

The confirmation modal makes more sense. I pushed a fix. As for the datagrid, the data won't change in the view with anything but the latest frame. If the update fails, then the frame would not change and there would be no updated cell on cell edit. I do need to make a few changes in the datagrid though to fully support this.

Even though the datagrid itself doesn't need to know whether the event was successful or not, I think there might be cases where the panel needs to know what the user selected on the modal, so I changed the onUpdateData and added back the boolean promise.

mdvictor · 2023-04-25T09:03:52Z

public/app/plugins/datasource/grafana/utils.ts

+
+import { GrafanaQuery, GrafanaQueryType } from './types';
+
+export const changeToSnapshotData = (frames: DataFrame[], panel: PanelModel) => {


@torkelo I was thinking that maybe this would sit better in a place like public/app/core/utils/changeToSnapshotData.ts instead of the grafana datasource. Wdyt?

torkelo · 2023-04-25T12:02:24Z

@mdvictor pushed some refactorings this morning (to move move code to the util function)

mdvictor

LGTM!

torkelo · 2023-04-25T12:56:03Z

@mdvictor merged!

torkelo added 2 commits April 20, 2023 19:13

PanelContext: onUpdateQueries

8f7ed2d

Added onUpdateData

edfd373

torkelo requested review from a team as code owners April 20, 2023 17:29

torkelo requested review from ivanortegaalba, juanicabanas, tskarhed and ashharrison90 and removed request for a team April 20, 2023 17:29

torkelo marked this pull request as draft April 20, 2023 17:29

torkelo requested review from dprokop and removed request for ivanortegaalba, juanicabanas, tskarhed and ashharrison90 April 20, 2023 17:29

grafanabot added the area/frontend label Apr 20, 2023

torkelo requested review from ryantxu and mdvictor April 20, 2023 17:30

ryantxu reviewed Apr 21, 2023

View reviewed changes

torkelo commented Apr 21, 2023

View reviewed changes

torkelo mentioned this pull request Apr 21, 2023

Datagrid Panel: Edit data within your dashboards #66353

Merged

3 tasks

add confirmation modal for onUpdateData

56753f2

fix test

208fbcb

torkelo commented Apr 24, 2023

View reviewed changes

Merge branch 'main' into update-queries-panel-context

9c14426

mdvictor added 3 commits April 25, 2023 11:44

refactor confirmation modal

69b7f57

revert unwanted modifications

974c5d4

fix

c3f4ab7

mdvictor reviewed Apr 25, 2023

View reviewed changes

Refactorings

d8ea2cc

mdvictor added this to the 10.0.0 milestone Apr 25, 2023

mdvictor added add to changelog no-backport Skip backport of PR labels Apr 25, 2023

mdvictor changed the title ~~PanelContext: Exploring two ways to update queries or update data~~ PanelContext: Add functionality to update snapshot data from panel Apr 25, 2023

mdvictor changed the title ~~PanelContext: Add functionality to update snapshot data from panel~~ PanelContext: Add functionality to update data from panel Apr 25, 2023

mdvictor marked this pull request as ready for review April 25, 2023 10:51

mdvictor requested a review from ryantxu April 25, 2023 10:52

mdvictor approved these changes Apr 25, 2023

View reviewed changes

torkelo merged commit de18ed6 into main Apr 25, 2023
24 checks passed

torkelo deleted the update-queries-panel-context branch April 25, 2023 12:55

zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PanelContext: Add functionality to update data from panel #66993

PanelContext: Add functionality to update data from panel #66993

torkelo commented Apr 20, 2023 •

edited

ryantxu left a comment

ryantxu Apr 21, 2023

mdvictor Apr 21, 2023

mdvictor Apr 21, 2023

torkelo Apr 21, 2023

ryantxu Apr 21, 2023

torkelo Apr 21, 2023

mdvictor commented Apr 21, 2023

torkelo commented Apr 21, 2023

mdvictor commented Apr 21, 2023

torkelo Apr 21, 2023

mdvictor Apr 21, 2023

mdvictor commented Apr 24, 2023

torkelo Apr 24, 2023 •

edited

torkelo Apr 24, 2023

mdvictor Apr 25, 2023

mdvictor Apr 25, 2023

torkelo commented Apr 25, 2023

mdvictor left a comment

torkelo commented Apr 25, 2023


		import { GrafanaQuery, GrafanaQueryType } from './types';

		export const changeToSnapshotData = (frames: DataFrame[], panel: PanelModel) => {

PanelContext: Add functionality to update data from panel #66993

PanelContext: Add functionality to update data from panel #66993

Conversation

torkelo commented Apr 20, 2023 • edited

PanelContext.onUpdateQueries

PanelContext.onUpdateDAta

ryantxu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdvictor commented Apr 21, 2023

torkelo commented Apr 21, 2023

mdvictor commented Apr 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdvictor commented Apr 24, 2023

torkelo Apr 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torkelo commented Apr 25, 2023

mdvictor left a comment

Choose a reason for hiding this comment

torkelo commented Apr 25, 2023

torkelo commented Apr 20, 2023 •

edited

torkelo Apr 24, 2023 •

edited