fix: Legacy script in JavaScript console not imported to modern console #2963
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe changes refactor tab and script ID management to use UUIDs instead of numeric IDs, enhance legacy script import logic to prevent duplicates and ensure correct ordering, and extend dashboard settings to manage both view preferences and scripts. Navigation guards are added to the playground to warn users about unsaved changes before navigating away. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Playground
participant ScriptManager
participant Browser
User->>Playground: Attempt to close tab or navigate away
Playground->>Playground: Check for unsaved changes
alt Unsaved changes exist
Playground->>User: Show confirmation dialog
alt User confirms
Playground->>Browser: Proceed with navigation/unload
else User cancels
Playground->>Browser: Prevent navigation/unload
end
else No unsaved changes
Playground->>Browser: Proceed with navigation/unload
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
mtrezza
changed the title
mtrezza
changed the title
![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
🧹 Nitpick comments (2)
src/dashboard/Data/Playground/Playground.react.js (2)
656-744: Consider refactoring to reduce code duplication.The unsaved changes detection logic is duplicated between
useBeforeUnloadand thecheckForUnsavedChangesfunction. While the implementation works correctly, consider extracting this logic into a shared function.Extract the common logic:
+ const hasUnsavedChanges = useCallback(() => { + for (const tab of tabs) { + if (tab.saved === false) { + return true; + } + + let currentContent = ''; + if (tab.id === activeTabId && editorRef.current) { + currentContent = editorRef.current.value; + } else { + currentContent = tab.code; + } + + const savedTab = savedTabs.find(saved => saved.id === tab.id); + + if (!savedTab) { + if (currentContent.trim() !== '') { + return true; + } + } else { + if (currentContent !== savedTab.code) { + return true; + } + } + } + return false; + }, [tabs, activeTabId, savedTabs]); useBeforeUnload( useCallback( (event) => { - // Check for unsaved changes across all tabs - let hasChanges = false; - // ... duplicate code ... - if (hasChanges) { + if (hasUnsavedChanges()) { const message = 'You have unsaved changes in your playground tabs. Are you sure you want to leave?'; event.preventDefault(); event.returnValue = message; return message; } }, - [tabs, activeTabId, savedTabs] + [hasUnsavedChanges] ) );
180-180: Consider using ScriptManager's generateScriptId() for consistency.While directly using
crypto.randomUUID()works correctly, the codebase would be more maintainable if all script/tab ID generation went throughScriptManager.generateScriptId(). This would centralize UUID generation logic and make future changes easier.Consider using the centralized method:
- const newTabId = crypto.randomUUID(); + const newTabId = scriptManagerRef.current?.generateScriptId() || crypto.randomUUID();Also applies to: 258-258, 267-267, 317-317
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/dashboard/Data/Playground/Playground.react.js(5 hunks)src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(7 hunks)src/lib/ScriptManager.js(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtrezza
PR: parse-community/parse-dashboard#2828
File: src/dashboard/Data/Browser/Browser.react.js:1605-1607
Timestamp: 2025-05-27T12:09:47.644Z
Learning: In script execution dialogs in Parse Dashboard (specifically the `confirmExecuteScriptRows` method in `src/dashboard/Data/Browser/Browser.react.js`), individual `setState` calls to update `processedScripts` counter should be kept as-is rather than batched, because this provides real-time progress feedback to users in the dialog UI.
📚 Learning: in script execution dialogs in parse dashboard (specifically the `confirmexecutescriptrows` method i...
Learnt from: mtrezza
PR: parse-community/parse-dashboard#2828
File: src/dashboard/Data/Browser/Browser.react.js:1605-1607
Timestamp: 2025-05-27T12:09:47.644Z
Learning: In script execution dialogs in Parse Dashboard (specifically the `confirmExecuteScriptRows` method in `src/dashboard/Data/Browser/Browser.react.js`), individual `setState` calls to update `processedScripts` counter should be kept as-is rather than batched, because this provides real-time progress feedback to users in the dialog UI.
Applied to files:
src/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Settings/DashboardSettings/DashboardSettings.react.jssrc/lib/ScriptManager.js
📚 Learning: preference reads and writes in the classpreferences.js module are expected to be infrequent operatio...
Learnt from: mtrezza
PR: parse-community/parse-dashboard#2769
File: src/lib/ClassPreferences.js:26-26
Timestamp: 2025-05-02T11:55:52.809Z
Learning: Preference reads and writes in the ClassPreferences.js module are expected to be infrequent operations, so optimizing for performance (like caching) is unnecessary in this context.
Applied to files:
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
📚 Learning: the bcryptjs library is used in parse dashboard for password encryption and validation in three file...
Learnt from: mtrezza
PR: parse-community/parse-dashboard#0
File: :0-0
Timestamp: 2025-05-11T16:43:27.354Z
Learning: The bcryptjs library is used in Parse Dashboard for password encryption and validation in three files: Parse-Dashboard/Authentication.js (compareSync), Parse-Dashboard/CLI/mfa.js (genSaltSync, hashSync), and src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (genSaltSync, hashSync).
Applied to files:
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
⏰ 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: Docker linux/amd64
🔇 Additional comments (14)
src/lib/ScriptManager.js (5)
48-79: LGTM! Well-implemented legacy script import logic.The implementation correctly:
- Prevents duplicate legacy scripts from being imported multiple times
- Ensures the legacy script opens as the first tab (order 0)
- Marks it as saved to avoid unnecessary unsaved warnings
- Preserves the order of existing scripts by incrementing them
260-267: LGTM! Clean public API for script ID generation.Good encapsulation of the UUID generation logic with proper JSDoc documentation.
283-284: LGTM! Correct handling of UUID script IDs from server.The change correctly preserves the script ID as a string to support UUIDs.
372-376: LGTM! Proper legacy script initialization.Good decision to mark the imported legacy script as saved, preventing unnecessary unsaved change warnings for this one-time migration.
412-417: Approved: crypto.randomUUID() ImplementationThis is the correct, web-standard approach for UUID v4 generation. According to MDN,
crypto.randomUUID()is supported in:
- Chrome 92+ / Edge 92+ / Opera 78+
- Firefox 95+
- Safari 15.4+
- Secure contexts only (HTTPS or localhost)
- Node.js 16.7.0+ (
crypto.randomUUID()in the crypto module) and Web Crypto API in Node.js 18+No code changes required. If you must support older browsers or non-HTTPS environments, include a polyfill.
src/dashboard/Data/Playground/Playground.react.js (5)
180-184: LGTM! Proper UUID initialization for tabs.Good use of
useMemoto ensure the initial tab ID is generated only once.
317-329: LGTM! Clean refactoring to UUID-based tab IDs.The removal of
nextTabIdstate simplifies the code while maintaining correct functionality.
258-268: LGTM! Consistent UUID usage in tab initialization.All code paths correctly generate UUIDs for new tabs, maintaining consistency.
600-600: LGTM! Correct removal of obsolete nextTabId logic.
603-653: LGTM! Comprehensive navigation guard for browser unload events.The implementation correctly:
- Checks all tabs for unsaved changes
- Handles both active (editor content) and inactive (stored code) tabs
- Uses the standard
beforeunloadpattern for maximum compatibilitysrc/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (4)
21-21: LGTM! Clean integration of ScriptManager.The initialization follows the existing pattern and the method rename appropriately reflects its expanded scope.
Also applies to: 32-32, 62-71
129-141: LGTM! Proper handling of dual storage deletion.The implementation correctly ensures both view preferences and scripts are deleted, with appropriate success/failure messaging.
474-478: LGTM! Clear and informative UI updates.The explanatory text effectively communicates the expanded scope of server storage to include JS Console scripts.
483-483: LGTM! Improved clarity in field descriptions.The updated descriptions are more concise and the warning emojis effectively highlight important information.
Also applies to: 503-503, 519-519
# [7.4.0-alpha.3](7.4.0-alpha.2...7.4.0-alpha.3) (2025-08-04) ### Bug Fixes * Legacy script in JavaScript console not imported to modern console ([#2963](#2963)) ([8c8d084](8c8d084))
|
🎉 This change has been released in version 7.4.0-alpha.3 |
# [7.4.0](7.3.0...7.4.0) (2025-09-01) ### Bug Fixes * Legacy script in JavaScript console not imported to modern console ([#2963](#2963)) ([8c8d084](8c8d084)) ### Features * Add App Settings option to store dashboard settings on server ([#2958](#2958)) ([666e078](666e078)) * Add config parameter name to quick add dialogs in Config page ([#2970](#2970)) ([31988f6](31988f6)) * Add info panel setting to auto-load first row on opening new browser tab ([#2972](#2972)) ([020a25d](020a25d)) * Modernize JavaScript console with tabs and server-side storage of scripts ([#2962](#2962)) ([6e0c7f2](6e0c7f2))
|
🎉 This change has been released in version 7.4.0 |
* source: chore(release): 7.4.0 [skip ci] empty commit to trigger CI chore(release): 7.4.0-alpha.5 [skip ci] feat: Add info panel setting to auto-load first row on opening new browser tab (parse-community#2972) chore(release): 7.4.0-alpha.4 [skip ci] feat: Add config parameter name to quick add dialogs in Config page (parse-community#2970) refactor: Bump @babel/runtime-corejs3 from 7.27.4 to 7.28.3 (parse-community#2966) chore(release): 7.4.0-alpha.3 [skip ci] fix: Legacy script in JavaScript console not imported to modern console (parse-community#2963) chore(release): 7.4.0-alpha.2 [skip ci] feat: Modernize JavaScript console with tabs and server-side storage of scripts (parse-community#2962) chore(release): 7.4.0-alpha.1 [skip ci] feat: Add App Settings option to store dashboard settings on server (parse-community#2958) refactor: Bump inquirer from 12.6.3 to 12.9.0 (parse-community#2959)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor