feat: configurable default remote assignment validity period

[thomasbeaudry]

Closes #1433

Summary

Admins can now set an instance-wide default number of days that new remote assignments stay valid, replacing the previously hard-coded one-year default. The remote-assignment create dialog seeds its expiry date from this setting, falling back to 365 days when it hasn't been configured.

Changes

• packages/schemas
  • assignment: added DEFAULT_ASSIGNMENT_DURATION_DAYS (365) as the shared fallback.
  • setup: added defaultAssignmentDurationDays (positive int, ≤ MAX_ASSIGNMENT_DURATION_DAYS = 3650) to $SetupState and $UpdateSetupStateData; MAX_ASSIGNMENT_DURATION_DAYS is exported as the single source of truth for the bound.
• apps/api
  • New optional defaultAssignmentDurationDays Int? column on the SetupState Prisma model.
  • getState() returns it; UpdateSetupStateDto accepts it (persisted via the existing updateState).
• apps/web
  • Remote-assignment dialog computes the default "Expires At" from the configured days.
  • Admin Application Settings page consolidated into a single card with titled, separator-divided sections (Features / Settings / Preferences) and autosaves every field (no Save buttons) with a SaveStatus indicator. The duration field autosaves via debounce-on-type, flush-on-blur, and flush-on-unmount so navigating away always persists.
  • Fix: pressing Enter in the instrument search bar with no matching results no longer submits the underlying SearchBar <form> — previously this reloaded the app and bounced the user to the login page (affected both the administer-instruments and remote-assignment pages).
  • Remote-assignment create dialog now autofocuses the search bar, and moves initial focus to the submit button so Enter submits the form instead of focus landing on the date picker.
• Tests
  • Schema unit tests for the new bound.
  • SetupService unit test proving updateState persists the field and getState returns it.
  • Web unit tests: default assignment expiry resolution, the settings duration parser, and the instrument-showcase Enter behaviour (regression test for the login-bounce bug).
  • E2E spec (testing/src/specs/settings.spec.ts) that sets the duration and verifies it persists across reload.

Testing

• pnpm --filter @opendatacapture/web lint (tsc + eslint) — clean.
• New and existing unit tests for the affected schema/api/web code — green.

🤖 Generated with Claude Code

[gdevenyi]

Review — configurable assignment validity period

The core feature is small, well-scoped and correct: the constant/bound live in schemas, the API change is minimal, and the tests are pointed at the right things (parseDurationDays, getDefaultAssignmentExpiry, and the Enter regression). The InstrumentShowcase fix is a genuine bug fix — libui's SearchBar really does render a bare <form> with no onSubmit, so Enter was triggering a native submit and a full page reload. The comment explaining that is exactly the kind AGENTS.md wants.

The issues are mostly in the autosave rewrite, which is a bigger and riskier change than the feature it's carrying.

Should fix

1. A failed save reports success. autosave sets 'saved' in onSettled, which runs on error too. useUpdateSetupStateMutation has no onError, so the global axios interceptor fires a red toast — and the user sees a red "request failed" toast next to a green "All changes saved" pill, with the input still showing the value that wasn't stored. onSuccess/onError instead of onSettled, and revert the input on error.

2. The e2e test can't be run twice. It sets the duration to 45 and asserts on the "All changes saved" pill. On a second run against the same database the value is already 45, so flushDurationOnBlur's parsed !== savedDurationRef.current guard short-circuits, no request is sent, the pill never appears, and the test fails. Even on a clean run the pill self-hides after 2000 ms, so it's a race on a slow runner. The uniqueId fixture exists for exactly this.

3. The autosave state machine itself is untested. Debounce-on-type, flush-on-blur, flush-on-unmount, revert-on-invalid, and resync-when-the-server-value-changes are five interacting behaviours and the place bugs will actually live — but the three new unit tests cover two pure helpers and one keydown. parseDurationDays in isolation doesn't tell you the field saves when you navigate away.

4. Route modules now export test-only helpers. export { parseDurationDays } and export { getDefaultAssignmentExpiry } exist only so the tests can import them. Moving them to @/utils keeps route files exporting just Route, and getDefaultAssignmentExpiry in particular is domain logic that has nothing to do with routing.

Worth considering

5. MAX_ASSIGNMENT_DURATION_DAYS lives in schemas/setup while DEFAULT_ASSIGNMENT_DURATION_DAYS lives in schemas/assignment, and settings.tsx imports from both to render one field. They're two halves of one rule.

6. Focusing the dialog's submit button on open is an accessibility regression and makes a stray Enter create an assignment with the default expiry — see the inline note.

7. SaveStatus hardcodes bg-white/95 / border-slate-200/70 rather than theme tokens, so it won't follow a branded instance's palette.

8. The spec sets both test.use({ actingRole: 'ADMIN' }) and calls authenticateAs('ADMIN'); actingRole only feeds getPageModel, which this spec doesn't use. One of the two is dead.

Coordination with the other open PRs

