fix: Saved filter is not recognized in data browser filter dialog#3108
fix: Saved filter is not recognized in data browser filter dialog#3108mtrezza merged 5 commits intoparse-community:alphafrom
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
|
Warning Rate limit exceeded@mtrezza has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 43 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces ClassPreferences with a prop-driven Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant BrowserToolbar
participant BrowserFilter
participant Server
User->>Browser: Open Data Browser for class
Browser->>Server: GET saved filters for class
Server-->>Browser: Return savedFilters array
Browser->>Browser: Store in classFilters[className]
Browser->>BrowserToolbar: Render with savedFilters prop
BrowserToolbar->>BrowserFilter: Pass savedFilters
BrowserFilter->>BrowserFilter: updateFilterInfoIfNeeded()
User->>BrowserFilter: Save new filter
BrowserFilter->>BrowserFilter: save() (async) -> await onSaveFilter(...)
BrowserFilter->>Browser: onSaveFilter (prop call)
Browser->>Server: POST filter
Server-->>Browser: Return saved filter (with id)
Browser->>Browser: reloadClassFilters(className)
Browser->>Server: GET updated savedFilters
Server-->>Browser: Return merged savedFilters
Browser->>BrowserFilter: componentDidUpdate(prevProps) -> sync UI with savedFilters
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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🧪 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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @src/components/BrowserFilter/BrowserFilter.react.js:
- Around line 248-264: In the matchingFilter callback passed to
savedFilters.find, the local variable savedFilters shadows the outer
savedFilters; rename the parsed JSON variable to parsedFilters (or similar) when
doing JSON.parse(savedFilter.filter) and update subsequent uses (e.g., mapping
to normalizedSavedFilters and stringifying) so the outer savedFilters array is
not shadowed and comparisons still use parsedFilters.
- Around line 545-564: The find callback for matchingFilter shadows outer
savedFilters and inner map parameter names; rename the parsed JSON variable
(e.g., parsedSavedFilters or parsedFilters) instead of reusing savedFilters,
rename the map item parameter (e.g., savedFilterItem), and update all references
(including building normalizedSavedFilters and savedFiltersString) so there is
no variable shadowing in the matchingFilter logic that compares against
currentFilterInfo, currentFiltersString and this.props.className.
- Around line 161-177: The callback for savedFilters.find shadows the outer
savedFilters by declaring const savedFilters = JSON.parse(savedFilter.filter);;
rename the inner variable (e.g., parsedFilters or parsedSavedFilters) and update
all uses inside the callback (normalizedSavedFilters mapping, savedFiltersString
generation, and the JSON.parse call site) so the outer savedFilters remains
unshadowed; keep the existing normalization logic involving urlClassName and the
comparison to urlFiltersString intact and preserve the try/catch behavior.
In @src/dashboard/Data/Browser/Browser.react.js:
- Around line 1445-1464: The merge uses serverFilterIds = new
Set(existingFilters.map(f => f.id)) which treats undefined IDs as a valid key
and causes local filters without ids to be considered duplicates; update the
merge in Browser.react.js to build a de-duplication key that ignores undefined
ids and matches legacy filters by name (e.g., when creating serverFilterIds only
add f.id if defined, and for id-less filters use a name-based key matching
legacyFilterName), then compute localOnlyFilters by checking for either an id
match or a name match against that same key strategy so legacy filters are
preserved.
🧹 Nitpick comments (2)
src/dashboard/Data/Browser/Browser.react.js (1)
318-353: Consider adding unmount guard for async state updates.The async
loadAllClassFiltersmethod updates state after awaitingPromise.all. If the component unmounts while filters are loading (e.g., user navigates away), callingsetStateon an unmounted component could cause issues.♻️ Suggested improvement
async loadAllClassFilters(propsToUse) { if (!this.filterPreferencesManager) { return; } // Use provided props or fall back to this.props const props = propsToUse || this.props; const schema = props.schema; if (!schema || !schema.data) { return; } const classFilters = {}; // Load filters for all classes const classList = schema.data.get('classes'); if (classList) { const classNames = Object.keys(classList.toObject()); await Promise.all( classNames.map(async (className) => { try { const filters = await this.filterPreferencesManager.getFilters( this.context.applicationId, className ); classFilters[className] = filters || []; } catch (error) { console.error(`Failed to load filters for class ${className}:`, error); classFilters[className] = []; } }) ); } + // Guard against unmounted component + if (!this._isMounted) { + return; + } this.setState({ classFilters }); }You would also need to set
this._isMounted = trueincomponentDidMountandthis._isMounted = falseincomponentWillUnmount.src/components/BrowserFilter/BrowserFilter.react.js (1)
101-107: Consider migrating from deprecatedcomponentWillReceiveProps.
componentWillReceivePropsis deprecated in React 16.3+ and will be removed in a future major version. The logic here could be moved tocomponentDidUpdateorgetDerivedStateFromProps.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/BrowserFilter/BrowserFilter.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Browser/BrowserToolbar.react.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Browser/Browser.react.js (2)
src/lib/ClassPreferences.js (3)
className(62-62)preferences(26-26)preferences(63-63)src/lib/ColumnPreferences.js (2)
className(56-56)preferences(57-57)
🔇 Additional comments (7)
src/dashboard/Data/Browser/BrowserToolbar.react.js (1)
30-30: LGTM! Prop correctly threaded through the component.The
savedFiltersprop is properly added to the component's parameter list and forwarded toBrowserFilter. This enables the filter UI to consume server-backed saved filters.Also applies to: 494-494
src/dashboard/Data/Browser/Browser.react.js (4)
50-50: LGTM! Good use of constant for referential stability.Using a constant empty array instead of inline
[]prevents unnecessary re-renders since the reference remains stable across renders.
1568-1587: LGTM! Clean implementation for reloading class-specific filters.The method correctly handles errors and updates state for the specific class only, avoiding unnecessary reloads of other classes' filters.
2466-2476: LGTM! Appropriate fallback logic for backwards compatibility.The conditional fallback to local storage (ClassPreferences) only occurs when server storage is not configured or preferred, ensuring seamless migration for existing users.
2672-2672: LGTM! Correct prop passing with stable fallback.Uses the
EMPTY_FILTERS_ARRAYconstant for the fallback, maintaining referential stability.src/components/BrowserFilter/BrowserFilter.react.js (2)
84-114: LGTM! Good handling of async savedFilters loading.The
componentDidUpdatehook properly detects whensavedFiltersprop changes (async load completes) and updates the filter info. This ensures the UI reflects the correct filter name once server data is available.
684-684: LGTM! Async save properly awaits the filter save operation.Making
save()async and awaitingonSaveFilterensures that the filter ID is available before updating the URL and state, which is necessary for the server-backed storage workflow.Also applies to: 734-734
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/components/BrowserFilter/BrowserFilter.react.js:
- Around line 247-263: The find callback that computes matchingFilter
deserializes savedFilter.filter and compares with urlFiltersString using
JSON.stringify which is order-dependent; change this to canonicalize both sides
consistently (same as isCurrentFilterSaved): parse savedFilter.filter, normalize
each filter by removing class when it equals urlClassName, then canonicalize
each object by sorting object keys recursively (or otherwise producing a stable
string representation) and sort the array of canonicalized filters before
joining/serializing for comparison against the similarly canonicalized
urlFilters representation; update the logic in the matchingFilter callback (the
JSON.parse(savedFilter.filter), normalizedSavedFilters map, and comparison step)
to use the stable/canonical comparison instead of raw JSON.stringify.
- Around line 160-176: The comparison in the matchingFilter callback uses
JSON.stringify on normalizedSavedFilters (inside savedFilters.find) which is
order-dependent and can yield false negatives; instead, after parsing and
normalizing each savedFilter (the normalizedSavedFilters array where you remove
class when it matches urlClassName), compare it to the parsed urlFilters (or
urlFiltersString equivalent) using a deep equality check (e.g. lodash.isEqual)
or a stable/sorted-key stringify function; update the matchingFilter logic to
parse urlFilters once, import/use isEqual (or implement key-sorted comparison),
replace the JSON.stringify equality check with isEqual(normalizedSavedFilters,
parsedUrlFilters), and preserve the existing try/catch to return false on parse
errors.
🧹 Nitpick comments (2)
src/components/BrowserFilter/BrowserFilter.react.js (2)
162-162: Variable shadowing: rename inner variable for clarity.The variable
savedFilterson Line 162 shadows the outersavedFiltersfrom Line 127. This makes the code harder to understand and maintain.♻️ Rename to avoid shadowing
- const savedFilters = JSON.parse(savedFilter.filter); + const parsedFilterData = JSON.parse(savedFilter.filter); // Normalize saved filters for comparison (remove class property if it matches current className) - const normalizedSavedFilters = savedFilters.map(filter => { + const normalizedSavedFilters = parsedFilterData.map(filter => {
249-249: Variable shadowing: rename inner variable.The variable
savedFiltersshadows the outersavedFiltersfrom Line 193. Rename toparsedFilterDataor similar for clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/BrowserFilter/BrowserFilter.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 (5)
src/components/BrowserFilter/BrowserFilter.react.js (5)
83-98: LGTM! Well-structured lifecycle helper.The method appropriately guards against unnecessary updates and correctly syncs filter info from savedFilters when the dialog is in edit mode.
297-337: LGTM! Clean refactoring with proper error handling.The method correctly uses the savedFilters prop and includes appropriate try-catch blocks for JSON parsing.
375-398: LGTM! Collision detection logic is sound.The method correctly handles both modern filters (with ID) and legacy filters (without ID) when checking for name collisions.
683-761: LGTM! Async save flow is well-implemented.The async/await pattern correctly ensures that URL updates and state transitions happen after the save operation completes, preventing race conditions.
108-113: No issue found. The reference equality comparison is appropriate for this use case.The parent component (Browser.react.js) stores
classFiltersin state and only updates the array reference when filters are actually saved or loaded viasetState, not on every render. For the undefined case, it returnsEMPTY_FILTERS_ARRAY, a singleton constant. Therefore,prevProps.savedFilters !== this.props.savedFilterswill only trigger when filters genuinely change, not on every render. Additionally,updateFilterInfoIfNeeded()contains guards (this.state.open && this.state.showMore && !this.state.name) that prevent unnecessary state updates even if the condition is met.
# [8.2.0-alpha.20](8.2.0-alpha.19...8.2.0-alpha.20) (2026-01-11) ### Bug Fixes * Saved filter is not recognized in data browser filter dialog ([#3108](#3108)) ([8a4ce15](8a4ce15))
|
🎉 This change has been released in version 8.2.0-alpha.20 |
# [8.2.0](8.1.0...8.2.0) (2026-01-15) ### Bug Fixes * Data browser pagination is ignored when using browser navigation or page reload ([#3097](#3097)) ([bcc4d5f](bcc4d5f)) * Batch-navigation is active even if info panels are not visible ([#3053](#3053)) ([91b544a](91b544a)) * Calculated value drop-down in menu bar overlaps with info panel buttons ([#3116](#3116)) ([0f6f729](0f6f729)) * Context menu in data browser disappears behind menu bar ([#3106](#3106)) ([2c6c471](2c6c471)) * Data browser table headers misaligned when scrolling horizontally ([#3067](#3067)) ([f495dd1](f495dd1)) * Graph panel covers right-most columns of data browser table ([#3112](#3112)) ([00b0d70](00b0d70)) * Graph panel shows date tick labels on x-axis in local time instead of UTC ([#3111](#3111)) ([85d4946](85d4946)) * Header row in View table disappears when scrolling up ([#3105](#3105)) ([2923e86](2923e86)) * Info panel covers whole sidebar if fewer objects than panels in multi-panel scenario ([#3042](#3042)) ([dd3ba8d](dd3ba8d)) * Info panel not refreshing on script execution ([#3040](#3040)) ([f57e7e2](f57e7e2)) * Prefetch for multiple info panels in data browser doesn't refresh stale cached data ([#3080](#3080)) ([e71d4e6](e71d4e6)) * Right-click on info panel header selects the object ([#3082](#3082)) ([ae87114](ae87114)) * Saved filter is not recognized in data browser filter dialog ([#3108](#3108)) ([8a4ce15](8a4ce15)) * Some context menu sub-menu lists don't change background color on mouse hover in Safari browser ([#3109](#3109)) ([6269d18](6269d18)) ### Features * Add AI agent browser control for autonomous development ([#3114](#3114)) ([5940455](5940455)) * Add confirmation dialog to handle conflicts when migrating settings to server ([#3092](#3092)) ([ae50b8d](ae50b8d)) * Add getting related records to context menu of info panel header ([#3083](#3083)) ([2623802](2623802)) * Add graph panel for data visualization ([#3110](#3110)) ([1e15e27](1e15e27)) * Add keyboard shortcuts for quick actions in data browser ([#3073](#3073)) ([858d0cc](858d0cc)) * Add Node 24 support ([#3041](#3041)) ([8cf2735](8cf2735)) * Add storing data browser filters on server ([#3090](#3090)) ([b991734](b991734)) * Add storing data browser graphs to support multiple graphs per class ([#3113](#3113)) ([e76f605](e76f605)) * Add support for `Video` type in View table to display videos ([#3061](#3061)) ([bd4aa4f](bd4aa4f)) * Allow selecting objects by click-dragging over info panel headers ([#3074](#3074)) ([d6ef86c](d6ef86c)) * Auto-expand filter list when clicking on class in data browser ([#3101](#3101)) ([30a733c](30a733c)) * Execute scripts via right-click on info panel header column ([#3068](#3068)) ([2983741](2983741)) ### Performance Improvements * Add local caching for server-stored settings to reduce loading from server ([#3094](#3094)) ([409973a](409973a)) * Remove unnecessary data fetches from data browser pagination ([#3098](#3098)) ([bc59998](bc59998))
* source: (82 commits) chore(release): 8.2.0 [skip ci] empty commit to trigger CI chore(release): 8.2.0-alpha.27 [skip ci] fix: Calculated value drop-down in menu bar overlaps with info panel buttons (parse-community#3116) chore(release): 8.2.0-alpha.26 [skip ci] feat: Add AI agent browser control for autonomous development (parse-community#3114) chore(release): 8.2.0-alpha.25 [skip ci] feat: Add storing data browser graphs to support multiple graphs per class (parse-community#3113) chore(release): 8.2.0-alpha.24 [skip ci] fix: Graph panel covers right-most columns of data browser table (parse-community#3112) chore(release): 8.2.0-alpha.23 [skip ci] fix: Graph panel shows date tick labels on x-axis in local time instead of UTC (parse-community#3111) chore(release): 8.2.0-alpha.22 [skip ci] feat: Add graph panel for data visualization (parse-community#3110) chore(release): 8.2.0-alpha.21 [skip ci] fix: Some context menu sub-menu lists don't change background color on mouse hover in Safari browser (parse-community#3109) chore(release): 8.2.0-alpha.20 [skip ci] fix: Saved filter is not recognized in data browser filter dialog (parse-community#3108) chore(release): 8.2.0-alpha.19 [skip ci] fix: Context menu in data browser disappears behind menu bar (parse-community#3106) ...
2 participants
New Pull Request Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.