From 091c1463090f08a95963d704406cc13d49d4c11b Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Wed, 12 Mar 2025 23:54:56 +0530 Subject: [PATCH] chore: Refactoring focus strategy to get the correct key name on history removal for all IDEs (#39689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Refactoring focus strategy to get the correct key name on history removal for all IDEs Fixes [#39597](https://github.com/appsmithorg/appsmith/issues/39597) ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: ee66ead13719fbb882567cb2697af4cab10a5294 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Wed, 12 Mar 2025 14:19:55 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Added an enhanced mechanism for generating unique navigation keys that include branch details, improving focus state management. - **Refactor** - Restructured module organization to streamline focus history handling and overall navigation state operations. - **Chores** - Simplified URL assignment for improved clarity and maintainability. --------- Co-authored-by: Hetu Nandu --- ...{FocusSetters.ts => AppIDEFocusSetters.ts} | 0 .../src/ce/navigation/FocusElements/AppIDE.ts | 4 +- .../FocusStrategy/AppIDEFocusStrategy.ts | 12 +++- .../FocusStrategy/NoIDEFocusStrategy.ts | 7 ++- .../src/ce/navigation/FocusStrategy/types.ts | 50 +++++++++++++++ app/client/src/ce/sagas/DatasourcesSagas.ts | 2 +- .../src/ee/navigation/AppIDEFocusSetters.ts | 1 + app/client/src/ee/navigation/FocusSetters.ts | 1 - .../src/ee/navigation/FocusStrategy/types.ts | 1 + app/client/src/sagas/FocusRetentionSaga.ts | 61 +++++-------------- 10 files changed, 85 insertions(+), 54 deletions(-) rename app/client/src/ce/navigation/{FocusSetters.ts => AppIDEFocusSetters.ts} (100%) create mode 100644 app/client/src/ce/navigation/FocusStrategy/types.ts create mode 100644 app/client/src/ee/navigation/AppIDEFocusSetters.ts delete mode 100644 app/client/src/ee/navigation/FocusSetters.ts create mode 100644 app/client/src/ee/navigation/FocusStrategy/types.ts diff --git a/app/client/src/ce/navigation/FocusSetters.ts b/app/client/src/ce/navigation/AppIDEFocusSetters.ts similarity index 100% rename from app/client/src/ce/navigation/FocusSetters.ts rename to app/client/src/ce/navigation/AppIDEFocusSetters.ts diff --git a/app/client/src/ce/navigation/FocusElements/AppIDE.ts b/app/client/src/ce/navigation/FocusElements/AppIDE.ts index 9286194b85..040480abdd 100644 --- a/app/client/src/ce/navigation/FocusElements/AppIDE.ts +++ b/app/client/src/ce/navigation/FocusElements/AppIDE.ts @@ -55,10 +55,10 @@ import { setSelectedEntityUrl, setSelectedJSObject, setSelectedQuery, -} from "ee/navigation/FocusSetters"; +} from "ee/navigation/AppIDEFocusSetters"; import { getFirstDatasourceId } from "selectors/datasourceSelectors"; import { FocusElement, FocusElementConfigType } from "navigation/FocusElements"; -import type { FocusElementsConfigList } from "sagas/FocusRetentionSaga"; +import type { FocusElementsConfigList } from "ee/navigation/FocusStrategy/types"; import { ActionExecutionResizerHeight } from "PluginActionEditor/components/PluginActionResponse/constants"; import { getPluginActionConfigSelectedTab, diff --git a/app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts b/app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts index a358014d55..fe90514245 100644 --- a/app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts +++ b/app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts @@ -1,5 +1,8 @@ import { all, select, take } from "redux-saga/effects"; -import type { FocusPath, FocusStrategy } from "sagas/FocusRetentionSaga"; +import type { + FocusPath, + FocusStrategy, +} from "ee/navigation/FocusStrategy/types"; import type { AppsmithLocationState } from "utils/history"; import { NavigationMethod } from "utils/history"; import type { FocusEntityInfo } from "navigation/FocusEntity"; @@ -288,6 +291,13 @@ export const AppIDEFocusStrategy: FocusStrategy = { // We do not have to add any query params because this url is used as the key return parentUrl.split("?")[0]; }, + getUrlKey: function* (url: string) { + const branch: string | undefined = yield select( + selectGitApplicationCurrentBranch, + ); + + return `${url}#${branch}`; + }, *waitForPathLoad(currentPath: string, previousPath?: string) { if (previousPath) { // When page is changing, there may be some items still not loaded, diff --git a/app/client/src/ce/navigation/FocusStrategy/NoIDEFocusStrategy.ts b/app/client/src/ce/navigation/FocusStrategy/NoIDEFocusStrategy.ts index 11a1d37ae3..e9f6d0ee3d 100644 --- a/app/client/src/ce/navigation/FocusStrategy/NoIDEFocusStrategy.ts +++ b/app/client/src/ce/navigation/FocusStrategy/NoIDEFocusStrategy.ts @@ -1,4 +1,4 @@ -import type { FocusStrategy } from "sagas/FocusRetentionSaga"; +import type { FocusStrategy } from "ee/navigation/FocusStrategy/types"; import NoIDEFocusElements from "../FocusElements/NoIDE"; export const NoIDEFocusStrategy: FocusStrategy = { focusElements: NoIDEFocusElements, @@ -8,8 +8,11 @@ export const NoIDEFocusStrategy: FocusStrategy = { *getEntitiesForStore() { return []; }, - getEntityParentUrl(): string { + getEntityParentUrl: (): string => { return ""; }, + getUrlKey: function* (url: string) { + return url; + }, *waitForPathLoad() {}, }; diff --git a/app/client/src/ce/navigation/FocusStrategy/types.ts b/app/client/src/ce/navigation/FocusStrategy/types.ts new file mode 100644 index 0000000000..d4bf16562e --- /dev/null +++ b/app/client/src/ce/navigation/FocusStrategy/types.ts @@ -0,0 +1,50 @@ +import type { FocusEntityInfo } from "navigation/FocusEntity"; +import type { FocusEntity } from "navigation/FocusEntity"; +import type { FocusElementConfig } from "navigation/FocusElements"; +import type { AppsmithLocationState } from "utils/history"; + +export interface FocusPath { + key: string; + entityInfo: FocusEntityInfo; +} + +export type FocusElementsConfigList = { + [key in FocusEntity]?: FocusElementConfig[]; +}; + +export interface FocusStrategy { + focusElements: FocusElementsConfigList; + /** based on the route change, what states need to be set in the upcoming route **/ + getEntitiesForSet: ( + previousPath: string, + currentPath: string, + state: AppsmithLocationState, + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) => Generator, any>; + /** based on the route change, what states need to be stored for the previous route **/ + getEntitiesForStore: ( + path: string, + currentPath: string, + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) => Generator, any>; + /** For entities with hierarchy, return the parent entity path for storing its state **/ + getEntityParentUrl: ( + entityInfo: FocusEntityInfo, + parentEntity: FocusEntity, + ) => string; + /** Get focus history key for the URL */ + getUrlKey: ( + url: string, + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) => Generator; + /** Define a wait (saga) before we start setting states **/ + waitForPathLoad: ( + currentPath: string, + previousPath: string, + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) => Generator; +} diff --git a/app/client/src/ce/sagas/DatasourcesSagas.ts b/app/client/src/ce/sagas/DatasourcesSagas.ts index a3230c23cf..5f41d17faf 100644 --- a/app/client/src/ce/sagas/DatasourcesSagas.ts +++ b/app/client/src/ce/sagas/DatasourcesSagas.ts @@ -447,7 +447,7 @@ export function* deleteDatasourceSaga( const isValidResponse: boolean = yield validateResponse(response); if (isValidResponse) { - const currentUrl = `${window.location.pathname}`; + const currentUrl = window.location.pathname; yield call(handleDatasourceDeleteRedirect, id); yield call(FocusRetention.handleRemoveFocusHistory, currentUrl); diff --git a/app/client/src/ee/navigation/AppIDEFocusSetters.ts b/app/client/src/ee/navigation/AppIDEFocusSetters.ts new file mode 100644 index 0000000000..9d5b562d57 --- /dev/null +++ b/app/client/src/ee/navigation/AppIDEFocusSetters.ts @@ -0,0 +1 @@ +export * from "ce/navigation/AppIDEFocusSetters"; diff --git a/app/client/src/ee/navigation/FocusSetters.ts b/app/client/src/ee/navigation/FocusSetters.ts deleted file mode 100644 index c3eb13e44f..0000000000 --- a/app/client/src/ee/navigation/FocusSetters.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "ce/navigation/FocusSetters"; diff --git a/app/client/src/ee/navigation/FocusStrategy/types.ts b/app/client/src/ee/navigation/FocusStrategy/types.ts new file mode 100644 index 0000000000..fcfa880059 --- /dev/null +++ b/app/client/src/ee/navigation/FocusStrategy/types.ts @@ -0,0 +1 @@ +export * from "ce/navigation/FocusStrategy/types"; diff --git a/app/client/src/sagas/FocusRetentionSaga.ts b/app/client/src/sagas/FocusRetentionSaga.ts index 5a24703479..9e4bac0258 100644 --- a/app/client/src/sagas/FocusRetentionSaga.ts +++ b/app/client/src/sagas/FocusRetentionSaga.ts @@ -21,47 +21,10 @@ import type { Plugin } from "entities/Plugin"; import { getIDETypeByUrl } from "ee/entities/IDE/utils"; import { getIDEFocusStrategy } from "ee/navigation/FocusStrategy"; import { IDE_TYPE } from "ee/IDE/Interfaces/IDETypes"; -import { selectGitApplicationCurrentBranch } from "selectors/gitModSelectors"; - -export interface FocusPath { - key: string; - entityInfo: FocusEntityInfo; -} - -export type FocusElementsConfigList = { - [key in FocusEntity]?: FocusElementConfig[]; -}; - -export interface FocusStrategy { - focusElements: FocusElementsConfigList; - /** based on the route change, what states need to be set in the upcoming route **/ - getEntitiesForSet: ( - previousPath: string, - currentPath: string, - state: AppsmithLocationState, - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) => Generator, any>; - /** based on the route change, what states need to be stored for the previous route **/ - getEntitiesForStore: ( - path: string, - currentPath: string, - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) => Generator, any>; - /** For entities with hierarchy, return the parent entity path for storing its state **/ - getEntityParentUrl: ( - entityInfo: FocusEntityInfo, - parentEntity: FocusEntity, - ) => string; - /** Define a wait (saga) before we start setting states **/ - waitForPathLoad: ( - currentPath: string, - previousPath: string, - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) => Generator; -} +import type { + FocusPath, + FocusStrategy, +} from "ee/navigation/FocusStrategy/types"; /** * Context switching works by restoring the states of ui elements to as they were @@ -123,23 +86,27 @@ class FocusRetention { } public *handleRemoveFocusHistory(url: string) { - const branch: string | undefined = yield select( - selectGitApplicationCurrentBranch, - ); const removeKeys: string[] = []; + const entityKey: string = yield call(this.focusStrategy.getUrlKey, url); const focusEntityInfo = identifyEntityFromPath(url); - removeKeys.push(`${url}#${branch}`); + removeKeys.push(entityKey); const parentElement = FocusStoreHierarchy[focusEntityInfo.entity]; if (parentElement) { - const parentPath = this.focusStrategy.getEntityParentUrl( + const parentUrl: string = yield call( + this.focusStrategy.getEntityParentUrl, focusEntityInfo, parentElement, ); - removeKeys.push(`${parentPath}#${branch}`); + const parentEntityKey: string = yield call( + this.focusStrategy.getUrlKey, + parentUrl, + ); + + removeKeys.push(parentEntityKey); } for (const key of removeKeys) {