223 lines
17 KiB
Markdown
223 lines
17 KiB
Markdown
# GFIBER-689 — Incentive Verification
|
|
|
|
## Context
|
|
|
|
Jira ticket [GFIBER-689](https://willowtree.atlassian.net/browse/GFIBER-689) ("Incentive Verification", reported by Josias Jimenez, assigned to Alejandro Lembke, status *In Development*) asks for cosmetic and functional changes to the admin dashboard's verification table:
|
|
|
|
1. Rename "Upsell Verification" → "Offer Verification".
|
|
2. Rename the Status column → "Incentive", with values Yes / No / Pending.
|
|
3. Add a "Sales Coach Notes" free-text field.
|
|
4. Add a button that opens an "Offers Table" with date filters and the offer details + notes.
|
|
|
|
This ticket lands one day after **PR #100** ("Upsell Verification — 3 validaciones con Status derivado", merged 2026-07-13) shipped the exact feature it's extending: `components/dashboard/AcceptedTicketsVerification.tsx` already derives a `pending`/`accepted`/`rejected` status from three checks (`full_flow_completed`, `initiation_type`, `objection_handled_correctly`) via `utils/upsell/verification.ts`'s `deriveVerificationStatus()`. Docmost's `Estado_Actual_del_Proyecto` and the code agree exactly — verified directly, no drift.
|
|
|
|
Brainstormed with the user to resolve what the ticket text alone couldn't answer, reusing the existing "all agents" performance/attendance tab pattern (`AllTeamsCard.tsx`) as the template for the new "Offers Table" ask, which turned out to really be a per-agent aggregate (not a per-ticket detail view). Final resolved scope, in the user's own words: the existing verification table becomes a "Validations" tab; a new "Information by Agent" tab shows per-agent Pending/Valid/Rejected/Total counts with its own Excel export; both views exclude `admin`/`owner`, showing only `role="user"` agents.
|
|
|
|
**Why this matters beyond the literal ticket text:** the existing feature has hard-won business rules (a `customer_initiated` ticket can never be `accepted`; `full_flow_completed = false` is a non-overridable rejection backstop) that must survive this change untouched — this plan treats `deriveVerificationStatus()` as frozen and only relabels its output, never recomputes it differently.
|
|
|
|
## Scope decisions (confirmed with the user)
|
|
|
|
| Question | Decision |
|
|
|---|---|
|
|
| Incentive Yes/No/Pending vs. existing derived status | Pure UI relabel, 1:1 (Yes=accepted, No=rejected, Pending=pending). No logic change. |
|
|
| Sales Coach Notes location | Inside `TicketVerificationModal` only — no new table column. |
|
|
| Sales Coach Notes vs. existing Salesforce-notes/objections_handled | Fully new, independent field. |
|
|
| "Offers Table" shape | New "Information by Agent" tab inside a new wrapper card, mirroring `AllTeamsCard`'s tab pattern — not a per-ticket detail view. |
|
|
| Role scope | Both tabs (Validations + Information by Agent) show only `role="user"` agents/tickets — `admin`/`owner` excluded entirely. |
|
|
| Team-toggle interaction | Information by Agent ignores the Mine/All-teams toggle — always the full role=user roster, same as today's Validations table (rendered identically in both branches of `page.tsx`). |
|
|
| Export button scope | Dedicated single-sheet export just for the Information-by-Agent table (new sibling route), not folded into the existing 5-sheet "Export report". |
|
|
|
|
Verified directly against code (not just docs) before finalizing this plan:
|
|
- `StatusBadge` ([index.tsx](packages/design-system/src/components/StatusBadge/index.tsx#L13-L17)) renders `{status}` literally with no label-override capability — a DS change is required for the Yes/No/Pending relabel.
|
|
- The accepted-tickets query in `page.tsx` ([lines 124-130](app/(protected)/dashboard/page.tsx#L124-L130)) has **no role filter today** — only `.eq("state", "accepted")`. `allAgents` (role="user", pre-team-filter) is already computed at [line 164](app/(protected)/dashboard/page.tsx#L164), before the `acceptedTickets` line ([194](app/(protected)/dashboard/page.tsx#L194)) — the fix reuses that existing set.
|
|
|
|
## Implementation
|
|
|
|
### 1. Migration — `sales_coach_notes`
|
|
|
|
New file `supabase/migrations/20260714120000_add_sales_coach_notes_to_upsell_tickets.sql`, following the exact comment style of `20260710090000_add_verification_columns_to_upsell_tickets.sql`:
|
|
|
|
```sql
|
|
alter table public.upsell_tickets
|
|
add column sales_coach_notes text default null;
|
|
|
|
comment on column public.upsell_tickets.sales_coach_notes is
|
|
'Free-text coaching notes an admin/owner can attach to a ticket during '
|
|
'verification review. NULL = no notes recorded. Write access restricted to '
|
|
'admin/owner at the application layer (setSalesCoachNotes Server Action).';
|
|
```
|
|
|
|
No backfill needed — `NULL` is the correct value for every pre-existing row. Apply to DEV (`supabase db push`, project `vshcimexazocttjmyrqy`), then `supabase gen types typescript --linked > utils/supabase/database.types.ts`. Do not push to PROD (`hwxkegbdnbrzvekrqxbd`) as part of this branch.
|
|
|
|
### 2. `StatusBadge` — optional label override (DS change)
|
|
|
|
[packages/design-system/src/components/StatusBadge/index.tsx](packages/design-system/src/components/StatusBadge/index.tsx): add an optional `label` prop that overrides the displayed text while `status` still drives the color class (fully backward-compatible — no existing caller passes it today).
|
|
|
|
```ts
|
|
type TStatusBadgeProps = {
|
|
status: TStatus,
|
|
/** Optional display text override — `status` still drives the color class. */
|
|
label?: string,
|
|
className?: string,
|
|
};
|
|
```
|
|
|
|
The SCSS applies `text-transform: lowercase` to `.badge`; since "Yes/No/Pending" must render capitalized, add a `styles.customLabel` modifier (no text-transform) applied only when `label` is passed.
|
|
|
|
DS-first updates: `StatusBadge.stories.tsx` (new `CustomLabel` story), `StatusBadge.test.tsx` (label override renders instead of status text; color class still keyed by `status`), and `StatusBadge.test.cy.tsx` if it exists.
|
|
|
|
Apply the same Yes/No/Pending relabel to the modal's header badge too (`TicketVerificationModal` renders its own `StatusBadge` from `ticket.status` — thread a new `statusLabel?: string` prop through so the modal shows "Yes" rather than "accepted", keeping the table and modal consistent for the same ticket).
|
|
|
|
### 3. Header/subtext + Incentive column rename
|
|
|
|
In [components/dashboard/AcceptedTicketsVerification.tsx](components/dashboard/AcceptedTicketsVerification.tsx):
|
|
- `aria-label="Upsell Verification"` / `Card title="Upsell Verification"` → `"Offer Verification"`.
|
|
- Subtitle → `"Review 3 criteria checks for each offer"`.
|
|
- Column `header: 'Status'` → `header: 'Incentive'`.
|
|
- Add a local label map next to the existing `PLAN_LABELS` helper:
|
|
```ts
|
|
const INCENTIVE_LABELS: Record<VerificationStatus, string> = {
|
|
accepted: 'Yes',
|
|
rejected: 'No',
|
|
pending: 'Pending',
|
|
};
|
|
```
|
|
Render `<StatusBadge status={derived} label={INCENTIVE_LABELS[derived]} />`.
|
|
|
|
Update `__tests__/accepted-tickets-verification.test.tsx`: heading assertion → "Offer Verification"; status-column assertions → "Yes"/"No"/"Pending" instead of raw values.
|
|
|
|
### 4. `setSalesCoachNotes` Server Action
|
|
|
|
New export in [app/(protected)/dashboard/actions.ts](app/(protected)/dashboard/actions.ts), placed after `setObjectionHandled`, mirroring its exact structure — `requireRole(admin, owner)` gate, `state === "accepted"` guard (kept, since the modal only ever opens from accepted tickets today — no other entry point exists), `revalidatePath("/dashboard")`:
|
|
|
|
```ts
|
|
export type SalesCoachNotesResult = { ok: true } | { error: string };
|
|
|
|
export async function setSalesCoachNotes(
|
|
ticketId: string,
|
|
notes: string | null,
|
|
): Promise<SalesCoachNotesResult> {
|
|
// identical shape to setObjectionHandled, updating sales_coach_notes
|
|
}
|
|
```
|
|
|
|
New test `__tests__/dashboard-sales-coach-notes.test.ts`, structured identically to `__tests__/dashboard-initiation-type.test.ts`: permission checks, state guard, null clears notes, `revalidatePath` on success only, DB-error propagation.
|
|
|
|
### 5. `TicketVerificationModal` — 4th field
|
|
|
|
[packages/design-system/src/components/TicketVerificationModal/index.tsx](packages/design-system/src/components/TicketVerificationModal/index.tsx): add a 4th block using the existing DS `Textarea` component (already exported from `@gfiber/design-system` — no new primitive needed):
|
|
|
|
- New props: `salesCoachNotes: string | null`, `onSetSalesCoachNotes: (value: string | null) => void`, `savingSalesCoachNotes: boolean`.
|
|
- Local `notesDraft` state seeded from `ticket.salesCoachNotes ?? ''`, save-on-blur (`onSetSalesCoachNotes(notesDraft.trim() === '' ? null : notesDraft)`), disabled while saving.
|
|
- Include `savingSalesCoachNotes` in the modal's existing "saving" guard (blocks Escape/backdrop-close mid-save, same as the other two checks).
|
|
|
|
DS-first updates: `TicketVerificationModal.test.tsx` (renders textarea with value; blur calls the handler; empty blur → `null`; disabled while saving), `.stories.tsx` (new args + `WithNotes` story), `.test.cy.tsx` (type + blur mount test).
|
|
|
|
Wire up in `AcceptedTicketsVerification.tsx`: own `useTransition` for notes saving, extend the `AcceptedTicket` type with `sales_coach_notes: string | null`, pass the new props through.
|
|
|
|
### 6. New wrapper — `ValidationsCard`
|
|
|
|
New file `components/dashboard/ValidationsCard.tsx` (client component), mirroring `AllTeamsCard.tsx`'s tab mechanics exactly — local `useState<'validations' | 'information'>('validations')`, `role="tablist"`/`role="tab"`/`role="tabpanel"`, no URL state:
|
|
|
|
- Props: `{ tickets: AcceptedTicket[] }` — same shape already used, no new data-fetching, pure wrapper.
|
|
- **"Validations" tab**: renders `<AcceptedTicketsVerification tickets={tickets} hideHeader />` — add a `hideHeader?: boolean` prop to `AcceptedTicketsVerification` so its own `Card` title/subtitle don't double up inside the wrapper's own header.
|
|
- **"Information by Agent" tab** (exact label): a small table (columns: Agent | Pending | Valid | Rejected | Total), built from `buildValidationSummaryByAgent()` (below), plus a dedicated export link (see §8).
|
|
|
|
`page.tsx`: replace both occurrences of `<AcceptedTicketsVerification tickets={acceptedTickets} />` (the "all" branch and the "mine" branch) with `<ValidationsCard tickets={acceptedTickets} />`.
|
|
|
|
### 7. Per-agent aggregation — `buildValidationSummaryByAgent`
|
|
|
|
New pure function in `utils/upsell/verification.ts` (co-located with `deriveVerificationStatus`, since it directly consumes it):
|
|
|
|
```ts
|
|
export type ValidationSummaryRow = {
|
|
agentId: string;
|
|
displayName: string | null;
|
|
email: string;
|
|
pending: number;
|
|
valid: number;
|
|
rejected: number;
|
|
total: number;
|
|
};
|
|
|
|
export function buildValidationSummaryByAgent(
|
|
tickets: (VerifiableTicket & { created_by: string | null })[],
|
|
agents: { id: string; displayName: string | null; email: string }[],
|
|
): ValidationSummaryRow[]
|
|
```
|
|
|
|
Groups tickets by `created_by`, tallies `deriveVerificationStatus()` per ticket into pending/valid/rejected, and returns one row per agent in the roster (zero-ticket agents included with all-zero counts, same convention as `buildDashboardMetrics`), sorted by total desc then name. Tickets with `created_by === null` are excluded.
|
|
|
|
New unit test `__tests__/utils/upsell/validation-summary.test.ts`: counts derived via the real `deriveVerificationStatus` (not reimplemented), zero-ticket agents appear, null `created_by` excluded, sort order.
|
|
|
|
### 8. Role filter fix + dedicated export route
|
|
|
|
**`page.tsx`** (the actual bug this ticket surfaces): hoist a role-scoped id set right after `allAgents` is computed ([~line 164](app/(protected)/dashboard/page.tsx#L164)):
|
|
|
|
```ts
|
|
const allAgentIds = new Set(allAgents.map((u) => u.id));
|
|
```
|
|
|
|
Then filter `acceptedTickets` ([line 194](app/(protected)/dashboard/page.tsx#L194)) against `allAgentIds` (not the team-scoped `agentIds`, since Information-by-Agent must ignore the Mine/All toggle per the confirmed scope decision):
|
|
|
|
```ts
|
|
const acceptedTickets = (acceptedTicketsResult.data ?? [])
|
|
.filter((t) => t.profiles !== null && t.created_by !== null && allAgentIds.has(t.created_by))
|
|
as unknown as AcceptedTicket[];
|
|
```
|
|
|
|
Also add `sales_coach_notes` to the `acceptedTicketsResult` query's `select(...)` list ([line 126](app/(protected)/dashboard/page.tsx#L126)).
|
|
|
|
**New export route** `app/(protected)/dashboard/api/export/validations/route.ts`, following the same pattern as the existing `app/(protected)/dashboard/api/export/route.ts` (Route Handler, `runtime = "nodejs"`, `dynamic = "force-dynamic"`, `requireRole(admin, owner)` gate, `exceljs`) but scoped to a single sheet:
|
|
|
|
- Query params: `start`/`end` (date range only — no `team`, since this view ignores the team toggle).
|
|
- Fetch `upsell_tickets` (`state = "accepted"`, date-range scoped) + `profiles` (role="user"), reusing `buildValidationSummaryByAgent`.
|
|
- New shared helper `utils/dashboard/excel.ts` — `addSheetFromRows(workbook, name, columns, rows)` (extracted since no such helper exists yet; used only by this new route, not retrofitted onto the existing 5-sheet export to avoid unrelated regression risk).
|
|
- Single worksheet "Information by Agent" with columns Agent/Pending/Valid/Rejected/Total, streamed the same way as the existing route (`Content-Disposition: attachment`).
|
|
- Trigger: a plain `<a href="/dashboard/api/export/validations?start=...&end=..." download>` inside the Information-by-Agent tab panel — same `<a>` + `download` convention as the existing export button, no new button component.
|
|
|
|
New tests: `__tests__/utils/dashboard/excel.test.ts` (bold header row, correct rows for `addSheetFromRows`), `__tests__/dashboard-export-validations-route.test.ts` (new route: correct single-sheet output, admin/owner tickets excluded, auth/role gating mirrors the existing export route's tests).
|
|
|
|
## Files touched
|
|
|
|
| File | Change |
|
|
|---|---|
|
|
| `supabase/migrations/20260714120000_add_sales_coach_notes_to_upsell_tickets.sql` | new column |
|
|
| `utils/supabase/database.types.ts` | regenerated after DEV push |
|
|
| `packages/design-system/src/components/StatusBadge/*` | `label` prop + custom-label style + story/tests |
|
|
| `packages/design-system/src/components/TicketVerificationModal/*` | notes textarea + `statusLabel` prop + story/tests |
|
|
| `components/dashboard/AcceptedTicketsVerification.tsx` | rename copy, Incentive labels, `hideHeader` prop, notes wiring |
|
|
| `components/dashboard/ValidationsCard.tsx` | new — tab wrapper |
|
|
| `utils/upsell/verification.ts` | new `buildValidationSummaryByAgent` |
|
|
| `utils/dashboard/excel.ts` | new — `addSheetFromRows` helper |
|
|
| `app/(protected)/dashboard/actions.ts` | new `setSalesCoachNotes` |
|
|
| `app/(protected)/dashboard/page.tsx` | role-filter fix, `sales_coach_notes` in select, swap in `ValidationsCard` |
|
|
| `app/(protected)/dashboard/api/export/validations/route.ts` | new — dedicated single-sheet export |
|
|
|
|
## Tests (TDD — write these first, red before green)
|
|
|
|
- `packages/design-system/.../StatusBadge.test.tsx` (+ stories, + Cypress) — label override.
|
|
- `packages/design-system/.../TicketVerificationModal.test.tsx` (+ stories, + Cypress) — notes field, `statusLabel`.
|
|
- `__tests__/dashboard-sales-coach-notes.test.ts` — new Server Action, mirrors `dashboard-initiation-type.test.ts`.
|
|
- `__tests__/utils/upsell/validation-summary.test.ts` — new aggregation function.
|
|
- `__tests__/accepted-tickets-verification.test.tsx` — updated copy/label assertions, notes wiring, `hideHeader`.
|
|
- New component test for `ValidationsCard` (tab switching, literal "Information by Agent" label, aggregate counts, export link `href`).
|
|
- `__tests__/utils/dashboard/excel.test.ts` — new `addSheetFromRows` helper.
|
|
- `__tests__/dashboard-export-validations-route.test.ts` — new export route.
|
|
- `__tests__/upsell-verification-status.test.ts` — unchanged, regression guard that `deriveVerificationStatus` logic/return values are untouched.
|
|
- Do **not** touch `dashboard-initiation-type.test.ts` / `dashboard-objection-handled.test.ts` — those actions are unmodified.
|
|
|
|
## Verification (end-to-end)
|
|
|
|
1. `npm run lint && npm test` (Vitest) and the design-system's Jest + Cypress suites — all green.
|
|
2. `supabase db push` against DEV (`vshcimexazocttjmyrqy`), regenerate types, confirm `tsc --noEmit`/`npm run build` clean.
|
|
3. `npm run dev`, log in as admin/owner, open `/dashboard`: confirm "Offer Verification" header/subtext, Incentive column shows Yes/No/Pending, kebab modal shows Yes/No badge + notes textarea; type notes, blur, reload the page (not just `router.refresh()`) to confirm persistence survived a real fetch.
|
|
4. Switch to "Information by Agent" tab: counts match a manually-tallied ticket set for a known agent; confirm the tab ignores the Mine/All toggle (counts identical regardless of which team tab is selected elsewhere on the page).
|
|
5. Confirm role filter: find or seed an `upsell_tickets` row with `created_by` pointing to an `admin`/`owner` profile and `state = 'accepted'` — confirm it appears in **neither** tab (and would have appeared before the `page.tsx` fix, to prove the filter is the actual cause).
|
|
6. Click the Information-by-Agent export button, open the downloaded `.xlsx`, confirm one sheet named "Information by Agent" with matching data and the same admin/owner exclusion.
|
|
7. Sanity-check no collision with the still-unmerged "Upsell State Override" proposal (separate plan, operates on manual `state` reclassification) — no shared logic, only `actions.ts` and `AcceptedTicketsVerification.tsx` are touched by both, and here only additively.
|
|
|
|
## Git workflow
|
|
|
|
Branch `feat/gfiber-689-incentive-verification` from `dev` (dashboard features branch from/merge to `dev` per `docs/knowledge/conventions.md`), PR → `dev`.
|