[MISC] Remove success alerts from deployments and pipelines #1755
Conversation
Remove success alerts from API deployments, pipelines, and file history operations to reduce notification clutter. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRemoved many user-facing success alerts, simplified a file-history hook signature, added local hasChanges/isSaved state in several editor components, and refactored the tools list to introduce an internal DefaultCustomButtons component and consolidated rendering. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
chandrasekharan-zipstack
changed the title
chandrasekharan-zipstack
requested review from
jagadeeswaran-zipstack and
vishnuszipstack
Remove non-critical success alerts from 11 frontend components: - Document parser, users, list of tools, file history modal - Default triad, LLM profiles, pre/post amble modal - Custom data/synonyms, tool settings, configure data source Co-Authored-By: Claude Opus 4.5 <[email protected]>
![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
🤖 Fix all issues with AI agents
In `@frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx`:
- Around line 25-33: The Space component usage currently sets an unsupported
prop gap={16}; replace it with size={16} (e.g., update the Space containing
CustomButton in ListOfTools.jsx) so the spacing is applied correctly; ensure any
other Space instances in this component also use size instead of gap to match
antd 5.5.1 API.
🧹 Nitpick comments (1)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
362-371: CompleteuseCallbackdeps (or drop memoization)
CustomButtonsComponentcloses overhandleNewProjectBtnClick, but it’s not in the dependency array. Either add the missing deps or removeuseCallbackif memoization isn’t needed.♻️ Proposed tweak
- const CustomButtonsComponent = useCallback( + const CustomButtonsComponent = useCallback( () => ( <DefaultCustomButtons setOpenImportTool={setOpenImportTool} isImportLoading={isImportLoading} handleNewProjectBtnClick={handleNewProjectBtnClick} /> ), - [isImportLoading] + [isImportLoading, handleNewProjectBtnClick, setOpenImportTool] );
|
@chandrasekharan-zipstack Request to get the list of places where we have taken out success alerts reviewed by the product owners too, before proceeding to merge. That being said, appreciate the effort. Keep it coming! |
[FIX] Fix Save→Saved button text not showing in settings components Fix useEffect dependency bug that was resetting isSaved state immediately after save, preventing "Saved" text from displaying on Save button. Co-authored-by: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/custom-tools/custom-data-settings/CustomDataSettings.jsx (1)
65-75: Adddetails?.custom_datato the useEffect dependency array to sync editor state after save.The effect depends only on
details?.tool_id, but when you save, the API response updatesdetails?.custom_datawithout changingtool_id. The editor won't refresh, leaving stale data in the UI while the store has the correct server response. Includedetails?.custom_datain dependencies so edits sync after save, or explicitly refreshjsonValuein the.then()block whenupdateCustomToolis called.
🤖 Fix all issues with AI agents
In `@frontend/src/components/custom-tools/custom-synonyms/CustomSynonyms.jsx`:
- Around line 249-251: When the component's details prop changes, reset the save
state to avoid showing a stale "Saved"; add an effect that watches details and
sets isSaved = false and hasChanges = false (or recalculates hasChanges) so the
Save button reflects the new tool context — update the CustomSynonyms.jsx
component to reset these flags in a useEffect (or equivalent lifecycle) that
references details, isSaved, and hasChanges.
@harini-venkataraman @Deepak-Kesavan @muhammad-ali-e FYI regarding this success alert removal in Prompt Studio and Pipeline pages. This was also agreed over slack |
- Add hasChanges/isSaved state to track modifications - Reset state on tool_id change to prevent stale button state - Disable Save button until changes are made - Show "Saved" feedback after successful save Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/custom-tools/custom-synonyms/CustomSynonyms.jsx (1)
198-213: Race condition: edits during save may leave changes unmarked.If the user modifies a synonym while an API request is in flight, the
.then()callback will sethasChanges=falseandisSaved=true, even though the new edits haven't been persisted. The Save button will incorrectly show "Saved" and remain disabled.Consider capturing a "dirty" snapshot before the request or comparing state after completion.
🔧 Suggested fix using a ref to track in-flight changes
+ const pendingChangesRef = useRef(false); + const handleChange = (index, propertyName, value) => { const updatedSynonyms = [...synonyms]; updatedSynonyms[index][propertyName] = value; setSynonyms(updatedSynonyms); setHasChanges(true); setIsSaved(false); + if (isLoading) { + pendingChangesRef.current = true; + } }; // ... in updateSynonyms .then() callback: .then(() => { if (actionType === actionTypes.delete) { setSynonyms(listOfSynonyms); } - setHasChanges(false); - if (actionType === actionTypes.save) { - setIsSaved(true); - } + if (pendingChangesRef.current) { + pendingChangesRef.current = false; + // Changes made during save - keep hasChanges true + } else { + setHasChanges(false); + if (actionType === actionTypes.save) { + setIsSaved(true); + } + } })frontend/src/components/custom-tools/pre-and-post-amble-modal/PreAndPostAmbleModal.jsx (1)
68-95: AddisLoadingto the disabled prop for proper button state feedback.The
loadingprop prevents theonClickhandler from firing during requests, but it does not set the DOMdisabledattribute. This should be added explicitly for correct UX and accessibility—users should see the button as disabled while the request is in flight.- disabled={isPublicSource || !hasChanges} + disabled={isPublicSource || !hasChanges || isLoading}Also applies to: 167-170
|
What
Remove success alerts from various frontend components to reduce notification clutter:
Delete Success Alerts Removed:
Settings/Config Save Alerts Removed:
Previously Removed (first commit):
Why
How
setAlertDetails({ type: "success", ... })blockssuccessMsgin CustomSynonyms.jsx)Can this PR break any existing features?
No, this only removes success notifications. All core functionality and error handling remains unchanged.
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Verify that success alerts no longer appear when:
Verify that these still work:
Screenshots
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 [email protected]