Added new endpoint to retrieve address for order by order id #1608
Conversation
WalkthroughThis pull request introduces new functionality for managing address-related operations. It adds a GET endpoint to the security configuration and the AddressController, allowing users with specific roles to retrieve order addresses. Additionally, a PATCH endpoint for updating addresses is introduced. The changes include the creation of a new AddressService interface and its implementation, a mapper for converting OrderAddress entities to DTOs, and corresponding test cases to ensure the reliability of these additions. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant AC as AddressController
participant AS as AddressServiceImpl
participant OR as OrderAddressRepository
participant MM as ModelMapper
C->>AC: GET /get-address-for-order/{orderId}
AC->>AS: getAddressForOrder(orderId)
AS->>OR: findByOrderId(orderId)
OR-->>AS: OrderAddress (or none)
alt Address found
AS->>MM: map(OrderAddress to UpdateAddressDto)
MM-->>AS: UpdateAddressDto
AS-->>AC: UpdateAddressDto (with orderId set)
AC-->>C: 200 OK with UpdateAddressDto
else Address not found
AS-->>AC: throw NotFoundException
AC-->>C: Exception Response
end
Possibly related PRs
Suggested reviewers
Poem
✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
service/src/main/java/greencity/service/ubs/AddressServiceImpl.java (1)
21-28: Consider adding transaction management.The implementation looks good with proper error handling. Consider adding
@Transactional(readOnly = true)annotation since this is a read-only operation.@Override + @Transactional(readOnly = true) public UpdateAddressDto getAddressForOrder(Long orderId) {service/src/main/java/greencity/mapping/location/OrderAddressToUpdateAddressDto.java (1)
12-31: Consider adding null safety checks.The mapping logic is comprehensive, but consider adding null safety checks for optional fields to prevent potential NullPointerExceptions.
protected UpdateAddressDto convert(OrderAddress source) { + if (source == null) { + return null; + } return UpdateAddressDto.builder()service/src/test/java/greencity/mapping/location/OrderAddressToUpdateAddressDtoTest.java (1)
16-35: Add tests for edge cases.While the happy path is well tested, consider adding tests for:
- Null input handling
- Empty/blank string fields
- Special characters in address fields
Example test case for null handling:
@Test void convertNullTest() { var result = mapper.convert(null); assertNull(result); }service/src/test/java/greencity/service/ubs/AddressServiceTest.java (1)
46-55: Consider adding more edge cases.While the negative test case is good, consider adding tests for:
- Null orderId
- Zero orderId
- Very large orderId values
core/src/main/java/greencity/controller/AddressController.java (1)
229-245: Enhance API documentation for better clarity.The documentation could be improved by:
- Adding possible error scenarios in the description
- Documenting the response structure
- Including example values
Example documentation:
/** * Retrieves the address for an order with the given id. * * @param orderId The id of the order * @return The address details for the order * @throws NotFoundException if the order or address is not found * @response.example { * "id": 1, * "street": "Main St", * "city": "Example City" * } */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/src/main/java/greencity/configuration/SecurityConfig.java(1 hunks)core/src/main/java/greencity/controller/AddressController.java(4 hunks)core/src/test/java/greencity/controller/AddressControllerTest.java(3 hunks)service-api/src/main/java/greencity/service/ubs/AddressService.java(1 hunks)service/src/main/java/greencity/mapping/location/OrderAddressToUpdateAddressDto.java(1 hunks)service/src/main/java/greencity/service/ubs/AddressServiceImpl.java(1 hunks)service/src/test/java/greencity/ModelUtils.java(1 hunks)service/src/test/java/greencity/mapping/location/OrderAddressToUpdateAddressDtoTest.java(1 hunks)service/src/test/java/greencity/service/ubs/AddressServiceTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
service-api/src/main/java/greencity/service/ubs/AddressService.java (1)
5-14: Well-structured interface with clear documentation!The interface follows single responsibility principle and includes comprehensive JavaDoc documentation.
service/src/test/java/greencity/service/ubs/AddressServiceTest.java (1)
30-44: Good test coverage for successful address retrieval.The test effectively verifies:
- Correct repository method invocation
- Proper mapping of the result
- Response validation
core/src/main/java/greencity/configuration/SecurityConfig.java (1)
150-152: Security configuration looks good.The endpoint is properly secured with appropriate role-based access control (ADMIN and UBS_EMPLOYEE roles).
service/src/test/java/greencity/ModelUtils.java (1)
5856-5872: Well-structured test data method!The new
getOrderAddress1()method is a good addition to the test utilities. It provides a complete test OrderAddress object with both Ukrainian and English translations, which will be valuable for testing address-related functionality.
| @Test | ||
| void getAddressForOrderTest() throws Exception { | ||
| mockMvc.perform(get(ubsLink + "/get-address-for-order/{id}", 1L) | ||
| .contentType(MediaType.APPLICATION_JSON)) | ||
| .andExpect(status().isOk()); | ||
|
|
||
| verify(addressService).getAddressForOrder(1L); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage for the getAddressForOrder endpoint.
While the basic success case is covered, consider adding tests for:
- Invalid order ID
- Not found scenario
- Response body validation
Example test for error scenario:
@Test
void getAddressForOrderNotFoundTest() throws Exception {
when(addressService.getAddressForOrder(-1L))
.thenThrow(new NotFoundException("Address not found"));
mockMvc.perform(get(ubsLink + "/get-address-for-order/{id}", -1L)
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isNotFound());
}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/main/java/greencity/controller/AddressController.java (1)
243-243: Consider using a more RESTful endpoint path.The current path
/get-address-for-order/{orderId}uses a verb in the URL. Consider a more RESTful approach:-@GetMapping("/get-address-for-order/{orderId}") +@GetMapping("/orders/{orderId}/address")This follows REST conventions by using nouns and hierarchical structure to represent resources.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/src/main/java/greencity/controller/AddressController.java(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
core/src/main/java/greencity/controller/AddressController.java (3)
14-14: LGTM! Dependency injection is properly implemented.The AddressService is correctly injected using constructor injection via @requiredargsconstructor, following best practices.
Also applies to: 50-50
205-227: LGTM! Well-documented PATCH endpoint with proper validation.The endpoint follows REST best practices with:
- Appropriate use of PATCH for updates
- Input validation
- Comprehensive error documentation
- Clear API documentation
229-246: LGTM! Successfully implemented the requested functionality.The endpoint correctly implements the PR objective to retrieve address for order by order ID, with proper error handling and documentation.
|
Summary by CodeRabbit
New Features
Tests