AmoghAtreya commented on code in PR #44958:
URL: https://github.com/apache/superset/pull/44958#discussion_r4187623771
##########
superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx:
##########
@@ -274,6 +275,88 @@ const renderWithRouter = ({
return { ...result, history };
};
+test('fetches datasource metadata when the datasource changes', async () => {
+ const datasourceKey = '2__table';
+ const initialState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ form_data: {
+ datasource: '1__table',
+ viz_type: VizType.Table,
+ metrics: [],
+ },
+ },
+ };
+ fetchMock.get(
+ `glob:*/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
+ {
+ id: 2,
+ type: 'table',
+ columns: [],
+ metrics: [],
+ },
+ );
+ const store = createStore(initialState, reducerIndex);
+ renderWithRouter({ initialState, store: store as Store });
+
+ act(() => {
+ store.dispatch(
+ exploreActions.setControlValue('datasource', datasourceKey, []),
+ );
+ });
+
+ await waitFor(() =>
+ expect(
+ fetchMock.callHistory.called(
+ `glob:*/fetch_datasource_metadata?datasourceKey=${datasourceKey}`,
+ ),
+ ).toBe(true),
+ );
+});
+
+test('shows an error when fetching datasource metadata fails', async () => {
+ const datasourceKey = '3__table';
+ const initialState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ form_data: {
+ datasource: '1__table',
+ viz_type: VizType.Table,
+ metrics: [],
+ },
+ },
+ };
+ const store = createStore(initialState, reducerIndex);
+ renderWithRouter({ initialState, store: store as Store });
+ jest
+ .spyOn(SupersetClient, 'get')
+ .mockRejectedValueOnce(new Error('Request failed'));
Review Comment:
Switched the failure case to `fetchMock.get(..., 500)` for
`/fetch_datasource_metadata`, matching the success test. Dropped the
`SupersetClient.get` spy so this no longer depends on how the thunk issues the
request.
##########
superset-frontend/src/explore/components/ExploreViewContainer/index.tsx:
##########
@@ -704,8 +704,11 @@ function ExploreViewContainer(props:
ExploreViewContainerProps) {
(previousControls.datasource == null ||
props.controls.datasource.value !==
previousControls.datasource.value)
) {
- // this should really be handled by actions
- fetchDatasourceMetadata(props.form_data.datasource);
+ Promise.resolve(
+ props.actions.fetchDatasourceMetadata(props.form_data.datasource),
+ ).catch(() => {
+ props.addDangerToast(t('Failed to load datasource metadata'));
+ });
Review Comment:
Moved the action call into `.then()` so both a sync throw from
`fetchDatasourceMetadata`/`dispatch` and an async rejection hit the same
`.catch()` and toast.
--
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]
