rusackas opened a new pull request, #40040:
URL: https://github.com/apache/superset/pull/40040
### SUMMARY
Continues #39927. Completes the SqlLab feature area by finishing the
migration #40037 deferred. Also incidentally fixes two pre-existing failing
tests in this file.
**Stacked on #40037** — please review/merge that first. When #40037 lands,
GitHub will auto-retarget this PR's base to master.
### What this finishes
#40037 migrated 18 of 19 SqlLab \`useDispatch\` call sites to
\`useAppDispatch\`. \`SaveDatasetModal/index.tsx\` was the holdout because its
test used \`jest.spyOn(reactRedux, 'useSelector'/'useDispatch')\` to inject
mocked hooks directly. That pattern can't intercept calls routed through the
typed hooks in \`src/views/store\` — the typed hooks capture the function
references at module evaluation time, so the spy on \`react-redux\`'s namespace
export is bypassed.
This PR refactors the test off that anti-pattern and then migrates the
production code.
### Changes
#### Test refactor (\`SaveDatasetModal.test.tsx\`)
- Drop \`import * as reactRedux from 'react-redux'\` and the two
\`jest.spyOn\` calls.
- Use the existing \`createWrapper\` + \`initialState\` pattern to preload
the SqlLab user fixture into the mock store. The component's
\`useSelector(state => state.user)\` then returns the expected value through
normal Redux flow.
- Mock \`createDatasource\` to return a thunk that resolves to \`{ id: 123
}\`. The mock store includes redux-thunk middleware (via RTK's
\`getDefaultMiddleware\`), so \`dispatch(createDatasource(...))\` correctly
unwraps the thunk and the production \`.then\` chain receives the expected
payload.
- Centralize the \`render(<SaveDatasetModal {...mockedProps} />, { useRedux:
true, initialState: { user } })\` boilerplate into a tiny \`renderModal()\`
helper.
- Net effect: -58 lines of mock setup / repetition.
#### Drive-by fix in \`setupOverwriteFlow\`
Two tests using this helper have been failing on master (\`sends
template_params when overwriting...\` and \`does not send template_params when
overwriting...\`). The cause: the AsyncSelect's debounced fetch needs fake
timers advanced via \`await act(async () => jest.runAllTimers())\` after the
dropdown opens. The helper was missing that, so the option list never
populated. Added it; both tests now pass. **17/17 SaveDatasetModal tests
green.** Full SqlLab suite: 47/47 suites, 410/411 tests (1 skipped, 0 failed).
#### Production migration (\`SaveDatasetModal/index.tsx\`)
- \`useDispatch\` → \`useAppDispatch\`; drop the obsolete \`<(dispatch: any)
=> Promise<JsonObject>>\` workaround annotation and the now-unused
\`JsonObject\` import.
- The \`.then((data: { id: number })\` annotation now type-checks naturally
against the tightened thunk return type (see below).
#### Action signature (\`actions/sqlLab.ts\`)
- \`createDatasource\`'s return type tightened from \`Promise<unknown>\` to
\`Promise<{ id: number }>\`. This matches what \`/api/v1/dataset/\` actually
returns and what \`createDatasourceSuccess\` already expects. The only caller
is \`SaveDatasetModal\` (grep'd), so the change is local.
### TESTING INSTRUCTIONS
1. Pre-commit clean on all three changed files.
2. \`npm test -- --testPathPatterns='SaveDatasetModal'\` → 17/17 pass.
3. \`npm test -- --testPathPatterns='SqlLab'\` → 47/47 suites pass.
4. In dev: open SQL Lab, run a query, click "Save as new dataset" /
"Overwrite existing dataset" — verify both flows still work end to end.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #39927
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
