Conversation
Add RTK Query slice, MSW mock handlers, and three summary cards (violations over time chart, violation rate, top data consumers) with time range toggle for the access control summary page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
clients/admin-ui/src/features/access-control/summary/SummaryCards.tsx
Outdated
Show resolved
Hide resolved
…/ethyca/fides into access-control-summary-dashboard
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…fides into access-control-summary-dashboard
| return 80; | ||
| } | ||
| const lineCount = (violation.sql_statement ?? "").split("\n").length; | ||
| return Math.max(80, lineCount * 18 + 42); |
There was a problem hiding this comment.
Somewhat arbitrary sizing/nudging
There was a problem hiding this comment.
Code Review: ENG-2943 PBAC Dashboard
Good overall structure — the feature is well-decomposed into small, single-purpose components and the RTK Query slice is clean. A few issues worth addressing before merge:
Critical (Must Fix)
router.query read in lazy initializer may be empty on first render (useRequestLogFilters.ts, line 65–73)
In Next.js, router.query is not populated until after hydration. The lazy useState initializer runs synchronously on first render, where router.query is {}, so URL-encoded facets from a direct link or page refresh will be silently dropped. See inline comment for the fix.
Suggestions
Dual URL mutation systems (useRequestLogFilters.ts, line 97)
applyFacets uses window.history.replaceState while the useEffect sync uses router.replace. The skipUrlSyncRef flag is the glue holding this together, but it's fragile — any missed skipUrlSyncRef.current = true set causes a double-update. Consolidating to a single navigation method would eliminate the coordination overhead entirely.
Live tail doesn't reset cursor (useInfiniteViolationLogs.ts, line 32)
When a user has scrolled down (cursor is set) and then enables live tail, the polling re-fetches the same mid-list page rather than the newest violations. The cursor should be reset to null when live tail becomes active.
Timestamp formatted via string slicing (ViolationDetailDrawer.tsx, line 113–114)
.toISOString().slice(0, 10) / .slice(11, 19) is fragile if the string shape ever changes and doesn't respect locale. date-fns format() is already in the project.
skip option missing on useGetFiltersQuery (index.tsx)
When the feature flag is off, useGetFiltersQuery still fires because hooks must be called unconditionally. Passing skip: !flags.alphaPurposeBasedAccessControl prevents the unnecessary network request.
Nice to Have
Row key separator collision (FindingsTable.tsx, line 43)
${policy}-${control} can theoretically collide if either value contains a hyphen. Using :: as the separator is safer.
Disabled footer buttons (ViolationDetailDrawer.tsx, lines 59–62)
"Edit policy" and "Create new policy" are always disabled. A brief TODO comment would make it clear this is intentional placeholder UI rather than a bug.
clients/admin-ui/src/features/access-control/hooks/useRequestLogFilters.ts
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/hooks/useRequestLogFilters.ts
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/hooks/useInfiniteViolationLogs.ts
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/ViolationDetailDrawer.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/DataConsumersCard.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/ViolationDetailDrawer.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/access-control/ViolationDetailDrawer.tsx
Outdated
Show resolved
Hide resolved
- Fix router.query hydration: use useEffect with router.isReady instead of lazy useState - Remove live tail feature entirely (liveTail state, polling, LIVE_TAIL_POLL_MS) - Skip useGetFiltersQuery when alphaPurposeBasedAccessControl flag is off Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- DataConsumersCard: use Row/Col with wrap={false}, add Dividers, request size=5 server-side
- ViolationDetailDrawer: remove redundant width, use PageSpinner
- ChartText: accept width prop to prevent label wrapping
- access-control.slice: add size param to ConsumersByViolationsParams
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@gilluminate hm I should handle the empty data state for the faceted search. |

Ticket [ENG-2943]
Description Of Changes
Adds the PBAC (Policy-Based Access Control) summary dashboard with violation charts, data consumer rankings, faceted search, and a drill-down violation log.
Dashboard layout:
Table tabs:
Filters & navigation:
Code Changes
New feature files (
src/features/access-control/):access-control.slice.ts— RTK Query slice with 7 endpoints injected into baseApitypes.ts— TypeScript interfaces for all API response shapestrendUtils.ts— shared trend prefix/color helpersAccessControlTableTabs.tsx— tab container managing summary ↔ log switching and detail drawerViolationsChartCard.tsx— area/bar chart card with line/bar toggleViolationRateCard.tsx— donut chart violation rate cardDataConsumersCard.tsx— top consumers mini-tableFindingsTable.tsx— paginated policy violations summary tableRequestLogTable.tsx— infinite-scroll violation log tableViolationDetailDrawer.tsx— side drawer with violation details and SQL viewerFacetedSearchInput.tsx— multi-select grouped search inputrequestLogColumns.tsx/violationsColumns.tsx— column definitionshooks/useRequestLogFilters.ts— filter state context (date range, facets, URL sync)hooks/useInfiniteViolationLogs.ts— cursor pagination accumulator with live-tail pollingModified files:
src/features/common/api.slice.ts— added "Access Control" tag typesrc/features/common/nav/routes.ts— removed unused sub-route constantssrc/pages/data-discovery/access-control/index.tsx— replaced placeholder with full dashboardsummary/index.tsxandrequest-log/index.tsx(flattened into single page)fidesui tweaks:
ChartText.tsx— addedmaxLines={1}to prevent label overflowchart-constants.ts— widenedLABEL_WIDTHfrom 110 → 120Steps to Confirm
npm run devfromclients/admin-ui/with a local backend/data-discovery/access-controlPre-Merge Checklist
CHANGELOG.mdupdated