From 3757f3197f001951b51b1989b29e0449c7e55e83 Mon Sep 17 00:00:00 2001 From: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com> Date: Tue, 20 Feb 2024 10:31:57 +0530 Subject: [PATCH] fix: Module instance delete navigation (#31140) ## Description This PR addresses the below issues, 1. In App editor, deleting a package is not redirecting to add screen 2. In package Editor, deleting a js/query should fall back to the main 3. Icon not displaying for tabs of package instance #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/30894 #### Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed ## Summary by CodeRabbit - **New Features** - Added IDE type detection for enhanced redirection logic in code editing workflows. - Introduced customizable module icons in the Explorer view for better visual differentiation. - **Enhancements** - Improved IDE tab management to support JavaScript and Query modules, ensuring a smoother development experience. --- app/client/src/ce/sagas/JSActionSagas.ts | 9 ++++++--- .../pages/Editor/Explorer/ExplorerIcons.tsx | 18 ++++++++++++++++++ app/client/src/sagas/ActionSagas.ts | 7 +++++-- app/client/src/sagas/IDESaga.tsx | 18 ++++++++++++------ 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/app/client/src/ce/sagas/JSActionSagas.ts b/app/client/src/ce/sagas/JSActionSagas.ts index 33d450319c..01c2ec2286 100644 --- a/app/client/src/ce/sagas/JSActionSagas.ts +++ b/app/client/src/ce/sagas/JSActionSagas.ts @@ -70,6 +70,8 @@ import { getWidgets } from "sagas/selectors"; import { removeFocusHistoryRequest } from "actions/focusHistoryActions"; import { getIsEditorPaneSegmentsEnabled } from "@appsmith/selectors/featureFlagsSelectors"; import { handleJSEntityRedirect } from "sagas/IDESaga"; +import { getIDETypeByUrl } from "@appsmith/entities/IDE/utils"; +import { IDE_TYPE } from "@appsmith/entities/IDE/constants"; export function* fetchJSCollectionsSaga( action: EvaluationReduxAction, @@ -281,22 +283,23 @@ export function* deleteJSCollectionSaga( ) { try { const id = actionPayload.payload.id; + const currentUrl = window.location.pathname; const pageId: string = yield select(getCurrentPageId); const response: ApiResponse = yield JSActionAPI.deleteJSCollection(id); const isValidResponse: boolean = yield validateResponse(response); + const ideType = getIDETypeByUrl(currentUrl); if (isValidResponse) { // @ts-expect-error: response.data is of type unknown toast.show(createMessage(JS_ACTION_DELETE_SUCCESS, response.data.name), { kind: "success", }); - const currentUrl = window.location.pathname; const isEditorPaneSegmentsEnabled: boolean = yield select( getIsEditorPaneSegmentsEnabled, ); - if (isEditorPaneSegmentsEnabled) { + if (isEditorPaneSegmentsEnabled && ideType === IDE_TYPE.App) { yield call(handleJSEntityRedirect, id); - } else if (pageId) { + } else { history.push(builderURL({ pageId })); } yield put(removeFocusHistoryRequest(currentUrl)); diff --git a/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx b/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx index 6ecfa5056d..19d02fa9ed 100644 --- a/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx +++ b/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx @@ -334,3 +334,21 @@ export function AppsmithAIIcon() { export function ActionUrlIcon(url: string) { return ; } + +export function ModuleIcon( + height = 18, + width = 18, + noBackground = false, + noBorder = false, +) { + return ( + + + + ); +} diff --git a/app/client/src/sagas/ActionSagas.ts b/app/client/src/sagas/ActionSagas.ts index 17bc3799f8..e9abe6ac4f 100644 --- a/app/client/src/sagas/ActionSagas.ts +++ b/app/client/src/sagas/ActionSagas.ts @@ -137,6 +137,8 @@ import { removeFocusHistoryRequest } from "../actions/focusHistoryActions"; import { getIsEditorPaneSegmentsEnabled } from "@appsmith/selectors/featureFlagsSelectors"; import { resolveParentEntityMetadata } from "@appsmith/sagas/helpers"; import { handleQueryEntityRedirect } from "./IDESaga"; +import { IDE_TYPE } from "@appsmith/entities/IDE/constants"; +import { getIDETypeByUrl } from "@appsmith/entities/IDE/utils"; export const DEFAULT_PREFIX = { QUERY: "Query", @@ -575,7 +577,9 @@ export function* deleteActionSaga( try { const id = actionPayload.payload.id; const name = actionPayload.payload.name; + const currentUrl = window.location.pathname; const action: Action | undefined = yield select(getAction, id); + const ideType = getIDETypeByUrl(currentUrl); if (!action) return; @@ -610,12 +614,11 @@ export function* deleteActionSaga( queryName: name, }); } - const currentUrl = window.location.pathname; const isEditorPaneSegmentsEnabled: boolean = yield select( getIsEditorPaneSegmentsEnabled, ); - if (isEditorPaneSegmentsEnabled) { + if (isEditorPaneSegmentsEnabled && ideType === IDE_TYPE.App) { yield call(handleQueryEntityRedirect, action.id); } else { if (!!actionPayload.payload.onSuccess) { diff --git a/app/client/src/sagas/IDESaga.tsx b/app/client/src/sagas/IDESaga.tsx index 80e88aa742..56afe727d5 100644 --- a/app/client/src/sagas/IDESaga.tsx +++ b/app/client/src/sagas/IDESaga.tsx @@ -19,12 +19,18 @@ import log from "loglevel"; export function* updateIDETabsOnRouteChangeSaga(entityInfo: FocusEntityInfo) { const { entity, id } = entityInfo; - if (entity === FocusEntity.JS_OBJECT) { + if ( + entity === FocusEntity.JS_OBJECT || + entity === FocusEntity.JS_MODULE_INSTANCE + ) { const jsTabs: string[] = yield select(getJSTabs); const newTabs: string[] = yield call(getUpdatedTabs, id, jsTabs); yield put(setJSTabs(newTabs)); } - if (entity === FocusEntity.QUERY) { + if ( + entity === FocusEntity.QUERY || + entity === FocusEntity.QUERY_MODULE_INSTANCE + ) { const queryTabs: string[] = yield select(getQueryTabs); const newTabs: string[] = yield call(getUpdatedTabs, id, queryTabs); yield put(setQueryTabs(newTabs)); @@ -46,12 +52,12 @@ export function* handleJSEntityRedirect(deletedId: string) { const redirectAction = getNextEntityAfterDelete(deletedId, allJsItems); switch (redirectAction.action) { case RedirectAction.CREATE: - history.push(jsCollectionAddURL({})); + history.push(jsCollectionAddURL({ pageId })); break; case RedirectAction.ITEM: if (!redirectAction.payload) { log.error("Redirect item does not have a payload"); - history.push(jsCollectionAddURL({})); + history.push(jsCollectionAddURL({ pageId })); break; } const { payload } = redirectAction; @@ -66,11 +72,11 @@ export function* handleQueryEntityRedirect(deletedId: string) { const redirectAction = getNextEntityAfterDelete(deletedId, allQueryItems); switch (redirectAction.action) { case RedirectAction.CREATE: - history.push(queryAddURL({})); + history.push(queryAddURL({ pageId })); break; case RedirectAction.ITEM: if (!redirectAction.payload) { - history.push(queryAddURL({})); + history.push(queryAddURL({ pageId })); log.error("Redirect item does not have a payload"); break; }