Implementation of Order feature.#15
Merged
Merged
Conversation
liliia-ponomarenko
suggested changes
May 30, 2025
liliia-ponomarenko
left a comment
liliia-ponomarenko
left a comment
Contributor
There was a problem hiding this comment.
Good job! Let’s improve your solution ;)
Comment on lines
+6
to
+7
| @Size(max = 255, message = "Shipping address must not exceed 255 characters") | ||
| String shippingAddress |
Comment on lines
+6
to
+7
| public record OrderStatusUpdateDto( | ||
| @NotBlank |
Contributor
There was a problem hiding this comment.
Suggested change
| public record OrderStatusUpdateDto( | |
| @NotBlank | |
| public record OrderStatusUpdateDto( | |
| @NotNull |
| private OrderStatus orderStatus = OrderStatus.CREATED; | ||
| @Column(name = "total", nullable = false) | ||
| private BigDecimal total = BigDecimal.ZERO; | ||
| @Column(name = "order_date", nullable = false) |
Contributor
There was a problem hiding this comment.
CreationTimestamp
Comment on lines
+27
to
+28
| @ManyToOne | ||
| @JoinColumn(name = "order_id", nullable = false) |
| @ManyToOne | ||
| @JoinColumn(name = "order_id", nullable = false) | ||
| private Order order; | ||
| @ManyToOne |
Comment on lines
+84
to
+87
| if (!orderRepository.existsByIdAndUserId(orderId, userId)) { | ||
| throw new EntityNotFoundException( | ||
| NO_ORDER_FOR_USER_MESSAGE.formatted(userId, orderId)); | ||
| } |
Contributor
There was a problem hiding this comment.
Do you really need this check
| return orderMapper.toDto(order); | ||
| } | ||
|
|
||
| private void resolveShippingAddress(User user, OrderRequestDto orderRequestDto, |
Contributor
There was a problem hiding this comment.
Make shippingAddress required field and avoid this check
| return order.getOrderItems().stream() | ||
| .map(orderItem -> orderItem.getPrice() | ||
| .multiply(new BigDecimal(orderItem.getQuantity()))) | ||
| .reduce(BigDecimal::add).orElseThrow(() -> new ArithmeticException( |
Contributor
There was a problem hiding this comment.
Avoid throwing ArithmeticException
Comment on lines
+150
to
+155
| private void emptyShoppingCart(ShoppingCart shoppingCart) { | ||
| shoppingCart.getCartItems() | ||
| .forEach(item -> item.setShoppingCart(null)); | ||
| shoppingCart.getCartItems().clear(); | ||
|
|
||
| } |
Contributor
There was a problem hiding this comment.
Create a separate clear shopping cart method in ShoppingCartService
| } | ||
|
|
||
| private ShoppingCart getUsersShoppingCart(Long userId) { | ||
| protected ShoppingCart getUsersShoppingCart(Long userId) { |
Elena-Bruyako
suggested changes
May 30, 2025
| @PatchMapping("/{orderId}") | ||
| public OrderResponseDto updateStatus( | ||
| @PathVariable Long orderId, | ||
| @RequestBody OrderStatusUpdateDto dto |
There was a problem hiding this comment.
Suggested change
| @RequestBody OrderStatusUpdateDto dto | |
| @RequestBody @Valid OrderStatusUpdateDto dto |
|
|
||
| public record OrderRequestDto( | ||
| @Size(max = 255, message = "Shipping address must not exceed 255 characters") | ||
| @NotNull |
There was a problem hiding this comment.
Suggested change
| @NotNull | |
| @NotBlank |
| @CreationTimestamp | ||
| @Column(name = "order_date", nullable = false) | ||
| private LocalDateTime orderDate = LocalDateTime.now(); | ||
| @Column(name = "shipping_address", nullable = false) |
There was a problem hiding this comment.
Suggested change
| @Column(name = "shipping_address", nullable = false) | |
| @Column(nullable = false) |
| @OneToMany(fetch = FetchType.LAZY, mappedBy = "order", | ||
| cascade = CascadeType.ALL, orphanRemoval = true) | ||
| private Set<OrderItem> orderItems = new HashSet<>(); | ||
| @Column(name = "is_deleted", columnDefinition = "TINYINT", nullable = false) |
There was a problem hiding this comment.
Suggested change
| @Column(name = "is_deleted", columnDefinition = "TINYINT", nullable = false) | |
| @Column(columnDefinition = "TINYINT", nullable = false) |
| private LocalDateTime orderDate = LocalDateTime.now(); | ||
| @Column(name = "shipping_address", nullable = false) | ||
| private String shippingAddress; | ||
| @OneToMany(fetch = FetchType.LAZY, mappedBy = "order", |
There was a problem hiding this comment.
Suggested change
| @OneToMany(fetch = FetchType.LAZY, mappedBy = "order", | |
| @OneToMany(mappedBy = "order", |
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "book_id", nullable = false) | ||
| private Book book; | ||
| @Column(name = "quantity", nullable = false) |
|
|
||
| void createUsersCart(User user); | ||
|
|
||
| default void clearShoppingCart(ShoppingCart shoppingCart) { |
There was a problem hiding this comment.
you can create method in ShoppingCart entity
public void clearCart() {
cartItems.clear();
}
| private final OrderItemMapper orderItemMapper; | ||
| private final ShoppingCartService shoppingCartService; | ||
|
|
||
| public OrderResponseDto createOrder(Long userId, OrderRequestDto orderRequestDto) { |
| public OrderResponseDto createOrder(Long userId, OrderRequestDto orderRequestDto) { | ||
| ShoppingCart shoppingCart = getShoppingCart(userId); | ||
| User owner = shoppingCart.getUser(); | ||
| if (shoppingCartService.isCartEmpty(shoppingCart)) { |
There was a problem hiding this comment.
why do you nee separate method for this logic?
Suggested change
| if (shoppingCartService.isCartEmpty(shoppingCart)) { | |
| if (shoppingCart.getCartItems().isEmpty()) { |
|
|
||
| @Override | ||
| public OrderItemResponseDto getSpecificItem(Long itemId, Long orderId, Long userId) { | ||
| return orderItemRepository.findByIdAndOrderId(itemId, orderId).map(orderItemMapper::toDto) |
There was a problem hiding this comment.
Suggested change
| return orderItemRepository.findByIdAndOrderId(itemId, orderId).map(orderItemMapper::toDto) | |
| return orderItemRepository.findByIdAndOrderIdAndUserId(itemId, orderId).map(orderItemMapper::toDto) |
Elena-Bruyako
approved these changes
Jun 2, 2025
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
3 participants
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.
No description provided.