From fc7514304b34f6c600f20712051c37c137287588 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 26 Oct 2026 17:06:30 +0530 Subject: [PATCH 1/5] OpenConceptLab/ocl_issues#2830 | stop Auto Match when leaving the project - Ask before leaving while a run is going ("Stay" / "Stop and leave"); the browser's prompt covers closing the tab. - Leaving stops the run at once: its record closes as stopped, late results are dropped, and no autosave is scheduled after unmount. - Closing the tab closes the run record with a keepalive request. - Fix isRunningBulkScispacyCandidates checking the start time for its end. Co-Authored-By: Claude Opus 5.5 --- src/components/map-projects/MapProject.jsx | 102 ++++++++++++- .../map-projects/__tests__/leaveGuard.test.js | 137 ++++++++++++++++++ src/components/map-projects/leaveGuard.js | 44 ++++++ src/i18n/locales/en/translations.json | 4 + src/i18n/locales/es/translations.json | 4 + src/i18n/locales/pt-BR/translations.json | 4 + src/i18n/locales/zh/translations.json | 4 + src/services/APIService.js | 7 + 8 files changed, 299 insertions(+), 7 deletions(-) create mode 100644 src/components/map-projects/__tests__/leaveGuard.test.js create mode 100644 src/components/map-projects/leaveGuard.js diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index 4a8cfd9..dddb3ee 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -94,6 +94,7 @@ import DataGridControls from './DataGridControls' import { getPreviewEligibleRowIndexes, getRowsToProcess, countAIOnlyRows, getAutoMatchAIOnOpen, takeRunRows, clearRunRowStatuses, canRunAlgorithm, spendsMatchQuota, shouldStopAIStep, getAIRequestIdempotencyKey, getCandidatePoolFingerprint, hasCurrentAnalysis, getScispacyRowResults, getPendingRowLookups, waitForLookups, AI_LOOKUP_WAIT_MS, RERANK_LOOKUP_WAIT_MS, RERANK_MAX_IN_FLIGHT, isScispacyWarmingUp } from './autoMatchRows' import { aiOnByDefault, getMaxRunRows, getMapperSettings } from '@openconceptlab/oclmap-extensions' import { createAutosaveScheduler, createLatestSender, saveOnLeave, trackSave, whenSaved, installUnloadGuard } from './autosave' +import { blockLeaving, continueLeaving, createRunClosers } from './leaveGuard' import MatchSummaryCard from './MatchSummaryCard' import MappingDecisionResult from './MappingDecisionResult' import DecisionSelector from './DecisionSelector' @@ -279,6 +280,13 @@ const MapProject = () => { // A ref so every per-row backend call reads the run id synchronously without // prop-drilling or re-renders; null outside a run → 'mapper-ui-manual'. const automatchRunRef = React.useRef(null); + // Lets leaving close open run records at once. + const runClosersRef = React.useRef(null) + if(!runClosersRef.current) + runClosersRef.current = createRunClosers() + const isRunActiveRef = React.useRef(false) + const [leaveTarget, setLeaveTarget] = React.useState(null) + const ownNavigationRef = React.useRef(false) const refreshRowStageSnapshotRef = React.useRef({}); const [previewLimit, setPreviewLimit] = React.useState(null) // {errorCode, limit, used} or null @@ -315,6 +323,7 @@ const MapProject = () => { const configSnapshotOnOpenRef = React.useRef(null) const projectIdRef = React.useRef(null) const onSaveRef = React.useRef(null) + const projectLogRef = React.useRef(() => {}) // A save can complete after the user has left the project // (OpenConceptLab/ocl_issues#2829): these track the request still in flight // (accurate after unmount, unlike isSaving state) and whether this page is @@ -372,6 +381,7 @@ const MapProject = () => { // captured when the change happened rather than what is on screen now. React.useEffect(() => { onSaveRef.current = onSave + projectLogRef.current = projectLog }) // Leaving the project unmounts this page, often inside the autosave window: @@ -382,6 +392,14 @@ const MapProject = () => { installUnloadGuard() return () => { isMountedRef.current = false + // Leaving stops a run at once, not gracefully: its record closes now and + // late results are dropped. + if(isRunActiveRef.current || runClosersRef.current.hasOpen()) { + abortRef.current = true + projectLogRef.current({action: 'auto_match_stopped_on_leave'}) + autosaveSchedulerRef.current.schedule('auto_match_stopped') + } + runClosersRef.current.closeAll(true) saveOnLeave({ scheduler: autosaveSchedulerRef.current, inFlightSave: inFlightSaveRef.current, @@ -395,7 +413,7 @@ const MapProject = () => { // covers a save that outlives this page. React.useEffect(() => { const warnIfUnsaved = event => { - if(!autosaveSchedulerRef.current.hasPending() && !inFlightSaveRef.current) + if(!autosaveSchedulerRef.current.hasPending() && !inFlightSaveRef.current && !isRunActiveRef.current) return event.preventDefault() event.returnValue = '' @@ -404,6 +422,18 @@ const MapProject = () => { return () => window.removeEventListener('beforeunload', warnIfUnsaved) }, []) + // Closing the tab stops a run too, and closes its record. + React.useEffect(() => { + const stopRunOnPageHide = () => { + if(!runClosersRef.current.hasUnsettled()) + return + abortRef.current = true + runClosersRef.current.closeAll(true, {keepalive: true}) + } + window.addEventListener('pagehide', stopRunOnPageHide) + return () => window.removeEventListener('pagehide', stopRunOnPageHide) + }, []) + // repo state const [repo, setRepo] = React.useState(false) const [repoVersion, setRepoVersion] = React.useState(false) @@ -1768,8 +1798,14 @@ const MapProject = () => { if(!isUpdate) refreshMapperQuotaCache() // A save that lands after the user left mustn't pull them back here. - if(response.data.url && isMountedRef.current) - history.push(response.data.url) + if(response.data.url && isMountedRef.current) { + ownNavigationRef.current = true + try { + history.push(response.data.url) + } finally { + ownNavigationRef.current = false + } + } if(!isAutoSave) baseSetAlert({severity: 'success', message: t('map_project.successfully_saved'), duration: 2000}) @@ -1830,7 +1866,12 @@ const MapProject = () => { logsSendersRef.current.get(url).trigger({logs}) } - const scheduleAutoSave = reason => autosaveSchedulerRef.current.schedule(reason) + // Not after unmount: frozen state plus a late run's refs would save an + // inconsistent project. + const scheduleAutoSave = reason => { + if(isMountedRef.current) + autosaveSchedulerRef.current.schedule(reason) + } // ── Waiting out a busy server (ocl_issues#2849) ─────────────────────────── // The gate for a request's server, shared by every request this tab sends it. @@ -2036,7 +2077,7 @@ const MapProject = () => { // here, a newer run's record may be the current one (ocl_issues#2849). // An AI-only run is recorded from aiOutcomes, its own AI results, not the // shared row stages a manual action or a stopped run's late call can change. - const completeAutomatchRun = async (selectedAlgos, intendedRows, run, aborted, aiOutcomes = {}) => { + const completeAutomatchRun = async (selectedAlgos, intendedRows, run, aborted, aiOutcomes = {}, {keepalive = false} = {}) => { if(automatchRunRef.current === run) automatchRunRef.current = null if(!run?.id) return @@ -2050,6 +2091,9 @@ const MapProject = () => { aborted, stoppedForQuota: Boolean(matchQuotaStopRef.current || (run.aiOnly && aiQuotaExhaustedRef.current)), }) + // The tab is closing: one request, sent before this function yields. + if(keepalive) + return APIService.new().overrideURL('/auto-match-runs/' + run.id + '/').keepalive('PATCH', payload) // Closing the run is idempotent, so a busy server or a gateway error is // retried; Stop doesn't end it, so a stopped run is closed too // (ocl_issues#2849). requestWithCapacityRetry never rejects. @@ -2579,6 +2623,7 @@ const MapProject = () => { // began: it changed nothing to save. const runStarted = !isRunStopped() const aiOutcomes = {} + const closeRun = runClosersRef.current.add((aborted, options) => completeAutomatchRun(_selectedAlgos, rowsToProcess, runRecord, aborted, aiOutcomes, options)) try { bulkMatchAlgoIdsRef.current = map(_selectedAlgos, 'id') // Reset all algo stages to -1 for every row before starting so that @@ -2698,7 +2743,7 @@ const MapProject = () => { // way (ocl_issues#2849). if(runStarted) scheduleAutoSave(isRunStopped() ? 'auto_match_stopped' : 'auto_match') - await completeAutomatchRun(_selectedAlgos, rowsToProcess, runRecord, isRunStopped(), aiOutcomes) + await closeRun(isRunStopped()) refreshMapperQuotaCache() } }, 1000) @@ -2866,7 +2911,34 @@ const MapProject = () => { const isRunningBulkAnalysis = bulkAIAnalysisEndedAt ? moment().isBetween(bulkAIAnalysisStartedAt, bulkAIAnalysisEndedAt) : Boolean(bulkAIAnalysisStartedAt) const isRunningBulkBridgeCandidates = bridgeCandidatesEndedAt ? moment().isBetween(bridgeCandidatesStartedAt, bridgeCandidatesEndedAt) : Boolean(bridgeCandidatesStartedAt) - const isRunningBulkScispacyCandidates = scispacyCandidatesStartedAt ? moment().isBetween(scispacyCandidatesStartedAt, scispacyCandidatesEndedAt) : Boolean(scispacyCandidatesStartedAt) + const isRunningBulkScispacyCandidates = scispacyCandidatesEndedAt ? moment().isBetween(scispacyCandidatesStartedAt, scispacyCandidatesEndedAt) : Boolean(scispacyCandidatesStartedAt) + + // Leaving while a run is going asks first. + const isRunActive = Boolean(loadingMatches || isRunningBulkAnalysis || isRunningBulkBridgeCandidates || isRunningBulkScispacyCandidates) + isRunActiveRef.current = isRunActive + const unblockLeavingRef = React.useRef(null) + React.useEffect(() => { + if(!isRunActive) + return + const unblock = blockLeaving(history, { + isAllowed: () => ownNavigationRef.current, + onBlocked: setLeaveTarget, + }) + unblockLeavingRef.current = unblock + return () => { + unblockLeavingRef.current = null + unblock() + } + }, [isRunActive]) + + const onStopRunAndLeave = () => { + const target = leaveTarget + setLeaveTarget(null) + unblockLeavingRef.current?.() + unblockLeavingRef.current = null + if(target) + continueLeaving(history, target) + } // Only the latest request may set the version: a slower response is for a // repository no longer chosen. @@ -6788,6 +6860,22 @@ const MapProject = () => { } + setLeaveTarget(null)} maxWidth='xs' fullWidth> + {t('map_project.leave_while_running_title')} + + + {t('map_project.leave_while_running_body')} + + + + + + + { deleteProject && project?.id && setDeleteProject(false)} onDeleted={() => { autosaveSchedulerRef.current.cancel(); refreshMapperQuotaCache() }} project={project} /> diff --git a/src/components/map-projects/__tests__/leaveGuard.test.js b/src/components/map-projects/__tests__/leaveGuard.test.js new file mode 100644 index 0000000..7c796d6 --- /dev/null +++ b/src/components/map-projects/__tests__/leaveGuard.test.js @@ -0,0 +1,137 @@ +/** + * Tests for leaving a project while Auto Match runs. + */ + +import test from 'node:test' +import assert from 'node:assert/strict' + +import { blockLeaving, continueLeaving, createRunClosers } from '../leaveGuard.js' + +// Stands in for a history v4 object: block's prompt decides each transition. +const createFakeHistory = () => { + let prompt = null + const calls = [] + const transition = (action, location) => { + if(prompt && prompt(location, action) === false) + return + calls.push({action, location}) + } + return { + calls, + block: fn => { + prompt = fn + return () => { prompt = null } + }, + push: location => transition('PUSH', location), + replace: location => transition('REPLACE', location), + } +} + +test('blockLeaving holds a navigation and hands it over', () => { + const history = createFakeHistory() + const blocked = [] + blockLeaving(history, {onBlocked: next => blocked.push(next)}) + + history.push('/projects') + + assert.deepEqual(history.calls, []) + assert.deepEqual(blocked, [{location: '/projects', action: 'PUSH'}]) +}) + +test('blockLeaving lets an allowed navigation through', () => { + const history = createFakeHistory() + const blocked = [] + blockLeaving(history, {isAllowed: location => location === '/project/1', onBlocked: next => blocked.push(next)}) + + history.push('/project/1') + + assert.deepEqual(history.calls, [{action: 'PUSH', location: '/project/1'}]) + assert.deepEqual(blocked, []) +}) + +test('continueLeaving replays a held navigation once unblocked', () => { + const history = createFakeHistory() + let held = null + const unblock = blockLeaving(history, {onBlocked: next => { held = next }}) + + history.replace('/elsewhere') + unblock() + continueLeaving(history, held) + + assert.deepEqual(history.calls, [{action: 'REPLACE', location: '/elsewhere'}]) +}) + +test('continueLeaving pushes a held Back', () => { + const history = createFakeHistory() + continueLeaving(history, {location: '/back', action: 'POP'}) + assert.deepEqual(history.calls, [{action: 'PUSH', location: '/back'}]) +}) + +test('a run closer closes its run once, whoever calls first', async () => { + const closers = createRunClosers() + const closed = [] + const close = closers.add(aborted => { closed.push(aborted) }) + + assert.equal(closers.hasOpen(), true) + // Leaving closes it at once; the run's own close, once its requests settle, + // must not close it again with a later status. + await closers.closeAll(true) + await close(false) + + assert.deepEqual(closed, [true]) + assert.equal(closers.hasOpen(), false) +}) + +test('a run that closed itself is not closed again on leave', async () => { + const closers = createRunClosers() + const closed = [] + const close = closers.add(aborted => { closed.push(aborted) }) + + await close(false) + await closers.closeAll(true) + + assert.deepEqual(closed, [false]) +}) + +test('closeAll closes every run still open', async () => { + const closers = createRunClosers() + const closed = [] + closers.add(aborted => { closed.push(['first', aborted]) }) + closers.add(aborted => { closed.push(['second', aborted]) }) + + await closers.closeAll(true) + + assert.deepEqual(closed, [['first', true], ['second', true]]) +}) + +test('closing the tab resends a close still in flight, with its own status', async () => { + const closers = createRunClosers() + const calls = [] + let settle + const close = closers.add((aborted, options) => { + calls.push([aborted, Boolean(options?.keepalive)]) + if(calls.length === 1) + return new Promise(resolve => { settle = resolve }) + }) + + const own = close(false) + assert.equal(closers.hasOpen(), false) + assert.equal(closers.hasUnsettled(), true) + await closers.closeAll(true, {keepalive: true}) + settle() + await own + + assert.deepEqual(calls, [[false, false], [false, true]]) + assert.equal(closers.hasUnsettled(), false) +}) + +test('closing the tab sends nothing for a run already closed', async () => { + const closers = createRunClosers() + const calls = [] + const close = closers.add((aborted, options) => { calls.push([aborted, Boolean(options?.keepalive)]) }) + + await close(false) + await closers.closeAll(true, {keepalive: true}) + + assert.deepEqual(calls, [[false, false]]) +}) diff --git a/src/components/map-projects/leaveGuard.js b/src/components/map-projects/leaveGuard.js new file mode 100644 index 0000000..4e62ab8 --- /dev/null +++ b/src/components/map-projects/leaveGuard.js @@ -0,0 +1,44 @@ +// Leaving a project while Auto Match runs. + +// Holds in-app navigation, unless isAllowed, and hands it to onBlocked as +// {location, action}. Returns the unblock function. +export const blockLeaving = (history, {isAllowed = () => false, onBlocked}) => + history.block((location, action) => { + if(isAllowed(location, action)) + return undefined + onBlocked({location, action}) + return false + }) + +// A held Back/Forward (POP) can't be replayed by distance, so it is pushed. +export const continueLeaving = (history, {location, action}) => + action === 'REPLACE' ? history.replace(location) : history.push(location) + +// Each closer closes its run's record once, whether the run ends or the page +// closes it first on leave. A close still in flight as the tab closes is sent +// again with keepalive, as it was, since the tab's closing cancels it. +export const createRunClosers = () => { + const unsettled = new Set() + const add = close => { + let result = null + let closedAs + const closeOnce = (aborted, options = {}) => { + if(result && (!unsettled.has(closeOnce) || !options.keepalive)) + return result + if(!result) + closedAs = aborted + result = Promise.resolve(close(closedAs, options)).finally(() => unsettled.delete(closeOnce)) + return result + } + closeOnce.isClosing = () => Boolean(result) + unsettled.add(closeOnce) + return closeOnce + } + return { + add, + closeAll: (aborted, options) => Promise.all([...unsettled].map(close => close(aborted, options))), + // A run whose record nothing has started closing yet. + hasOpen: () => [...unsettled].some(close => !close.isClosing()), + hasUnsettled: () => unsettled.size > 0, + } +} diff --git a/src/i18n/locales/en/translations.json b/src/i18n/locales/en/translations.json index 1806ef7..03b3101 100644 --- a/src/i18n/locales/en/translations.json +++ b/src/i18n/locales/en/translations.json @@ -373,6 +373,10 @@ "loading_project": "Loading project...", "stopping_gracefully": "Stopping gracefully...", "stop_processing": "Stop Processing", + "leave_while_running_title": "Auto Match is running", + "leave_while_running_body": "Leaving this project stops the run now. What it has matched so far is saved.", + "stay": "Stay", + "stop_and_leave": "Stop and leave", "auto_match_all_including_approved_confirm": "I understand that {{approvedCount}} approved decisions will be overwritten", "auto_match_selected_rows_note_no_count": "Select rows in the left panel to enable this option", "auto_match_rows_failed": "Matching failed for {{count}} input row(s), so they're missing candidates: {{details}}. Rows count from the first data row of your file. Run AutoMatch on those rows again to retry them.", diff --git a/src/i18n/locales/es/translations.json b/src/i18n/locales/es/translations.json index 2aa5ce3..cfa1dac 100644 --- a/src/i18n/locales/es/translations.json +++ b/src/i18n/locales/es/translations.json @@ -359,6 +359,10 @@ "loading_project": "Cargando proyecto...", "stopping_gracefully": "Deteniendo suavemente...", "stop_processing": "Detener Procesamiento", + "leave_while_running_title": "Auto Match está en ejecución", + "leave_while_running_body": "Salir de este proyecto detiene la ejecución ahora. Lo que ya se ha emparejado se guarda.", + "stay": "Quedarse", + "stop_and_leave": "Detener y salir", "auto_match_all_including_approved_confirm": "Entiendo que se sobrescribirán {{approvedCount}} decisiones aprobadas", "auto_match_selected_rows_note_no_count": "Seleccione filas en el panel izquierdo para habilitar esta opción", "auto_match_rows_failed": "La coincidencia falló en {{count}} fila(s) de entrada, así que les faltan candidatos: {{details}}. Las filas se cuentan desde la primera fila de datos de tu archivo. Vuelve a ejecutar AutoMatch en esas filas para reintentarlo.", diff --git a/src/i18n/locales/pt-BR/translations.json b/src/i18n/locales/pt-BR/translations.json index cc0cd1b..74d6902 100644 --- a/src/i18n/locales/pt-BR/translations.json +++ b/src/i18n/locales/pt-BR/translations.json @@ -373,6 +373,10 @@ "loading_project": "Carregando projeto...", "stopping_gracefully": "Encerrando com segurança...", "stop_processing": "Parar Processamento", + "leave_while_running_title": "O Auto Match está em execução", + "leave_while_running_body": "Sair deste projeto interrompe a execução agora. O que já foi correspondido é salvo.", + "stay": "Ficar", + "stop_and_leave": "Parar e sair", "auto_match_all_including_approved_confirm": "Entendo que {{approvedCount}} decisões aprovadas serão sobrescritas", "auto_match_selected_rows_note_no_count": "Selecione linhas no painel esquerdo para habilitar esta opção", "auto_match_rows_failed": "O AutoMatch falhou para {{count}} linha(s) de entrada, então elas estão sem candidatos: {{details}}. As linhas contam a partir da primeira linha de dados do seu arquivo. Execute o AutoMatch nessas linhas novamente para tentar de novo.", diff --git a/src/i18n/locales/zh/translations.json b/src/i18n/locales/zh/translations.json index feedf28..efcfe94 100644 --- a/src/i18n/locales/zh/translations.json +++ b/src/i18n/locales/zh/translations.json @@ -371,6 +371,10 @@ "loading_project": "正在加载项目...", "stopping_gracefully": "正在优雅地停止...", "stop_processing": "停止处理", + "leave_while_running_title": "自动匹配正在运行", + "leave_while_running_body": "离开此项目会立即停止运行。已匹配的结果会被保存。", + "stay": "留下", + "stop_and_leave": "停止并离开", "auto_match_all_including_approved_confirm": "我明白将覆盖 {{approvedCount}} 个已批准的决策", "auto_match_selected_rows_note_no_count": "请在左侧面板选择行以启用此选项", "auto_match_rows_failed": "{{count}} 个输入行匹配失败，缺少候选项：{{details}}。行号从文件的第一个数据行开始计数。请对这些行重新运行 AutoMatch 以重试。", diff --git a/src/services/APIService.js b/src/services/APIService.js index 8d63ade..725c901 100644 --- a/src/services/APIService.js +++ b/src/services/APIService.js @@ -176,6 +176,13 @@ class APIService { }); }; + // For a request sent as the page closes: fetch's keepalive outlives the page, + // axios's XHR doesn't. Best effort, never rejects. + keepalive(method, data, token) { + const { url, headers } = this.getRequest(method, data, token, {}); + return fetch(url, {method, headers, body: JSON.stringify(data), keepalive: true}).catch(() => null); + } + getHeaders(token, headers) { token = token || (token !== false ? currentUserToken() : token); const obj = defaults(headers, this.headers); From fa48989b5ecd500b9729584bf1cf75062c572bba Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 26 Oct 2026 17:16:44 +0530 Subject: [PATCH 2/5] OpenConceptLab/ocl_issues#2830 | stop and save the run on Stop and leave, also when the page is reused --- src/components/map-projects/MapProject.jsx | 37 ++++++++++++++-------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index dddb3ee..4d7cf02 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -387,24 +387,31 @@ const MapProject = () => { // Leaving the project unmounts this page, often inside the autosave window: // save the pending change now rather than drop it // (OpenConceptLab/ocl_issues#2829). Nothing to save once signed out. + const saveNowOnLeave = () => saveOnLeave({ + scheduler: autosaveSchedulerRef.current, + inFlightSave: inFlightSaveRef.current, + save: reasons => currentUserToken() && onSaveRef.current({source: 'auto', closeConfigure: false, reasons, leaving: true}) + }) + + // Leaving stops a run at once, not gracefully: its record closes now and + // what it matched so far is saved. Once per run, whether or not leaving + // unmounts this page: a navigation to another project URL reuses it. + const stopRunOnLeave = () => { + if(!runClosersRef.current.hasOpen() && !(isRunActiveRef.current && !abortRef.current)) + return + abortRef.current = true + projectLogRef.current({action: 'auto_match_stopped_on_leave'}) + autosaveSchedulerRef.current.schedule('auto_match_stopped') + runClosersRef.current.closeAll(true) + } + React.useEffect(() => { isMountedRef.current = true installUnloadGuard() return () => { isMountedRef.current = false - // Leaving stops a run at once, not gracefully: its record closes now and - // late results are dropped. - if(isRunActiveRef.current || runClosersRef.current.hasOpen()) { - abortRef.current = true - projectLogRef.current({action: 'auto_match_stopped_on_leave'}) - autosaveSchedulerRef.current.schedule('auto_match_stopped') - } - runClosersRef.current.closeAll(true) - saveOnLeave({ - scheduler: autosaveSchedulerRef.current, - inFlightSave: inFlightSaveRef.current, - save: reasons => currentUserToken() && onSaveRef.current({source: 'auto', closeConfigure: false, reasons, leaving: true}) - }) + stopRunOnLeave() + saveNowOnLeave() } }, []) @@ -1798,7 +1805,7 @@ const MapProject = () => { if(!isUpdate) refreshMapperQuotaCache() // A save that lands after the user left mustn't pull them back here. - if(response.data.url && isMountedRef.current) { + if(response.data.url && isMountedRef.current && !options.leaving) { ownNavigationRef.current = true try { history.push(response.data.url) @@ -2934,6 +2941,8 @@ const MapProject = () => { const onStopRunAndLeave = () => { const target = leaveTarget setLeaveTarget(null) + stopRunOnLeave() + saveNowOnLeave() unblockLeavingRef.current?.() unblockLeavingRef.current = null if(target) From 3feee854761f0b37f61fb05e6ce34693c89d2d7f Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 26 Oct 2026 17:49:04 +0530 Subject: [PATCH 3/5] OpenConceptLab/ocl_issues#2830 | remount the page for another project, and keep a save from pulling the user back --- src/components/app/App.jsx | 6 ++--- src/components/map-projects/MapProject.jsx | 8 +++--- .../map-projects/MapProjectRoute.jsx | 14 +++++++++++ .../map-projects/__tests__/leaveGuard.test.js | 25 ++++++++++++++++++- src/components/map-projects/leaveGuard.js | 10 ++++++++ 5 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 src/components/map-projects/MapProjectRoute.jsx diff --git a/src/components/app/App.jsx b/src/components/app/App.jsx index d5c8734..977140e 100644 --- a/src/components/app/App.jsx +++ b/src/components/app/App.jsx @@ -25,7 +25,7 @@ import Header from './Header'; import OIDLoginCallback from '../users/OIDLoginCallback'; import { OperationsContext } from './LayoutContext'; import Alert from '../common/Alert'; -import MapProject from '../map-projects/MapProject' +import MapProjectRoute from '../map-projects/MapProjectRoute' import MapProjects from '../map-projects/MapProjects' import { useTranslation } from 'react-i18next'; @@ -166,8 +166,8 @@ const App = props => { - - + + diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index 4d7cf02..da92cfe 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -1773,6 +1773,9 @@ const MapProject = () => { else service = service.post(formData, null, {"Content-Type": "multipart/form-data"}, undefined, true) + // Where the user was when saving: a save that lands once they went + // elsewhere mustn't pull them back. + const pathAtSave = history.location.pathname const request = service.then(response => { setIsSaving(false) const status = response?.response?.status || response?.status @@ -1804,11 +1807,10 @@ const MapProject = () => { setProject(response.data) if(!isUpdate) refreshMapperQuotaCache() - // A save that lands after the user left mustn't pull them back here. - if(response.data.url && isMountedRef.current && !options.leaving) { + if(response.data.url && isMountedRef.current && !options.leaving && history.location.pathname === pathAtSave) { ownNavigationRef.current = true try { - history.push(response.data.url) + history.push(response.data.url, {mapProjectFrom: pathAtSave}) } finally { ownNavigationRef.current = false } diff --git a/src/components/map-projects/MapProjectRoute.jsx b/src/components/map-projects/MapProjectRoute.jsx new file mode 100644 index 0000000..be5af2f --- /dev/null +++ b/src/components/map-projects/MapProjectRoute.jsx @@ -0,0 +1,14 @@ +import React from 'react' +import { useLocation } from 'react-router-dom' +import MapProject from './MapProject' +import { getPageKey } from './leaveGuard' + +const MapProjectRoute = () => { + const location = useLocation() + const pageRef = React.useRef({key: null, path: null}) + if(pageRef.current.path !== location.pathname) + pageRef.current = getPageKey(pageRef.current, location) + return +} + +export default MapProjectRoute diff --git a/src/components/map-projects/__tests__/leaveGuard.test.js b/src/components/map-projects/__tests__/leaveGuard.test.js index 7c796d6..7fb6cc5 100644 --- a/src/components/map-projects/__tests__/leaveGuard.test.js +++ b/src/components/map-projects/__tests__/leaveGuard.test.js @@ -5,7 +5,7 @@ import test from 'node:test' import assert from 'node:assert/strict' -import { blockLeaving, continueLeaving, createRunClosers } from '../leaveGuard.js' +import { blockLeaving, continueLeaving, createRunClosers, getPageKey } from '../leaveGuard.js' // Stands in for a history v4 object: block's prompt decides each transition. const createFakeHistory = () => { @@ -135,3 +135,26 @@ test('closing the tab sends nothing for a run already closed', async () => { assert.deepEqual(calls, [[false, false]]) }) + +test('getPageKey gives another project a new page', () => { + const first = getPageKey({key: null, path: null}, {pathname: '/orgs/A/map-projects/1/'}) + assert.deepEqual(first, {key: '/orgs/A/map-projects/1/', path: '/orgs/A/map-projects/1/'}) + + const next = getPageKey(first, {pathname: '/orgs/A/map-projects/2/'}) + assert.deepEqual(next, {key: '/orgs/A/map-projects/2/', path: '/orgs/A/map-projects/2/'}) +}) + +test("getPageKey keeps the page for a save's own navigation", () => { + const page = getPageKey({key: null, path: null}, {pathname: '/map-projects/new'}) + const saved = getPageKey(page, {pathname: '/orgs/A/map-projects/1/', state: {mapProjectFrom: '/map-projects/new'}}) + assert.deepEqual(saved, {key: '/map-projects/new', path: '/orgs/A/map-projects/1/'}) +}) + +test('getPageKey gives a new page on Back to an entry a save pushed', () => { + // new -> saved as 1 (kept) -> project 2 -> Back to 1, whose entry still + // carries the save's state. + const saved = getPageKey(getPageKey({key: null, path: null}, {pathname: '/map-projects/new'}), {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}) + const other = getPageKey(saved, {pathname: '/p/2'}) + const back = getPageKey(other, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}) + assert.deepEqual(back, {key: '/p/1', path: '/p/1'}) +}) diff --git a/src/components/map-projects/leaveGuard.js b/src/components/map-projects/leaveGuard.js index 4e62ab8..b138e19 100644 --- a/src/components/map-projects/leaveGuard.js +++ b/src/components/map-projects/leaveGuard.js @@ -42,3 +42,13 @@ export const createRunClosers = () => { hasUnsettled: () => unsettled.size > 0, } } + +// MapProject loads its project once, on mount, so another project's URL needs +// a new page. Its own navigation (a save's) passes the path it left in +// location.state.mapProjectFrom and keeps the page. Returns the key and path +// to keep for the next location. +export const getPageKey = ({key, path}, location) => { + if(key && location.state?.mapProjectFrom === path) + return {key, path: location.pathname} + return {key: location.pathname, path: location.pathname} +} From 1120fa04f701d88e7e6ca2547c3a1e44bfd65a21 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 26 Oct 2026 18:38:40 +0530 Subject: [PATCH 4/5] OpenConceptLab/ocl_issues#2830 | give Back and Forward a new page, keeping it only for a save's own navigation --- .../map-projects/MapProjectRoute.jsx | 5 ++- .../map-projects/__tests__/leaveGuard.test.js | 40 ++++++++++++------- src/components/map-projects/leaveGuard.js | 13 +++--- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/components/map-projects/MapProjectRoute.jsx b/src/components/map-projects/MapProjectRoute.jsx index be5af2f..3a5e2ea 100644 --- a/src/components/map-projects/MapProjectRoute.jsx +++ b/src/components/map-projects/MapProjectRoute.jsx @@ -1,13 +1,14 @@ import React from 'react' -import { useLocation } from 'react-router-dom' +import { useHistory, useLocation } from 'react-router-dom' import MapProject from './MapProject' import { getPageKey } from './leaveGuard' const MapProjectRoute = () => { const location = useLocation() + const history = useHistory() const pageRef = React.useRef({key: null, path: null}) if(pageRef.current.path !== location.pathname) - pageRef.current = getPageKey(pageRef.current, location) + pageRef.current = getPageKey(pageRef.current, location, history.action) return } diff --git a/src/components/map-projects/__tests__/leaveGuard.test.js b/src/components/map-projects/__tests__/leaveGuard.test.js index 7fb6cc5..d89fadd 100644 --- a/src/components/map-projects/__tests__/leaveGuard.test.js +++ b/src/components/map-projects/__tests__/leaveGuard.test.js @@ -136,25 +136,37 @@ test('closing the tab sends nothing for a run already closed', async () => { assert.deepEqual(calls, [[false, false]]) }) -test('getPageKey gives another project a new page', () => { - const first = getPageKey({key: null, path: null}, {pathname: '/orgs/A/map-projects/1/'}) - assert.deepEqual(first, {key: '/orgs/A/map-projects/1/', path: '/orgs/A/map-projects/1/'}) +const start = {key: null, path: null} - const next = getPageKey(first, {pathname: '/orgs/A/map-projects/2/'}) - assert.deepEqual(next, {key: '/orgs/A/map-projects/2/', path: '/orgs/A/map-projects/2/'}) +test('getPageKey gives another project a new page', () => { + const first = getPageKey(start, {pathname: '/p/1'}, 'PUSH') + const next = getPageKey(first, {pathname: '/p/2'}, 'PUSH') + assert.equal(first.path, '/p/1') + assert.deepEqual(next, {key: first.key + 1, path: '/p/2'}) }) test("getPageKey keeps the page for a save's own navigation", () => { - const page = getPageKey({key: null, path: null}, {pathname: '/map-projects/new'}) - const saved = getPageKey(page, {pathname: '/orgs/A/map-projects/1/', state: {mapProjectFrom: '/map-projects/new'}}) - assert.deepEqual(saved, {key: '/map-projects/new', path: '/orgs/A/map-projects/1/'}) + const page = getPageKey(start, {pathname: '/map-projects/new'}, 'POP') + const saved = getPageKey(page, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'PUSH') + assert.deepEqual(saved, {key: page.key, path: '/p/1'}) +}) + +test('getPageKey gives a new page on Back and Forward after new -> saved', () => { + const page = getPageKey(start, {pathname: '/map-projects/new'}, 'POP') + const saved = getPageKey(page, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'PUSH') + const back = getPageKey(saved, {pathname: '/map-projects/new'}, 'POP') + // Forward replays the save's state, but it isn't the save. + const forward = getPageKey(back, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'POP') + + assert.notEqual(back.key, saved.key) + assert.notEqual(forward.key, back.key) + assert.deepEqual(forward, {key: back.key + 1, path: '/p/1'}) }) test('getPageKey gives a new page on Back to an entry a save pushed', () => { - // new -> saved as 1 (kept) -> project 2 -> Back to 1, whose entry still - // carries the save's state. - const saved = getPageKey(getPageKey({key: null, path: null}, {pathname: '/map-projects/new'}), {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}) - const other = getPageKey(saved, {pathname: '/p/2'}) - const back = getPageKey(other, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}) - assert.deepEqual(back, {key: '/p/1', path: '/p/1'}) + const page = getPageKey(start, {pathname: '/map-projects/new'}, 'POP') + const saved = getPageKey(page, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'PUSH') + const other = getPageKey(saved, {pathname: '/p/2'}, 'PUSH') + const back = getPageKey(other, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'POP') + assert.deepEqual(back, {key: other.key + 1, path: '/p/1'}) }) diff --git a/src/components/map-projects/leaveGuard.js b/src/components/map-projects/leaveGuard.js index b138e19..53a66a8 100644 --- a/src/components/map-projects/leaveGuard.js +++ b/src/components/map-projects/leaveGuard.js @@ -44,11 +44,12 @@ export const createRunClosers = () => { } // MapProject loads its project once, on mount, so another project's URL needs -// a new page. Its own navigation (a save's) passes the path it left in -// location.state.mapProjectFrom and keeps the page. Returns the key and path -// to keep for the next location. -export const getPageKey = ({key, path}, location) => { - if(key && location.state?.mapProjectFrom === path) +// a new page. Only a save's own navigation keeps it: a push or replace that +// passes the path it left in location.state.mapProjectFrom. Back/Forward +// replays that state, so a POP always gets a new page. Returns the key and +// path to keep for the next location. +export const getPageKey = ({key, path}, location, action) => { + if(key && action !== 'POP' && location.state?.mapProjectFrom === path) return {key, path: location.pathname} - return {key: location.pathname, path: location.pathname} + return {key: (key || 0) + 1, path: location.pathname} } From 3652ac197f854b31ada4dbd780d43a8b454b6619 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 27 Oct 2026 07:34:35 +0530 Subject: [PATCH 5/5] OpenConceptLab/ocl_issues#2830 | send nothing after leaving, build the leave save at once, and warn when an unsaved project is discarded --- src/components/map-projects/MapProject.jsx | 32 +++++++---- .../map-projects/__tests__/autosave.test.js | 56 +++++++++++++------ .../map-projects/__tests__/leaveGuard.test.js | 30 ++++++++-- src/components/map-projects/autosave.js | 20 ++++--- src/components/map-projects/leaveGuard.js | 10 +++- src/i18n/locales/en/translations.json | 1 + src/i18n/locales/es/translations.json | 1 + src/i18n/locales/pt-BR/translations.json | 1 + src/i18n/locales/zh/translations.json | 1 + src/services/__tests__/retry.test.js | 30 ++++++++++ src/services/retry.js | 2 + 11 files changed, 139 insertions(+), 45 deletions(-) create mode 100644 src/services/__tests__/retry.test.js diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index da92cfe..c7f53d7 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -390,7 +390,7 @@ const MapProject = () => { const saveNowOnLeave = () => saveOnLeave({ scheduler: autosaveSchedulerRef.current, inFlightSave: inFlightSaveRef.current, - save: reasons => currentUserToken() && onSaveRef.current({source: 'auto', closeConfigure: false, reasons, leaving: true}) + save: (reasons, {after}) => currentUserToken() && onSaveRef.current({source: 'auto', closeConfigure: false, reasons, leaving: true, after}) }) // Leaving stops a run at once, not gracefully: its record closes now and @@ -1766,12 +1766,16 @@ const MapProject = () => { const isUpdate = Boolean(project?.id) if(!isAutoSave) GAService.recordUpsertEvent('MapProject', isUpdate, 'map_project') - let service = APIService.new().overrideURL(owner).appendToUrl('map-projects/') - if(isUpdate) - service = service.appendToUrl(project.id + '/') - .put(formData, null, {"Content-Type": "multipart/form-data"}, undefined, true) - else - service = service.post(formData, null, {"Content-Type": "multipart/form-data"}, undefined, true) + const send = () => { + const service = APIService.new().overrideURL(owner).appendToUrl('map-projects/') + if(isUpdate) + return service.appendToUrl(project.id + '/') + .put(formData, null, {"Content-Type": "multipart/form-data"}, undefined, true) + return service.post(formData, null, {"Content-Type": "multipart/form-data"}, undefined, true) + } + // options.after: a save still in flight. This payload is built now and + // sent once that settles, so the older one can't land after it. + const service = options.after ? Promise.resolve(options.after).then(send, send) : send() // Where the user was when saving: a save that lands once they went // elsewhere mustn't pull them back. @@ -1835,6 +1839,7 @@ const MapProject = () => { inFlightSaveRef.current = request if(isUpdate) trackSave(project.id, request) + return request } // onSave takes options, so it can't be bound to onClick directly — the click @@ -4497,7 +4502,10 @@ const MapProject = () => { if(!isNumber(index)) return null // A run's rerank ends when the run stops, and a stopped run proposes no // more mappings; a row reranked by hand isn't stopped (ocl_issues#2849). - const isStopped = isRunTraffic ? getRunStopCheck() : () => false + const runStop = isRunTraffic ? getRunStopCheck() : () => false + // Nothing is sent once the page is gone. + const isStopped = () => !isMountedRef.current || runStop() + if(!isMountedRef.current) return null // The run this call belongs to, captured now: a newer run's stages are // its own (ocl_issues#2849). const runTicket = autoMatchRunTicketRef.current @@ -4724,7 +4732,7 @@ const MapProject = () => { || (Array.isArray(def?.names) && def.names.some(n => n?.name)) ) const scheduleRerank = (rowIndex) => { - if(!isNumber(rowIndex)) return + if(!isNumber(rowIndex) || !isMountedRef.current) return const stage = rowStageRef.current[rowIndex] || {} // ocl_online#105 Phase 5: this debounced path does most of the reranking // during a bulk auto-match (candidates stream in asynchronously, so the @@ -4772,7 +4780,7 @@ const MapProject = () => { const scheduledTicket = autoMatchRunTicketRef.current rerankDebounceRef.current[rowIndex] = setTimeout(() => { delete rerankDebounceRef.current[rowIndex] - if(isRunTraffic && scheduledTicket !== autoMatchRunTicketRef.current) + if(!isMountedRef.current || (isRunTraffic && scheduledTicket !== autoMatchRunTicketRef.current)) return // isBulk=false (don't double the setAutoMatched side-effect the explicit // bulk rerank already performs); isRunTraffic carries the run attribution. @@ -6046,7 +6054,7 @@ const MapProject = () => { baseDelayMs: 3000, backoffFactor: 4, jitterFactor: 0.25, - isCancelled: () => abortRef.current, + isCancelled: () => abortRef.current || !isMountedRef.current, isRetryable: isTransientNetworkError, onAttemptFailed: (err, attempt) => { const errorMessage = err?.response?.data?.detail || err?.message || t('unknown_error') @@ -6875,7 +6883,7 @@ const MapProject = () => { {t('map_project.leave_while_running_title')} - {t('map_project.leave_while_running_body')} + {t(project?.id ? 'map_project.leave_while_running_body' : 'map_project.leave_while_running_body_unsaved')} diff --git a/src/components/map-projects/__tests__/autosave.test.js b/src/components/map-projects/__tests__/autosave.test.js index 181c180..dece609 100644 --- a/src/components/map-projects/__tests__/autosave.test.js +++ b/src/components/map-projects/__tests__/autosave.test.js @@ -233,33 +233,55 @@ test('saveOnLeave: nothing pending, nothing saved', () => { assert.deepEqual(leaveSaves, []) }) -test('saveOnLeave: waits for a save in flight, so the newer payload lands last', async () => { +test('saveOnLeave: builds the payload at once, to send after the save in flight', () => { const { scheduler } = setup() - const order = [] - let finish - const inFlightSave = new Promise(resolve => { finish = resolve }).then(() => order.push('in-flight save')) + const inFlightSave = new Promise(() => {}) + const leaveSaves = [] scheduler.schedule('decision_change') - saveOnLeave({ scheduler, inFlightSave, save: reasons => order.push(`leave save: ${reasons}`) }) - assert.deepEqual(order, [], 'must not save on top of the in-flight save') + saveOnLeave({ scheduler, inFlightSave, save: (reasons, {after}) => leaveSaves.push({reasons, after}) }) - finish() - await inFlightSave - await nextTick() - assert.deepEqual(order, ['in-flight save', 'leave save: decision_change']) + // Called now, before anything after leaving can change what it saves. + assert.equal(leaveSaves.length, 1) + assert.deepEqual(leaveSaves[0].reasons, ['decision_change']) + assert.equal(leaveSaves[0].after, inFlightSave) }) -test('saveOnLeave: still saves when the in-flight save fails', async () => { +test('saveOnLeave: a rejected leave save is logged', async () => { const { scheduler } = setup() - const leaveSaves = [] - const inFlightSave = Promise.reject(new Error('network')) + const logged = [] + const originalError = console.error + console.error = (...args) => logged.push(args[0]) + try { + scheduler.schedule('decision_change') + saveOnLeave({ scheduler, inFlightSave: null, save: () => Promise.reject(new Error('network')) }) + await nextTick() + } finally { + console.error = originalError + } + assert.deepEqual(logged, ['Mapper: saving on leave failed']) +}) - scheduler.schedule('decision_change') - saveOnLeave({ scheduler, inFlightSave, save: reasons => leaveSaves.push(reasons) }) +test('saveOnLeave: reopening the project waits for the leave save, not just the save before it', async () => { + const { scheduler } = setup() + const order = [] + let finishFirst + let finishLeave + const first = trackSave('project-chain', new Promise(resolve => { finishFirst = resolve }).then(() => order.push('first save'))) + const send = () => new Promise(resolve => { finishLeave = resolve }).then(() => order.push('leave save')) - await inFlightSave.catch(() => {}) + scheduler.schedule('auto_match_stopped') + // As onSave does with {after}: tracked now, sent once the first settles. + saveOnLeave({ scheduler, inFlightSave: first, save: (reasons, {after}) => trackSave('project-chain', Promise.resolve(after).then(send, send)) }) + const reopened = whenSaved('project-chain').then(() => order.push('reopen')) + + finishFirst() + await first await nextTick() - assert.deepEqual(leaveSaves, [['decision_change']]) + assert.deepEqual(order, ['first save'], 'the reopen still waits') + finishLeave() + await reopened + assert.deepEqual(order, ['first save', 'leave save', 'reopen']) }) // Saves that outlive their page: reopening the project waits for them, and diff --git a/src/components/map-projects/__tests__/leaveGuard.test.js b/src/components/map-projects/__tests__/leaveGuard.test.js index d89fadd..f4c9916 100644 --- a/src/components/map-projects/__tests__/leaveGuard.test.js +++ b/src/components/map-projects/__tests__/leaveGuard.test.js @@ -54,17 +54,17 @@ test('continueLeaving replays a held navigation once unblocked', () => { let held = null const unblock = blockLeaving(history, {onBlocked: next => { held = next }}) - history.replace('/elsewhere') + history.replace({pathname: '/elsewhere'}) unblock() continueLeaving(history, held) - assert.deepEqual(history.calls, [{action: 'REPLACE', location: '/elsewhere'}]) + assert.deepEqual(history.calls, [{action: 'REPLACE', location: {pathname: '/elsewhere', state: undefined}}]) }) test('continueLeaving pushes a held Back', () => { const history = createFakeHistory() - continueLeaving(history, {location: '/back', action: 'POP'}) - assert.deepEqual(history.calls, [{action: 'PUSH', location: '/back'}]) + continueLeaving(history, {location: {pathname: '/back'}, action: 'POP'}) + assert.deepEqual(history.calls, [{action: 'PUSH', location: {pathname: '/back', state: undefined}}]) }) test('a run closer closes its run once, whoever calls first', async () => { @@ -170,3 +170,25 @@ test('getPageKey gives a new page on Back to an entry a save pushed', () => { const back = getPageKey(other, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'POP') assert.deepEqual(back, {key: other.key + 1, path: '/p/1'}) }) + +test('continueLeaving drops the save marker, so a held Forward to a saved entry gets a new page', () => { + // new saved as /p/1 (kept), Back to new, Forward to /p/1 held by the dialog + // and continued as a push. + const history = createFakeHistory() + const page = getPageKey(start, {pathname: '/map-projects/new'}, 'POP') + const saved = getPageKey(page, {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, 'PUSH') + const back = getPageKey(saved, {pathname: '/map-projects/new'}, 'POP') + + continueLeaving(history, {location: {pathname: '/p/1', state: {mapProjectFrom: '/map-projects/new'}}, action: 'POP'}) + const [pushed] = history.calls + const forward = getPageKey(back, pushed.location, pushed.action) + + assert.equal(pushed.location.state, undefined) + assert.deepEqual(forward, {key: back.key + 1, path: '/p/1'}) +}) + +test('continueLeaving keeps other state', () => { + const history = createFakeHistory() + continueLeaving(history, {location: {pathname: '/x', state: {mapProjectFrom: '/y', other: 1}}, action: 'PUSH'}) + assert.deepEqual(history.calls[0].location.state, {other: 1}) +}) diff --git a/src/components/map-projects/autosave.js b/src/components/map-projects/autosave.js index 1b32b6e..a1b62b8 100644 --- a/src/components/map-projects/autosave.js +++ b/src/components/map-projects/autosave.js @@ -83,12 +83,12 @@ export const createAutosaveScheduler = ({ * Leaving the project unmounts MapProject, often inside the autosave window, * and dropping the pending autosave there lost the last change — a whole Auto * Match run if the user left within 5s of it finishing - * (OpenConceptLab/ocl_issues#2829). This saves it now instead. A save still in - * flight goes first, so the older payload can't land after the newer one. + * (OpenConceptLab/ocl_issues#2829). This saves it now instead. * - * The save always starts asynchronously: an error while building the payload - * must not surface inside React's unmount and take down the page the user is - * going to. + * save(reasons, {after}) builds the payload now, so nothing that changes once + * the page is gone gets into it, and sends it once after, the save still in + * flight, settles, so the older payload can't land after the newer one. An + * error building it is logged, never thrown into React's unmount. * * Returns whether a save was queued. */ @@ -96,10 +96,12 @@ export const saveOnLeave = ({ scheduler, inFlightSave, save }) => { const reasons = scheduler.takePending() if(!reasons) return false - const run = () => save(reasons) - Promise.resolve(inFlightSave) - .then(run, run) - .catch(error => console.error('Mapper: saving on leave failed', error)) + const fail = error => console.error('Mapper: saving on leave failed', error) + try { + Promise.resolve(save(reasons, {after: inFlightSave})).catch(fail) + } catch(error) { + fail(error) + } return true } diff --git a/src/components/map-projects/leaveGuard.js b/src/components/map-projects/leaveGuard.js index 53a66a8..8f0f3d7 100644 --- a/src/components/map-projects/leaveGuard.js +++ b/src/components/map-projects/leaveGuard.js @@ -10,9 +10,13 @@ export const blockLeaving = (history, {isAllowed = () => false, onBlocked}) => return false }) -// A held Back/Forward (POP) can't be replayed by distance, so it is pushed. -export const continueLeaving = (history, {location, action}) => - action === 'REPLACE' ? history.replace(location) : history.push(location) +// A held Back/Forward (POP) can't be replayed by distance, so it is pushed, +// without its mapProjectFrom: the push is the user's, not a save's. +export const continueLeaving = (history, {location, action}) => { + const { mapProjectFrom, ...state } = location.state || {} // eslint-disable-line no-unused-vars + const next = {...location, state: Object.keys(state).length ? state : undefined} + return action === 'REPLACE' ? history.replace(next) : history.push(next) +} // Each closer closes its run's record once, whether the run ends or the page // closes it first on leave. A close still in flight as the tab closes is sent diff --git a/src/i18n/locales/en/translations.json b/src/i18n/locales/en/translations.json index 03b3101..40e03d6 100644 --- a/src/i18n/locales/en/translations.json +++ b/src/i18n/locales/en/translations.json @@ -375,6 +375,7 @@ "stop_processing": "Stop Processing", "leave_while_running_title": "Auto Match is running", "leave_while_running_body": "Leaving this project stops the run now. What it has matched so far is saved.", + "leave_while_running_body_unsaved": "This project isn't saved yet. Leaving stops the run now and discards the project and its results.", "stay": "Stay", "stop_and_leave": "Stop and leave", "auto_match_all_including_approved_confirm": "I understand that {{approvedCount}} approved decisions will be overwritten", diff --git a/src/i18n/locales/es/translations.json b/src/i18n/locales/es/translations.json index cfa1dac..3ef8477 100644 --- a/src/i18n/locales/es/translations.json +++ b/src/i18n/locales/es/translations.json @@ -361,6 +361,7 @@ "stop_processing": "Detener Procesamiento", "leave_while_running_title": "Auto Match está en ejecución", "leave_while_running_body": "Salir de este proyecto detiene la ejecución ahora. Lo que ya se ha emparejado se guarda.", + "leave_while_running_body_unsaved": "Este proyecto aún no se ha guardado. Salir detiene la ejecución ahora y descarta el proyecto y sus resultados.", "stay": "Quedarse", "stop_and_leave": "Detener y salir", "auto_match_all_including_approved_confirm": "Entiendo que se sobrescribirán {{approvedCount}} decisiones aprobadas", diff --git a/src/i18n/locales/pt-BR/translations.json b/src/i18n/locales/pt-BR/translations.json index 74d6902..ccf4006 100644 --- a/src/i18n/locales/pt-BR/translations.json +++ b/src/i18n/locales/pt-BR/translations.json @@ -375,6 +375,7 @@ "stop_processing": "Parar Processamento", "leave_while_running_title": "O Auto Match está em execução", "leave_while_running_body": "Sair deste projeto interrompe a execução agora. O que já foi correspondido é salvo.", + "leave_while_running_body_unsaved": "Este projeto ainda não foi salvo. Sair interrompe a execução agora e descarta o projeto e seus resultados.", "stay": "Ficar", "stop_and_leave": "Parar e sair", "auto_match_all_including_approved_confirm": "Entendo que {{approvedCount}} decisões aprovadas serão sobrescritas", diff --git a/src/i18n/locales/zh/translations.json b/src/i18n/locales/zh/translations.json index efcfe94..acf56c8 100644 --- a/src/i18n/locales/zh/translations.json +++ b/src/i18n/locales/zh/translations.json @@ -373,6 +373,7 @@ "stop_processing": "停止处理", "leave_while_running_title": "自动匹配正在运行", "leave_while_running_body": "离开此项目会立即停止运行。已匹配的结果会被保存。", + "leave_while_running_body_unsaved": "此项目尚未保存。离开会立即停止运行，并丢弃该项目及其结果。", "stay": "留下", "stop_and_leave": "停止并离开", "auto_match_all_including_approved_confirm": "我明白将覆盖 {{approvedCount}} 个已批准的决策", diff --git a/src/services/__tests__/retry.test.js b/src/services/__tests__/retry.test.js new file mode 100644 index 0000000..4e47785 --- /dev/null +++ b/src/services/__tests__/retry.test.js @@ -0,0 +1,30 @@ +import test from 'node:test' +import assert from 'node:assert/strict' + +import { retryWithBackoff } from '../retry.js' + +test('retryWithBackoff: cancelled during the wait, it sends nothing more', async () => { + let cancelled = false + let attempts = 0 + const fn = async () => { + attempts++ + // The user leaves while the retry waits. + setTimeout(() => { cancelled = true }, 0) + throw new Error('connect failed') + } + + await assert.rejects(retryWithBackoff(fn, {baseDelayMs: 5, jitterFactor: 0, isRetryable: () => true, isCancelled: () => cancelled}), /connect failed/) + assert.equal(attempts, 1) +}) + +test('retryWithBackoff: retries a retryable failure', async () => { + let attempts = 0 + const result = await retryWithBackoff(async () => { + if(++attempts < 3) + throw new Error('busy') + return 'ok' + }, {baseDelayMs: 1, jitterFactor: 0, isRetryable: () => true}) + + assert.equal(result, 'ok') + assert.equal(attempts, 3) +}) diff --git a/src/services/retry.js b/src/services/retry.js index 2150783..d39b79f 100644 --- a/src/services/retry.js +++ b/src/services/retry.js @@ -19,6 +19,8 @@ export const retryWithBackoff = async (fn, { maxRetries = 2, baseDelayMs = 3000, const base = baseDelayMs * Math.pow(backoffFactor, attempt); const delay = base * (1 - jitterFactor + Math.random() * jitterFactor * 2); await sleep(delay); + // Cancelled while waiting: don't send it again. + if (isCancelled()) throw err; } } };