fix: improve global forms modal behavior#945
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR refactors the form template save flow to properly manage list refresh state and prevent duplicate submissions. The action thunk now reads pagination context to refresh the list with correct parameters, and the dialog adds save-state gating to prevent concurrent submissions while controlling closing behavior. ChangesForm Template Save Flow Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/pages/sponsors-global/form-templates/form-template-popup.js`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d4168d1-1521-4511-a8ce-ac6bd79c8b12
📒 Files selected for processing (2)
src/actions/form-template-actions.jssrc/pages/sponsors-global/form-templates/form-template-popup.js
| 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); | ||
| }); |
There was a problem hiding this comment.
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.
| 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.
Signed-off-by: Priscila Moneo <priscila_moneo@hotmail.com.ar>
priscila-moneo
force-pushed
the
fix/global-forms-form-close-update-list
branch
from
ee79623 to
245192f
Compare
3 participants
ref: https://app.clickup.com/t/86b8tct5y?comment=90140205951559
Summary by CodeRabbit
Release Notes