feat: Add data browser filter condition containedIn
#2979
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 or files that can be reviewed per hour. Please wait 2 minutes and 42 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 (1)
📝 WalkthroughWalkthroughAdds a new "containedIn" constraint across Filters, UI, and query translation. Adds constraint metadata, a JSON-array input path in FilterRow, switches containedIn filter payloads to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FR as FilterRow (UI)
participant FS as Filters State
participant QF as queryFromFilters
participant Q as Query Builder
participant V as Views (handleOpenAllPointers)
U->>FR: Select constraint = "containedIn" and enter text
FR->>FR: Render input as JSON string if value is array
FR->>FR: On change -> try JSON.parse
alt parsed is array
FR->>FS: onChangeCompareTo([values])
else parsed not array or parse error
FR->>FS: onChangeCompareTo(raw string)
end
U->>V: Click "Open pointers" on column
V->>V: Collect pointers, group by class
alt single class & ids present
V->>FS: Build filter { field: 'objectId', constraint:'containedIn', compareTo: [ids] }
V->>Q: Navigate to browser/<Class>?filters=<encoded_filters>
else zero pointers
V-->>U: Show note "No pointers found in this column"
else multiple classes
V-->>U: Show error about multiple classes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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.✅ security/snyk check is complete. No issues have been found. (View Details) |
Uffizzi Ephemeral Environment
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/PushAudiencesSelector/PushAudiencesSelector.react.js (1)
23-29: Avoid converting compareTo array to Immutable.List.
fromJSturns the$inarray into an Immutable.List, which then reachesquery.containedInand breaks. Construct this filter withMapsocompareTostays a plain array.Apply this diff:
-import { fromJS } from 'immutable'; +import { fromJS, Map } from 'immutable'; @@ - ? filters.push( - fromJS({ - field: 'deviceType', - constraint: 'containedIn', - compareTo: query.deviceType['$in'], - }) - ) + ? filters.push( + Map({ + field: 'deviceType', + constraint: 'containedIn', + compareTo: query.deviceType['$in'], + }) + )
🧹 Nitpick comments (2)
src/lib/queryFromFilters.js (1)
99-104: Minor: guard JSON.parse errors in key helpers.*
IfcompareTois not a JSON string here,JSON.parsewill throw. Not new but becomes more likely ascompareTousage expands. Consider try/catch to avoid breaking the whole filter pipeline.src/components/BrowserFilter/FilterRow.react.js (1)
251-253: Nice: passing currentConstraint into compareValue. One more UX tweak suggested.
When switching tocontainedIn, consider initializingcompareToto[]so users start from a valid value and we avoid accidental string submissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/BrowserFilter/FilterRow.react.js(2 hunks)src/components/PushAudiencesSelector/PushAudiencesSelector.react.js(1 hunks)src/lib/Filters.js(1 hunks)src/lib/queryFromFilters.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/lib/Filters.js (1)
src/components/ExplorerQueryComposer/ExplorerFilter.js (2)
FieldConstraints(47-54)FieldConstraints(47-54)
src/components/BrowserFilter/FilterRow.react.js (2)
src/components/PushAudienceDialog/InstallationCondition.react.js (1)
setFocus(26-30)src/components/ExplorerQueryComposer/ExplorerQueryComposer.react.js (1)
setFocus(103-107)
⏰ 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 (3)
src/lib/Filters.js (2)
178-182: Good addition: new constraint descriptor is consistent with existing shape.
containedInis correctly introduced and markedcomparable: true. No issues here.
185-189: Ensure 'containedIn' values are coerced to the correct Parse typessrc/lib/Filters.js enables
containedInfor Pointer, Boolean, Number, String, and Date — ensure the query builder (FilterRow / queryFromFilters) encodes/coerces values before sending to Parse: Pointers → Pointer-format/Parse.Object, Dates → Date objects / Parse-encoded dates, Booleans/Numbers → native types; otherwise Parse will receive mismatched types. I could not locate FilterRow or queryFromFilters in the repo — verify/implement this serialization where filters are turned into Parse queries.src/components/BrowserFilter/FilterRow.react.js (1)
31-33: Signature change looks good.
PassingcurrentConstraintintocompareValueis the right hook for rendering the containedIn-specific input.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/dashboard/Data/Views/Views.react.js (3)
575-576: Accessibility copy: minor polish and consistency.Consider shorter, action-first phrasing and keep aria-label/title in sync with other headers. Example: “Filter rows by pointers in {name}.” This is optional, purely UX copy.
-aria-label={`Filter to show all pointers from ${name} column`} -title="Filter to show all pointers from this column" +aria-label={`Filter rows by pointers in ${name}`} +title="Filter rows by pointers in this column"
873-876: Use a neutral note for “no pointers” instead of an error.This is an expected state, not an error. Show a neutral/positive note for better UX signal.
- this.showNote('No pointers found in this column', true); + this.showNote('No pointers found in this column', false);
894-903: Stabilize filter URL by sorting IDs (optional).Sorting makes the generated URL deterministic across renders (useful for caching/sharing and tests).
-const uniqueObjectIds = Array.from(pointersByClass.get(targetClassName)); +const uniqueObjectIds = Array.from(pointersByClass.get(targetClassName)).sort();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/dashboard/Data/Views/Views.react.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Views/Views.react.js (1)
src/lib/generatePath.js (2)
filters(8-8)generatePath(3-34)
⏰ 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/dashboard/Data/Views/Views.react.js (5)
878-885: Good deduping by class and objectId.Using Map + Set avoids duplicates and keeps the flow clear.
887-892: Clear guard against mixed target classes.Good user-facing error with actionable guidance.
913-915: Success note is helpful.Good concise feedback with pluralization handled.
899-912: Guard extremely long filter URLsSearch shows filters are embedded in generated URLs in multiple places — e.g. src/dashboard/Data/Views/Views.react.js (generatePath + window.open), src/dashboard/Data/Browser/Browser.react.js (generatePath/navigate and several containedIn usages), src/components/PermissionsDialog/PermissionsDialog.react.js, src/components/CategoryList/CategoryList.react.js, src/components/PushAudiencesSelector/PushAudiencesSelector.react.js. Implement a single length guard in generatePath or your navigation helper (conservative threshold ~4000 chars); on exceed, show a short user message or fall back to a short server-side filterId (or chunk/post) instead of embedding the full filters. If centralizing is not possible, apply the same conservative check before any window.open/navigate call.
899-903: Confirm containedIn payload contract across the app.queryFromFilters passes filter.get('compareTo') directly to Parse.Query.containedIn (so it must be an array, not an un-parsed JSON/string). Verify the UI/storage/serialization consistently produce an array for containedIn.
- Check src/lib/queryFromFilters.js — case 'containedIn' calls query.containedIn(filter.get('field'), filter.get('compareTo')).
- Check src/components/BrowserFilter/FilterRow.react.js — currentConstraint === 'containedIn' renders a text input; confirm compareValue/onChangeCompareTo normalize to an array (or otherwise produce a JSON-parsed array).
- Check src/dashboard/Data/Views/Views.react.js — filters are JSON.stringify([...]) with compareTo: uniqueObjectIds (array); ensure consumers parse/preserve the array before query construction.
# [7.5.0-alpha.2](7.5.0-alpha.1...7.5.0-alpha.2) (2025-09-11) ### Features * Add data browser filter condition `containedIn` ([#2979](#2979)) ([c1dc5bb](c1dc5bb))
|
🎉 This change has been released in version 7.5.0-alpha.2 |
New Pull Request Checklist
Issue Description
Data browser filter is missing a "contained in" filter condition.
Approach
Adds new filter condition "contained in" to data browser. For example, allows to filter for objects with IDs contained in
[1, 2].Summary by CodeRabbit
New Features
Refactor
UX Improvements