feat: disable create js object option in workflows editor #36094
Conversation
WalkthroughThe changes made to the codebase focus on refining the handling of JavaScript objects within workflows. A conditional check was introduced to prevent the addition of a "create new JS object" operation when in the workflow editor. Additionally, modifications to the Changes
Assessment against linked issues
Poem
Tip We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
github-actions
bot
added
Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Files/index.tsx (3 hunks)
Additional comments not posted (5)
app/client/src/pages/Editor/Explorer/Files/index.tsx (3)
70-70: Good work simplifying theonCreatefunction's dependencies!Removing the
dispatchdependency from theonCreatefunction's dependency array is a positive change. It simplifies the function's behavior and execution context.
144-144: Nice job enhancing the component's responsiveness to parent entity type changes!Including
parentEntityTypein the callback's dependency array is a good decision. It allows the component to respond appropriately to changes in the parent entity's type, improving its adaptability to different contexts.
164-164: Excellent work making thehandleClickfunction responsive to parent entity type!Adding
parentEntityTypeto thehandleClickfunction's dependency array is a smart move. It allows the function to adapt its behavior based on the parent entity's type, potentially altering the flow of actions triggered by user interactions in a context-sensitive manner.app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.tsx (2)
42-42: Import statement looks good!The import statement for
urlBuilderandEDITOR_TYPEis necessary to support the new conditional logic. Well done!
131-137: Conditional block to disable "create new JS object" option in workflow editor looks good!The new conditional block effectively addresses the current limitation where workflows do not support multiple JS objects. By checking if the user is in the workflow editor using
urlBuilder.getDefaultEditorType()and only adding the "create new JS object" operation when not in the workflow editor, it prevents users from encountering errors when attempting to create additional JS objects.The TODO comment serves as a helpful reminder to revisit this logic once workflows gain support for multiple JS objects. At that point, the conditional block can be removed to re-enable the "create new JS object" option in the workflow editor.
Great work on implementing this temporary workaround while clearly documenting the future enhancement!
ayushpahwa
changed the title
github-actions
bot
added
Enhancement
github-actions
bot
added
Bug
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
app/client/src/ce/utils/workflowHelpers.ts (1)
5-10: Excellent work on thecheckIfJSObjectCreationAllowedfunction!The function is correctly returning
falseto hide the option to create a new JavaScript object in the workflow editor. This is a good temporary solution until the workflow runner supports multiple JavaScript objects.A few suggestions to consider:
- Consider adding a more descriptive comment explaining the rationale behind this function. For example:
// The current workflow runner does not support multiple JavaScript objects. // Therefore, we hide the option to create a new JavaScript object in the workflow editor. // TODO: Remove this function once the workflow runner supports multiple JavaScript objects.
- If possible, consider adding a feature flag to control this behavior. This will make it easier to enable/disable this feature in the future without modifying the code.
Overall, great job on implementing this function! Let me know if you have any questions or need further assistance.
app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.test.ts (1)
335-351: Consider removing this test case.This test case is identical to the previous one, where
disableJSObjectCreationis set tofalse. It verifies the same behavior and does not add any additional coverage.To maintain a clean and concise test suite, it's best to remove redundant test cases. The behavior when
disableJSObjectCreationis not explicitly set is already covered by the test case where it is set tofalse.Apply this diff to remove the redundant test case:
- it("should show new js object option if disableJSObjectCreation is not set", () => { - const fileOptions = useFilteredAndSortedFileOperations({ - query: "new js", - allDatasources: [], - recentlyUsedDSMap: {}, - canCreateActions: true, - canCreateDatasource: true, - disableJSObjectCreation: false, - }); - - expect(fileOptions.length).toEqual(2); - expect(fileOptions[0]).toEqual( - expect.objectContaining({ - title: "New JS Object", - }), - ); - });
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- app/client/src/ce/utils/workflowHelpers.ts (1 hunks)
- app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.test.ts (1 hunks)
- app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.tsx
Additional comments not posted (3)
app/client/src/ce/utils/workflowHelpers.ts (1)
1-2: Great job! TheuseWorkflowOptionsfunction looks good.The function is correctly returning an empty array as expected. Keep up the good work!
app/client/src/components/editorComponents/GlobalSearch/GlobalSearchHooks.test.ts (2)
299-315: Great job on this test case!This test case effectively verifies that the "New JS Object" option is not shown when
disableJSObjectCreationis set totrue. It checks both the length of the returned array and the presence of the "New datasource" option.Keep up the good work in ensuring the correct behavior of the
useFilteredAndSortedFileOperationsfunction under different conditions.
317-333: Excellent test case!This test case complements the previous one by verifying that the "New JS Object" option is shown when
disableJSObjectCreationis set tofalse. It checks both the length of the returned array and the presence of the "New JS Object" option.Your test cases are covering the different scenarios effectively, ensuring the robustness of the
useFilteredAndSortedFileOperationsfunction.
ayushpahwa
deleted the
fix/32239-workflows-disable-multiple-js-objects
branch
…g#36094) ## Description Workflows is not currently supporting multiple js objects. Hence, we need to disable the option for the workflows editor. EE branch: https://github.com/appsmithorg/appsmith-ee/pull/5027 Fixes appsmithorg#32239 ## Automation /test js ### 🔍 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/10686636668> > Commit: 5cb101f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10686636668&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.JS` > Spec: > <hr>Tue, 03 Sep 2024 16:45:16 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced logic for creating new JavaScript objects, preventing unnecessary additions in the workflow editor. - Introduced a function to determine if creating a new JavaScript object is allowed, improving user experience in the workflow editor. - Improved responsiveness of the Files component based on the parent entity type, allowing for context-sensitive behavior. - **Bug Fixes** - Addressed limitations in workflow runner support for multiple JavaScript objects. - **Refactor** - Simplified the `onCreate` function's dependencies for improved clarity and functionality. - **Tests** - Added new test cases to validate the behavior of JavaScript object creation options based on user settings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Workflows is not currently supporting multiple js objects. Hence, we need to disable the option for the workflows editor.
EE branch: https://github.com/appsmithorg/appsmith-ee/pull/5027
Fixes #32239
Automation
/test js
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10686636668
Commit: 5cb101f
Cypress dashboard.
Tags:
@tag.JSSpec:
Tue, 03 Sep 2024 16:45:16 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
onCreatefunction's dependencies for improved clarity and functionality.Tests