Skip to content

Type currentValues correctly (or persist it into meta) — follow-up to #274 review #680

Description

@dnishiyama

What currentValues is

When someone edits an existing map entity (an event/workout, an AO, or a location), the map's edit form shows the old value struck-through beneath each input so the person can see what they're changing from — e.g. under the "Workout Name" box you see the previous name with a line through it. That "before" snapshot is carried on the request payload as currentValues.

It's an optional field on the edit/move request schemas (EditEventSchema, EditAOAndLocationSchema, MoveAOToNewLocationSchema, MoveAOToDifferentLocationSchema). For an event it mirrors EventFields:

EventFields = {
  eventName:      z.string().min(3, ),
  eventDayOfWeek: z.enum(DayOfWeek),
  eventStartTime: z.string().regex(/^\d{4}$/, ),   // HHmm
  eventEndTime:   z.string().regex(/^\d{4}$/, ),   // HHmm
  eventDescription: z.string().optional(),
  eventTypeIds:   z.array(z.number()).min(1, ),
}

Its lifecycle (created → shown → dropped)

  1. Created (client): apps/map/src/utils/open-request-modal.tsgetFormValues() returns { defaults, currentValues: cur, formValues } (~L512). cur is the entity's existing values, fetched to seed the form.
  2. Shown (client): apps/map/src/app/_components/forms/form-inputs/event-details-form.tsx reads form.watch("currentValues") and renders the struck-through "was: X" hint under each field when the current value differs from the new one (L70–195).
  3. Submitted: it's part of the request body because it's in the schema.
  4. Dropped (server): recordUpdateRequest and the domain handlers never read currentValues (grep: zero references) — it is not persisted. It exists only for the life of the form round-trip. On the admin approve path the "before" state is re-derived, not read from storage.

The problem

Today the field is typed as currentValues: makeSchemaLoose(EventFields).optional(). makeSchemaLoose (request-schemas.ts L254) rewrites every field to z.unknown().optional(), so it validates nothing and gives no real types — its inferred type is { eventName?: unknown; … }. Because of that, event-details-form.tsx has to hand-declare its own currentValues?: Partial<EventFieldsType> (L23) just to get usable types in the component — a manual re-typing that can silently drift from the schema.

The #274 review suggested typing it honestly as EventFields.partial().optional(), which would give real types and let us delete the hand-written Partial<EventFieldsType>.

Why it isn't a one-line change (the tradeoff)

.partial() only makes keys optional — it keeps the per-field validators (.min(3) on eventName, the HHmm regex on times, .min(1) on eventTypeIds).

Since #274, the admin approve endpoint validates its entire input at the boundary:

.input(z.preprocess(normalizeAdminRequestInput, ValidateSubmissionByAdminSchema))

So currentValues would now be validated on approve too. If an entity's prior value is illegal under a newer rule — e.g. a 2-character event name created before the min(3) rule existed, or a legacy start time not in HHmm — the approve would be rejected purely because the historical snapshot fails validation, even though the reviewer isn't touching that field. Honest-but-strict typing turns old data into an approval blocker. That's the regression we deliberately avoided in #274 by leaving it loose + .optional().

Options

  1. Honest typing, validators stripped (shape-only). Type currentValues with real field types but without the .min()/regex refinements — e.g. a makeSchemaShaped(EventFields) helper that maps each field to its base type .optional() (rebuilding the shape, since Zod's .partial() preserves refinements). ✅ Real types, deletes the manual Partial<EventFieldsType>, never rejects old data. ⚠️ Needs the small helper.
  2. Persist currentValues into meta. Store the submitted snapshot at persistence so the reviewer sees exactly what the submitter saw, instead of re-deriving (and possibly showing a blank "before"). ✅ Fixes a real UX gap. ⚠️ Bigger change; decide whether the stored snapshot or the live-derived value is the source of truth; small meta growth.
  3. Leave loose + optional (status quo). ✅ Zero risk. ⚠️ Keeps makeSchemaLoose + the drift-prone hand-written Partial<EventFieldsType>.

Recommendation: Option 1 for the stated goal (honest types, delete the re-declaration, no old-data rejection). Option 2 is a separate, worthwhile UX improvement and can follow — the two aren't mutually exclusive.

Files

  • packages/validators/src/request-schemas.ts — the 4 currentValues: makeSchemaLoose(…) sites (L118, L131, L159, L223), makeSchemaLoose (L254), and the EventFields/AOFields/LocationFields definitions.
  • apps/map/src/app/_components/forms/form-inputs/event-details-form.tsx — hand-written currentValues?: Partial<EventFieldsType> (L23) + the .watch("currentValues") render hints (L30, L70–195); the re-declaration can be deleted once the schema is typed.
  • apps/map/src/utils/open-request-modal.tsgetFormValues builds currentValues (~L512); persistence wiring (for option 2) is on the API side.
  • packages/api/src/router/request.tsnormalizeAdminRequestInput + the boundary z.preprocess(...) (~L851) that would newly validate currentValues on approve — the crux of the tradeoff.

Acceptance criteria

  • Editing an event/AO/location still shows the correct struck-through "before" values in the form (no visual regression).
  • event-details-form.tsx no longer hand-declares Partial<EventFieldsType>; it gets currentValues' type from the schema.
  • Approving a request whose entity holds a legitimately-old value that predates a current validation rule (e.g. a 2-char event name) still succeeds — old data is not an approval blocker.
  • Decision recorded on whether currentValues is now persisted (option 2) or remains review-only (options 1/3).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions