From e8c86bef3b85a4ceb7df022894ea48c11b19ec82 Mon Sep 17 00:00:00 2001
From: iris <iris@hyperhive>
Date: Wed, 27 May 2026 17:13:31 +0200
Subject: [PATCH] tabs: dedup target-chips + interval-parse between new/edit
 schedule forms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

mara: \"look through the code for dedups, structural improvements
and so on\". Two near-identical blocks across the new-schedule and
edit-schedule forms folded into shared helpers.

## `buildTargetChips({ idPrefix, fieldName, checked, extraNames })`

Was inlined twice in 18-line blocks that built the same
`<label class="schedule-field">` + `<div class="schedule-targets">`
+ candidate-list logic (containers + operator + manager). Now one
function, two callers; `extraNames` lets the edit form keep
showing already-active targets that have vanished from the live
container list so the operator can still uncheck them
intentionally.

## `intervalSecondsFromFormData(fd, namePrefix)`

Both submit handlers had the same ~13-line d/h/m/s → total-seconds
parser (with `NaN` propagation on bad input). Pulled into one
helper next to `buildIntervalComposer`; call sites become 3 lines:

  const intervalTotal = intervalSecondsFromFormData(fd, 'interval_');
  if (Number.isNaN(intervalTotal)) { alert(...); return; }
  const interval_seconds = intervalTotal > 0 ? intervalTotal : null;

Net -13 lines, but the bigger win is shape — when (not if) a new
schedule field surfaces, there's one chip-render path + one
interval-parser to thread it through instead of two.

Zero behaviour change. Built clean.
---
 frontend/packages/dashboard/src/tabs.js | 169 +++++++++++-------------
 1 file changed, 78 insertions(+), 91 deletions(-)

diff --git a/frontend/packages/dashboard/src/tabs.js b/frontend/packages/dashboard/src/tabs.js
index dae04d2f..8dcaed8b 100644
--- a/frontend/packages/dashboard/src/tabs.js
+++ b/frontend/packages/dashboard/src/tabs.js
@@ -2104,6 +2104,60 @@ window.marked = marked;
     return { wrapper, getSeconds, fillFromSeconds, setParts, updatePreview };
   }
 
