-
Notifications
You must be signed in to change notification settings - Fork 7
Feature/9 admin vendor portal #23
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: feature/9-admin-vendor-portal
Are you sure you want to change the base?
Feature/9 admin vendor portal #23
Conversation
…nto feature/9-admin-vendor-portal
|
@rohitnandi12 Please review PR , i'm will double check this tomorrow / day after again |
| import org.springframework.data.web.SortDefault; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; |
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.
expand
...rc/main/java/org/lbcc/bms/bms_monolith/admin/controller/AdminVendorOnboardingController.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/lbcc/bms/bms_monolith/admin/controller/AdminVendorOnboardingController.java
Outdated
Show resolved
Hide resolved
| @PageableDefault(page = 0, size = 10) | ||
| @SortDefault(sort = "name", direction = Sort.Direction.ASC) Pageable pageable) { | ||
|
|
||
| Page<Vendor> vendorsPage = adminService.searchVendors(searchRequest, pageable); |
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.
service should return Page
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.
adminService.searchVendors(searchRequest, pageable); method is already returning a Page right,
not understanding this comment
...ms-monolith/src/main/java/org/lbcc/bms/bms_monolith/admin/controller/SeatTypeController.java
Outdated
Show resolved
Hide resolved
...nd/java/bms-monolith/src/main/java/org/lbcc/bms/bms_monolith/admin/service/AdminService.java
Outdated
Show resolved
Hide resolved
...bms-monolith/src/main/java/org/lbcc/bms/bms_monolith/admin/service/VendorSpecifications.java
Show resolved
Hide resolved
...ckend/java/bms-monolith/src/main/java/org/lbcc/bms/bms_monolith/common/config/JpaConfig.java
Show resolved
Hide resolved
|
|
||
| @ManyToOne //fetch = FetchType.LAZY | ||
| @JoinColumn(name = "seat_id", nullable = false) | ||
| private Seat seat; |
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.
This may lead to a Hibernate N+1 query problem.
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.
have tried to resolve this with eager fetching , please confirm if this solves the issue
|
|
||
| @ManyToOne(cascade = CascadeType.ALL, fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "address_id") | ||
| private Address address; |
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.
How is this ManyToOne? One Venue can have only one Address right?
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.
the logic is 1 address can belong to multiple venues (assuming they are nearby) , 1 venue will have 1 address.
we can discuss and clarify things
|
|
||
| @GetMapping | ||
| public ResponseEntity<?> getAllVendors( | ||
| @ModelAttribute VendorSearchRequest searchRequest, |
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.
@ModelAttribute is optional right?
PR comments resolution
…nto feature/9-admin-vendor-portal # Conflicts: # book-my-show/backend/java/bms-monolith/src/main/java/org/lbcc/bms/bms_monolith/admin/controller/AdminVendorOnboardingController.java
No description provided.