The Edit Form Pruned Selections It Could Not See

The short version

On detecting a selected ID missing from the monitor list refetched from the delivery system (v2), the edit form automatically pruned it from the selection inside a useEffect and overwrote the state. Open the form intending only to fix a price and save, and if even one monitor moved on the v2 side, the target monitors are finalised in a reduced state with nothing to do with the operator’s intent.

The state it could become

This form is built so that “choosing a school fetches the monitors belonging to it from v2 and shows them as checkboxes”. When editing, it opens with the contracted target monitor IDs already in selected.

Immediately after a successful fetch, the pre-fix code read like this.

if (r.ok) {
  setLoaded({ schoolId, options: r.monitors, error: null });
  // Keep monitors deleted on v2 / belonging to another school out of the selection
  // (leaving them means a 409 hold in v2's cross-boundary validation = a slot that never runs).
  const valid = new Set(r.monitors.map((m) => m.id));
  const kept = selected.filter((id) => valid.has(id));
  if (kept.length !== selected.length) onChange(kept);
}

The intent is as the comment says; this is not malicious code. Leaving an ID missing from the refetched list in the selection makes the later server validation reject the save outright, and this avoids that. But the moment it runs is “immediately after opening the form and fetching the monitor list”, entirely unrelated to whether the operator actually changed the monitor selection. The selected array is silently rewritten the instant the edit form opens.

Why

There are several conditions under which an ID missing from the refetched list appears: the monitor was deleted on v2, it moved to a different school, or the list changed on a retry after a transient fetch error. This code chose to “resolve the divergence between selection and list on the spot, automatically” whenever such a case arose.

But resolving it automatically was effectively a silent write. The form only calls onChange(kept) and shows no warning on screen. From the operator’s point of view, they changed one price field and saved, and the composition of the target monitors changed according to the result of a monitor-list fetch. And the save itself succeeds: because the server has validation that “only monitor IDs that exist in the list may be sent”, every post-pruning ID exists and the validation lets it through as a normal save. The client-side pruning was erasing, before it ever reached validation, exactly the divergence the server-side integrity check was supposed to detect.

Fixing it

We stopped pruning automatically and turned detection into a warning instead.

// IDs in the selection but not in the fetched list = deleted on v2 / another school's monitor
// (rejected on save).
const missingSelected =
  fresh && !error ? selected.filter((id) => !options.some((o) => o.id === id)) : [];

{missingSelected.length > 0 && (
  <>
    {/* No checkbox drawn = it drops out of the submission, which saves "a slot quietly one
        monitor short". Keep submitting it as hidden and let the server reject (fail-closed). */}
    {missingSelected.map((id) => (
      <input key={id} type="hidden" name="target_monitor_ids" value={id} />
    ))}
    <p className="mt-2 rounded bg-amber-50 p-2 text-[11px] text-amber-800">
      選択中の {missingSelected.length}台が v2 の一覧にありません（v2 側で削除された可能性）。
      このままでは保存できません。選び直してください。
    </p>
  </>
)}

IDs missing from the list are not removed from selected; they stay in the submission as hidden inputs. On the server that receives them, validation already exists checking whether each monitor ID is present in the list, and it rejects the whole save if even one is not.

const byId = new Map(toMonitorOptions(r.data.monitors).map((m) => [m.id, m]));
const missing = monitorIds.filter((id) => !byId.has(id));
if (missing.length > 0)
  return {
    ok: false,
    error: `選択したモニタのうち ${missing.length}台がこの学校の v2 モニタとして見つかりません（他校のモニタ、または v2 側で削除された可能性）。選び直してください。`,
  };

That validation is not something this fix added. It existed all along, and never once had the chance to fire because the client removed the divergence before saving. Stopping the client’s helpful auto-correction is what finally let it do its job.

Preventing a repeat

We standardised on the fail-closed policy “on finding a selection missing from the list, warn and stop rather than delete”. The same commit applies the same thinking to the case where connectivity to v2 fails outright (the list returns zero entries, so only saves that keep the existing selection and do not change targeting are allowed).

Client-side logic that “helpfully fixes things automatically” becomes a silent data change whenever the only means of verifying the fix is correct is the operator’s own eyes. Where the server holds the real integrity validation, the client is better off going no further than showing the detected divergence to the user and leaving the choice to them, with the decision to actually reject left to the server’s validation.