-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Simpler implementation for empty instance check #34682
Conversation
Warning Review failedThe pull request is closed. WalkthroughThe recent changes introduce and utilize a new method, Changes
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 (
|
Failed server tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/bridge/Bridge.java (1 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/bridge/BridgeQuery.java (1 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java (2 hunks)
Additional comments not posted (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java (1)
41-43
: Ensure correct usage ofBridge.notIn
.The updated implementation uses
Bridge.notIn
to filter out system-generated emails. This approach simplifies the logic and improves readability. Ensure thatgetSystemGeneratedUserEmails()
returns the correct set of emails to be excluded.app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/bridge/Bridge.java (1)
95-98
: New methodnotIn
looks good.The
notIn
method is a useful addition for filtering out items not in a given collection. It is consistent with other methods in theBridge
class.app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/bridge/BridgeQuery.java (1)
80-83
: New methodnotIn
looks good.The
notIn
method correctly adds a criteria check to exclude items in the specified collection. It is consistent with other methods in theBridgeQuery
class.
…4682) Refactored the implementation of `isUsersEmpty` so that it works the same on `release` and on `pg` branches. **/test sanity** <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9797721649> > Commit: f0ed5ee > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9797721649&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Enhanced user filtering to improve the accuracy of user-related queries. This update ensures more precise results by excluding system-generated emails from user checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Refactored the implementation of
isUsersEmpty
so that it works the same onrelease
and onpg
branches./test sanity
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9797721649
Commit: f0ed5ee
Cypress dashboard.
Tags:
@tag.Sanity
Summary by CodeRabbit