This branch collides with two of the others, which GitHub won't show because it compares each against main independently:

Conflict	Files
#1441 × #1442	apps/web/src/routes/_app/admin/settings.tsx (both rewrite the same Toggle/handleSave block into incompatible autosave layouts) and apps/web/src/components/SaveStatus.tsx (add/add)
#1441 × #1439	apps/web/src/routes/_app/session/remote-assignment.tsx — #1439 deletes the entire libui <Form> this PR modifies, replacing it with a hand-rolled free-text date field. Both PRs also add the identical document.querySelector('[data-testid="instrument-search-bar"] input') autofocus effect.

Worth noting that this PR's version of autosave is the correct one: it destructures mutate and depends on [mutate] (with a comment explaining why), whereas #1442 depends on [updateSetupStateMutation], which React Query recreates every render. If these get reconciled, keep this one.

[gdevenyi]

Since this is the validation boundary and is unit-tested as such, Number() is looser than the tests imply: parseDurationDays('0x10') → 16, '1e3' → 1000, '+45' → 45, ' 45 ' → 45. The <input type="number"> makes most of those unreachable in practice, but the function is exported and tested as the rule, so the rule should be the one you mean:

const parseDurationDays = (raw: string): null | number => {
  if (!/^\d+$/.test(raw.trim())) return null;
  const parsed = Number(raw);
  return parsed >= 1 && parsed <= MAX_ASSIGNMENT_DURATION_DAYS ? parsed : null;
};

Also worth adding '0x10' / '1e3' to the rejection table so the intent is pinned.

[gdevenyi]

A trailing export appended after Route purely so the test can reach it. getDefaultAssignmentExpiry in remote-assignment.tsx does the same.

Both are pure functions with no routing involvement — @/utils/assignment-duration.ts (or similar) would let the tests import them directly, keep route modules exporting only Route, and let settings.tsx and remote-assignment.tsx share the day-count logic instead of each owning half of it.

[gdevenyi]

This assertion makes the spec non-repeatable and racy.

Non-repeatable: the test writes 45 and never restores it. On the next run the stored value is already 45, so flushDurationOnBlur's parsed !== savedDurationRef.current guard skips the mutation entirely — no request, no pill, test fails. That also breaks the "independent, self-seeding tests" property established in #1429.

Racy: SaveStatus reverts to idle 2000 ms after the save settles. Playwright's auto-retry doesn't help if the whole window closes before the first poll on a loaded runner.

Suggest asserting only on the durable outcome, and varying the value:

const days = 30 + (Number(uniqueId.replace(/\D/g, '').slice(-2)) % 60);
await settingsPage.setDefaultAssignmentDuration(days);
await page.reload();
await expect(settingsPage.defaultAssignmentDurationInput).toHaveValue(String(days));

The reload already proves the save happened; the pill adds a timing dependency for no extra coverage. (Also: this instance-wide write leaks into any other spec that reads the setting — worth restoring it in an afterEach.)

[gdevenyi]

Two concerns with moving initial focus to the submit button.

Accessibility: a screen-reader or keyboard user opening this dialog now lands on "Submit" rather than the dialog's heading or first control, so they never hear the title or the expiry field unless they shift-tab backwards. Radix's default (focus the first focusable element) is the accessible behaviour.

Accidental submission: combined with InstrumentShowcase's Enter-to-select, one Enter opens the dialog and the next creates a real remote assignment with the default expiry. Assignments are user-visible artefacts with live URLs, so an accidental one isn't free.

If the goal is "Enter submits rather than opening the date picker", focusing the expiry input achieves that too and keeps context. querySelector('button[type="submit"]') is also a DOM reach into libui's internals — a ref on the submit button would survive a libui restructure.

[gdevenyi]

This works — libui's SearchBar spreads data-testid onto its <form> and renders the <input> inside — but it couples a route component to a test attribute and to libui's internal DOM shape, and fails silently if either changes.

InstrumentShowcase already owns the input; an autoFocus prop on it (or a forwarded ref) makes the intent explicit and type-checked.

Note #1439 adds this exact effect to this exact file as well.

[gdevenyi]

The bound lives here but the fallback (DEFAULT_ASSIGNMENT_DURATION_DAYS = 365) lives in schemas/assignment, so settings.tsx imports from two modules to render one input and one hover-card. They're the min/max/default of a single rule.

Since the value is persisted on SetupState and validated by $SetupState/$UpdateSetupStateData, both constants belong here; schemas/assignment can re-export if the assignment side wants it under that name.

[joshunrau]

CI is red on this branch, so I have not reviewed it yet.

Failing check: lint-and-test — run 30070326044

The unit tests and lint pass (33/33 turbo tasks). The e2e suite fails on one spec, the one this PR adds:

[firefox] › src/specs/settings.spec.ts:7:3 › application settings ›
  should persist the default assignment duration @smoke

Error: expect(locator).toBeVisible() failed
  Locator: getByText('All changes saved')
  Expected: visible
  Timeout: 15000ms
  Error: element(s) not found

  > 15 |     await expect(page.getByText('All changes saved')).toBeVisible();

