Fix review findings from the shared screening session

builder-frontend/README.md

@@ -74,11 +74,12 @@ on the service; `subscribe()` returns an unsubscribe function and updates fire
 
 ## Testing with a real form-js form
 
-Tests that type into a rendered form-js form use `src/testing/formJs.ts`. form-js
-redraws a form about 60ms after an import, wiping anything typed before then, and
-passes typed text on only after a 300ms pause or when the field loses focus
-(`focusout`, which Preact uses for `onBlur`). The helpers wait out the redraw and
-send `focusout` so each answer reaches the form at once.
+Tests that type into a rendered form-js form use `answer()` from
+`src/testing/formJs.ts`. form-js passes typed text on only after a 300ms pause or
+when the field loses focus (`focusout`, which Preact uses for `onBlur`), and it
+redraws a form about 60ms after an import, resetting any field whose text it
+hasn't received yet. `answer()` sends `focusout`, so each answer reaches the form
+at once and survives the redraw.
 
 ## Print summary
 

builder-frontend/src/components/screener/Screener.test.tsx

@@ -7,7 +7,7 @@ import {
   fetchPublishedScreener,
 } from "@/api/publishedScreener";
 import type { ScreenerResult } from "@/types";
-import { input, settle, type, until } from "@/testing/formJs";
+import { answer, input, until } from "@/testing/formJs";
 
 const integration = vi.hoisted(() => ({
   enabled: true,
@@ -98,8 +98,7 @@ describe("published screener integration lifecycle", () => {
     );
     expect(input("City")!.value).toBe("Philadelphia");
 
-    await settle();
-    type("City", "Pittsburgh");
+    answer("City", "Pittsburgh");
     await until(() => expect(integration.result).toHaveBeenCalledTimes(2));
     expect(integration.result).toHaveBeenLastCalledWith(
       { custom: { city: "Pittsburgh" } },
@@ -173,7 +172,7 @@ describe("published screener integration lifecycle", () => {
     expect(integration.fail).toHaveBeenCalledExactlyOnceWith(
       "ORIGIN_NOT_ALLOWED",
     );
-    await until(() => expect(input("City")).toBeDefined());
+    await until(() => expect(input("City")).toBeTruthy());
     expect(container.textContent).toContain(
       "isn’t set up to send results to https://crm.example",
     );
@@ -188,7 +187,7 @@ describe("published screener integration lifecycle", () => {
     dispose = render(() => <Screener />, container);
     await vi.waitFor(() => expect(integration.start).toHaveBeenCalledOnce());
     integration.disconnect!("timeout");
-    await until(() => expect(input("City")).toBeDefined());
+    await until(() => expect(input("City")).toBeTruthy());
     expect(container.textContent).not.toContain("Waiting for information");
     expect(container.textContent).toContain("couldn’t connect");
     expect(evaluatePublishedScreener).not.toHaveBeenCalled();
@@ -205,19 +204,18 @@ describe("published screener integration lifecycle", () => {
     await vi.waitFor(() =>
       expect(container.textContent).toContain("no longer connected"),
     );
-    expect(input("City")).toBeDefined();
+    expect(input("City")).toBeTruthy();
     expect(evaluatePublishedScreener).toHaveBeenCalledOnce();
   });
 
   it("keeps the standalone form available without waiting or initial evaluation", async () => {
     integration.enabled = false;
     vi.mocked(evaluatePublishedScreener).mockResolvedValue(result("TRUE"));
     dispose = render(() => <Screener />, container);
-    await until(() => expect(input("City")).toBeDefined());
+    await until(() => expect(input("City")).toBeTruthy());
     expect(container.textContent).not.toContain("Waiting for information");
     expect(evaluatePublishedScreener).not.toHaveBeenCalled();
-    await settle();
-    type("City", "Pittsburgh");
+    answer("City", "Pittsburgh");
     await until(() => expect(container.textContent).toContain("Eligible"));
     expect(integration.result).not.toHaveBeenCalled();
   });

builder-frontend/src/components/screener/Screener.tsx

@@ -17,12 +17,9 @@ import {
 
 import HiddenQuestionsNotice from "@/components/shared/HiddenQuestionsNotice";
 import ScreeningComplete from "@/components/shared/ScreeningComplete";
-import {
-  createScreeningSession,
-  type ScreeningAnswers,
-} from "@/components/shared/screeningSession/screeningSession";
+import { createScreeningSession } from "@/components/shared/screeningSession/screeningSession";
 
-import type { PublishedScreener } from "@/types";
+import type { PublishedScreener, ScreeningAnswers } from "@/types";
 import {
   createScreeningBridge,
   type ScreeningBridge,

builder-frontend/src/components/screenerEditor/preview/Preview.test.tsx

@@ -3,7 +3,7 @@ import { render } from "solid-js/web";
 import { afterEach, expect, it, vi } from "vitest";
 
 import type { ScreenerResult } from "@/types";
-import { input, settle, type, until } from "@/testing/formJs";
+import { answer, input, until } from "@/testing/formJs";
 
 const previewResults = vi.hoisted(() => ({ props: undefined as any }));
 vi.mock("@/api/screener", () => ({ evaluateScreener: vi.fn() }));
@@ -50,15 +50,14 @@ it("evaluates the working screener on open and keeps the last results when an ev
     ),
     container,
   );
-  await until(() => expect(input("Answer")).toBeDefined());
+  await until(() => expect(input("Answer")).toBeTruthy());
   await until(() =>
     expect(evaluateScreener).toHaveBeenCalledExactlyOnceWith("working", {
       answer: "",
     }),
   );
-  await settle();
 
-  type("Answer", "first");
+  answer("Answer", "first");
   await until(() => expect(evaluateScreener).toHaveBeenCalledTimes(2));
   await until(() =>
     expect(container.querySelector('[role="status"]')?.textContent).toContain(
@@ -74,10 +73,14 @@ it("evaluates the working screener on open and keeps the last results when an ev
     expect.objectContaining({ answer: "first" }),
   );
 
-  type("Answer", "second");
+  expect(previewResults.props.resultsLoading()).toBe(false);
+
+  answer("Answer", "second");
+  expect(previewResults.props.resultsLoading()).toBe(true);
   await until(() =>
     expect(container.querySelector('[role="alert"]')).not.toBeNull(),
   );
   expect(container.querySelector('[role="status"]')).toBeNull();
   expect(previewResults.props.results()).toEqual(decided);
+  expect(previewResults.props.resultsLoading()).toBe(false);
 });

builder-frontend/src/components/screenerEditor/preview/Preview.tsx

@@ -76,7 +76,9 @@ const Preview = ({ screener, formSchema }) => {
         <Results
           inputData={session.answersSent}
           results={session.results}
-          resultsLoading={session.loading}
+          // From an edit, not just once the answers are sent, so rows that no
+          // longer match the answers are marked as outdated.
+          resultsLoading={() => session.stale() && !session.failed()}
         />
       </div>
     </div>

builder-frontend/src/components/screenerEditor/preview/Results.tsx

@@ -1,7 +1,10 @@
 import { Accessor, For, Match, Show, Switch } from "solid-js";
 
-import { PreviewFormData, ScreenerResult } from "./types";
-import type { ParameterValues } from "@/types";
+import type {
+  ParameterValues,
+  ScreenerResult,
+  ScreeningAnswers,
+} from "@/types";
 import { checkDisplayName } from "@/utils/checkDisplayName";
 import { createReconciledResults } from "@/utils/reconciledResults";
 
@@ -30,7 +33,7 @@ export default function Results({
   results,
   resultsLoading,
 }: {
-  inputData: Accessor<PreviewFormData>;
+  inputData: Accessor<ScreeningAnswers>;
   results: Accessor<ScreenerResult | undefined>;
   resultsLoading: Accessor<boolean>;
 }) {

builder-frontend/src/components/shared/screeningSession/screeningSession.test.tsx

@@ -3,13 +3,12 @@ import { createRoot, createSignal } from "solid-js";
 import { insert } from "solid-js/web";
 import { afterEach, describe, expect, it, vi } from "vitest";
 
-import type { ScreenerResult } from "@/types";
+import type { ScreenerResult, ScreeningAnswers } from "@/types";
 import {
   createScreeningSession,
-  type ScreeningAnswers,
   type ScreeningSessionOptions,
 } from "./screeningSession";
-import { input, settle, type, until } from "@/testing/formJs";
+import { answer, input, until } from "@/testing/formJs";
 
 const schema = {
   type: "default",
@@ -74,7 +73,6 @@ async function start(
   await until(() =>
     expect(container.querySelector(".fjs-form")).not.toBeNull(),
   );
-  await settle();
   return { session, container, dispose, setSchema };
 }
 
@@ -84,9 +82,9 @@ describe("screening session", () => {
     const { session } = await start({ evaluate });
     expect(evaluate).not.toHaveBeenCalled();
 
-    type("Answer", "fir");
+    answer("Answer", "fir");
     await until(() => expect(session.stale()).toBe(true));
-    type("Answer", "first");
+    answer("Answer", "first");
     expect(session.loading()).toBe(false);
 
     await until(() => expect(evaluate).toHaveBeenCalledOnce());
@@ -110,10 +108,10 @@ describe("screening session", () => {
       .mockResolvedValueOnce(result("FALSE"));
     const { session } = await start({ evaluate, onResults });
 
-    type("Answer", "older");
+    answer("Answer", "older");
     await until(() => expect(evaluate).toHaveBeenCalledOnce());
     expect(session.loading()).toBe(true);
-    type("Answer", "newer");
+    answer("Answer", "newer");
     await until(() => expect(session.results()).toEqual(result("FALSE")));
     older.resolve(result("TRUE"));
     await older.promise;
@@ -132,18 +130,21 @@ describe("screening session", () => {
     const evaluate = vi.fn(() => inFlight.promise);
     const { session } = await start({ evaluate, onResults });
 
-    type("Answer", "first");
+    answer("Answer", "first");
     await until(() => expect(evaluate).toHaveBeenCalledOnce());
-    type("Answer", "second");
+    expect(session.loading()).toBe(true);
+    answer("Answer", "second");
+    expect(session.loading()).toBe(false);
     inFlight.resolve(result("TRUE"));
     await inFlight.promise;
 
     expect(session.results()).toBeUndefined();
+    expect(session.loading()).toBe(false);
     expect(session.stale()).toBe(true);
     expect(onResults).not.toHaveBeenCalled();
   });
 
-  it("keeps the last results when an evaluation fails, until one succeeds", async () => {
+  it("keeps the last results when an evaluation fails, until one succeeds, and clears the failure on an edit", async () => {
     vi.spyOn(console, "error").mockImplementation(() => {});
     const onEvaluationFailed = vi.fn();
     const evaluate = vi
@@ -153,16 +154,17 @@ describe("screening session", () => {
       .mockResolvedValueOnce(result("FALSE"));
     const { session } = await start({ evaluate, onEvaluationFailed });
 
-    type("Answer", "first");
+    answer("Answer", "first");
     await until(() => expect(session.results()).toEqual(result("TRUE")));
-    type("Answer", "second");
+    answer("Answer", "second");
     await until(() => expect(session.failed()).toBe(true));
     expect(session.results()).toEqual(result("TRUE"));
     expect(session.stale()).toBe(true);
     expect(session.loading()).toBe(false);
     expect(onEvaluationFailed).toHaveBeenCalledOnce();
 
-    type("Answer", "third");
+    answer("Answer", "third");
+    expect(session.failed()).toBe(false);
     await until(() => expect(session.results()).toEqual(result("FALSE")));
     expect(session.failed()).toBe(false);
   });
@@ -177,9 +179,9 @@ describe("screening session", () => {
       .mockResolvedValueOnce(result("TRUE"));
     const { session } = await start({ evaluate, onEvaluationFailed });
 
-    type("Answer", "older");
+    answer("Answer", "older");
     await until(() => expect(evaluate).toHaveBeenCalledOnce());
-    type("Answer", "newer");
+    answer("Answer", "newer");
     older.reject(new Error("Unavailable"));
     await until(() => expect(session.results()).toEqual(result("TRUE")));
 
@@ -191,13 +193,13 @@ describe("screening session", () => {
     const evaluate = vi.fn(async () => result("FALSE", ["income"]));
     const { session } = await start({ evaluate });
 
-    type("Answer", "kept");
+    answer("Answer", "kept");
     await until(() => expect(input("Income")).toBeUndefined());
     expect(session.unneededQuestionCount()).toBe(1);
     expect(input("Answer")!.value).toBe("kept");
 
     session.toggleShowAllQuestions();
-    await until(() => expect(input("Income")).toBeDefined());
+    await until(() => expect(input("Income")).toBeTruthy());
     expect(session.showAllQuestions()).toBe(true);
     expect(input("Answer")!.value).toBe("kept");
     expect(evaluate).toHaveBeenCalledOnce();
@@ -207,15 +209,15 @@ describe("screening session", () => {
     const evaluate = vi.fn(async () => result("TRUE"));
     const { setSchema } = await start({ evaluate });
 
-    type("Answer", "kept");
+    answer("Answer", "kept");
     setSchema({
       ...schema,
       components: [
         ...schema.components,
         { id: "age", type: "number", key: "age", label: "Age" },
       ],
     });
-    await until(() => expect(input("Age")).toBeDefined());
+    await until(() => expect(input("Age")).toBeTruthy());
     expect(input("Answer")!.value).toBe("kept");
   });
 
@@ -237,7 +239,7 @@ describe("screening session", () => {
       expect.objectContaining({ married: null }),
     );
 
-    type("Answer", "Edited");
+    answer("Answer", "Edited");
     await until(() => expect(evaluate).toHaveBeenCalledTimes(2));
     expect(evaluate).toHaveBeenLastCalledWith(
       expect.objectContaining({ answer: "Edited", married: null }),
@@ -294,7 +296,7 @@ describe("screening session", () => {
     const evaluate = vi.fn(() => inFlight.promise);
     const first = await start({ evaluate, onResults });
 
-    type("Answer", "sent");
+    answer("Answer", "sent");
     await until(() => expect(evaluate).toHaveBeenCalledOnce());
     first.dispose();
     inFlight.resolve(result("TRUE"));
@@ -303,7 +305,7 @@ describe("screening session", () => {
     expect(first.session.results()).toBeUndefined();
 
     const second = await start({ evaluate });
-    type("Answer", "never sent");
+    answer("Answer", "never sent");
     second.dispose();
     await new Promise((resolve) => setTimeout(resolve, 1000));
     expect(evaluate).toHaveBeenCalledOnce();
@@ -374,7 +376,7 @@ describe("next steps for CRM hosts", () => {
       "Shown group",
     );
 
-    type("City", "Philadelphia");
+    answer("City", "Philadelphia");
     await until(() => expect(projected).toHaveLength(1));
     expect(input("City")).toBeUndefined();
     expect(projected[0]).toEqual(session.projectNextSteps(decided));