sadpandajoe commented on code in PR #44366:
URL: https://github.com/apache/superset/pull/44366#discussion_r4048678039
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -471,9 +471,11 @@ const FiltersConfigForm = (
formFilter?.filterType,
);
- const canDependOnOtherFilters = filterSupportsDependencies(
- formFilter?.filterType,
- );
+ // Use itemTypeField, not formFilter?.filterType directly: the latter can
+ // be undefined on the first render before the antd Form hydrates (see
+ // itemTypeField's own fallback chain above), which would otherwise hide
+ // this section for a filter type that does support cascade dependencies.
+ const canDependOnOtherFilters = filterSupportsDependencies(itemTypeField);
Review Comment:
This first-render regression is not protected by the new tests: they only
call `buildDependencyMap`, and the existing modal tests assert that the
dependency control is absent for single-filter cases. Could we add a case to
`FiltersConfigModal.test.tsx` that opens a saved Select/Range filter with a
cascade parent, asserts that the dependency control and saved parent appear
without user interaction, and demonstrates failure when this fallback is
reverted? If the existing mount-effect rerender makes the difference
unobservable, please document that as the test exemption instead.
--
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]