33 of 34 e2e tests passed; this was the only failure.

Please get that spec green and push. I'll pick the review back up then.

[joshunrau]

Suggested model: Fable 5

Sizing the remaining work on this PR for whoever picks it up, human or agent. It touches apps/api/prisma/schema.prisma, which is an automatic escalation, and spans 17 files across four workspaces. The e2e failure also needs diagnosing rather than just re-running — unlike the other red branches here, this one is a genuine failure of the spec this PR adds.

[joshunrau]

Requesting changes so this tracks as needing author action.

Blocking: lint-and-test is failing on a genuine error — the e2e spec this PR adds. Details in my earlier comment: settings.spec.ts:7 "should persist the default assignment duration" times out waiting for getByText('All changes saved'). 33 of 34 e2e tests pass; this is the only failure.

No content review has been done on this branch yet — it's gated on CI being green. That starts once the spec passes.

One thing to know before you rebase: this PR overlaps #1442 on six files and #1439 on five, and all three independently add apps/web/src/components/SaveStatus.tsx, which doesn't exist on main. Ownership of the shared files is being settled; hold off restructuring anything in apps/api/src/setup/, packages/schemas/src/setup/setup.ts or SaveStatus.tsx until we confirm.

[joshunrau]

CI is red on this branch, so I have not reviewed it yet.

The lint-and-test job fails in the end-to-end suite:

[firefox] › src/specs/settings.spec.ts:7:3 › application settings ›
  should persist the default assignment duration @smoke

Error: expect(locator).toBeVisible() failed
Error: element(s) not found

> 15 |     await expect(page.getByText('All changes saved')).toBeVisible();

The spec this PR adds waits for the text All changes saved after saving the default assignment duration, and that text never appears. Please get the job green — either the save indicator needs to render that text, or the spec needs to assert on whatever the settings page actually shows on a successful save.

Review resumes automatically once CI passes.

Reviewed at commit bf54846.

[joshunrau]

Thanks — the feature works and the search-bar Enter fix is a genuinely good catch. Lint is clean and the unit tests pass. A few things before this goes in.

Good news on scope: the autosave redesign of the settings page is approved and stays in this PR, so nothing needs unwinding there. The new Prisma column still needs Joshua's sign-off, which is the one thing outstanding on his side. Everything below is yours — and note that items 2, 3 and 8 are exactly the autosave machinery being kept, so they matter more now, not less:

1. apps/web/src/utils/assignment-duration.ts:7 — parseDurationDays hand-writes the same rule $DefaultAssignmentDurationDays already encodes in packages/schemas/src/setup/setup.ts:183 (integer, >= 1, <= max). Export the schema from packages/schemas and make the parser a safeParse of Number(raw) over it, so there is one definition rather than two that have to stay in step.
2. apps/web/src/routes/_app/admin/settings.tsx:67 — the save indicator flips to "All changes saved" in onSettled, so it reports success when the PATCH fails. Move it to onSuccess and give failure its own state.
3. apps/web/src/routes/_app/admin/settings.tsx:116 — the unmount flush calls mutate from a component that is unmounting, so React Query's global throwOnError has nowhere to throw and a failed save disappears silently while the admin has already navigated away. Pass an explicit onError that surfaces it.
4. apps/web/src/routes/_app/session/remote-assignment.tsx:98 — the document.querySelector('[data-testid="instrument-search-bar"] input') autofocus effect is a verbatim copy of the one in accessible-instruments.tsx:20, and it drives focus by test id from outside React. Please give InstrumentShowcase an autoFocus prop that focuses its own SearchBar via a ref, and use it at both call sites.
5. apps/web/src/routes/_app/session/remote-assignment.tsx:133 — onOpenAutoFocus finds the submit button with querySelector; use a ref. And could you say why focus should start on Submit? As written a keyboard or screen-reader user opens the dialog one Enter away from creating an assignment, having never reached the "Expires At" field.
6. There is no end-to-end test for the actual feature. testing/src/specs/settings.spec.ts proves the settings input persists, but nothing checks that the create-assignment dialog's "Expires At" reflects the configured duration — which is what #1433 asked for. Please add that.
7. testing/src/specs/settings.spec.ts:8 — the @smoke tag is what makes this spec run in two browsers against one shared instance-wide value, which is the collision the uniqueId-derived duration works around. testing/AGENTS.md reserves @smoke for critical flows; dropping it here removes the problem and the workaround.
8. apps/web/src/components/SaveStatus.tsx:12 — bg-white/95, border-slate-200/70, dark:bg-slate-800/95 and text-green-600 are raw colour scales; apps/web/AGENTS.md asks for semantic libui tokens (bg-popover, border-border).

If you hand this to Claude Code, Fable 5 is the right size — the work spans the Prisma schema, the shared Zod contract, two web routes and the e2e suite at once.

Reviewed at commit d1359af.