feat: integrates on page unload behavior with backend for deployed mode of the app #41036
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Changes Made - **New Action**: `executePageUnloadActions` added to `pluginActionActions.ts` - **Redux Action Types**: Added `EXECUTE_PAGE_UNLOAD_ACTIONS`, `EXECUTE_PAGE_UNLOAD_ACTIONS_SUCCESS`, and `EXECUTE_PAGE_UNLOAD_ACTIONS_ERROR` to `ReduxActionConstants.tsx`
### Changes Made - **New Selector**: `getPageUnloadActions` added to `editorSelectors.tsx` - Filters JavaScript actions to return those with `runBehaviour` set to `ON_PAGE_UNLOAD`. - **Imports**: Included necessary imports for `getAllJSCollectionActions` and `ActionRunBehaviour` to support the new functionality.
### Changes Made - Updated `executePageUnloadActions` in `pluginActionActions.ts` to remove the unused `actionExecutionContext` parameter, streamlining the action's payload structure.
### Changes Made - **New Action**: `navigateToAnotherPage` added to `pageActions.tsx` to facilitate navigation between pages. - **Redux Action Type**: Introduced `NAVIGATE_TO_ANOTHER_PAGE` in `ReduxActionConstants.tsx`. - **Saga Integration**: Updated `NavigationSagas.ts` to handle the new navigation action using `navigateToAnyPageInApplication`. - **Payload Type**: Defined `NavigateToAnotherPagePayload` in `NavigateActionSaga.ts` to structure the navigation payload. - **History Management**: Enhanced `pushToHistory` function to manage page transitions effectively with state handling.
…gation ### Changes Made - Replaced `NavLink` with a styled `div` in `MenuItem.styled.tsx` for better customization. - Integrated `useDispatch` and `useLocation` hooks in `MenuItem.tsx` to manage navigation actions. - Implemented `handleClick` function to dispatch `navigateToAnotherPage` action on item click. - Updated className logic to reflect active state based on current page path.
### Changes Made - **New Saga**: Created `PluginActionSaga` to handle the execution of plugin actions, including page load actions and file uploads. - **Action Handling**: Integrated various action types such as `executePluginActionRequest`, `executePluginActionSuccess`, and `executePluginActionError`. - **Blob URL Management**: Implemented functions to resolve and manage blob URLs for file uploads, ensuring proper handling of large files. - **Error Handling**: Enhanced error handling for action execution, including logging and user feedback through AppsmithConsole. - **Analytics Integration**: Added analytics tracking for action execution success and failure events. - **Refactored Logic**: Streamlined the logic for evaluating action parameters and managing action execution context.
…improving readability ### Changes Made - Removed deprecated functions related to blob URL handling and file reading. - Streamlined the `evaluateActionParams` function for better clarity and maintainability. - Enhanced error handling and logging within the action execution flow. - Updated instrumentation logic for file uploads to ensure accurate tracking. - Consolidated action execution logic to improve overall performance and readability.
WalkthroughThis change implements a Redux and saga-based mechanism to trigger "onPageUnload" actions when navigating between pages in both deployed and editor modes. It introduces new action types, selectors, sagas, hooks, and component refactors to ensure all unload actions are executed reliably during navigation. Comprehensive unit tests are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MenuItem/Dropdown
participant useNavigateToAnotherPage
participant Redux Store
participant SagaMiddleware
participant PluginActionSaga
User->>MenuItem/Dropdown: Clicks page navigation
MenuItem/Dropdown->>useNavigateToAnotherPage: Calls navigation callback
useNavigateToAnotherPage->>Redux Store: Dispatch NAVIGATE_TO_ANOTHER_PAGE
Redux Store->>SagaMiddleware: Triggers navigation saga
SagaMiddleware->>PluginActionSaga: Dispatch EXECUTE_PAGE_UNLOAD_ACTIONS
PluginActionSaga->>PluginActionSaga: Execute all onPageUnload JS actions
PluginActionSaga->>SagaMiddleware: Dispatch EXECUTE_PAGE_UNLOAD_ACTIONS_SUCCESS
SagaMiddleware->>SagaMiddleware: Continue navigation after unload actions
SagaMiddleware->>Redux Store: Update navigation state (history push)
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 124, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
### Changes Made - **New Enum Value**: Added `PAGE_UNLOAD` to `ActionExecutionContext` to represent the context for page unload actions. - **New Saga**: Implemented `executePageUnloadActionsSaga` to handle the execution of actions triggered on page unload. - **Redux Integration**: Updated `watchPluginActionExecutionSagas` to include the new `EXECUTE_PAGE_UNLOAD_ACTIONS` action type. - **New Selector**: Renamed `getPageUnloadActions` to `getLayoutOnUnloadActions` for consistency and clarity in action retrieval. - **Error Handling**: Enhanced error handling and logging within the new saga to ensure robust execution flow during page unload events.
### Changes Made - Replaced the `isPageActive` utility function with a direct check using `pathname.indexOf(page.pageId) > -1` to determine if the menu item is active. This improves readability and reduces dependency on external utility functions.
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 126, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
### Changes Made - Created a new test file `MenuItem.test.tsx` to implement unit tests for the MenuItem component. - Added tests to verify rendering of the page name, active state detection based on the current path, and navigation action dispatching in both PUBLISHED and EDIT modes. - Mocked necessary dependencies and Redux store for comprehensive testing of component behavior and interactions.
### Changes Made - Created a new test file `NavigateActionSaga.test.ts` to implement unit tests for the navigation saga. - Added tests to verify navigation actions for different scenarios, including navigating to pages in the same window, new window, and handling external URLs. - Mocked necessary dependencies and Redux store to ensure comprehensive testing of saga behavior and interactions. - Included tests for error handling when invalid URLs are provided and for query parameter navigation.
### Changes Made - Created a new test file `onPageUnloadSaga.test.ts` to implement unit tests for the `executePageUnloadActionsSaga`. - Added tests to verify the execution of page unload actions, including scenarios for no actions present, single and multiple JS actions, error handling, and missing collections. - Mocked necessary dependencies and Redux store to ensure comprehensive testing of saga behavior and interactions.
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 126, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
sneha122
reviewed
Jun 26, 2025
| @@ -114,3 +115,30 @@ export default function* navigateActionSaga( | |||
| }); | |||
| } | |||
| } | |||
|
|
|||
| export interface NavigateToAnotherPagePayload { | |||
There was a problem hiding this comment.
Instead of creating this interface in NavigationActionSaga itself, can we create this in a separate types file? This is to avoid cyclic dependencies in future.
sneha122
reviewed
Jun 26, 2025
app/client/src/pages/AppViewer/Navigation/components/MenuItem.styled.tsx
Show resolved
Hide resolved
sneha122
reviewed
Jun 26, 2025
app/client/src/pages/AppViewer/Navigation/components/MenuItem.tsx
Outdated
Show resolved
Hide resolved
sneha122
reviewed
Jun 26, 2025
app/client/src/sagas/ActionExecution/PluginActionSaga/onPageUnloadSaga.test.ts
Outdated
Show resolved
Hide resolved
sneha122
reviewed
Jun 26, 2025
app/client/src/sagas/ActionExecution/NavigateActionSaga.test.ts
Outdated
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
### Changes Made - Updated all instances of `SourceEntity` import from `entities/AppsmithConsole` to `entities/AppsmithConsole/types` for better organization and clarity. - This change enhances code maintainability by centralizing type definitions in a dedicated file.
…ta imports ### Changes Made - Introduced `pr_circular_deps.txt` to document circular dependencies within the codebase for better visibility and management. - Updated imports of `TriggerMeta` across multiple files to reference the new `types.ts` file, enhancing code organization and maintainability. - Removed outdated import paths from `ActionExecutionSagas.ts` and other related files to streamline the code structure.
…on handling ### Changes Made - Replaced `StyledMenuItemInDropdown` with a new `MoreDropDownButtonItem` component to encapsulate dropdown item logic and improve code organization. - Introduced a custom hook `useNavigateToAnotherPage` to streamline navigation logic and reduce redundancy in the `MenuItem` and `MoreDropdownButton` components. - Updated the `MoreDropdownButton` to utilize the new dropdown item component and hook, enhancing readability and maintainability. - Adjusted type definitions for navigation state in `NavigateToAnotherPagePayload` to ensure consistency across the codebase. - Removed unused imports and streamlined component logic for better performance and clarity.
…avigateActionSaga ### Changes Made - Updated the `pushToHistory` function to accept both `NavigateToAnotherPagePayload` and `Path` types, improving flexibility in handling navigation payloads. - Simplified the logic for pushing to history by differentiating between string and object payloads, enhancing code clarity and maintainability. - Removed unnecessary query string handling in the same window navigation case, streamlining the navigation process. - Added type imports for better type safety and clarity in the navigation saga.
### Changes Made - Modified the XPath selector in the `_getActivePage` method to target a `div` with the class `is-active` instead of an `a` tag, ensuring accurate identification of the active page element in the application settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/client/src/pages/AppViewer/Navigation/hooks/useNavigateToAnotherPage.tsx (1)
27-35: Consider memoizing the returned callback function.The callback function is recreated on every render, which could cause unnecessary re-renders in consuming components.
+import { useCallback } from "react-redux"; - return () => { - dispatch( - navigateToAnotherPage({ - pageURL: trimQueryString(pageURL), - query, - state, - }), - ); - }; + return useCallback(() => { + dispatch( + navigateToAnotherPage({ + pageURL: trimQueryString(pageURL), + query, + state, + }), + ); + }, [dispatch, pageURL, query, state]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
app/client/cypress/support/Pages/AppSettings/AppSettings.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/components/ErrorView/ErrorView.tsx(1 hunks)app/client/src/actions/debuggerActions.ts(1 hunks)app/client/src/ce/entities/AppsmithConsole/utils.ts(1 hunks)app/client/src/ce/sagas/analyticsSaga.ts(1 hunks)app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/DebuggerEntityLink.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/ErrorLogs/ErrorLogItem.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogHelper.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/LogItem/LogItem.tsx(1 hunks)app/client/src/ee/sagas/ActionExecution/types.ts(1 hunks)app/client/src/entities/AppsmithConsole/index.ts(1 hunks)app/client/src/entities/AppsmithConsole/types.ts(1 hunks)app/client/src/pages/AppViewer/Navigation/components/MenuItem/index.tsx(2 hunks)app/client/src/pages/AppViewer/Navigation/components/MoreDropDownButtonItem.tsx(1 hunks)app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.styled.tsx(1 hunks)app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.tsx(2 hunks)app/client/src/pages/AppViewer/Navigation/hooks/useNavigateToAnotherPage.tsx(1 hunks)app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx(1 hunks)app/client/src/sagas/ActionExecution/CopyActionSaga.ts(1 hunks)app/client/src/sagas/ActionExecution/DownloadActionSaga.ts(1 hunks)app/client/src/sagas/ActionExecution/ModalSagas.ts(1 hunks)app/client/src/sagas/ActionExecution/NavigateActionSaga/index.ts(3 hunks)app/client/src/sagas/ActionExecution/NavigateActionSaga/types.ts(1 hunks)app/client/src/sagas/ActionExecution/ResetWidgetActionSaga.ts(1 hunks)app/client/src/sagas/ActionExecution/ShowAlertActionSaga.ts(1 hunks)app/client/src/sagas/ActionExecution/geolocationSaga.ts(1 hunks)app/client/src/sagas/DebuggerSagas.ts(1 hunks)app/client/src/sagas/ErrorSagas.tsx(1 hunks)app/client/src/sagas/EvaluationsSaga.ts(1 hunks)app/client/src/workers/Evaluation/evaluate.ts(1 hunks)app/client/src/workers/Evaluation/fns/overrides/console.ts(1 hunks)app/client/src/workers/Evaluation/fns/utils/ExecutionMetaData.ts(1 hunks)
✅ Files skipped from review due to trivial changes (24)
- app/client/src/components/editorComponents/Debugger/DebuggerEntityLink.tsx
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx
- app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/components/ErrorView/ErrorView.tsx
- app/client/src/sagas/ActionExecution/DownloadActionSaga.ts
- app/client/src/workers/Evaluation/fns/utils/ExecutionMetaData.ts
- app/client/src/sagas/ActionExecution/ResetWidgetActionSaga.ts
- app/client/src/sagas/ActionExecution/ShowAlertActionSaga.ts
- app/client/src/sagas/EvaluationsSaga.ts
- app/client/src/sagas/ActionExecution/geolocationSaga.ts
- app/client/src/sagas/ActionExecution/ModalSagas.ts
- app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogHelper.tsx
- app/client/src/ee/sagas/ActionExecution/types.ts
- app/client/src/sagas/ActionExecution/CopyActionSaga.ts
- app/client/cypress/support/Pages/AppSettings/AppSettings.ts
- app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx
- app/client/src/workers/Evaluation/fns/overrides/console.ts
- app/client/src/components/editorComponents/Debugger/ErrorLogs/ErrorLogItem.tsx
- app/client/src/sagas/ErrorSagas.tsx
- app/client/src/actions/debuggerActions.ts
- app/client/src/sagas/DebuggerSagas.ts
- app/client/src/ce/sagas/analyticsSaga.ts
- app/client/src/components/editorComponents/Debugger/LogItem/LogItem.tsx
- app/client/src/ce/entities/AppsmithConsole/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/pages/AppViewer/Navigation/components/MenuItem/index.tsx
- app/client/src/sagas/ActionExecution/NavigateActionSaga/index.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: brayn003
PR: appsmithorg/appsmith#40462
File: app/client/src/instrumentation/index.ts:0-0
Timestamp: 2025-04-29T09:12:14.552Z
Learning: Only comment on files that are directly related to the PR's objectives, even if other files appear in the diff. For PR #40462, the focus is on the import override feature for artifacts, not on instrumentation or telemetry files.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-06-27T14:27:49.746Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/index.mdc:0-0
Timestamp: 2025-06-23T12:22:10.042Z
Learning: Pull request titles in the Appsmith codebase should follow the Conventional Commits specification, using the format 'type(scope): description' with valid types such as feat, fix, docs, style, refactor, perf, test, build, ci, chore, and revert.
Learnt from: brayn003
PR: appsmithorg/appsmith#37984
File: app/client/src/git/requests/pullRequest.ts:6-10
Timestamp: 2024-12-05T10:58:36.272Z
Learning: Error handling is not required for the `pullRequest` function in `app/client/src/git/requests/pullRequest.ts`.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#33809
File: app/client/src/workers/Evaluation/evaluate.ts:31-31
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User prefers to handle extensive changes like improving type safety by replacing `any` with more specific types in a separate PR to manage the scope effectively.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#33947
File: app/client/src/sagas/BuildingBlockSagas/PasteBuildingBlockWidgetSaga.test.ts:140-140
Timestamp: 2024-07-26T21:12:57.228Z
Learning: In the `appsmith` project, UI notifications are handled by separate redux actions and do not need to be included in unit tests for error handling.
Learnt from: brayn003
PR: appsmithorg/appsmith#37830
File: app/client/packages/git/src/actions/checkoutBranchActions.ts:11-18
Timestamp: 2024-11-29T05:38:54.262Z
Learning: In the Git Redux actions (`app/client/packages/git/src/actions`), the `createSingleArtifactAction` function already handles loading state and error management, so additional checks in action creators are unnecessary.
app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.styled.tsx (7)
Learnt from: jacquesikot
PR: appsmithorg/appsmith#40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.605Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
Learnt from: KelvinOm
PR: appsmithorg/appsmith#29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The `CloseIcon` is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the `Modal` component's directory.
Learnt from: KelvinOm
PR: appsmithorg/appsmith#29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `CloseIcon` is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the `Modal` component's directory.
Learnt from: brayn003
PR: appsmithorg/appsmith#38088
File: app/client/src/git/components/GitQuickActions/BranchButton/index.tsx:72-74
Timestamp: 2024-12-11T08:33:24.352Z
Learning: In the 'BranchButton' component in 'app/client/src/git/components/GitQuickActions/BranchButton/index.tsx' (TypeScript, React), the `useEffect` hook that checks for label ellipsis does not need to include `currentBranch` in its dependency array.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#37912
File: app/client/src/git/components/QuickActions/helpers.ts:22-25
Timestamp: 2024-12-03T10:13:43.282Z
Learning: In `app/client/src/git/components/QuickActions/helpers.ts`, the unnecessary `@ts-ignore` comments will be removed in future PRs.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/Editor/gitSync/components/GitChangesList/StaticChange.tsx:52-52
Timestamp: 2024-11-12T07:37:42.598Z
Learning: The icon provider components in `app/client/packages/design-system/widgets-old/src/Icon/index.tsx` and `app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx` should provide both `settings-2-line` and `settings-v3` icons and should not be updated to remove the old icon.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx:95-95
Timestamp: 2024-11-12T11:42:28.998Z
Learning: In icon provider components within the TypeScript/React codebase, old settings icons like `"settings-2-line"` and `"settings-control"` are intentionally provided alongside new icons. These references are acceptable and should not be flagged for updates.
app/client/src/workers/Evaluation/evaluate.ts (12)
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-07-22T12:21:58.545Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: AmanAgarwal041
PR: appsmithorg/appsmith#36539
File: app/client/src/workers/Evaluation/handlers/jsLibrary.ts:329-329
Timestamp: 2024-09-25T18:58:34.275Z
Learning: In `app/client/src/workers/Evaluation/handlers/jsLibrary.ts`, when removing keys from `self`, use `delete` to completely remove the property, as assigning `undefined` leaves the key in the object and can affect subsequent difference checks.
Learnt from: AmanAgarwal041
PR: appsmithorg/appsmith#36539
File: app/client/src/workers/Evaluation/handlers/jsLibrary.ts:329-329
Timestamp: 2024-10-08T15:32:39.374Z
Learning: In `app/client/src/workers/Evaluation/handlers/jsLibrary.ts`, when removing keys from `self`, use `delete` to completely remove the property, as assigning `undefined` leaves the key in the object and can affect subsequent difference checks.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:8-13
Timestamp: 2024-12-10T10:52:38.873Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts` and similar Git sagas, error handling for `baseArtifactId` is managed outside the scope, so validation checks for `baseArtifactId` within the saga functions are unnecessary.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:28-34
Timestamp: 2024-12-10T10:52:38.244Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts`, error handling is managed outside the scope, so casting errors directly to strings is acceptable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/deleteBranchSaga.ts:38-45
Timestamp: 2024-12-10T10:52:57.789Z
Learning: In the TypeScript file `app/client/src/git/sagas/deleteBranchSaga.ts`, within the `deleteBranchSaga` function, error handling is managed outside the scope of the catch block. Therefore, casting `error` to `string` in this context is acceptable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:98-111
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `updateJSCollection` method in `JSActionAPI.tsx` uses optional chaining and logical AND to safely handle potential null or undefined values for `action.datasource`.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:46-65
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `any` for the `entityProperties` parameter in the `getJSActionBindings` function within `EntityProperties.tsx` is intentional and should not be suggested for refactoring to improve type safety.
app/client/src/entities/AppsmithConsole/index.ts (15)
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:240-0
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The `ENTITY_TYPE.MODULE_INSTANCE` case in `EntityProperties.tsx` is intentionally a combination of the logic from both `ENTITY_TYPE.ACTION` and `ENTITY_TYPE.JSACTION`, which explains the presence of what might seem like duplicated code.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:240-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `ENTITY_TYPE.MODULE_INSTANCE` case in `EntityProperties.tsx` is intentionally a combination of the logic from both `ENTITY_TYPE.ACTION` and `ENTITY_TYPE.JSACTION`, which explains the presence of what might seem like duplicated code.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/ce/entities/DataTree/types.ts:182-186
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The use of `any` type for `moduleInstanceEntities` in `app/client/src/ce/entities/DataTree/types.ts` is intentional and was done by another developer. This decision is likely to serve a specific purpose within the codebase, which may be related to maintaining compatibility between CE and EE or other architectural reasons.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/ce/entities/DataTree/types.ts:182-186
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `any` type for `moduleInstanceEntities` in `app/client/src/ce/entities/DataTree/types.ts` is intentional and was done by another developer. This decision is likely to serve a specific purpose within the codebase, which may be related to maintaining compatibility between CE and EE or other architectural reasons.
Learnt from: brayn003
PR: appsmithorg/appsmith#36622
File: app/client/src/entities/Engine/AppEditorEngine.ts:0-0
Timestamp: 2024-10-11T09:28:32.636Z
Learning: In the file `app/client/src/entities/Engine/AppEditorEngine.ts`, the IDs (`currentApplication.baseId`) and branches (`currentBranch`) are obfuscated, so it's acceptable to log them for debugging purposes.
Learnt from: brayn003
PR: appsmithorg/appsmith#37984
File: app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts:3-7
Timestamp: 2024-12-05T11:02:12.715Z
Learning: When reviewing TypeScript interfaces for API responses in `app/client/src/git/requests/`, note that even if interfaces appear identical, they may be intentionally kept separate because the API responses might change independently in the future.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#37912
File: app/client/src/git/components/QuickActions/helpers.ts:22-25
Timestamp: 2024-12-03T10:13:43.282Z
Learning: In `app/client/src/git/components/QuickActions/helpers.ts`, the unnecessary `@ts-ignore` comments will be removed in future PRs.
Learnt from: brayn003
PR: appsmithorg/appsmith#37984
File: app/client/src/git/requests/updateGlobalConfigRequest.ts:9-13
Timestamp: 2024-12-05T10:57:15.397Z
Learning: In the TypeScript files within `app/client/src/git/requests/`, functions should not include internal error handling or request timeouts; they should allow errors to be thrown directly.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:46-65
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `any` for the `entityProperties` parameter in the `getJSActionBindings` function within `EntityProperties.tsx` is intentional and should not be suggested for refactoring to improve type safety.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37056
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor/JSObjectNameEditor.tsx:22-25
Timestamp: 2024-10-24T08:38:20.429Z
Learning: "constants/AppConstants" does not export "SaveActionNameParams".
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#36560
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx:28-28
Timestamp: 2024-10-08T15:32:34.115Z
Learning: When type definitions like `SaveActionNameParams` are declared in multiple places, consolidating them by removing duplicates and reusing a single definition maintains consistency in the codebase.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#36560
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx:28-28
Timestamp: 2024-09-26T06:52:44.158Z
Learning: When type definitions like `SaveActionNameParams` are declared in multiple places, consolidating them by removing duplicates and reusing a single definition maintains consistency in the codebase.
app/client/src/sagas/ActionExecution/NavigateActionSaga/types.ts (1)
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.tsx (10)
Learnt from: jacquesikot
PR: appsmithorg/appsmith#40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.605Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
Learnt from: KelvinOm
PR: appsmithorg/appsmith#29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The `CloseIcon` is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the `Modal` component's directory.
Learnt from: KelvinOm
PR: appsmithorg/appsmith#29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `CloseIcon` is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the `Modal` component's directory.
Learnt from: brayn003
PR: appsmithorg/appsmith#38088
File: app/client/src/git/components/GitQuickActions/BranchButton/index.tsx:72-74
Timestamp: 2024-12-11T08:33:24.352Z
Learning: In the 'BranchButton' component in 'app/client/src/git/components/GitQuickActions/BranchButton/index.tsx' (TypeScript, React), the `useEffect` hook that checks for label ellipsis does not need to include `currentBranch` in its dependency array.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#37912
File: app/client/src/git/components/QuickActions/helpers.ts:22-25
Timestamp: 2024-12-03T10:13:43.282Z
Learning: In `app/client/src/git/components/QuickActions/helpers.ts`, the unnecessary `@ts-ignore` comments will be removed in future PRs.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts:16-24
Timestamp: 2024-12-10T10:53:17.146Z
Learning: In the `getPullBtnStatus` function (`app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts`), default parameter values should be explicitly mentioned to handle component state properly, even if all props are required.
Learnt from: brayn003
PR: appsmithorg/appsmith#38088
File: app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts:16-21
Timestamp: 2024-12-11T08:31:10.356Z
Learning: In the file `app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts`, when defining the function `getPullBtnStatus`, it's acceptable to use default parameters in destructuring. Do not suggest removing default parameters in this context.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#37289
File: app/client/src/widgets/JSONFormWidget/fields/SelectField.tsx:201-201
Timestamp: 2024-11-08T09:16:18.899Z
Learning: In the `SelectField` component of the JSONForm widget (`app/client/src/widgets/JSONFormWidget/fields/SelectField.tsx`), the default fallback dropdown width of `100` is sufficient and does not need adjustment.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx:95-95
Timestamp: 2024-11-12T11:42:28.998Z
Learning: In icon provider components within the TypeScript/React codebase, old settings icons like `"settings-2-line"` and `"settings-control"` are intentionally provided alongside new icons. These references are acceptable and should not be flagged for updates.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/Editor/gitSync/components/GitChangesList/StaticChange.tsx:52-52
Timestamp: 2024-11-12T07:37:42.598Z
Learning: The icon provider components in `app/client/packages/design-system/widgets-old/src/Icon/index.tsx` and `app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx` should provide both `settings-2-line` and `settings-v3` icons and should not be updated to remove the old icon.
app/client/src/entities/AppsmithConsole/types.ts (9)
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/ce/entities/DataTree/types.ts:182-186
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The use of `any` type for `moduleInstanceEntities` in `app/client/src/ce/entities/DataTree/types.ts` is intentional and was done by another developer. This decision is likely to serve a specific purpose within the codebase, which may be related to maintaining compatibility between CE and EE or other architectural reasons.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/ce/entities/DataTree/types.ts:182-186
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `any` type for `moduleInstanceEntities` in `app/client/src/ce/entities/DataTree/types.ts` is intentional and was done by another developer. This decision is likely to serve a specific purpose within the codebase, which may be related to maintaining compatibility between CE and EE or other architectural reasons.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:240-0
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The `ENTITY_TYPE.MODULE_INSTANCE` case in `EntityProperties.tsx` is intentionally a combination of the logic from both `ENTITY_TYPE.ACTION` and `ENTITY_TYPE.JSACTION`, which explains the presence of what might seem like duplicated code.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:240-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `ENTITY_TYPE.MODULE_INSTANCE` case in `EntityProperties.tsx` is intentionally a combination of the logic from both `ENTITY_TYPE.ACTION` and `ENTITY_TYPE.JSACTION`, which explains the presence of what might seem like duplicated code.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx:46-65
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `any` for the `entityProperties` parameter in the `getJSActionBindings` function within `EntityProperties.tsx` is intentional and should not be suggested for refactoring to improve type safety.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: brayn003
PR: appsmithorg/appsmith#37984
File: app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts:3-7
Timestamp: 2024-12-05T11:02:12.715Z
Learning: When reviewing TypeScript interfaces for API responses in `app/client/src/git/requests/`, note that even if interfaces appear identical, they may be intentionally kept separate because the API responses might change independently in the future.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#33726
File: app/client/src/utils/autocomplete/CodemirrorTernService.ts:199-199
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `entityDef` property in `CodeMirrorTernService` class within `CodemirrorTernService.ts` is intentionally initialized as an empty object.
app/client/src/pages/AppViewer/Navigation/components/MoreDropDownButtonItem.tsx (2)
Learnt from: jacquesikot
PR: appsmithorg/appsmith#40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.605Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts:16-24
Timestamp: 2024-12-10T10:53:17.146Z
Learning: In the `getPullBtnStatus` function (`app/client/src/git/components/GitQuickActions/helpers/getPullButtonStatus.ts`), default parameter values should be explicitly mentioned to handle component state properly, even if all props are required.
app/client/src/pages/AppViewer/Navigation/hooks/useNavigateToAnotherPage.tsx (1)
Learnt from: brayn003
PR: appsmithorg/appsmith#38088
File: app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts:40-43
Timestamp: 2024-12-11T08:25:39.197Z
Learning: In `app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts`, the `useMemo` hook includes dependencies `artifactType` and `baseArtifactId` in its dependency array.
🧬 Code Graph Analysis (2)
app/client/src/entities/AppsmithConsole/types.ts (1)
app/client/src/ce/entities/DataTree/types.ts (1)
ENTITY_TYPE(20-25)
app/client/src/pages/AppViewer/Navigation/hooks/useNavigateToAnotherPage.tsx (3)
app/client/src/pages/Editor/utils.tsx (1)
useHref(294-307)app/client/src/actions/pageActions.tsx (1)
navigateToAnotherPage(701-706)app/client/src/utils/helpers.tsx (1)
trimQueryString(794-800)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (7)
app/client/src/workers/Evaluation/evaluate.ts (1)
12-12: LGTM! Import path refactoring improves code organization.The updated import path for
TriggerMetacentralizes type definitions and enhances modularity. This is a clean refactoring that maintains functionality while improving code structure.app/client/src/entities/AppsmithConsole/types.ts (2)
5-17: Well-structured interface definition.The
SourceEntityinterface is properly defined with clear documentation and appropriate optional properties. The type safety is maintained with specific types likeENTITY_TYPE,PluginType, andHTTP_METHOD.
1-1: Verify the import path for ENTITY_TYPE.The import references
"ee/entities/AppsmithConsole/utils"but the relevant code snippet showsENTITY_TYPEis defined in"ce/entities/DataTree/types.ts". Please confirm this import path is correct or if there's a re-export happening.#!/bin/bash # Description: Verify the ENTITY_TYPE export from ee/entities/AppsmithConsole/utils # Expected result: Should show ENTITY_TYPE export or re-export # Check if the file exists and contains ENTITY_TYPE export fd "utils.ts" --type f app/client/src/ee/entities/AppsmithConsole/ --exec cat {} \; | head -20 # Also check the ce version for comparison fd "types.ts" --type f app/client/src/ce/entities/DataTree/ --exec cat {} \; | grep -A 10 "ENTITY_TYPE"app/client/src/entities/AppsmithConsole/index.ts (1)
2-5: Clean import reorganization.The import refactoring follows TypeScript best practices with type-only imports and proper modularization. Moving
SourceEntityto a dedicated types file improves code organization and reusability.app/client/src/sagas/ActionExecution/NavigateActionSaga/types.ts (1)
3-7: Clean interface definition for navigation payload.The interface structure is well-designed with appropriate types for URL, query parameters, and location state.
app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.styled.tsx (1)
58-58: Appropriate change from NavLink to div element.This change aligns with the architectural shift to programmatic navigation via Redux actions instead of direct URL routing.
app/client/src/pages/AppViewer/Navigation/components/MoreDropdownButton.tsx (1)
81-88: Good refactoring to extract dropdown item logic.Delegating the dropdown item rendering to a dedicated
MoreDropDownButtonItemcomponent improves code organization and maintainability.
app/client/src/pages/AppViewer/Navigation/components/MoreDropDownButtonItem.tsx
Show resolved
Hide resolved
### Changes Made - Updated tests in `NavigateActionSaga.test.ts` to utilize the new `state` property in `NavigateToAnotherPagePayload`, ensuring accurate testing of navigation actions with state. - Simplified the `call(pushToHistory, ...)` assertions by directly passing the URL string instead of an object, improving test clarity. - Added new test cases to cover various scenarios for string paths, including handling of query parameters and edge cases like empty strings and root paths. - Ensured all navigation scenarios are thoroughly tested, enhancing the robustness of the navigation saga.
### Changes Made - Added a mock for the `useNavigateToAnotherPage` hook in `MenuItem.test.tsx` to improve test coverage for navigation functionality. - Implemented a mock implementation of the hook to simulate navigation actions based on the application mode, ensuring accurate testing of the `MenuItem` component's behavior in different scenarios. - Updated the test suite to utilize the mocked hook, enhancing the robustness of the component tests.
…riggerMeta imports" This reverts commit 233dbc5.
This reverts commit 3e66087.
This reverts commit c06bc97.
### Changes Made - Introduced `PluginActionSaga.ts` to manage the execution of plugin actions, including handling blob URLs, action timeouts, and error responses. - Removed the deprecated `baseExectutePluginSaga.ts`, consolidating functionality into the new saga for improved maintainability. - Updated related sagas to utilize the new structure, ensuring seamless integration with existing action execution flows. - Enhanced error handling and logging for better debugging and user feedback during action execution. - Implemented soft refresh and unload action handling to improve user experience during page transitions.
chore: fixes cyclic dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/client/src/sagas/__tests__/onPageUnloadSaga.test.ts (2)
252-279: Remove generator function syntax from mockThe mock function doesn't need to be a generator since it doesn't yield anything.
282-317: Use proper Jest mocking instead of reassigning importsReassigning imported variables is not recommended. Use Jest's mocking capabilities instead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/sagas/ActionExecution/NavigateActionSaga/index.ts(3 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(3 hunks)app/client/src/sagas/__tests__/onPageUnloadSaga.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-06-27T14:27:49.746Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: brayn003
PR: appsmithorg/appsmith#40462
File: app/client/src/instrumentation/index.ts:0-0
Timestamp: 2025-04-29T09:12:14.552Z
Learning: Only comment on files that are directly related to the PR's objectives, even if other files appear in the diff. For PR #40462, the focus is on the import override feature for artifacts, not on instrumentation or telemetry files.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/index.mdc:0-0
Timestamp: 2025-06-23T12:22:10.042Z
Learning: Pull request titles in the Appsmith codebase should follow the Conventional Commits specification, using the format 'type(scope): description' with valid types such as feat, fix, docs, style, refactor, perf, test, build, ci, chore, and revert.
Learnt from: sneha122
PR: appsmithorg/appsmith#30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Learnt from: sneha122
PR: appsmithorg/appsmith#30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#33947
File: app/client/src/sagas/BuildingBlockSagas/PasteBuildingBlockWidgetSaga.test.ts:140-140
Timestamp: 2024-07-26T21:12:57.228Z
Learning: In the `appsmith` project, UI notifications are handled by separate redux actions and do not need to be included in unit tests for error handling.
Learnt from: brayn003
PR: appsmithorg/appsmith#37830
File: app/client/packages/git/src/actions/checkoutBranchActions.ts:11-18
Timestamp: 2024-11-29T05:38:54.262Z
Learning: In the Git Redux actions (`app/client/packages/git/src/actions`), the `createSingleArtifactAction` function already handles loading state and error management, so additional checks in action creators are unnecessary.
Learnt from: sumitsum
PR: appsmithorg/appsmith#29875
File: app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConsolidatedAPIServiceImpl.java:0-0
Timestamp: 2024-10-08T15:32:34.114Z
Learning: Test cases for the `getConsolidatedInfoForPageLoad` method have already been added, and the refactoring of the method into smaller methods is planned for later and is being tracked in a separate GitHub issue.
Learnt from: sumitsum
PR: appsmithorg/appsmith#29875
File: app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConsolidatedAPIServiceImpl.java:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: Test cases for the `getConsolidatedInfoForPageLoad` method have already been added, and the refactoring of the method into smaller methods is planned for later and is being tracked in a separate GitHub issue.
Learnt from: brayn003
PR: appsmithorg/appsmith#38634
File: app/client/src/git/sagas/initGitSaga.ts:38-39
Timestamp: 2025-01-13T18:31:44.457Z
Learning: In Git sagas where operations are primarily Redux `put` effects, error handling with try-catch blocks is unnecessary as these effects don't throw errors.
app/client/src/sagas/ActionExecution/NavigateActionSaga/index.ts (10)
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:8-13
Timestamp: 2024-12-10T10:52:38.873Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts` and similar Git sagas, error handling for `baseArtifactId` is managed outside the scope, so validation checks for `baseArtifactId` within the saga functions are unnecessary.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:28-34
Timestamp: 2024-12-10T10:52:38.244Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts`, error handling is managed outside the scope, so casting errors directly to strings is acceptable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/deleteBranchSaga.ts:38-45
Timestamp: 2024-12-10T10:52:57.789Z
Learning: In the TypeScript file `app/client/src/git/sagas/deleteBranchSaga.ts`, within the `deleteBranchSaga` function, error handling is managed outside the scope of the catch block. Therefore, casting `error` to `string` in this context is acceptable.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#36560
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx:28-28
Timestamp: 2024-10-08T15:32:34.115Z
Learning: When type definitions like `SaveActionNameParams` are declared in multiple places, consolidating them by removing duplicates and reusing a single definition maintains consistency in the codebase.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#36560
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx:28-28
Timestamp: 2024-09-26T06:52:44.158Z
Learning: When type definitions like `SaveActionNameParams` are declared in multiple places, consolidating them by removing duplicates and reusing a single definition maintains consistency in the codebase.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-07-22T12:21:58.545Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: brayn003
PR: appsmithorg/appsmith#38634
File: app/client/src/git/sagas/initGitSaga.ts:38-39
Timestamp: 2025-01-13T18:31:44.457Z
Learning: In Git sagas where operations are primarily Redux `put` effects, error handling with try-catch blocks is unnecessary as these effects don't throw errors.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37056
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor/JSObjectNameEditor.tsx:22-25
Timestamp: 2024-10-24T08:38:20.429Z
Learning: "constants/AppConstants" does not export "SaveActionNameParams".
app/client/src/sagas/ActionExecution/PluginActionSaga.ts (5)
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-06-26T06:22:15.976Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:98-111
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `updateJSCollection` method in `JSActionAPI.tsx` uses optional chaining and logical AND to safely handle potential null or undefined values for `action.datasource`.
Learnt from: brayn003
PR: appsmithorg/appsmith#38634
File: app/client/src/git/sagas/initGitSaga.ts:38-39
Timestamp: 2025-01-13T18:31:44.457Z
Learning: In Git sagas where operations are primarily Redux `put` effects, error handling with try-catch blocks is unnecessary as these effects don't throw errors.
app/client/src/sagas/__tests__/onPageUnloadSaga.test.ts (24)
Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-07-16T06:44:55.118Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep`, and other related sleep functions in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:8-13
Timestamp: 2024-12-10T10:52:38.873Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts` and similar Git sagas, error handling for `baseArtifactId` is managed outside the scope, so validation checks for `baseArtifactId` within the saga functions are unnecessary.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:28-34
Timestamp: 2024-12-10T10:52:38.244Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts`, error handling is managed outside the scope, so casting errors directly to strings is acceptable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#37289
File: app/client/src/widgets/JSONFormWidget/fields/SelectField.test.tsx:137-138
Timestamp: 2024-11-08T09:17:10.831Z
Learning: In TypeScript test files like `SelectField.test.tsx`, it's acceptable to use the `delete` operator to remove mock objects such as `ResizeObserver` when their eventual removal is required.
Learnt from: brayn003
PR: appsmithorg/appsmith#38060
File: app/client/src/git/sagas/deleteBranchSaga.ts:38-45
Timestamp: 2024-12-10T10:52:57.789Z
Learning: In the TypeScript file `app/client/src/git/sagas/deleteBranchSaga.ts`, within the `deleteBranchSaga` function, error handling is managed outside the scope of the catch block. Therefore, casting `error` to `string` in this context is acceptable.
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-07-24T08:29:41.208Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-06-27T14:27:49.746Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#34563
File: app/client/src/sagas/BuildingBlockSagas/index.ts:77-87
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `accessNestedObjectValue` function in the `sagas/PasteWidgetUtils` module has been tested by the user (rahulbarwal) and confirmed to handle null or undefined values correctly.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-07-22T12:21:58.545Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#39562
File: app/client/src/ce/sagas/moduleInterfaceSagas.ts:17-21
Timestamp: 2025-03-05T09:14:26.778Z
Learning: In the Appsmith codebase, bridge functions between CE and EE are sometimes implemented as generator functions without yield statements in the CE version. These functions serve as placeholders that get extended or overridden in the EE version with the actual implementation. This pattern is intentional and doesn't require yield statements in the base implementation.
Learnt from: sharat87
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:0-0
Timestamp: 2024-11-30T00:37:12.269Z
Learning: In `deploy/docker/fs/opt/appsmith/utils/bin/backup.js`, exception handling is performed outside the `executePostgresDumpCMD` function, so adding try-catch blocks within `executePostgresDumpCMD` that rethrow the error is unnecessary.
Learnt from: sharat87
PR: appsmithorg/appsmith#37326
File: app/client/packages/rts/src/ctl/restore.ts:157-191
Timestamp: 2024-11-30T00:33:25.779Z
Learning: In `restore.ts`, exceptions are already caught outside the individual restore functions (`restoreMongoDB`, `restorePostgres`), so adding try-catch blocks inside these functions is unnecessary.
Learnt from: vhemery
PR: appsmithorg/appsmith#37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47
Timestamp: 2024-11-13T09:07:54.931Z
Learning: When reviewing code in `app/client/cypress/**` paths, ensure that suggestions are appropriate for test code and not for `src` code. Avoid applying source code checks to test code unless relevant.
Learnt from: ashit-rath
PR: appsmithorg/appsmith#37912
File: app/client/src/git/components/QuickActions/helpers.ts:22-25
Timestamp: 2024-12-03T10:13:43.282Z
Learning: In `app/client/src/git/components/QuickActions/helpers.ts`, the unnecessary `@ts-ignore` comments will be removed in future PRs.
Learnt from: AmanAgarwal041
PR: appsmithorg/appsmith#36539
File: app/client/src/workers/Evaluation/handlers/jsLibrary.ts:329-329
Timestamp: 2024-09-25T18:58:34.275Z
Learning: In `app/client/src/workers/Evaluation/handlers/jsLibrary.ts`, when removing keys from `self`, use `delete` to completely remove the property, as assigning `undefined` leaves the key in the object and can affect subsequent difference checks.
Learnt from: AmanAgarwal041
PR: appsmithorg/appsmith#36539
File: app/client/src/workers/Evaluation/handlers/jsLibrary.ts:329-329
Timestamp: 2024-10-08T15:32:39.374Z
Learning: In `app/client/src/workers/Evaluation/handlers/jsLibrary.ts`, when removing keys from `self`, use `delete` to completely remove the property, as assigning `undefined` leaves the key in the object and can affect subsequent difference checks.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-06-26T06:22:15.976Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Learnt from: sharat87
PR: appsmithorg/appsmith#34471
File: app/client/src/ce/api/JSActionAPI.tsx:98-111
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `updateJSCollection` method in `JSActionAPI.tsx` uses optional chaining and logical AND to safely handle potential null or undefined values for `action.datasource`.
Learnt from: brayn003
PR: appsmithorg/appsmith#38634
File: app/client/src/git/sagas/initGitSaga.ts:38-39
Timestamp: 2025-01-13T18:31:44.457Z
Learning: In Git sagas where operations are primarily Redux `put` effects, error handling with try-catch blocks is unnecessary as these effects don't throw errors.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#33947
File: app/client/src/sagas/BuildingBlockSagas/PasteBuildingBlockWidgetSaga.test.ts:140-140
Timestamp: 2024-07-26T21:12:57.228Z
Learning: In the `appsmith` project, UI notifications are handled by separate redux actions and do not need to be included in unit tests for error handling.
🪛 Biome (1.9.4)
app/client/src/sagas/ActionExecution/PluginActionSaga.ts
[error] 1691-1691: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
app/client/src/sagas/__tests__/onPageUnloadSaga.test.ts
[error] 254-279: This generator function doesn't contain yield.
(lint/correctness/useYield)
[error] 285-285: The imported variable handleExecuteJSFunctionSaga is read-only
The variable is imported here
Use a local variable instead of reassigning an import.
(lint/suspicious/noImportAssign)
[error] 315-315: The imported variable handleExecuteJSFunctionSaga is read-only
The variable is imported here
Use a local variable instead of reassigning an import.
(lint/suspicious/noImportAssign)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (4)
app/client/src/sagas/ActionExecution/NavigateActionSaga/index.ts (2)
49-53: Cleaner ternary operator usageGood improvement on making the condition explicit.
111-140: Well-structured navigation flow with unload actionsThe implementation correctly integrates page unload actions into the navigation flow. The saga properly waits for unload actions to complete before pushing to history.
app/client/src/sagas/ActionExecution/PluginActionSaga.ts (1)
1701-1731: Robust page unload actions executionThe saga correctly handles multiple unload actions in parallel with proper error handling and telemetry.
app/client/src/sagas/__tests__/onPageUnloadSaga.test.ts (1)
85-394: Comprehensive test coverageExcellent test suite covering all edge cases including error scenarios, missing collections, and parallel execution.
sneha122
approved these changes
Jul 2, 2025
This was referenced Jul 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TLDR: Adds support for executing page unload actions during navigation in deployed mode, refactors related components, and improves action handling.
Problem
Page unload actions were not triggered during navigation in deployed mode, leading to incomplete workflows especially for cleanup.
Root cause
The application lacked integration for executing unload actions on page transitions, and related components did not properly handle navigation or action execution.
Solution
This PR handles the integration of page unload action execution during navigation in deployed mode. It introduces selectors for unload actions, refactors the MenuItem component for better navigation handling, and improves the PluginActionSaga for executing plugin actions. Unused parameters and functions are removed for clarity and maintainability.
Fixes #40997
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/16021075820
Commit: f09e3c4
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 02 Jul 2025 10:21:00 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores