From 9b94c72e96bf4ae9b230f842d36abc014abc4886 Mon Sep 17 00:00:00 2001 From: Chris Ho Date: Sat, 5 Sep 2026 19:28:58 -0400 Subject: [PATCH 1/6] fixed forms callback delivery and action feedback Co-authored-by: Codex --- .../forms-delivery-feedback/investigation.md | 115 ++++++ .../features/forms-delivery-feedback/spec.md | 67 ++++ .../features/forms-delivery-feedback/srd.md | 115 ++++++ .../forms-delivery-feedback/status.md | 118 +++++++ .../forms-delivery-feedback/test-cases.md | 72 ++++ .../admin/forms/admin-form-builder.tsx | 52 ++- .../admin/forms/form-callback-mappings.ts | 32 +- .../admin/forms/form-callbacks-dialog.tsx | 330 +++++++++++++----- .../admin/forms/form-responses-dashboard.tsx | 113 ++++-- .../forms/generic-form-respondent.tsx | 13 +- .../forms/generic-form-response-form.tsx | 60 +++- .../admin/forms/[formId]/responses/page.tsx | 4 +- apps/blade/src/app/form/[slug]/page.tsx | 5 + .../src/tests/e2e/forms-platform.spec.ts | 22 +- .../forms/admin-form-builder-dialogs.test.tsx | 149 +++++++- .../tests/forms/form-action-feedback.test.tsx | 241 +++++++++++++ .../forms/form-callback-delivery.test.tsx | 118 +++++++ .../forms/generic-form-respondent.test.tsx | 2 +- .../forms/generic-form-response-form.test.tsx | 6 + .../api/src/tests/forms/callbacks.test.ts | 15 +- .../tests/forms/database-callbacks.test.ts | 229 ++++++++++++ .../api/src/utils/forms/callback-policy.ts | 6 +- 22 files changed, 1735 insertions(+), 149 deletions(-) create mode 100644 .forge/features/forms-delivery-feedback/investigation.md create mode 100644 .forge/features/forms-delivery-feedback/spec.md create mode 100644 .forge/features/forms-delivery-feedback/srd.md create mode 100644 .forge/features/forms-delivery-feedback/status.md create mode 100644 .forge/features/forms-delivery-feedback/test-cases.md create mode 100644 apps/blade/src/tests/forms/form-action-feedback.test.tsx create mode 100644 apps/blade/src/tests/forms/form-callback-delivery.test.tsx create mode 100644 packages/api/src/tests/forms/database-callbacks.test.ts diff --git a/.forge/features/forms-delivery-feedback/investigation.md b/.forge/features/forms-delivery-feedback/investigation.md new file mode 100644 index 000000000..5b3d3f29f --- /dev/null +++ b/.forge/features/forms-delivery-feedback/investigation.md @@ -0,0 +1,115 @@ +# Forms Delivery and Action Feedback Investigation + +Inspected 2026-09-05 at local Forge revision `1c1457e0`. +Scope: code/history inspection, user evidence, offline checks, and planning. +No production or database access was used in this investigation. + +## Confirmed delivery defect + +Screenshot 04 shows three `recruiting.notify` failures with one attempt and: + +> Invalid Form Body nonce[NONCE_TYPE_TOO_LONG]: Must be 25 or fewer characters long. + +It also shows one cancelled execution with zero attempts. This supersedes the +earlier observation of no executions: configuration/enqueue now exist for +these samples, and a dispatcher reached Discord. The screenshot cannot tell +whether Cron or manual Retry performed those attempts. Cancellation is +consistent with the reported deletion; it is not evidence of Discord failure. + +`packages/api/src/utils/forms/callback-policy.ts:17` returns `executionId` +unchanged. Executions use UUIDs. `database-callbacks.ts:214` posts that value as +`nonce` with `enforce_nonce: true`. A synthetic invocation of the real helper +returned length **36**, exceeding Discord's **25** character limit. + +The [Discord Create Message contract](https://docs.discord.com/developers/resources/message#create-message) +confirms the length limit and says nonce deduplication covers the past few +minutes. The defect is in request construction, independent of the editor's +chosen note. Coolify access is unnecessary to diagnose this error. It does not +prove all deployment settings are correct or that no additional error will +appear after the nonce is fixed. + +The existing `packages/api/src/tests/forms/callbacks.test.ts:278` explicitly +asserts that the full UUID is returned. It tests identity/stability but omits +the provider length contract. This explains why the suite passes with the bug. + +## Submission confirmation + +`generic-form-response-form.tsx:634` ignores the create response result and +uses `window.location.reload()` when no `onSubmitted` callback is passed. The +public form route passes none. The API already returns `formResponseId`. + +`database-responses.ts:210` only selects a saved response for a multiple-response +form when `requestedResponseId` is provided. Reloading the bare form URL thus +renders an open blank form for `multiple_locked`. The existing +`?responseId=...` route can instead render an ownership-checked receipt. + +The submitted-state panel exists in `generic-form-respondent.tsx:233`, so it +would be inaccurate to say there is no confirmation implementation anywhere. +The confirmed gap is the successful create transition, particularly for +multiple-response forms. Chris reports the symptom; screenshot 03 is a blank +form state, not independent proof of the preceding mutation or live mode. +Editable responses also reload without a distinct update-success announcement. + +## Callback dialog overflow and clarity + +Screenshots 01 and 02 show the same callback dialog at opposite horizontal +scroll positions. The primary action's text is outside the initial view. +Screenshot 05 shows the native dropdown and raw `recruiting.notify` label. + +`form-callbacks-dialog.tsx:58` places a nested grid inside a width-limited dialog +with vertical auto overflow; its selects and grid children have no explicit +minimum-width reset. Native selects can size to the longest option, even when +that option is not selected. Shared buttons default to `whitespace-nowrap`. +These are plausible contributors; exact CSS causality needs a real browser +with long labels and viewport measurements. No browser reproduction was run. + +Further source-confirmed usability gaps: + +- `admin-form-builder.tsx:101` always initializes `discord.assign-role`, even + when unavailable to the editor. +- The dialog shows active configurations by raw slug instead of catalog label. +- The note source select uses only an accessible name, and fixed input relies + on placeholder copy. The callback description is not rendered. +- `addCallback` sets a message on the underlying builder, without closing the + dialog or refreshing configurations. The modal obscures that message. +- Disable refreshes, but has no rejection handler in its promise chain. +- The current mapped contract is only `memberId` plus `note`. Selecting + "What is your name?" maps that answer to the note; it does not construct a + full application summary or select a team director. + +## Response deletion feedback + +`form-responses-dashboard.tsx:642` already has an explicit pre-delete warning +and Delete permanently action. However, the mutation at line 902 only refreshes +on success and exposes neither success toast nor failure feedback. The detail +selection stores a response object, so refreshing list props alone does not +explicitly close that selected detail. Plan deletion as one complete interaction +with pending, success, and failure states rather than adding another warning. + +## Blast radius and previous diagnosis correction + +- The invalid nonce affects every `recruiting.notify` execution using this + helper, across all forms. It does not establish that role assignment or all + other Discord features fail; they do not use this message path. +- Callback dialog/feedback changes apply to the shared generic form editor. +- Receipt defects particularly affect multiple-response forms; verify all modes. +- Deletion feedback affects the shared generic response dashboard. +- All five Club team slugs share these generic surfaces: Sponsorship, Workshop, + Design, Outreach, and Dev. Only Outreach's current failure is pictured; other + forms' live configurations and execution counts remain unknown. +- Earlier migration/history inspection found no automatic legacy-connection + backfill and no historical director-routing parity. Those are separate gaps. + Missing configuration was a plausible explanation for the earlier empty + Delivery view, not a confirmed production root cause. The new configured + failures have a directly evidenced code cause: the invalid nonce. + +## Remaining checks + +1. Reproduce long-label overflow locally with synthetic data in a browser. +2. Exercise actual submit/delete interactions and receipts in all response modes. +3. Test the real outbound handler against a mock enforcing Discord limits. +4. Confirm desired scope of historical director-routing parity before adding + payload fields or configuration migration. +5. After the code repair is deployed, an authorized maintainer verifies matching + Blade/Cron revisions and performs one agreed delivery check. Only investigate + environment or cron if fresh evidence indicates those failures. diff --git a/.forge/features/forms-delivery-feedback/spec.md b/.forge/features/forms-delivery-feedback/spec.md new file mode 100644 index 000000000..95a130b33 --- /dev/null +++ b/.forge/features/forms-delivery-feedback/spec.md @@ -0,0 +1,67 @@ +# Forms Delivery and Action Feedback Spec + +Status: Core scope approved; implementation and validation in progress + +## Purpose and users + +Members need an unmistakable receipt after submitting a form. Form editors +need understandable notification settings, reliable Discord delivery, and clear +confirmation when configuring a callback or deleting a response. + +This proposal addresses Chris's five screenshots and submission/deletion report +from 2026-09-05. See [investigation.md](./investigation.md) for evidence. The +before/after screenshots are hosted in the pull request rather than committed +to the repository. + +## Proposed PR scope + +- Fix recruiting notification delivery rejected by Discord's nonce limit. +- Keep the callback dialog and its actions within the viewport, including long + question labels, mobile widths, and browser zoom. +- Make recruiting configuration understandable: human-readable action names, + visible labels, explanation of the note source, and confirmation of saved + configuration. Unavailable actions remain discoverable but cannot be selected + for configuration. +- Show a persistent submission receipt, including when multiple responses are + allowed. Starting another response must be an explicit choice. +- Confirm successful response deletion and show failures without losing context. +- Show callback configure/disable/retry results in the surface where initiated. + +## Acceptance criteria + +1. A valid configured recruiting callback can reach Succeeded with a + provider-compliant request. Submission success is independent of delivery. +2. Neither the document nor callback dialog requires horizontal scrolling at + 320px, 375px, 768px, and desktop widths. Long labels do not push actions away. + Vertical scrolling remains available for short viewports. +3. Existing configurations display a readable name, enabled state, and useful + summary. Editors can tell what message will be sent and where its note comes + from. Internal identifiers are secondary diagnostic information. +4. A successful submission shows "Response submitted" and a way to review the + saved response. Refresh retains the receipt. A form accepting multiple + responses offers an explicit "Submit another response" action. +5. Failed submissions retain answers and visibly explain the failure. Pending + requests prevent repeated clicks. Editable responses acknowledge updates. +6. Deletion retains the existing destructive warning. Only confirmed successful + deletion closes the detail view, updates the list/count, and announces + "Response deleted". Failure keeps the response available with an error. +7. Saved callback changes become visible immediately. Errors appear inside the + open dialog; retry reports the returned delivery outcome accurately. + +## Boundaries and open decisions + +- Chris approved implementing the proposed order, including mobile, on 2026-09-05. + No production changes, replay, messages, commit, push, issue, or PR creation. +- Missing instruction video is out of scope. +- No schema, dependency, permission, or deployment change is currently needed + for the core fixes. +- Historical team-director mentions and structured applicant summaries remain + a separate unresolved parity requirement. Proposed follow-up: a guided team + selector using existing organizational configuration, a bounded summary, and + an explicit allowlist of mentions. Confirm desired summary fields and whether + this belongs in the same PR before extending the callback payload. +- Do not imply that selecting a question maps every applicant field: the + existing recruiting callback accepts one note. UI copy must describe that + contract honestly while parity remains unresolved. +- A full role-picker redesign and automatic legacy configuration migration are + outside the proposed core PR. Record them separately if required. diff --git a/.forge/features/forms-delivery-feedback/srd.md b/.forge/features/forms-delivery-feedback/srd.md new file mode 100644 index 000000000..29ef1de0c --- /dev/null +++ b/.forge/features/forms-delivery-feedback/srd.md @@ -0,0 +1,115 @@ +# Forms Delivery and Action Feedback SRD + +Status: Core implementation approved on 2026-09-05 + +## Ownership and constraints + +Follow [engineering principles](../../../docs/agentic-development/forge-engineering-principles.md), +[repository conventions](../../../docs/REPO-CONVENTIONS.md), and +[Blade design system](../../../apps/blade/DESIGN_SYSTEM.md). + +Blade owns respondent receipts, callback configuration, and action feedback. +`@forge/api` owns execution dispatch and provider payload validation. Cron is an +affected consumer of the API helper; it should not need a new workflow. +Preserve current form/section permissions, callback permissions, locked modes, +response ownership checks, and server-only external effects. + +## Proposed implementation sequence + +### 1. Repair the Discord request contract + +- Replace the identity implementation in + `packages/api/src/utils/forms/callback-policy.ts:formCallbackDeliveryNonce` + with a deterministic encoding no longer than 25 characters. +- Preferred candidate: encode the full 16 UUID bytes as unpadded base64url + (22 characters), with explicit canonical UUID validation. This preserves all + identity bits; do not truncate a UUID or generate a fresh nonce per retry. +- Keep database execution UUIDs and `enforce_nonce: true` unchanged. The + dispatcher must derive the nonce from the same execution on every attempt. +- Test the actual recruiting request through a mocked Discord boundary, not + only a separately modeled dispatcher. Use synthetic member/config data. +- Preserve succeeded/cancelled guards and fenced lease completion. Discord + documents nonce deduplication only for the past few minutes, so do not claim + unconditional exactly-once delivery across an arbitrarily delayed crash. +- No table migration is required for this proposal. Failed executions contain + input snapshots and derive their nonce at dispatch time. + +### 2. Make the callback dialog fit and explain its state + +Likely files: `apps/blade/src/app/_components/admin/forms/` +`form-callbacks-dialog.tsx`, `admin-form-builder.tsx`, +`form-callback-mappings.ts`, and adjacent tests. + +- Reproduce the overflow with a long unselected question option and the + disabled permission label; measure actual scrollWidth/clientWidth before + choosing CSS. Suspected causes are native select intrinsic width and nested + grid minimum sizing, compounded by non-wrapping actions. +- Use existing Blade Select/combobox conventions, explicit constrained widths, + shrinkable grid children, wrapping summaries/actions, and viewport gutters. + Do not hide overflow as a substitute for making controls usable. +- Select the first permitted action (or an empty explanatory state), rather + than hardcoding unavailable role assignment. Keep server enforcement intact. +- Resolve configured slugs through catalog labels, show description and saved + note-source summary. Existing `getAdminForm` callback mappings should be + inspected before adding any API fields. +- Give note source/fixed value visible labels. Saved settings should seed an + edit flow rather than presenting an unrelated empty draft. +- Keep save failures in the dialog. On success, close and announce the result, + refresh server-read props, and preserve unrelated unsaved builder edits. + Add explicit error handling for disable. + +### 3. Retain the response receipt + +Likely files: `generic-form-response-form.tsx`, `generic-form-respondent.tsx`, +and `apps/blade/src/app/form/[slug]/page.tsx`. + +- Use the returned `formResponseId` to navigate to the existing + `/form/?responseId=` ownership-checked receipt path instead of + unconditionally reloading the same URL. Keep pages server components. +- Display and focus a clear submitted state; preserve the receipt on refresh. + Use a transient immediate success state if navigation is delayed. +- For `multiple_locked`, provide an explicit action back to the bare form URL + to start another response. Preserve single-locked and editable behavior. +- Audit all `GenericFormResponseForm` consumers and its optional `onSubmitted` + callback before changing the component contract. Announce update success. +- Failed mutations preserve answers and show an accessible error. Do not infer + that a callback failure means the response failed to save. + +### 4. Confirm deletion and callback outcomes + +Likely file: `form-responses-dashboard.tsx` and its interaction tests. + +- Keep the existing pre-delete confirmation. Await deletion before closing the + selected detail, refreshing the list/counts, and showing a success toast. +- Keep the detail open on failure and expose the error. Reset selection only + for the response actually deleted; keep search and active tab. +- Retry returns a result that may itself be `failed` despite mutation success. + Do not show a delivered toast merely because the HTTP mutation resolved. +- Display friendly callback labels and status explanations, retaining provider + detail for diagnosis. Cancelled/deleted executions must not offer retry. + +## Compatibility and rollout + +- A new configuration affects future responses only; the PR must not silently + enqueue historical submissions or revive legacy synchronous connections. +- An authorized maintainer deploys API-consuming Blade and Cron images and + checks the revision of both, since manual retry and scheduled delivery can + otherwise use different code. +- After deployment, an authorized operator may retry one retained execution + that failed specifically with `NONCE_TYPE_TOO_LONG`, then verify its result. + This is a separate side-effecting action, not part of this investigation. +- Cancelled executions whose responses were deleted remain cancelled. Failed + executions are not automatically selected by `dispatchPendingFormCallbacks`. +- If a fresh execution remains pending, investigate cron scheduling. If it + reports a different provider error, investigate that error separately. +- Reverting the nonce repair restores the known rejection; pause further + operational retries if rollback is needed. + +## Questions before expanding scope + +- Historical parity: exact team-selection source, intended director mention, + and minimum summary fields need confirmation. Reuse existing role/team data; + do not scatter yearly Discord IDs into form components. +- Confirm Outreach's response mode in a permitted read-only view. The reported + cleared form is directly explained by the multiple-response branch, but its + live mode was not provided. diff --git a/.forge/features/forms-delivery-feedback/status.md b/.forge/features/forms-delivery-feedback/status.md new file mode 100644 index 000000000..8a772d795 --- /dev/null +++ b/.forge/features/forms-delivery-feedback/status.md @@ -0,0 +1,118 @@ +# Forms Delivery and Action Feedback Status + +Current phase: Implementation and automated local validation complete; manual zoom review and production delivery verification remain + +## Decisions + +- 2026-09-05: Chris requested investigation and durable planning for Discord + recruiting failure, callback dialog overflow/clarity, submission receipts, + and response deletion feedback. Preserve the supplied screenshots for the PR. +- New screenshot evidence confirms Discord rejects the nonce; it replaces + missing configuration as the explanation for these attempted executions. +- Proposed core PR owns Blade forms UX and the shared API nonce repair. Cron + is a verification/deployment consumer. Historical team routing remains an + explicit scope decision, not an assumed completed requirement. +- Work branch: `codex/forms-delivery-feedback`, rebased on 2026-09-05 from + `1c1457e0` onto current `origin/main` at `f4436df1` before publication. +- Chris approved starting the proposed implementation order with mobile support. +- Core nonce repair, callback editor, submission receipt, deletion feedback and + retry outcome feedback are implemented. Automated tests used only synthetic + local/disposable PostgreSQL data and cleaned up their fixtures. No production + access, real callback invocation, Discord send, branch push, or PR creation + occurred. The tracking issue was created in Chris's fork. + +## Ordered work + +- [x] Inspect current code and user evidence; correct earlier hypotheses. +- [x] Preserve screenshots and create investigation/spec/SRD/test-case bundle. +- [x] Measure the real helper's nonce length and run focused existing tests. +- [x] Approve core PR; historical director mentions and summary expansion remain + a separate open follow-up. +- [x] Reproduce provider-contract failure and overflow/interaction bugs with + tests that exercise the actual implementation boundary. +- [x] Repair nonce, callback editor, receipt transition, and deletion feedback. +- [x] Verify isolated negative cases, unavailable actions, all receipt modes, + retry outcomes, and mobile/desktop component layout. +- [x] Run root checks and changed React analysis; preserve exact blockers below. +- [x] Restore local dependencies and PostgreSQL; pass the full automated gate + and targeted authenticated forms E2E. +- [ ] Check literal 200% browser zoom and complete formal review. +- [x] Prepare local PR text with before/after evidence and deployment checklist. + External publication requires a subsequent request. +- [ ] Authorized maintainer validates one delivery after deploying matching + Blade/Cron revisions. Do not replay historical responses automatically. + +## Investigation baseline captured on 2026-09-05 + +- Real `formCallbackDeliveryNonce` called with a synthetic UUID via `tsx`: + `{ nonceLength: 36, discordMaxLength: 25, violatesDiscordLimit: true }`. +- `pnpm --filter @forge/api test -- src/tests/forms/callbacks.test.ts src/tests/forms/responses.test.ts`: + **27 tests passed**, 2 files. +- `pnpm --filter @forge/blade test -- src/tests/forms/generic-form-response-form.test.tsx src/tests/forms/generic-form-respondent.test.tsx src/tests/forms/form-responses-dashboard.test.tsx src/tests/forms/admin-form-builder-dialogs.test.tsx src/tests/forms/form-callback-mappings.test.ts`: + **29 tests passed**, 5 files. +- These passing baseline tests do not establish correct provider payload or + browser layout. No new regression test or end-to-end run occurred this phase. +- Documentation formatting passed; all local Markdown links resolve and all + five preserved screenshots were verified byte-for-byte against attachments. + +## Implementation verification + +- Nonce regression failed at 36 > 25 before the fix. The full API forms suite + now passes 67 tests in 11 files, including 7 actual enqueue/dispatcher tests + using mocked database and Discord boundaries. +- Blade forms suite: 131 tests passed in 20 files after rebasing onto current + `origin/main`. Covers pending submit, + failure retention, each response mode, callback save/edit/permission states, + deletion confirmation/failure, and accurate retry outcomes. +- Cron suite: 29 tests passed in 6 files. No cron process was started. +- Headed Chrome with real components and synthetic mocked data reproduced the + old dialog at clientWidth 373 / scrollWidth 1412 on a 375px viewport. + Updated dialog clientWidth equals scrollWidth at 320, 375, 768, and 1440px. + Synthetic receipt navigation/refresh and mobile deletion also passed. +- Edge cases: 320x480 short viewport, 240-character unbroken question label at + 320px, and 720x450 reflow passed. The unbroken-label regression was reproduced + first (scrollWidth 3245), then fixed and rechecked (scrollWidth 271). +- Mobile submission/deletion failures retained context. Rechecked synthetic + answer text after receipt navigation and refresh. Inspect the preserved + after screenshots in the pull request's Screenshots section. +- The synthetic browser checks isolate components rather than a deployed system. + The authenticated local Next route is covered separately below. No real + Discord behavior was tested. +- Restored dependencies from the unchanged lockfile and refreshed stale + generated validator/UI declarations. No dependency declarations or lockfile + were changed. +- React analyzer: 10 tracked changed TSX files, 8 components, zero failures. +- `pnpm format`: passed, 24 tasks. `git diff --check`: passed. +- Repository lint without stale ESLint caches: 31 tasks passed with warnings and + zero errors. The normal cached run had replayed unresolved-type errors created + before dependencies were restored. +- `pnpm typecheck`: 33 tasks passed. `pnpm build`: 21 tasks passed. +- `pnpm test`: 29 tasks passed on the second full run. The first run completed + all 136 database assertions but timed out dropping one disposable database; + that file passed 10/10 in isolation before the successful full rerun. +- Extended `forms-platform.spec.ts` to assert receipt URL/refresh and deletion + toast/count. The targeted authenticated journey passed 1/1 in 26.5 seconds + against localhost PostgreSQL. Earlier timeouts came from Playwright reusing a + stale unresponsive Node 24 server on port 3100; a fresh Node 25 server passed. +- After rebasing, the API forms suite passed 69 tests in 11 files, Cron passed + 29 tests in 6 files, and root typecheck passed all 33 tasks. +- The automated gate is green. Formal human/CodeRabbit review and literal 200% + browser zoom remain before merge readiness. + +## Next owner actions + +1. Check literal 200% browser zoom (720x450 reflow was tested, but is not an + actual browser zoom setting) and review the final diff for scope. +2. Complete review and request publication separately. After approved deployment, + an authorized maintainer verifies matching Blade/Cron revisions and retries + one retained NONCE_TYPE_TOO_LONG failure, without replaying all old responses. + +## Links + +- [Investigation and source evidence](./investigation.md) +- [Product scope](./spec.md) +- [Technical plan](./srd.md) +- [Acceptance tests](./test-cases.md) +- Existing baseline: [Forms and Event Feedback](../forms-and-event-feedback/spec.md) +- Tracking issue: [ChrisH0125/forge#1](https://github.com/ChrisH0125/forge/issues/1) +- Pull request: pending publication. diff --git a/.forge/features/forms-delivery-feedback/test-cases.md b/.forge/features/forms-delivery-feedback/test-cases.md new file mode 100644 index 000000000..e1ef7916c --- /dev/null +++ b/.forge/features/forms-delivery-feedback/test-cases.md @@ -0,0 +1,72 @@ +# Forms Delivery and Action Feedback Test Cases + +Status: Core regression tests implemented; validation in progress + +## API and provider boundary + +- **TC-001:** Given a canonical execution UUID, derive a stable string nonce + of at most 25 characters. Distinct UUIDs (including a difference in the last + byte) remain distinct. Invalid identities fail before an outbound request. +- **TC-002:** Enqueue a synthetic configured recruiting submission in a + disposable local test environment; dispatch through the actual database + dispatcher with mocked Discord. Assert the request meets the nonce limit, + preserves `enforce_nonce` and mention restrictions, and records Succeeded. +- **TC-003:** Mock a provider rejection. Response remains saved, delivery is + Failed with bounded error and attempts. Retry uses the same nonce; completed + and cancelled executions cannot send again. Existing lease tests stay green. +- **TC-004:** No active configuration produces no execution. A valid active + configuration creates work for new submissions only. Configuration changes + do not rewrite existing execution inputs or enqueue old responses. +- **TC-005:** Distinguish automatic dispatch of due pending/expired running + work from explicitly retried failed work. Exercise deleted-response guards. + +## Callback editor and responsive browser checks + +- **TC-006:** At 320px, 375px, 768px, and desktop widths plus 200% browser zoom, + open the real callback dialog with a long question prompt, long permission + label, and active configuration. The dialog and document have no horizontal + overflow. Save, Disable, and Close remain reachable. Verify real bounding + boxes and screenshots; jsdom cannot prove layout. +- **TC-007:** Role assignment unavailable, recruiting available: no unavailable + action is selected for configuration. If none are available, saving is + disabled with an explanation. Keyboard selection remains usable. +- **TC-008:** Configure recruiting from fixed note and from a compatible text + question. Visible labels explain both choices. Saved action label and source + summary match persisted mappings on reopen. No raw UUID is required for the + recruiting workflow. Long or optional answers exercise the existing note + length/non-empty contract; do not silently truncate or invent values. +- **TC-009:** Successful configure/disable updates the visible state and + announces completion. Failed operations display errors where the user is + working. Callback saves must not erase unsaved question edits. + +## Member submission and response administration + +- **TC-010:** Submit a `multiple_locked` form through the actual route. A clear + receipt uses the returned response ID, survives refresh, and shows saved + answers. Only "Submit another response" opens a new blank form. +- **TC-011:** Repeat for `single_locked` and `single_editable`. Locked behavior + is preserved; updating an editable response visibly confirms the update. +- **TC-012:** Failed submit/update retains answers and exposes an accessible + error. Pending state prevents repeated clicks. A different user's response + ID cannot be viewed through the receipt URL. +- **TC-013:** Delete from the response detail. The existing warning precedes + deletion. Successful deletion closes detail, updates count/list, preserves + search/tab, and announces success. Rejected deletion retains detail and + reports the error; cancelled confirmation does not mutate anything. +- **TC-014:** Retry failure is not reported as delivery success. Cancelled rows + have no Retry button. Friendly names retain diagnostic procedure identifiers + as secondary detail. Respondents never see callback errors or controls. + +## Verification placement and limits + +- API contract/dispatch tests: `packages/api/src/tests/forms/`, using the real + handler with mocked provider and disposable test data where persistence is + needed. Do not start Cron against a real database to run tests. +- Blade interaction tests: `apps/blade/src/tests/forms/`; browser flows and + screenshots: `apps/blade/src/tests/e2e/forms-platform.spec.ts` or a focused + adjacent spec. Test all generic form modes and both desktop/mobile. +- API shared changes require affected Blade/Cron typechecks and tests. Finish + with required root format/lint/typecheck/test/build checks and changed React + analysis once implementation exists. +- Production smoke testing is an explicitly authorized maintainer action. + Local tests must not send Discord messages or load production data. diff --git a/apps/blade/src/app/_components/admin/forms/admin-form-builder.tsx b/apps/blade/src/app/_components/admin/forms/admin-form-builder.tsx index 521764cf4..c34fe34f6 100644 --- a/apps/blade/src/app/_components/admin/forms/admin-form-builder.tsx +++ b/apps/blade/src/app/_components/admin/forms/admin-form-builder.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useReducer, useState } from "react"; +import { useEffect, useReducer, useState, useTransition } from "react"; import Link from "next/link"; import { usePathname, useRouter, useSearchParams } from "next/navigation"; import { @@ -15,6 +15,7 @@ import { ArrowLeft, FilePenLine } from "lucide-react"; import type { RouterOutputs } from "@forge/api"; import { Badge } from "@forge/ui/badge"; import { Button } from "@forge/ui/button"; +import { toast } from "@forge/ui/toast"; import { checkUploadMetadata, FORM_BANNER_UPLOAD_POLICY, @@ -26,7 +27,10 @@ import type { BuilderInitial, CallbackCatalogItem, } from "./form-builder-types"; -import type { FormCallbackDraft } from "./form-callback-mappings"; +import type { + ConfiguredFormCallback, + FormCallbackDraft, +} from "./form-callback-mappings"; import type { MediaInstruction } from "./form-definition-draft"; import { AdminPageHeader, @@ -68,7 +72,7 @@ export function AdminFormBuilder({ shareAssets, }: { callbacks: CallbackCatalogItem[]; - configuredCallbacks?: { active: boolean; callbackSlug: string; id: string }[]; + configuredCallbacks?: ConfiguredFormCallback[]; initial?: BuilderInitial; readOnly?: boolean; respondentRoles: { id: string; name: string }[]; @@ -103,9 +107,11 @@ export function AdminFormBuilder({ draftAvailability(initial, sections), ); const [message, setMessage] = useState(null); + const [callbackError, setCallbackError] = useState(null); + const [callbacksRefreshing, refreshCallbacks] = useTransition(); const [callbackDraft, setCallbackDraft] = useState({ questionId: "", - slug: "discord.assign-role", + slug: callbacks.find((callback) => callback.available)?.slug ?? "", value: "", }); const [openDialog, setOpenDialog] = useState("none"); @@ -142,6 +148,7 @@ export function AdminFormBuilder({ dialog: Exclude, open: boolean, ) { + if (dialog === "callbacks") setCallbackError(null); setOpenDialog(open ? dialog : "none"); } @@ -242,15 +249,18 @@ export function AdminFormBuilder({ async function addCallback() { if (!initial) return; + setCallbackError(null); try { await configureCallback.mutateAsync({ callbackSlug: callbackDraft.slug, formId: initial.id, mappings: callbackInputMappings(callbackDraft), }); - setMessage("Callback configured for future responses."); + setOpenDialog("none"); + toast.success("Callback saved for future responses."); + refreshCallbacks(() => router.refresh()); } catch (cause) { - setMessage( + setCallbackError( cause instanceof Error ? cause.message : "Callback configuration failed.", @@ -258,14 +268,21 @@ export function AdminFormBuilder({ } } - function disableFormCallback(callbackSlug: string) { + async function disableFormCallback(callbackSlug: string) { if (!initial) return; - void disableCallback - .mutateAsync({ callbackSlug, formId: initial.id }) - .then(() => { - setMessage("Callback disabled for future responses."); - router.refresh(); - }); + setCallbackError(null); + try { + await disableCallback.mutateAsync({ callbackSlug, formId: initial.id }); + setOpenDialog("none"); + toast.success("Callback disabled for future responses."); + refreshCallbacks(() => router.refresh()); + } catch (cause) { + setCallbackError( + cause instanceof Error + ? cause.message + : "Callback could not be disabled.", + ); + } } function deleteFormPermanently() { @@ -455,9 +472,14 @@ export function AdminFormBuilder({ setOpenDialog("none")} onDisableCallback={disableFormCallback} diff --git a/apps/blade/src/app/_components/admin/forms/form-callback-mappings.ts b/apps/blade/src/app/_components/admin/forms/form-callback-mappings.ts index c277c92f4..431dc3759 100644 --- a/apps/blade/src/app/_components/admin/forms/form-callback-mappings.ts +++ b/apps/blade/src/app/_components/admin/forms/form-callback-mappings.ts @@ -1,6 +1,6 @@ import type { z } from "zod"; -import type { callbackConfigurationSchema } from "@forge/validators"; +import { callbackConfigurationSchema } from "@forge/validators"; /** * How the callback dialog's three inputs become the input mappings @@ -20,6 +20,36 @@ export interface FormCallbackDraft { value: string; } +export interface ConfiguredFormCallback { + active: boolean; + callbackSlug: string; + id: string; + mappings: unknown; +} + +export function savedCallbackDraft( + callback: ConfiguredFormCallback, +): FormCallbackDraft { + const parsed = callbackConfigurationSchema.shape.mappings.safeParse( + callback.mappings, + ); + const source = parsed.success + ? parsed.data.find( + ({ inputKey }) => + inputKey === + (callback.callbackSlug === "discord.assign-role" ? "roleId" : "note"), + )?.source + : undefined; + return { + slug: callback.callbackSlug, + questionId: source?.kind === "question" ? source.questionId : "", + value: + source?.kind === "fixed" && typeof source.value === "string" + ? source.value + : "", + }; +} + /** * Every callback is handed the responding member, because a callback that * cannot identify the member has nothing to act on. The second input is what diff --git a/apps/blade/src/app/_components/admin/forms/form-callbacks-dialog.tsx b/apps/blade/src/app/_components/admin/forms/form-callbacks-dialog.tsx index 017803305..02a4ef3af 100644 --- a/apps/blade/src/app/_components/admin/forms/form-callbacks-dialog.tsx +++ b/apps/blade/src/app/_components/admin/forms/form-callbacks-dialog.tsx @@ -3,6 +3,7 @@ import type { Dispatch, SetStateAction } from "react"; import type { FormQuestion } from "@forge/validators"; +import { Badge } from "@forge/ui/badge"; import { Button } from "@forge/ui/button"; import { Dialog, @@ -13,10 +14,23 @@ import { DialogTitle, } from "@forge/ui/dialog"; import { Input } from "@forge/ui/input"; +import { Label } from "@forge/ui/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@forge/ui/select"; +import { Textarea } from "@forge/ui/textarea"; import type { FormResponseMode } from "./form-availability-draft"; import type { CallbackCatalogItem } from "./form-builder-types"; -import type { FormCallbackDraft } from "./form-callback-mappings"; +import type { + ConfiguredFormCallback, + FormCallbackDraft, +} from "./form-callback-mappings"; +import { savedCallbackDraft } from "./form-callback-mappings"; export function FormCallbacksDialog({ callbackDraft, @@ -24,6 +38,7 @@ export function FormCallbacksDialog({ configureCallbackPending, configuredCallbacks, disableCallbackPending, + error, onAddCallback, onClose, onDisableCallback, @@ -36,127 +51,264 @@ export function FormCallbacksDialog({ callbackDraft: FormCallbackDraft; callbacks: CallbackCatalogItem[]; configureCallbackPending: boolean; - configuredCallbacks: { active: boolean; callbackSlug: string; id: string }[]; + configuredCallbacks: ConfiguredFormCallback[]; disableCallbackPending: boolean; + error?: string | null; onAddCallback: () => Promise; onClose: () => void; - onDisableCallback: (callbackSlug: string) => void; + onDisableCallback: (callbackSlug: string) => Promise; onOpenChange: (open: boolean) => void; open: boolean; questions: FormQuestion[]; responseMode: FormResponseMode; setCallbackDraft: Dispatch>; }) { - function updateCallbackDraft( - key: Key, - value: FormCallbackDraft[Key], - ) { - setCallbackDraft((current) => ({ ...current, [key]: value })); + const selected = callbacks.find(({ slug }) => slug === callbackDraft.slug); + const pending = configureCallbackPending || disableCallbackPending; + const editable = responseMode !== "single_editable"; + const textQuestions = questions.filter( + (question) => + !question.retired && + (question.type === "short_text" || question.type === "paragraph"), + ); + + function selectCallback(slug: string) { + const saved = configuredCallbacks.find( + (callback) => callback.callbackSlug === slug, + ); + setCallbackDraft( + saved ? savedCallbackDraft(saved) : { slug, questionId: "", value: "" }, + ); } return ( - - - + { + if (!pending) onOpenChange(next); + }} + > + + Callbacks - Configure code-owned actions for future locked responses. + Choose what happens after a new response is submitted. Changes apply + to future responses only. -
- {configuredCallbacks - .filter(({ active }) => active) - .map((callback) => ( +
+ {configuredCallbacks.map((callback) => { + const catalog = callbacks.find( + ({ slug }) => slug === callback.callbackSlug, + ); + const saved = savedCallbackDraft(callback); + const question = questions.find( + ({ id }) => id === saved.questionId, + ); + return (
- {callback.callbackSlug} - +
+ + {catalog?.label ?? callback.callbackSlug} + + + {callback.active ? "Enabled" : "Disabled"} + +
+

+ {callback.callbackSlug === "recruiting.notify" + ? saved.questionId + ? `Note from answer: ${question?.prompt ?? "Unavailable question. Choose another source."}` + : `Fixed note: ${saved.value || "Not configured"}` + : "Assigns the configured Blade role to the respondent."} +

+
+ + {callback.active && ( + + )} +
- ))} - - {callbackDraft.slug === "recruiting.notify" && ( - - - {questions - .filter( - (question) => - question.type === "short_text" || - question.type === "paragraph", - ) - .map((question) => ( - + + + + + {callbacks.map((callback) => ( + + {callback.label} + {callback.available + ? "" + : ` (requires ${callback.requiredPermission})`} + ))} - + + +
+ {selected && ( +

+ {selected.slug === "recruiting.notify" + ? "Sends the respondent’s name, email, and one note to the configured recruiting Discord channel. Choose a fixed note or a text answer below." + : selected.description} +

+ )} + {selected?.slug === "recruiting.notify" && ( +
+ + + {callbackDraft.questionId && ( +

+ The selected answer must contain text and be no more than + 1,500 characters. +

+ )} +
+ )} + {selected && + (!callbackDraft.questionId || + selected.slug === "discord.assign-role") && ( +
+ + {selected.slug === "discord.assign-role" ? ( + + setCallbackDraft((current) => ({ + ...current, + value: event.target.value, + })) + } + /> + ) : ( +