feat: Replacing execute on load toggle with run behaviour in all actions / jsactions #40355
Conversation
…-backend feat: Migrate executeOnLoad to runBehavior enum
…ig settings (#40339) ## Description Replacing `executeOnLoad` with `runBehavior` in all action config settings Figma: https://www.figma.com/design/qFXimqMSH45aviBOdTfDXu/Reactive-Queries-JS?node-id=61-16773&t=rv594VlMSm75Bvcb-0 Fixes [#39831](#39831) ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
WalkthroughThis change replaces the legacy boolean Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant ClientAPI
participant ServerAPI
participant DB
User->>UI: Selects run behavior from dropdown
UI->>ClientAPI: updateActionRunBehaviour(actionId, runBehaviour)
ClientAPI->>ServerAPI: PUT /runBehaviour/{actionId} {behaviour: runBehaviour}
ServerAPI->>DB: Update action.runBehaviour
DB-->>ServerAPI: Confirmation
ServerAPI-->>ClientAPI: Updated ActionDTO
ClientAPI-->>UI: Update action state
UI-->>User: Show updated run behavior
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 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
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14619899131. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json (1)
8-23: Replace executeOnLoad with runBehavior dropdown in MongoPluginThe configuration has been correctly updated to use a dropdown for run behavior instead of a toggle switch. The options and descriptions are clear and consistent with the client-side implementation.
However, I notice the options are duplicated across multiple plugin setting files. This could lead to maintenance issues if these options need to be updated in the future.
Consider extracting these common settings into a shared template or configuration that can be referenced by all plugins to reduce duplication and make future updates easier.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java (2)
322-349: Added new setRunBehavior method with backward compatibilityThe new method correctly implements the reverse mapping from enum value to boolean flag, maintaining consistency between both properties. This ensures a smooth transition from the boolean-based to enum-based approach.
However, consider adding Javadoc to document this method's purpose and behavior, similar to other methods in this class.
+ /** + * Updates the runBehavior of an action and synchronizes the executeOnLoad flag for backward compatibility. + * + * @param id ID of the action to update + * @param behavior The new RunBehaviorEnum value to set + * @return Updated ActionDTO + */ @Override public Mono<ActionDTO> setRunBehavior(String id, RunBehaviorEnum behavior) {
427-434: Properly initialized runBehavior during action creationThe code appropriately sets the initial
runBehaviorvalue based on the action creation context, maintaining consistency with the existingexecuteOnLoadbehavior.Consider extracting the boolean-to-enum mapping logic into helper methods to avoid duplication and ensure consistent behavior across the codebase.
+ private RunBehaviorEnum getRunBehaviorFromExecuteOnLoad(Boolean executeOnLoad) { + return Boolean.TRUE.equals(executeOnLoad) ? RunBehaviorEnum.ON_PAGE_LOAD : RunBehaviorEnum.MANUAL; + } + + private Boolean getExecuteOnLoadFromRunBehavior(RunBehaviorEnum behavior) { + return RunBehaviorEnum.ON_PAGE_LOAD.equals(behavior); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts(1 hunks)app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts(1 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts(10 hunks)app/client/src/components/formControls/DropDownControl.tsx(4 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts(1 hunks)app/client/src/entities/Action/index.ts(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts(1 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json(8 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json(8 hunks)app/client/test/factories/MockPluginsState.ts(4 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java(2 hunks)app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(3 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/components/formControls/DropDownControl.tsx (1)
app/client/src/ce/pages/workspace/InviteUsersForm.tsx (1)
OptionLabel(127-134)
app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (5)
app/client/src/ce/pages/workspace/InviteUsersForm.tsx (1)
OptionLabel(127-134)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/JSObjectNameEditor.tsx (1)
Text(54-58)app/client/packages/design-system/ads/src/Select/Select.tsx (2)
Select(143-143)Option(143-143)app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR_VALUES(23-23)RUN_BEHAVIOR(10-21)app/client/packages/design-system/ads/src/Select/Select.types.ts (1)
SelectOptionProps(16-19)
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: server-spotless / spotless-check
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
🔇 Additional comments (60)
app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts (2)
12-12: Added import for ActionRunBehaviour enumThis import is needed to support the new run behavior property being implemented. The enum represents the execution mode options, replacing the boolean toggle approach.
20-20: Added runBehavior property to BASE_ACTIONGood addition of the runBehavior property consistent with the executeOnLoad property. Setting it to MANUAL aligns with the executeOnLoad: false value, maintaining consistent test behavior.
app/client/src/entities/Action/index.ts (2)
13-13: Added import for ActionRunBehaviour typeThis import is required to support the new run behavior property being added to the BaseAction interface.
144-144: Added runBehavior property to BaseAction interfaceGood implementation - making the property optional allows for backward compatibility while introducing the new enum-based behavior.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts (1)
5-5: Updated value type to support string valuesExpanded the type definition to include string values for the new enum-based run behavior options.
app/client/src/ce/constants/messages.ts (1)
2586-2586: Updated ON_LOAD_TITLE messageUpdated message from "Choose the functions to run on page load" to "Choose functions run behavior" to reflect the change from a boolean toggle to a more descriptive enum-based approach.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java (2)
4-4: Added appropriate import for RunBehaviorEnumClean addition of the required import for the new enum type.
21-21: Good addition of setRunBehavior methodThe new method follows the same pattern as existing methods in the interface and properly handles the new behavior enum parameter.
While the
setExecuteOnLoadmethod still exists, consider adding a@Deprecatedannotation to it to indicate the preferred usage ofsetRunBehavioras part of the migration path.app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java (1)
1-9: Well-structured enum with clear documentationThe enum definition is clean, with appropriate documentation for the class and individual values. The naming is meaningful and aligns with the feature's purpose.
app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (2)
1-4: Good import of run behavior constantsProper import of the required constants from the central location.
12-16: Appropriately replaced toggle with dropdown controlThe replacement of the boolean toggle with a dropdown selection provides a more flexible and descriptive UI for run behavior. The initialization defaults to manual mode as expected.
app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (2)
1-4: Good import of run behavior constantsProper import of the required constants from the central location.
12-16: Good replacement of toggle with dropdownThe change consistently implements the same pattern as seen in other configuration files, correctly replacing the boolean toggle with a dropdown selection.
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (1)
9-12: New imports for run behavior constantsThe change appropriately imports the required constants from the PluginActionConstants file to support the new run behavior dropdown.
app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json (1)
8-23: Replace executeOnLoad with runBehavior dropdown in GoogleAiPluginThe implementation correctly replaces the executeOnLoad toggle with a runBehavior dropdown. The configuration mirrors what was done in other plugins, maintaining consistency across the platform.
app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json (1)
8-23:✅ Verification successful
Replace executeOnLoad with runBehavior dropdown in OpenAiPlugin
The changes correctly implement the runBehavior dropdown with appropriate options and default value. The configuration is consistent with other plugins.
Since this pattern is repeated across multiple plugins, ensure that all plugins are updated consistently.
🏁 Script executed:
#!/bin/bash # Check if any plugin setting.json files still use executeOnLoad instead of runBehavior fd "setting.json" app/server/appsmith-plugins -x grep -l "executeOnLoad" {} \;Length of output: 79
🏁 Script executed:
#!/bin/bash set -e echo "Searching for 'executeOnLoad' in all setting.json under app/server/appsmith-plugins…" # 1) Using ripgrep to target setting.json files rg -l "executeOnLoad" -g "setting.json" app/server/appsmith-plugins || echo "No matches via rg." # 2) Fallback: recursive grep across the plugin directory grep -R "executeOnLoad" app/server/appsmith-plugins || echo "No matches via grep."Length of output: 409
All plugins updated to use runBehavior
Verified: no remaining “executeOnLoad” references in any setting.json under app/server/appsmith-plugins. The runBehavior dropdown is implemented consistently across all plugins.
app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json (1)
8-23: Well‑implemented runBehavior dropdown
The newrunBehaviordropdown cleanly replaces the old booleanexecuteOnLoadswitch. TheconfigProperty,controlType,initialValue, and option values (PAGE_LOAD/MANUAL) are correctly aligned with theRunBehaviorEnum.app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json (1)
8-23: Consistent runBehavior configuration for Oracle plugin
This segment correctly mirrors the pattern from other plugins:runBehavioris defined as a dropdown with proper enum values, descriptive labels/subtexts, and a default of"MANUAL".app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json (1)
8-23: Correct runBehavior dropdown in MSSQL plugin
The dropdown control forrunBehavioris implemented consistently, with accurateconfigProperty,controlType, options, and default alignment to the new enum.app/client/test/factories/MockPluginsState.ts (1)
6955-6973: Good improvement replacing executeOnLoad toggle with runBehavior dropdown.The change consistently replaces boolean toggles with more descriptive dropdown selectors across multiple plugin types (REST API, GraphQL, Appsmith AI, and PostgreSQL). The new dropdown implementation includes helpful subtext descriptions that clarify the behavior of each option, making the UI more intuitive for users.
Also applies to: 7035-7053, 7115-7133, 7159-7177
app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json (1)
8-23: Good job on enhancing the execution control semanticsReplacing the binary toggle with a dropdown that has descriptive options improves the UX significantly. The labels and subtext provide clear guidance on when the query will execute.
A suggestion: For consistency with other plugins, ensure all have similar wording in the subText descriptions.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java (2)
18-18: LGTM: Import for the new enum typeClean import addition for the RunBehaviorEnum.
104-113: Proper deprecation and introduction of new propertyThe addition of Javadoc with migration guidance and proper @deprecated annotation follows best practices. The default value of MANUAL for runBehavior maintains consistent behavior with the original false default of executeOnLoad.
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java (3)
5-5: LGTM: Import for the new enum typeClean import addition for the RunBehaviorEnum.
119-127: Well-structured new endpoint for runBehaviorThe new endpoint follows consistent naming and implementation patterns. Good logging of the operation.
129-132: Proper deprecation of the executeOnLoad endpointThe deprecation notice with clear migration path follows best practices.
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (3)
5-8: Good enum definitionClean enum definition with clear naming that matches the server-side enum.
10-23: Well-structured constant for UI representationThe RUN_BEHAVIOR constant provides a comprehensive mapping with good labels and descriptive subText that helps users understand the behavior differences.
23-23: Utility array for dropdown optionsExporting the values as an array makes it easy to use in dropdown components.
app/client/src/components/formControls/DropDownControl.tsx (5)
23-23: Updated imports to include styling modules.The import for
styledand additional components from@appsmith/adsare now included to support the enhanced dropdown UI.
32-33: Added styled-components import.
34-43: Added styled components for option labels and subtexts.The new styled components provide consistent styling for dropdown options with labels and descriptive subtexts, supporting the run behavior dropdown UI pattern.
477-477: Added fixed listHeight property to Select component.Setting a consistent height of 240px for dropdown lists ensures proper display when options include subtexts.
510-541: Enhanced renderOptionWithIcon to support options with subtexts.The function now conditionally renders a different layout for options with subtexts, using a vertical Flex container to properly display both the label and subtext.
This enhancement provides a better user experience by displaying more descriptive information for dropdown options. The vertical layout ensures proper readability when options include additional explanatory text.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (5)
1-22: Updated imports for new dropdown-based run behavior selection.Added imports for Select components and run behavior constants to support the new execution behavior dropdown UI.
24-38: Added styled components for dropdown UI elements.These styled components ensure consistent styling for dropdown options with proper text colors, font sizes, and weight to maintain visual hierarchy between labels and subtexts.
50-51: Replaced executeOnLoad boolean with runBehavior enum.State is now managed using a more descriptive runBehavior value from the action configuration, improving code readability.
53-68: Updated callback to handle runBehavior selection.The callback now works with the ActionRunBehaviour enum instead of boolean values, while maintaining analytics event tracking with the updated value type.
70-109: Replaced switch toggle with dropdown for run behavior selection.The UI now uses a dropdown with descriptive labels and subtexts instead of a simple toggle, providing users with clearer information about each execution option.
The new dropdown UI improves usability by explaining each option's behavior through subtexts, making it easier for users to understand when their JS functions will execute.
app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json (8)
86-87: Added runBehavior property alongside existing executeOnLoad.The new runBehavior property with value "MANUAL" has been added alongside the existing executeOnLoad boolean for backward compatibility.
2547-2562: Updated settings configurations to use runBehavior dropdown.This configuration change replaces the executeOnLoad boolean toggle with a more descriptive runBehavior dropdown control for MongoDB plugin settings.
2593-2608: Updated PostgreSQL settings for consistent runBehavior pattern.Applied the same runBehavior dropdown pattern to PostgreSQL query settings, ensuring consistency across different plugin types.
2639-2654: Updated API settings with runBehavior dropdown.API settings now use the same runBehavior dropdown pattern, with consistent options and subtexts.
2691-2706: Updated Firebase settings with runBehavior dropdown.Firebase plugin settings now use the runBehavior dropdown control with consistent options.
2730-2745: Updated Google Sheets settings with runBehavior dropdown.Google Sheets plugin settings now use the same runBehavior dropdown pattern, maintaining consistency across the platform.
2776-2791: Updated S3 settings with runBehavior dropdown.Amazon S3 plugin settings now use the runBehavior dropdown control, completing the transition across all plugin types.
2822-2837: Updated Redis settings with runBehavior dropdown.Redis plugin settings now use the runBehavior dropdown control for consistent execution behavior configuration.
app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json (3)
86-86: Consistent implementation of new runBehavior propertyThe addition of the "runBehavior" property with "MANUAL" value aligns with the PR's goal of replacing the boolean executeOnLoad with an enum-based approach. This provides more descriptive control over execution behavior.
2546-2562: Well-structured dropdown replacement for executeOnLoadGood implementation of the dropdown control for runBehavior with clear labels and helpful subText descriptions for each option. The UI change from a switch to a dropdown with descriptive options will improve user understanding of the execution options.
2592-2608: Consistent implementation across all pluginsThe same dropdown pattern is consistently applied across all plugin configurations, maintaining a uniform user experience. This consistency is important for usability.
Also applies to: 2638-2654, 2690-2706, 2729-2745, 2775-2791, 2821-2837
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java (3)
11-11: Appropriate import for the new enumThe import for RunBehaviorEnum is correctly added to support the new tests.
1357-1402: Comprehensive test for runBehavior property changesThis test thoroughly verifies the bidirectional synchronization between runBehavior and executeOnLoad properties. Good pattern of testing:
- Creation with default values
- Changing to ON_PAGE_LOAD and verifying all affected properties
- Changing back to MANUAL and verifying state
- Confirming persistence by fetching from repository
The test correctly asserts that userSetOnLoad remains true even when toggling back to MANUAL mode, which is important for maintaining user intent.
1404-1441: Complete test for backward compatibilityThis test ensures backward compatibility by verifying that the legacy executeOnLoad property still functions correctly and properly updates the new runBehavior property. This is essential for maintaining compatibility with existing code and data.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java (2)
9-9: Added new import for RunBehaviorEnumThe import for
RunBehaviorEnumsupports the transition from boolean toggle to enum-based run behavior control.
304-310: Synchronized executeOnLoad with runBehavior for forward compatibilityGood implementation of bidirectional synchronization between the legacy boolean flag and the new enum. This ensures that existing code using
executeOnLoadwill still work with the newrunBehaviorproperty.app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (5)
2310-2311: Good mapping between old and new execution control properties.The addition of
runBehavior: "PAGE_LOAD"paired withexecuteOnLoad: truemaintains backward compatibility while introducing the new enum-based approach for execution control.
2395-2396: Consistent implementation of execution control mapping.The implementation correctly maps
executeOnLoad: falsetorunBehavior: "MANUAL", ensuring consistent behavior with the previous implementation.
10043-10060: Well-designed dropdown replacement for toggle switch.The dropdown implementation provides better UX than the previous boolean toggle by:
- Using descriptive labels
- Including helpful subtext that explains each option's behavior
- Setting a sensible default value
The explanation texts will help users understand execution behavior more clearly than the previous implementation.
10094-10111: Consistent dropdown implementation across multiple configurations.The dropdown configuration for run behavior is consistently implemented across multiple action types, maintaining a unified user experience.
Also applies to: 10145-10162, 10225-10242
10417-10418: Consistent property addition across JS actions.The
runBehaviorproperty has been consistently added to all JS action configurations with the correct default value matching the originalexecuteOnLoadsetting.Also applies to: 10468-10469, 10553-10554, 10604-10605
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts
Outdated
Show resolved
Hide resolved
app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json
Outdated
Show resolved
Hide resolved
app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json
Outdated
Show resolved
Hide resolved
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json
Outdated
Show resolved
Hide resolved
app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json
Outdated
Show resolved
Hide resolved
app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json (1)
8-23: Optional: Add a tooltip for extra clarityTo help users understand the enum options at a glance, consider adding a
tooltipText, for example:"tooltipText": "Select when this query should automatically run"This can guide users who may be unfamiliar with
PAGE_LOADvs.MANUAL.app/client/src/components/formControls/DropDownControl.tsx (1)
477-477: Consider making the listHeight configurable.While setting a fixed height of 240px works for the current use case, consider making this a configurable prop with a default value to accommodate different dropdown sizes in various contexts.
- listHeight={240} + listHeight={props.listHeight || 240}app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (1)
24-37: Consider reusing styled components.These styled components are defined almost identically in
DropDownControl.tsx. Consider creating shared components to avoid duplication.Consider creating these styled components in a shared location like a
components/stylesdirectory to avoid duplication:-const OptionLabel = styled(Text)` - color: var(--ads-v2-color-fg); - font-size: 14px; - font-weight: 500; -`; - -const OptionSubText = styled(Text)` - color: var(--ads-v2-color-fg-muted); - font-size: 12px; -`; +import { OptionLabel, OptionSubText } from "components/styles/DropdownStyles";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts(1 hunks)app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts(1 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts(10 hunks)app/client/src/components/formControls/DropDownControl.tsx(4 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts(1 hunks)app/client/src/entities/Action/index.ts(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts(1 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json(8 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json(8 hunks)app/client/test/factories/MockPluginsState.ts(4 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java(2 hunks)app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(3 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/components/formControls/DropDownControl.tsx (2)
app/client/src/ce/pages/workspace/InviteUsersForm.tsx (1)
OptionLabel(127-134)app/client/packages/design-system/ads/src/Select/Select.tsx (1)
Option(143-143)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (64)
app/client/src/entities/Action/index.ts (2)
13-13: Appropriate import for new functionality.The import for ActionRunBehaviour is correctly added to support the new run behavior feature.
144-144: Good addition of runBehavior property with backward compatibility.The new optional
runBehaviorproperty complements the existingexecuteOnLoadboolean, allowing for a more descriptive execution model while maintaining backward compatibility. This is a well-designed transition strategy.app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts (1)
5-5: Appropriate type expansion to support new enum values.Extending the
valuetype to includestringproperly supports the new string-based enum values for run behavior. This complements the changes in the Action entity.app/client/src/ce/constants/messages.ts (1)
2586-2586: Updated message reflects the new functionality accurately.The message has been updated from focusing on "functions to run on page load" to the more general "functions run behavior", correctly reflecting the shift from a binary flag to a more flexible run behavior model.
app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts (2)
12-12: Correctly imported enum for test updates.The ActionRunBehaviour import is appropriately added to support the test modifications.
20-20: Test data properly updated with new property.The BASE_ACTION test constant now includes the
runBehaviorproperty set to MANUAL, which is consistent with the existingexecuteOnLoad: falsesetting. This ensures tests properly validate the updated data model.app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java (1)
1-9: Well-structured enum definition for action run behaviors.This new enum provides a clear type-safe representation of how actions should execute, replacing the boolean
executeOnLoadflag with more descriptive options. The comments on each enum value are helpful for understanding their purpose.app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java (2)
3-4: Appropriate import added for the new enum.The import for RunBehaviorEnum is correctly added to support the new method.
21-21: Good addition of the new service method.This method complements the existing
setExecuteOnLoadmethod, providing a more type-safe way to set the run behavior using the enum. The method signature follows the same pattern as other methods in this interface.app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (2)
1-4: Appropriate imports for run behavior constants.These imports bring in the necessary constants for the dropdown options.
11-17: Good conversion from boolean switch to dropdown.The configuration now uses a dropdown with descriptive options instead of a simple boolean toggle, providing more clarity to users. The dropdown is correctly initialized with the MANUAL behavior, and uses the predefined RUN_BEHAVIOR_VALUES.
app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (2)
1-4: Appropriate imports for run behavior constants.These imports are consistent with other settings files, maintaining uniformity.
11-17: Consistent implementation of run behavior dropdown.This configuration mirrors the changes in other settings files, ensuring a consistent user experience across different types of actions. The dropdown replaces the previous boolean toggle with more descriptive options.
app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json (1)
8-23: Replace boolean toggle with descriptive dropdown: approvedThe new
runBehaviordropdown cleanly replaces the old booleanexecuteOnLoadswitch and aligns with the enum‑based run behavior used across the application. Labels, subtexts, and values (PAGE_LOADandMANUAL) are concise and user‑friendly.app/client/test/factories/MockPluginsState.ts (1)
7035-7177: Good improvement to execution control model.The change from a boolean
executeOnLoadtoggle to a more descriptiverunBehaviordropdown with explicit options enhances clarity for users. The dropdown properly includes descriptive subtexts explaining each behavior option.app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json (1)
8-23: Good implementation of run behavior dropdown replacing execute on load toggleThe implementation correctly replaces the boolean toggle with a more descriptive dropdown control. The options clearly differentiate between automatic execution on page load and manual execution.
A few observations:
- The dropdown uses "MANUAL" as the default value, which is a safer default than auto-execution
- The subText provides clear explanation of each behavior option
- The control is properly configured with all required properties
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (2)
9-12: Appropriate imports for run behavior constantsThe imports for RUN_BEHAVIOR and RUN_BEHAVIOR_VALUES are correctly added from PluginActionConstants, enabling consistent usage of the run behavior options.
20-24: Good implementation of run behavior dropdown in API settingsThe run behavior dropdown is properly implemented in the API settings form configuration:
- Uses the same property name "runBehavior" for consistency with backend
- Sets MANUAL as the default value via RUN_BEHAVIOR.MANUAL.label
- Utilizes RUN_BEHAVIOR_VALUES for options to maintain consistency
app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of run behavior dropdown in MySQL pluginThe implementation matches the pattern used in other plugins, maintaining consistency across the application. The dropdown options provide clear explanations of the behavior differences.
app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of run behavior dropdown in Firestore pluginThe implementation is consistent with other plugins, maintaining a uniform user experience. The dropdown provides clear options with helpful explanations in the subText fields.
app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json (1)
8-23: Implementation of run behavior enum is complete and correct.The implementation replaces the boolean toggle with a dropdown control offering well-defined behavior options. The descriptions are clear and informative for users.
app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json (1)
8-23: Implementation follows the same pattern consistently.The dropdown implementation is consistent with other plugins and provides clear user guidance.
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json (1)
8-23: Dropdown implementation matches the pattern used across plugins.The control and options are implemented correctly and consistently.
app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json (1)
8-23: Implementation consistently follows the established pattern.The control configuration is correct and descriptions are clear and helpful.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java (3)
18-18: Correctly imported the RunBehaviorEnum class.
104-110: Properly deprecated the executeOnLoad field with clear documentation.The Javadoc comment clearly instructs developers to use the new runBehavior field instead.
112-113: Added new runBehavior field with appropriate default value.The field is properly annotated with the same JsonView as the executeOnLoad field to maintain consistency in serialization.
app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json (1)
8-23: Appropriate replacement of boolean toggle with descriptive enum dropdownThe change from a boolean "executeOnLoad" to an enum-based "runBehavior" dropdown improves clarity by providing explicit options with helpful descriptions. The dropdown values PAGE_LOAD and MANUAL with their descriptive subtexts make the behavior more self-documenting than a simple toggle.
app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of run behavior dropdownThe implementation follows the same pattern as in other plugins, maintaining consistency across the application. The dropdown options provide clear descriptions of each behavior.
app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of run behavior dropdownConfiguration matches the implementation in other plugins, ensuring a consistent user experience across different database connectors.
app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of run behavior dropdownThe configuration for this AI plugin correctly implements the same pattern as the other plugins, ensuring consistent user experience across different plugin types.
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java (4)
5-5: Appropriate import for new enum typeThe addition of the RunBehaviorEnum import supports the new endpoint functionality.
119-127: Well-implemented new endpoint for run behaviorThe new endpoint follows the controller's established patterns, with appropriate logging and response handling. It properly leverages the service layer method
setRunBehavior.
129-132: Properly deprecated old endpointGood practice to include both a Javadoc comment and @deprecated annotation when deprecating endpoints. The comment clearly directs developers to use the new endpoint instead.
5-141:✅ Verification successful
Verify backward compatibility handling in service layer
The controller changes look good, but it's important to ensure that the service layer properly synchronizes the new runBehavior with the legacy executeOnLoad property for backward compatibility.
🏁 Script executed:
#!/bin/bash # Check if the service implementation properly handles synchronization between runBehavior and executeOnLoad rg -A 10 "setRunBehavior|setExecuteOnLoad" --type java app/server/appsmith-server/src/Length of output: 70778
Backward compatibility sync verified
TheLayoutActionServiceCEImplmethods forsetRunBehaviorandsetExecuteOnLoadcorrectly update bothrunBehaviorandexecuteOnLoad, ensuring legacy behavior is preserved. No further changes needed.app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (3)
5-8: Well-designed enum for representing run behavior.The
ActionRunBehaviourenum uses string values which ensures they're serializable when stored in JSON or transmitted over APIs. This is a good practice for enums that need to be persisted.
10-21: Good use of constant mapping for rich metadata.The
RUN_BEHAVIORobject provides a well-structured mapping of enum values to their display metadata. Including both labels and descriptive subtexts improves the UX by providing clear explanations of each option's behavior.
23-23: Convenient helper for UI components.Creating
RUN_BEHAVIOR_VALUESmakes it easy to use these options in dropdown selects and other UI components that expect arrays of options.app/client/src/components/formControls/DropDownControl.tsx (2)
32-43: Good separation of styling concerns with styled components.The new styled components
OptionLabelandOptionSubTexthelp maintain consistent styling for dropdown options throughout the application. This approach is preferable to inline styling.
511-527: Good use of conditional rendering for option display.The conditional rendering based on the presence of
subTextprovides flexibility while keeping the component structure clean. The nested Flex components create a good visual hierarchy.app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json (2)
85-86: Appropriate backward compatibility approach.Keeping both
executeOnLoadand addingrunBehaviorensures backward compatibility while transitioning to the new model. This allows for a smooth migration path.
2546-2561: Consistent dropdown configuration across settings.The dropdown configuration follows a consistent pattern across all plugin settings with clear labels and helpful subtexts. This promotes a unified user experience throughout the application.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (4)
17-21: Good practice importing and using centralized constants.Using the centralized constants from
PluginActionConstants.tsensures consistency across the application and avoids duplication.
50-51: Good state management transition.Successfully transitioned from managing a boolean state to an enum value, with proper initialization from props.
53-68: Well-updated callback for the new data type.The callback correctly handles the new enum value and updates both local state and propagates the change upward. The analytics event logging is properly updated to track the new value format.
80-108: Good implementation of the dropdown with rich options.The Select component is well-implemented with appropriate props and option rendering. The StyledSelect with width="fit-content" ensures the dropdown doesn't take excessive space.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java (3)
304-309: Effective bidirectional sync between executeOnLoad and runBehavior.The code correctly updates the runBehavior enum based on the boolean executeOnLoad value, maintaining proper synchronization between the legacy and new approach.
322-339: Well-implemented setRunBehavior method with proper backwards compatibility.The new method properly sets the runBehavior enum while also updating the legacy executeOnLoad boolean for backward compatibility. This ensures a smooth transition between the two properties.
426-434: Proper initialization of runBehavior during action creation.The code correctly sets the initial runBehavior based on both direct creation and clone scenarios, ensuring consistent behavior.
app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json (3)
87-87: Correct addition of runBehavior property in action configuration.The runBehavior property is correctly added with "MANUAL" value, which aligns with the existing executeOnLoad: false setting for backward compatibility.
2547-2607: Consistent implementation of runBehavior dropdown across plugin settings.The dropdown implementation is consistent and well-structured with clear labels and helpful subtext descriptions for each option, improving UX compared to the previous boolean toggle.
2691-2745: Consistent pattern applied across all plugins.The same dropdown pattern is consistently applied across all plugins, maintaining a unified interface for the run behavior setting.
app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (9)
2310-2311: Good implementation of runBehavior alongside executeOnLoadThe code correctly adds the new
runBehavior: "PAGE_LOAD"property that corresponds to the existingexecuteOnLoad: trueboolean value. This maintains backward compatibility while transitioning to the more descriptive enum-based approach.
2395-2396: Correct mapping from boolean to enum valueThe implementation properly maps
executeOnLoad: falsetorunBehavior: "MANUAL", maintaining consistency with the new enum pattern.
10043-10061: Well-designed dropdown control for runBehaviorThe dropdown implementation has clear labels and helpful descriptive text for each option. The control type, property mapping, and initial value are all properly configured.
10094-10111: Consistent dropdown implementation across configurationsThis dropdown control maintains the same structure and options as the previous one, ensuring a consistent user experience.
10145-10162: Consistent dropdown implementation for API settingsThe dropdown configuration for APIs matches the query settings, maintaining uniform behavior across different action types.
10225-10242: Matching dropdown implementationThis implementation matches the previous dropdown configurations, maintaining consistency throughout the codebase.
10417-10418: Proper JS action configurationThe JS action correctly implements the
runBehavior: "MANUAL"property alongsideexecuteOnLoad: false.
10468-10469: Consistent implementation across JS actionsAll JS action configurations maintain the same pattern of adding
runBehavior: "MANUAL"alongsideexecuteOnLoad: false, ensuring consistent behavior.Also applies to: 10553-10554, 10604-10605
2308-10608:❓ Verification inconclusive
Verify transition strategy for backward compatibility
The implementation retains both the old
executeOnLoadproperty and the newrunBehaviorproperty. Ensure there's handling logic elsewhere in the codebase that manages this transition appropriately.
🏁 Script executed:
#!/bin/bash # Search for places where both properties are accessed or where one property is used to set the other echo "Searching for code that handles both properties together..." rg -A 3 -B 3 "executeOnLoad.*runBehavior|runBehavior.*executeOnLoad" --type ts echo "Checking for migration/transition logic..." rg -A 5 "runBehavior.*PAGE_LOAD.*executeOnLoad.*true|executeOnLoad.*true.*runBehavior.*PAGE_LOAD" --type ts rg -A 5 "runBehavior.*MANUAL.*executeOnLoad.*false|executeOnLoad.*false.*runBehavior.*MANUAL" --type tsLength of output: 520
No transition logic found for backward compatibility
The search for code that handles both
executeOnLoadandrunBehaviorproperties together didn't return any results. There doesn't appear to be explicit migration logic between these properties in the codebase.Let's search for more specific implementations to verify how this is handled:
🏁 Script executed:
#!/bin/bash # Search for places where these properties are used independently echo "Searching for executeOnLoad usage..." rg "executeOnLoad" --type ts -l | head -10 echo "Searching for runBehavior usage..." rg "runBehavior" --type ts -l | head -10 # Look for places where actions are executed or initialized echo "Searching for action execution logic..." rg -A 5 "execute.*action|initialize.*action" --type ts | head -20Length of output: 2663
Let's look more specifically at how these properties are used by searching in the action execution flow:
🏁 Script executed:
#!/bin/bash # Check for actual usage in the action execution logic echo "Searching in ActionExecutionSagas..." rg -A 5 "executeOnLoad|runBehavior" app/client/src/sagas/ActionExecution/ # Look for where the properties are defined in the action model echo "Looking at Action entity definition..." rg -A 10 -B 5 "runBehavior|executeOnLoad" app/client/src/entities/Action/index.ts # Check if there's UI for setting these properties echo "Checking UI components for action settings..." rg -A 5 "executeOnLoad|runBehavior" app/client/src/pages/Editor/QueryEditor/Length of output: 2236
Let's check the settings components and form configuration to see how these properties are related:
🏁 Script executed:
#!/bin/bash # Look for how runBehavior is used in form configuration echo "Checking form configurations for runBehavior..." rg -A 10 "runBehavior" app/client/src/constants/AppsmithActionConstants/formConfig/ # Check for places where one property might be set based on the other echo "Looking for potential property mappings..." rg -A 5 "runBehavior.*=.*executeOnLoad|executeOnLoad.*=.*runBehavior" --type ts # Check the settings UI components echo "Checking settings UI components..." rg -A 10 "runBehavior" app/client/src/pages/Editor/Length of output: 26438
Both properties coexist without explicit transition logic
Based on the codebase analysis, I've confirmed that both
executeOnLoadandrunBehaviorproperties exist in parallel without explicit transition logic between them:
- Both properties are defined in the Action entity model
- The execution logic in
PluginActionSaga.tsstill relies onexecuteOnLoadfor determining whether to clear action responses- The UI components and form configurations have moved to using
runBehaviorexclusively- No code was found that synchronizes or maps between these properties
The codebase appears to be in a transition state where newer components use
runBehaviorwhile older execution logic depends onexecuteOnLoad. Without explicit mapping logic, there's a risk that these properties could become out of sync, especially if they're set through different paths.A proper backward compatibility strategy would include code that explicitly synchronizes these values (e.g., setting
executeOnLoad=truewhenrunBehavior=PAGE_LOADand vice versa).app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java (3)
11-11: Proper import for the new RunBehaviorEnum.Good addition of the RunBehaviorEnum import, which aligns with the migration from boolean executeOnLoad to enumerated run behavior.
1357-1402: Well-structured test for the new RunBehavior functionality.This test thoroughly validates the RunBehavior functionality by:
- Confirming default values (MANUAL for runBehavior, FALSE for executeOnLoad)
- Testing transitions between behavior states (MANUAL ↔ ON_PAGE_LOAD)
- Verifying that changes persist correctly in the repository
The test ensures that the new RunBehavior enum correctly synchronizes with the legacy executeOnLoad flag, maintaining backward compatibility.
1404-1441: Good backward compatibility test for executeOnLoad.This test properly verifies that the legacy executeOnLoad API still functions correctly while updating the new runBehavior field appropriately:
- Setting executeOnLoad=TRUE updates runBehavior=ON_PAGE_LOAD
- Setting executeOnLoad=FALSE updates runBehavior=MANUAL
This ensures a smooth transition for existing code still using the executeOnLoad property.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java (1)
119-141: 💡 Verification agent🧩 Analysis chain
Verify endpoint requests in client code
Ensure all client code using the old
/executeOnLoadendpoint is updated to use the new/runBehaviorendpoint.
🏁 Script executed:
#!/bin/bash # Check for any remaining usages of the old endpoint in client code rg -A 2 -B 2 "executeOnLoad.*branchedActionId" --glob "*.{ts,tsx,js,jsx}"Length of output: 75
Let's broaden the search to ensure no hardcoded references to the deprecated
/executeOnLoadremain:
🏁 Script executed:
#!/bin/bash # Search for any remaining usage of the deprecated endpoint path rg -n -F "/executeOnLoad" --glob "*.{ts,tsx,js,jsx,html,vue}" rg -n "executeOnLoad\(" --glob "*.{ts,tsx,js,jsx}"Length of output: 353
Update deprecated
/executeOnLoadreferences in client codePlease replace all remaining calls to the old endpoint with the new
/runBehaviorAPI:• app/client/src/api/ActionAPI.tsx (around line 308)
– Change
ts return API.put( ActionAPI.url + `/executeOnLoad/${actionId}`, undefined, { … } );
to something like
ts return API.put( ActionAPI.url + `/runBehavior/${actionId}`, { behavior: flag ? RunBehaviorEnum.ON_LOAD : RunBehaviorEnum.MANUAL }, { … } );• app/client/cypress/support/commands.js (around line 637)
– Update the intercept from
js cy.intercept("PUT", "/api/v1/actions/executeOnLoad/*").as("setExecuteOnLoad");
to
js cy.intercept("PUT", "/api/v1/actions/runBehavior/*").as("setRunBehavior");
🧹 Nitpick comments (9)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java (1)
1-9: Enum definition looks good but could align naming with client.The enum and documentation are clear. However, there's a naming discrepancy between server (
ON_PAGE_LOAD) and client (PAGE_LOAD). Consider aligning these for consistency, though it's acceptable if there's mapping logic elsewhere.- ON_PAGE_LOAD // Action will run when the page loads + PAGE_LOAD // Action will run when the page loadsapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java (1)
19-21: Consider deprecating the old setExecuteOnLoad method.Since you're adding a new setRunBehavior method, it would be helpful to mark the old setExecuteOnLoad method as deprecated to guide developers toward using the new method.
+ @Deprecated Mono<ActionDTO> setExecuteOnLoad(String id, Boolean isExecuteOnLoad);app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (1)
19-23: Terminology mismatch in label vs. file name.The label mentions "API" while the file name refers to "GoogleSheets". This might be confusing to developers. Consider updating the label to be more specific to Google Sheets.
- label: "Request confirmation before running this API", + label: "Request confirmation before running this Google Sheets action",app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json (1)
8-23: IntroducerunBehaviordropdown in OpenAI plugin settings.
The booleanexecuteOnLoadtoggle is correctly replaced with an enum-driven dropdown offeringPAGE_LOADandMANUAL, consistent with the newActionRunBehaviourdesign.
Consider extracting this dropdown definition into a shared schema or constants file to avoid duplication across plugins.app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json (1)
12-23: Refine option descriptions for clarity
ThesubTextstrings are informative but could be more concise. For example:
- Change
"Query runs when the page loads or when manually triggered"→"Runs automatically on page load; also triggerable manually."- Change
"Query only runs when called in an event or JS with .run()"→"Executes only when explicitly triggered (e.g., via events or.run())."
This will improve readability in the UI.app/client/src/components/formControls/DropDownControl.tsx (1)
511-528: Enhanced dropdown option rendering with support for subtextsThe conditional rendering in
renderOptionWithIconprovides a clean way to display additional explanatory text for dropdown options when needed.Consider extracting these styled components to a shared location to avoid duplication if they're used in multiple files.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (1)
24-37: Duplicated styled componentsThese styled components are very similar to those defined in DropDownControl.tsx.
Consider extracting these to a shared component library to maintain DRY principles:
-const OptionLabel = styled(Text)` - color: var(--ads-v2-color-fg); - font-size: 14px; - font-weight: 500; -`; - -const OptionSubText = styled(Text)` - color: var(--ads-v2-color-fg-muted); - font-size: 12px; -`; +import { OptionLabel, OptionSubText } from "components/shared/DropdownStyles";app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (2)
10151-10160: Update subText to reference API instead of QueryThe subText in the options refers to "Query" but this configuration is for an API action.
- "Query runs when the page loads or when manually triggered", + "API runs when the page loads or when manually triggered",- "Query only runs when called in an event or JS with .run()", + "API only runs when called in an event or JS with .run()",
10231-10240: Update subText to reference API instead of QueryThe subText in the options refers to "Query" but this configuration is for an API action.
- "Query runs when the page loads or when manually triggered", + "API runs when the page loads or when manually triggered",- "Query only runs when called in an event or JS with .run()", + "API only runs when called in an event or JS with .run()",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts(1 hunks)app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts(1 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts(10 hunks)app/client/src/components/formControls/DropDownControl.tsx(4 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts(1 hunks)app/client/src/entities/Action/index.ts(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts(1 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json(8 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json(8 hunks)app/client/test/factories/MockPluginsState.ts(4 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviorEnum.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java(2 hunks)app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(3 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR(10-21)RUN_BEHAVIOR_VALUES(23-23)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (5)
app/client/src/ce/pages/workspace/InviteUsersForm.tsx (1)
OptionLabel(127-134)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/JSObjectNameEditor.tsx (1)
Text(54-58)app/client/packages/design-system/ads/src/Select/Select.tsx (2)
Select(143-143)Option(143-143)app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (2)
RUN_BEHAVIOR_VALUES(23-23)RUN_BEHAVIOR(10-21)app/client/packages/design-system/ads/src/Select/Select.types.ts (1)
SelectOptionProps(16-19)
app/client/src/components/formControls/DropDownControl.tsx (1)
app/client/src/ce/pages/workspace/InviteUsersForm.tsx (1)
OptionLabel(127-134)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (56)
app/client/src/ce/constants/messages.ts (1)
2586-2586: Message update aligns with the new run behavior dropdownThe message text has been updated from "Choose the functions to run on page load" to "Choose functions run behavior" to reflect the new dropdown-based run behavior selection instead of the previous boolean toggle.
app/client/src/entities/Action/index.ts (2)
13-13: Import for new ActionRunBehaviour enumImporting the ActionRunBehaviour enum which will be used for the new run behavior property.
144-144: Added new runBehavior property to BaseAction interfaceThis is a key change for the PR - adding the new runBehavior property while keeping the executeOnLoad boolean for backward compatibility. This enables a more expressive way to define when actions should run.
Let's verify that runBehavior is properly used throughout the codebase to ensure consistent behavior:
#!/bin/bash # Check if runBehavior is properly used alongside executeOnLoad rg "runBehavior.*executeOnLoad|executeOnLoad.*runBehavior" -A 5 -B 5app/client/src/pages/Editor/JSEditor/JSEditorToolbar/types.ts (1)
5-5: Extended OnUpdateSettingsProps.value type to support string valuesType definition has been expanded from
boolean | numbertoboolean | number | stringto support the new string-based enum values for run behavior in settings controls.app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts (2)
12-12: Import ActionRunBehaviour enum for testsImporting the ActionRunBehaviour enum to use in the test constants.
20-20: Added runBehavior to test constantsAdded the new runBehavior property with MANUAL value to BASE_ACTION test constant, which is consistent with the default executeOnLoad=false value. This ensures tests properly validate the new property.
Let's check if all test cases involving actions have been updated:
#!/bin/bash # Find test files that might need updating for runBehavior rg "executeOnLoad.*test|test.*executeOnLoad" --type=ts "app/client/src/**/*.test.ts"app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts (2)
1-4: Imports look good.These imports bring in the necessary RUN_BEHAVIOR constants and values for the dropdown implementation.
12-16:❓ Verification inconclusive
Appropriate replacement of executeOnLoad toggle with runBehavior dropdown.
The switch from a boolean toggle to an enum-based dropdown provides more flexibility and better UX. Setting MANUAL as the initial value is a good default for new queries.
🏁 Script executed:
#!/bin/bash # Check if there's migration logic for existing queries with executeOnLoad rg -A 3 "executeOnLoad.*runBehavior|runBehavior.*executeOnLoad" --type tsLength of output: 75
Ensure migration from
executeOnLoadtorunBehaviorI didn’t find any code that migrates existing queries’
executeOnLoadboolean into the newrunBehaviorenum. Without this, older queries may default incorrectly.• File to update:
– app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts
• Action item:
– Add a data migration or initialization step that mapsexecuteOnLoad: true→runBehavior: RUN_BEHAVIOR.ON_PAGE_LOAD.label, andfalse→RUN_BEHAVIOR.MANUAL.labelapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java (1)
4-4: Import for new enum looks good.The import statement is correctly added for the new RunBehaviorEnum.
app/client/src/constants/AppsmithActionConstants/formConfig/GoogleSheetsSettingsConfig.ts (2)
1-4: Imports look good.Correctly importing the necessary run behavior constants and values.
12-16: Dropdown implementation looks good.The dropdown implementation for run behavior is consistent with other action types, which is good for UX consistency.
app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json (1)
8-23: Good integration of runBehavior dropdown with clear optionsThe addition of the runBehavior dropdown with PAGE_LOAD and MANUAL options provides a more expressive way to configure query execution than the previous boolean toggle. The descriptive subtext for each option clearly communicates the behavior to users.
Note that the default value has been set to "MANUAL" which means queries won't automatically run on page load unless specifically configured. This is a reasonable default from a performance perspective.
app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation of runBehavior dropdownThe implementation maintains consistency with other plugins, using the same structure, options, and default values. This consistency is important for providing a uniform experience across different plugin types.
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts (1)
9-12: Proper import of run behavior constantsGood addition of the necessary imports from PluginActionConstants to support the new run behavior dropdown.
app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation across plugin configurationsThis change maintains the same pattern seen in other plugins, which is excellent for consistency. The dropdown options and their descriptions clearly communicate the execution behavior to users.
app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json (1)
8-23: AdoptrunBehaviordropdown for MySQL plugin.
This mirrors the update in other plugins, with a default ofMANUALand clear labels/subtexts.app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json (1)
8-23: ReplaceexecuteOnLoadwithrunBehaviordropdown in Google AI plugin.
Configuration aligns with platform-wide enum approach and defaults correctly toMANUAL.app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json (1)
8-23: UserunBehaviordropdown in Oracle plugin settings.
The new controlType and options match the intended design and maintain consistency with other plugins.app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json (1)
8-23: Switch torunBehaviordropdown for Mongo plugin.
This change is consistent with the enum-based run behaviour pattern and properly defaults toMANUAL.app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json (1)
8-23: Appropriate replacement of boolean toggle with dropdown enumThe change from a boolean
executeOnLoadtoggle to a more expressiverunBehaviordropdown improves the user experience by providing clearer options with helpful descriptive text. The two options - "On page load" and "Manual" - are well-labeled with explanatory subtext.Setting the default to "MANUAL" is a sensible choice for safety, preventing unintended automatic executions.
app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json (1)
8-23: LGTM - Good consistency with other plugin settingsThis implementation matches the pattern established in the Amazon S3 plugin, maintaining consistency across the platform. The dropdown options are clear and the default value is appropriate.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java (2)
18-18: Correct import for the new enum typeThe import for
RunBehaviorEnumis correctly added to support the new field.
104-113: Well-implemented backward compatibility patternThe implementation correctly:
- Properly deprecates the old field with clear documentation
- Maintains the same JSON view annotations for both fields
- Sets an appropriate default value for the new field
This approach ensures a smooth transition without breaking existing code.
app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json (1)
8-23: Consistent implementation across pluginsThis implementation consistently follows the same pattern used in other plugins, ensuring a uniform user experience across different data sources. The dropdown options and default values match the platform-wide pattern.
app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json (1)
8-11: Verify default runBehavior and migration path
The new dropdown forrunBehaviorlooks correct, but please confirm that the default"MANUAL"value aligns with your intended migration strategy. Ensure any existingexecuteOnLoad: truesettings are mapped to"PAGE_LOAD"(andfalseto"MANUAL") so users don’t experience unexpected behavior post-upgrade.app/client/test/factories/MockPluginsState.ts (3)
6955-6973: Good implementation of the new run behavior dropdown.The change replaces the boolean
executeOnLoadtoggle with a more expressiverunBehaviorenum dropdown. This provides clearer options with descriptive subtexts that explain when queries will execute.
7115-7133: Consistent implementation in AI plugin settings.The same run behavior dropdown pattern is correctly applied to the Appsmith AI plugin settings, maintaining consistency across different plugin types.
7159-7177: Run behavior dropdown properly implemented for database plugins.The pattern is consistently applied to database plugins as well, completing the migration from boolean toggle to enum dropdown across all plugin types in the mock state.
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (3)
5-8: Well-defined enum for action run behaviorThe
ActionRunBehaviourenum provides clear, descriptive options that replace the boolean toggle approach, making the intention more explicit in code.
10-21: Good implementation of run behavior constants with descriptive labels and subtextsThe
RUN_BEHAVIORobject provides a complete set of metadata for each behavior option, including human-readable labels and explanatory subtexts that will improve the UI experience.
23-23: Clean utility export for dropdown integrationCreating
RUN_BEHAVIOR_VALUESmakes it straightforward to use these options in dropdown components throughout the application.app/client/src/components/formControls/DropDownControl.tsx (2)
32-43: Good implementation of styled components for dropdown optionsThe
OptionLabelandOptionSubTextcomponents provide consistent styling for dropdown options with clear visual hierarchy.
477-477: Fixed list height for better UI consistencySetting a consistent
listHeighthelps maintain UI predictability across different dropdown instances.app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (4)
17-21: Good import of run behavior constantsImporting the necessary enum and constants ensures consistency with the definitions in PluginActionConstants.
50-51: Updated state management for run behaviorGood transition from boolean state (
executeOnPageLoad) to enum-based state (runBehavior), ensuring the component correctly uses the new property.
53-68: Updated callback with proper typingThe callback correctly handles the new
ActionRunBehaviourtype and properly updates both local state and parent component state.
79-108: Improved UI with dropdown select instead of toggleThe dropdown provides a more extensible UI that can accommodate additional run behavior options in the future. Good use of the styled components for consistent presentation.
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java (3)
5-5: Added import for the new enumCorrectly importing the RunBehaviorEnum class for use in the new endpoint.
119-127: Well-implemented new endpoint for setting run behaviorThe new endpoint correctly accepts a RunBehaviorEnum parameter and delegates to the appropriate service method. The logging statement is descriptive and follows the pattern used elsewhere in the controller.
129-133: Proper deprecation of the old endpointGood practice to include both a Javadoc comment explaining the deprecation and the @deprecated annotation. This provides clear guidance to API consumers on migration.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java (3)
304-309: Good implementation of backward compatibility.The code correctly updates the new
runBehaviorenum based on the existingexecuteOnLoadboolean, ensuring forward compatibility while maintaining the old property.
322-349: Well-structured bidirectional compatibility implementation.The new
setRunBehaviormethod properly handles the synchronization betweenrunBehaviorandexecuteOnLoad, ensuring both properties remain consistent. This maintains compatibility with legacy code that might still rely on the boolean flag.
426-434: Consistent behavior for cloned pages.The implementation correctly handles the initialization of
runBehaviorfor both new actions and cloned pages, maintaining the expected behavior while transitioning to the new enum-based property.app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json (3)
87-87: Good addition of runBehavior property.Adding the
runBehaviorproperty alongside the existingexecuteOnLoadprovides a smooth transition path for clients.
2547-2607: Improved UI with descriptive options.The dropdown implementation is more user-friendly than the previous boolean toggle, with clear labels and helpful subtext explaining each option's behavior.
2730-2745: Consistent dropdown implementation across plugins.The same pattern is consistently applied across different plugin types, maintaining a uniform user experience throughout the application.
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java (3)
11-11: Import addition looks goodAdding the necessary import for RunBehaviorEnum to support new test cases.
1357-1402: Well-structured test for RunBehavior synchronizationGood test implementation that verifies the bidirectional relationship between the new RunBehaviorEnum and the legacy executeOnLoad flag. The test confirms:
- Default values (MANUAL/false) for new actions
- Setting to ON_PAGE_LOAD properly updates executeOnLoad to TRUE
- Setting back to MANUAL updates executeOnLoad while preserving userSetOnLoad
- Changes are persisted in the database
1404-1441: Good test coverage for backward compatibilityThis test ensures that the legacy executeOnLoad property remains functional and synchronizes correctly with the new runBehavior enum:
- Setting executeOnLoad=TRUE updates runBehavior to ON_PAGE_LOAD
- Setting executeOnLoad=FALSE updates runBehavior to MANUAL
This is essential for maintaining backward compatibility during the transition.
app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json (3)
86-86: Added runBehavior property to match backend changesCorrectly added the new runBehavior property with default "MANUAL" value to align with the backend changes.
2546-2561: Good UI update for MongoDB plugin settingsThe implementation replaces the boolean toggle with a more descriptive dropdown that offers better user guidance through subtext explanations of each option.
2592-2607: Consistent UI changes across all pluginsThe same dropdown pattern has been consistently applied across all plugins (PostgreSQL, REST API, Firestore, Google Sheets, S3, Redis), maintaining a uniform user experience.
The dropdown options and descriptions are identical across all plugin types, which is good for usability.
Also applies to: 2638-2653, 2690-2705, 2729-2744, 2775-2790, 2821-2836
app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (4)
2310-2311: Correct runBehavior implementation for PAGE_LOADThe
runBehavior: "PAGE_LOAD"property correctly corresponds toexecuteOnLoad: true, maintaining backward compatibility while introducing the new enum-based approach.
2395-2396: Correct runBehavior implementation for MANUALThe
runBehavior: "MANUAL"property correctly corresponds toexecuteOnLoad: false, maintaining consistency with the new pattern.
10043-10060: Well-structured dropdown control for run behaviorThe dropdown implementation provides clear labels and helpful explanatory text for each option, which is a significant UX improvement over a simple boolean toggle.
10417-10418: Consistent implementation for JS actionsThe
runBehaviorproperty is correctly implemented for JS actions, maintaining the same pattern used for queries and APIs.
app/client/src/constants/AppsmithActionConstants/formConfig/ApiSettingsConfig.ts
Outdated
Show resolved
Hide resolved
app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json
Outdated
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-40355.dp.appsmith.com |
…ior` on frontend (#40358) ## Description Replacing remaining instances of `executeOnLoad` with `runBehavior` on frontend Fixes [#40311](#40311) [#39830](#39830) ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
ankitakinger
requested review from
ApekshaBhosale and
ayushpahwa
as code owners
|
/build-deploy-preview skip-tests=true |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/utils/JSPaneUtils.tsx (1)
6-6: Note the inconsistent spelling between "Behaviour" and "behavior".There's an inconsistency in naming conventions between the imported
ActionRunBehaviour(British spelling) and its usage asrunBehavior(American spelling) throughout the code. Consider standardizing on one spelling across the codebase.Also applies to: 134-134, 233-234, 245-246, 285-286
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (1)
5-25: Inconsistent spelling between "Behaviour" and "behavior".The codebase mixes British English (
Behaviourin enum and type names) and American English (behaviorin constant names). Consider standardizing on one spelling convention throughout the codebase to avoid confusion.app/client/src/sagas/JSPaneSagas.ts (1)
1-957: Suggestion: Add documentation about ActionRunBehaviour values.Consider adding documentation explaining the available values for
ActionRunBehaviourto make the code more maintainable. A comment with the possible enum values would be helpful for future developers.🧰 Tools
🪛 Biome (1.9.4)
[error] 363-363: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 369-369: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 377-377: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 540-540: 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)
[error] 583-583: 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)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (46)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts(1 hunks)app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts(1 hunks)app/client/src/actions/pluginActionActions.ts(2 hunks)app/client/src/api/ActionAPI.tsx(2 hunks)app/client/src/api/PageApi.tsx(2 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(4 hunks)app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts(4 hunks)app/client/src/ce/reducers/entityReducers/actionsReducer.tsx(3 hunks)app/client/src/ce/reducers/entityReducers/jsActionsReducer.tsx(4 hunks)app/client/src/ce/sagas/PageSagas.tsx(2 hunks)app/client/src/ce/utils/autocomplete/EntityDefinitions.test.ts(2 hunks)app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts(10 hunks)app/client/src/components/editorComponents/utils.test.ts(2 hunks)app/client/src/entities/Action/actionProperties.test.ts(1 hunks)app/client/src/entities/Action/index.ts(2 hunks)app/client/src/pages/Editor/EntityNavigation/JSObjectsPane/index.ts(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/utils.test.ts(2 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/FinalState.json(8 hunks)app/client/src/pages/Editor/SaaSEditor/__data__/InitialState.json(8 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(2 hunks)app/client/src/sagas/ActionSagas.ts(4 hunks)app/client/src/sagas/BuildingBlockSagas/tests/fixtures.ts(1 hunks)app/client/src/sagas/JSPaneSagas.ts(4 hunks)app/client/src/utils/DynamicBindingUtils.test.ts(1 hunks)app/client/src/utils/FilterInternalProperties/JsAction.ts(2 hunks)app/client/src/utils/JSPaneUtils.test.ts(11 hunks)app/client/src/utils/JSPaneUtils.tsx(5 hunks)app/client/test/factories/Actions/API.ts(1 hunks)app/client/test/factories/Actions/GoogleSheetFactory.ts(1 hunks)app/client/test/factories/Actions/JSObject.ts(2 hunks)app/client/test/factories/Actions/Postgres.ts(1 hunks)app/client/test/factories/MockPluginsState.ts(4 hunks)app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json(1 hunks)app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- app/client/src/components/editorComponents/utils.test.ts
- app/client/src/utils/JSPaneUtils.test.ts
- app/client/src/pages/Editor/SaaSEditor/data/InitialState.json
🚧 Files skipped from review as they are similar to previous changes (18)
- app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/setting.json
- app/client/src/PluginActionEditor/transformers/RestActionTransformers.test.ts
- app/client/src/entities/Action/index.ts
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/mongoPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/openAiPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/googleAiPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/setting.json
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/googleSheetsPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/anthropicPlugin/src/main/resources/setting.json
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/setting.json
- app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts
- app/client/src/pages/Editor/SaaSEditor/data/FinalState.json
- app/client/test/factories/MockPluginsState.ts
- app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/client/src/sagas/ActionExecution/PluginActionSaga.ts (2)
app/client/src/ce/reducers/entityReducers/actionsReducer.tsx (1)
ActionData(18-22)app/client/src/ce/selectors/entitiesSelector.ts (1)
getCurrentActions(684-692)
app/client/src/api/ActionAPI.tsx (1)
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (1)
ActionRunBehaviourType(25-25)
app/client/src/actions/pluginActionActions.ts (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1281-1324)
🪛 Biome (1.9.4)
app/client/src/ce/sagas/PageSagas.tsx
[error] 543-543: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 546-546: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (54)
app/client/src/utils/DynamicBindingUtils.test.ts (1)
123-123: Property change matches the new run behavior model.The change from
executeOnLoad: falsetorunBehavior: "MANUAL"properly aligns with the broader migration from boolean flags to the more descriptive enum-based approach in the action configuration model.app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts (1)
47-47: Consistent migration to new run behavior model across test objects.All JS action test objects have been properly updated from the boolean
executeOnLoad: falseto the more descriptiverunBehavior: "MANUAL". This maintains consistency with the codebase-wide migration to the new run behavior model.Also applies to: 94-94, 242-242, 289-289
app/client/src/ce/utils/autocomplete/EntityDefinitions.test.ts (1)
47-47: Test data updated to use new run behavior property.The JS object test configurations have been correctly migrated from
executeOnLoad: falsetorunBehavior: "MANUAL", ensuring test data remains consistent with the new action execution model.Also applies to: 89-89
app/client/test/factories/Actions/API.ts (1)
66-66: API factory updated to use run behavior enum.The API factory definition has been updated from
executeOnLoad: falsetorunBehavior: "MANUAL", ensuring that test actions generated by this factory align with the new execution behavior model.app/client/test/factories/Actions/JSObject.ts (1)
33-33: Correctly migrated to the new runBehavior propertyThe change from
executeOnLoad: falsetorunBehavior: "MANUAL"aligns with the broader refactoring effort throughout the codebase. This ensures test factories generate objects with the new property structure.Also applies to: 73-73
app/client/src/sagas/BuildingBlockSagas/tests/fixtures.ts (1)
132-132: Correctly migrated to the new runBehavior propertyThe change from
executeOnLoad: truetorunBehavior: "ON_PAGE_LOAD"in the action fixture is correctly implemented. This ensures the test data matches the new execution behavior model.app/client/test/factories/Actions/GoogleSheetFactory.ts (1)
66-66: Correctly migrated to the new runBehavior propertyThe change from
executeOnLoad: falsetorunBehavior: "MANUAL"is appropriate for this Google Sheets action factory. This ensures tests using this factory will create actions with the new property structure.app/client/src/entities/Action/actionProperties.test.ts (1)
13-13: Correctly migrated to the new runBehavior propertyThe DEFAULT_ACTION test fixture has been properly updated from
executeOnLoad: falsetorunBehavior: "MANUAL". This ensures test cases use the new property structure while maintaining the same logical behavior.app/client/test/factories/Actions/Postgres.ts (1)
34-34: Migration torunBehaviorproperty looks good.This change aligns with the PR objective of replacing boolean
executeOnLoadwith the more expressiverunBehaviorenum. The value "MANUAL" is consistent with the new execution behavior model.app/client/src/sagas/ActionExecution/PluginActionSaga.ts (2)
173-173: Import of ActionRunBehaviour enum added correctly.The imported enum is used in the function below to check run behavior types.
1557-1566: Function updated to use runBehavior instead of executeOnLoad.The function now uses the new
ActionRunBehaviour.ON_PAGE_LOADenum value to determine which actions should have their responses cleared, maintaining the same logic but with the new property.app/client/src/utils/FilterInternalProperties/JsAction.ts (2)
4-4: Required import for ActionRunBehaviour added.This import is needed for the updated condition below.
19-22: Logic updated to use runBehavior property.The condition now checks for
jsChild.runBehavior === ActionRunBehaviour.ON_PAGE_LOADinstead of the deprecatedexecuteOnLoadboolean, maintaining the same behavior with the new enum-based approach.app/client/src/pages/Editor/EntityNavigation/JSObjectsPane/index.ts (1)
84-84: Updated navigation path check to use runBehavior.Property path check properly updated to look for "runBehavior" instead of "executeOnLoad" to ensure navigation to the correct tab.
app/client/src/pages/Editor/JSEditor/utils.test.ts (2)
9-9: Import added for ActionRunBehaviour enum.The import is correctly added to support the migration from boolean executeOnLoad to enum-based runBehavior.
48-48: Boolean executeOnLoad replaced with enum-based runBehavior.The change from
executeOnLoad: falsetorunBehavior: ActionRunBehaviour.MANUALaligns with the broader refactoring effort. This makes the code more semantically clear about when the action should run.app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx (2)
7-7: Import added for ActionRunBehaviour enum.The import is correctly added to support the migration from boolean executeOnLoad to enum-based runBehavior in the test file.
58-59: Boolean executeOnLoad replaced with enum-based runBehavior in test objects.Test data now properly uses
runBehavior: ActionRunBehaviour.ON_PAGE_LOADandrunBehavior: ActionRunBehaviour.MANUALinstead of boolean values. This maintains the existing test assertions while aligning with the new API design.Also applies to: 62-63
app/client/src/api/PageApi.tsx (2)
12-12: Import added for ActionRunBehaviour type.The import is correctly added to support the interface changes.
73-73: Boolean executeOnLoad replaced with typed runBehavior in interface.The
SavePageResponseDatainterface now correctly usesrunBehavior: ActionRunBehaviourinstead of the previous boolean type. This ensures type safety and aligns with the broader refactoring effort.app/client/src/api/ActionAPI.tsx (2)
13-13: Import added for ActionRunBehaviourType.The import is correctly added to support the method parameter type changes.
305-311: Method renamed and parameter type changed for better semantics.The method has been properly updated from
toggleActionExecuteOnLoadtoupdateActionRunBehavior, with the parameter changing from a boolean to the typedActionRunBehaviourType. The API endpoint and payload structure have also been updated accordingly.This change improves code clarity by:
- Using a more descriptive method name
- Using a more expressive parameter type
- Making the API endpoint more consistent with its function
app/client/src/ce/sagas/PageSagas.tsx (3)
91-92: Import changes align with the run behavior refactoring.The imports have been renamed from
setActionsToExecuteOnPageLoadandsetJSActionsToExecuteOnPageLoadtosetActionsRunBehaviorandsetJSActionsRunBehaviorrespectively, which is consistent with the broader refactoring effort.
539-540: Updated action dispatch to use new action creator.The action has been properly updated from
setActionsToExecuteOnPageLoadtosetActionsRunBehavior, maintaining the same parameter structure while aligning with the new run behavior model.
547-548: Updated JS action dispatch to use new action creator.The action has been properly updated from
setJSActionsToExecuteOnPageLoadtosetJSActionsRunBehavior, maintaining the same parameter structure while aligning with the new run behavior model.app/client/src/utils/JSPaneUtils.tsx (3)
6-6: Added import for ActionRunBehaviour enum.The new import correctly brings in the enum needed for the updated run behavior model.
134-134: Replaced executeOnLoad boolean with runBehavior enum.Updated from
executeOnLoad: falsetorunBehavior: ActionRunBehaviour.MANUAL, which is semantically equivalent and more descriptive.
233-234: Consistent replacement of executeOnLoad with runBehavior across all JS actions.All instances of the boolean flag have been properly updated to use the enum value.
Also applies to: 245-246, 285-286
app/client/src/ce/reducers/entityReducers/actionsReducer.tsx (2)
16-16: Added import for ActionRunBehaviour type.Correctly imported the type needed for the run behavior property.
314-332: Updated reducer handler for run behavior.The reducer handler has been properly updated:
- Action type changed from
SET_ACTION_TO_EXECUTE_ON_PAGELOADtoSET_ACTION_RUN_BEHAVIOR- Payload type now includes
runBehaviorinstead ofexecuteOnLoad- State update logic properly assigns
runBehaviorvaluesThis implementation correctly maintains the overall structure while adopting the new run behavior model.
app/client/src/PluginActionEditor/constants/PluginActionConstants.ts (4)
5-8: Well-structured enum for action run behaviors.The enum provides clear, distinct values for the two run behavior modes.
10-21: Comprehensive metadata for run behavior options.The
RUN_BEHAVIORobject provides excellent user-facing information with:
- Descriptive labels for UI display
- Helpful subtext explaining the behavior of each option
- References to the enum values for type safety
This approach facilitates better UI experiences while maintaining type safety.
23-23: Added convenience array for iteration.The
RUN_BEHAVIOR_VALUESarray enables easy iteration over behavior options in UI components.
25-25: Type alias using template literal type.The
ActionRunBehaviourTypeprovides type safety using TypeScript's template literal types, which is a good practice for enum-based string types.app/client/src/actions/pluginActionActions.ts (3)
23-23: Good addition of type import.The import of
ActionRunBehaviourtype aligns with the transition from boolean flags to enum-based control.
355-366: Function renamed to better reflect its purpose.The renaming from
setActionsToExecuteOnPageLoadtosetActionsRunBehaviorclearly indicates the intent of this action creator. The payload structure is properly updated to use the more expressiverunBehaviorproperty.
368-380: Consistent naming applied to JS actions.The same pattern is correctly applied to JS actions, maintaining consistency across the codebase while providing more flexibility with the enum-based approach.
app/client/src/ce/reducers/entityReducers/jsActionsReducer.tsx (3)
11-11: Type import aligns with other files.Proper import of the
ActionRunBehaviourtype ensures type safety throughout the reducer.
364-392: Properly updated reducer handler.The reducer now correctly handles the
UPDATE_FUNCTION_RUN_BEHAVIOR_SUCCESSaction type and updates the state with therunBehaviorproperty instead ofexecuteOnLoad.
393-420: Batch update handler correctly updated.The reducer logic for batch updates correctly sets the
runBehaviorproperty on all relevant actions in the collection.app/client/src/sagas/ActionSagas.ts (5)
150-150: Type import for ActionRunBehaviourType.Correctly imported the specific type needed for saga function parameters.
1042-1052: Property name check updated in setActionPropertySaga.The conditional check now properly targets the new
runBehaviorproperty instead ofexecuteOnLoad, dispatching the appropriate action with the correct type.
1064-1089: Saga function renamed and implementation updated.The function is renamed from
toggleActionExecuteOnLoadSagatoupdateActionRunBehaviorSagawith updated parameter types and API calls.
1269-1271: Updated watcher saga configuration.The watcher saga now listens for the correct action type and calls the renamed saga function.
464-464:❌ Incorrect review comment
Verify if
onPageLoadparameter should be updated.The
onPageLoadparameter in this function might be related to the oldexecuteOnLoadbehavior. Consider whether this should be updated to work with the newrunBehaviorproperty instead.
🏁 Script executed:
#!/bin/bash # Search for other usages of onPageLoad to see if it's related to executeOnLoad rg "onPageLoad.*executeOnLoad|executeOnLoad.*onPageLoad" -A 3 -B 3Length of output: 1472
No
onPageLoadparameter in this saga – no updates needed
I searchedActionSagas.tsand only foundtimeoutInMillisecondat line 464. There’s noonPageLoadin this function, andexecuteOnLoadshows up only in Cypress locators. No migration torunBehavioris required here.Likely an incorrect or invalid review comment.
app/client/src/sagas/JSPaneSagas.ts (4)
104-104: Type import for ActionRunBehaviourType.Correctly imported the specific type needed for saga function parameters.
786-797: Property name check updated in setFunctionPropertySaga.The conditional check now properly targets the new
runBehaviorproperty instead ofexecuteOnLoad.
850-882: Saga function renamed and implementation updated.The function is properly renamed from
toggleFunctionExecuteOnLoadSagatoupdateFunctionRunBehaviorSagawith consistent parameter and payload updates.
947-949: Updated watcher saga configuration.The watcher saga now listens for the correct action type and calls the renamed saga function.
app/client/src/ce/constants/ReduxActionConstants.tsx (5)
69-72: Well-structured new JS action run behavior constantsThese new constants support the migration from executeOnLoad toggle to the run behavior enum model for JavaScript actions. They follow the established pattern of having init, success, and error action types.
87-87: Appropriate error handling constant addedThe error constant for JS function run behavior updates aligns with the other constants in this file, maintaining consistency.
808-810: Consistent implementation of run behavior actions for regular actionsThese constants mirror the JS action equivalents, providing a consistent approach for both action types in the codebase.
824-824: Error constant completes the run behavior action setThe error constant for regular action run behavior updates follows the same pattern as the JS action constants, ensuring consistency.
69-824:✅ Verification successful
Verify test failures are not related to these constants
The PR mentions Cypress test failures. While these constants follow proper naming conventions and patterns, ensure the failures aren't related to action type name mismatches or missing constants.
🏁 Script executed:
#!/bin/bash # Check for any hardcoded references to the old action types echo "Checking for any hardcoded references to toggle execute-on-load action types..." rg -i "toggle.*execute.*on.*load" --type ts --type tsxLength of output: 253
I suspect the previous ripgrep flags didn’t include
.tsxfiles properly. Let’s search by glob:
🏁 Script executed:
#!/usr/bin/env bash # 1. Find any code or tests referencing the page‐load action constant rg -l "EXECUTE_PAGE_LOAD_ACTIONS" -g "*.{ts,tsx}" # 2. Look for any hardcoded “execute on page load” text rg -i "execute.*on.*page.*load" -g "*.{ts,tsx}"Length of output: 1941
Constants usage verified for EXECUTE_PAGE_LOAD_ACTIONS
All references to
EXECUTE_PAGE_LOAD_ACTIONSare defined and used consistently—there’s no name mismatch between the constant and its uses in code or Cypress tests. Test failures shouldn’t stem from a missing or renamed action constant.Locations checked:
- app/client/src/ce/constants/ReduxActionConstants.tsx (definition)
- app/client/src/actions/pluginActionActions.ts
- app/client/src/sagas/BatchSagas.tsx
- app/client/src/sagas/ActionExecution/PluginActionSaga.ts
- app/client/src/sagas/EvaluationsSaga.ts
- app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx
- Cypress specs under app/client/cypress/e2e/…
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14639026981. |
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, 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. |
…o feat/reactive-actions-run-behaviour
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14639200519. |
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, 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. |
|
Deploy-Preview-URL: https://ce-40355.dp.appsmith.com |
…o feat/reactive-actions-run-behaviour
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, 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. |
## Description Update type for run behavior for some instances Fixes # ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, 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. |
…o feat/reactive-actions-run-behaviour
…o feat/reactive-actions-run-behaviour
…com/appsmithorg/appsmith into feat/reactive-actions-run-behaviour
…40555) ## Description Updating fixtures to replace executeOnLoad with reactiveActions Fixes [#39833](#39833) ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
…o feat/reactive-actions-run-behaviour
…com/appsmithorg/appsmith into feat/reactive-actions-run-behaviour
…com/appsmithorg/appsmith into feat/reactive-actions-run-behaviour
…com/appsmithorg/appsmith into feat/reactive-actions-run-behaviour
…o feat/reactive-actions-run-behaviour
## Description Fixing partial export spec Fixes [#39833](#39833) ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #40308, #40306, #40309, _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.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14906330551> > Commit: 9817e40 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14906330551&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 08 May 2025 13:52:08 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No --------- Co-authored-by: “sneha122” <“sneha@appsmith.com”> Co-authored-by: nilansh <nilansh@appsmith.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java (1)
821-824: Updated disabled MSSQL test to reference RunBehaviourThough this test is disabled (commented out), it was properly updated to use the new RunBehaviour property instead of executeOnLoad. Consider enabling this test if MSSQL integration is still supported.
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java (2)
893-896: Updated assertions to use enum-based run behavior.The assertions now correctly verify the
RunBehaviourEnumvalues instead of the deprecated boolean flags. Note the presence of unnecessary semicolons on lines 894 and 896.Remove the unnecessary semicolons at the end of lines 894 and 896:
assertThat(tuple.getT1().getRunBehaviour()).isEqualTo(RunBehaviourEnum.ON_PAGE_LOAD); - ; + assertThat(tuple.getT2().getRunBehaviour()).isEqualTo(RunBehaviourEnum.ON_PAGE_LOAD); - ; +
1121-1123: Updated assertions forRunBehaviourEnumin AST logic test.The assertions correctly verify the run behavior using the enum values. Similar to the previous comment, there's an unnecessary semicolon on line 1122.
Remove the unnecessary semicolon at the end of line 1122:
assertThat(tuple.getT1().getRunBehaviour()).isEqualTo(RunBehaviourEnum.ON_PAGE_LOAD); - ; +app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutExecutableUpdateDTO.java (2)
32-45: Well-implemented backward compatibility getter.The custom getter correctly provides fallback logic when runBehaviour is null, ensuring backward compatibility with systems still using executeOnLoad. The mapping logic between boolean and enum values is clear and correct.
However, consider adding a corresponding custom setter to maintain consistency between the fields during the transition period:
+ /** + * Custom setter for runBehaviour that also updates executeOnLoad for backward compatibility + * + * @param runBehaviour The run behavior to set + */ + public void setRunBehaviour(RunBehaviourEnum runBehaviour) { + this.runBehaviour = runBehaviour; + if (runBehaviour != null) { + this.executeOnLoad = RunBehaviourEnum.ON_PAGE_LOAD.equals(runBehaviour); + } + }
15-46: Consider updating class-level documentation.While the implementation is correct, it would be helpful to add class-level documentation mentioning the migration from executeOnLoad to runBehaviour to make the transition intent clear to other developers.
/** * This class would be used to send any action updates that have happened as part of update layout. The client should * consume this structure to update the actions in its local storage (instead of fetching all the page actions afresh). + * + * Note: The boolean executeOnLoad field is being replaced with the enum-based runBehaviour field. + * Clients should migrate to using runBehaviour instead of executeOnLoad. */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (46)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/LayoutExecutableUpdateDTO.java(2 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Executable.java(2 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutExecutableUpdateDTO.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ce/ActionViewCE_DTO.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java(5 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(4 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java(2 hunks)app/server/appsmith-server/src/main/resources/CRUD-DB-Table-Template-Application.json(76 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java(7 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/fork/ApplicationForkingServiceTests.java(3 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java(3 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/imports/internal/ImportServiceTests.java(7 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java(17 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java(5 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java(5 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java(14 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationServiceCETest.java(6 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java(7 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/PartialExportServiceTest.java(3 hunks)app/server/appsmith-server/src/test/resources/com/appsmith/server/git/application.json(120 hunks)app/server/appsmith-server/src/test/resources/com/appsmith/server/git/autocommit/application.json(120 hunks)app/server/appsmith-server/src/test/resources/com/appsmith/server/git/autocommit/autocommit.json(12 hunks)app/server/appsmith-server/src/test/resources/com/appsmith/server/git/ce-automation-test.json(120 hunks)app/server/appsmith-server/src/test/resources/com/appsmith/server/imports/internal/faulty-dsl.json(30 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/building-block.json(6 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/invalid-application-without-pageId-action-collection.json(2 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/partial-export-resource.json(8 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/partial-export-valid-without-widget.json(2 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-custom-themes.json(3 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-page-added.json(16 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-un-configured-datasource.json(2 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-without-action-collection.json(2 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-without-theme.json(3 hunks)app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application.json(4 hunks)
✅ Files skipped from review due to trivial changes (27)
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/partial-export-valid-without-widget.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-custom-themes.json
- app/server/appsmith-server/src/test/java/com/appsmith/server/fork/ApplicationForkingServiceTests.java
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-without-action-collection.json
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java
- app/server/appsmith-server/src/test/java/com/appsmith/server/imports/internal/ImportServiceTests.java
- app/server/appsmith-server/src/test/resources/com/appsmith/server/imports/internal/faulty-dsl.json
- app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java
- app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
- app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java
- app/server/appsmith-server/src/test/resources/com/appsmith/server/git/autocommit/application.json
- app/server/appsmith-server/src/main/resources/CRUD-DB-Table-Template-Application.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/partial-export-resource.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-page-added.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/invalid-application-without-pageId-action-collection.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application.json
- app/server/appsmith-server/src/test/resources/com/appsmith/server/git/application.json
- app/server/appsmith-server/src/test/resources/com/appsmith/server/git/ce-automation-test.json
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-un-configured-datasource.json
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCE.java
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/building-block.json
- app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-without-theme.json
- app/server/appsmith-server/src/test/resources/com/appsmith/server/git/autocommit/autocommit.json
- app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java
🚧 Files skipped from review as they are similar to previous changes (5)
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java
- app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java (1)
LayoutSpanCE(5-30)
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java (1)
Fields(64-91)
🔇 Additional comments (60)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Executable.java (4)
24-25: Appropriate deprecation of the boolean flag method.Good practice to maintain backward compatibility by deprecating rather than removing the method.
27-28: New enum-based method appropriately introduced.The new
getRunBehaviour()method establishes a clearer semantic model than the boolean flag it replaces.
75-75: DTO creation updated to use new run behavior model.Correctly updates the DTO generation to use the new enum-based property.
82-83: Appropriate setter added for the new enum property.Completes the interface contract with the corresponding setter method.
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java (2)
5-5: Required import added for the new enum type.Correctly imports the RunBehaviourEnum class.
182-186: Repository method renamed and updated to use enum-based filtering.The method name change clearly communicates its purpose, and the query logic has been properly updated to filter by the enum value
RunBehaviourEnum.ON_PAGE_LOADinstead of a boolean flag.app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java (6)
12-12: Required import added for the new enum type.Correctly imports the RunBehaviourEnum class.
299-299: Method name updated to reflect the new enum-based behavior model.The method renaming from
updateExecutablesExecuteOnLoadtoupdateExecutablesRunBehaviourclearly indicates the change in approach.
314-317: Condition updated to check for enum value instead of boolean.Correctly uses enum comparison for determining on-page-load executables.
368-369: Update operation changed to use enum value instead of boolean.Correctly sets
RunBehaviourEnum.MANUALinstead of a booleanFALSE.
374-375: Update operation changed to use enum value instead of boolean.Correctly sets
RunBehaviourEnum.ON_PAGE_LOADinstead of a booleanTRUE.
1266-1268: Helper method updated to check enum value instead of boolean.The logic has been properly updated to check if the run behavior is not
ON_PAGE_LOADrather than checking for a false boolean value.app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java (2)
18-18: Import added for RunBehaviourEnum.The addition of this import supports the migration from boolean flags to enum-based run behavior.
456-458: Migrated from executeOnLoad to runBehaviour.This change correctly implements the migration from boolean-based execution control to the more descriptive enum-based approach. The previous
setExecuteOnLoad(actionDTO.getId(), true)is nowsetRunBehaviour(actionDTO.getId(), RunBehaviourEnum.ON_PAGE_LOAD).app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/PartialExportServiceTest.java (3)
12-12: Import added for RunBehaviourEnum.Added import to support the migration from boolean flags to enum-based run behavior in test cases.
301-301: Updated test case to use RunBehaviourEnum.Test case correctly updated to use the new enum-based approach instead of the deprecated boolean flag.
361-361: Updated test case to use RunBehaviourEnum.Similar to the previous change, this test case has been correctly updated to use the new enum-based approach.
app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java (3)
57-57: Updated import for new tracing span constant.Changed from using the deprecated
UPDATE_EXECUTABLES_EXECUTE_ONLOADconstant to the newUPDATE_EXECUTABLES_RUN_BEHAVIOURto align with the new terminology.
200-201: Updated comment to reference runBehaviour.Comment has been updated to use the new terminology, maintaining documentation clarity during the migration.
203-206: Updated method call and tracing span name.Method call properly changed from
updateExecutablesExecuteOnLoadtoupdateExecutablesRunBehaviour, and the corresponding tracing span name updated to match. This ensures consistent terminology throughout the code and tracing data.app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationServiceCETest.java (4)
14-14: Added import for RunBehaviourEnum.The import is correctly added to support the migration from boolean
executeOnLoadto the more expressive enum-based approach.
1947-1947: Replaced executeOnLoad boolean with RunBehaviour enum.This change properly updates the test to use the new enum-based pattern, making the code more expressive and maintainable.
3640-3640: Updated action configuration to use RunBehaviour enum.Correctly replaced the boolean flag with the appropriate enum value, consistent with the application-wide refactoring.
4640-4640: Migrated test action to use new RunBehaviour enum.Properly updated test case to use the new enum pattern instead of the deprecated boolean flag.
app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java (4)
10-10: Added import for RunBehaviourEnumThe import for RunBehaviourEnum is properly added to support the migration from boolean executeOnLoad to enum-based run behavior.
597-597: Properly migrated action configuration to use RunBehaviourChanged from setting executeOnLoad boolean to using RunBehaviourEnum.ON_PAGE_LOAD, maintaining the same functionality while using the new enum-based approach.
740-742: Updated assertion to verify RunBehaviour cloningTests now properly verify that the RunBehaviour property is correctly cloned along with other action properties.
969-980: Correct validation of RunBehaviour cloning in Git contextThis assertion properly verifies that the RunBehaviour enum value is maintained during Git-based page cloning operations, ensuring the migration from boolean to enum doesn't affect Git functionality.
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java (6)
16-16: Added import for RunBehaviourEnumNecessary import added to support the migration from boolean executeOnLoad to enum-based run behavior.
683-687: Updated MySQL assertions to use RunBehaviour enumTests now correctly verify that SELECT queries use ON_PAGE_LOAD behavior while other actions use MANUAL behavior, maintaining the same validation logic with the new enum-based approach.
763-767: Updated Redshift assertions to use RunBehaviour enumSimilar to the MySQL assertions, tests for Redshift now properly validate the enum-based run behavior settings.
891-896: Updated Snowflake assertions to use RunBehaviour enumTests now correctly verify the run behavior for Snowflake datasource actions.
1058-1063: Updated Google Sheets assertions to use RunBehaviour enumTests for Google Sheets actions now properly check for ON_PAGE_LOAD or MANUAL behavior using the new enum approach.
1138-1143: Updated MongoDB assertions to use RunBehaviour enumTests now validate that the FIND action runs ON_PAGE_LOAD while other MongoDB actions run MANUALLY, maintaining the same test coverage with the migrated enum approach.
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java (3)
10-10: Added import for newRunBehaviourEnumtype.The inclusion of this import supports the migration from the boolean
executeOnLoadflag to the more expressive enum-based behavior model.
348-348: Replaced boolean flag with enum-based behavior.Good change from
action.setExecuteOnLoad(false)toaction.setRunBehaviour(RunBehaviourEnum.MANUAL), providing more clarity on the action's execution behavior.
1220-1221: Updated assertions for non-AST logic test.Both assertions now correctly use the enum values. This ensures test compatibility with the migration from boolean flags to enum-based run behavior.
app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ce/ActionViewCE_DTO.java (4)
3-3: Added import forRunBehaviourEnum.This import supports the new enum-based run behavior model.
41-44: Deprecated theexecuteOnLoadboolean flag.Good approach to mark the field as deprecated and restrict it to internal JSON views, supporting backward compatibility while encouraging migration to the new API.
45-47: Added newrunBehaviourenum field.This replaces the boolean flag with a more expressive enum-based approach, while maintaining public JSON visibility.
48-56: Added custom getter with fallback logic.Well-implemented backward compatibility. The getter first checks for the new enum value and falls back to interpreting the deprecated boolean flag if needed.
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java (13)
14-14: Added import forRunBehaviourEnum.Necessary import to support the migration to enum-based run behavior in tests.
306-306: Updated to use enum for run behavior in git-connected action test.Correctly replaced
setExecuteOnLoad(true)withsetRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD).
340-340: Updated run behavior for permission test.Appropriately changed to the enum-based approach for setting run behavior.
353-353: Updated assertion to use enum value.The assertion now correctly checks for
RunBehaviourEnum.MANUALinstead of a boolean value.
412-412: Updated run behavior for git-connected app test.Correctly using the enum-based approach for setting run behavior.
423-423: Updated assertion in git-connected app test.The assertion now correctly verifies the enum value
RunBehaviourEnum.MANUAL.
802-803: Updated method name fromsetExecuteOnLoadtosetRunBehaviour.The test now uses the new method that supports the enum-based approach.
1152-1152: Updated run behavior for default context test.Correctly using the enum-based approach in
testExecuteOnLoadParamOnActionCreateWithDefaultContext.
1162-1162: Updated assertion in default context test.The assertion now correctly checks for
RunBehaviourEnum.MANUALinstead of a boolean value.
1176-1176: Updated run behavior for clone page context test.Using the new enum-based approach in the clone page context test case.
1190-1190: Updated assertion in clone page context test.The assertion now correctly verifies that
getRunBehaviour()returnsRunBehaviourEnum.ON_PAGE_LOAD.
1277-1277: Updated run behavior for out-of-range timeout test.Correctly using the enum-based approach in the timeout validation test.
1309-1309: Updated run behavior for valid range timeout test.Consistently using the enum-based approach across all test cases.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/LayoutExecutableUpdateDTO.java (3)
3-3: Added import forRunBehaviourEnum.This import supports the new enum-based run behavior model.
25-27: Deprecated theexecuteOnLoadboolean field.Good approach to mark the field as deprecated while maintaining it for backward compatibility.
33-46: Added custom getter with fallback logic for backward compatibility.Well-implemented getter that provides appropriate fallback logic to handle the transition period. It first checks the newer field, then falls back to interpreting the deprecated boolean if needed.
app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutExecutableUpdateDTO.java (3)
3-3: Import of RunBehaviourEnum is properly added.The import statement correctly adds the required enum class from the external models package.
25-27: Good approach for deprecating the executeOnLoad field.The field is properly marked as deprecated and its visibility is restricted to internal views only, which is the correct approach when phasing out a field while maintaining backward compatibility.
29-30: New runBehaviour field with public view is correctly implemented.The new field using the enum type is properly exposed with a public JSON view, making it available for client consumption.
.../appsmith-interfaces/src/main/java/com/appsmith/external/dtos/LayoutExecutableUpdateDTO.java
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14909767364. |
|
Deploy-Preview-URL: https://ce-40355.dp.appsmith.com |
Failed server tests
|
## Description This PR provides backward compatibility for older actions which do not have runBehaviour property in the DB. When such actions have been configured to be run on page load explicitly, they will get executed as other actions which get set to be run on page load when binding happens. ### Steps to reproduce: - Create app on production ([app.appsmith.com](http://app.appsmith.com/)) - Create mock db users datasource - Create 3 queries, Q1, Q2 and Q3 - Attach Q1 to table so it runs on page load - Change executeOnLoad setting of Q3 to page load - Export this app - Import this on our DP - Observe that Q3 does not run on page load Fixes #40633 _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.Datasource, @tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14979473254> > Commit: 5009eb4 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14979473254&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Datasource, @tag.Sanity` > Spec: > <hr>Tue, 13 May 2025 03:57:32 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No --------- Co-authored-by: “sneha122” <“sneha@appsmith.com”>
Description
Replacing execute on load toggle with run behaviour in all actions / jsactions to remove boolean values and instead use an enum.
Fixes #40311 #40309 #40305 #39831 #39830 #39833 #40306 #40307 #40308
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/14988089906
Commit: 7cd7438
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 13 May 2025 05:28:17 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Style
Tests
Chores