coderabbitai[bot]
commented
23 hours ago Walkthrough
The changes in this pull request modify the populateExecuteActionDTO method in the ActionExecutionSolutionCEImpl class. The primary enhancement is the logic to set the datasourceId in the ExecuteActionDTO based on the viewMode. If viewMode is TRUE, the datasourceId is derived from the publishedAction; otherwise, it comes from the unpublishedAction. Additionally, code formatting improvements were made for better clarity, but no new methods or signatures were introduced.
Changes
| Files | Change Summary |
|---|---|
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java |
Modified populateExecuteActionDTO to set datasourceId based on viewMode; improved code formatting. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Set datasourceId to executeActionDTO when an action is executed (#37627) | ✅ |
Possibly related PRs
- #34691: Modifies logic for handling datasource validation and assignment, related to
datasourceIdsetting. - #34754: Addresses validation for datasource in action import and save flow, relevant to datasource handling.
- #34789: Fixes issues with queries not fetching data due to incorrect datasource handling, aligning with the main PR's focus.
- #37268: Removes an unused method in the
ActionExecuteDTO, relevant to the class where the main PR's changes occur.
Suggested reviewers
- mohanarpit
- nidhi-nair
- sondermanish
🎉 In the code's dance, a new step we see,
WithdatasourceId, it flows so free!
InviewMode's light, it finds its way,
Clarity shines, come what may!
Let's celebrate this code's fine art,
A leap forward, a brand new start! 🎊
📜 Recent review details
**Configuration used: .coderabbit.yaml** **Review profile: CHILL**📥 Commits
Reviewing files that changed from the base of the PR and between 89f9b591ec58679ab07b669a6d55e20942d5f641 and 458b5c0d90fa138c57ed7f1dd491d985bfe0b5df.📒 Files selected for processing (1)
* `app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java` (1 hunks)🔇 Additional comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java (2)
`255-272`: **LGTM! The datasourceId assignment logic is correct.** The conditional logic properly sets the datasourceId from either publishedAction or unpublishedAction based on the viewMode, which aligns with the PR objective. --- `255-272`: **Consider handling null datasource case.** While the logic is correct, it might be good to add a null check for the datasource object before accessing its ID. Let's verify if there are cases where datasource could be null: Consider adding null checks: ```diff if (TRUE.equals(executeActionDTO.getViewMode())) { + if (newAction.getPublishedAction().getDatasource() != null) { populatedExecuteActionDTO.setDatasourceId( newAction.getPublishedAction().getDatasource().getId()); + } } else { + if (newAction.getUnpublishedAction().getDatasource() != null) { populatedExecuteActionDTO.setDatasourceId( newAction.getUnpublishedAction().getDatasource().getId()); + } } ```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?
github-actions[bot]
Description
Fixes https://github.com/appsmithorg/appsmith/issues/37627
Automation
/ok-to-test tags="@tag.Sanity"
:mag: Cypress test results
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
datasourceIdbased on the action's context, improving action execution accuracy.Refactor