Resource Sharing v1 + v1.1 (webapp): share modal, groups, People tab, per-role Roles page, chart/KPI/metric sharing#347
Draft
siddhant3030 wants to merge 76 commits into
Draft
Resource Sharing v1 + v1.1 (webapp): share modal, groups, People tab, per-role Roles page, chart/KPI/metric sharing#347siddhant3030 wants to merge 76 commits into
siddhant3030 wants to merge 76 commits into
Conversation
…rd-tile dashboard_id wiring
Task 6 (frontend) of the resource-sharing feature — consumes the /api/access/*
endpoints built on the paired backend branch.
- hooks/api/useResourceAccess.ts: SWR read hook + addGrant/removeGrant/
setGeneralAccess mutation fns, typed against the verbatim backend response
(task-05 report), not the brief's sketch (grants carry name; pending rows
have principal_id: null).
- components/ui/share-modal.tsx: adds optional entityType prop that drives
new "People with access" and "General access" sections via
useResourceAccess, rendered strictly off capabilities flags. Narrowing
general access surfaces persisting_grants and re-sends with
remove_grant_ids on confirm. Read-only mode when the viewer can't share.
Legacy prop-driven usage (reports) is untouched — entityType is optional
and gates the new sections. dashboard-list-v2.tsx and
dashboard-native-view.tsx now pass entityType="dashboard".
- lib/rbac.tsx: adds CAN_SHARE_REPORTS/ALERTS/METRICS/KPIS permission slugs.
- components/dashboard/chart-element-view.tsx + hooks/api/useChart.ts: thread
dashboardId through to GET /api/charts/{id}/ and .../data/ so Members can
view dashboard tiles now that the backend requires dashboard_id in that
context (chart builder / standalone Charts page unaffected).
- components/dashboard/dashboard-list-v2.tsx: removes the "Show only shared"
filter — it keyed off dashboard.is_public, a field the list endpoint never
actually returns, so it silently hid every dashboard when checked.
Known gaps (see task-06 report): the dashboard list DTO lacks
general_audience/is_owner/is_creator, blocking list badges and a real
"Shared with you" filter; /api/organizations/users lacks an orguser id,
so the add-person picker's Add action is UI-complete but disabled pending
a backend field.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the Settings → Groups page (create/rename/delete a group, manage
members) backed by a new useUserGroups hook mirroring useResourceAccess's
envelope-peeling pattern, and extends ShareModal's add-principal picker
with a functional Groups source (group ids are available even while
person-add stays disabled by the T6 orguser_id gap). Lets an org create
"Funders" once and share a dashboard with it in one action.
- hooks/api/useUserGroups.ts: list/detail SWR hooks + create/rename/delete/
addMember/removeMember mutations against /api/groups/*.
- components/settings/groups/*: table (name/member-count/shared-count),
create+rename dialog with inline collision error, delete confirm with an
access-removal warning, and a Sheet-based member drawer. Member-add is
built but permanently disabled with an inline hint — confirmed against
user_org_api.py that GET /api/organizations/users has no orguser_id yet.
- share-modal.tsx: group grant rows (icon + member count) and a new "Add a
group" block wired to addGrant({principal_type: "group", ...}).
- lib/rbac.tsx: CAN_VIEW_USER_GROUPS / CAN_MANAGE_USER_GROUPS slugs.
- constants/analytics.ts: group CRUD events + Settings → Groups feature.
131 suites / 1522 passed / 4 skipped (baseline 124/1482/4 + 40 new tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tile access context Closes the frontend gaps left by Task 6b's backend changes: - Dashboard list: 🔒 Private / audience badge and "Shared with you" badge from the new general_audience/general_level/is_owner/is_creator DTO fields, plus a working "Show only shared with me" filter (replaces the dead is_public-based filter removed in Task 6). - ShareModal "Add a person" and Groups drawer "Add a member": wired to the new orguser_id on GET /api/organizations/users, sending it as principal_id / orguser_id instead of the disabled Coming-soon state. - Table/map dashboard tiles now send chart_id + dashboard_id to chart-data-preview(+total-rows) (query params) and map-data-overlay (body fields), matching the backend's new access gate. Builder and standalone chart pages are unaffected — they call the same hooks without these args. - AccessGrant.email is now string | null (group grants have no email); fixed test mocks that stood in with '' to use null like the real API. Full suite: 133 suites / 1544 passed / 4 skipped, no regressions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… audience badge tooltip
Review fixes on the Task 6c commit:
- getActiveFilterCount now includes nameFilters.showShared — with only
"Show only shared with me" checked, the "1 filter active" banner and
its Clear-all button render, and an emptied list shows "No dashboards
found" instead of "No dashboards yet". Test pins all four behaviors
(verified red against the unfixed condition).
- general_level is no longer a dead field: the audience badge's title
tooltip now reads "{audience} · {level}" ("Everyone in org · Viewer"),
the same labels/format as ShareModal's read-only General-access
summary; falls back to the bare audience label when level is null.
Full suite: 133 suites / 1547 passed / 4 skipped (+3 new tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lets an org member paste/type many emails at once into the "Add a person" section and invite each at the selected View/Edit permission. Parses comma/space/newline-separated input into chips (invalid-format emails get an inline error chip; duplicates deduped case-insensitively), sends one POST per email in batches of 5 concurrent requests (not one big fan-out, to avoid hammering the backend/mail sender on a 30-email paste), and surfaces a "N shared · N invited · N failed" summary toast with failed chips retained for retry. Known org emails resolve to an instant active grant server-side; unknown ones get a Member invitation + pending grant — the client never pre-checks membership, it just reads the response's status. Plan deviation: the plan sketched an invite-role picker, but the backend (task 9) deliberately hardcodes the invited role to Member for this path, so this adds a plain "New people will join as Members" hint instead of a role dropdown. Extends useResourceAccess's AddGrantPayload to accept an optional `email` alongside principal_id, matching the backend's GrantCreate schema. Adds sharing:email_invite_sent analytics (count only, no emails, fired on the success path). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The format:write pre-commit hook runs `git add .`, which staged an unrelated pre-existing local font-loading change on this file alongside the sharing commit. Reverting it back out here; it stays as an uncommitted local diff, same as before this session started. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nge on pending grants
Review fixes (task 10, round 1):
1. Pasting the common mail-client format "Name <a@x.org>" previously
produced a green chip for the literal "<a@x.org>" and POSTed it —
the old EMAIL_REGEX accepted angle brackets and the backend has no
email-format validation on this path. splitEmailTokens now splits
entries on commas/semicolons/newlines and unwraps an anchored
"Name <email>" (or bare "<email>") entry to the inner address;
EMAIL_REGEX excludes <> so an unmatched-bracket leftover fails
validation as an invalid chip instead of reaching the backend.
2. The permission dropdown was dead on pending grant rows (render gate
and handler both required a resolved principal_id). Pending rows now
get the same dropdown as active ones; changing it re-POSTs via the
email path — the backend's update_or_create keyed on pending_email
updates the pending row's permission in place (verified in
sharing_actions._invite_and_create_pending_grant). Updated the old
share-modal-access.test.tsx pin ("no dropdown on pending") to the
new behavior — a deliberate spec change from review, not a silenced
failure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns, comment moderation prop (Milestone 9) Adds the frontend half of the request-access flow: lib/api.ts stamps the HTTP status onto thrown errors (lib/utils.ts:getApiErrorStatus reads it) so a 403 on the dashboard/report/alert detail fetch renders <RequestAccessScreen> (components/sharing/) instead of the generic error state — a permission picker + note, "Request sent" / "Request pending" on revisit, and a reload on the "already have access" 400. ShareModal gains an in-modal "Pending requests" section (approve with a downgrade-capped permission choice, or decline) driven by the caller's own decidable inbox. Metric/KPI have no reachable single-resource 403 seam today (list- scoped views, no by-id fetch on open) — documented rather than faked. Carries over two loose ends from earlier tasks: CommentPopover takes a canModerate prop so a resolver-edit viewer can moderate others' comments (author self-rights unchanged, Task 14's backend gate); the report page's summary comment trigger is unwrapped from the canEdit gate that silently blocked Members from commenting at all. The alerts page now reads Task 13's `?alertId=` notification deep-link, highlighting the matching row (mirroring metrics-library's existing `?highlight=` pattern) or showing the request-access screen on a 403. New hooks/api/useAccessRequests.ts is a sibling to useResourceAccess.ts — a distinct mental model (asking for / deciding on access) from grants and general access. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- useResourceAccess: transferOwnership() POSTs new_owner_orguser_id to
/api/access/{rtype}/{id}/owner/ (task-12 backend contract).
- OrgPreferences type gains allow_public_sharing/default_general_audience/
default_general_level (task-11 GET /api/orgpreferences/ additions);
reused the existing useOrgPreferences() hook rather than duplicating it.
- updateSharingPreferences() PUTs a partial body to
/api/orgpreferences/sharing/.
Foundation for the Access-Mgmt settings page and ShareModal's owner-
transfer flow (next commits).
New admin-only settings page under Settings — "Allow public sharing" switch and org-default General-access picker (audience + level), both PUT /api/orgpreferences/sharing/. Gated with RoleGuard(ADMIN_ROLES), matching how Billing/User Management gate (and the backend's direct is_admin_or_super_admin role check on this endpoint — no existing permission slug fits, per task-11's backend report). - app/settings/access-management/page.tsx — thin RoleGuard wrapper. - components/settings/access-management/AccessManagement.tsx — the three controls, reusing the existing useOrgPreferences() hook. - Nav entry added to main-layout.tsx (visibleToRoles: ADMIN_ROLES). - Analytics: settings:sharing_settings_updated event + FEATURES entry + PATHNAME_TO_FEATURE mapping.
New always-rendered OwnerSection (extracted from PeopleWithAccessSection, which is gated on capabilities.grants) so the owner row and Transfer ownership action show up even for grantless rtypes like metric/kpi — the backend's transfer endpoint supports them (task-12 backend report). - Transfer visibility mirrors the backend's require_owner_access gate: access.viewer.is_owner || an org admin role — general-access/grant- derived "edit" is deliberately not enough. - Pick (org-member combobox, same pattern as the grants add-row) -> confirm (plain-language copy stating who keeps Edit access — "you" only when the actor IS the current owner, otherwise names the actual current owner) -> POST -> revalidate + toast. Errors surface via toastError.api. - Analytics: sharing:ownership_transferred fires with entity_type only (no emails/ids).
Hides the public-link/toggle card for dashboards AND reports when the org's allow_public_sharing preference is false — same hard-hide treatment as the existing capabilities.public_link === false gate, so the frontend never presents a control the backend will 403 on the enable path (task-11 backend kill switch). Gates on `orgPreferences?.allow_public_sharing !== false` specifically (not falsiness) so an unloaded/no-row org preferences response — which mirrors the backend's own no-row-means-True default — never hides the toggle for every other resource/test that doesn't care about this setting.
…e menu
Reports list and detail pages now open the same rtype-driven ShareModal
dashboards use (entityType="report") instead of the old two-item
dropdown (report-share-menu.tsx) — grants, general access, public
toggle, and (now) owner/transfer all come for free from the shared
modal (T8/T10/T12f).
Retired (no other callers after the sweep):
- components/reports/report-share-menu.tsx
- components/reports/share-via-link-dialog.tsx (legacy ShareModal
wrapper, no entityType)
- components/reports/share-via-email-dialog.tsx (custom subject-line +
recipients dialog)
Affordance dropped, flagged rather than silently lost: the retired
email dialog let the sender customize the email SUBJECT line;
ShareModal's existing "Share via Email" card (onShareViaEmail prop,
previously wired to nothing) only takes recipients + an optional
message, no subject field. Wired reports' Share via Email onto that
existing card as-is rather than extending the shared modal's API
further. Report emails now use whatever default subject the backend's
share-email endpoint applies.
Incidental fixes made while touching these call sites (both were
dead/misleading before — reports never had their own can_share_reports
gate on the button, and the list page's per-row testid collided
across rows):
- Both report pages gated the share button on CAN_SHARE_DASHBOARDS
(copy-paste leftover) — now CAN_SHARE_REPORTS.
- List page's share button testid is now `report-share-btn-${id}`
(was a single fixed id repeated on every row).
- Removed a snapshot-viewer-page test asserting the share button hides
for a "non-creator" — the page has never had a creator check; the
old assertion only passed because of mockHasPermission state bleeding
in from the PRECEDING test in the same file (order-dependent, not a
real behavior).
Test suites migrated: reports-page.test.tsx (list),
snapshot-viewer-page.test.tsx + [snapshotId]/__tests__/page.test.tsx
(detail) — both now stub ShareModal and assert it opens with
entityType="report".
…ion fn Frontend contract for task-17f: BulkItemRef/BulkAction/BulkAccessRequest/ BulkAccessResponse types plus a bulkApplyAccess() mutation function in useResourceAccess.ts, unwrapping the standard success envelope like the other mutation fns in this file.
Backs the bulk-share selection bars on Dashboards/Reports/Alerts (task-17f). Selection persists across pagination and caps at 100 (BULK_MAX_ITEMS) so a downstream bulk POST can never exceed the backend's item limit; a targeted remove() lets callers deselect just the ids a bulk apply actually committed.
…blic link Shared dialog for the bulk-selection bars (task-17f milestone 10). One POST per action via bulkApplyAccess: add_grant, set_general (with the aggregated narrow-confirm step — mirrors ShareModal's GeneralAccessSection concept across multiple resources, re-sending a flat remove_grant_ids list), and toggle_public (omitted entirely for alerts via allowPublicLink=false). Plain-language SKIP_REASON_COPY maps the backend's stable reason codes to NGO-readable text, grouped by reason with counts in the result panel. Analytics (sharing:bulk_applied) fires once per terminal response — after the set_general re-send when a narrow-confirm round-trip happened, using that call's own counts.
… (task-17f) Checkbox column (gated on can_share_dashboards, matching the row Share button's existing gate) + a bulk bar that appears once >=1 row is selected. Selection persists across pagination and caps at 100 via useMultiSelect; "Select all" fills the current page only. Applied ids are deselected after a bulk apply (skipped/pending-confirmation ids stay selected), and the list revalidates so audience badges reflect the change.
…ask-17f) Same wiring as the Dashboards list: checkbox column gated on can_share_reports, bulk bar once >=1 selected, selection persists across pagination (capped at 100), applied ids deselect after apply while skipped/pending-confirmation ids stay selected, list revalidates.
… bar Cross-task gap closure (task-17f scope item 8): alerts were fully shareable via the plan but no surface mounted ShareModal. AlertsTable gains an optional canShare/onShare pair (per-row Share button, gated on can_share_alerts) and a bulk-select checkbox column, both default-off so existing callers are unaffected. AlertsPage wires the row Share button to the shared ShareModal (entityType="alert" — capability flags already omit the public-link section, no alert conditionals added to the modal) and adds the same bulk-selection bar + BulkShareDialog pattern as Dashboards/Reports, with allowPublicLink=false (alert is public_link=False; showing the control would be dead since the backend skips every item). getAlertShareStatus/updateAlertSharing are local stubs — alerts have no public-link endpoint, but ShareModal calls fetchShareStatus unconditionally on open regardless of entityType, so the required props still need a same-shape resolver even though the section they'd back never renders.
Dashboards list — the bar's "(maximum 100 reached)" hint and a checkbox disabling once selectedIds.size hits MAX_BULK_SELECTION. Mocks useMultiSelect (delegating to the real implementation everywhere else in the file) rather than driving 100 real clicks or the pagination Select (flaky with Radix in jsdom) — cap ENFORCEMENT itself stays pinned in hooks/__tests__/useMultiSelect.test.ts.
…sable, imports - Add the brief's explicit test: real ShareModal with entityType="alert" and capabilities.public_link=false renders no share-toggle while People/General sections still show (no alert-specific conditional in the modal — same capability-gate mechanism as every other rtype). - AlertsTable row checkbox now disables at the 100-item cap, matching Dashboards/Reports (previously enforced by the hook with no visual affordance on alerts). - Merge the split lucide-react import in app/alerts/page.tsx.
…page-only uncheck
Closes three task-17f review findings on bulk-share:
- Bulk bar denominator was page-local ("N of {visible}"), so it kept saying
"5 of 5 selected" with every visible checkbox unchecked once selection
carried onto another page. Now shows the true cross-page total ("5
selected") plus an explicit "N on other pages" hint when some selected
ids aren't on the current page. Applied to Dashboards, Reports, Alerts.
- BulkShareDialog derived `items` live from the parent's shrinking
selection, so a second action after the first apply's onApplied-deselect
silently narrowed to the leftover subset, and an all-applied first action
emptied `items` entirely, 400ing the next POST. The dialog now snapshots
items on open and acts on that snapshot for its whole open lifecycle;
action buttons are also guarded against an empty snapshot.
- Dashboards/Reports header "select all" checkbox fell back to clear() on
uncheck, wiping the whole cross-page selection despite its own
"on this page" aria-label. Now calls deselectPage(visibleIds), matching
Alerts' existing (correct) behavior.
Covering suites: hooks/useMultiSelect, bulk-share-dialog,
dashboard-bulk-share, dashboard-list-v2, reports-bulk-share, reports-page,
alerts page-bulk-share, alerts page — 78 tests passing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s recovery to 400 - Extract lib/access-labels.ts as the single source of the audience/level vocabulary (ShareModal's wording, which users see most) and consume it from AccessManagement, ShareModal, BulkShareDialog, and the dashboard audience badge, closing the review's M1/M2 findings — an admin setting the org default to "Private" previously never saw that word again since every per-resource surface called the same enum value "Restricted". - RequestAccessScreen's already-have-access reload now also requires getApiErrorStatus(error) === 400 alongside the message match (M3), so a future wording change on an unrelated error can't trigger a reload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prior commit's pre-commit hook (npm run format:write -> git add .) swept in this untracked local dev-env file (non-secret localhost URLs). Untrack it and gitignore .env.local so it can't happen again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ople/Groups tables (Phase A: A1, A2) - UsersTable gains a Created By column (avatar initial + inviter email, dash when the user wasn't invited); actions were already a kebab - GroupsTable Members cell becomes an overlapping avatar stack from member_preview with a teal +N overflow and an aria-label carrying the full count; adds Created By and Created (MMM d, yyyy) columns and keeps Shared with - shared AvatarInitial/CreatedByCell helpers under components/settings/ - OrgUser.invited_by and UserGroup.member_preview types Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ransfer copy to Figma (Phase A: A3, A4)
- Bulk-selection bar (dashboards/reports/alerts) restyled to design
frame 1992:2488: inline "{N} selected · Select All · Clear" on the
left, Share on the right; keeps the off-page count, 100-cap and all
handlers/testids
- Public sharing card: title "Public sharing" + subtitle "Anyone with
the link can view" (frames 1426:2063/2115); copy-link and kill-switch
behavior unchanged
- RequestAccessScreen (frame 1184:6222): heading "Request access to
this {rtype}", supporting line naming the resource when available, and
a "You're logged in as {email}" footer; keeps the View/Edit choice and
note field
- In-modal "{N} users are requesting access" header for 2+ pending
requests (frame 1353:14586); singular unchanged
- Transfer-ownership confirm copy aligned toward frame 1184:6198 but
kept truthful (no "reclaim anytime"): "Ownership of this {rtype}
transfers to {owner}. They can then delete it or transfer it again.
You keep Edit access."
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e B)
One Settings → Access page at /settings/access with PEOPLE | GROUPS | ROLES
tabs, replacing the three sidebar pages (user-management, groups,
access-management), per Figma frames 1335:2070 / 1184:3242.
- AccessPage owns the header (title, subtitle, tab-swapped primary button:
INVITE USER / CREATE GROUP / none) and lifts the invite/create dialog state
into the panels via controlled props.
- People and Roles tabs render only for ADMIN_ROLES; Groups for all page
viewers (DATA_SECTION_ROLES) — same semantics as the old per-page guards.
Hidden/unknown ?tab deep links fall back to the first visible tab.
- Old routes redirect: user-management → ?tab=people, groups → ?tab=groups,
access-management → ?tab=roles.
- Sidebar: three entries replaced by one Access entry (ShieldCheck,
DATA_SECTION_ROLES).
- People panel keeps its Users / Pending Invitations tabs as a segmented
control so they don't clash with the page-level underline tabs.
- Roles panel gains the design's role descriptions as helper text; the
permission matrix stays unbuilt (blocked on product decision).
- Analytics: new settings_access feature (+{ tab } on change); legacy route
prefixes removed from PATHNAME_TO_FEATURE.
- Tests: AccessPage tab visibility/button-swap/deep-link suite, page
RoleGuard suite, legacy-redirect suite; UserGroups + main-layout tests
updated for the new shape.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…SHARE commit (Phase C: C1, C2 + C3 wiring) Replaces the share modal's three add pickers (org-member combobox, invite-by-email panel, group combobox) with ONE search input that mixes org users (role tag), groups (Group tag), and free-typed/pasted emails. Picked entries stage as rows (permission pill + remove); the modal footer gains a SHARE button that applies all staged rows in one action — user and group rows via the single-grant POST, email rows via the email-grants path in batches of 5 with the chosen invite_role (Member default; Analyst/Admin options render for admins only). Failed rows stay staged with their error for retry; duplicates and already-granted principals are marked, never re-sent; invalid emails are rejected inline exactly as before (same EMAIL_REGEX, Name <email> unwrapping, paste splitting). Staged state is scratch — closing the modal discards it. Live-editing of existing grant rows (permission change / remove) is unchanged; only ADDING is staged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ubmit guard, residual-text flush F1 (Important): restore browse-without-typing. Focusing the empty unified search now opens the scrollable dropdown listing all groups first, then all org members (Figma's share-list pattern), with the same dup handling as the typed path (staged/granted entries disabled). Focus/blur live on the wrapper with a relatedTarget guard so dropdown picks and Tab+Enter keyboard navigation keep working. F2 (Minor): commit() now trips a ref-based in-flight guard, so a rapid double invoke (double click / Enter+click) sends exactly one batch instead of relying on the disabled-button re-render. F3 (Minor): typed-but-unstaged text is never silently discarded. SHARE first flushes residual input through the existing tokenize/validate/stage path — valid tokens join the batch, invalid ones stage as inline-error rows and abort the commit. Email-looking residual text also stages on blur (parity with the old email panel). hasPendingInput keeps SHARE clickable while text sits unstaged in the box. Also splits buildEmailEntries and SearchResultsDropdown out of ShareAddPeopleSearch to stay under the 300-line component rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… commit, invite-role picker)
…eal inviter attribution)
- Requests header: collapsible "N users are requesting access" bar with
chevron at the very top (2+ requests); rows read "{email} wants to
edit" with the permission word bold (inline lowercase dropdown when a
downgrade is offered), plain-text Deny + filled Approve; removed the
old "Pending requests" card/sub-header. Approve/decline behavior and
analytics unchanged.
- Search block: "Search for people, group or add emails" label above
the input; placeholder and browse/typeahead dropdown untouched.
- Staged rows: scroll internally when long (max-h), borderless View/Edit
control; amber unknown-email treatment stays.
- People with access: every row gets a role tag (Admin/Analyst/Member)
joined client-side from the org-users list; owner row is icon · email ·
role tag · plain-text "Owner" (no pill, no border, no control); rows
get the circular avatar icon; list scrolls internally when long.
- Permission control: borderless text + chevron ("View ⌄", flips open)
on staged AND granted rows — still an accessible Radix Select, shared
as PermissionSelect in principal-search-shared.tsx.
- Transfer ownership: reconciled with 'Transfer ownership.jpg' — a
"Transfer Ownership" item inside each eligible grantee's permission
dropdown (targets that person, amber confirm below the list). The
grantless-rtype (metric/kpi) standalone button + combobox flow is kept
as-is since no frame covers those.
- General access, public sharing card, CANCEL/SHARE footer: unchanged.
Tests: requests suite covers collapse/expand + Deny/Approve order;
access suite covers role tags, plain-text owner, borderless control;
owner-transfer suite rewritten for the per-row entry point (grantless
combobox path still covered). 359 tests / 31 suites green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n triggers Reviewer finding (WCAG 2.4.7): focus-visible:ring-0 left no focus indicator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ags, plain-text Owner, borderless View/Edit controls, transfer in grantee dropdown
Live-browser check at 1512px found the share modal clipping the request rows' Approve buttons and the General-access selects, with the second request row's note forcing a mid-sentence wrap onto three ragged lines. - Widen DialogContent from sm:max-w-md (448px) to sm:max-w-3xl (768px), matching the RBAC design frames (~850px dialog) and the existing wide-dialog convention (AlertWizardModal, SourceForm). - Give the owner row, grant rows, and pending-request rows real min-w-0 + truncate on their name/email children so the fixed-width controls (Deny/Approve, permission select, remove ✕) never get pushed off the edge. - Restructure the pending-request row into a single non-wrapping flex line: email and permission stay fully visible (flex-shrink-0), while the free-text note is the flex-basis-0 "expendable" child that truncates first when space is tight, instead of wrapping the whole row. Verified live at 1512px, 900px, and 700px viewports (dev server on :3002) — no clipping, single-line rows. Full share-modal jest suite (93 tests) passes.
… exact-match RBAC frames - Remove per-resource General access entirely (Analysts/Members Selects, narrowing Keep/Remove confirm panel, capability wiring) -- design decision, it appears in no RBAC frame. Per-resource General access now has no single-resource UI; settable only via the bulk action or the API. Bulk dialog, dashboard-list badges, and Roles-tab org defaults are untouched. - Flatten People-with-access from a bordered Card to plain content, matching every frame -- only Public sharing keeps its own rounded bordered card. - Give every staged add-people row (not just unknown-email ones) the full-width light-gray rounded pill the frames show, distinct from the plain committed People-with-access rows below. - Add the full-bleed header/footer hairlines every frame shows, independent of card wrapping. - Prune the matching General-access test coverage and dead share-general-section assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uted grant-row X Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two live bugs from 2026-07-16 manual testing of the Create-group modal:
1. Dropdown couldn't be dismissed after staging an unknown email: it
stayed open over the invite-role block and the dialog footer (Cancel
was unclickable — clicks landed on dropdown rows), and Escape closed
the WHOLE dialog (Radix listens on the document in the capture phase,
so no input-level handler can intercept it), losing all staged work.
Now: staging an email collapses the dropdown (the address intent is
complete; typing or clicking the input reopens it), and Escape is
layered via DialogContent.onEscapeKeyDown + an imperative
closeDropdownIfOpen handle — first Escape closes only the dropdown,
second closes the dialog.
2. Re-selecting an already-added principal gave no feedback: the
"Invite {email}" row never checked stagedEmails (always actionable),
and a repeat Enter was silently swallowed by dedupeStage. Now the
Invite row renders disabled with "Added" like the user/group rows,
and repeat Enter (typed email, or single-match search) shows an
inline hint — "X is already added" / "X is already in this group"
(edit mode) — never a duplicate chip, never a silent no-op. Also
stops staging re-add chips for existing members typed by email in
edit mode (staging's dedupe only knew about chips, not members).
Shared share-modal pieces untouched; its suites stay green (151 tests
across the groups + share-modal suites).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ShareModal's add-people dropdown had the same Escape gap the group create dialog just fixed: Radix Dialog's capture-phase Escape closed the whole dialog (discarding staged rows) even when the browse/ typeahead dropdown was open. ShareAddPeopleSearch now exposes a closeDropdownIfOpen() handle (mirrors GroupMemberSearchHandle); the dialog's onEscapeKeyDown asks it first, so a first Escape closes only the dropdown and a second closes the dialog. Also makes typing/ clicking the search input reopen a dropdown an Escape just closed, matching the group typeahead's recovery behavior. Added regression tests mirroring GroupFormDialog's Escape-layering suite (dialog survives + staged rows intact on first Escape, closes on second) and manually verified against the running dev server.
…ber staging partitionAgainstStaged only checked entries against the pre-call staged snapshot, so pasting the same new email twice in one paste let both copies through as "fresh" -- dedupeStage then silently dropped the second, with no dup hint firing. Now tracks keys/emails seen within the loop so intra-batch repeats land in `dupes` too. Also makes duplicateNotice's multi-dupe branch distinguish "already added" (staged this session) from "already in this group" (existing member in edit mode), matching the singular branch's existing distinction instead of always saying "Already added". Adds regression tests: mixed paste (fresh + already-staged email together), same-email-twice-in-one-paste, and a chips-retained assertion on the first Escape in the dropdown-layering test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors the alerts list pattern: a per-row/per-card Share action opens ShareModal (entityType metric/kpi), and a multi-select bulk bar opens BulkShareDialog (share + General access, no public-link action). This is General access's first UI for metric/kpi, closing a hole where it was previously API-only. Metrics list gets a checkbox column; the KPI card grid gets a per-card checkbox overlay (no row chrome to add a column to) plus a Share item in each card's existing menu. Adds member-block presentation in the shared share-modal-staging.tsx, gated to metric/kpi only (dashboard/report/alert unaffected, pinned by a parallel regression test): the add-people typeahead hides Member-role users, the invite-role picker never offers Member and defaults to Analyst, and non-admins see copy explaining new users can't be invited directly yet instead of the normal "invited as member" promise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Registers 'chart' as a ShareableResourceType and mounts the sharing surface everywhere the other rtypes have it, mirroring M5's KPI/Metric wiring exactly: - Type plumbing: ShareableResourceType gains 'chart'; PERMISSIONS gains CAN_SHARE_CHARTS (can_share_charts, mirrors the backend seed); SHARE_PERMISSION_BY_RTYPE and RESOURCE_NOUNS cover chart. - Charts list (app/charts/page.tsx): row Share action + a SEPARATE bulk-selection bar/checkbox column (useMultiSelect) driving ShareModal/BulkShareDialog, allowPublicLink=false. No access badge — the list's ChartResponse payload has no analyst_level/member_level fields yet (backend M1 didn't add them to the list schema); per the brief, this is left unbadged rather than hacked with a per-row fetch. - Chart detail (ChartDetailClient.tsx): header Share button, and a 403 from the standalone chart fetch now renders RequestAccessScreen (rtype="chart") instead of the generic error state. - Member-block states: 'chart' joins MEMBER_GRANTS_DEFERRED_RTYPES in share-modal-staging.tsx (typeahead hides Members, invite-role picker drops Member) — mirrors metric/kpi from M5. - RequestAccessScreen: a permanent-block 400 (e.g. a Member requesting a chart, which backend rejects with "...request access to the dashboard instead") now renders inline as an answer, not a toast — generalized for any rtype hitting this path, not chart-specific. Excludes the embed/broadening warning modals and chart-selector-modal.tsx (M3b, waits on the backend coverage endpoint) and all backend files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… 8 implicit-any tsc errors Reviewer finding on M3a: the report's tsc-parity claim was wrong; these test files introduced TS7023/TS7018 errors. Now zero errors in the new files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fordance, 403→request screen)
…ct adaptations
The two un-archived design frames land as reusable, capability-driven confirm
dialogs, and the webapp adapts to M2's three contract changes:
- addGrant now returns the GrantCreateResponse envelope (grant at .grant);
a requires_confirmation response from a dashboard grant-add renders the
broadening confirm instead of committing — SHARE-commit batch (one
aggregated prompt, per-grant extend subsets on re-send), permission
changes, and the bulk dialog (add_grant / set_general raise /
toggle_public enable, ONE prompt per selection).
- Public-toggle enable handles requires_confirmation: YES re-sends with
proceed, CANCEL leaves the switch visually off (nothing flipped).
- BroadeningConfirmDialog ("resource sharing- warning modal.jpg"): charts
named, CANCEL default (red outline, focused), teal YES = extend-all
(extend_chart_ids of extendable+editable charts) + proceed; non-extendable
exposure copy says charts stay visible inline regardless.
- EmbedCoverageDialog ("Analyst-warning on adding charts.jpg"): chart picker
pre-flights GET /api/dashboards/{id}/chart-coverage/?chart_id= — covered
embeds silently; gap+Edit offers extend; view-only gets the
request-Edit/ask-owner prompt (files a chart Edit access request);
Cancel aborts the pick. Fail-open on pre-flight errors (server re-validates).
- Dashboard autosave: picker decisions ride the PUT as
extend_chart_ids/proceed (embed-coverage-save.ts helpers); a 409
EmbedCoverageConfirmation surfaces as toast + header error + the same
dialog (YES re-saves with the confirmation; a cancelled prompt doesn't
reopen every autosave tick). apiFetch now stamps the parsed error body
(getApiErrorBody) so the 409 payload is reachable.
- Member CSV exports: dashboard-tile download-csv carries
chart_id/dashboard_id; charts-list table export carries chart_id
(routes the M2 warehouse gate through the per-chart resolver).
jest: 172 suites, 1879 passed / 4 skipped, 0 failed. tsc/eslint: no new
errors vs baseline (live pass against the M2 backend is the coordinator's
post-merge step — sandbox deliberately serves pre-M2 code).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comments-only pass before final review: delete plan/milestone/design-frame references and reviewer-facing narration; compress load-bearing warnings (fail-open pre-flights, requires-confirmation contracts, Escape layering, dup-guard invariants) to 1-2 plain-English lines. No code, copy, test-name, or type changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
user_permission from the detail GET widens canEdit: a Member holding an edit grant on one dashboard gets the Edit button and the editor for that dashboard only. Role-slug holders are unaffected; the denied screen now waits for the fetch instead of firing before it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The complete Resource Sharing frontend, aligned frame-by-frame with the Dalgo 2.0 Figma designs.
Surfaces
Ships together with the paired backend PR (DalgoT4D/DDP_backend#1433). The backend makes three deliberate contract changes (grant-create response shape,
requires_confirmationon public toggle, dashboard tile validation) that this webapp adapts to — deploy both together.Testing
Full jest suite green at tip (172 suites / 1879 tests; the one intermittent failure under parallel load is a pre-existing
AlertWizardModaltiming flake untouched by this branch). Every milestone independently reviewed (spec + quality); all user-flagged frames browser-verified against the build with Playwright. Comments swept to repo convention (plain 1–2 line constraint notes only). Specs/plans/reports:dalgo-core/features/access-control/resourcesharing/.🤖 Generated with Claude Code