feat: Add the rest of the CRUD#23
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds PUT and DELETE endpoints (update and delete) for Driver, Order, Payment, Rating, and Vehicle; implements corresponding in-place MapStruct mappers, transactional service update/delete logic, controller routes with role-based authorization, and extensive unit/integration tests covering success and failure cases. (≈50 words) Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller
participant Service
participant Mapper
participant Repository
Client->>Controller: PUT /{entity}/{id} + request DTO
Controller->>Service: updateEntity(id, request)
Service->>Repository: findById(id)
alt entity found
Service->>Mapper: updateEntity(request, existingEntity)
Mapper-->>Service: mutated existingEntity
Service->>Repository: save(existingEntity)
Repository-->>Service: savedEntity
Service->>Controller: Optional<DTO>
Controller-->>Client: 200 OK + DTO
else not found or validation fails
Service-->>Controller: Optional.empty
Controller-->>Client: 400 Bad Request
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/java/com/angel/autonow/driver/DriverController.java`:
- Around line 52-66: The controller currently returns 400 when the driver is
absent; update DriverController so updateDriver and deleteDriver return 404 Not
Found for missing IDs: change the updateDriver flow that now maps
driverService.updateDriver(id,
request).orElse(ResponseEntity.badRequest().build()) to return
ResponseEntity.notFound().build() when the Optional is empty, and change
deleteDriver to return ResponseEntity.notFound().build() when
driverService.deleteDriver(id) indicates the resource was not found (instead of
ResponseEntity.badRequest().build()); use the existing
driverService.updateDriver and driverService.deleteDriver return values to
decide between ResponseEntity.ok / noContent and ResponseEntity.notFound().
In `@backend/src/main/java/com/angel/autonow/driver/DriverService.java`:
- Around line 45-52: The current deleteDriver method has a TOCTOU race by
calling driverRepository.existsById(id) before driverRepository.deleteById(id);
remove the existsById check and instead perform a single delete call and handle
the "not found" outcome by catching the repository exception
(EmptyResultDataAccessException) around driverRepository.deleteById(id),
returning false when caught and true on successful deletion; update imports to
include the exception class and keep the method signature deleteDriver(Long id)
unchanged.
In `@backend/src/main/java/com/angel/autonow/order/OrderController.java`:
- Around line 53-55: The current `@PreAuthorize` on OrderController.updateOrder
allows any CUSTOMER to update any order by ID; restrict this by adding an
ownership check instead of the broad "hasAnyRole('ADMIN','CUSTOMER')"—either
replace the annotation with `@PreAuthorize`("hasRole('ADMIN') or
`@orderSecurity.isOrderOwner`(`#id`, authentication)") and implement an
OrderSecurity.isOrderOwner(Long id, Authentication auth) bean that queries
orderService (or repository) to compare the authenticated principal's user id to
the order.owner id, or perform the same owner verification at the start of
orderService.updateOrder(Long id, OrderRequestDTO request) using
SecurityContextHolder.getAuthentication() and throw AccessDeniedException if not
owner; ensure you do not rely on client-supplied owner fields when checking
ownership.
In `@backend/src/main/java/com/angel/autonow/order/OrderService.java`:
- Around line 118-122: The current existsById then deleteById sequence in
OrderService has a TOCTOU race; remove the existsById check and instead call
orderRepository.deleteById(id) inside a try/catch that catches
org.springframework.dao.EmptyResultDataAccessException (or the repository's
equivalent) and return false when that exception is thrown, otherwise return
true; this keeps the operation atomic under concurrency and references
orderRepository.deleteById and the OrderService deletion method.
- Around line 85-111: The code mutates the OrderEntity via
orderMapper.updateEntity and order.setUser/setDriver/setVehicle before
validating related entities, risking partial flushes when returning
Optional.empty(); fix by performing all lookups/validations first (call
userRepository.findById, driverRepository.findById, vehicleRepository.findById
and return Optional.empty() immediately on any missing), then call
orderMapper.updateEntity(orderRequest, order) and setUser/setDriver/setVehicle
only after all required entities are present; alternatively ensure the method is
transactional and throw an exception instead of returning Optional.empty() so
partial changes are not committed.
In `@backend/src/main/java/com/angel/autonow/payment/PaymentController.java`:
- Around line 53-57: The updatePayment endpoint in PaymentController currently
uses `@PreAuthorize`("hasAnyRole('ADMIN', 'CUSTOMER')") which allows any CUSTOMER
to update any payment by id; modify the endpoint to enforce ownership by
validating the authenticated user's account matches the payment owner before
updating: retrieve the principal (or userId) from the security context or method
parameter and pass it to paymentService.updatePayment(userId, id, request) (or
call a service method like paymentService.updatePaymentIfOwner(userId, id,
request)) and have the service verify ownership and return empty/forbidden if
mismatch; alternatively use a SpEL preauthorize expression that compares
principal.id to the payment owner id, but ensure the ownership check is
performed in the updatePayment method path.
In `@backend/src/main/java/com/angel/autonow/payment/PaymentService.java`:
- Around line 65-71: When reassigning payment.getOrder() to a new order in
PaymentService, add a uniqueness guard: before calling
orderRepository.findById(request.orderId()) and payment.setOrder(...), query the
payment repository (e.g., paymentRepository.findByOrderId(request.orderId()))
and if a different PaymentEntity exists for that orderId (existing.getId() !=
payment.getId()), return Optional.empty() (or surface a conflict) to avoid
violating the unique order_id constraint; then proceed to load the OrderEntity
and set it only when no conflicting payment exists.
- Around line 78-84: The deletePayment(Long id) method currently calls
paymentRepository.existsById(id) then paymentRepository.deleteById(id), which
leaves a TOCTOU race; replace this pattern by performing an atomic delete and
checking its result: either add a repository method (e.g., in PaymentRepository
add a `@Modifying` `@Query`("delete from Payment p where p.id = :id") int
deleteByIdReturningCount(Long id)) and have deletePayment call that and return
(count > 0), or keep deleteById but remove the existsById check and wrap
paymentRepository.deleteById(id) in a try/catch for
EmptyResultDataAccessException to return false on exception; update
deletePayment to use one of these approaches and remove the separate existsById
call.
In `@backend/src/main/java/com/angel/autonow/rating/RatingController.java`:
- Around line 53-65: The updateRating and deleteRating endpoints allow any
CUSTOMER to modify a rating by ID—add an ownership check so only the rating
owner can update/delete: retrieve the current user's id from the SecurityContext
(or Principal) in RatingController, fetch the rating owner via ratingService
(e.g., a getOwnerById or findById method), compare ownerId to currentUserId, and
return ResponseEntity.status(HttpStatus.FORBIDDEN).build() when they differ;
alternatively, extend ratingService.updateRating/deleteRating to accept
currentUserId and enforce ownership inside the service (methods: updateRating,
deleteRating) and return empty/false for unauthorized attempts so the controller
maps to Forbidden rather than allowing the operation.
In `@backend/src/main/java/com/angel/autonow/rating/RatingService.java`:
- Around line 74-78: The current delete flow in RatingService checks
ratingRepository.existsById(id) before calling ratingRepository.deleteById(id),
which introduces a TOCTOU race; instead, remove the existsById check and call
ratingRepository.deleteById(id) inside a try/catch that catches
org.springframework.dao.EmptyResultDataAccessException (thrown when the id does
not exist) and return false in that catch block, returning true when deleteById
completes without exception; update the method to reference RatingService and
ratingRepository.deleteById accordingly.
In `@backend/src/main/java/com/angel/autonow/vehicle/VehicleService.java`:
- Around line 43-50: Replace the non-atomic existsById + deleteById pattern in
VehicleService.deleteVehicle with a single retrieval-and-delete flow: use
vehicleRepository.findById(id) (or findById(...).map(...)) to fetch the entity
and, if present, call vehicleRepository.delete(entity) and return true,
otherwise return false; mark deleteVehicle as `@Transactional` (or ensure the
service class is transactional) so the read+delete happens in one transaction
and avoids the race with concurrent deletes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 079abb49-8162-4490-ad46-f40d3ed27cb0
📒 Files selected for processing (15)
backend/src/main/java/com/angel/autonow/driver/DriverController.javabackend/src/main/java/com/angel/autonow/driver/DriverMapper.javabackend/src/main/java/com/angel/autonow/driver/DriverService.javabackend/src/main/java/com/angel/autonow/order/OrderController.javabackend/src/main/java/com/angel/autonow/order/OrderMapper.javabackend/src/main/java/com/angel/autonow/order/OrderService.javabackend/src/main/java/com/angel/autonow/payment/PaymentController.javabackend/src/main/java/com/angel/autonow/payment/PaymentMapper.javabackend/src/main/java/com/angel/autonow/payment/PaymentService.javabackend/src/main/java/com/angel/autonow/rating/RatingController.javabackend/src/main/java/com/angel/autonow/rating/RatingMapper.javabackend/src/main/java/com/angel/autonow/rating/RatingService.javabackend/src/main/java/com/angel/autonow/vehicle/VehicleController.javabackend/src/main/java/com/angel/autonow/vehicle/VehicleMapper.javabackend/src/main/java/com/angel/autonow/vehicle/VehicleService.java
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/main/java/com/angel/autonow/payment/PaymentService.java (1)
86-93:⚠️ Potential issue | 🟠 MajorReplace
existsById+deleteByIdwith a single-step delete flow.Line [87] to Line [92] still has a TOCTOU window: another transaction can delete between the check and delete, which can surface as an unexpected failure path.
Suggested fix
+import org.springframework.dao.EmptyResultDataAccessException; ... public boolean deletePayment(Long id) { - if (!paymentRepository.existsById(id)) { - return false; - } - - paymentRepository.deleteById(id); - - return true; + try { + paymentRepository.deleteById(id); + return true; + } catch (EmptyResultDataAccessException ex) { + return false; + } }#!/bin/bash # Verify the current TOCTOU pattern is present in PaymentService. rg -n -C2 '\bexistsById\s*\(|\bdeleteById\s*\(' backend/src/main/java/com/angel/autonow/payment/PaymentService.java🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/com/angel/autonow/payment/PaymentService.java` around lines 86 - 93, The TOCTOU exists because deletePayment(Long id) uses paymentRepository.existsById(id) then paymentRepository.deleteById(id); remove the pre-check and perform a single-step delete: call paymentRepository.deleteById(id) inside a try/catch, catch org.springframework.dao.EmptyResultDataAccessException (thrown when the id is missing) and return false in that case, otherwise return true; update imports accordingly and keep the method signature deletePayment(Long id) and use paymentRepository as the repository reference.
🧹 Nitpick comments (6)
backend/src/test/java/com/angel/autonow/vehicle/VehicleControllerIT.java (1)
131-152: Assert persistence side effects in successful update/delete tests.Both success-path tests should also verify repository state after the HTTP assertion, so they validate behavior end-to-end (not just response mapping).
Suggested test hardening
+import static org.assertj.core.api.Assertions.assertThat; @@ void updateVehicle_asAdmin() throws Exception { @@ mockMvc.perform(put("/api/vehicles/{id}", vehicle.getId()) .with(TestData.adminJwt()) .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(updateRequest))) .andExpect(status().isOk()) .andExpect(jsonPath("$.brand").value("Honda")) .andExpect(jsonPath("$.model").value("Civic")) .andExpect(jsonPath("$.numberOfSeats").value(4)); + + var updated = vehicleRepository.findById(vehicle.getId()).orElseThrow(); + assertThat(updated.getBrand()).isEqualTo("Honda"); + assertThat(updated.getModel()).isEqualTo("Civic"); + assertThat(updated.getNumberOfSeats()).isEqualTo(4); } @@ void deleteVehicle_asAdmin() throws Exception { var vehicle = TestData.createVehicleEntity(); vehicleRepository.save(vehicle); - mockMvc.perform(delete("/api/vehicles/{id}", vehicle.getId()).with(TestData.adminJwt())).andExpect(status().isNoContent()); + mockMvc.perform(delete("/api/vehicles/{id}", vehicle.getId()).with(TestData.adminJwt())) + .andExpect(status().isNoContent()); + assertThat(vehicleRepository.existsById(vehicle.getId())).isFalse(); }Also applies to: 201-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/vehicle/VehicleControllerIT.java` around lines 131 - 152, The updateVehicle_asAdmin test (and the companion delete test) only asserts the HTTP response; after the mockMvc.perform call retrieve the entity from vehicleRepository (e.g., vehicleRepository.findById(vehicle.getId()).orElseThrow()) and assert the persisted fields match the updateRequest (brand, model, numberOfSeats, airConditioning, trunkCapacity, vehicleType) to verify persistence side effects; for the delete test, after the HTTP success assertion verify the entity is absent (e.g., vehicleRepository.existsById(deletedId) is false or findById returns empty).backend/src/test/java/com/angel/autonow/rating/RatingControllerIT.java (1)
305-317: Strengthen delete success tests with post-delete persistence checks.Both success-path delete tests currently assert only HTTP status. Adding a repository assertion prevents false positives if endpoint behavior regresses to returning
204without deleting.Suggested test hardening
`@Test` void deleteRating_asCustomer() throws Exception { var rating = TestData.createRatingEntity(order, 5, "Great"); ratingRepository.save(rating); mockMvc.perform(delete("/api/ratings/{id}", rating.getId()).with(TestData.customerJwt())).andExpect(status().isNoContent()); + assertFalse(ratingRepository.existsById(rating.getId())); } `@Test` void deleteRating_asAdmin() throws Exception { var rating = TestData.createRatingEntity(order, 5, "Great"); ratingRepository.save(rating); mockMvc.perform(delete("/api/ratings/{id}", rating.getId()).with(TestData.adminJwt())).andExpect(status().isNoContent()); + assertFalse(ratingRepository.existsById(rating.getId())); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/rating/RatingControllerIT.java` around lines 305 - 317, Update the deleteRating_asCustomer and deleteRating_asAdmin tests to assert persistence after the HTTP 204: after performing mockMvc.perform(delete(...)) and asserting status().isNoContent(), query the ratingRepository (e.g., ratingRepository.findById(rating.getId()) or ratingRepository.existsById(rating.getId())) and assert the rating no longer exists; use the existing rating variable from TestData.createRatingEntity(order, ...) to locate the record and fail the test if the repository still contains the entity.backend/src/test/java/com/angel/autonow/rating/RatingServiceTest.java (1)
167-192: Add explicit “no mapper call” assertions for early exits.For not-found and missing-order branches, asserting mapper non-invocation makes the tests more regression-resistant.
Suggested assertion additions
`@Test` void updateRating_notFound_returnsEmpty() { RatingRequestDTO request = TestData.createRatingRequest(1L); when(ratingRepository.findById(NON_EXISTENT_ID)).thenReturn(Optional.empty()); var result = ratingService.updateRating(NON_EXISTENT_ID, request); assertTrue(result.isEmpty()); verify(ratingRepository, never()).save(any()); + verify(ratingMapper, never()).updateEntity(any(), any()); } `@Test` void updateRating_orderNotFound_returnsEmpty() { RatingRequestDTO request = TestData.createRatingRequest(2L); OrderEntity order = OrderEntity.builder().id(1L).build(); RatingEntity existing = RatingEntity.builder().id(1L).order(order).rating(3).createdAt(NOW).build(); when(ratingRepository.findById(1L)).thenReturn(Optional.of(existing)); when(orderRepository.findById(2L)).thenReturn(Optional.empty()); var result = ratingService.updateRating(1L, request); assertTrue(result.isEmpty()); verify(ratingRepository, never()).save(any()); + verify(ratingMapper, never()).updateEntity(any(), any()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/rating/RatingServiceTest.java` around lines 167 - 192, The tests for updateRating early-exit branches should also assert that the mapper is not called to prevent regressions; in both updateRating_notFound_returnsEmpty and updateRating_orderNotFound_returnsEmpty add an explicit verification such as verifyNoInteractions(ratingMapper) (or verify(ratingMapper, never()).<relevantMappingMethod>(any()) if you prefer method-specific checks) after the existing verify(ratingRepository, never()).save(any()) to ensure RatingService.updateRating does not invoke the mapper on early returns.backend/src/test/java/com/angel/autonow/payment/PaymentServiceTest.java (1)
200-212: Add a unit test for the “order already paid” update branch.
updatePaymentnow has a conflict path when another payment already exists for the target order; this branch should be explicitly asserted inPaymentServiceTestto prevent regressions.Suggested additional test
+ `@Test` + void updatePayment_targetOrderAlreadyHasPayment_returnsEmpty() { + PaymentRequestDTO request = TestData.createPaymentRequest(2L); + OrderEntity currentOrder = OrderEntity.builder().id(1L).build(); + OrderEntity targetOrder = OrderEntity.builder().id(2L).build(); + + PaymentEntity existing = PaymentEntity.builder().id(1L).order(currentOrder).build(); + PaymentEntity otherPayment = PaymentEntity.builder().id(99L).order(targetOrder).build(); + + when(paymentRepository.findById(1L)).thenReturn(Optional.of(existing)); + when(paymentRepository.findByOrderId(2L)).thenReturn(Optional.of(otherPayment)); + + var result = paymentService.updatePayment(1L, request); + + assertTrue(result.isEmpty()); + verify(paymentRepository, never()).save(any()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/payment/PaymentServiceTest.java` around lines 200 - 212, Add a new unit test in PaymentServiceTest that covers the "order already paid" branch of PaymentService.updatePayment: arrange a PaymentEntity existing (id 1) linked to a different OrderEntity (id 2) and a new PaymentRequestDTO targeting that same order id, mock paymentRepository.findById(1L) to return the existing payment and orderRepository.findById(2L) to return an Optional.of(order) but also mock paymentRepository.findByOrderId(2L) or whichever method updatePayment uses to detect an existing payment to return a non-empty Optional (or list) so updatePayment takes the "order already paid" path, call paymentService.updatePayment(1L, request) and assert the result matches the expected conflict response (e.g., Optional.empty() or specific exception/flag) to prevent regressions in the updatePayment conflict handling.backend/src/test/java/com/angel/autonow/order/OrderControllerIT.java (1)
253-260: Assert delete side effect, not only HTTP status.The success test currently validates
204but not that the entity is actually gone. Adding a repository assertion will make this test harder to false-pass.Proposed test hardening
mockMvc.perform(delete("/api/orders/{id}", order.getId()) .with(TestData.adminJwt())) .andExpect(status().isNoContent()); + + assertFalse(orderRepository.existsById(order.getId()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/order/OrderControllerIT.java` around lines 253 - 260, The test deleteOrder_asAdmin currently only asserts HTTP 204; after performing mockMvc.perform(...) you should verify the side-effect by checking the repository: use orderRepository (e.g., existsById or findById) with order.getId() to assert the entity is removed (expect false or empty). Ensure you call this assertion after the mockMvc.perform block in deleteOrder_asAdmin() where TestData.createOrderEntity(user) and orderRepository.save(order) were used so the test fails if the delete did not actually remove the record.backend/src/test/java/com/angel/autonow/order/OrderServiceTest.java (1)
84-89: Consider extracting a shared request factory for repeated builders.These repeated blocks are identical in structure and will be easier to maintain via a local helper or
TestDatafactory.Refactor sketch
+ private OrderRequestDTO baseOrderRequest(Long userId) { + return OrderRequestDTO.builder() + .userId(userId) + .vehicleType(VehicleType.TAXI) + .pickupAddress(TestData.DEFAULT_PICKUP_ADDRESS) + .pickupLatitude(TestData.DEFAULT_PICKUP_LAT) + .pickupLongitude(TestData.DEFAULT_PICKUP_LNG) + .dropoffAddress(TestData.DEFAULT_DROPOFF_ADDRESS) + .dropoffLatitude(TestData.DEFAULT_DROPOFF_LAT) + .dropoffLongitude(TestData.DEFAULT_DROPOFF_LNG) + .estimatedPrice(15.50) + .distanceKm(5.2) + .estimatedDurationMinutes(15) + .build(); + }Also applies to: 118-123, 138-143, 269-274, 289-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/angel/autonow/order/OrderServiceTest.java` around lines 84 - 89, The repetitive OrderRequestDTO builder usage should be extracted to a single shared factory method to reduce duplication; add a helper like TestData.buildDefaultOrderRequest() or a private method in OrderServiceTest (e.g., createDefaultOrderRequest()) that returns an OrderRequestDTO pre-populated with userId(1L), driverId(2L), vehicleId(3L), vehicleType(VehicleType.TAXI), pickup/dropoff address/lat/lng from TestData, estimatedPrice(15.50), distanceKm(5.2), and estimatedDurationMinutes(15), then replace the identical builder blocks in OrderServiceTest (occurrences around the existing builder at lines shown) with calls to that factory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/test/java/com/angel/autonow/order/OrderServiceTest.java`:
- Around line 318-324: In the test deleteOrder_notFound_returnFalse, add a
negative interaction assertion to ensure deleteById is not invoked when
existsById returns false: after calling
orderService.deleteOrder(NON_EXISTENT_ID) and asserting the result is false,
verify that orderRepository.deleteById(NON_EXISTENT_ID) was never called (use
verify(orderRepository, never()) or equivalent) to confirm no deletion attempt
occurs.
---
Duplicate comments:
In `@backend/src/main/java/com/angel/autonow/payment/PaymentService.java`:
- Around line 86-93: The TOCTOU exists because deletePayment(Long id) uses
paymentRepository.existsById(id) then paymentRepository.deleteById(id); remove
the pre-check and perform a single-step delete: call
paymentRepository.deleteById(id) inside a try/catch, catch
org.springframework.dao.EmptyResultDataAccessException (thrown when the id is
missing) and return false in that case, otherwise return true; update imports
accordingly and keep the method signature deletePayment(Long id) and use
paymentRepository as the repository reference.
---
Nitpick comments:
In `@backend/src/test/java/com/angel/autonow/order/OrderControllerIT.java`:
- Around line 253-260: The test deleteOrder_asAdmin currently only asserts HTTP
204; after performing mockMvc.perform(...) you should verify the side-effect by
checking the repository: use orderRepository (e.g., existsById or findById) with
order.getId() to assert the entity is removed (expect false or empty). Ensure
you call this assertion after the mockMvc.perform block in deleteOrder_asAdmin()
where TestData.createOrderEntity(user) and orderRepository.save(order) were used
so the test fails if the delete did not actually remove the record.
In `@backend/src/test/java/com/angel/autonow/order/OrderServiceTest.java`:
- Around line 84-89: The repetitive OrderRequestDTO builder usage should be
extracted to a single shared factory method to reduce duplication; add a helper
like TestData.buildDefaultOrderRequest() or a private method in OrderServiceTest
(e.g., createDefaultOrderRequest()) that returns an OrderRequestDTO
pre-populated with userId(1L), driverId(2L), vehicleId(3L),
vehicleType(VehicleType.TAXI), pickup/dropoff address/lat/lng from TestData,
estimatedPrice(15.50), distanceKm(5.2), and estimatedDurationMinutes(15), then
replace the identical builder blocks in OrderServiceTest (occurrences around the
existing builder at lines shown) with calls to that factory.
In `@backend/src/test/java/com/angel/autonow/payment/PaymentServiceTest.java`:
- Around line 200-212: Add a new unit test in PaymentServiceTest that covers the
"order already paid" branch of PaymentService.updatePayment: arrange a
PaymentEntity existing (id 1) linked to a different OrderEntity (id 2) and a new
PaymentRequestDTO targeting that same order id, mock
paymentRepository.findById(1L) to return the existing payment and
orderRepository.findById(2L) to return an Optional.of(order) but also mock
paymentRepository.findByOrderId(2L) or whichever method updatePayment uses to
detect an existing payment to return a non-empty Optional (or list) so
updatePayment takes the "order already paid" path, call
paymentService.updatePayment(1L, request) and assert the result matches the
expected conflict response (e.g., Optional.empty() or specific exception/flag)
to prevent regressions in the updatePayment conflict handling.
In `@backend/src/test/java/com/angel/autonow/rating/RatingControllerIT.java`:
- Around line 305-317: Update the deleteRating_asCustomer and
deleteRating_asAdmin tests to assert persistence after the HTTP 204: after
performing mockMvc.perform(delete(...)) and asserting status().isNoContent(),
query the ratingRepository (e.g., ratingRepository.findById(rating.getId()) or
ratingRepository.existsById(rating.getId())) and assert the rating no longer
exists; use the existing rating variable from TestData.createRatingEntity(order,
...) to locate the record and fail the test if the repository still contains the
entity.
In `@backend/src/test/java/com/angel/autonow/rating/RatingServiceTest.java`:
- Around line 167-192: The tests for updateRating early-exit branches should
also assert that the mapper is not called to prevent regressions; in both
updateRating_notFound_returnsEmpty and updateRating_orderNotFound_returnsEmpty
add an explicit verification such as verifyNoInteractions(ratingMapper) (or
verify(ratingMapper, never()).<relevantMappingMethod>(any()) if you prefer
method-specific checks) after the existing verify(ratingRepository,
never()).save(any()) to ensure RatingService.updateRating does not invoke the
mapper on early returns.
In `@backend/src/test/java/com/angel/autonow/vehicle/VehicleControllerIT.java`:
- Around line 131-152: The updateVehicle_asAdmin test (and the companion delete
test) only asserts the HTTP response; after the mockMvc.perform call retrieve
the entity from vehicleRepository (e.g.,
vehicleRepository.findById(vehicle.getId()).orElseThrow()) and assert the
persisted fields match the updateRequest (brand, model, numberOfSeats,
airConditioning, trunkCapacity, vehicleType) to verify persistence side effects;
for the delete test, after the HTTP success assertion verify the entity is
absent (e.g., vehicleRepository.existsById(deletedId) is false or findById
returns empty).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a5bc11e-0711-4a3d-807c-684d4c769722
📒 Files selected for processing (17)
backend/src/main/java/com/angel/autonow/driver/DriverRequestDTO.javabackend/src/main/java/com/angel/autonow/order/OrderRequestDTO.javabackend/src/main/java/com/angel/autonow/order/OrderService.javabackend/src/main/java/com/angel/autonow/payment/PaymentRequestDTO.javabackend/src/main/java/com/angel/autonow/payment/PaymentService.javabackend/src/main/java/com/angel/autonow/rating/RatingRequestDTO.javabackend/src/main/java/com/angel/autonow/vehicle/VehicleRequestDTO.javabackend/src/test/java/com/angel/autonow/driver/DriverControllerIT.javabackend/src/test/java/com/angel/autonow/driver/DriverServiceTest.javabackend/src/test/java/com/angel/autonow/order/OrderControllerIT.javabackend/src/test/java/com/angel/autonow/order/OrderServiceTest.javabackend/src/test/java/com/angel/autonow/payment/PaymentControllerIT.javabackend/src/test/java/com/angel/autonow/payment/PaymentServiceTest.javabackend/src/test/java/com/angel/autonow/rating/RatingControllerIT.javabackend/src/test/java/com/angel/autonow/rating/RatingServiceTest.javabackend/src/test/java/com/angel/autonow/vehicle/VehicleControllerIT.javabackend/src/test/java/com/angel/autonow/vehicle/VehicleServiceTest.java
✅ Files skipped from review due to trivial changes (3)
- backend/src/main/java/com/angel/autonow/vehicle/VehicleRequestDTO.java
- backend/src/main/java/com/angel/autonow/rating/RatingRequestDTO.java
- backend/src/test/java/com/angel/autonow/vehicle/VehicleServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/main/java/com/angel/autonow/order/OrderService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Tests