Apply full-field validation to detail saves

[karilint]

Summary

• Thread optional full-form validation through DetailView context and use it for save/write validation paths.
• Add full-field validators for Reference plus TimeBound, TimeUnit, Person, Museum, Region, Locality, and Species so required fields are checked even when absent from sparse editData.
• Keep entity field validators as the single source of truth by reusing the same validator maps for single-field and full-form validation.
• Fix TimeUnit full-field validation so helper bound ID fields are not required once resolved upper/lower bound objects exist, unblocking the TimeUnit e2e create/save flow.
• Update affected tests and harness mocks for the broader detail validation path.

Closes #1176

Validation

• npm run tsc:frontend
• npm run lint:frontend
• npm run lint:cypress
• npm run test -- --runTestsByPath src/tests/shared/validators/validator.test.ts src/tests/shared/validators/reference.test.ts src/tests/shared/validators/speciesMesowear.test.ts src/tests/components/StagingView.test.tsx src/tests/components/DetailContextDirtyState.test.tsx src/tests/components/LocalitySpeciesTaxonStatusDropdown.test.tsx --runInBand (from frontend)
• npx jest tests/timeUnitForm.test.tsx --runInBand (from frontend)
• commit hook: npm run lint, npm run tsc

Note: I did not run the Cypress spec locally because it calls cy.resetDatabase(); per repo DB safety policy, that needs confirmed isolated test DB settings before execution.

[karilint]

@copilot Please review this pull request.

[Copilot]

@copilot Please review this pull request.

Code Review

Summary

The PR correctly implements full-field validation for all major entity detail views. The core design — threading an optional validateFields function through DetailView props and the DetailContext, and using it in WriteButton for an upfront scan on mode entry — is sound and well-tested. Here are my findings.

---

What works well

field as a non-enumerable property (validator.ts lines 21–28)
Using Object.defineProperty with enumerable: false to carry the field key separately from the display name is a clever design. It lets callers use errorObject.field ?? errorObject.name for key lookup without the field key polluting toEqual assertions in tests.

TimeUnit helper ID fix (timeUnit.ts)
The up_bound/low_bound validators correctly make the required check conditional on the actual object being absent (value === null/undefined), not on the presence of the helper up_bnd/low_bnd IDs. The test at validator.test.ts:67–75 directly verifies this. Logic is correct:

• When up_bound = { bid: 11, age: 4.37 } and low_bound = { bid: 14, age: 4.631 }: value is truthy → required not triggered; miscCheck fires and passes (4.631 > 4.37 ✓).
• When editData = {}: both are undefined → required fires → 'This field is required'.

Error key alignment (02777ab)
Fixing errorObject.name → errorObject.field ?? errorObject.name in components.tsx, StagingView.tsx, ReferenceTab.tsx, and TimeUnitTab.tsx was necessary: without it, an error for field ref_authors (field key) would be stored under ref_authors (display name), mismatching what individual field editors read back from fieldsWithErrors.

Cypress loginWithSession validation
Replacing the cy.visit('/') approach with a localStorage check is faster and avoids triggering a full page load during session cache validation. One non-blocking note: JSON.parse(storedUserState) has no explicit error handler for malformed JSON, but Cypress will surface the thrown error as a test failure in .should(window => { ... }), which is acceptable behaviour for a test harness.

reg_coord_id condition (region.ts)

condition: editData => 'reg_coord_id' in editData,

This correctly skips the required check for new regions where the ID hasn't been assigned yet.

---

One architectural observation (pre-existing, not a regression)

WriteButton useEffect deps: [mode, editData.references] (components.tsx:166)

The full-form validateFields scan runs when mode changes or editData.references changes. It does not re-run when other editData fields change. That means:

1. Entering edit mode → all fields validated ✓
2. User edits field A on the current tab → individual field useEffect (in each editing component) updates fieldsWithErrors[A] in real time ✓
3. User edits field B (on the current tab) which makes field C (on a hidden tab) cross-field-invalid → field C's error is not updated until either mode or editData.references changes ✗

This was also true of the old for (const field in editData) fallback, so this is not a new regression. For most validators the cross-field surface area is small (e.g. min_age/max_age, up_bound/low_bound), and the initial scan on mode entry is the main missing-field use case the PR is targeting. Still worth noting for a future iteration: adding editData to the dep array (with a stable memoised validateFields reference) would close this gap, at the cost of validating on every keystroke.

---

Minor observations

• miscCheck parameter is unused in timeUnit.ts: Both up_bound and low_bound use miscCheck: () => { … } with zero parameters (vs. the declared (obj: object) => ValidationError). They close over editData instead. TypeScript allows this via structural subtyping, and the runtime result is identical (the passed obj is editData.up_bound/editData.low_bound). No action needed, but a comment could improve readability.

• validateFields fallback path is now unreachable for all shipped entities: every DetailView usage in the PR passes validateFields. The old for (const field in editData) branch is dead code for all current entities. It still provides a safe default for any future entity that omits the prop, so keeping it is correct.

• createReferenceValidators captures editData for end_page (reference.ts:231–236): the closure correctly captures the specific editData snapshot at call time (not a stale ref), so end_page >= start_page cross-field validation is consistent with the rest of the form state.

---

Test coverage

• validator.test.ts covers all six new validateXxxFields functions with appropriate missing-field and conditional cases.
• reference.test.ts covers createReferenceFieldsValidatorWithLabels.
• StagingView.test.tsx mock updated to include createReferenceFieldsValidatorWithLabels, keeping the dialog save path testable.
• timeUnitForm.test.tsx e2e: the create/save flow is the main scenario unblocked by the TimeUnit fix.

Overall the implementation is correct and well-tested. The single medium-severity finding (cross-field validation lag on hidden tabs) is pre-existing and the PR does not make it worse.