JJDZR14KA-40 added delegation and pagination #29
Merged
Conversation
This file contains 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
szablewski
reviewed
Aug 7, 2024
There was a problem hiding this comment.
- Brakuje komunikatów na froncie, gdy wykonamy jakąś operacjęn np. dodamy nowy transport czy accomodation, to samo się tyczy gdy usuniemy obiekt lub zrobimy update.
- Brak daty przy edycji lub gdy cofnie nas walidacja,
- Masz walidacje @NotNull(message = "Transport list is a required field!") natomiast kiedy dodam delegacje bez transportu to przechodzi
| @@ -108,14 +124,14 @@ private void validateAccommodation(Accommodation accommodation){ | |||
|
|
|||
| private void logSuccessMessage(Accommodation accommodation) { | |||
| String successMessage = String.format("Successfully updated accommodation: %s, type: $s, address: %s, ", | |||
There was a problem hiding this comment.
String.format("Successfully updated accommodation: %s", accommodation.getNameAccommodation()) ;
przydało by się to wyświetlić na foncie
| }); | ||
| delegation.setAccommodations(accommodations); | ||
|
|
||
| Double totalPrice = calculatorTotalPrice(transports, accommodations); |
There was a problem hiding this comment.
jeślichodzi o obliczanie pieniędzy został do t ego celu stworzony spejcalny obiekt BigDecimal
|
|
||
| public class DelegationNotFoundException extends IOException { | ||
| public DelegationNotFoundException(Long delegationId) { | ||
| super(String.format("No delegation found with id %d", delegationId)); |
There was a problem hiding this comment.
Suggested change
| super(String.format("No delegation found with id %d", delegationId)); | |
| super(String.format("Not found delegation with provide id: %d", delegationId)); |
| public String getAllAccommodations(@RequestParam(value = "page", defaultValue = "0") int page, | ||
| @RequestParam(value = "size", defaultValue = "10") int size, | ||
| Model model) throws IOException { | ||
| Page<Accommodation> accommodations = accommodationService.fetchAccommodationPage(PageRequest.of(page, 10)); |
There was a problem hiding this comment.
Suggested change
| Page<Accommodation> accommodations = accommodationService.fetchAccommodationPage(PageRequest.of(page, 10)); | |
| Page<Accommodation> accommodations = accommodationService.fetchAccommodationPage(PageRequest.of(page, size)); |
| List<Accommodation> accommodationList = accommodationService.fetchAccommodationList(); | ||
| log.info("Fetched accommodationList: {}", accommodationList.size()); | ||
| model.addAttribute("accommodations", accommodationList); | ||
| public String getAllAccommodations(@RequestParam(value = "page", defaultValue = "0") int page, |
There was a problem hiding this comment.
ze zmiennych defaultValue = "0" powinno zostać wyeksportowane do zmiennej private static final String PAGE_VALUE= "0"
to samo się tyczy size we wszystkich controlerach.
| @OneToMany(mappedBy = "delegation", cascade = CascadeType.ALL) | ||
| private List<Accommodation> accommodations; | ||
|
|
||
| @Digits(integer = 5, fraction = 2, message = "Invalid format. Max 5 digits and 2 decimals.") |
There was a problem hiding this comment.
Suggested change
| @Digits(integer = 5, fraction = 2, message = "Invalid format. Max 5 digits and 2 decimals.") | |
| @Positive() |
| this.employee = employee; | ||
| this.transports = transports; | ||
| this.accommodations = accommodations; | ||
| this.totalPrice = totalPrice != null ? totalPrice : 0.0; |
There was a problem hiding this comment.
nie robimy walidacji w konstruktorze
| } | ||
|
|
||
| @PostMapping("/delegation") | ||
| public String saveDelegation(@Valid @ModelAttribute("delegation") Delegation delegation, |
There was a problem hiding this comment.
- Nie potrzebujesz tu try/catch
Comment on lines
+70
to
+76
| if (bindingResult.hasErrors()) { | ||
| log.error("Validation errors occurred: {}", bindingResult.getAllErrors()); | ||
| model.addAttribute("employees", employeeService.loadEmployeeFromFile()); | ||
| model.addAttribute("transports", transportService.loadTransportFromFile()); | ||
| model.addAttribute("accommodations", accommodationService.loadAccommodationFromFile()); | ||
| return "delegation/createDelegationForm"; | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (bindingResult.hasErrors()) { | |
| log.error("Validation errors occurred: {}", bindingResult.getAllErrors()); | |
| model.addAttribute("employees", employeeService.loadEmployeeFromFile()); | |
| model.addAttribute("transports", transportService.loadTransportFromFile()); | |
| model.addAttribute("accommodations", accommodationService.loadAccommodationFromFile()); | |
| return "delegation/createDelegationForm"; | |
| } | |
| if (bindingResult.hasErrors()) { | |
| log.error("Validation errors occurred: {}", bindingResult.getAllErrors()); | |
| return "redirect:/delegation"; | |
| } |
There was a problem hiding this comment.
radził bym to zrobić w ten sposób, ponieważ ładowanie wszystkich obiektów wszystkich encji jest złą praktyką
| } | ||
|
|
||
| @Override | ||
| public void createDelegation(Delegation delegation) throws IOException { |
There was a problem hiding this comment.
zastanawiam się po szukasz tych wszystkich List transport, accomodation
szablewski
approved these changes
Aug 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.