+  // Parse the d/h/m/s sub-fields of a `buildIntervalComposer` instance
+  // out of submitted FormData. Returns total seconds, or NaN if any
+  // field carries a non-integer / negative value — the caller alerts
+  // and bails. Total `0` means "one-shot" at the call site (wire is
+  // `interval_seconds: null`). `namePrefix` matches the prefix passed
+  // to `buildIntervalComposer` (e.g. `interval_` or `edit_interval_`).
+  function intervalSecondsFromFormData(fd, namePrefix) {
+    const part = (suffix, mult) => {
+      const raw = String(fd.get(namePrefix + suffix) || '').trim();
+      if (!raw) return 0;
+      const n = parseInt(raw, 10);
+      if (!Number.isFinite(n) || n < 0) return NaN;
+      return n * mult;
+    };
+    return part('d', 86400) + part('h', 3600) + part('m', 60) + part('s', 1);
+  }
+
+  // Targets multi-select chip box — shared between the new-schedule
+  // form and the edit-schedule form. Same DOM shape, same candidate
+  // list (containers + operator + manager); only the chip element id
+  // prefix and checkbox field name vary. Used to be inlined twice in
+  // near-identical 18-line blocks; consolidated here so a future
+  // change (new chip kind, candidate-list source swap, etc.) lives in
+  // one place. Returns the wrapping `<label class="schedule-field">`
+  // ready to append to the form.
+  function buildTargetChips({ idPrefix, fieldName, checked, extraNames = [] }) {
+    const candidates = ['operator', 'manager'];
+    const containerNames = Array.from(containersState.values())
+      .map((c) => c.name)
+      .filter((n) => n !== 'manager' && n !== 'operator')
+      .sort();
+    for (const n of containerNames) candidates.push(n);
+    // `extraNames` lets the edit form keep showing an already-active
+    // target that's vanished from the live container list (operator's
+    // typo, container destroyed mid-schedule, etc.) so it's still
+    // explicitly uncheckable. New-schedule callers pass `[]`.
+    for (const n of extraNames) if (!candidates.includes(n)) candidates.push(n);
+
+    const label = el('label', { class: 'schedule-field' },
+      el('span', { class: 'schedule-field-label' }, 'targets'));
+    const box = el('div', { class: 'schedule-targets' });
+    for (const name of candidates) {
+      const id_ = idPrefix + name;
+      const cb = el('input', {
+        type: 'checkbox', name: fieldName, value: name, id: id_,
+      });
+      if (checked.has(name)) cb.checked = true;
+      box.append(el('label', { class: 'schedule-target-chip', for: id_ },
+        cb, el('span', {}, name)));
+    }
+    label.append(box);
+    return label;
+  }
+
   function renderScheduleNewForm() {
     const liveRoot = $('schedule-new-section');
     if (!liveRoot) return;
@@ -2115,38 +2169,17 @@ window.marked = marked;
     const carry = readScheduleFormCarry(liveRoot);
     paintAtomic(liveRoot, (root) => {
 
-    // Targets multi-select pulls from the live containers list. The
-    // backend accepts any string (forward-compat for richer recipients),
-    // so we also surface the special `operator` and `manager` names
-    // as fixed options on top.
-    const candidateTargets = ['operator', 'manager'];
-    const containerNames = Array.from(containersState.values())
-      .map((c) => c.name)
-      .filter((n) => n !== 'manager' && n !== 'operator')
-      .sort();
-    for (const n of containerNames) candidateTargets.push(n);
-
     const form_ = el('form', { class: 'schedule-new-form' });
     form_.addEventListener('submit', (e) => {
       e.preventDefault();
       submitNewSchedule(form_);
     });
 
-    const targetsLabel = el('label', { class: 'schedule-field' },
-      el('span', { class: 'schedule-field-label' }, 'targets'));
-    const targetsBox = el('div', { class: 'schedule-targets' });
-    const restoredTargets = new Set(carry.targets);
-    for (const name of candidateTargets) {
-      const id_ = 'st-' + name;
-      const cb = el('input', {
-        type: 'checkbox', name: 'targets', value: name, id: id_,
-      });
-      if (restoredTargets.has(name)) cb.checked = true;
-      targetsBox.append(el('label', { class: 'schedule-target-chip', for: id_ },
-        cb, el('span', {}, name)));
-    }
-    targetsLabel.append(targetsBox);
-    form_.append(targetsLabel);
+    form_.append(buildTargetChips({
+      idPrefix: 'st-',
+      fieldName: 'targets',
+      checked: new Set(carry.targets),
+    }));
 
     const bodyLabel = el('label', { class: 'schedule-field' },
       el('span', { class: 'schedule-field-label' }, 'prompt body'));
@@ -2247,25 +2280,13 @@ window.marked = marked;
     const firstFireDate = new Date(firstFireStr);
     if (Number.isNaN(firstFireDate.getTime())) { alert('first fire is not a valid datetime'); return; }
     const first_fire_at_unix = Math.floor(firstFireDate.getTime() / 1000);
-    // Interval composer (#466): combine d/h/m/s into total seconds.
-    // Negative values are rejected; total == 0 means one-shot, same
-    // semantic the backend expects when interval_seconds is null.
-    const partSecs = (name, mult) => {
-      const raw = String(fd.get(name) || '').trim();
-      if (!raw) return 0;
-      const n = parseInt(raw, 10);
-      if (!Number.isFinite(n) || n < 0) return NaN;
-      return n * mult;
-    };
-    const partD = partSecs('interval_d', 86400);
-    const partH = partSecs('interval_h', 3600);
-    const partM = partSecs('interval_m', 60);
-    const partS = partSecs('interval_s', 1);
-    if ([partD, partH, partM, partS].some((v) => Number.isNaN(v))) {
+    // Interval composer (#466): total seconds from d/h/m/s sub-fields.
+    // `0` means one-shot at the wire level (backend expects null).
+    const intervalTotal = intervalSecondsFromFormData(fd, 'interval_');
+    if (Number.isNaN(intervalTotal)) {
       alert('interval fields must be non-negative integers (or blank for one-shot)');
       return;
     }
-    const intervalTotal = partD + partH + partM + partS;
     const interval_seconds = intervalTotal > 0 ? intervalTotal : null;
     const payload = { targets, body, first_fire_at_unix };
     if (interval_seconds != null) payload.interval_seconds = interval_seconds;
@@ -2513,45 +2534,22 @@ window.marked = marked;
     form_.addEventListener('input', saveCarry);
     form_.addEventListener('change', saveCarry);
 
-    // Targets multi-select (#474 fast-follow). Mirrors the new-schedule
-    // form's chip pattern: live container names + operator + manager,
-    // pre-checked for whichever targets are currently active. Submit
-    // diffs against the original set to populate `targets_add` /
-    // `targets_remove` on the PATCH body. Cancelled tombstones aren't
-    // listed (re-adding them flows through `targets_add`, which the
-    // backend replace-on-conflict drops the tombstone for).
+    // Targets multi-select (#474 fast-follow). Shares the chip-box
+    // pattern with the new-schedule form via `buildTargetChips`;
+    // cancelled tombstones aren't listed (re-adding them flows
+    // through `targets_add`, which the backend replace-on-conflict
+    // drops the tombstone for). Submit diffs against the original
+    // active set to populate `targets_add` / `targets_remove` on the
+    // PATCH body.
     const originalActiveTargets = new Set(
       (s.targets || []).filter((t) => !t.cancelled_at_unix).map((t) => t.target),
     );
-    const targetsLabel = el('label', { class: 'schedule-field' },
-      el('span', { class: 'schedule-field-label' }, 'targets'));
-    const targetsBox = el('div', { class: 'schedule-targets' });
-    const candidateTargets = ['operator', 'manager'];
-    const containerNames = Array.from(containersState.values())
-      .map((c) => c.name)
-      .filter((n) => n !== 'manager' && n !== 'operator')
-      .sort();
-    for (const n of containerNames) candidateTargets.push(n);
-    // Any already-active target that's NOT in the live candidate list
-    // (operator's typo, a container that has since been destroyed,
-    // etc.) still shows so the operator can intentionally drop it.
-    for (const t of originalActiveTargets) {
-      if (!candidateTargets.includes(t)) candidateTargets.push(t);
-    }
-    const restoredTargets = carry.targets
-      ? new Set(carry.targets)
-      : originalActiveTargets;
-    for (const name of candidateTargets) {
-      const id_ = 'se-' + s.id + '-' + name;
-      const cb = el('input', {
-        type: 'checkbox', name: 'edit_targets', value: name, id: id_,
-      });
-      if (restoredTargets.has(name)) cb.checked = true;
-      targetsBox.append(el('label', { class: 'schedule-target-chip', for: id_ },
-        cb, el('span', {}, name)));
-    }
-    targetsLabel.append(targetsBox);
-    form_.append(targetsLabel);
+    form_.append(buildTargetChips({
+      idPrefix: 'se-' + s.id + '-',
+      fieldName: 'edit_targets',
+      checked: carry.targets ? new Set(carry.targets) : originalActiveTargets,
+      extraNames: [...originalActiveTargets],
+    }));
     form_.append(el('p', { class: 'meta schedule-edit-targets-note' },
       're-adding a previously cancelled target drops its history and starts fresh; '
       + 'unchecking an active target cancels it.'));
@@ -2588,22 +2586,11 @@ window.marked = marked;
 
     // Compute interval total from the d/h/m/s fields (composer uses
     // `edit_interval_*` names in this form).
-    const partSecs = (name, mult) => {
-      const raw = String(fd.get(name) || '').trim();
-      if (!raw) return 0;
-      const n = parseInt(raw, 10);
-      if (!Number.isFinite(n) || n < 0) return NaN;
-      return n * mult;
-    };
-    const partD = partSecs('edit_interval_d', 86400);
-    const partH = partSecs('edit_interval_h', 3600);
-    const partM = partSecs('edit_interval_m', 60);
-    const partS = partSecs('edit_interval_s', 1);
-    if ([partD, partH, partM, partS].some((v) => Number.isNaN(v))) {
+    const intervalTotal = intervalSecondsFromFormData(fd, 'edit_interval_');
+    if (Number.isNaN(intervalTotal)) {
       alert('interval fields must be non-negative integers');
       return;
     }
-    const intervalTotal = partD + partH + partM + partS;
     const newIntervalSeconds = intervalTotal > 0 ? intervalTotal : null;
 
     // Compute target diff against the schedule's currently-active set