OpenConceptLab/ocl_issues#2188 | Mapper auto save

[snyaggarwal]

Linked Issue

ClosesOpenConceptLab/ocl_issues#2188

Summary

Main tasks

• Added throttled autosave so eligible changes are saved after a short delay, giving users time to click manual Save first.
• Autosave only runs for saved projects and will not work on unsaved projects.
• Project Logs subtly differentiates manual saves from autosaves.

Autosave runs for:

• AutoMatch completion
• Decision changes
• Bulk decisions
• Review/approval actions
• Configuration changes when the drawer is closed with unsaved changes

Autosave does not run for:

• Manual Map
• AI Analysis
• Candidates Refresh
• Comments
• Review Notes

Test Plan

• Focused map-project tests passed.

[askanter]

As said on the call... I think anything that touches the concept or map should autosave... so even manual maps. However, "environmental" changes such as regenerating candidates perhaps should not. However, as soon as the environment is used to change actual data on a row, then that would generate a whole project save...

[paynejd]

Review — Mapper auto save

Overall a sound, well-scoped implementation that meets the issue's ACs. The ref-mirroring of logs/projectLogs is the right way to read fresh state from a debounced setTimeout, and the config-diff-on-close heuristic is false-negative-safe. One correctness item worth fixing before merge, plus a few minor notes.

Correctness

• ⚠️ Stale isSaving in the debounce closure (main concern).scheduleAutoSave is recreated each render, and its setTimeout callback closes over that render's isSaving, project, and onSave. The reschedule branch calls scheduleAutoSave(reason), which resolves to the same render's binding — so it keeps observing the schedule-time isSaving forever. If a schedule happens while isSaving === true (a save in flight), the timer sees stale true, reschedules, and never observes the save completing → autosave can silently wedge until the next fresh trigger. Recommend backing this with a ref:

  constisSavingRef=React.useRef(false)// wherever you setIsSaving(x): isSavingRef.current = x; setIsSaving(x)// in the timer: if (isSavingRef.current) { scheduleAutoSave(reason); return }

  Same reasoning applies to project?.id read inside the timer — a ref avoids a stale null right after an initial create.

• Double setIsSaving(false) (benign). The .then sets it and the new .finally sets it again on success. Harmless — and the .finally is a real improvement, since isSaving previously could stick true on a rejected save.

• Remaining setConfigure(false) calls (in fetchAndSetProject and the post-save success block) correctly bypass closeConfiguration — those are load/save paths, not a user closing the drawer, so no config-diff autosave should fire. The two user-facing close points and both child setConfigure props were switched to setConfigureWithAutosave/closeConfiguration. 👍

Style

• onSave event-vs-options overload is a bit fragile.const saveOptions = options?.preventDefault ? {} : options sniffs a click SyntheticEvent vs an options object via preventDefault. It works and is guarded, but a thin onManualSave = () => onSave({source: 'manual'}) for the onClick={onSave} bindings would read cleaner. Minor.
• 'Auto Saved' → .toLowerCase() → 'auto saved' matches both ProjectLogs checks — consistent with the other lowercase action comparisons. ✅

Test coverage

No new tests cover the autosave logic — debounce coalescing, project?.id gating, the isSaving reschedule path, or manual-save cancelling a pending autosave. A couple of fake-timer unit tests around scheduleAutoSave would be high-value and would have surfaced the stale-isSaving case.

Performance / security

Negligible perf impact (one debounce timer, torn down on unmount). No security concerns — same authenticated logs/ endpoint and payload shape; the 5s debounce keeps write frequency bounded.

Verdict: Approve modulo the isSaving ref fix (cheap) + ideally a fake-timer test or two.

[paynejd]

Tested the updated PR end-to-end in a signed-in browser (project 127): config-change autosave fires silently with an "Auto saved changes" log entry, a no-op config close does not save, an aborted AutoMatch does not save, and manual save stays distinct ("Updated"). Unit tests 7/7, eslint clean. The extracted createAutosaveScheduler + ref-getters is a clean fix for the stale-isSaving wedge. ✅ Two things to address, rest optional.

🔴 Must fix — rebase on main (blocks login)

This branch predates the OpenConceptLab/ocl_online#131 PKCE commits now required by the prod OIDC. As-is, sign-in fails with invalid_request: Missing parameter: code_challenge_method. A merge of origin/main is clean (no conflicts) and restores login — please rebase before merging.

🐛 Bug — "Created" history line shows a raw user id after any save

After a save reloads the project, the Created entry in Project History flips from the username to the numeric created_by (e.g. jon → 2456), because getTitle reads project.created_by off the PUT response. Autosave makes saves frequent, so users now hit this constantly. Prefer the log's own recorded user — src/components/map-projects/ProjectLogs.jsx:110 (line is outside this PR's diff, so pasting the fix rather than a one-click suggestion):

- created_by: project.created_by || '',+ created_by: log.user || project.created_by || '',

Optional (consider)

• onSave event-vs-options overload — const saveOptions = options?.preventDefault ? {} : options sniffs a click SyntheticEvent. A thin onManualSave = () => onSave({source: 'manual'}) for the onClick={onSave} bindings would read cleaner than guarding inside onSave.
• Double setIsSaving(false) (.then + new .finally) — harmless; keep the .finally, it fixes isSaving sticking true on a rejected save.