fix: Update datasource deletion message for clarity#34822
Conversation
WalkthroughThe changes involve updating error messages related to datasource deletion in both client-side and server-side code. Specifically, the term "action(s)" is replaced with "query" in error messages to make them more user-friendly. The updates span across Cypress tests, server-side service implementations, and test cases. Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ServerSide/QueryPane/MySQL_Spec.ts (1 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java (1 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ServerSide/QueryPane/MySQL_Spec.ts (1)
Pattern
app/client/cypress/**/**.*: Follow best practices for Cypress code and e2e automation.
Avoid using cy.wait in code.
Avoid using cy.pause in code.
Use variables for locators, not strings.
Usedata-*attributes for selectors; avoid Xpaths and CSS attributes.
Avoid selectors like.btn.submitorbutton[type=submit].
Perform logins via API withLoginFromAPI.
Only interact with controlled sites/servers.
Ensure tests can run withit.onlyand are independent.
Usebefore,beforeEach,after,afterEachcorrectly; clean state before tests.
Check new specs for flakiness by running them 10 times on CI.
Use multiple assertions; don't treat Cypress as unit tests.
Use constants for strings.
Include datasource operations inbeforehooks.
Additional comments not posted (3)
app/client/cypress/e2e/Regression/ServerSide/QueryPane/MySQL_Spec.ts (1)
94-94: LGTM! The updated error message improves clarity.The change from "action(s)" to "query" or "queries" makes the error message more user-friendly.
app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java (1)
505-505: LGTM! The updated error message improves clarity and provides intelligent error messaging.The change from "action(s)" to "query" or "queries" based on the count makes the error message more user-friendly.
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java (1)
809-811: LGTM! The updated error message improves clarity and provides intelligent error messaging.The change from "action(s)" to "query" or "queries" based on the count makes the error message more user-friendly.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9886831097. |
|
Deploy-Preview-URL: https://ce-34822.dp.appsmith.com |
|
@AnnaHariprasad5123 thanks for your contribution! We might need to add the second parameter "query" also in the error message here |
|
Other than this failing test, rest changes LGTM. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java (1 hunks)
Additional comments not posted (1)
app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java (1)
950-951: LGTM! Ensure consistency with the new error message format.The test method correctly verifies the updated error message. Ensure that all related error messages in the codebase are also updated for consistency.
|
Hi @NilanshBansal , Could you check once. |
|
@AnnaHariprasad5123 there are a large number of cypress test case failures due to this assertion. Can you change this assertion to "Cannot delete datasource" |
e0d3d53 to
766dec4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/support/Pages/DataSources.ts (1 hunks)
- app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/cypress/support/Pages/DataSources.ts
Files skipped from review as they are similar to previous changes (1)
- app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java
Done |
|
The testing on the following has been done. The PR is ready to be merged!
|
Fixes #34604
Hi @nidhi-nair ,
Screenshots :



Earlier :
After update the error message :
Summary by CodeRabbit
Bug Fixes
Tests
Documentation