Copilot commented on code in PR #33960:
URL: https://github.com/apache/superset/pull/33960#discussion_r2173169120
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx:
##########
@@ -466,6 +466,33 @@ function FiltersConfigModal({
handleErroredFilters();
+ // Validate dependencies to prevent saving cyclic dependencies
+ validateDependencies();
+
+ // Check if validation added any dependency errors
+ const fieldsWithErrors = form.getFieldsError();
+ const FORM_FIELD_FILTERS_INDEX = 0;
+ const FORM_FIELD_DEPENDENCIES_INDEX = 2;
+
+ const hasDependencyErrors = fieldsWithErrors.some(field =>
+ field.name?.[FORM_FIELD_FILTERS_INDEX] === 'filters' &&
+ field.name?.[FORM_FIELD_DEPENDENCIES_INDEX] === 'dependencies' &&
+ field.errors?.length > 0
+ );
+
+ if (hasDependencyErrors) {
+ // Focus on the first filter with dependency errors
+ const errorField = fieldsWithErrors.find(field =>
+ field.name?.[FORM_FIELD_FILTERS_INDEX] === 'filters' &&
+ field.name?.[FORM_FIELD_DEPENDENCIES_INDEX] === 'dependencies' &&
+ field.errors?.length > 0
+ );
Review Comment:
Consider extracting the dependency error check logic (currently using
constant indexes for field names) into a separate helper function. This
refactoring could improve readability and reduce duplication, making future
maintenance easier if the form structure evolves.
```suggestion
const checkDependencyErrors = (fields: any[]) => {
const FORM_FIELD_FILTERS_INDEX = 0;
const FORM_FIELD_DEPENDENCIES_INDEX = 2;
return fields.some(field =>
field.name?.[FORM_FIELD_FILTERS_INDEX] === 'filters' &&
field.name?.[FORM_FIELD_DEPENDENCIES_INDEX] === 'dependencies' &&
field.errors?.length > 0,
);
};
const getFirstDependencyErrorField = (fields: any[]) => {
const FORM_FIELD_FILTERS_INDEX = 0;
const FORM_FIELD_DEPENDENCIES_INDEX = 2;
return fields.find(field =>
field.name?.[FORM_FIELD_FILTERS_INDEX] === 'filters' &&
field.name?.[FORM_FIELD_DEPENDENCIES_INDEX] === 'dependencies' &&
field.errors?.length > 0,
);
};
// Check if validation added any dependency errors
const fieldsWithErrors = form.getFieldsError();
const FORM_FIELD_FILTERS_INDEX = 0;
const FORM_FIELD_DEPENDENCIES_INDEX = 2;
const hasDependencyErrors = checkDependencyErrors(fieldsWithErrors);
if (hasDependencyErrors) {
// Focus on the first filter with dependency errors
const errorField = getFirstDependencyErrorField(fieldsWithErrors);
```
--
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]
