From 49a011ce9592c9719137069267d654070c2d8211 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 16 Jul 2026 12:18:18 +0530 Subject: [PATCH 1/4] OpenConceptLab/ocl_issues#2188 | auto save --- src/components/map-projects/MapProject.jsx | 195 ++++++++++++++++---- src/components/map-projects/ProjectLogs.jsx | 4 + src/i18n/locales/en/translations.json | 1 + src/i18n/locales/es/translations.json | 1 + src/i18n/locales/zh/translations.json | 1 + 5 files changed, 168 insertions(+), 34 deletions(-) diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index 0b77f48..e9f5f1c 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -141,6 +141,7 @@ const MapProject = () => { const queryParams = new URLSearchParams(location.search) return queryParams.get('templateFrom') }, [location.search]) + const AUTOSAVE_DELAY_MS = 5000 const bridgeRef = React.useRef() const facetsRequestsRef = React.useRef({}) @@ -229,6 +230,11 @@ const MapProject = () => { const [columnWidth, setColumnWidth] = React.useState({}) const [logs, setLogs] = React.useState({}) const [projectLogs, setProjectLogs] = React.useState([]) + const logsRef = React.useRef({}) + const projectLogsRef = React.useRef([]) + const autosaveTimerRef = React.useRef(null) + const autosaveReasonsRef = React.useRef([]) + const configSnapshotOnOpenRef = React.useRef(null) const [filterModel, setFilterModel] = React.useState({ items: [] }); const [filterPanelAnchorEl, setFilterPanelAnchorEl] = React.useState(null) const [retired, setRetired] = React.useState(false) @@ -250,6 +256,19 @@ const MapProject = () => { setAlert(false) }, []) + React.useEffect(() => { + logsRef.current = logs + }, [logs]) + + React.useEffect(() => { + projectLogsRef.current = projectLogs + }, [projectLogs]) + + React.useEffect(() => () => { + if(autosaveTimerRef.current) + clearTimeout(autosaveTimerRef.current) + }, []) + // repo state const [repo, setRepo] = React.useState(false) const [repoVersion, setRepoVersion] = React.useState(false) @@ -666,8 +685,12 @@ const MapProject = () => { setFilters(response.data?.filters || {}) if(response.data?.url) { APIService.new().overrideURL(response.data.url).appendToUrl('logs/').get().then(response => { - setLogs(response.data.logs?.row_logs || []) - setProjectLogs(response.data.logs?.project_logs || []) + const rowLogs = response.data.logs?.row_logs || [] + const savedProjectLogs = response.data.logs?.project_logs || [] + logsRef.current = rowLogs + projectLogsRef.current = savedProjectLogs + setLogs(rowLogs) + setProjectLogs(savedProjectLogs) projectLog({action: 'Opened'}) }) } @@ -1365,7 +1388,17 @@ const MapProject = () => { }) } - const onSave = () => { + const onSave = (options = {}) => { + const saveOptions = options?.preventDefault ? {} : options + const saveSource = saveOptions.source || 'manual' + const isAutoSave = saveSource === 'auto' + if(isAutoSave && !project?.id) + return + if(!isAutoSave && autosaveTimerRef.current) { + clearTimeout(autosaveTimerRef.current) + autosaveTimerRef.current = null + autosaveReasonsRef.current = [] + } if(!repoVersion?.version_url || !selectedTargetRepoVersion) { setConfigure(true) setAlert({ @@ -1378,6 +1411,8 @@ const MapProject = () => { }) return } + const rowLogsForSave = saveOptions.logs || logsRef.current + const projectLogsForSave = saveOptions.projectLogs || projectLogsRef.current setIsSaving(true) const f = getFileObjectFromRows() const selected = map(mapSelected, (data, i) => { @@ -1459,30 +1494,70 @@ const MapProject = () => { service.then(response => { setIsSaving(false) if(response?.data?.id) { - projectLog({action: isUpdate ? 'Updated' : 'Created', extras: isUpdate ? undefined : {project: response.data}}) - setConfigure(false) + const saveLog = { + action: isAutoSave ? 'Auto Saved' : (isUpdate ? 'Updated' : 'Created'), + description: isAutoSave ? t('map_project.auto_saved_changes') : undefined, + created_at: moment().toDate(), + user: user.username || user.id, + extras: isAutoSave ? {reasons: saveOptions.reasons || []} : (isUpdate ? undefined : {project: response.data}) + } + const savedProjectLogs = [saveLog, ...projectLogsForSave] + projectLogsRef.current = savedProjectLogs + setProjectLogs(savedProjectLogs) + if(saveOptions.closeConfigure !== false) { + configSnapshotOnOpenRef.current = null + setConfigure(false) + } setProjectPromptTemplateKey(response.data?.prompt_template_key || getProjectPromptTemplateKey()) setProject(response.data) if(response.data.url) history.push(response.data.url) - baseSetAlert({severity: 'success', message: t('map_project.successfully_saved'), duration: 2000}) + if(!isAutoSave) + baseSetAlert({severity: 'success', message: t('map_project.successfully_saved'), duration: 2000}) - APIService.new().overrideURL(response.data.url).appendToUrl('logs/').post({logs: {row_logs: logs, project_logs: projectLogs}}).then(() => {}) + APIService.new().overrideURL(response.data.url).appendToUrl('logs/').post({logs: {row_logs: rowLogsForSave, project_logs: savedProjectLogs}}).then(() => {}) } - }) + }).finally(() => setIsSaving(false)) } const log = (data, index) => { let idx = index === undefined ? rowIndex : index - setLogs(prev => ({...prev, [idx]: [{created_at: moment().toDate(), user: user.username || user.id, ...data}, ...(prev[idx] || [])]})) + const nextLogs = {...logsRef.current, [idx]: [{created_at: moment().toDate(), user: user.username || user.id, ...data}, ...(logsRef.current[idx] || [])]} + logsRef.current = nextLogs + setLogs(nextLogs) } const projectLog = data => { const newLog = {...data, created_at: moment().toDate(), user: user.username || user.id} - const newLogs = [newLog, ...projectLogs] - setProjectLogs(prev => [newLog, ...prev]) + const newLogs = [newLog, ...projectLogsRef.current] + projectLogsRef.current = newLogs + setProjectLogs(newLogs) if(project?.url) - APIService.new().overrideURL(project.url).appendToUrl('logs/').post({logs: {row_logs: logs, project_logs: newLogs}}).then(() => {}) + APIService.new().overrideURL(project.url).appendToUrl('logs/').post({logs: {row_logs: logsRef.current, project_logs: newLogs}}).then(() => {}) + } + + const scheduleAutoSave = reason => { + if(!project?.id) + return + + autosaveReasonsRef.current = uniq([...autosaveReasonsRef.current, reason]) + if(autosaveTimerRef.current) + clearTimeout(autosaveTimerRef.current) + + autosaveTimerRef.current = setTimeout(() => { + autosaveTimerRef.current = null + if(!project?.id) { + autosaveReasonsRef.current = [] + return + } + if(isSaving) { + scheduleAutoSave(reason) + return + } + const reasons = autosaveReasonsRef.current + autosaveReasonsRef.current = [] + onSave({source: 'auto', closeConfigure: false, reasons}) + }, AUTOSAVE_DELAY_MS) } // ── ocl_online#105 Phase 5: AutomatchRun attribution ────────────────────── @@ -1619,6 +1694,53 @@ const MapProject = () => { return includeDefaultFilter ? allFilters : omit(allFilters, Object.keys(defaultFilters)) } + const getConfigurationSnapshot = () => JSON.stringify({ + owner, + name, + description, + repo_url: repo?.url, + repo_version_url: repoVersion?.version_url, + algorithms: map(algosSelected, algo => omit(algo, ['__key'])), + score_configuration: candidatesScore, + lookup_config: lookupConfig, + namespace, + encoder_model: encoderModel || DEFAULT_ENCODER_MODEL, + include_retired: retired, + filters: getFilters(), + prompt_template_key: getProjectPromptTemplateKey(), + prompt_output_locale: promptOutputLocale || '', + input_locale: inputLocale || '', + use_lexical_variants: Boolean(useLexicalVariants), + columns: map(columns, col => ({ + dataKey: col.dataKey, + mapped: col.mapped, + hidden: columnVisibilityModel[col.dataKey] === false, + width: columnWidth[col.dataKey] || undefined, + ai_assistant_hidden: AIAssistantColumns[col.dataKey] === false + })) + }) + + const closeConfiguration = () => { + const openedSnapshot = configSnapshotOnOpenRef.current + const changed = openedSnapshot && openedSnapshot !== getConfigurationSnapshot() + setConfigure(false) + configSnapshotOnOpenRef.current = null + if(changed) + scheduleAutoSave('configuration_change') + } + + const setConfigureWithAutosave = nextConfigure => { + if(nextConfigure) + setConfigure(true) + else + closeConfiguration() + } + + React.useEffect(() => { + if(configure && !configSnapshotOnOpenRef.current) + configSnapshotOnOpenRef.current = getConfigurationSnapshot() + }, [configure]) + const getPayloadForMatching = (rows, _repo, _filters) => { return { rows: map(rows, row => prepareRow(row)), @@ -1984,6 +2106,8 @@ const MapProject = () => { } : {}) } }) + if(!abortRef.current) + scheduleAutoSave('auto_match') } finally { await completeAutomatchRun(_selectedAlgos, rowsToProcess) } @@ -2644,7 +2768,7 @@ const MapProject = () => { conceptCacheRef.current = next setConceptCache(next) }) - setConfigure(false) + closeConfiguration() setShowProjectLogs(false) setRow(csvRow) setSearchStr(getRowNameValue(csvRow) || '') @@ -2721,6 +2845,7 @@ const MapProject = () => { const newRowStatuses = {...rowStatuses, reviewed: uniq([...rowStatuses.reviewed, rowIndex]), readyForReview: without(rowStatuses.readyForReview, rowIndex), unmapped: without(rowStatuses.unmapped, rowIndex)} setRowStatuses(newRowStatuses) log({'action': 'approved'}) + scheduleAutoSave('decision_change') if(next){ const nextRow = data[selectedRowStatus === 'all' ? rowIndex + 1 : find(rowStatuses[selectedRowStatus], idx => idx > rowIndex)] if(nextRow !== undefined) @@ -2813,6 +2938,7 @@ const MapProject = () => { }) if(newValue !== 'map' && !logged) log({action: newValue || 'decision_changed', description: t('map_project.decision_changed_to_none'), extras: newValue ? {} : {decision: t('map_project.none')}}) + scheduleAutoSave('decision_change') } const getBulkActionLabel = action => { @@ -2828,25 +2954,24 @@ const MapProject = () => { const addBulkLogs = (indexes, action, description, extras = {}) => { const createdAt = moment().toDate() - setLogs(prev => { - const next = {...prev} - indexes.forEach(index => { - const resolvedDescription = typeof description === 'function' ? description(index) : description - const resolvedExtras = typeof extras === 'function' ? extras(index) : extras - next[index] = [{ - created_at: createdAt, - user: user.username || user.id, - action, - description: resolvedDescription, - extras: { - bulk_origin: 'mapper-left-panel-selection', - bulk_action: action, - ...resolvedExtras - } - }, ...(next[index] || [])] - }) - return next + const next = {...logsRef.current} + indexes.forEach(index => { + const resolvedDescription = typeof description === 'function' ? description(index) : description + const resolvedExtras = typeof extras === 'function' ? extras(index) : extras + next[index] = [{ + created_at: createdAt, + user: user.username || user.id, + action, + description: resolvedDescription, + extras: { + bulk_origin: 'mapper-left-panel-selection', + bulk_action: action, + ...resolvedExtras + } + }, ...(next[index] || [])] }) + logsRef.current = next + setLogs(next) } const getSelectedRowIndexes = (_rows = data) => { @@ -3006,6 +3131,8 @@ const MapProject = () => { duration: 4, message: t('map_project.bulk_action_summary', {changed: changed.length, skipped: skipped.length}) }) + if(changed.length) + scheduleAutoSave('decision_change') clearBulkSelection() } @@ -4688,7 +4815,7 @@ const MapProject = () => { isValidColumnValue={isValidColumnValue} updateColumn={updateColumn} configure={configure} - setConfigure={setConfigure} + setConfigure={setConfigureWithAutosave} columnVisibilityModel={columnVisibilityModel} setColumnVisibilityModel={setColumnVisibilityModel} onSave={onSave} @@ -4895,14 +5022,14 @@ const MapProject = () => { onProjectLogsClick={() => { const newValue = !showProjectLogs if(newValue) { - setConfigure(false) + closeConfiguration() onCloseDecisions() } setShowProjectLogs(newValue) }} isProjectsLogOpen={showProjectLogs} configure={configure} - setConfigure={setConfigure} + setConfigure={setConfigureWithAutosave} onCopyClick={onCopyClick} /> } diff --git a/src/components/map-projects/ProjectLogs.jsx b/src/components/map-projects/ProjectLogs.jsx index f21f4bf..93d4695 100644 --- a/src/components/map-projects/ProjectLogs.jsx +++ b/src/components/map-projects/ProjectLogs.jsx @@ -36,6 +36,8 @@ const ProjectLogs = ({onClose, logs, project}) => { return [, 'primary'] if(action === 'updated') return [, 'warning'] + if(action === 'auto saved') + return [, 'primary'] if(action === 'saved_to_collection') return [, 'primary'] if(action === 'opened') @@ -101,6 +103,8 @@ const ProjectLogs = ({onClose, logs, project}) => { /> } + if(log?.action?.toLowerCase() === 'auto saved') + return log.description || t('map_project.auto_saved_changes') if(log.description) return log.description return startCase(log.action) diff --git a/src/i18n/locales/en/translations.json b/src/i18n/locales/en/translations.json index b3ca22c..9d031ea 100644 --- a/src/i18n/locales/en/translations.json +++ b/src/i18n/locales/en/translations.json @@ -595,6 +595,7 @@ "sort_by_unified_score": "Unified Score", "project_logs": "Project History", "project_logs_tooltip": "Project History", + "auto_saved_changes": "Auto saved changes", "saved_to_collection": "Saved to collection", "bridge_terminology_search": "Bridge terminology search<0>Premium", "bridge_terminology_search_description": "Include mappings in the <0>CIEL Interface Terminology to identify additional high quality candidates. Only available for compatible target repositories and matching algorithms.", diff --git a/src/i18n/locales/es/translations.json b/src/i18n/locales/es/translations.json index ac4c1ea..9fa4ff2 100644 --- a/src/i18n/locales/es/translations.json +++ b/src/i18n/locales/es/translations.json @@ -565,6 +565,7 @@ "sort_by_unified_score": "Puntuación Unificada", "project_logs": "Registros del proyecto", "project_logs_tooltip": "Ver registros del proyecto", + "auto_saved_changes": "Cambios guardados automáticamente", "saved_to_collection": "Guardado en la colección", "bridge_terminology_search": "Búsqueda de terminología Bridge<0>Premium", "bridge_terminology_search_description": "Incluir mapeos en la <0>Terminología de Interfaz CIEL para identificar candidatos adicionales de alta calidad. Disponible solo para repositorios de destino y algoritmos de coincidencia compatibles.", diff --git a/src/i18n/locales/zh/translations.json b/src/i18n/locales/zh/translations.json index 1c1c555..9d92c37 100644 --- a/src/i18n/locales/zh/translations.json +++ b/src/i18n/locales/zh/translations.json @@ -590,6 +590,7 @@ "sort_by_unified_score": "按统一分数排序", "project_logs": "项目日志", "project_logs_tooltip": "查看项目日志", + "auto_saved_changes": "已自动保存更改", "saved_to_collection": "已保存到集合", "bridge_terminology_search": "Bridge 术语搜索<0>高级版", "bridge_terminology_search_description": "包含 <0>CIEL 接口术语 中的映射，以识别更多高质量候选项。仅适用于兼容的目标仓库和匹配算法。", From b5baf7130834e853796d99ac896a733694ed7f9b Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 3 Sep 2026 12:27:36 +0530 Subject: [PATCH 2/4] OpenConceptLab/ocl_issues#2188 | review feedback --- src/components/map-projects/MapProject.jsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index e9f5f1c..ba3e049 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -235,6 +235,8 @@ const MapProject = () => { const autosaveTimerRef = React.useRef(null) const autosaveReasonsRef = React.useRef([]) const configSnapshotOnOpenRef = React.useRef(null) + const isSavingRef = React.useRef(false) + const projectIdRef = React.useRef(null) const [filterModel, setFilterModel] = React.useState({ items: [] }); const [filterPanelAnchorEl, setFilterPanelAnchorEl] = React.useState(null) const [retired, setRetired] = React.useState(false) @@ -264,6 +266,14 @@ const MapProject = () => { projectLogsRef.current = projectLogs }, [projectLogs]) + React.useEffect(() => { + isSavingRef.current = isSaving + }, [isSaving]) + + React.useEffect(() => { + projectIdRef.current = project?.id + }, [project]) + React.useEffect(() => () => { if(autosaveTimerRef.current) clearTimeout(autosaveTimerRef.current) @@ -1537,7 +1547,7 @@ const MapProject = () => { } const scheduleAutoSave = reason => { - if(!project?.id) + if(!projectIdRef.current) return autosaveReasonsRef.current = uniq([...autosaveReasonsRef.current, reason]) @@ -1546,11 +1556,11 @@ const MapProject = () => { autosaveTimerRef.current = setTimeout(() => { autosaveTimerRef.current = null - if(!project?.id) { + if(!projectIdRef.current) { autosaveReasonsRef.current = [] return } - if(isSaving) { + if(isSavingRef.current) { scheduleAutoSave(reason) return } From ffe19f099a36d74003e8e43bea223350332ad827 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 3 Sep 2026 12:37:54 +0530 Subject: [PATCH 3/4] OpenConceptLab/ocl_issues#2188 | autosave tests --- src/components/map-projects/MapProject.jsx | 53 +++--- .../map-projects/__tests__/autosave.test.js | 168 ++++++++++++++++++ src/components/map-projects/autosave.js | 67 +++++++ 3 files changed, 255 insertions(+), 33 deletions(-) create mode 100644 src/components/map-projects/__tests__/autosave.test.js create mode 100644 src/components/map-projects/autosave.js diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index ba3e049..308637a 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -87,6 +87,7 @@ import ConfigurationForm from './ConfigurationForm' import Controls from './Controls' import DataGridControls from './DataGridControls' import { getRowsToProcess } from './autoMatchRows' +import { createAutosaveScheduler } from './autosave' import MatchSummaryCard from './MatchSummaryCard' import MappingDecisionResult from './MappingDecisionResult' import DecisionSelector from './DecisionSelector' @@ -141,7 +142,6 @@ const MapProject = () => { const queryParams = new URLSearchParams(location.search) return queryParams.get('templateFrom') }, [location.search]) - const AUTOSAVE_DELAY_MS = 5000 const bridgeRef = React.useRef() const facetsRequestsRef = React.useRef({}) @@ -232,11 +232,17 @@ const MapProject = () => { const [projectLogs, setProjectLogs] = React.useState([]) const logsRef = React.useRef({}) const projectLogsRef = React.useRef([]) - const autosaveTimerRef = React.useRef(null) - const autosaveReasonsRef = React.useRef([]) const configSnapshotOnOpenRef = React.useRef(null) const isSavingRef = React.useRef(false) const projectIdRef = React.useRef(null) + const onSaveRef = React.useRef(null) + const autosaveSchedulerRef = React.useRef(null) + if(!autosaveSchedulerRef.current) + autosaveSchedulerRef.current = createAutosaveScheduler({ + getProjectId: () => projectIdRef.current, + isSaving: () => isSavingRef.current, + onFire: reasons => onSaveRef.current({source: 'auto', closeConfigure: false, reasons}) + }) const [filterModel, setFilterModel] = React.useState({ items: [] }); const [filterPanelAnchorEl, setFilterPanelAnchorEl] = React.useState(null) const [retired, setRetired] = React.useState(false) @@ -274,9 +280,15 @@ const MapProject = () => { projectIdRef.current = project?.id }, [project]) + // Autosave fires long after it was scheduled, so it must call the current + // onSave — an older render's closure would build the payload from state + // captured when the change happened rather than what is on screen now. + React.useEffect(() => { + onSaveRef.current = onSave + }) + React.useEffect(() => () => { - if(autosaveTimerRef.current) - clearTimeout(autosaveTimerRef.current) + autosaveSchedulerRef.current.cancel() }, []) // repo state @@ -1404,11 +1416,8 @@ const MapProject = () => { const isAutoSave = saveSource === 'auto' if(isAutoSave && !project?.id) return - if(!isAutoSave && autosaveTimerRef.current) { - clearTimeout(autosaveTimerRef.current) - autosaveTimerRef.current = null - autosaveReasonsRef.current = [] - } + if(!isAutoSave) + autosaveSchedulerRef.current.cancel() if(!repoVersion?.version_url || !selectedTargetRepoVersion) { setConfigure(true) setAlert({ @@ -1546,29 +1555,7 @@ const MapProject = () => { APIService.new().overrideURL(project.url).appendToUrl('logs/').post({logs: {row_logs: logsRef.current, project_logs: newLogs}}).then(() => {}) } - const scheduleAutoSave = reason => { - if(!projectIdRef.current) - return - - autosaveReasonsRef.current = uniq([...autosaveReasonsRef.current, reason]) - if(autosaveTimerRef.current) - clearTimeout(autosaveTimerRef.current) - - autosaveTimerRef.current = setTimeout(() => { - autosaveTimerRef.current = null - if(!projectIdRef.current) { - autosaveReasonsRef.current = [] - return - } - if(isSavingRef.current) { - scheduleAutoSave(reason) - return - } - const reasons = autosaveReasonsRef.current - autosaveReasonsRef.current = [] - onSave({source: 'auto', closeConfigure: false, reasons}) - }, AUTOSAVE_DELAY_MS) - } + const scheduleAutoSave = reason => autosaveSchedulerRef.current.schedule(reason) // ── ocl_online#105 Phase 5: AutomatchRun attribution ────────────────────── // Build the X-OCL-Request-Source + X-OCL-Event-Metadata headers for a single diff --git a/src/components/map-projects/__tests__/autosave.test.js b/src/components/map-projects/__tests__/autosave.test.js new file mode 100644 index 0000000..2e9e803 --- /dev/null +++ b/src/components/map-projects/__tests__/autosave.test.js @@ -0,0 +1,168 @@ +/** + * Tests for the Mapper autosave debounce (OpenConceptLab/ocl_issues#2188). + * + * These drive the real scheduler used by MapProject.jsx through an injected + * virtual clock, so the 5s debounce window is asserted without waiting it out. + * + * The reschedule and project-id tests cover the stale-closure defect raised in + * PR #56 review: the timer callback used to read the `isSaving` and `project` + * captured by the render that scheduled it, so a save that completed after + * scheduling was never observed and the autosave wedged. + */ + +import test from 'node:test' +import assert from 'node:assert/strict' + +import { createAutosaveScheduler, AUTOSAVE_DELAY_MS } from '../autosave.js' + +// Virtual clock standing in for setTimeout/clearTimeout. Timers scheduled +// while the clock is being advanced land in a later window, as real timers +// would — that is what makes the reschedule path observable. +const createFakeClock = () => { + let now = 0 + let nextId = 0 + const timers = new Map() + + const setTimeoutFn = (fn, delay) => { + const id = ++nextId + timers.set(id, { fn, at: now + delay }) + return id + } + + const clearTimeoutFn = id => { + timers.delete(id) + } + + const tick = ms => { + now += ms + const due = [...timers.entries()] + .filter(([, timer]) => timer.at <= now) + .sort((a, b) => a[1].at - b[1].at) + for(const [id, timer] of due) { + timers.delete(id) + timer.fn() + } + } + + return { setTimeoutFn, clearTimeoutFn, tick, pendingCount: () => timers.size } +} + +// Mirrors how MapProject.jsx wires the scheduler: project id and isSaving are +// read through getters backed by refs, so the test can move them mid-window. +const setup = ({ projectId = 'project-1', saving = false } = {}) => { + const clock = createFakeClock() + const state = { projectId, saving } + const saves = [] + const scheduler = createAutosaveScheduler({ + getProjectId: () => state.projectId, + isSaving: () => state.saving, + onFire: reasons => saves.push(reasons), + setTimeoutFn: clock.setTimeoutFn, + clearTimeoutFn: clock.clearTimeoutFn + }) + return { scheduler, clock, state, saves } +} + +const DELAY = AUTOSAVE_DELAY_MS + +test('autosave debounce: triggers within one window coalesce into a single save', () => { + const { scheduler, clock, saves } = setup() + + scheduler.schedule('decision_change') + clock.tick(2000) + scheduler.schedule('bulk_decision') + clock.tick(2000) + scheduler.schedule('decision_change') // duplicate reason, restarts the window + + assert.equal(saves.length, 0, 'still inside the debounce window of the last trigger') + + clock.tick(DELAY) + + assert.equal(saves.length, 1, 'three triggers must coalesce into one autosave') + assert.deepEqual(saves[0], ['decision_change', 'bulk_decision'], + 'reasons accumulate across the window and dedupe') +}) + +test('autosave debounce: leaves exactly one pending timer behind', () => { + const { scheduler, clock } = setup() + + scheduler.schedule('decision_change') + scheduler.schedule('bulk_decision') + scheduler.schedule('auto_match') + + assert.equal(clock.pendingCount(), 1, 'each trigger must replace the pending timer, not stack one') + + clock.tick(DELAY) + assert.equal(clock.pendingCount(), 0, 'no timer left running after the save fires') +}) + +test('autosave regression: a save in flight reschedules, then fires once it completes (#2188 PR review)', () => { + const { scheduler, clock, state, saves } = setup({ saving: true }) + + scheduler.schedule('decision_change') + + clock.tick(DELAY) + assert.equal(saves.length, 0, 'must not autosave on top of an in-flight save') + assert.equal(scheduler.hasPending(), true, 'the autosave must be rescheduled, not dropped') + + state.saving = false // the in-flight save completes + + clock.tick(DELAY) + assert.equal(saves.length, 1, 'autosave must observe the completed save and fire on the next window') + assert.deepEqual(saves[0], ['decision_change']) +}) + +test('autosave regression: reasons queued during a save survive the reschedule', () => { + const { scheduler, clock, state, saves } = setup({ saving: true }) + + scheduler.schedule('decision_change') + clock.tick(DELAY) // reschedules — save still in flight + scheduler.schedule('bulk_decision') + + state.saving = false + clock.tick(DELAY) + + assert.equal(saves.length, 1) + assert.deepEqual(saves[0], ['decision_change', 'bulk_decision'], + 'a reason queued while saving must not be lost by the reschedule') +}) + +test('autosave: cancel() preempts a pending autosave, as a manual save does', () => { + const { scheduler, clock, saves } = setup() + + scheduler.schedule('decision_change') + scheduler.cancel() + + assert.equal(scheduler.hasPending(), false, 'pending timer must be cleared') + assert.deepEqual(scheduler.pendingReasons(), [], 'queued reasons must be dropped') + + clock.tick(DELAY * 2) + assert.equal(saves.length, 0, 'a cancelled autosave must never fire') + + // scheduling afterward still works, and does not resurrect the old reason + scheduler.schedule('bulk_decision') + clock.tick(DELAY) + assert.deepEqual(saves, [['bulk_decision']]) +}) + +test('autosave gating: scheduling is a no-op for a project with no id', () => { + const { scheduler, clock, saves } = setup({ projectId: null }) + + scheduler.schedule('decision_change') + + assert.equal(scheduler.hasPending(), false) + clock.tick(DELAY * 2) + assert.equal(saves.length, 0, 'autosave must never run for an unsaved project') +}) + +test('autosave gating: a pending autosave is dropped if the project id goes away', () => { + const { scheduler, clock, state, saves } = setup() + + scheduler.schedule('decision_change') + state.projectId = null + + clock.tick(DELAY) + + assert.equal(saves.length, 0, 'no save without a project id') + assert.deepEqual(scheduler.pendingReasons(), [], 'queued reasons must be cleared') +}) diff --git a/src/components/map-projects/autosave.js b/src/components/map-projects/autosave.js new file mode 100644 index 0000000..374975f --- /dev/null +++ b/src/components/map-projects/autosave.js @@ -0,0 +1,67 @@ +export const AUTOSAVE_DELAY_MS = 5000 + +/** + * Debounces autosave triggers into a single save, coalescing the reasons + * queued during the window (OpenConceptLab/ocl_issues#2188). + * + * Project id and in-flight-save state are read through getters rather than + * captured at creation time. A scheduler is created once per MapProject + * instance and reused across renders, so a timer scheduled by one render + * must still observe values committed by later renders — reading a snapshot + * here is what previously let a pending autosave wedge behind a save that + * had already completed. + * + * setTimeoutFn/clearTimeoutFn are injectable so tests can drive a virtual + * clock instead of waiting out the real debounce window. + */ +export const createAutosaveScheduler = ({ + getProjectId, + isSaving, + onFire, + delay = AUTOSAVE_DELAY_MS, + setTimeoutFn = setTimeout, + clearTimeoutFn = clearTimeout +}) => { + let timer = null + let reasons = [] + + // Preempts a pending autosave — used by a manual save and on unmount. + const cancel = () => { + if(timer) { + clearTimeoutFn(timer) + timer = null + } + reasons = [] + } + + const schedule = reason => { + if(!getProjectId()) + return + + reasons = [...new Set([...reasons, reason])] + if(timer) + clearTimeoutFn(timer) + + timer = setTimeoutFn(() => { + timer = null + if(!getProjectId()) { + reasons = [] + return + } + if(isSaving()) { + schedule(reason) + return + } + const firedReasons = reasons + reasons = [] + onFire(firedReasons) + }, delay) + } + + return { + schedule, + cancel, + hasPending: () => Boolean(timer), + pendingReasons: () => reasons + } +} From 7e0d287cb0eec92e8fc32d41ec19332dd3f9282b Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 4 Sep 2026 06:59:43 +0530 Subject: [PATCH 4/4] OpenConceptLab/ocl_issues#2188 | review feedback --- src/components/map-projects/MapProject.jsx | 20 +++++++++++--------- src/components/map-projects/ProjectLogs.jsx | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index 308637a..0177976 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -1411,9 +1411,7 @@ const MapProject = () => { } const onSave = (options = {}) => { - const saveOptions = options?.preventDefault ? {} : options - const saveSource = saveOptions.source || 'manual' - const isAutoSave = saveSource === 'auto' + const isAutoSave = options.source === 'auto' if(isAutoSave && !project?.id) return if(!isAutoSave) @@ -1430,8 +1428,8 @@ const MapProject = () => { }) return } - const rowLogsForSave = saveOptions.logs || logsRef.current - const projectLogsForSave = saveOptions.projectLogs || projectLogsRef.current + const rowLogsForSave = options.logs || logsRef.current + const projectLogsForSave = options.projectLogs || projectLogsRef.current setIsSaving(true) const f = getFileObjectFromRows() const selected = map(mapSelected, (data, i) => { @@ -1518,12 +1516,12 @@ const MapProject = () => { description: isAutoSave ? t('map_project.auto_saved_changes') : undefined, created_at: moment().toDate(), user: user.username || user.id, - extras: isAutoSave ? {reasons: saveOptions.reasons || []} : (isUpdate ? undefined : {project: response.data}) + extras: isAutoSave ? {reasons: options.reasons || []} : (isUpdate ? undefined : {project: response.data}) } const savedProjectLogs = [saveLog, ...projectLogsForSave] projectLogsRef.current = savedProjectLogs setProjectLogs(savedProjectLogs) - if(saveOptions.closeConfigure !== false) { + if(options.closeConfigure !== false) { configSnapshotOnOpenRef.current = null setConfigure(false) } @@ -1539,6 +1537,10 @@ const MapProject = () => { }).finally(() => setIsSaving(false)) } + // onSave takes options, so it can't be bound to onClick directly — the click + // event would arrive where the options object is expected. + const onManualSave = () => onSave({source: 'manual'}) + const log = (data, index) => { let idx = index === undefined ? rowIndex : index const nextLogs = {...logsRef.current, [idx]: [{created_at: moment().toDate(), user: user.username || user.id, ...data}, ...(logsRef.current[idx] || [])]} @@ -4815,7 +4817,7 @@ const MapProject = () => { setConfigure={setConfigureWithAutosave} columnVisibilityModel={columnVisibilityModel} setColumnVisibilityModel={setColumnVisibilityModel} - onSave={onSave} + onSave={onManualSave} isSaving={isSaving} candidatesScore={candidatesScore} onScoreChange={setCandidatesScore} @@ -5007,7 +5009,7 @@ const MapProject = () => { isCoreUser={isCoreUser} project={project} onDownload={onDownloadClick} - onSave={onSave} + onSave={onManualSave} onDelete={() => setDeleteProject(true)} owner={owner} file={file} diff --git a/src/components/map-projects/ProjectLogs.jsx b/src/components/map-projects/ProjectLogs.jsx index 93d4695..1eee337 100644 --- a/src/components/map-projects/ProjectLogs.jsx +++ b/src/components/map-projects/ProjectLogs.jsx @@ -97,7 +97,7 @@ const ProjectLogs = ({onClose, logs, project}) => { , ]} values={{ - created_by: project.created_by || '', + created_by: log.user || project.created_by || '', owner: project.owner ? `${project.owner_type}:${project.owner}` : '', }} />