Skip to content

Super-admin org CRUD review fixes + name-length validation#430

Merged
jhodapp merged 5 commits into
mainfrom
jim/admin-org-review-fixes
Jul 2, 2026
Merged

Super-admin org CRUD review fixes + name-length validation#430
jhodapp merged 5 commits into
mainfrom
jim/admin-org-review-fixes

Conversation

@jhodapp

@jhodapp jhodapp commented Jul 2, 2026

Copy link
Copy Markdown
Member

Follow-up fixes to the super-admin org CRUD feature (#426), which merged before these review-round changes were committed. All wire-compatible with AdminOrganizationCRUD v2; the name-length handling matches the backend's admin_org_name_validation_and_delete_fk_hardening hardening (PR refactor-platform-rs#366).

Fixes

1. Reach /admin for super admins with no org selected
The /admin Organizations sidebar link was nested under an org-scoped role gate (isAdminOrSuperAdmin requires an org context), so a system SuperAdmin who belongs to no org couldn't reach it. The gate is lifted to the org-independent altitude; the org-scoped Members sub-item is separately gated on a selected org. isSuperAdmin's null check is loosened to == null for consistency with isOrganizationArchived.

2. Invalidate all org caches on archive/unarchive
Archive/unarchive were raw sub-action POSTs that only revalidated the current ?status= list, leaving the sibling active/archived/all caches stale. They now route through a new useOrganizationArchiveMutation hook that fires a global cache invalidation (shared invalidateEntityCache predicate extracted from useEntityMutation).

3. Degrade the archived byline gracefully
The "Archived {date} by {name}" byline could stick on "…" when the archived_by GET /users/{id} lookup errored. It now shows "…" only while in flight and degrades to "a former admin" on error or absent archived_by. Also memoizes the org list sort.

4. Validate organization name length (422 + client pre-check)
Handles the backend's new 422 validation_error on org create/rename (empty or >255-char name) in the same inline name-field slot as the 409 organization_name_taken, scoped to the org form so the shared validation_error discriminator isn't treated globally. Adds a client-side pre-check (non-empty, ≤255) to avoid the round-trip; the server 422 stays source of truth.

Verification

  • tsc --noEmit clean; 1620 unit tests green.
  • Live-verified via Playwright as SuperAdmin against the rebuilt backend (PR Enhancement: SSE reconnection with exponential backoff #366 HEAD 27152e88): archive/unarchive tab-cache invalidation both directions; byline resolves "by Admin" and degrades correctly; org name 422 (empty + >255) both inline with no toast and no round-trip; valid create/edit/delete unaffected. Role gating re-confirmed for org-admin and regular user (both 404 on /admin).

jhodapp added 4 commits July 2, 2026 11:14
The /admin Organizations sidebar link was nested under an org-scoped role
gate (isAdminOrSuperAdmin requires hasAccess, false when no org is selected),
so a system SuperAdmin who belongs to no org couldn't reach it. Lift the gate
to the org-independent altitude and gate only the org-scoped Members sub-item
on a selected org. Also loosen isSuperAdmin's organization_id check to == null
for consistency with isOrganizationArchived.
Archive/unarchive were raw sub-action POSTs that only revalidated the current
?status= list, leaving the sibling active/archived/all caches stale until an
unrelated revalidation. Route them through a new useOrganizationArchiveMutation
hook that fires a global cache invalidation, extracting the shared
invalidate-by-baseUrl predicate out of useEntityMutation.
The "Archived {date} by {name}" byline could stick on an ellipsis when the
archived_by GET /users/{id} lookup errored. Show the ellipsis only while the
lookup is in flight and degrade to "a former admin" on error or absent
archived_by. Also memoize the org list sort.
Handle the backend's new 422 validation_error on org create/rename (empty or
>255-char name): organizationNameInvalidMessage() surfaces it in the same
inline name-field slot as the 409 organization_name_taken, scoped to the org
form so the shared validation_error discriminator isn't treated globally.
Add validateOrganizationName() as a client-side pre-check (non-empty, <=255)
to avoid the round-trip; the server 422 stays source of truth for edge cases.
@jhodapp jhodapp self-assigned this Jul 2, 2026
@jhodapp jhodapp added the enhancement Improves existing functionality or feature label Jul 2, 2026
@jhodapp jhodapp added this to the 1.0.0-beta3 milestone Jul 2, 2026
@jhodapp jhodapp marked this pull request as ready for review July 2, 2026 16:17
@jhodapp jhodapp changed the title fix(admin): super-admin org CRUD review fixes + name-length validation Super-admin org CRUD review fixes + name-length validation Jul 2, 2026
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR delivers follow-up hardening to the super-admin org CRUD feature, addressing four distinct gaps: sidebar access for org-less SuperAdmins, stale cache after archive/unarchive sub-action POSTs, a byline that could permanently show "…" on archiver-lookup failure, and missing client-side/server-side 422 handling for org-name length violations. The previous P1 (client validating the trimmed length but sending the raw padded string) and the P2 (orphaned JSDoc on useEntityMutation) that were flagged in the earlier review round are both resolved here.

  • Cache invalidation: Extracts invalidateEntityCache into a standalone utility inside EntityApi and routes archive/unarchive through a new useOrganizationArchiveMutation hook that calls it globally, ensuring all active/archived/all SWR cache slices are cleared after a status toggle.
  • Name validation: Adds validateOrganizationName for a round-trip-free client pre-check, organizationNameInvalidMessage for the 422 server fallback (with an explicit status === 422 guard so the shared validation_error discriminator isn't treated globally), and trims formData.name before both validation and the API call.
  • Sidebar gate + byline: Lifts the admin-section collapsible gate to isSuperAdmin || isAdminOrSuperAdmin(currentUserRoleState) so org-less SuperAdmins can reach /admin; separately gates the Members link on currentOrganizationId. Byline now shows "…" only during flight and degrades to "a former admin" on error or absent archived_by.

Confidence Score: 5/5

Safe to merge — all four fix areas are correct and well-tested; previous round's blocking trim-mismatch issue is resolved.

The trim-before-validate-and-send fix is confirmed in the form dialog and verified by a new test. The cache invalidation path correctly fires in a finally-guarded hook, covers all three status-filter SWR keys, and degrades cleanly on error. The sidebar gate change is minimal and the byline degradation logic is straightforward. No regressions detected across any of the changed paths.

No files require special attention.

Important Files Changed

Filename Overview
src/lib/api/organizations.ts Adds useOrganizationArchiveMutation hook with correct isLoading lifecycle (finally block), global cache invalidation via invalidateEntityCache, and error propagation to caller.
src/lib/api/entity-api.ts Extracts invalidateEntityCache as a reusable namespace-level utility; useEntityMutation now delegates to it. JSDoc ordering is correct (each block directly precedes its declaration).
src/lib/api/organization-errors.ts Adds organizationNameInvalidMessage (422 with explicit status check to avoid shared-discriminator leakage) and validateOrganizationName (trims before measuring); both well-tested.
src/components/ui/admin/organization-form-dialog.tsx Trims name before validation and before the API call (fixes the previous P1); inline error now handles both 409 name-taken and 422 validation_error without a toast.
src/components/ui/app-sidebar.tsx Gate lifted to isSuperAdmin
src/components/ui/admin/organization-archived-byline.tsx Byline degrades correctly: '…' only while in-flight, 'a former admin' on error or absent archived_by; useUser('') correctly skips fetch via falsy-URL guard.
src/components/ui/admin/organization-row.tsx Replaced local setIsArchiving state with hook's isLoading; Archive/Unarchive menu item correctly disabled during in-flight operation; error toast retained in catch.
src/types/user.ts Loosened isSuperAdmin's organization_id check to == null (tolerates omitted/undefined field), consistent with isOrganizationArchived.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OrganizationRow] -->|uses| B[useOrganizationArchiveMutation]
    B -->|calls| C[OrganizationApi.archive / unarchive]
    C -->|POST /organizations/:id/archive| D[Backend]
    B -->|on success| E[EntityApi.invalidateEntityCache]
    E -->|mutate predicate| F[SWR Global Cache]
    F -->|revalidates all keys containing ORGANIZATIONS_BASEURL| G[useAllOrganizations active/archived/all]

    H[OrganizationFormDialog] -->|handleSubmit| I{client pre-check validateOrganizationName}
    I -->|invalid| J[setNameError - no round-trip]
    I -->|valid| K[useOrganizationMutation.create/update]
    K -->|success| L[toast.success + close dialog]
    K -->|409 name taken| M[setNameError org_name_taken]
    K -->|422 validation_error| N[organizationNameInvalidMessage setNameError]
    K -->|other error| O[toast.error]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[OrganizationRow] -->|uses| B[useOrganizationArchiveMutation]
    B -->|calls| C[OrganizationApi.archive / unarchive]
    C -->|POST /organizations/:id/archive| D[Backend]
    B -->|on success| E[EntityApi.invalidateEntityCache]
    E -->|mutate predicate| F[SWR Global Cache]
    F -->|revalidates all keys containing ORGANIZATIONS_BASEURL| G[useAllOrganizations active/archived/all]

    H[OrganizationFormDialog] -->|handleSubmit| I{client pre-check validateOrganizationName}
    I -->|invalid| J[setNameError - no round-trip]
    I -->|valid| K[useOrganizationMutation.create/update]
    K -->|success| L[toast.success + close dialog]
    K -->|409 name taken| M[setNameError org_name_taken]
    K -->|422 validation_error| N[organizationNameInvalidMessage setNameError]
    K -->|other error| O[toast.error]
Loading

Reviews (2): Last reviewed commit: "fix(admin): send trimmed org name; resto..." | Re-trigger Greptile

Address Greptile review on #430:
- organization-form-dialog: trim the name before both the client pre-check and
  the request payload, so a whitespace-padded value can't pass validation yet
  differ from what hits the wire (and the stored org name has no stray padding).
- entity-api: move the useEntityMutation JSDoc back onto its function; it was
  left orphaned above the extracted invalidateEntityCache helper.
@jhodapp jhodapp merged commit badfb05 into main Jul 2, 2026
8 checks passed
@jhodapp jhodapp deleted the jim/admin-org-review-fixes branch July 2, 2026 16:51
@github-project-automation github-project-automation Bot moved this from Review to ✅ Done in Refactor Coaching Platform Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improves existing functionality or feature

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant