gabotorresruiz commented on code in PR #45052:
URL: https://github.com/apache/superset/pull/45052#discussion_r4224419423
##########
superset-frontend/src/dashboard/reducers/dashboardInfo.ts:
##########
@@ -218,6 +350,40 @@ export default function dashboardInfoReducer(
return {
...state,
...action.data.dashboardInfo,
+ semanticDatasets:
+ state.semanticDatasets?.dashboardId === action.data.dashboardInfo.id
+ ? state.semanticDatasets
+ : null,
+ semanticDatasetsRequestId:
+ state.id === action.data.dashboardInfo.id ||
+ state.semanticDatasets?.dashboardId === action.data.dashboardInfo.id
+ ? state.semanticDatasetsRequestId
+ : undefined,
+ semanticDatasetRequests:
+ state.id === action.data.dashboardInfo.id ||
+ state.semanticDatasets?.dashboardId === action.data.dashboardInfo.id
+ ? state.semanticDatasetRequests
+ : {},
Review Comment:
`HYDRATE_DASHBOARD` keeps `semanticDatasetRequests` when the dashboard id is
unchanged, which leaves one case the generations do not cover: a same-dashboard
remount with an add's `/fetch_datasource_metadata` still in flight. That
`sourceKey` is then in `pageModifiedKeys`, so the freshly loaded page metadata
for it is discarded, and the pre-remount response still matches at line 222 and
becomes the published snapshot. Checking it against the reducer, the published
`is_dttm` is the pre-remount one, which would drop the grain if the view's
schema changed in between.
Not a blocker: the window is one request long, and master froze the snapshot
at load time anyway. But would clearing this even for a matching id be safe? A
remount's page load is strictly newer than anything issued before it, and
dropping the request just leaves the source unproven, which retains the grain.
Or is there a case where the pre-remount response is the one you want?
##########
superset-frontend/src/dashboard/reducers/dashboardInfo.ts:
##########
@@ -126,6 +139,125 @@ export default function dashboardInfoReducer(
action: DashboardInfoReducerAction,
): DashboardInfoState {
switch (action.type) {
+ case REPLACE_DASHBOARD_SEMANTIC_DATASETS: {
+ const {
+ dashboardId,
+ datasets,
+ requestId,
+ isRefreshStart,
+ expectedGeneration,
+ } = action as DashboardInfoAction;
+ if (
+ dashboardId === undefined ||
+ (state.id !== undefined && dashboardId !== state.id)
Review Comment:
`REPLACE` tolerates `state.id === undefined` here, while `UPDATE` requires
an exact match at line 214. Any reason for the asymmetry? Let's align them
unless the looser form is load-bearing for some early dispatch.
##########
superset-frontend/src/dashboard/actions/dashboardInfo.ts:
##########
@@ -46,6 +50,94 @@ export function dashboardSaveSucceeded(dashboardId: number) {
export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED';
export const DASHBOARD_INFO_FILTERS_CHANGED = 'DASHBOARD_INFO_FILTERS_CHANGED';
+export const REPLACE_DASHBOARD_SEMANTIC_DATASETS =
+ 'REPLACE_DASHBOARD_SEMANTIC_DATASETS';
+export const UPDATE_DASHBOARD_SEMANTIC_DATASET =
+ 'UPDATE_DASHBOARD_SEMANTIC_DATASET';
+
+type SemanticDataset = NonNullable<
+ DashboardInfo['semanticDatasets']
+>['datasets'][number];
+
+export function provenSemanticDataset(
+ value: unknown,
+ sourceKey: string,
+): SemanticDataset | null {
+ if (!value || typeof value !== 'object') return null;
+ const candidate = value as Partial<SemanticDataset> & { id?: number };
+ const sourceId = /^([1-9]\d*)__semantic_view$/.exec(sourceKey)?.[1];
Review Comment:
Small thing: the source-key pattern hardcodes `semantic_view` while the next
line uses `DatasourceType.SemanticView`. If that enum value ever moves, this
stops matching silently and every grain becomes unproven rather than failing
loudly. Worth hoisting it so there is one source of truth?
```ts
const SEMANTIC_SOURCE_KEY_RE = new RegExp(
`^([1-9]\\d*)__${DatasourceType.SemanticView}$`,
);
```
--
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]
