mikebridge commented on code in PR #43350:
URL: https://github.com/apache/superset/pull/43350#discussion_r3823010229
##########
superset-frontend/src/explore/actions/hydrateExplore.ts:
##########
@@ -213,6 +279,10 @@ export const hydrateExplore =
exploreState,
);
});
+ const hydratedFormData = {
+ ...initialFormData,
+ ...getFormDataFromControls(exploreState.controls),
+ };
Review Comment:
Addressed by the save-time exact-match guard: projected hydration
transitions are retained as candidates, but only transitions whose presence and
value match the finalized params payload are sent. Controls absent from the
actual save are therefore dropped and cannot become stale backend evidence. The
finalized-payload matching is covered by 87fb4dbcb2.
##########
superset-frontend/src/explore/actions/saveModalActions.ts:
##########
@@ -233,21 +245,69 @@ export const updateSlice =
new?: boolean;
},
) =>
- async (dispatch: Dispatch, getState: () => Partial<QueryFormData>) => {
+ async (
+ dispatch: Dispatch,
+ getState: () => Partial<QueryFormData> & {
+ versionHistory?: {
+ chartNormalization?: ChartNormalizationTrackingState | null;
+ };
+ },
+ ) => {
const { slice_id: sliceId, editors, form_data: formDataFromSlice } = slice;
- const formData = getState().explore?.form_data;
+ const initialState = getState();
+ const formData = JSON.parse(
+ JSON.stringify(initialState.explore?.form_data ?? {}),
+ ) as QueryFormData;
+ const tracking = initialState.versionHistory?.chartNormalization;
+ const saveAttemptId = nanoid();
+ const matchingExclusions = Object.fromEntries(
+ Object.entries(tracking?.exclusions ?? {}).filter(
+ ([control, transition]) =>
+ !tracking?.invalidatedControls[control] &&
+ Object.hasOwn(formData, control) === transition.to_present &&
+ (!transition.to_present ||
+ JSON.stringify(formData[control]) ===
+ JSON.stringify(transition.to_value)),
+ ),
+ ) as AutomaticNormalizationExclusions;
Review Comment:
Fixed in 87fb4dbcb2. Transition matching now happens after getSlicePayload
and uses the form data parsed from payload.params, so adhoc and temporal-filter
rewrites are included. A regression test covers the extra temporal-filter
rewrite.
##########
superset-frontend/src/explore/actions/saveModalActions.ts:
##########
@@ -233,21 +245,69 @@ export const updateSlice =
new?: boolean;
},
) =>
- async (dispatch: Dispatch, getState: () => Partial<QueryFormData>) => {
+ async (
+ dispatch: Dispatch,
+ getState: () => Partial<QueryFormData> & {
+ versionHistory?: {
+ chartNormalization?: ChartNormalizationTrackingState | null;
+ };
+ },
+ ) => {
const { slice_id: sliceId, editors, form_data: formDataFromSlice } = slice;
- const formData = getState().explore?.form_data;
+ const initialState = getState();
+ const formData = JSON.parse(
+ JSON.stringify(initialState.explore?.form_data ?? {}),
+ ) as QueryFormData;
+ const tracking = initialState.versionHistory?.chartNormalization;
+ const saveAttemptId = nanoid();
+ const matchingExclusions = Object.fromEntries(
+ Object.entries(tracking?.exclusions ?? {}).filter(
+ ([control, transition]) =>
+ !tracking?.invalidatedControls[control] &&
+ Object.hasOwn(formData, control) === transition.to_present &&
+ (!transition.to_present ||
+ JSON.stringify(formData[control]) ===
+ JSON.stringify(transition.to_value)),
+ ),
+ ) as AutomaticNormalizationExclusions;
+ const shouldAttachNormalization =
+ isFeatureEnabled(FeatureFlag.VersionHistory) &&
+ tracking?.chartId === sliceId;
+ if (shouldAttachNormalization) {
+ dispatch(
+ beginChartNormalizationSave(
+ sliceId,
+ tracking.hydrationSessionId,
+ saveAttemptId,
+ ),
+ );
+ }
try {
+ const payload = await getSlicePayload(
+ sliceName,
+ formData,
+ dashboards,
+ editors as [],
+ formDataFromSlice,
+ );
+ if (shouldAttachNormalization && Object.keys(matchingExclusions).length)
{
+ payload.normalization_changes = Object.values(matchingExclusions);
+ }
const response = await SupersetClient.put({
endpoint: `/api/v1/chart/${sliceId}`,
- jsonPayload: await getSlicePayload(
- sliceName,
- formData,
- dashboards,
- editors as [],
- formDataFromSlice,
- ),
+ jsonPayload: payload,
});
+ if (shouldAttachNormalization) {
+ dispatch(
+ completeChartNormalizationSave(
+ sliceId,
+ tracking.hydrationSessionId,
+ saveAttemptId,
+ {},
Review Comment:
The empty transition map is intentional. A successful save consumes the
hydration evidence so it cannot be resent on a later overwrite. The payload
receives the matching transitions before the request; completion clears them
only after the PUT succeeds.
##########
superset-frontend/src/features/versionHistory/sessionLogMiddleware.ts:
##########
@@ -151,5 +165,23 @@ export const versionSessionLogMiddleware: Middleware =
}),
);
}
+ if (action.type !== HYDRATE_EXPLORE) {
+ const state = store.getState() as SessionLogState;
+ const controls = changedFormDataKeys(before, state.explore?.form_data);
+ if (
+ action.type === SET_FIELD_VALUE &&
Review Comment:
These dispatches serve separate state domains: the append records a
human-readable unsaved edit, while invalidation prevents hydration evidence
from suppressing that user edit during save. Both are required for a
user-initiated control change. The boundary adapter now makes that separation
explicit.
##########
superset/commands/chart/update.py:
##########
@@ -78,6 +88,15 @@ def run(self) -> Model:
self._properties["last_saved_at"] = datetime.now()
self._properties["last_saved_by"] = g.user
+ if "params" in self._properties:
+ register_matching_normalization_context(
+ db.session,
+ self._model.id,
+ self._normalization_changes,
+ self._model.params,
+ self._properties["params"],
+ )
Review Comment:
Fixed in 87fb4dbcb2 by restoring the command-level normalization_changes
guard. The helper already returned before parsing, but the outer guard makes
the short-circuit explicit and avoids the unnecessary call.
--
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]
