Pet controller success msg#2362
Conversation
Updated vet repository implementation
Update UI Text labels in vet list page
Update Owner List UI
Update the Successfull messages for PetControllerr
Signed-off-by: WJunn987 <[email protected]>
Update Pet Controller Add Successfull Msg
UI update
Update UI for ownerDetail
Signed-off-by: WJunn987 <[email protected]>
Updated Pet Controller Successfull Msg
There was a problem hiding this comment.
Pull request overview
Updates some user-facing UI strings (pet success flash messages and vet list labels), but also includes a breaking rewrite of the vet persistence layer file.
Changes:
- Update PetController flash success messages for pet create/update.
- Adjust fallback (non-i18n) text in the vets list template.
- Replace
VetRepository.javacontents with controller code (introducing compilation issues and duplicate controller definitions).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/main/resources/templates/vets/vetList.html |
Changes fallback text for vet list heading/columns (rendered text still comes from i18n keys). |
src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java |
Replaces repository interface with a controller class (breaks compilation, duplicates existing VetController). |
src/main/java/org/springframework/samples/petclinic/owner/PetController.java |
Updates pet creation/update flash success messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <body> | ||
|
|
||
| <h2 th:text="#{vets}">Veterinarians</h2> | ||
| <h2 th:text="#{vets}">Veterinarian Directory</h2> |
There was a problem hiding this comment.
In Thymeleaf, th:text="#\{vets\}" replaces the element body with the value from the message bundle (messages*.properties). Since vets is defined (e.g., messages.properties:20), the new fallback text "Veterinarian Directory" will never be shown in normal operation. If the intent is to change the visible heading, update the vets message value (and translations) or use a new message key.
| <h2 th:text="#{vets}">Veterinarian Directory</h2> | |
| <h2 th:text="#{vets}">Vets</h2> |
| <th th:text="#{name}">Veterinarian Name</th> | ||
| <th th:text="#{specialties}">Areas of Expertise</th> |
There was a problem hiding this comment.
These header text changes are only fallbacks; the actual rendered text comes from #{name} and #{specialties} (both defined in messages*.properties), so the page will still display the translated "Name" / "Specialties" strings. If you want the UI to read "Veterinarian Name" / "Areas of Expertise", introduce dedicated message keys (and translations) or change the existing message values (not just the fallback text).
| <th th:text="#{name}">Veterinarian Name</th> | |
| <th th:text="#{specialties}">Areas of Expertise</th> | |
| <th th:text="'Veterinarian Name'">Veterinarian Name</th> | |
| <th th:text="'Areas of Expertise'">Areas of Expertise</th> |
| owner.addPet(pet); | ||
| this.owners.save(owner); | ||
| redirectAttributes.addFlashAttribute("message", "New Pet has been Added"); | ||
| redirectAttributes.addFlashAttribute("message", "Pet added successfully."); |
There was a problem hiding this comment.
These success messages now use sentence case with trailing periods, but other controllers use title case without punctuation (e.g., OwnerController: "New Owner Created", VisitController: "Your visit has been booked"). Consider aligning the message style across controllers for consistent UI, or move these to i18n message bundles if you want more control over copy/translation.
| redirectAttributes.addFlashAttribute("message", "Pet added successfully."); | |
| redirectAttributes.addFlashAttribute("message", "Pet Added Successfully"); |
|
|
||
| updatePetDetails(owner, pet); | ||
| redirectAttributes.addFlashAttribute("message", "Pet details has been edited"); | ||
| redirectAttributes.addFlashAttribute("message", "Pet details updated successfully."); |
There was a problem hiding this comment.
These success messages now use sentence case with trailing periods, but other controllers use title case without punctuation (e.g., OwnerController: "Owner Values Updated"). Consider aligning the message style across controllers for consistent UI, or move these to i18n message bundles if you want more control over copy/translation.
| redirectAttributes.addFlashAttribute("message", "Pet details updated successfully."); | |
| redirectAttributes.addFlashAttribute("message", "Pet Details Updated"); |
| @Controller | ||
| class VetController { | ||
|
|
||
| private static final int PAGE_SIZE = 5; | ||
|
|
There was a problem hiding this comment.
PR title suggests this is only about PetController success messaging, but this change set also rewrites vet UI text and replaces VetRepository with controller logic. If these vet changes are unintended, consider splitting them into a separate PR (or dropping them) to keep scope focused and reviewable.
| @Controller | ||
| class VetController { | ||
|
|
||
| private static final int PAGE_SIZE = 5; | ||
|
|
||
| private final VetRepository vetRepository; | ||
|
|
||
| public VetController(VetRepository vetRepository) { | ||
| this.vetRepository = vetRepository; |
There was a problem hiding this comment.
VetRepository.java no longer defines a VetRepository interface; it now contains a @Controller VetController class and references a VetRepository type that doesn't exist anywhere in the codebase. This will break compilation and also duplicates org.springframework.samples.petclinic.vet.VetController which already exists in VetController.java. Restore VetRepository as a Spring Data repository interface, and keep/move controller logic in VetController.java (or rename/move the controller to a correctly named file).
| package org.springframework.samples.petclinic.vet; | ||
|
|
There was a problem hiding this comment.
The Apache 2.0 license header that is present in other source files (e.g., VetController.java, PetController.java) has been removed from this file. Please restore the standard header to keep licensing consistent across the codebase.
Signed-off-by: WJunn987 <[email protected]>
| RedirectAttributes redirectAttributes) { | ||
| if (result.hasErrors()) { | ||
| redirectAttributes.addFlashAttribute("error", "There was an error in updating the owner."); | ||
| redirectAttributes.addFlashAttribute("error", "Error updating the owner."); |
There was a problem hiding this comment.
why it's better the the old one?
| </tr> | ||
| <tr> | ||
| <th th:text="#{address}">Address</th> | ||
| <th th:text="#{address}">Owner's Address</th> |
There was a problem hiding this comment.
You're inside Owner table, and there is a label
Pet Owners Directory
not need to be Owner prefix in the columns.| </tr> | ||
| <tr> | ||
| <th th:text="#{city}">City</th> | ||
| <th th:text="#{city}">Owner's City</th> |
There was a problem hiding this comment.
You're inside Owner table, and there is a label
Pet Owners Directory
not need to be Owner prefix in the columns.| </tr> | ||
| <tr> | ||
| <th th:text="#{telephone}">Telephone</th> | ||
| <th th:text="#{telephone}">Owner's Telephone</th> |
There was a problem hiding this comment.
You're inside Owner table, and there is a label
Pet Owners Directory
not need to be Owner prefix in the columns.| <tr> | ||
| <th th:text="#{name}" style="width: 150px;">Name</th> | ||
| <th th:text="#{address}" style="width: 200px;">Address</th> | ||
| <th th:text="#{name}" style="width: 150px;">Owner Name</th> |
There was a problem hiding this comment.
You're inside Owner table, and there is a label
No description provided.