Feat/medical record repo - #6
Merged
Merged
Conversation
annikaholmqvist94
requested review from
TatjanaTrajkovic,
johanbriger and
lindaeskilsson
March 26, 2026 09:46
📝 WalkthroughWalkthroughA new medical records feature is introduced, consisting of a JPA entity Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/example/vet1177/entities/MedicalRecord.java (2)
76-77: Avoid public mutation of generated primary key.Line 77 exposes a public setter for a generated ID. This makes accidental identity mutation possible and can cause persistence-context inconsistencies.
Suggested tightening
public UUID getId() { return id; } - public void setId(UUID id) { this.id = id; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/vet1177/entities/MedicalRecord.java` around lines 76 - 77, The class exposes a public setter for the generated primary key (method setId(UUID) in MedicalRecord), which allows external mutation; remove or tighten this setter to prevent identity changes — either delete the public setId(UUID) entirely or change its visibility to protected or package-private so only the ORM or same-package test code can set it (keep getId() public). Update any tests or framework usage to rely on the ORM reflection/constructors rather than calling the public setId(UUID).
51-55: Align timestamp column nullability with required persistence contract.Line 51 and Line 54 should explicitly declare
nullable = falseto match required timestamp behavior and tighten schema/entity consistency.Suggested annotation update
- `@Column`(name = "created_at", updatable = false) + `@Column`(name = "created_at", nullable = false, updatable = false) private Instant createdAt; @@ - `@Column`(name = "updated_at") + `@Column`(name = "updated_at", nullable = false) private Instant updatedAt;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/vet1177/entities/MedicalRecord.java` around lines 51 - 55, The MedicalRecord entity's timestamp columns lack explicit nullability which can mismatch the persistence contract; update the `@Column` annotations on the createdAt and updatedAt fields in class MedicalRecord to include nullable = false (i.e., `@Column`(name = "created_at", updatable = false, nullable = false) and `@Column`(name = "updated_at", nullable = false)) so the JPA mapping and DB schema enforce non-null timestamps.src/main/java/org/example/vet1177/repository/MedicalRecordRepository.java (1)
15-30: Consider paginated query variants and matching DB indexes for these filters.Line 15–Line 30 currently expose only unbounded
Listfetches. For medical records, these result sets can grow quickly and create avoidable memory pressure. Add paginated variants and ensure indexes exist forpet_id,owner_id,assigned_vet_id,clinic_id, and(clinic_id, status).Suggested repository additions
+import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; + + Page<MedicalRecord> findByPetId(UUID petId, Pageable pageable); + Page<MedicalRecord> findByOwnerId(UUID ownerId, Pageable pageable); + Page<MedicalRecord> findByAssignedVetId(UUID vetId, Pageable pageable); + Page<MedicalRecord> findByClinicId(UUID clinicId, Pageable pageable); + Page<MedicalRecord> findByStatus(RecordStatus status, Pageable pageable); + Page<MedicalRecord> findByClinicIdAndStatus(UUID clinicId, RecordStatus status, Pageable pageable);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/vet1177/repository/MedicalRecordRepository.java` around lines 15 - 30, Replace the unbounded List fetch methods with or add paginated overloads that accept Pageable and return Page<MedicalRecord> (e.g., add findByPetId(UUID petId, Pageable p) -> Page<MedicalRecord>, findByOwnerId(UUID ownerId, Pageable p), findByAssignedVetId(UUID vetId, Pageable p), findByClinicId(UUID clinicId, Pageable p), findByStatus(RecordStatus status, Pageable p), and findByClinicIdAndStatus(UUID clinicId, RecordStatus status, Pageable p)); update callers to use paging; and ensure DB indexes exist on pet_id, owner_id, assigned_vet_id, clinic_id and a composite index on (clinic_id, status) in the schema/migration so these paged queries are efficient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/example/vet1177/entities/MedicalRecord.java`:
- Around line 85-109: MedicalRecord allows inconsistent states between status
and closedAt; update the setters in MedicalRecord to enforce the invariant: in
setStatus(RecordStatus status) if status == RecordStatus.CLOSED ensure closedAt
is non-null (either set closedAt = Instant.now() when currently null or throw
IllegalStateException per project policy), and if changing from CLOSED to any
other status clear closedAt (set to null); in setClosedAt(Instant closedAt) if
closedAt != null set status = RecordStatus.CLOSED (or validate it already is),
and if closedAt == null ensure status != RecordStatus.CLOSED (or change status
accordingly). Apply this logic to any construction/path that mutates these
fields (e.g., constructors or builders) to keep status and closedAt consistent.
---
Nitpick comments:
In `@src/main/java/org/example/vet1177/entities/MedicalRecord.java`:
- Around line 76-77: The class exposes a public setter for the generated primary
key (method setId(UUID) in MedicalRecord), which allows external mutation;
remove or tighten this setter to prevent identity changes — either delete the
public setId(UUID) entirely or change its visibility to protected or
package-private so only the ORM or same-package test code can set it (keep
getId() public). Update any tests or framework usage to rely on the ORM
reflection/constructors rather than calling the public setId(UUID).
- Around line 51-55: The MedicalRecord entity's timestamp columns lack explicit
nullability which can mismatch the persistence contract; update the `@Column`
annotations on the createdAt and updatedAt fields in class MedicalRecord to
include nullable = false (i.e., `@Column`(name = "created_at", updatable = false,
nullable = false) and `@Column`(name = "updated_at", nullable = false)) so the JPA
mapping and DB schema enforce non-null timestamps.
In `@src/main/java/org/example/vet1177/repository/MedicalRecordRepository.java`:
- Around line 15-30: Replace the unbounded List fetch methods with or add
paginated overloads that accept Pageable and return Page<MedicalRecord> (e.g.,
add findByPetId(UUID petId, Pageable p) -> Page<MedicalRecord>,
findByOwnerId(UUID ownerId, Pageable p), findByAssignedVetId(UUID vetId,
Pageable p), findByClinicId(UUID clinicId, Pageable p),
findByStatus(RecordStatus status, Pageable p), and findByClinicIdAndStatus(UUID
clinicId, RecordStatus status, Pageable p)); update callers to use paging; and
ensure DB indexes exist on pet_id, owner_id, assigned_vet_id, clinic_id and a
composite index on (clinic_id, status) in the schema/migration so these paged
queries are efficient.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02c8b03e-1c6f-4666-bdca-040bdcde5ce0
📒 Files selected for processing (3)
src/main/java/org/example/vet1177/entities/MedicalRecord.javasrc/main/java/org/example/vet1177/entities/RecordStatus.javasrc/main/java/org/example/vet1177/repository/MedicalRecordRepository.java
Comment on lines
+85
to
+109
| public RecordStatus getStatus() { return status; } | ||
| public void setStatus(RecordStatus status) { this.status = status; } | ||
|
|
||
| public Pet getPet() { return pet; } | ||
| public void setPet(Pet pet) { this.pet = pet; } | ||
|
|
||
| public User getOwner() { return owner; } | ||
| public void setOwner(User owner) { this.owner = owner; } | ||
|
|
||
| public Clinic getClinic() { return clinic; } | ||
| public void setClinic(Clinic clinic) { this.clinic = clinic; } | ||
|
|
||
| public User getAssignedVet() { return assignedVet; } | ||
| public void setAssignedVet(User assignedVet) { this.assignedVet = assignedVet; } | ||
|
|
||
| public User getCreatedBy() { return createdBy; } | ||
| public void setCreatedBy(User createdBy) { this.createdBy = createdBy; } | ||
|
|
||
| public User getUpdatedBy() { return updatedBy; } | ||
| public void setUpdatedBy(User updatedBy) { this.updatedBy = updatedBy; } | ||
|
|
||
| public Instant getCreatedAt() { return createdAt; } | ||
| public Instant getUpdatedAt() { return updatedAt; } | ||
| public Instant getClosedAt() { return closedAt; } | ||
| public void setClosedAt(Instant closedAt) { this.closedAt = closedAt; } |
There was a problem hiding this comment.
Enforce status ↔ closedAt consistency to prevent invalid record states.
Line 86 and Line 109 allow RecordStatus.CLOSED with closedAt == null (and vice versa), which creates inconsistent medical record states.
Suggested invariant handling in setters
public RecordStatus getStatus() { return status; }
- public void setStatus(RecordStatus status) { this.status = status; }
+ public void setStatus(RecordStatus status) {
+ this.status = status;
+ if (status == RecordStatus.CLOSED && this.closedAt == null) {
+ this.closedAt = Instant.now();
+ } else if (status != RecordStatus.CLOSED) {
+ this.closedAt = null;
+ }
+ }
@@
public Instant getClosedAt() { return closedAt; }
- public void setClosedAt(Instant closedAt) { this.closedAt = closedAt; }
+ public void setClosedAt(Instant closedAt) {
+ this.closedAt = closedAt;
+ if (closedAt != null) {
+ this.status = RecordStatus.CLOSED;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/vet1177/entities/MedicalRecord.java` around lines
85 - 109, MedicalRecord allows inconsistent states between status and closedAt;
update the setters in MedicalRecord to enforce the invariant: in
setStatus(RecordStatus status) if status == RecordStatus.CLOSED ensure closedAt
is non-null (either set closedAt = Instant.now() when currently null or throw
IllegalStateException per project policy), and if changing from CLOSED to any
other status clear closedAt (set to null); in setClosedAt(Instant closedAt) if
closedAt != null set status = RecordStatus.CLOSED (or validate it already is),
and if closedAt == null ensure status != RecordStatus.CLOSED (or change status
accordingly). Apply this logic to any construction/path that mutates these
fields (e.g., constructors or builders) to keep status and closedAt consistent.
This was referenced Mar 26, 2026
Merged
Merged
This was referenced Apr 5, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit