-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: add onPageUnload option in UI behind feature flag. #41008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add onPageUnload option in UI behind feature flag. #41008
Conversation
- Introduced a new action run behavior, ON_PAGE_UNLOAD, to the ActionRunBehaviour enum. - Updated RUN_BEHAVIOR_VALUES to include the new ON_PAGE_UNLOAD option with appropriate labels and descriptions. - Enhanced JSFunctionSettings component to utilize feature flags for determining available run behaviors. - Added utility functions to manage run behavior options based on feature flags. - Implemented tests for the new functionality to ensure correct behavior under various feature flag states.
""" WalkthroughA new "On Page Unload" run behavior has been introduced for actions, including updates to the enum, UI options, feature flags, and supporting utility logic. The relevant UI components now dynamically display this option based on feature flags. Comprehensive unit tests have been added for the utility functions managing these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant FeatureFlag
participant Utils
User->>UI_Component: Opens Action Settings
UI_Component->>FeatureFlag: Check reactive & onPageUnload flags
UI_Component->>Utils: getRunBehaviorOptionsBasedOnFeatureFlags(flags)
Utils-->>UI_Component: Filtered run behavior options
UI_Component->>User: Display options (including "On Page Unload" if enabled)
User->>UI_Component: Selects run behavior
UI_Component->>Utils: getDefaultRunBehaviorOptionWhenFeatureFlagIsDisabled(selected, flags)
Utils-->>UI_Component: Default/fallback option if needed
UI_Component->>User: Update selection in UI
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/utils.ts (1)
9-19
: Solid filtering logic with minor type assertion consideration.The filtering logic correctly excludes options based on feature flag states. The boolean conditions are clear and handle both feature flags appropriately.
The type assertion
as SelectOptionProps[]
on line 19 may be redundant ifRUN_BEHAVIOR_VALUES
is already properly typed, but it's not problematic.Consider verifying if the type assertion is necessary:
- ) as SelectOptionProps[]; + );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/PluginActionEditor/types/PluginActionTypes.ts
(1 hunks)app/client/src/ce/entities/FeatureFlag.ts
(2 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/PluginSettings.ts
(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx
(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/utils.test.ts
(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/utils.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
🔇 Additional comments (9)
app/client/src/PluginActionEditor/types/PluginActionTypes.ts (1)
9-9
: LGTM! Clean enum addition.The new
ON_PAGE_UNLOAD
enum member follows the established pattern and naming convention.app/client/src/constants/AppsmithActionConstants/formConfig/PluginSettings.ts (1)
24-29
: LGTM! Well-structured option addition.The new "On page unload" option follows the established pattern with appropriate label, description, and enum value mapping.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (3)
58-60
: Update feature flag reference after fixing typo.This will need to be updated once the feature flag name typo is corrected in the FeatureFlag.ts file.
- const isOnPageUnloadEnabled = useFeatureFlag( - FEATURE_FLAG.release_jsobjects_onpageunloadctions_enabled, - ); + const isOnPageUnloadEnabled = useFeatureFlag( + FEATURE_FLAG.release_jsobjects_onpageunloadactions_enabled, + );
62-69
: LGTM! Excellent use of memoization and utility functions.The memoized options calculation prevents unnecessary re-computations and the utility function extraction improves maintainability.
73-83
: LGTM! Clean default behavior handling.The utility function properly handles edge cases when feature flags disable the current selection.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/utils.test.ts (2)
12-52
: LGTM! Excellent comprehensive test coverage.The matrix-based testing approach ensures all feature flag combinations are properly tested. Well-structured and thorough.
54-139
: LGTM! Thorough edge case testing.The test matrix covers all scenarios for default behavior selection when feature flags are disabled. Good use of data-driven testing.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/utils.ts (2)
1-8
: Clean import structure and type definitions.The imports are well-organized and appropriately typed. Good use of type imports where applicable.
21-47
: Well-structured fallback logic with appropriate error handling.The function correctly handles both disabled feature flag scenarios:
- AUTOMATIC → ON_PAGE_LOAD when reactive actions disabled
- ON_PAGE_UNLOAD → MANUAL when on page unload disabled
The use of nullish coalescing (
??
) provides proper fallback to null when expected options aren't found, which is good defensive programming.
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15774535342. |
Deploy-Preview-URL: https://ce-41008.dp.appsmith.com |
…UES in utils and tests - Introduced JS_OBJECT_RUN_BEHAVIOR_VALUES to include the new ON_PAGE_UNLOAD action run behavior. - Updated references in the JSEditor utility functions and tests to use the new constant for consistency and improved functionality. - Ensured all related tests reflect the changes to maintain expected behavior under various feature flag states.
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15774924346. |
Deploy-Preview-URL: https://ce-41008.dp.appsmith.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Problem
The action configuration UI lacked support for running actions specifically on page unload, limiting automation and flexibility for users.
Root cause
There was no ON_PAGE_UNLOAD option in the ActionRunBehaviour enum or related UI, and feature flag handling for this behavior was missing.
Solution
This PR handles the addition of the ON_PAGE_UNLOAD run behavior to the ActionRunBehaviour enum, updates UI and settings to support it, leverages feature flags for dynamic option availability, and adds tests to ensure robust behavior.
Fixes #40995
or
Fixes
Issue URL
Warning
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.JS"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15774903938
Commit: fe0b500
Cypress dashboard.
Tags:
@tag.JS
Spec:
Fri, 20 Jun 2025 09:27:32 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit