Conversation
Elena-Bruyako
left a comment
Elena-Bruyako
left a comment
There was a problem hiding this comment.
Good job! Let's improve your solution
| description = "Retrieves a list of orders for the currently authenticated user.") | ||
| @GetMapping | ||
| public List<OrderResponseDto> getOrders(Authentication authentication) { | ||
| return orderService.getOrderByUser((User) authentication.getPrincipal()); |
There was a problem hiding this comment.
You can create separate method for returning Long userId instead of providing
(User) authentication.getPrincipal() to service
Check other places
| description = "Retrieves a specific item from an order " | ||
| + "by itemId, within the order identified by orderId.") | ||
| @GetMapping("/{orderId}/items/{itemId}") | ||
| public OrderItemResponseDto getOrderItem(@PathVariable Long orderId, |
| List<OrderItem> findAllByOrder(Order order); | ||
|
|
||
| Optional<OrderItem> findByIdAndOrderId(Long orderItemId, Long orderId); | ||
|
|
There was a problem hiding this comment.
remove empty line
| private final ShoppingCartRepository shoppingCartRepository; | ||
|
|
||
| @Override | ||
| @Transactional |
| + " for user with id: " + user.getId() | ||
| )); | ||
| if (shoppingCart.getCartItems().isEmpty()) { | ||
| throw new EntityNotFoundException("Shopping cart is empty " |
There was a problem hiding this comment.
| throw new EntityNotFoundException("Shopping cart is empty " | |
| throw new OrderProcessingException("Shopping cart is empty " |
| public List<OrderItemResponseDto> getOrderItemsByOrderId(Long orderId, Long userId) { | ||
| Order order = orderRepository | ||
| .findByIdAndUserId(orderId, userId) | ||
| .orElseThrow(() -> new EntityNotFoundException("Order items not found " |
There was a problem hiding this comment.
wrong message, here you try to find order
| .orElseThrow(() -> new EntityNotFoundException("Order items not found " | ||
| + "for order id: " + orderId + " and user id: " + userId | ||
| )); | ||
| return orderItemMapper.toDto(orderItemRepository.findAllByOrder(order)); |
There was a problem hiding this comment.
create List<OrderItemDto> toOrderItemDtoList(List<OrderItem> orderItems); in OrderItemMapper
| return orderItemMapper.toDto(orderItemRepository.findAllByOrder(order)); | |
| return orderItemMapper.toOrderItemDtoList(order.getOrderItems()); |
|
|
||
| if (existingCartItem.isPresent()) { | ||
| CartItem cartItem = existingCartItem.get(); | ||
| CartItem cartItem = existingCartItem.orElseThrow(); |
There was a problem hiding this comment.
is you use orElseThrow() it's better to provide exception with informative message and parameter
There was a problem hiding this comment.
@JJJazl wrote me that "It is recommended to use orElseThrow() instead of `get()'. You can also see this in the java doc"
There was a problem hiding this comment.
You can use get() or just orElseThrow() here, since above you are checking that the value is present
| spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver | ||
|
|
||
| spring.jpa.hibernate.ddl-auto=validate | ||
| spring.jpa.hibernate.ddl-auto=update |
| name: shipping_address | ||
| type: varchar(255) | ||
| constraints: | ||
| nullable: false |
| order.setUser(userRepository.findById(userId) | ||
| .orElseThrow(() -> new EntityNotFoundException("User with id " | ||
| + userId + " not found"))); |
There was a problem hiding this comment.
You can pass User user directly to the service method to avoid additional SELECT query
|
|
||
| if (existingCartItem.isPresent()) { | ||
| CartItem cartItem = existingCartItem.get(); | ||
| CartItem cartItem = existingCartItem.orElseThrow(); |
There was a problem hiding this comment.
You can use get() or just orElseThrow() here, since above you are checking that the value is present
3 participants
No description provided.