fix: permalink save/overwrites in explore

[hughhhh]

SUMMARY

Fixing the bug whenever a user is trying to do an overwrite action from permalink state. The main issue is when using a permalink the explore SaveModal doesn't properly pass the slice_id to allow the reload of the update chart. To fix this i've added logic to make sure the slice_id is always present before redirecting.

Screen.Recording.2023-09-07.at.1.26.31.PM.mov

Related PR: #23627

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[geido]

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

[justinpark]

+1

[hughhhh]

@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()
});

[john-bodley]

@hughhhh given this is a fix, would you mind adding some unit tests?

[michael-s-molina]

@hughhhh If I recall correctly, permalinks don't use the slice_id, they have the form of superset/explore/p/BwdQzab51P6/. How are permalinks failing? Can you add a video demonstrating the problem?

[hughhhh]

@john-bodley + @michael-s-molina I updated the PR with more context information

[eschutho]

do you need this still?

[eschutho]

I think you can remove the todo and just say that you're exporting this for testing.

[eschutho]

LGTM. I just left a small nit comment.