-
Notifications
You must be signed in to change notification settings - Fork 0
add-shopping-cart-a #13
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
base: master
Are you sure you want to change the base?
Conversation
adrianbucur83
left a comment
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.
Nice job overall, see comments
| //create ShoppingCart | ||
| @PostMapping | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public void createShoppingCart(@RequestBody ShoppingCartDto shoppingCartDto){ |
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.
| } | ||
|
|
||
| @GetMapping("/description/{description}") | ||
| public List<ShoppingCartReturnDto> getShoppingCartByDescription |
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.
shopping carts are a bit special, you kinda want to set a cookie on the users' browser to identify his shopping cart id so that every time he comes to your website he has the products already in the cart. Maybe you want to implement something like this
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.
also don't take long strings as path variables it use request param better or request body. it will make your urls like this:
| //update ShoppingCart | ||
| @PutMapping | ||
| @ResponseStatus(HttpStatus.NO_CONTENT) | ||
| public void updateShoppingCart(@RequestBody ShoppingCartUpdateDto shoppingCartUpdateDto){ |
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.
add validation
| //@Data | ||
| //@Builder | ||
| @Entity | ||
| @Table(name="shopping_cart") |
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.
use plural
|
|
||
| List<ShoppingCart> findByDescriptionProduct(String description); | ||
|
|
||
| @Query(value="SELECT * FROM public.shopping_cart s WHERE s.description_product = :description", |
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.
nice mix of native and jpa queries, good job
| { | ||
| throw new RuntimeException("Delivery cannot be made on date specified"); | ||
| } | ||
| if (shoppingCartDto.getDescriptionProduct().length()<1) |
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.
you can use the the @Valid on the dto to check for min length and such
| public void create(ShoppingCartDto shoppingCartDto){ | ||
| if (!shoppingCartDto.isAvailability()) | ||
| { | ||
| throw new RuntimeException("Delivery is not available"); |
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.
define your own exception eg BusinessException and maybe an @ExceptionHandler too
No description provided.