feat: Graph support for nested Object field values#3326
feat: Graph support for nested Object field values#3326mtrezza merged 4 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. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds support for Object-typed columns with nested dot-path references (e.g., Changes
Sequence DiagramsequenceDiagram
actor User
participant GraphDialog
participant ObjectPathModal
participant State
participant GraphDataUtils
User->>GraphDialog: Opens Graph editor / selects column
alt Selected column is Object
GraphDialog->>ObjectPathModal: Open "Enter Object Path" modal
ObjectPathModal->>User: Prompt for dot-path (e.g., "views.count")
User->>ObjectPathModal: Enter & confirm dot-path
ObjectPathModal->>GraphDialog: confirmObjectPath(path)
GraphDialog->>State: Append full dot-path (e.g., "metadata.views.count")
GraphDialog->>GraphDataUtils: Save config referencing dot-path
else Selected column is non-Object
GraphDialog->>State: Update field selection normally
end
GraphDialog->>GraphDataUtils: validateGraphConfig(config) -> uses isValidColumn for dot-path checks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.gitignore (1)
22-23: Pattern inconsistency in AI tools ignore rules.The
.claudeentry (line 22) has no leading slash, meaning it will ignore.claudefiles in any directory. However,/.superpowers(line 23) has a leading slash, restricting it to only the repository root.If both AI tool files should be ignored regardless of location, consider using consistent patterns:
♻️ Proposed fix for consistency
# AI tools .claude -/.superpowers +.superpowers🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 22 - 23, The .gitignore has inconsistent patterns for AI tool files: ".claude" (no leading slash) ignores files in any directory while "/.superpowers" (leading slash) only ignores the repo root; pick one behavior and make them consistent—either change "/.superpowers" to ".superpowers" to ignore that file anywhere, or change ".claude" to "/.claude" to restrict both to the repo root; update the entries for the symbols ".claude" and ".superpowers" accordingly.src/dashboard/Data/Browser/GraphDialog.react.js (1)
288-292: Missing validation for the nested path format.The
confirmObjectPathmethod directly concatenatesfield.pathwithout validating the path format. Users could enter invalid paths like:
..nested(leading dot)nested.(trailing dot)nested..value(consecutive dots)- Paths with spaces or special characters
While the Modal's
disabledprop prevents empty paths, it doesn't prevent malformed ones. These invalid paths would pass tovalidateGraphConfigandisValidColumn, potentially causing unexpected behavior during chart rendering.🛡️ Proposed fix to add path validation
Add a validation helper and use it in the Modal:
+ isValidObjectPath(path) { + if (!path || typeof path !== 'string') { + return false; + } + // Path should be valid identifier(s) separated by dots + // Each segment must start with letter/underscore, contain only alphanumeric/underscore + const segments = path.split('.'); + return segments.every(seg => seg && /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(seg)); + } + confirmObjectPath = () => { const { field, path, onConfirm } = this.state.objectPathInput; + if (!this.isValidObjectPath(path)) { + return; + } onConfirm(`${field}.${path}`); this.setState({ objectPathInput: null }); };And update the Modal's disabled prop:
- disabled={!this.state.objectPathInput.path || !this.state.objectPathInput.path.trim()} + disabled={!this.isValidObjectPath(this.state.objectPathInput.path)}Also applies to: 1334-1356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/Data/Browser/GraphDialog.react.js` around lines 288 - 292, The confirmObjectPath method currently concatenates field and path from this.state.objectPathInput without validating path format, allowing malformed nested paths; add a helper (e.g., isValidNestedPath) to validate that the path: has no leading/trailing dots, no consecutive dots, contains only allowed characters (no spaces/special chars), and is non-empty, then use that helper in confirmObjectPath to refuse/clear invalid input before calling onConfirm(`${field}.${path}`) and update the Modal's disabled prop to also check isValidNestedPath(this.state.objectPathInput.path) (and/or combine with existing empty checks) so malformed paths are blocked before validateGraphConfig/isValidColumn see confirmObjectPath, objectPathInput, validateGraphConfig, isValidColumn, and the Modal disabled usage.src/lib/tests/GraphDataUtils.test.js (1)
688-705: Duplicate assertion values in pie data test.Both assertions on lines 702-703 check for the same value (
toContain(30)), which passes because both categories have a final value of 30. While technically correct, this makes the test less precise—it only proves that 30 appears somewhere in the array twice, not that both A and B specifically have the expected values.🔧 Suggested fix for more precise assertions
- expect(result.datasets[0].data).toContain(30); // A: 10 + 20 - expect(result.datasets[0].data).toContain(30); // B: 30 + // Get values by label position for precise assertions + const idxA = result.labels.indexOf('Value (A)'); + const idxB = result.labels.indexOf('Value (B)'); + expect(result.datasets[0].data[idxA]).toBe(30); // A: 10 + 20 + expect(result.datasets[0].data[idxB]).toBe(30); // B: 30🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/tests/GraphDataUtils.test.js` around lines 688 - 705, The test for processPieData uses duplicate toContain(30) assertions which don't assert values per-category; update the assertions to locate the index of each label (e.g., find index = result.labels.indexOf('Value (A)') and indexB = result.labels.indexOf('Value (B)')) and then assert exact values at those indices against expected sums (A => 10+20, B => 30) from result.datasets[0].data; this ensures the test verifies that 'Value (A)' and 'Value (B)' map to their correct numeric values rather than just asserting that 30 appears in the array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 23: The .gitignore has inconsistent use of a leading slash between
entries: change the pattern for .superpowers to match the style used for .claude
(remove the leading slash) so both entries are written as ".superpowers" and
".claude" (or alternatively make both root-only by adding a leading slash to
.claude); update the line containing "/.superpowers" to ".superpowers" to ensure
consistent matching behavior across directories.
---
Nitpick comments:
In @.gitignore:
- Around line 22-23: The .gitignore has inconsistent patterns for AI tool files:
".claude" (no leading slash) ignores files in any directory while
"/.superpowers" (leading slash) only ignores the repo root; pick one behavior
and make them consistent—either change "/.superpowers" to ".superpowers" to
ignore that file anywhere, or change ".claude" to "/.claude" to restrict both to
the repo root; update the entries for the symbols ".claude" and ".superpowers"
accordingly.
In `@src/dashboard/Data/Browser/GraphDialog.react.js`:
- Around line 288-292: The confirmObjectPath method currently concatenates field
and path from this.state.objectPathInput without validating path format,
allowing malformed nested paths; add a helper (e.g., isValidNestedPath) to
validate that the path: has no leading/trailing dots, no consecutive dots,
contains only allowed characters (no spaces/special chars), and is non-empty,
then use that helper in confirmObjectPath to refuse/clear invalid input before
calling onConfirm(`${field}.${path}`) and update the Modal's disabled prop to
also check isValidNestedPath(this.state.objectPathInput.path) (and/or combine
with existing empty checks) so malformed paths are blocked before
validateGraphConfig/isValidColumn see confirmObjectPath, objectPathInput,
validateGraphConfig, isValidColumn, and the Modal disabled usage.
In `@src/lib/tests/GraphDataUtils.test.js`:
- Around line 688-705: The test for processPieData uses duplicate toContain(30)
assertions which don't assert values per-category; update the assertions to
locate the index of each label (e.g., find index = result.labels.indexOf('Value
(A)') and indexB = result.labels.indexOf('Value (B)')) and then assert exact
values at those indices against expected sums (A => 10+20, B => 30) from
result.datasets[0].data; this ensures the test verifies that 'Value (A)' and
'Value (B)' map to their correct numeric values rather than just asserting that
30 appears in the array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 734dfb44-bab1-4ea7-8d04-c6b873790ca5
📒 Files selected for processing (4)
.gitignoresrc/dashboard/Data/Browser/GraphDialog.react.jssrc/lib/GraphDataUtils.jssrc/lib/tests/GraphDataUtils.test.js
# [9.1.0-alpha.12](9.1.0-alpha.11...9.1.0-alpha.12) (2026-04-07) ### Features * Graph support for nested Object field values ([#3326](#3326)) ([4562381](4562381))
|
🎉 This change has been released in version 9.1.0-alpha.12 |
# [9.1.0](9.0.0...9.1.0) (2026-04-07) ### Bug Fixes * Bump fast-xml-parser from 5.3.5 to 5.3.6 ([#3223](#3223)) ([aee458b](aee458b)) * Cell content not selected on double clicking data browser cell ([#3271](#3271)) ([9df3beb](9df3beb)) * Column resizing mouse cursor in data browser not visible in Safari browser ([#3246](#3246)) ([e6fb4d7](e6fb4d7)) * Date value cannot be pasted into date field in data browser ([#3243](#3243)) ([e902bea](e902bea)) * Edit icon does not disappear when hovering out of saved filter name in data browser sidebar ([#3245](#3245)) ([d3dcfce](d3dcfce)) * Layout issues when resizing Cloud Config parameter dialog ([#3241](#3241)) ([c6e95d9](c6e95d9)) * Remove unused dependencies ([#3227](#3227)) ([3ba250d](3ba250d)) * Security removal dependency null-loader ([#3230](#3230)) ([5e1b1fa](5e1b1fa)) * Security removal dependency svg-prep ([#3236](#3236)) ([abb08c6](abb08c6)) * Security upgrade transitive dependency ajv ([#3231](#3231)) ([d1e5e41](d1e5e41)) * Security upgrade transitive dependency qs ([#3228](#3228)) ([225c710](225c710)) * Security upgrade transitive dependency undici ([#3229](#3229)) ([8e1be1f](8e1be1f)) * Security upgrade undici ([#3265](#3265)) ([df23ef8](df23ef8)) ### Features * Add confirmation dialog when closing Cloud Config edit parameter dialog without saving changes ([#3247](#3247)) ([9ec03e0](9ec03e0)) * Add diff view to Cloud Config parameter dialog for better conflict handling ([#3239](#3239)) ([f007a68](f007a68)) * Add support for data import in data browser ([#3244](#3244)) ([16f60f4](16f60f4)) * Enforce remote access restrictions on `agent` endpoint ([#3255](#3255)) ([edef824](edef824)) * Graph support for nested Object field values ([#3326](#3326)) ([4562381](4562381)) * Highlight row of selected cell in data browser ([#3270](#3270)) ([298ae63](298ae63))
|
🎉 This change has been released in version 9.1.0 |
Pull Request
Issue
Graphs cannot display values from Object-type fields. For example, if a field
metadatacontains{ "views": { "count": 1 } }, there is no way to graphmetadata.views.count.Approach
Object fields appear in all graph dropdowns with an
[object]suffix. Clicking one opens a modal to enter the nested key path. The composed dot-path (e.g.,metadata.views.count) becomes a selectable field like any other. No data processing changes needed sincegetNestedValuealready handles dot-path traversal.Tasks
Summary by CodeRabbit
New Features
Tests
Chores