-
Notifications
You must be signed in to change notification settings - Fork 8
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
BugFix #8182 Refactored funtionality for receiving total number of orders for employees. #1616
Conversation
…rders by list of tariffs in the BigOrderTableRepository; Added tests for getOrdersCountByTariffs method in the BigOrderTableRepositoryTest; Added method getTotalNumberOfOrdersByEmployee in the BigOrderTableServiceView and created realisation for it in the BigOrderTableViewServiceImpl; Added unit tests for method getTotalNumberOfOrdersByEmployee into BigOrderTableViewServiceImplTest; Renamed class BigOrderTableServiceImplTest as BigOrderTableViewServiceImplTest; Removed old realisation of receiving total number of orders from UBSManagementService, UBSManagementServiceImpl and UBSManagementServiceImplTest; Refactored method getOrdersCount in the ManagementOrderController according to new requerements; Refactored tests in the ManagementOrderControllerTest according to changes in the controller;
WalkthroughThis pull request refactors the order counting functionality to focus on employee-specific data. The changes update the endpoint in the management controller to receive a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ManagementOrderController
participant Service as BigOrderTableViewServiceImpl
participant Repo as BigOrderTableRepository
Client->>Controller: GET /orders/count (with Principal)
Controller->>Service: getTotalNumberOfOrdersByEmployee(principal.getName())
Service->>Repo: getOrdersCountByTariffs(tariffIds)
Repo-->>Service: Order count result
Service-->>Controller: OrderCountDto
Controller-->>Client: OrderCountDto (or 404 if not found)
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
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
|
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
🧹 Nitpick comments (1)
core/src/test/java/greencity/repository/BigOrderTableRepositoryTest.java (1)
490-497
: Good test implementation for the new repository method.The test for
getOrdersCountByTariffs
verifies that the method returns a positive result when given a list of tariff IDs. This ensures the basic functionality works correctly.Consider enhancing the test with additional assertions to verify specific count values for known test data rather than just checking for a positive result. This would provide more thorough validation of the method's behavior.
@Test void getOrdersCountByTariffsTest() { List<Long> tariffsInfoIds = List.of(1L, 2L, 3L); + // Expected count based on test data setup + long expectedCount = ModelUtils.getAllBOTViewsASC().size(); long result = bigOrderTableRepository.getOrdersCountByTariffs(tariffsInfoIds); - assertTrue(result > 0); + assertEquals(expectedCount, result, "Order count should match the number of test orders"); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
core/src/main/java/greencity/controller/ManagementOrderController.java
(1 hunks)core/src/test/java/greencity/controller/ManagementOrderControllerTest.java
(1 hunks)core/src/test/java/greencity/repository/BigOrderTableRepositoryTest.java
(6 hunks)dao/src/main/java/greencity/repository/BigOrderTableRepository.java
(1 hunks)service-api/src/main/java/greencity/service/ubs/UBSManagementService.java
(0 hunks)service-api/src/main/java/greencity/service/ubs/manager/BigOrderTableServiceView.java
(2 hunks)service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java
(0 hunks)service/src/main/java/greencity/service/ubs/manager/BigOrderTableViewServiceImpl.java
(2 hunks)service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java
(0 hunks)service/src/test/java/greencity/service/ubs/manager/BigOrderTableViewServiceImplTest.java
(6 hunks)
💤 Files with no reviewable changes (3)
- service-api/src/main/java/greencity/service/ubs/UBSManagementService.java
- service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java
- service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java
🔇 Additional comments (17)
service-api/src/main/java/greencity/service/ubs/manager/BigOrderTableServiceView.java (2)
4-4
: Appropriate import added.The import for OrderCountDto has been correctly added to support the newly introduced method that returns this type.
45-53
: Well-documented method with clear purpose.The new method for retrieving total orders by employee email is well-documented with proper Javadoc format, including parameter and return value descriptions, and author attribution.
core/src/test/java/greencity/controller/ManagementOrderControllerTest.java (1)
217-217
: Test properly updated to reflect service change.The test verification has been correctly updated to call the new
getTotalNumberOfOrdersByEmployee
method with the principal's name, aligning with the implementation change in the corresponding controller.service/src/main/java/greencity/service/ubs/manager/BigOrderTableViewServiceImpl.java (2)
7-7
: Necessary import added.The import for OrderCountDto has been correctly added to support the implementation of the new method.
108-115
:✅ Verification successful
Implementation follows established patterns.
The implementation of
getTotalNumberOfOrdersByEmployee
follows the same pattern as other methods in this class, appropriately handling employee lookup and exception handling. The code is clean and focused on its specific responsibility.Consider adding unit tests to verify the behavior with both valid and invalid employee emails.
Let's check if tests have been implemented for this method:
🏁 Script executed:
#!/bin/bash # Check for test coverage of the new method rg -A 3 "getTotalNumberOfOrdersByEmployee.*Test" --glob "*.java"Length of output: 1265
Implementation and Test Coverage Verified
File:
service/src/main/java/greencity/service/ubs/manager/BigOrderTableViewServiceImpl.java
(Lines 108–115)The implementation of
getTotalNumberOfOrdersByEmployee
adheres to established patterns with appropriate employee lookup and exception handling. Additionally, our verification confirmed that unit tests for both valid (getTotalNumberOfOrdersByEmployeeWithValidEmailTest
) and invalid (getTotalNumberOfOrdersByEmployeeWithNotValidEmailTest
) email scenarios are present inservice/src/test/java/greencity/service/ubs/manager/BigOrderTableViewServiceImplTest.java
.Keep maintaining this robust coverage as the code evolves.
core/src/test/java/greencity/repository/BigOrderTableRepositoryTest.java (1)
32-32
: Good practice using static imports for assertions.Using static imports for JUnit assertions improves test readability by reducing verbosity.
dao/src/main/java/greencity/repository/BigOrderTableRepository.java (1)
94-109
: Well-implemented repository method for counting orders by tariffs.The new
getOrdersCountByTariffs
method follows good practices with clear documentation and an efficient implementation that leverages the existing predicate methods. The method signature is clear about its purpose and return type.core/src/main/java/greencity/controller/ManagementOrderController.java (3)
584-586
: Documentation updated to reflect employee-specific orders count.The JavaDoc comment has been properly updated to describe the more specific functionality of retrieving order counts by employee rather than a general order count.
590-595
: API documentation updated with additional error response.Good addition of the 404 Not Found response code to the API documentation, which is appropriate since the method now depends on finding an employee by email.
598-601
: Method implementation correctly refactored to use employee-specific service.The implementation now properly accepts a Principal parameter and calls the new service method, aligning with the refactored functionality for employee-specific order counts.
service/src/test/java/greencity/service/ubs/manager/BigOrderTableViewServiceImplTest.java (7)
41-45
: Class renamed and constants added for better maintainability.Good refactoring: renaming the class to match the service implementation name and extracting email strings into constants improves code readability and maintainability.
32-36
: Improved static imports for cleaner test code.Adding static imports for error messages and for commonly used assertion and argument matcher methods makes the test code more concise and readable.
107-109
: Simplified assertion syntax.Updated to use direct
assertThrows
instead ofAssertions.assertThrows
for more concise code.
173-176
: Consistent assertion syntax throughout the tests.Simplified assertThrows syntax applied consistently, improving readability.
191-194
: Consistent assertion style.The assertion pattern has been updated consistently throughout the test class.
201-214
: Well-structured test for the new service method with valid email.This test thoroughly verifies that:
- The repository method is called with the correct tariffs list
- The service correctly finds the employee by email
- The service retrieves the tariffs associated with the employee
- All expected interactions occur exactly once
Good test coverage for the happy path scenario.
216-227
: Comprehensive test for error handling with invalid email.This test properly verifies that:
- An EntityNotFoundException is thrown when an employee can't be found by email
- The subsequent repository methods are not called when the exception occurs
- The verification confirms the exact number of method calls
Well-structured test for the error scenario.
|
Added method getOrdersCountByTariffs for receiving total numbers of orders by list of tariffs in the BigOrderTableRepository; (#1616) Added tests for getOrdersCountByTariffs method in the BigOrderTableRepositoryTest; Added method getTotalNumberOfOrdersByEmployee in the BigOrderTableServiceView and created realisation for it in the BigOrderTableViewServiceImpl; Added unit tests for method getTotalNumberOfOrdersByEmployee into BigOrderTableViewServiceImplTest; Renamed class BigOrderTableServiceImplTest as BigOrderTableViewServiceImplTest; Removed old realisation of receiving total number of orders from UBSManagementService, UBSManagementServiceImpl and UBSManagementServiceImplTest; Refactored method getOrdersCount in the ManagementOrderController according to new requerements; Refactored tests in the ManagementOrderControllerTest according to changes in the controller;
GreenCityUBS PR
Issue Link 📋
#8182
Changed
BigOrderTableRepository
;BigOrderTableRepositoryTest
;BigOrderTableServiceView
and created realization for it in theBigOrderTableViewServiceImpl
;BigOrderTableViewServiceImplTest
;BigOrderTableServiceImplTest
asBigOrderTableViewServiceImplTest
;UBSManagementService
,UBSManagementServiceImpl
, andUBSManagementServiceImplTest
;ManagementOrderController
according to new requirements;ManagementOrderControllerTest
according to changes in the controller;Summary Of Changes 🔥
Modified business logic for receiving total numbers of orders.
Now it returns the total numbers of orders for current employees according to requirements in the task #8182
Summary by CodeRabbit
New Features
Refactor