From 58f33d227a8779e0a02da6fa8d75ae175d232f89 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Wed, 26 Jun 2024 04:16:36 -0700 Subject: [PATCH] fix(explore): don't respect y-axis formatting (#29367) --- .../StashFormDataContainer.test.tsx | 30 ++++++++++++++++++- .../StashFormDataContainer/index.tsx | 9 ++---- .../src/explore/reducers/exploreReducer.js | 18 ++++++----- .../explore/reducers/exploreReducer.test.js | 10 +++++++ 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/superset-frontend/src/explore/components/StashFormDataContainer/StashFormDataContainer.test.tsx b/superset-frontend/src/explore/components/StashFormDataContainer/StashFormDataContainer.test.tsx index bedfb8de5..669114bff 100644 --- a/superset-frontend/src/explore/components/StashFormDataContainer/StashFormDataContainer.test.tsx +++ b/superset-frontend/src/explore/components/StashFormDataContainer/StashFormDataContainer.test.tsx @@ -17,7 +17,7 @@ * under the License. */ import { defaultState } from 'src/explore/store'; -import { render } from 'spec/helpers/testing-library'; +import { render, waitFor } from 'spec/helpers/testing-library'; import { useSelector } from 'react-redux'; import { ExplorePageState } from 'src/explore/types'; import StashFormDataContainer from '.'; @@ -54,3 +54,31 @@ test('should stash form data from fieldNames', () => { 'granularity_sqla', ); }); + +test('should restore form data from fieldNames', async () => { + const { granularity_sqla, ...formData } = defaultState.form_data; + const { container } = render( + + + , + { + useRedux: true, + initialState: { + explore: { + form_data: formData, + hiddenFormData: { + granularity_sqla, + }, + }, + }, + }, + ); + await waitFor(() => + expect(container.querySelector('div')).toHaveTextContent( + 'granularity_sqla', + ), + ); +}); diff --git a/superset-frontend/src/explore/components/StashFormDataContainer/index.tsx b/superset-frontend/src/explore/components/StashFormDataContainer/index.tsx index 3715140aa..5761bef0a 100644 --- a/superset-frontend/src/explore/components/StashFormDataContainer/index.tsx +++ b/superset-frontend/src/explore/components/StashFormDataContainer/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useEffect, useRef, FC } from 'react'; +import { useEffect, FC } from 'react'; import { useDispatch } from 'react-redux'; import { setStashFormData } from 'src/explore/actions/exploreActions'; @@ -33,16 +33,11 @@ const StashFormDataContainer: FC = ({ children, }) => { const dispatch = useDispatch(); - const isMounted = useRef(false); const onVisibleUpdate = useEffectEvent((shouldStash: boolean) => dispatch(setStashFormData(shouldStash, fieldNames)), ); useEffect(() => { - if (!isMounted.current && !shouldStash) { - isMounted.current = true; - } else { - onVisibleUpdate(shouldStash); - } + onVisibleUpdate(shouldStash); }, [shouldStash, onVisibleUpdate]); return <>{children}; diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 8985f032c..377a66d5b 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -261,14 +261,16 @@ export default function exploreReducer(state = {}, action) { } const restoredField = pick(hiddenFormData, fieldNames); - return { - ...state, - form_data: { - ...form_data, - ...restoredField, - }, - hiddenFormData: omit(hiddenFormData, fieldNames), - }; + return Object.keys(restoredField).length === 0 + ? state + : { + ...state, + form_data: { + ...form_data, + ...restoredField, + }, + hiddenFormData: omit(hiddenFormData, fieldNames), + }; }, [actions.SLICE_UPDATED]() { return { diff --git a/superset-frontend/src/explore/reducers/exploreReducer.test.js b/superset-frontend/src/explore/reducers/exploreReducer.test.js index 37f072d9e..bcc585630 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.test.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.test.js @@ -33,3 +33,13 @@ test('reset hiddenFormData on SET_STASH_FORM_DATA', () => { expect(newState2.form_data).toEqual({ c: 4 }); expect(newState2.hiddenFormData).toEqual({ a: 3 }); }); + +test('skips updates when the field is already updated on SET_STASH_FORM_DATA', () => { + const initialState = { + form_data: { a: 3, c: 4 }, + hiddenFormData: { b: 2 }, + }; + const restoreAction = setStashFormData(false, ['c', 'd']); + const newState = exploreReducer(initialState, restoreAction); + expect(newState).toBe(initialState); +});