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]

onSettled fires on both success and failure, so a rejected PATCH /v1/setup still shows the green check and "All changes saved".

useUpdateSetupStateMutation doesn't set meta.disableDefaultErrorNotification, so the axios interceptor also raises its generic error toast — the user gets a red failure toast and a green "saved" pill at the same time, with the input still displaying a value the server rejected. That's worse than the old explicit Save button, which at least left the state unambiguous.

mutate(data, {
  onError: () => setSaveState('error'),
  onSuccess: () => { setSaveState('saved'); /* …timer… */ }
});

SaveStatus would need an error state, and the duration input should snap back to savedDurationRef.current so what's on screen is what's stored.

[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]

The diagnosis and fix are right — libui's SearchBar renders <form className={...} {...props}> with no onSubmit, so Enter in the input triggers a native submit and reloads the SPA. Good catch, and the comment earns its place.

One consequence to be aware of: handleKeyDown is bound to the showcase's outer container (line ~96), so this now preventDefault()s Enter from every descendant. Previously the filteredInstruments.length === 0 early return let Enter through when the list was empty; now, with no results, pressing Enter while focused on the Kinds/Tags/Languages dropdown triggers is swallowed (preventDefault on keydown suppresses a button's click activation). With ≥1 result the hijack already existed, so this isn't newly broken — but it's the same root cause.

Scoping the handler to the search input would fix the reported bug at its source and stop intercepting Enter for the filter controls and instrument cards:

<SearchBar ... onKeyDown={handleKeyDown} />

with the arrow/Enter logic moved off the container div.

[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 dark variant uses slate tokens but the light variant hardcodes bg-white/95 and border-slate-200/70 instead of bg-background / border-border. This instance supports admin-configured branding (custom primary/secondary colours, eight login themes), so a themed deployment gets a pill that doesn't match anything around it.

Also: this file is added — with the same 26 lines — by #1439, and by #1442 with two extra es strings. Three add/add copies of one component. It's a generic status indicator with no page-specific logic, so it should land once (ideally in whichever PR merges first) and the others should drop their copy.

Since it's position: fixed, one instance hosted at the app layout with the save state in a store would also avoid multiple pills stacking if two autosaving pages are ever mounted together.

[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.