diff --git a/apps/web/src/pages/workspace/index.tsx b/apps/web/src/pages/workspace/index.tsx index 3fd7faa5..2604adb1 100644 --- a/apps/web/src/pages/workspace/index.tsx +++ b/apps/web/src/pages/workspace/index.tsx @@ -71,7 +71,11 @@ import { cardDocuments } from "./collaboration.ts"; import { useFiledVersions } from "./filed-versions.tsx"; import { threadUpdates } from "./specialist-updates.tsx"; import { useSettledApprovals, type OpenApproval } from "./settled-approvals.ts"; -import { useTypedYes } from "./typed-yes.tsx"; +import { + chooseApproachWithOpenApproval, + deliverWithOpenApproval, + useTypedYes, +} from "./typed-yes.tsx"; import { asksAbout, chosenApproach } from "./approach-choice.ts"; import { selectedDeliverable } from "./reading-selection.ts"; import { @@ -849,6 +853,9 @@ export function StageWorkspace({ setApprovalBusy(false); } }; + const openApproval = pendingApprovals.find( + (approval) => approval.packages === undefined, + ); // Choosing an approach is the person's yes to the version's open ask; the message names the pick. const chooseApproach = async ( message: string, @@ -863,32 +870,34 @@ export function StageWorkspace({ approval.packages === undefined && asksAbout(approval, kind, version), ); - if (asked !== undefined) { - const fault = await onDecideApproval({ - approvalId: asked.approvalId, - choice: "yes", - }); - if (fault !== null) return fault; - } - return (await deliver(message)) ? null : wording.approaches.notSent; + return chooseApproachWithOpenApproval({ + askedApprovalId: asked?.approvalId, + openApprovalId: openApproval?.approvalId, + decide: onDecideApproval, + deliver: () => deliver(message), + notSent: wording.approaches.notSent, + }); }; - const openApproval = pendingApprovals.find( - (approval) => approval.packages === undefined, - ); + const sendWithApproval = (body: string, clearComposer = false) => + deliverWithOpenApproval({ + openApprovalId: openApproval?.approvalId, + decide: onDecideApproval, + deliver: () => deliver(body, clearComposer), + }); const sendNow = () => { setCaution(null); - void deliver(value, true); + void sendWithApproval(value, true); }; const restoreToComposer = (message: string) => setValue((current) => (current.trim().length === 0 ? message : current)); + const undoTypedYes = async (message: string) => { + if (!(await sendWithApproval(message))) restoreToComposer(message); + }; const typedYes = useTypedYes({ pending: pendingApprovals.map((approval) => approval.approvalId), onApprove: (approvalId) => void onDecideApproval({ approvalId, choice: "yes" }), - onUndo: (message) => - void deliver(message).then((sent) => { - if (!sent) restoreToComposer(message); - }), + onUndo: (message) => void undoTypedYes(message), onDropped: restoreToComposer, }); const shell = useShellReply(documentPaneRef); @@ -1232,7 +1241,7 @@ export function StageWorkspace({ }, })} {...(sentId === undefined ? {} : { sentId })} - onAnswerQuestions={(reply) => deliver(reply)} + onAnswerQuestions={(reply) => sendWithApproval(reply)} questionsDisabled={sending || archived} > {archived ? ( @@ -1365,7 +1374,7 @@ export function StageWorkspace({ type="button" variant="outline" size="sm" - onClick={() => void deliver(reply)} + onClick={() => void sendWithApproval(reply)} > {reply} diff --git a/apps/web/src/pages/workspace/typed-yes.test.ts b/apps/web/src/pages/workspace/typed-yes.test.ts index 0fc5b661..5e288226 100644 --- a/apps/web/src/pages/workspace/typed-yes.test.ts +++ b/apps/web/src/pages/workspace/typed-yes.test.ts @@ -1,5 +1,9 @@ import { expect, test } from "bun:test"; -import { isAffirmation } from "./typed-yes.tsx"; +import { + chooseApproachWithOpenApproval, + deliverWithOpenApproval, + isAffirmation, +} from "./typed-yes.tsx"; test("only a whole plain affirmation approves", () => { for (const message of [ @@ -22,3 +26,90 @@ test("only a whole plain affirmation approves", () => { ]) expect(isAffirmation(message)).toBe(false); }); + +test("sending a message while a coordinator approval is open denies it first", async () => { + const steps: string[] = []; + const sent = await deliverWithOpenApproval({ + openApprovalId: "appr-design", + decide: async (decision) => { + steps.push(`decide:${decision.approvalId}:${decision.choice}`); + return null; + }, + deliver: async () => { + steps.push("deliver"); + return true; + }, + }); + expect(sent).toBe(true); + expect(steps).toEqual(["decide:appr-design:no", "deliver"]); +}); + +test("sending a message when no coordinator approval is open delivers directly", async () => { + const steps: string[] = []; + const sent = await deliverWithOpenApproval({ + openApprovalId: undefined, + decide: async () => { + steps.push("decide"); + return null; + }, + deliver: async () => { + steps.push("deliver"); + return true; + }, + }); + expect(sent).toBe(true); + expect(steps).toEqual(["deliver"]); +}); + +test("a failed denial stops before delivering the message", async () => { + const steps: string[] = []; + const sent = await deliverWithOpenApproval({ + openApprovalId: "appr-design", + decide: async () => { + steps.push("decide"); + return "That decision was not saved."; + }, + deliver: async () => { + steps.push("deliver"); + return true; + }, + }); + expect(sent).toBe(false); + expect(steps).toEqual(["decide"]); +}); + +test("choosing an approach approves a matching ask without denying it and denies an unrelated open ask", async () => { + const matchingSteps: string[] = []; + const matchingFault = await chooseApproachWithOpenApproval({ + askedApprovalId: "appr-approaches", + openApprovalId: "appr-approaches", + decide: async (decision) => { + matchingSteps.push(`decide:${decision.approvalId}:${decision.choice}`); + return null; + }, + deliver: async () => { + matchingSteps.push("deliver"); + return true; + }, + notSent: "not sent", + }); + expect(matchingFault).toBeNull(); + expect(matchingSteps).toEqual(["decide:appr-approaches:yes", "deliver"]); + + const unrelatedSteps: string[] = []; + const unrelatedFault = await chooseApproachWithOpenApproval({ + askedApprovalId: undefined, + openApprovalId: "appr-design", + decide: async (decision) => { + unrelatedSteps.push(`decide:${decision.approvalId}:${decision.choice}`); + return null; + }, + deliver: async () => { + unrelatedSteps.push("deliver"); + return true; + }, + notSent: "not sent", + }); + expect(unrelatedFault).toBeNull(); + expect(unrelatedSteps).toEqual(["decide:appr-design:no", "deliver"]); +}); diff --git a/apps/web/src/pages/workspace/typed-yes.tsx b/apps/web/src/pages/workspace/typed-yes.tsx index a60e4dad..3ed7054a 100644 --- a/apps/web/src/pages/workspace/typed-yes.tsx +++ b/apps/web/src/pages/workspace/typed-yes.tsx @@ -98,6 +98,59 @@ export function useTypedYes({ return { held, hold, undo }; } +/** + * Sends a message while a coordinator approval card may be open: denies the open card first + * so the parked coordinator wakes to read the message. + */ +export async function deliverWithOpenApproval({ + openApprovalId, + decide, + deliver, +}: { + openApprovalId: string | undefined; + decide: (decision: { + approvalId: string; + choice: "yes" | "always" | "no"; + }) => Promise; + deliver: () => Promise; +}): Promise { + if (openApprovalId !== undefined) { + const fault = await decide({ approvalId: openApprovalId, choice: "no" }); + if (fault !== null) return false; + } + return deliver(); +} + +/** + * Picks an approach while a coordinator approval card may be open: approves when the open + * card asks about the chosen version, or denies any other open coordinator card first. + */ +export async function chooseApproachWithOpenApproval({ + askedApprovalId, + openApprovalId, + decide, + deliver, + notSent, +}: { + askedApprovalId: string | undefined; + openApprovalId: string | undefined; + decide: (decision: { + approvalId: string; + choice: "yes" | "always" | "no"; + }) => Promise; + deliver: () => Promise; + notSent: string; +}): Promise { + if (askedApprovalId !== undefined) { + const fault = await decide({ approvalId: askedApprovalId, choice: "yes" }); + if (fault !== null) return fault; + } else if (openApprovalId !== undefined) { + const fault = await decide({ approvalId: openApprovalId, choice: "no" }); + if (fault !== null) return fault; + } + return (await deliver()) ? null : notSent; +} + /** The countdown on the card it will approve, with its undo. */ export function TypedYesCountdown({ held,