UN-1722 [FIX] Persist export reminder state across page reloads - #1733
Conversation
- Add sessionStorage persistence for hasUnsavedChanges flag per tool_id - Modify setHasUnsavedChanges to save/remove from sessionStorage - Modify markChangesAsExported to clear sessionStorage on export - Add restoreUnsavedChangesFromSession action to restore state - Add useEffect in ToolIde to restore state when tool is loaded This fixes the issue where the export reminder notification would disappear after page reload, even if changes hadn't been exported. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes implement sessionStorage-based persistence for unsaved changes in custom tools. A new effect in ToolIde restores unsaved state from session storage when a tool is loaded, and the store now manages per-tool unsaved-change entries in sessionStorage via new helper methods and an updated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
frontend/src/store/custom-tool-store.js (1)
71-99: Critical: sessionStorage not updated when adding/deleting prompts.Both
addNewInstance(line 86) anddeleteInstance(line 97) sethasUnsavedChanges = truedirectly but don't update sessionStorage. This breaks persistence across reloads:
- User adds or deletes a prompt →
hasUnsavedChangesis set totruein memory- User reloads the page → restoration reads sessionStorage (still empty)
- Export reminder doesn't appear, defeating the purpose of this PR
🔧 Proposed fix to sync sessionStorage in both methods
addNewInstance: (type) => { const newState = { ...getState() }; const promptsAndNotes = newState?.details?.prompts; if (type === promptType.prompt) { const newPrompt = { ...defaultPromptInstance }; newPrompt["prompt_id"] = `unsaved_${promptsAndNotes.length + 1}`; promptsAndNotes.push(newPrompt); } else { const newNote = { ...defaultNoteInstance }; newNote["prompt_id"] = `unsaved_${promptsAndNotes.length + 1}`; promptsAndNotes.push(newNote); } newState["details"]["prompts"] = [...promptsAndNotes]; // Mark as having unsaved changes when a new prompt/note is added newState["hasUnsavedChanges"] = true; + // Persist to sessionStorage + const toolId = newState.details?.tool_id; + if (toolId) { + sessionStorage.setItem(getSessionStorageKey(toolId), "true"); + } setState({ ...newState }); }, deleteInstance: (promptId) => { const newState = { ...getState() }; const promptsAndNotes = newState?.details?.prompts; const filteredData = promptsAndNotes.filter( (item) => item?.prompt_id !== promptId ); newState["details"]["prompts"] = filteredData; // Mark as having unsaved changes when a prompt/note is deleted newState["hasUnsavedChanges"] = true; + // Persist to sessionStorage + const toolId = newState.details?.tool_id; + if (toolId) { + sessionStorage.setItem(getSessionStorageKey(toolId), "true"); + } setState({ ...newState }); },
🧹 Nitpick comments (1)
frontend/src/store/custom-tool-store.js (1)
121-157: Consider adding error handling for sessionStorage operations.While sessionStorage is widely supported, operations can throw in restricted environments (e.g., storage disabled, quota exceeded). Wrapping calls in try-catch blocks would prevent potential crashes and degrade gracefully.
🛡️ Example: Graceful error handling
setHasUnsavedChanges: (hasChanges) => { const toolId = getState().details?.tool_id; if (toolId) { - if (hasChanges) { - sessionStorage.setItem(getSessionStorageKey(toolId), "true"); - } else { - sessionStorage.removeItem(getSessionStorageKey(toolId)); + try { + if (hasChanges) { + sessionStorage.setItem(getSessionStorageKey(toolId), "true"); + } else { + sessionStorage.removeItem(getSessionStorageKey(toolId)); + } + } catch (error) { + console.warn("Failed to persist unsaved changes to sessionStorage:", error); } } setState({ hasUnsavedChanges: hasChanges }); },Apply the same pattern to
markChangesAsExportedandrestoreUnsavedChangesFromSession.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
frontend/src/components/custom-tools/tool-ide/ToolIde.jsxfrontend/src/store/custom-tool-store.js
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/store/custom-tool-store.js (1)
frontend/src/components/agency/side-panel/SidePanel.jsx (1)
toolId(14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (1)
160-172: LGTM! Restoration logic is well-integrated.The restoration effect correctly:
- Runs when the tool loads or changes (via
details?.tool_iddependency)- Uses the proper Zustand
getState()pattern to avoid dependency issues- Resets the deployment usage check flag when unsaved changes are restored, ensuring the export reminder appears appropriately
frontend/src/store/custom-tool-store.js (2)
5-9: LGTM! Per-tool storage key pattern is clean.The prefix and helper function provide good isolation between tools, and the naming convention is clear.
148-157: LGTM! Restoration logic is correct.The method safely reads from sessionStorage and returns a boolean indicating whether restoration occurred, which allows the caller to take appropriate action.
4 participants
What
Why
How
Frontend Changes:
unstract-unsaved-changes-{toolId}for per-tool state isolationsetHasUnsavedChangesin custom-tool-store to persist to sessionStoragemarkChangesAsExportedto clear sessionStorage on successful exportrestoreUnsavedChangesFromSessionaction to restore state from sessionStorageToolIdeto restore state when tool is loadedStorage Pattern:
Can this PR break any existing features. If yes, please list possible items. If no, please explain why.
No, this PR will not break any existing features because:
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code