diff --git a/.claude/worktrees/admiring-leavitt-72e4d2 b/.claude/worktrees/admiring-leavitt-72e4d2 new file mode 160000 index 000000000..c54ef231c --- /dev/null +++ b/.claude/worktrees/admiring-leavitt-72e4d2 @@ -0,0 +1 @@ +Subproject commit c54ef231c43fa30a03a38d23400176260edfbb36 diff --git a/packages/web/e2e/amstar2-workflow.spec.ts b/packages/web/e2e/amstar2-workflow.spec.ts index f02da1dc3..510101482 100644 --- a/packages/web/e2e/amstar2-workflow.spec.ts +++ b/packages/web/e2e/amstar2-workflow.spec.ts @@ -15,7 +15,7 @@ import { switchUser, type DualReviewerScenario, } from './helpers'; -import { setupProjectWithStudy, markChecklistComplete } from './shared-steps'; +import { setupProjectWithStudy, markChecklistComplete, answerAllAMSTAR2 } from './shared-steps'; let scenario: DualReviewerScenario; @@ -43,13 +43,7 @@ test('Dual-Reviewer AMSTAR2 Workflow', async ({ context, page }) => { await expect(page).toHaveURL(/\/checklists\//, { timeout: 10_000 }); await page.waitForTimeout(2000); - // Answer all questions "Yes" - const yesRadios = page.getByRole('radio', { name: 'Yes' }); - const count = await yesRadios.count(); - for (let i = 0; i < count; i++) { - await yesRadios.nth(i).click(); - } - await page.waitForTimeout(1000); + await answerAllAMSTAR2(page, 'Yes'); await markChecklistComplete(page); await page.goto(`/projects/${projectId}`); @@ -87,13 +81,7 @@ test('Dual-Reviewer AMSTAR2 Workflow', async ({ context, page }) => { await page.waitForTimeout(2000); } - // Answer all questions "No" - const noRadios = page.getByRole('radio', { name: 'No' }); - const noCount = await noRadios.count(); - for (let i = 0; i < noCount; i++) { - await noRadios.nth(i).click(); - } - await page.waitForTimeout(1000); + await answerAllAMSTAR2(page, 'No'); await markChecklistComplete(page); await page.goto(`/projects/${projectId}`); diff --git a/packages/web/e2e/local-checklist.spec.ts b/packages/web/e2e/local-checklist.spec.ts new file mode 100644 index 000000000..6c6094562 --- /dev/null +++ b/packages/web/e2e/local-checklist.spec.ts @@ -0,0 +1,77 @@ +/** + * E2E Test: Local-practice checklist persistence + * + * Local appraisals live in a Y.Doc persisted via y-dexie under the + * `local-practice` project id (no WebSocket provider). This test verifies + * that answers round-trip through the full pipeline: + * UI patch → handlePartialUpdate → buildChecklistAnswerInput + * → updateChecklistAnswer (typed) → handler.updateAnswer + * → Y.Map → DexieYProvider → reload → restored state + * + * Covers AMSTAR2 (boolean-matrix per question) and ROB2 (discriminated + * per-key schemas). ROBINS-I shares ROB2's shape-family so we skip it here; + * the dual-reviewer workflow tests exercise both handlers' typed dispatch + * via the collab path. + * + * No auth needed — /checklist is a public route (see routes/_app.tsx). + * + * Prerequisites: + * pnpm --filter web dev (localhost:3010) + */ + +import { test, expect } from '@playwright/test'; +import { answerAllAMSTAR2, fillROB2Preliminary } from './shared-steps'; + +async function createLocalChecklist( + page: import('@playwright/test').Page, + type: 'AMSTAR2' | 'ROB2' | 'ROBINS_I', + name: string, +) { + await page.goto('/checklist'); + await expect(page.getByRole('heading', { name: /Start an Appraisal/i })).toBeVisible({ + timeout: 10_000, + }); + + await page.locator('#checklist-type').selectOption(type); + await page.locator('#checklist-name').fill(name); + await page.getByRole('button', { name: /^Start$/ }).click(); + + await expect(page).toHaveURL(/\/checklist\/[0-9a-f-]{36}/, { timeout: 10_000 }); + // Wait for the "Loading checklist..." gate to clear (phase === 'synced'). + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); +} + +test.describe('Local-practice checklists', () => { + test('AMSTAR2: answers persist across reload', async ({ page }) => { + await createLocalChecklist(page, 'AMSTAR2', 'Local AMSTAR2 Test'); + + await answerAllAMSTAR2(page, 'Yes'); + + const checkedBefore = await page.getByRole('radio', { name: 'Yes', checked: true }).count(); + expect(checkedBefore).toBeGreaterThan(0); + + await page.reload(); + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); + + // Give the Y.Doc a beat to hydrate from IndexedDB before reading state. + await page.waitForTimeout(1000); + + const checkedAfter = await page.getByRole('radio', { name: 'Yes', checked: true }).count(); + expect(checkedAfter).toBe(checkedBefore); + }); + + test('ROB2: preliminary fields persist across reload', async ({ page }) => { + await createLocalChecklist(page, 'ROB2', 'Local ROB2 Test'); + + await fillROB2Preliminary(page, 'Drug X', 'Placebo'); + + await page.reload(); + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); + await page.waitForTimeout(1000); + + // Y.Text fields round-trip via getTextRef + DexieYProvider. + await expect(page.getByPlaceholder(/experimental intervention/i)).toHaveValue('Drug X'); + await expect(page.getByPlaceholder(/comparator intervention/i)).toHaveValue('Placebo'); + await expect(page.getByPlaceholder(/e\.g\. RR/i)).toHaveValue('RR 1.5'); + }); +}); diff --git a/packages/web/e2e/rob2-workflow.spec.ts b/packages/web/e2e/rob2-workflow.spec.ts index a4ce5d33c..8b78cb7e8 100644 --- a/packages/web/e2e/rob2-workflow.spec.ts +++ b/packages/web/e2e/rob2-workflow.spec.ts @@ -16,7 +16,13 @@ import { switchUser, type DualReviewerScenario, } from './helpers'; -import { setupProjectWithStudy, addOutcome, markChecklistComplete } from './shared-steps'; +import { + setupProjectWithStudy, + addOutcome, + markChecklistComplete, + fillROB2Preliminary, + answerAllROB2Domains, +} from './shared-steps'; let scenario: DualReviewerScenario; @@ -28,43 +34,6 @@ test.afterAll(async () => { if (scenario) await cleanupScenario(scenario); }); -/** Fill the ROB2 preliminary section and select assessment aim */ -async function fillROB2Preliminary( - page: import('@playwright/test').Page, - intervention: string, - comparator: string, -) { - await page.getByText('Individually-randomized parallel-group trial').click(); - - // Select aim BEFORE filling text fields to avoid the bug where aim - // selection was overwriting Y.Text fields with empty values. - const aimBtn = page.getByText('to assess the effect of assignment to intervention'); - await aimBtn.scrollIntoViewIfNeeded(); - await aimBtn.click(); - await page.waitForTimeout(500); - - await page.getByPlaceholder(/experimental intervention/i).fill(intervention); - await page.getByPlaceholder(/comparator intervention/i).fill(comparator); - await page.getByPlaceholder(/e\.g\. RR/i).fill('RR 1.5'); - await page.waitForTimeout(500); -} - -/** Answer all ROB2 domain questions with a given response (Y or N) */ -async function answerAllROB2Domains(page: import('@playwright/test').Page, answer: string) { - for (const domain of ['D1', 'D2', 'D3', 'D4', 'D5']) { - await page.getByRole('button', { name: domain, exact: true }).click(); - await page.waitForTimeout(500); - - const buttons = page.getByRole('button', { name: answer, exact: true }); - const count = await buttons.count(); - for (let i = 0; i < count; i++) { - await buttons.nth(i).click(); - await page.waitForTimeout(100); - } - await page.waitForTimeout(300); - } -} - test('Dual-Reviewer ROB2 Workflow', async ({ context, page }) => { const projectId = await setupProjectWithStudy(context, page, scenario, 'ROB2 E2E Test'); diff --git a/packages/web/e2e/shared-steps.ts b/packages/web/e2e/shared-steps.ts index 14e79d8b9..eb5d95552 100644 --- a/packages/web/e2e/shared-steps.ts +++ b/packages/web/e2e/shared-steps.ts @@ -6,6 +6,54 @@ import path from 'node:path'; import { expect, type Page, type BrowserContext } from '@playwright/test'; import { loginAs, addProjectMember, type DualReviewerScenario } from './helpers'; +/** Click every radio on the AMSTAR2 checklist editor matching the given answer (e.g. "Yes", "No"). */ +export async function answerAllAMSTAR2(page: Page, answer: 'Yes' | 'No' | 'Partial Yes') { + const radios = page.getByRole('radio', { name: answer }); + const count = await radios.count(); + for (let i = 0; i < count; i++) { + await radios.nth(i).click(); + } + await page.waitForTimeout(1000); +} + +/** Fill the ROB2 preliminary section: study design, aim, interventions, numerical result. */ +export async function fillROB2Preliminary( + page: Page, + intervention: string, + comparator: string, + numericalResult = 'RR 1.5', +) { + await page.getByText('Individually-randomized parallel-group trial').click(); + + // Select aim BEFORE filling text fields to avoid the bug where aim + // selection was overwriting Y.Text fields with empty values. + const aimBtn = page.getByText('to assess the effect of assignment to intervention'); + await aimBtn.scrollIntoViewIfNeeded(); + await aimBtn.click(); + await page.waitForTimeout(500); + + await page.getByPlaceholder(/experimental intervention/i).fill(intervention); + await page.getByPlaceholder(/comparator intervention/i).fill(comparator); + await page.getByPlaceholder(/e\.g\. RR/i).fill(numericalResult); + await page.waitForTimeout(500); +} + +/** Answer every ROB2 domain signalling question with a given response (Y, N, PY, PN, NI). */ +export async function answerAllROB2Domains(page: Page, answer: string) { + for (const domain of ['D1', 'D2', 'D3', 'D4', 'D5']) { + await page.getByRole('button', { name: domain, exact: true }).click(); + await page.waitForTimeout(500); + + const buttons = page.getByRole('button', { name: answer, exact: true }); + const count = await buttons.count(); + for (let i = 0; i < count; i++) { + await buttons.nth(i).click(); + await page.waitForTimeout(100); + } + await page.waitForTimeout(300); + } +} + const FIXTURES_DIR = path.join(import.meta.dirname, 'fixtures'); /** diff --git a/packages/web/src/components/checklist/ChecklistYjsWrapper.tsx b/packages/web/src/components/checklist/ChecklistYjsWrapper.tsx index a76de2770..799558dd5 100644 --- a/packages/web/src/components/checklist/ChecklistYjsWrapper.tsx +++ b/packages/web/src/components/checklist/ChecklistYjsWrapper.tsx @@ -9,7 +9,8 @@ import { ChevronLeftIcon } from 'lucide-react'; import { ChecklistWithPdf } from '@/components/checklist/ChecklistWithPdf'; import { useProjectContext } from '@/components/project/ProjectContext'; import { connectionPool } from '@/project/ConnectionPool'; -import { useChecklistAnswers } from '@/primitives/useProject/checklists/useChecklistAnswers'; +import { useChecklistViewModel } from '@/primitives/useProject/checklists/useChecklistViewModel'; +import { buildChecklistAnswerInput } from '@/primitives/useProject/checklists'; import { useProjectStore, selectConnectionPhase } from '@/stores/projectStore'; import { useAuthStore, selectUser } from '@/stores/authStore'; import { ACCESS_DENIED_ERRORS } from '@/constants/errors.js'; @@ -27,41 +28,11 @@ import { AlertDialogFooter, AlertDialogAction, } from '@/components/ui/alert-dialog'; -import { getChecklistTypeFromState, scoreChecklistOfType } from '@/checklist-registry/index'; import { ScoreTag } from '@/components/checklist/ScoreTag'; import { isAMSTAR2Complete } from '@/components/checklist/AMSTAR2Checklist/checklist.js'; import { isROBINSIComplete } from '@/components/checklist/ROBINSIChecklist/checklist'; import { isROB2Complete } from '@/components/checklist/ROB2Checklist/checklist'; -// Valid answer keys for each checklist type (module-level for stable references) -const AMSTAR2_KEY_PATTERN = /^q\d+[a-z]*$/i; -const ROBINS_I_KEYS = new Set([ - 'planning', - 'sectionA', - 'sectionB', - 'sectionC', - 'sectionD', - 'confoundingEvaluation', - 'domain1a', - 'domain1b', - 'domain2', - 'domain3', - 'domain4', - 'domain5', - 'domain6', - 'overall', -]); -const ROB2_KEYS = new Set([ - 'preliminary', - 'domain1', - 'domain2a', - 'domain2b', - 'domain3', - 'domain4', - 'domain5', - 'overall', -]); - interface ChecklistYjsWrapperProps { projectId: string; studyId: string; @@ -97,15 +68,8 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli } }, [connectionState.error, navigate]); - const currentStudy = useProjectStore(s => { - const studies = s.projects[projectId]?.studies || []; - return studies.find((st: any) => st.id === studyId) || null; - }); - - const currentChecklist = useMemo(() => { - if (!currentStudy) return null; - return currentStudy.checklists?.find((c: any) => c.id === checklistId) || null; - }, [currentStudy, checklistId]); + const { currentStudy, currentChecklist, checklistForUI, checklistType, currentScore } = + useChecklistViewModel(projectId, studyId, checklistId); const isReadOnly = currentChecklist?.status ? !isEditable(currentChecklist.status) : false; @@ -214,30 +178,6 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli [orgId, projectId, studyId, studyPdfs, user?.id, addPdfToStudy], ); - const answers = useChecklistAnswers(projectId, studyId, checklistId); - - const checklistForUI = useMemo(() => { - if (!currentChecklist || !answers) return null; - return { - id: currentChecklist.id, - name: currentStudy?.name || 'Checklist', - reviewerName: '', - createdAt: currentChecklist.createdAt, - ...answers, - }; - }, [currentChecklist, currentStudy, answers]); - - const checklistType = useMemo(() => { - if (currentChecklist?.type) return currentChecklist.type; - if (checklistForUI) return getChecklistTypeFromState(checklistForUI); - return 'AMSTAR2'; - }, [currentChecklist, checklistForUI]); - - const currentScore = useMemo(() => { - if (!checklistForUI || !checklistType) return null; - return scoreChecklistOfType(checklistType, checklistForUI); - }, [checklistForUI, checklistType]); - const isChecklistValid = useMemo(() => { if (!checklistForUI) return false; if (checklistType === 'AMSTAR2') return isAMSTAR2Complete(checklistForUI as any); @@ -248,15 +188,11 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli const handlePartialUpdate = useCallback( (patch: Record) => { - if (isReadOnly) return; + if (isReadOnly || !checklistType) return; Object.entries(patch).forEach(([key, value]) => { - const isValidKey = - (checklistType === 'AMSTAR2' && AMSTAR2_KEY_PATTERN.test(key)) || - (checklistType === 'ROBINS_I' && ROBINS_I_KEYS.has(key)) || - (checklistType === 'ROB2' && ROB2_KEYS.has(key)); - if (isValidKey) { - updateChecklistAnswer(studyId, checklistId, key, value); - } + const input = buildChecklistAnswerInput(checklistType, key, value); + if (!input) return; + updateChecklistAnswer(studyId, checklistId, input); }); }, [isReadOnly, checklistType, updateChecklistAnswer, studyId, checklistId], @@ -374,7 +310,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli
- + {!isReadOnly ?