feat: Add App Settings option to store dashboard settings on server #2958
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThis change introduces a new abstraction for managing dashboard view preferences, supporting both local and server-side storage. It adds the Changes
Sequence Diagram(s)Dashboard Views: Loading and Saving with Storage PreferencesequenceDiagram
participant User
participant ViewsComponent
participant ViewPreferencesManager
participant ServerConfigStorage
participant LocalStorage
User->>ViewsComponent: Load dashboard views
ViewsComponent->>ViewPreferencesManager: getViews(appId)
alt Prefers server storage and enabled
ViewPreferencesManager->>ServerConfigStorage: getConfigsByPrefix()
ServerConfigStorage-->>ViewPreferencesManager: views[]
ViewPreferencesManager-->>ViewsComponent: views[]
else Prefers local storage or server disabled
ViewPreferencesManager->>LocalStorage: get views
LocalStorage-->>ViewPreferencesManager: views[]
ViewPreferencesManager-->>ViewsComponent: views[]
end
User->>ViewsComponent: Save/modify views
ViewsComponent->>ViewPreferencesManager: saveViews(appId, views)
alt Prefers server storage and enabled
ViewPreferencesManager->>ServerConfigStorage: setConfig()
ServerConfigStorage-->>ViewPreferencesManager: ack
ViewPreferencesManager-->>ViewsComponent: done
else Prefers local storage or server disabled
ViewPreferencesManager->>LocalStorage: save views
LocalStorage-->>ViewPreferencesManager: ack
ViewPreferencesManager-->>ViewsComponent: done
end
Dashboard Settings: Storage Preference and MigrationsequenceDiagram
participant User
participant DashboardSettings
participant ViewPreferencesManager
participant ServerConfigStorage
participant LocalStorage
User->>DashboardSettings: Change storage preference
DashboardSettings->>ViewPreferencesManager: setStoragePreference(appId, preference)
ViewPreferencesManager-->>DashboardSettings: ack
User->>DashboardSettings: Migrate to server storage
DashboardSettings->>ViewPreferencesManager: migrateToServer(appId)
ViewPreferencesManager->>LocalStorage: get views
LocalStorage-->>ViewPreferencesManager: views[]
ViewPreferencesManager->>ServerConfigStorage: setConfig()
ServerConfigStorage-->>ViewPreferencesManager: ack
ViewPreferencesManager-->>DashboardSettings: migration complete
User->>DashboardSettings: Delete from browser
DashboardSettings->>ViewPreferencesManager: deleteFromBrowser(appId)
ViewPreferencesManager->>LocalStorage: delete views
LocalStorage-->>ViewPreferencesManager: ack
ViewPreferencesManager-->>DashboardSettings: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (2)
59-75: Consider handling context changes after mount.The initialization logic is sound, but if the context changes after the component mounts (e.g., switching apps), the
viewPreferencesManagerwon't be reinitialized with the new context.Add
componentDidUpdateto handle context changes:+ componentDidUpdate(prevProps, prevState) { + if (this.context?.applicationId !== prevProps.context?.applicationId) { + this.initializeViewPreferencesManager(); + } + }
77-85: Simplify the notification message.The handler logic is correct, but the ternary operator in the notification message is redundant.
- this.showNote(`Storage preference changed to ${preference === 'server' ? 'server' : 'browser'}`); + this.showNote(`Storage preference changed to ${preference === 'server' ? 'Server' : 'Browser'}`);This capitalizes the storage location for consistency with the UI labels.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/dashboard/Data/Views/Views.react.js(6 hunks)src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(5 hunks)src/lib/ParseApp.js(2 hunks)src/lib/ServerConfigStorage.js(1 hunks)src/lib/StoragePreferences.js(1 hunks)src/lib/ViewPreferencesManager.js(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
src/dashboard/Data/Views/Views.react.js (1)
Learnt from: mtrezza
PR: #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.
src/lib/ServerConfigStorage.js (1)
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).
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (2)
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).
Learnt from: mtrezza
PR: #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.
src/lib/StoragePreferences.js (2)
Learnt from: mtrezza
PR: #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.
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).
src/lib/ViewPreferencesManager.js (1)
Learnt from: mtrezza
PR: #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.
🧬 Code Graph Analysis (2)
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (1)
src/lib/ViewPreferencesManager.js (1)
ViewPreferencesManager(17-271)
src/lib/ViewPreferencesManager.js (2)
src/lib/ServerConfigStorage.js (1)
ServerConfigStorage(14-194)src/lib/StoragePreferences.js (2)
prefersServerStorage(66-68)setStoragePreference(46-59)
🔇 Additional comments (4)
src/lib/ParseApp.js (1)
52-53: LGTM!The addition of the
configparameter to store server configuration is appropriate for enabling server-side storage features.Also applies to: 83-83
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (3)
20-20: Import looks good.The
ViewPreferencesManagerimport is correctly added to support the new storage preference functionality.
30-30: Constructor initialization is appropriate.The
viewPreferencesManagerproperty and new state fields are properly initialized. The comment explaining thatstoragePreferencewill be updated incomponentDidMountis helpful.Also applies to: 44-45
461-514: Well-structured UI implementation.The Settings Storage fieldset is well-designed with:
- Clear, descriptive labels and help text
- Appropriate visual feedback (loading states, color coding)
- Logical grouping of related actions
- Good accessibility with proper label associations
| async loadViews(app) { | ||
| // Initialize ViewPreferencesManager if not already done or if app changed | ||
| if (!this.viewPreferencesManager || this.viewPreferencesManager.app !== app) { | ||
| this.viewPreferencesManager = new ViewPreferencesManager(app); | ||
| } | ||
|
|
||
| try { | ||
| const views = await this.viewPreferencesManager.getViews(app.applicationId); | ||
| this.setState({ views, counts: {} }, () => { | ||
| views.forEach(view => { | ||
| if (view.showCounter) { | ||
| if (view.cloudFunction) { | ||
| // For Cloud Function views, call the function to get count | ||
| Parse.Cloud.run(view.cloudFunction, {}, { useMasterKey: true }) | ||
| .then(res => { | ||
| if (this._isMounted) { | ||
| this.setState(({ counts }) => ({ | ||
| counts: { ...counts, [view.name]: Array.isArray(res) ? res.length : 0 }, | ||
| })); | ||
| } | ||
| }) | ||
| .catch(error => { | ||
| if (this._isMounted) { | ||
| this.showNote(`Request failed: ${error.message || 'Unknown error occurred'}`, true); | ||
| } | ||
| }); | ||
| } else if (view.query && Array.isArray(view.query)) { | ||
| // For aggregation pipeline views, use existing logic | ||
| new Parse.Query(view.className) | ||
| .aggregate(view.query, { useMasterKey: true }) | ||
| .then(res => { | ||
| if (this._isMounted) { | ||
| this.setState(({ counts }) => ({ | ||
| counts: { ...counts, [view.name]: res.length }, | ||
| })); | ||
| } | ||
| }) | ||
| .catch(error => { | ||
| if (this._isMounted) { | ||
| this.showNote(`Request failed: ${error.message || 'Unknown error occurred'}`, true); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
| if (this._isMounted) { | ||
| this.loadData(this.props.params.name); | ||
| } | ||
| }); | ||
| if (this._isMounted) { | ||
| this.loadData(this.props.params.name); | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.error('Failed to load views from server, falling back to local storage:', error); | ||
| // Fallback to local storage | ||
| const views = ViewPreferences.getViews(app.applicationId); | ||
| this.setState({ views, counts: {} }, () => { | ||
| if (this._isMounted) { | ||
| this.loadData(this.props.params.name); | ||
| } | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider showing user feedback on storage errors
When views fail to load from or save to server storage, errors are only logged to console. Users won't know their views weren't saved to their preferred storage location.
Consider using the existing showNote method to inform users:
} catch (error) {
console.error('Failed to load views from server, falling back to local storage:', error);
+ this.showNote('Failed to load views from server, using local storage', true);
// Fallback to local storage
const views = ViewPreferences.getViews(app.applicationId);📝 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.
| async loadViews(app) { | |
| // Initialize ViewPreferencesManager if not already done or if app changed | |
| if (!this.viewPreferencesManager || this.viewPreferencesManager.app !== app) { | |
| this.viewPreferencesManager = new ViewPreferencesManager(app); | |
| } | |
| try { | |
| const views = await this.viewPreferencesManager.getViews(app.applicationId); | |
| this.setState({ views, counts: {} }, () => { | |
| views.forEach(view => { | |
| if (view.showCounter) { | |
| if (view.cloudFunction) { | |
| // For Cloud Function views, call the function to get count | |
| Parse.Cloud.run(view.cloudFunction, {}, { useMasterKey: true }) | |
| .then(res => { | |
| if (this._isMounted) { | |
| this.setState(({ counts }) => ({ | |
| counts: { ...counts, [view.name]: Array.isArray(res) ? res.length : 0 }, | |
| })); | |
| } | |
| }) | |
| .catch(error => { | |
| if (this._isMounted) { | |
| this.showNote(`Request failed: ${error.message || 'Unknown error occurred'}`, true); | |
| } | |
| }); | |
| } else if (view.query && Array.isArray(view.query)) { | |
| // For aggregation pipeline views, use existing logic | |
| new Parse.Query(view.className) | |
| .aggregate(view.query, { useMasterKey: true }) | |
| .then(res => { | |
| if (this._isMounted) { | |
| this.setState(({ counts }) => ({ | |
| counts: { ...counts, [view.name]: res.length }, | |
| })); | |
| } | |
| }) | |
| .catch(error => { | |
| if (this._isMounted) { | |
| this.showNote(`Request failed: ${error.message || 'Unknown error occurred'}`, true); | |
| } | |
| }); | |
| } | |
| } | |
| }); | |
| if (this._isMounted) { | |
| this.loadData(this.props.params.name); | |
| } | |
| }); | |
| if (this._isMounted) { | |
| this.loadData(this.props.params.name); | |
| } | |
| }); | |
| } catch (error) { | |
| console.error('Failed to load views from server, falling back to local storage:', error); | |
| // Fallback to local storage | |
| const views = ViewPreferences.getViews(app.applicationId); | |
| this.setState({ views, counts: {} }, () => { | |
| if (this._isMounted) { | |
| this.loadData(this.props.params.name); | |
| } | |
| }); | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Failed to load views from server, falling back to local storage:', error); | |
| + this.showNote('Failed to load views from server, using local storage', true); | |
| // Fallback to local storage | |
| const views = ViewPreferences.getViews(app.applicationId); | |
| this.setState({ views, counts: {} }, () => { | |
| if (this._isMounted) { | |
| this.loadData(this.props.params.name); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/dashboard/Data/Views/Views.react.js around lines 88 to 147, the loadViews
method logs errors to the console when failing to load views from the server but
does not inform the user. To fix this, add a call to this.showNote with a
user-friendly error message inside the catch block that handles the server load
failure. This will notify users that their views could not be loaded from the
server and the app is falling back to local storage.
| async migrateToServer() { | ||
| if (!this.viewPreferencesManager) { | ||
| this.showNote('ViewPreferencesManager not initialized'); | ||
| return; | ||
| } | ||
|
|
||
| if (!this.viewPreferencesManager.serverStorage.isServerConfigEnabled()) { | ||
| this.showNote('Server configuration is not enabled for this app. Please add a "config" section to your app configuration.'); | ||
| return; | ||
| } | ||
|
|
||
| this.setState({ migrationLoading: true }); | ||
|
|
||
| try { | ||
| const result = await this.viewPreferencesManager.migrateToServer(this.context.applicationId); | ||
| if (result.success) { | ||
| if (result.viewCount > 0) { | ||
| this.showNote(`Successfully migrated ${result.viewCount} view(s) to server storage.`); | ||
| } else { | ||
| this.showNote('No views found to migrate.'); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| this.showNote(`Failed to migrate views: ${error.message}`); | ||
| } finally { | ||
| this.setState({ migrationLoading: false }); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid direct access to internal serverStorage property.
The method accesses viewPreferencesManager.serverStorage directly at line 93, which violates encapsulation. Consider adding a method to ViewPreferencesManager to check if server config is enabled.
If no such method exists, consider adding one to ViewPreferencesManager instead of accessing internal properties directly.
🏁 Script executed:
#!/bin/bash
# Check if ViewPreferencesManager exposes a method to check server config status
ast-grep --pattern 'class ViewPreferencesManager {
$$$
isServerConfigEnabled($_) {
$$$
}
$$$
}'Length of output: 109
🏁 Script executed:
#!/bin/bash
# Locate the ViewPreferencesManager class definition
echo "==> Searching for ViewPreferencesManager class definition"
rg "class ViewPreferencesManager" -n .
# List any references to serverStorage across the codebase
echo "==> Searching for serverStorage references"
rg "serverStorage" -n .Length of output: 1620
Refactor server config checks into ViewPreferencesManager
Directly accessing viewPreferencesManager.serverStorage breaks encapsulation. Introduce a wrapper on ViewPreferencesManager and update all calls accordingly.
• In src/lib/ViewPreferencesManager.js, add:
export default class ViewPreferencesManager {
// … existing code …
/** Wraps serverStorage.isServerConfigEnabled */
isServerConfigEnabled() {
return this.serverStorage.isServerConfigEnabled();
}
// … existing code …
}• In src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js, replace at line 93 and line 460:
- this.viewPreferencesManager.serverStorage.isServerConfigEnabled()
+ this.viewPreferencesManager.isServerConfigEnabled()🤖 Prompt for AI Agents
In src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js between
lines 87 and 114, the code directly accesses
viewPreferencesManager.serverStorage.isServerConfigEnabled(), which breaks
encapsulation. To fix this, add an isServerConfigEnabled() method in
src/lib/ViewPreferencesManager.js that wraps the call to
serverStorage.isServerConfigEnabled(). Then, replace all direct calls to
viewPreferencesManager.serverStorage.isServerConfigEnabled() in
DashboardSettings.react.js (including line 93 and line 460) with
viewPreferencesManager.isServerConfigEnabled() to maintain proper encapsulation.
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (3)
97-97: Avoid direct access to internal serverStorage property.Direct access to
viewPreferencesManager.serverStoragebreaks encapsulation. Add a method toViewPreferencesManagerto check if server config is enabled.
132-148: Add confirmation dialog for destructive action.The delete operation is irreversible and should require user confirmation before proceeding.
480-480: Same encapsulation issue with serverStorage access.This line also directly accesses the internal
serverStorageproperty ofviewPreferencesManager.
🧹 Nitpick comments (3)
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (1)
85-85: Remove trailing spaces.Remove trailing spaces from these lines to maintain code cleanliness.
- this.setState({ storagePreference: preference }); - + this.setState({ storagePreference: preference }); +Also applies to: 108-108, 113-113
src/dashboard/Data/Browser/Browser.react.js (2)
1366-1367: Track the TODO for async conversion.The comment indicates this synchronous approach is temporary. Consider creating a GitHub issue to track the conversion to async when the component architecture supports it.
Would you like me to open an issue to track this TODO for converting to async methods?
284-284: Consider migrating to modern React lifecycle methods.This component uses deprecated lifecycle methods (
componentWillMount,componentWillReceiveProps). These have been deprecated since React 16.3 and may be removed in future versions.Consider migrating to:
componentWillMount→componentDidMountor constructorcomponentWillReceiveProps→componentDidUpdateorstatic getDerivedStateFromPropsThis will ensure compatibility with future React versions and improve maintainability.
Also applies to: 399-399
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/BrowserFilter/BrowserFilter.react.js(3 hunks)src/dashboard/Data/Browser/Browser.react.js(12 hunks)src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(5 hunks)src/lib/ClassPreferences.js(1 hunks)src/lib/FilterPreferencesManager.js(1 hunks)src/lib/StoragePreferences.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/lib/StoragePreferences.js
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/components/BrowserFilter/BrowserFilter.react.jssrc/lib/FilterPreferencesManager.jssrc/lib/ClassPreferences.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
📚 Learning: in parse dashboard's data browser selection logic (src/dashboard/data/browser/browsertable.react.js)...
Learnt from: mtrezza
PR: parse-community/parse-dashboard#2957
File: src/dashboard/Data/Browser/BrowserTable.react.js:584-597
Timestamp: 2025-07-31T06:12:17.707Z
Learning: In Parse Dashboard's data browser selection logic (src/dashboard/Data/Browser/BrowserTable.react.js), the `selection['*']` pattern is used to handle global operations that pass `{ '*': true }` to indicate all items are selected, particularly for bulk operations like delete. This is not dead code but serves as compatibility layer for global operations that don't go through normal individual row selection workflows.
Applied to files:
src/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
📚 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/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
🪛 GitHub Check: Lint
src/dashboard/Data/Browser/Browser.react.js
[failure] 379-379:
Trailing spaces not allowed
[failure] 371-371:
Trailing spaces not allowed
[failure] 356-356:
Trailing spaces not allowed
[failure] 354-354:
Trailing spaces not allowed
[failure] 1360-1360:
Trailing spaces not allowed
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
[failure] 118-118:
Expected { after 'if' condition
[failure] 117-117:
Expected { after 'if' condition
[failure] 113-113:
Trailing spaces not allowed
[failure] 108-108:
Trailing spaces not allowed
[failure] 85-85:
Trailing spaces not allowed
⏰ 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 (10)
src/components/BrowserFilter/BrowserFilter.react.js (2)
21-21: LGTM! Clean integration with the class preferences manager.The import and property initialization follow standard patterns and properly integrate with the new preferences management system.
Also applies to: 49-49
95-98: LGTM! Proper initialization in lifecycle method.The manager is correctly initialized in
componentDidMountwith appropriate context checking.src/lib/ClassPreferences.js (2)
164-169: LGTM! Well-implemented singleton pattern.The singleton correctly maintains one instance per app and handles app changes appropriately.
172-226: LGTM! Legacy API properly preserved.The legacy functions are correctly maintained for backward compatibility, which is essential for existing integrations.
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (1)
31-32: LGTM! Proper initialization of preference managers.The managers are correctly initialized in
componentDidMountwith appropriate context checking.Also applies to: 61-71
src/lib/FilterPreferencesManager.js (1)
17-319: Excellent implementation of the FilterPreferencesManager!The class is well-designed with:
- Robust error handling and fallback mechanisms
- Clean separation between server and local storage operations
- Comprehensive migration capabilities
- Proper async/await usage throughout
The implementation follows best practices and provides a solid foundation for preference management.
src/dashboard/Data/Browser/Browser.react.js (4)
32-32: LGTM! Clean integration of ClassPreferencesManager.The initialization pattern properly sets up the manager and ensures filters are loaded after schema is available.
Also applies to: 133-133, 290-297
155-156: LGTM! Appropriate state management for async filter loading.The new state properties properly support caching filters and tracking loading state.
1487-1550: LGTM! Consistent error handling pattern.The deleteFilter method properly implements the same defensive fallback pattern as saveFilters, ensuring compatibility.
2384-2387: LGTM! Good UX handling for filter loading.The implementation properly handles the loading state and sorts filters alphabetically for better user experience.
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/lib/ViewPreferencesManager.js (2)
139-147: Handle query parsing errors more robustlyThis matches a previous concern - when query parsing fails, the view is returned with a string query which could cause runtime errors downstream. The view should be skipped or marked as corrupted rather than passed through with an invalid query.
259-267: Improve view ID generation to prevent collisionsThis matches a previous concern - the current ID generation using name hash and timestamp could create collisions if multiple views are created with the same name within the same millisecond. Adding a random component would resolve this.
🧹 Nitpick comments (1)
src/lib/ViewPreferencesManager.js (1)
28-44: Consider fallback behavior when server storage fails.The current implementation returns an empty array when server storage is preferred but fails, rather than falling back to local storage. While this appears intentional based on the comment, it could result in poor UX where users temporarily lose access to their views during server outages.
Consider providing a more graceful fallback or at least adding user notification when server storage fails.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(5 hunks)src/lib/StoragePreferences.js(1 hunks)src/lib/ViewPreferencesManager.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
- src/lib/StoragePreferences.js
🧰 Additional context used
🧠 Learnings (1)
📚 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/lib/ViewPreferencesManager.js
🔇 Additional comments (8)
src/lib/ViewPreferencesManager.js (8)
1-13: LGTM!The imports and version constant are well-structured. Using a VERSION constant for localStorage versioning is a good practice for managing data format changes.
17-21: LGTM!The constructor correctly initializes dependencies and follows dependency injection patterns.
52-65: LGTM!Good error handling with fallback to local storage on server failures. This prevents data loss and provides a better user experience than the getViews method.
72-104: LGTM!Both methods have appropriate error handling and clear return values. The migration method properly validates server configuration before attempting migration.
111-122: LGTM!Clean delegation to storage preference utilities with appropriate abstraction.
167-215: LGTM!Excellent implementation of server storage with proper lifecycle management. The approach of deleting obsolete views first, cleaning null values, and stringifying complex queries is well-designed.
221-253: LGTM!Local storage methods have appropriate error handling and clean fallback behavior for localStorage failures.
272-296: LGTM!Legacy functions provide good backward compatibility during the migration period. The separate path function ensures no conflicts with the new implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/lib/ViewPreferencesManager.js (2)
148-156: Query parsing error handling implemented correctly.The implementation properly handles corrupted queries by skipping invalid views and logging appropriate warnings, which addresses potential runtime errors from malformed query strings.
269-278: View ID generation collision prevention implemented correctly.The implementation includes both timestamp and random components to ensure uniqueness, which effectively prevents collisions when multiple views are created rapidly.
🧹 Nitpick comments (1)
src/lib/ViewPreferencesManager.js (1)
283-307: Legacy API functions provide proper backward compatibility.The functions correctly maintain the original local storage behavior. Consider whether the
path()function could reuse the class's_getLocalPath()logic to reduce duplication, though the current separation may be intentional for API independence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/dashboard/Data/Views/Views.react.js(6 hunks)src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(5 hunks)src/lib/ServerConfigStorage.js(1 hunks)src/lib/StoragePreferences.js(1 hunks)src/lib/ViewPreferencesManager.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/lib/ServerConfigStorage.js
- src/dashboard/Data/Views/Views.react.js
- src/lib/StoragePreferences.js
- src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js
🧰 Additional context used
🧠 Learnings (1)
📚 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/lib/ViewPreferencesManager.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 (8)
src/lib/ViewPreferencesManager.js (8)
1-12: LGTM!The imports and version constant are properly structured and follow the project's conventions.
17-21: LGTM!The class constructor properly initializes dependencies and follows good practices for dependency injection.
28-44: LGTM!The method correctly implements the storage preference logic with proper error handling. When server storage is preferred but fails, it returns an empty array instead of falling back to local storage, which maintains user intent.
52-65: LGTM!The save method appropriately falls back to local storage when server storage fails, ensuring user data is not lost. This is the correct behavior for save operations.
72-89: LGTM!The migration method properly validates server availability, handles edge cases (empty local views), and provides useful feedback through return values and error propagation.
96-130: LGTM!The utility methods are well-implemented with appropriate delegation and error handling. The API design is clean and intuitive.
177-225: LGTM!The server storage implementation comprehensively handles view synchronization, including cleanup of obsolete views and proper data serialization. The error handling ensures failures are properly communicated to the caller.
231-262: LGTM!The local storage methods maintain backward compatibility and have appropriate error handling. The versioned localStorage key ensures data integrity across application versions.
# [7.4.0-alpha.1](7.3.0...7.4.0-alpha.1) (2025-08-03) ### Features * Add App Settings option to store dashboard settings on server ([#2958](#2958)) ([666e078](666e078))
|
🎉 This change has been released in version 7.4.0-alpha.1 |
# [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
Improvements