14 KiB
GFIBER-683 — Allow admins to correct offer data (accepted/rejected override + history)
Context
Jira GFIBER-683: "Agents in the pilot group have submitted incorrect upsells that compromised reporting metrics calculation. We need a mechanism to allow admins to validate, edit and correct these wrong entries."
Concretely: at the end of the Upsell Evaluator flow, an agent records whether the customer Accepted or Rejected the offer (upsell_tickets.state, mapped from the enum: accepted / declined_hard in practice — the only two "declined" the current UI can produce). Agents sometimes pick the wrong one by mistake. Since every dashboard KPI, rate tile, and export re-reads state live on every request, one wrong pick silently skews reporting metrics with no way to fix it today.
A detailed design already exists in Docmost (space GFiber → page Upsell_State_Override_—Plan_Historial_de_Cambios(Accepted-Rejected), ES/EN subpages, written 2026-07-09) and was refined further in this conversation against the current state of the repo. This plan supersedes the Docmost draft on two points where the repo/requirements moved since it was written (see below); the rest of the Docmost design (migration, RPC, RLS, server actions, tests) still holds and an Explore agent re-verified every file path/helper name against today's code.
What changed vs. the Docmost draft, confirmed with the user in this session:
- Table scope widened. Today
app/(protected)/dashboard/page.tsxfetches onlystate = "accepted"tickets for the "Offer Verification" table (ValidationsCard→AcceptedTicketsVerification). An agent who mis-marked an accepted offer as Rejected produces adeclined_hardticket that never appears in this table — there'd be nothing to click "History"/override on. Fix: broaden the query tostate IN ('accepted', 'declined_hard')— the only two states this feature's toggle ever produces or corrects. Not all 6 enum values —no_pitch,declined_soft,no_offer,in_progressstay out of this table's scope. - Menu design simplified. The Docmost draft split this into two menu items — "Verify" (existing modal + a new embedded "Override Outcome" section) and "History" (separate read-only modal). The user's clarified intent is simpler: leave the existing verification modal completely untouched, and add one new second menu item that opens a single new modal combining both the Accepted⇄Rejected override control and the change history in one view.
no_pitchstays excluded from reclassification, per the user — confirmed as-is from the Docmost draft.
Design
1. Migration — new audit table + atomic RPC
New file supabase/migrations/<timestamp>_add_upsell_state_changes.sql (timestamp must sort after the latest existing migration, 20260714120000_add_sales_coach_notes_to_upsell_tickets.sql):
- Table
upsell_state_changes:id uuid pk,ticket_id uuid references upsell_tickets(id) on delete cascade,changed_by uuid references profiles(id) on delete set null,changed_at timestamptz default now(),old_state upsell_state not null,new_state upsell_state not null,reason text. Index on(ticket_id, changed_at desc). - Modeled directly on the existing audit-table precedent
supabase/migrations/20260612120000_create_eligibility_checks.sql, but no INSERT/UPDATE/DELETE policy for any client role at all (unlikeeligibility_checks, which lets any authenticated user insert) — every write goes through the RPC below. RLS:selectpolicy gated topublic.current_profile_role() in ('admin','owner')(reuse the existing helper fromsupabase/migrations/20260707130000_fix_profiles_update_policy_recursion.sql:29-42— do not write a new one). - RPC
change_upsell_ticket_state(p_ticket_id uuid, p_new_state upsell_state, p_reason text default null) returns upsell_tickets,security definer,set search_path = '': checks caller role is admin/owner viaauth.uid()→profiles.role(orcurrent_profile_role()), checksp_new_state in ('accepted','declined_hard'), row-locks the ticket (for update), checks its currentstateis one of the terminal states('accepted','declined_hard','declined_soft','no_offer')(excludesin_progressandno_pitch), checks it isn't already inp_new_state, then in one transaction:update upsell_tickets set state = p_new_state, initiation_type = null where id = ...andinsert into upsell_state_changes (...).revoke ... from public; grant execute ... to authenticated. - Critical gotcha to get right: this RPC must be called via the session-scoped client (
createClient()fromutils/supabase/server.ts), notcreateAdminClient()(utils/supabase/admin.ts) — every other action indashboard/actions.tstoday usescreateAdminClient()after an app-layerrequireRolecheck, so this is a real deviation; call it out in the PR description/code review since it's easy to copy-paste the wrong client and haveauth.uid()silently resolve to null inside the function. - Regenerate
utils/supabase/database.types.tsafter applying the migration to DEV.
2. Server Actions — app/(protected)/dashboard/actions.ts
Add two new actions, following the exact import/helper pattern already used in this file (getCurrentUserWithProfile from utils/supabase/get-current-user.ts, requireRole/RoleError from utils/roles.ts):
changeTicketState(ticketId, newState: "accepted" | "declined_hard", reason?): app-layerrequireRole(profile, "admin", "owner")as defense-in-depth, thencreateClient().rpc("change_upsell_ticket_state", {...}), surface any raised exception as{ error },revalidatePath("/dashboard")on success.getTicketStateHistory(ticketId): same role gate, then a plain read viacreateAdminClient()(no RLS-sensitive write here) joiningprofiles!changed_by(display_name, email), orderedchanged_at desc.
3. Widen the verification table's query — app/(protected)/dashboard/page.tsx
- Change the
acceptedTicketsResultquery (~line 128) from.eq("state", "accepted")to.in("state", ["accepted", "declined_hard"]), and addstateto the.select(...)column list. AcceptedTickettype (components/dashboard/AcceptedTicketsVerification.tsx:17-30) gets a newstate: UpsellStatefield.- No other change needed here:
ValidationsCard's "Information by Agent" tab (buildValidationSummaryByAgent) derives its pending/valid/rejected tally fromfull_flow_completed/initiation_type/objection_handled_correctlyonly, not fromstate, so it picks up the wider ticket set automatically and correctly.
4. UI — kebab becomes a 2-item menu; second item opens one combined modal
New DS component KebabMenu (packages/design-system/src/components/KebabMenu/): wraps the existing IconButton (⋯ trigger) and portal-renders (createPortal to document.body, positioned from the trigger's getBoundingClientRect() — same technique already used by AttendanceQuickModal/TicketVerificationModal, needed because this table can scroll horizontally and a non-portaled menu risks clipping, the same overflow-bug class already hit twice in this repo for the Attendance grid). Closes on Escape/outside-click/item-select. role="menu" / role="menuitem", arrow-key navigation. Props: { ariaLabel: string; items: { label: string; onSelect: () => void }[] }. Ships with .stories.tsx, .test.tsx, .test.cy.tsx per this repo's DS-first convention.
components/dashboard/AcceptedTicketsVerification.tsx Actions column (currently a single IconButton at lines 179-194 calling setManagedTicket(ticket)) becomes:
<KebabMenu
ariaLabel={`Actions for ${agentName}'s ticket`}
items={[
{ label: "Verify", onSelect: () => setManagedTicket(ticket) },
{ label: "Change Status", onSelect: () => setStateModalTicket(ticket) },
]}
/>
TicketVerificationModal is not modified — it keeps rendering exactly as it does today (the 4 existing sections: full_flow_completed, initiation_type, objection_handled_correctly, sales coach notes).
New DS component TicketStateModal (packages/design-system/src/components/TicketStateModal/), opened by the new "Change Status" menu item, built on the same portal/focus-trap/Escape-to-close shell as TicketVerificationModal/AttendanceQuickModal:
- Header: agent name + Case ID + current
OUTCOME_BADGE(reused fromutils/upsell/outcomeDisplay.ts). - Override section: "Mark Accepted" / "Mark Rejected" buttons, disabled on whichever is currently active, plus an optional reason
TextField(DS component already used elsewhere, e.g.StageClose.tsx's Order ID field). On submit, callschangeTicketStatevia the existingstartTransition+router.refresh()pattern already used for the other actions in this file. - History section (same view, below the override controls): fetches
getTicketStateHistory(ticketId)on open, renders aDataTable(same componentSessionHistoryViewuses):changed_at→old_state → new_state(twoOUTCOME_BADGEs with an arrow between) →changed_by_name→reason. Empty state: "No manual overrides yet." - Footer: Close button.
- Ships with
.stories.tsx,.test.tsx,.test.cy.tsx.
AcceptedTicketsVerification.tsx gets one new local state (stateModalTicket) alongside the existing managedTicket, each independently rendering its own modal — mirrors the existing managed state pattern already used for TicketVerificationModal.
Side effects to call out in the PR description (no code change, just visibility)
- Reclassifying a client's most-recent ticket changes that client's live cooloff/eligibility on the next Upsell Evaluator lookup (
lookupClient()derives eligibility from the latest ticket'sstatelive, no cache). - Dashboard KPIs, rate tiles, Session History, and the XLSX export all shift on next load — nothing else to invalidate.
Files touched
| File | Change |
|---|---|
supabase/migrations/<ts>_add_upsell_state_changes.sql |
new table + RLS (admin/owner select-only, no insert/update/delete policy) + change_upsell_ticket_state() RPC |
utils/supabase/database.types.ts |
regenerated |
app/(protected)/dashboard/actions.ts |
new changeTicketState, new getTicketStateHistory |
app/(protected)/dashboard/page.tsx |
widen acceptedTicketsResult query to state IN ('accepted','declined_hard'), add state to select |
components/dashboard/AcceptedTicketsVerification.tsx |
AcceptedTicket.state field; swap lone kebab IconButton for KebabMenu (Verify / Change Status); new stateModalTicket local state rendering TicketStateModal |
packages/design-system/src/components/KebabMenu/* |
new — menu trigger + story + tests |
packages/design-system/src/components/TicketStateModal/* |
new — combined override+history modal + story + tests |
packages/design-system/src/components/index.tsx |
export KebabMenu + TicketStateModal |
Tests (TDD)
__tests__/dashboard-state-change.test.ts— mock.rpc()(this action calls the RPC, not.from().update()), role gate, success path, RPC exception messages surfaced verbatim,revalidatePathonly on success.__tests__/dashboard-state-history.test.ts— role gate, empty history →[],profiles!changed_byjoin mapping, DB error →{ error }.KebabMenu.test.tsx+.test.cy.tsx— open/close (click, Escape, outside-click, item-select),role="menu"/"menuitem", arrow-key nav, correctonSelectfired.TicketStateModal.test.tsx+.test.cy.tsx— override buttons disabled on current state, click inactive button callschangeTicketStatewith right args, history rows render (old→new badges, changed_by_name, reason), empty state, focus-trap/Escape (mirrorAttendanceQuickModal.test.tsx).- Extend
__tests__/accepted-tickets-verification.test.tsx: Actions column rendersKebabMenuwith exactly two items; "Verify" opens the (unmodified) verification modal; "Change Status" opensTicketStateModal; mutually exclusive modal states. - Extend/add a page-level test or integration check confirming the widened query actually returns
declined_hardtickets alongsideacceptedones. - No Vitest can exercise the Postgres function body (no pgTAP in this repo) — the RPC's internal guards (role check, terminal-state check, atomicity) can only be verified against a real DEV database; call this out as a manual verification step, not a gap for more unit tests.
Verification (end-to-end)
npm run lint && npm run test.- Apply migration to DEV (
supabase db pushagainstvshcimexazocttjmyrqy), regenerate types. - Exercise the RPC directly against DEV Postgres (SQL editor /
supabase db execute): non-adminauth.uid()rejected;in_progress/no_pitchticket rejected; same-state no-op rejected; a successful call leaves exactly one newupsell_state_changesrow and the ticket'sstate/initiation_typeupdated together in the same query check. npm run devas admin/owner: confirm the Offer Verification table now lists both accepted and declined_hard tickets. Open the kebab on an accepted ticket → "Change Status" → override to Rejected with a reason → confirm the row's status updates on refresh → reopen "Change Status" → confirm the new history row (old→new, your name, reason) appears above/below the override controls → reopen "Verify" → confirminitiation_typereset (Check 2 chips unselected).- Confirm the cooloff side effect: pick a client whose latest ticket is the one just reclassified, run a fresh Upsell Evaluator lookup for that GAID, confirm the eligibility banner reflects the new cooloff window.
Git workflow
New branch from dev: feat/gfiber-683-offer-state-override, per this repo's convention (branch from main... actually per CLAUDE.md this repo branches feature work from main, and periodically main→dev is merged for DEV testing — confirm target base branch matches how GFIBER-689/GFIBER-648 branches were structured before opening the PR).