From 22eeacd70d6820018c843dd7872d722bc13cfc41 Mon Sep 17 00:00:00 2001 From: jvega190 Date: Mon, 3 Aug 2026 11:35:30 -0600 Subject: [PATCH] [1227] --- .../ui/app/src/state/actions/pathNavigator.ts | 12 ++-- .../ui/app/src/state/epics/pathNavigator.ts | 69 +++++++++++++------ .../app/src/state/reducers/pathNavigator.ts | 56 +++++++++++---- 3 files changed, 98 insertions(+), 39 deletions(-) diff --git a/studio-ui/ui/app/src/state/actions/pathNavigator.ts b/studio-ui/ui/app/src/state/actions/pathNavigator.ts index f538a75e7..0c6c934f1 100644 --- a/studio-ui/ui/app/src/state/actions/pathNavigator.ts +++ b/studio-ui/ui/app/src/state/actions/pathNavigator.ts @@ -94,26 +94,30 @@ export const pathNavigatorFetchParentItems = /*#__PURE__*/ createAction< export const pathNavigatorFetchPath = /*#__PURE__*/ createAction>('PATH_NAV_FETCH_PATH'); +// The `path` on the results below is the path the request was issued for. The navigator may have been moved to a +// different path while the request was in flight (e.g. a background refresh triggered by another user's activity +// racing with the user navigating), in which case the result must be discarded rather than applied. export const pathNavigatorFetchPathComplete = - /*#__PURE__*/ createAction>( + /*#__PURE__*/ createAction>( 'PATH_NAV_FETCH_PATH_COMPLETE' ); export const pathNavigatorBulkFetchPathComplete = /*#__PURE__*/ createAction<{ - paths: PayloadWithId<{ parent?: ContentItem; children: GetChildrenResponse }>[]; + paths: PayloadWithId<{ path: string; parent?: ContentItem; children: GetChildrenResponse }>[]; }>('PATH_NAV_BULK_FETCH_PATH_COMPLETE'); export const pathNavigatorFetchParentItemsComplete = /*#__PURE__*/ createAction< - PayloadWithId<{ items: ContentItem[]; children: GetChildrenResponse }> + PayloadWithId<{ path: string; items: ContentItem[]; children: GetChildrenResponse }> >('PATH_NAV_FETCH_PARENT_ITEMS_COMPLETE'); export const pathNavigatorFetchPathFailed = /*#__PURE__*/ createAction<{ id: string; + path: string; error: Omit; }>('PATH_NAV_FETCH_PATH_FAILED'); export const pathNavigatorBulkFetchPathFailed = /*#__PURE__*/ createAction<{ - ids: string[]; + requests: PayloadWithId<{ path: string }>[]; error: Omit; }>('PATH_NAV_BULK_FETCH_PATH_FAILED'); diff --git a/studio-ui/ui/app/src/state/epics/pathNavigator.ts b/studio-ui/ui/app/src/state/epics/pathNavigator.ts index 146401238..4a4654ed0 100644 --- a/studio-ui/ui/app/src/state/epics/pathNavigator.ts +++ b/studio-ui/ui/app/src/state/epics/pathNavigator.ts @@ -126,12 +126,25 @@ export default [ sortStrategy: state.pathNavigator[id].sortStrategy, order: state.pathNavigator[id].order }).pipe( - map(({ item, children }) => pathNavigatorFetchPathComplete({ id, parent: item, children })), + map(({ item, children }) => + pathNavigatorFetchPathComplete({ + id, + path: state.pathNavigator[id].currentPath, + parent: item, + children + }) + ), catchAjaxError((error: AjaxError) => { - if (error.status === 404 && state.pathNavigator[id].rootPath !== state.pathNavigator[id].currentPath) { + if ( + error.status === 404 && + state.pathNavigator[id].rootPath !== state.pathNavigator[id].currentPath && + // The navigator may have moved on while this refresh was in flight. Sending it to the root + // over a path it already left would discard the user's navigation. + state$.value.pathNavigator[id]?.currentPath === state.pathNavigator[id].currentPath + ) { return pathNavigatorConditionallySetPath({ id, path: state.pathNavigator[id].rootPath }); } else { - return pathNavigatorFetchPathFailed({ error, id }); + return pathNavigatorFetchPathFailed({ error, id, path: state.pathNavigator[id].currentPath }); } }) ) @@ -147,10 +160,14 @@ export default [ const { requests } = payload; let paths = []; let optionsByPath = {}; + // The path each navigator is being refreshed for, so that results can be discarded if it navigates away + // before they arrive. + const pathById = {}; requests.forEach(({ id }) => { const chunk = state.pathNavigator[id]; const { currentPath, keyword, limit, offset, excludes, sortStrategy, order } = chunk; + pathById[id] = currentPath; paths.push(currentPath); optionsByPath[currentPath] = { keyword, @@ -171,14 +188,18 @@ export default [ pathNavigatorBulkFetchPathComplete({ paths: requests.map(({ id }) => ({ id, - parent: items.find((item) => - item.path.startsWith(withoutIndex(state.pathNavigator[id].currentPath)) - ), - children: children[state.pathNavigator[id].currentPath] + path: pathById[id], + parent: items.find((item) => item.path.startsWith(withoutIndex(pathById[id]))), + children: children[pathById[id]] })) }) ), - catchAjaxError((error) => pathNavigatorBulkFetchPathFailed({ ids: requests.map(({ id }) => id), error })) + catchAjaxError((error) => + pathNavigatorBulkFetchPathFailed({ + requests: requests.map(({ id }) => ({ id, path: pathById[id] })), + error + }) + ) ) : EMPTY; }) @@ -204,9 +225,9 @@ export default [ order: state.pathNavigator[id].order, ...(keyword && { keyword }) }).pipe( - map(({ item, children }) => pathNavigatorFetchPathComplete({ id, parent: item, children })), + map(({ item, children }) => pathNavigatorFetchPathComplete({ id, path, parent: item, children })), catchAjaxError( - (error) => pathNavigatorFetchPathFailed({ id, error }), + (error) => pathNavigatorFetchPathFailed({ id, path, error }), (error) => pushErrorDialog({ props: { error: error.response ?? error } }) ) ) @@ -259,8 +280,8 @@ export default [ sortStrategy: state.pathNavigator[id].sortStrategy, order: state.pathNavigator[id].order }).pipe( - map(({ item, children }) => pathNavigatorFetchPathComplete({ id, parent: item, children })), - catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id })) + map(({ item, children }) => pathNavigatorFetchPathComplete({ id, path, parent: item, children })), + catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id, path })) ) ) ), @@ -288,11 +309,14 @@ export default [ map((children) => pathNavigatorFetchPathComplete({ id, + path: state.pathNavigator[id].currentPath, parent: state.content.itemsByPath[state.pathNavigator[id].currentPath], children }) ), - catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id })) + catchAjaxError((error) => + pathNavigatorFetchPathFailed({ error, id, path: state.pathNavigator[id].currentPath }) + ) ) ) ), @@ -318,8 +342,12 @@ export default [ ...(Boolean(state.pathNavigator[id].keyword) && { keyword: state.pathNavigator[id].keyword }), offset }).pipe( - map((children) => pathNavigatorFetchPathComplete({ id, children })), - catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id })) + map((children) => + pathNavigatorFetchPathComplete({ id, path: state.pathNavigator[id].currentPath, children }) + ), + catchAjaxError((error) => + pathNavigatorFetchPathFailed({ error, id, path: state.pathNavigator[id].currentPath }) + ) ) ) ), @@ -351,13 +379,12 @@ export default [ order }) ]).pipe( - map(([items, children]) => pathNavigatorFetchParentItemsComplete({ id, items, children })), + map(([items, children]) => pathNavigatorFetchParentItemsComplete({ id, path, items, children })), catchAjaxError((error: AjaxError) => { - if (error.status === 404) { + if (error.status === 404 && state$.value.pathNavigator[id]?.currentPath === path) { return pathNavigatorConditionallySetPath({ id, path: getRootPath(path) }); - } else { - return pathNavigatorFetchPathFailed({ error, id }); } + return pathNavigatorFetchPathFailed({ error, id, path }); }) ); } else { @@ -369,8 +396,8 @@ export default [ sortStrategy: state.pathNavigator[id].sortStrategy, order: state.pathNavigator[id].order }).pipe( - map(({ item, children }) => pathNavigatorFetchPathComplete({ id, parent: item, children })), - catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id })) + map(({ item, children }) => pathNavigatorFetchPathComplete({ id, path, parent: item, children })), + catchAjaxError((error) => pathNavigatorFetchPathFailed({ error, id, path })) ); } } diff --git a/studio-ui/ui/app/src/state/reducers/pathNavigator.ts b/studio-ui/ui/app/src/state/reducers/pathNavigator.ts index 823016708..03cd75d5b 100644 --- a/studio-ui/ui/app/src/state/reducers/pathNavigator.ts +++ b/studio-ui/ui/app/src/state/reducers/pathNavigator.ts @@ -51,16 +51,26 @@ import StandardAction from '../../models/StandardAction'; import { CaseReducer } from '@reduxjs/toolkit/src/createReducer'; import GlobalState from '../../models/GlobalState'; +/** + * A result is stale when the navigator has been moved to a different path since the request was issued. Applying it + * would revert the navigation the user has already made. + */ +const isStaleResult = (chunk, requestedPath: string) => + Boolean(requestedPath) && withoutIndex(requestedPath) !== withoutIndex(chunk.currentPath); + const updatePath = (state, payload) => { - const { id, parent, children } = payload; + const { id, path: requestedPath, parent, children } = payload; + const chunk = state[id]; + if (!chunk || isStaleResult(chunk, requestedPath)) { + return; + } if ( // If it's not the first page, and the fetched data has no children, stay on the previous page. !(children.offset >= children.limit && children.length === 0) ) { - const chunk = state[id]; - const path = parent?.path ?? state[id].currentPath; + const path = parent?.path ?? chunk.currentPath; chunk.currentPath = path; - chunk.breadcrumb = getIndividualPaths(withoutIndex(path), withoutIndex(state[id].rootPath)); + chunk.breadcrumb = getIndividualPaths(withoutIndex(path), withoutIndex(chunk.rootPath)); chunk.itemsInPath = children.length === 0 ? [] : children.map((item) => item.path); chunk.levelDescriptor = children.levelDescriptor?.path; chunk.total = children.total; @@ -161,8 +171,15 @@ const reducer = createReducer({}, (builder) => { state[payload.id].error = payload.error; }) .addCase(pathNavigatorFetchPath, (state, { payload }) => { - state[payload.id].isFetching = true; - state[payload.id].error = null; + const chunk = state[payload.id]; + chunk.isFetching = true; + chunk.error = null; + if (payload.path) { + // Move to the requested path right away so that refreshes issued while this request is in flight fetch + // (and are validated against) the path the user navigated to, instead of the one being left behind. + chunk.currentPath = payload.path; + chunk.breadcrumb = getIndividualPaths(withoutIndex(payload.path), withoutIndex(chunk.rootPath)); + } }) .addCase(pathNavigatorFetchPathComplete, (state, { payload }) => { updatePath(state, payload); @@ -172,14 +189,22 @@ const reducer = createReducer({}, (builder) => { updatePath(state, path); }); }) - .addCase(pathNavigatorFetchPathFailed, (state, { payload: { id, error } }) => { - state[id].isFetching = false; - state[id].error = error; + .addCase(pathNavigatorFetchPathFailed, (state, { payload: { id, path, error } }) => { + const chunk = state[id]; + if (!chunk || isStaleResult(chunk, path)) { + return; + } + chunk.isFetching = false; + chunk.error = error; }) - .addCase(pathNavigatorBulkFetchPathFailed, (state, { payload: { ids, error } }) => { - ids.forEach((id) => { - state[id].isFetching = false; - state[id].error = error; + .addCase(pathNavigatorBulkFetchPathFailed, (state, { payload: { requests, error } }) => { + requests.forEach(({ id, path }) => { + const chunk = state[id]; + if (!chunk || isStaleResult(chunk, path)) { + return; + } + chunk.isFetching = false; + chunk.error = error; }); }) .addCase(pathNavigatorFetchParentItems, (state, { payload: { id, path } }) => { @@ -187,8 +212,11 @@ const reducer = createReducer({}, (builder) => { state[id].currentPath = path; state[id].error = null; }) - .addCase(pathNavigatorFetchParentItemsComplete, (state, { payload: { id, children } }) => { + .addCase(pathNavigatorFetchParentItemsComplete, (state, { payload: { id, path, children } }) => { const chunk = state[id]; + if (!chunk || isStaleResult(chunk, path)) { + return; + } const { currentPath, rootPath } = chunk; chunk.itemsInPath = children.map((item) => item.path); chunk.levelDescriptor = children.levelDescriptor?.path ?? null;