sadpandajoe commented on code in PR #45541:
URL: https://github.com/apache/superset/pull/45541#discussion_r4388403648
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -177,6 +184,44 @@ interface ChartHooks extends AsyncChartHooks {
onChartStateChange?: (chartState: AgGridChartState) => void;
}
+/**
+ * Whether a chart has a previous response it can stay on screen with while a
+ * new query is in flight.
+ */
+export function hasValidPreviousData(
+ queriesResponse: QueryData[] | null | undefined,
+ formData: QueryFormData,
+ vizType: string,
+): boolean {
+ // A Matrixify grid ignores the response: each cell fetches its own data when
+ // it mounts. Kept on screen, the cells would never pick up the refresh.
+ if (isMatrixifyEnabled(formData as MatrixifyFormData)) {
+ return false;
+ }
+ // A failed load leaves its errored response in the store, and the next
+ // CHART_UPDATE_STARTED clears chartAlert but not queriesResponse. Checking
+ // the response itself keeps that stale error from being drawn as chart data
+ // while the retry is in flight.
+ const hasAnyErrors = queriesResponse?.some(item => item?.error);
+ // The response of a chart that fetches its own data stays empty once it has
+ // loaded, so for those any response that landed counts as previous data.
+ const hasPreviousResponse =
+ (getChartMetadataRegistry().get(vizType)?.enableNoResults ?? true)
+ ? (queriesResponse?.length ?? 0) > 0
+ : Array.isArray(queriesResponse);
+ return hasPreviousResponse && !hasAnyErrors;
+}
+
+// The colour settings a dashboard merges into each chart's form data. They
+// change without a new query and sit in no control panel.
+const DASHBOARD_COLOR_KEYS = [
+ 'color_scheme',
+ 'own_color_scheme',
Review Comment:
The refresh tests only drive `color_scheme` through this list (the `draws
the previous data with the current dashboard colour scheme` and `without a
colour setting cleared` cases), so deleting `own_color_scheme`, `label_colors`,
`shared_label_colors` or `map_label_colors` would leave every test green. The
dashboard changes `label_colors` and `shared_label_colors` without running a
query (label colour edits, shared colour sync), so without them the retained
chart would be redrawn with the previous response's label colours while the
refresh runs. Could one of the existing colour tests change `label_colors` and
`shared_label_colors` in `formData` while the response and
`latestQueryFormData` stay the same, and assert the plugin receives the current
values during the refresh?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
