Skip to content
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

Updated Pet and Vaccination model classes, integrated vaccination record management, and modified the edit view #365

Closed
wants to merge 3 commits into from

Conversation

AviraLS22
Copy link
Collaborator

No description provided.

@@ -191,4 +192,15 @@ private ModelAndView fillPetAndImageUrl(ModelAndView modelAndView) {
modelAndView.addObject("defaultImage", defaultImage);
return modelAndView;
}

// Method to view vaccination records for a specific pet
@GetMapping(value = "/vaccinations/{uuid}")
Copy link
Owner

@josdem josdem Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need to add another end-point to receive vaccines, because you can add a List<Vaccination> vaccines collection to PetCommand so that you can re-use the same pet/update end-point

import static jakarta.persistence.GenerationType.IDENTITY;

import com.josdem.vetlog.enums.PetStatus;
import jakarta.persistence.CascadeType;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not format the code, it makes harder to review your changes, we have spotless plugin to format the code automatically when we run tests, for more information see:

test.dependsOn("spotlessApply")

@JoinColumn(name = "adopter_id")
private User adopter;

@OneToMany(mappedBy = "pet", fetch = FetchType.LAZY, cascade = CascadeType.ALL) // Map to Vaccination
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually avoid to have double relationship, I rely in only one entity definition, please notice we have a relationship defined in Vaccination entity:

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "pet_id")
private Pet pet;

// Relationship mapping to Pet
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "pet_id", nullable = false)
private Pet pet;
Copy link
Owner

@josdem josdem Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you adding an image relationship? This task has nothing to do with images.

@@ -58,6 +58,6 @@ public class Vaccination {
private VaccinationStatus status;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "pet_id")
@JoinColumn(name = "pet_id", nullable = false) // Ensure pet_id is not nullable
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary.


/**
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normaly call this overdesign, you do not need to add code beyond the acceptance criteria.

<tr th:each="vaccination : ${petCommand.vaccinations}">
<td th:text="${vaccination.name}"></td>
<td>
<select th:field="*{vaccinations[__${vaccination.index}__].status}">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to read the current vaccines status value here, I do not think this code will do the job.

@josdem josdem closed this Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants