Migrate Strategic Accounts dashboard to agent-native#1595
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Here's a visual recap of what changed:
Open the full interactive recap |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
steve8708
left a comment
There was a problem hiding this comment.
@NKoech123 this is still what I said not to do. this is hard-coding builder-specific use cases and workflows into the analytics app.
agent-native analytics is an open app for anyone to use for analytics. we need solutions that are not hardcoded like actions called update-strategic-accounts. if this dashboard is super specific to builder, it needs to be built as an extension
|
our dashboards are basically a generic way to attach data (through sources like integrations and SQL) to charts, tables, etc. most things should be able to be accomplished with these core primitives, and we can extend the primitives a bit if needed too (e.g. maybe a dashboard heavily depends on a chart type we don't support - great let's add a new chart type anyone can use) but if this isn't possible, if we can't "genericize" this, extensions is the escape hatch. those can be totally bespoke but usually the gist is you have a data source (integration), you have a visualizatio you want (a query) and our cure dashboars primitives should support that and ideally we do, unless something is so extremely niche that we need that escape hatch |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
This incremental update narrows the PR to dashboard nesting support: it adds a persisted parentId field, threads that metadata into sidebar list items, and renders one-level nested dashboards beneath their parent in the Analytics sidebar. The overall approach is reasonable and the defensive fallback behavior for orphans/self-references/cycles is good, but this is now a standard-risk UI/data-flow change because it affects persisted dashboard config plus sidebar interaction logic.
Key findings:
- 🟡 Nested dashboard metadata is not preserved on ordinary saves — the dashboard page still hydrates/saves configs without round-tripping
parentId, so editing a nested dashboard can silently clear its nesting. - 🟡 Manual drag ordering for nested dashboards is inconsistent — the sortable list now renders parent/child ids in a different order than the drag-end handler reorders, so some nested dashboard moves can be applied incorrectly.
Good patterns: validation was added for config.parentId, sidebar rendering avoids hiding orphans/cycles, and the change set is otherwise nicely scoped.
🧪 Browser testing: Will run after this review (PR touches UI code)
| /** | ||
| * Optional id of another dashboard this one nests under. When set, the | ||
| * sidebar renders this dashboard indented beneath its parent instead of at | ||
| * the top level. Orphans (parent missing/inaccessible) fall back to the top | ||
| * level. Self-references and cycles are ignored by the renderer. | ||
| */ | ||
| parentId?: string; |
There was a problem hiding this comment.
🟡 Nested dashboards lose parentId when saved from the dashboard page
This adds parentId to the stored dashboard config, but fetchDashboard()/prefetch on the dashboard page still rebuild the config without that field before later calling saveDashboard(updated). Renaming or editing a nested dashboard can therefore silently drop parentId and move it back to top level.
Additional Info
Introduced field at types.ts lines 117-123, but app/pages/adhoc/sql-dashboard/index.tsx fetchDashboard() and Sidebar prefetch both omit data.parentId when reconstructing SqlDashboardConfig.
| > | ||
| <SortableContext | ||
| items={displayedDashboards.map((d) => d.id)} | ||
| items={displayedDashboards.flatMap((d) => [ |
There was a problem hiding this comment.
🟡 Nested dashboard drag ordering uses a different ID order than the rendered list
The sortable list now flattens each parent immediately followed by its children, but handleDashboardDragEnd still computes indices from visibleDashboards, whose order can differ. That mismatch can apply arrayMove() to the wrong positions once a child is visually nested under a parent.
Additional Info
SortableContext items at lines 2078-2081 use displayedDashboards + dashboardChildren, while handleDashboardDragEnd (unchanged at ~1838-1843) still uses visibleDashboards.map(d => d.id).
@steve8708 Omg it took me so long to get this!!! Now i can see how our primitive dashboards already handle a lot of these use cases. For example, I just built the required "Strategic Accounts" dashboard: https://analytics.agent-native.com/dashboards/strategic-accounts-bq. While doing that, I realized our dashboard items need nesting support. The goal of this PR now changes to add nesting capabilities to our dashboard since we need it. now i get it. saw the agents.md instructions has good guidances on what ai should be doing here. are we connected to an integration, how it reads schema from integrations, where is the user in the ui, what the user is trying to accomplish, should i created app sql table and what in those fields what kind of config should it be, the nature of the query, the chartType ui primitives to use, panel arrangements, formatting for the data. And the actions we have so far are generic and no need of specific ones |
Summary
Adds generic, opt-in dashboard nesting to the Analytics sidebar: a dashboard can declare a
parentIdand render indented beneath its parent. This is the enabling change for migrating the Strategic Accounts dashboard — and its sub-dashboards Strategic Account Coverage and Implementation Blockers — into agent-native without hardcoding anything Builder-specific into the template.Why this change is necessary
The Strategic Accounts surface in fusion-analytics is a parent view with two child views. Recreating that grouping in agent-native initially looked like it needed either (a) Builder-specific navigation hardcoded in the template, or (b) a
parentIdconcept that didn't exist yet — the sidebar could only render SQL dashboards as a flat list.How it works
parentIdis an optional field on the dashboard config JSON — no schema migration, no new column. The agent/UI set it through the existingupdate-dashboard/mutate-dashboardactions.list-sql-dashboardsalready returns the full config, soparentIdflows to the sidebar automatically.parentIdrender top-level unchanged; orphans (missing/filtered/access-denied parent), self-references, cycles, and deeper-than-one-level descendants all fall back to top level so nothing is ever hidden.Key Changes
app/pages/adhoc/sql-dashboard/types.ts: add optionalparentIdtoSqlDashboardConfig.actions/update-dashboard.ts: validateparentId(non-empty string when present).actions/dashboard-mutation-api.ts: documentparentIdinDashboardPatch(thesetDashboardpath already passes it through).app/components/layout/Sidebar.tsx: carryparentIdfrom the list action, group children beneath top-level parents, and render them indented; preview-count and drag/drop updated to operate on top-level items.changelog/: user-facing "What's new" entry for the nesting capability.Validation:
pnpm typecheckclean; dashboard action tests pass; nesting verified end-to-end locally (create parent + children →list-sql-dashboardsreturnsparentId→ sidebar nests them).Tip
This PR wasn't reviewed by Quality Agent. Enable it in Project Settings to get AI-powered code review on every PR.
To clone this PR locally use the Github CLI with command
gh pr checkout 1595You can tag me at @BuilderIO for anything you want me to fix or change