Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: permalink save/overwrites in explore #25112

Merged
merged 12 commits into from
Oct 16, 2023
Merged

fix: permalink save/overwrites in explore #25112

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0bd7778
make sure slice_id is in params if not overwrite
hughhhh Aug 29, 2023
6227169
always make sure slce_id is added
hughhhh Aug 30, 2023
11f8ec5
save
hughhhh Sep 8, 2023
4c6083e
Merge branch 'master' of https://github.com/preset-io/superset into f…
hughhhh Sep 9, 2023
7833211
Merge branch 'master' of https://github.com/preset-io/superset into f…
hughhhh Oct 10, 2023
83fc411
get test to work properly
hughhhh Oct 10, 2023
7632855
ok
hughhhh Oct 13, 2023
b9b6d59
ok
hughhhh Oct 13, 2023
89e5e06
Update superset-frontend/src/explore/components/SaveModal.tsx
hughhhh Oct 13, 2023
526329a
Update superset-frontend/src/explore/components/SaveModal.tsx
hughhhh Oct 13, 2023
6152399
remove setSate
hughhhh Oct 13, 2023
6deb900
Update SaveModal.tsx
hughhhh Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions superset-frontend/src/explore/components/SaveModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,10 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
searchParams.set('save_action', this.state.action);
if (this.state.action !== 'overwrite') {
searchParams.delete('form_data_key');
}
if (this.state.action === 'saveas') {
} else {
searchParams.set('slice_id', value.id.toString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a small check in the tests to make sure that the slice_id always appears?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinpark + @geido so i've spent literally tons of time trying to get this test work with the current testing pattern and been having no luck. I did find that this is a known issue in enzyme when using shallow vs mount
enzymejs/enzyme#2086

What i suggest is we merge this to fix the current issue, and I can investigate on rewriting this file to work properly with RTL and i'll take full ownership over this

const defaultPropsTwo = {
  addDangerToast: jest.fn(),
  onHide: () => ({}),
  actions: {
    saveDataset: jest.fn().mockReturnValue({ id: 42 }),
    setFormData: jest.fn(),
    updateSlice: jest.fn().mockReturnValue({ id: 42 }),
  },
  form_data: { datasource: '107__table', url_params: { foo: 'bar' } },
  history: {
    replace: jest.fn(),
  }
};

const fetchChartEndpoint = `glob:*/api/v1/chart/${1}?q=(columns:!(dashboards.id))`;

beforeAll(() => {
  fetchMock.get(fetchDashboardsEndpoint, mockDashboardData)
  fetchMock.get(fetchChartEndpoint, { dashboards: [mockDashboardData]})
});

test('make sure saveOverwrite function always has slice_id attached to the url', () => {
  const wrapper = getWrapper(defaultPropsTwo, queryStore);
  const footerWrapper = shallow(wrapper.find(StyledModal).props().footer);
  wrapper.setState({
    action: 'overwrite',
  });
  
  const overwriteRadio = wrapper.find('#overwrite-radio');
  overwriteRadio.simulate('click');
  expect(wrapper.state().action).toBe('overwrite');
  const save = footerWrapper.find('#btn_modal_save');
  save.simulate('click');
  expect(defaultPropsTwo.history.replace).toBeCalled()
});

}

this.props.history.replace(`/explore/?${searchParams.toString()}`);

this.setState({ isLoading: false });
Expand Down
Loading