Skip to content

fix: improve global forms modal behavior #945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smarcet merged 1 commit into from
May 18, 2026
Merged

fix: improve global forms modal behavior #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 26 additions & 43 deletions src/actions/form-template-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
authErrorHandler,
escapeFilterValue
} from "openstack-uicore-foundation/lib/utils/actions";
import history from "../history";
import { getAccessTokenSafely } from "../utils/methods";
import {
DEFAULT_CURRENT_PAGE,
Expand Down Expand Up @@ -185,6 +184,8 @@ export const saveFormTemplate = (entity) => async (dispatch) => {
dispatch(startLoading());

const normalizedEntity = normalizeEntity(entity);
const materials = entity.materials ?? [];
const metaFields = entity.meta_fields ?? [];

if (entity.id) {
return putRequest(
Expand All @@ -198,42 +199,29 @@ export const saveFormTemplate = (entity) => async (dispatch) => {
.then(() => {
const promises = [];

if (entity.materials.length > 0) {
if (materials.length > 0) {
promises.push(saveFormTemplateMaterials(normalizedEntity)(dispatch));
}

if (entity.meta_fields.length > 0) {
if (metaFields.length > 0) {
promises.push(
saveFormTemplateMetaFieldTypes(normalizedEntity)(dispatch)
);
}

Promise.all(promises)
.then(() => {
dispatch(
showSuccessMessage(
T.translate("edit_form_template.form_template_saved")
)
);
})
.catch((err) => {
console.error(err);
})
.finally(() => {
dispatch(stopLoading());
});
return Promise.all(promises).then(() => {
dispatch(
showSuccessMessage(
T.translate("edit_form_template.form_template_saved")
)
);
});
})
.catch((err) => {
console.error(err);
.finally(() => {
dispatch(stopLoading());
});
}

const success_message = {
title: T.translate("general.done"),
html: T.translate("edit_form_template.form_template_created"),
type: "success"
};

return postRequest(
createAction(ADD_FORM_TEMPLATE),
createAction(FORM_TEMPLATE_ADDED),
Expand All @@ -246,31 +234,26 @@ export const saveFormTemplate = (entity) => async (dispatch) => {
const formTemplate = { ...normalizedEntity, id: response.id };
const promises = [];

if (entity.materials.length > 0) {
if (materials.length > 0) {
promises.push(saveFormTemplateMaterials(formTemplate)(dispatch));
}

if (entity.meta_fields.length > 0) {
if (metaFields.length > 0) {
promises.push(saveFormTemplateMetaFieldTypes(formTemplate)(dispatch));
}

Promise.all(promises)
.then(() => {
dispatch(
showMessage(success_message, () => {
history.push("/app/form-templates");
})
);
})
.catch((err) => {
console.error(err);
})
.finally(() => {
dispatch(stopLoading());
});
return Promise.all(promises).then(() => {
dispatch(
showMessage({
title: T.translate("general.done"),
html: T.translate("edit_form_template.form_template_created"),
type: "success"
})
);
});
})
.catch((err) => {
console.error(err);
.finally(() => {
dispatch(stopLoading());
});
};

Expand Down
14 changes: 13 additions & 1 deletion src/pages/sponsors-global/form-templates/form-template-list-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,18 @@ const FormTemplateListPage = ({
sortDir: orderDir
};

const handleOnSave = async (values) => {
await saveFormTemplate(values);
getFormTemplates(
"",
DEFAULT_CURRENT_PAGE,
perPage,
order,
orderDir,
showArchived
);
};

return (
<div className="container">
<h3>
Expand Down Expand Up @@ -322,7 +334,7 @@ const FormTemplateListPage = ({
entity={currentFormTemplate}
errors={currentFormTemplateErrors}
open={formTemplatePopupOpen}
onSave={saveFormTemplate}
onSave={handleOnSave}
toDuplicate={formTemplateDuplicate}
onClose={() => setFormTemplatePopupOpen(false)}
onMetaFieldTypeDeleted={deleteFormTemplateMetaFieldType}
Expand Down
43 changes: 37 additions & 6 deletions src/pages/sponsors-global/form-templates/form-template-popup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "react";
import React, { useState } from "react";
import T from "i18n-react/dist/i18n-react";
import PropTypes from "prop-types";
import {
Expand Down Expand Up @@ -35,6 +35,13 @@ const FormTemplateDialog = ({
onMetaFieldTypeValueDeleted,
entity: initialEntity
}) => {
const [isSaving, setIsSaving] = useState(false);

const closePopup = () => {
formik.resetForm();
onClose();
};

const formik = useFormik({
initialValues: {
...initialEntity,
Expand Down Expand Up @@ -64,15 +71,27 @@ const FormTemplateDialog = ({
}))
}))
};
onSave(finalValues);
if (isSaving) return;

setIsSaving(true);
Promise.resolve(onSave(finalValues))
.then(() => {
closePopup();
})
.catch(() => {
// keep dialog open on save error to preserve user input
})
.finally(() => {
setIsSaving(false);
});
Comment on lines +74 to +86
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Silent error swallowing hides save failures from users and developers.

The empty .catch(() => {}) suppresses all errors without logging or user notification. When a save fails, users won't know what happened or if a partial save occurred, and debugging becomes difficult without any error traces.

Proposed fix to add error logging 
       .catch(() => {
-        // keep dialog open on save error to preserve user input
+        // keep dialog open on save error to preserve user input
+        // errors are displayed via authErrorHandler in the action
+        console.error("Form template save failed");
       })

Alternatively, if onSave already displays error messages via authErrorHandler, consider documenting that explicitly in the comment so future maintainers know errors are handled upstream.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (isSaving) return;
setIsSaving(true);
Promise.resolve(onSave(finalValues))
.then(() => {
closePopup();
})
.catch(() => {
// keep dialog open on save error to preserve user input
})
.finally(() => {
setIsSaving(false);
});
if (isSaving) return;
setIsSaving(true);
Promise.resolve(onSave(finalValues))
.then(() => {
closePopup();
})
.catch(() => {
// keep dialog open on save error to preserve user input
// errors are displayed via authErrorHandler in the action
console.error("Form template save failed");
})
.finally(() => {
setIsSaving(false);
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/sponsors-global/form-templates/form-template-popup.js` around lines
74 - 86, The catch block for Promise.resolve(onSave(finalValues)) silently
swallows errors; update the catch to surface and record failures by logging the
error and/or forwarding it to the existing error handler so developers and users
see save failures (e.g., call console.error or the authErrorHandler with the
caught error), and optionally show a user-facing message before leaving the
finally that calls setIsSaving(false); keep references to
isSaving/setIsSaving/onSave/closePopup so you modify the promise chain around
onSave(finalValues) and ensure closePopup only runs on success.

}
});

useScrollToError(formik);

const handleClose = () => {
formik.resetForm();
onClose();
if (isSaving) return;
closePopup();
};

return (
Expand All @@ -84,10 +103,17 @@ const FormTemplateDialog = ({
disableEnforceFocus
disableAutoFocus
disableRestoreFocus
disableEscapeKeyDown={isSaving}
sx={{ zIndex: 1000 }}
>
<DialogTitle sx={{ display: "flex", justifyContent: "space-between" }}>
Edit Item
<IconButton size="small" onClick={() => handleClose()} sx={{ mr: 1 }}>
<IconButton
size="small"
onClick={() => handleClose()}
sx={{ mr: 1 }}
disabled={isSaving}
>
<CloseIcon fontSize="small" />
</IconButton>
</DialogTitle>
Expand Down Expand Up @@ -153,7 +179,12 @@ const FormTemplateDialog = ({
</DialogContent>
<Divider />
<DialogActions>
<Button type="submit" fullWidth variant="contained">
<Button
type="submit"
fullWidth
variant="contained"
disabled={isSaving}
>
{initialEntity.id
? T.translate("edit_form_template.save_changes")
: T.translate("edit_form_template.add_form")}
Expand Down
Loading