feat:sort apps in a workspace in alphabetical order #36312
Conversation
WalkthroughThe changes introduce a new method, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Service
User->>Controller: Request applications by workspace ID
Controller->>Service: findByWorkspaceIdAndBaseApplicationsInAlphabeticalOrder(workspaceId)
Service->>Service: Validate workspace ID
Service->>Service: Fetch applications
Service->>Service: Sort applications alphabetically
Service-->>Controller: Return sorted applications
Controller-->>User: Display sorted applications
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (2)
Additional comments not posted (3)
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
|
![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
Outside diff range and nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java (1)
130-130: Great work implementing the alphabetical sorting! Just one small suggestion.The change correctly updates the service method call to fetch applications in alphabetical order, which aligns perfectly with the PR objective.
However, since the sorting order is changed from recently used to alphabetical, consider renaming this controller method to accurately reflect its new behavior. For example:
- findByWorkspaceIdAndRecentlyUsedOrder + findByWorkspaceIdAndAlphabeticalOrderThis will improve code readability and maintainability by ensuring the method name matches its functionality.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java (1 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (2 hunks)
- app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java (1 hunks)
- app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationServiceCETest.java (1 hunks)
Additional comments not posted (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java (1)
32-32: Great work on adding the new method! 👍The
findByWorkspaceIdAndBaseApplicationsInAlphabeticalOrdermethod is a valuable addition to theApplicationServiceCEinterface. It complements the existingfindByWorkspaceIdAndBaseApplicationsInRecentlyUsedOrdermethod by providing an alternative sorting option based on the application names.This enhancement improves the application's ability to manage and display applications in a user-friendly manner, allowing for better organization and accessibility within a workspace. The method signature is clear, follows the naming convention, and effectively communicates its purpose.
Keep up the excellent work in improving the user experience! 🌟
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (2)
60-60: Good job importing the Comparator class!The
Comparatorclass is necessary for sorting applications alphabetically by name in the newfindByWorkspaceIdAndBaseApplicationsInAlphabeticalOrdermethod.
215-263: Excellent work implementing the new method to retrieve applications in alphabetical order!The
findByWorkspaceIdAndBaseApplicationsInAlphabeticalOrdermethod is well-structured and follows a similar pattern to the existingfindByWorkspaceIdAndBaseApplicationsInRecentlyUsedOrdermethod. Here are the key points:
- Error handling for invalid
workspaceIdis implemented correctly by returning aFluxerror.- The filtering logic for Git-connected applications ensures that only applications that are not connected to Git or are connected but revert to the default branch are included in the results.
- The alphabetical sorting using
Comparator.comparingwith a case-insensitive comparator is implemented correctly.Overall, the method implementation looks solid and should work as expected. Great job!
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationServiceCETest.java (1)
4523-4566: Excellent work!The new test method
findByWorkspaceIdAndBaseApplicationsInAlphabeticalOrder_applicationPresentInWorkspace_orderedAlphabeticallyis well-written and thoroughly tests the functionality of fetching applications in alphabetical order. It creates dummy applications, invokes the service method, and verifies if the returned applications are ordered alphabetically by name.The test follows good practices by:
- Creating a workspace and dummy applications for testing.
- Mocking the user data service to return empty recently used application IDs.
- Invoking the service method to fetch applications in alphabetical order.
- Asserting that the returned applications are ordered alphabetically by name.
- Cleaning up the created resources after the test.
Great job in adding this comprehensive test case! It enhances the test coverage and ensures the correctness of the alphabetical ordering functionality.
|
@appsmithorg/query-js-pod , Please add reviewer for this pr if you have enough bandwidth. |
rishabhrathod01
added
IDE Product
rishabhrathod01
requested review from
hetunandu,
albinAppsmith and
rohan-arthur
| return !GitUtils.isApplicationConnectedToGit(application) | ||
| || GitUtils.isDefaultBranchedApplication(application); | ||
| }) | ||
| .sort(Comparator.comparing( |
There was a problem hiding this comment.
Instead of sorting in application memory, we should perform this operation during the database query. In this scenario, we should query MongoDB to get the applications for the workspace in a sorted manner. This is because it's much faster to perform this operation in a database rather than in application memory. Also, moving such ops to the DB ensures long-term stability of this function.
There was a problem hiding this comment.
Thank you for the suggestion! I've updated the code to perform sorting at the MongoDB query level. Please let me know if further adjustments are needed.
hetunandu
requested review from
rajatagrawal,
ankitakinger and
vsvamsi1
and removed request for
hetunandu,
albinAppsmith and
rohan-arthur
…nto feat/sort_apps_in_a_workspace_in_alphabetical_order
|
@MajaharZemoso thanks for your interest but we have received another contribution for the same issue, and decided to go ahead with the other PR as the other one was raised earlier. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
Issue
Describtion
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Tests