Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
package org.example.vet1177.controllers;

import jakarta.validation.Valid;
import org.example.vet1177.entities.*;
import org.example.vet1177.policy.MedicalRecordPolicy;
import org.example.vet1177.services.MedicalRecordService;
import org.example.vet1177.services.UserService;
import org.springframework.http.ResponseEntity;
import org.example.vet1177.dto.request.medicalrecord.*;
import org.example.vet1177.dto.response.medicalrecord.*;
import org.example.vet1177.entities.*;
import org.example.vet1177.exception.ForbiddenException;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.*;

import java.util.List;
import java.util.UUID;

@RestController
@RequestMapping("/api/medical-records")
public class MedicalRecordController {

private final MedicalRecordService medicalRecordService;
private final MedicalRecordPolicy medicalRecordPolicy;
private final UserService userService;

public MedicalRecordController(
MedicalRecordService medicalRecordService,
MedicalRecordPolicy medicalRecordPolicy,
UserService userService) {
this.medicalRecordService = medicalRecordService;
this.medicalRecordPolicy = medicalRecordPolicy;
this.userService = userService;
}

// POST /api/medical-records
@PostMapping
@Transactional
public ResponseEntity<MedicalRecordResponse> create(
@Valid @RequestBody CreateMedicalRecordRequest request,
@AuthenticationPrincipal User currentUser) {

return ResponseEntity.ok(
MedicalRecordResponse.from(
medicalRecordService.create(
request.title(),
request.description(),
request.petId(),
request.clinicId(),
currentUser
)
)
);
}

// GET /api/medical-records/{id}
@GetMapping("/{id}")
@Transactional(readOnly = true)
public ResponseEntity<MedicalRecordResponse> getById(
@PathVariable UUID id,
@AuthenticationPrincipal User currentUser) {

MedicalRecord record = medicalRecordService.getById(id);
medicalRecordPolicy.canView(currentUser, record);
return ResponseEntity.ok(MedicalRecordResponse.from(record));
}

// GET /api/medical-records/my-records (för OWNER)
@GetMapping("/my-records")
@Transactional(readOnly = true)
public ResponseEntity<List<MedicalRecordSummaryResponse>> getMyRecords(
@AuthenticationPrincipal User currentUser) {

if (currentUser.getRole() != Role.OWNER) {
throw new ForbiddenException("Endast djurägare kan se sina egna ärenden");
}

return ResponseEntity.ok(
medicalRecordService.getByOwner(currentUser.getId())
.stream()
.map(MedicalRecordSummaryResponse::from)
.toList()
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.


@GetMapping("/owner/{ownerId}")
@Transactional(readOnly = true)
public ResponseEntity<List<MedicalRecordSummaryResponse>> getByOwner(
@PathVariable UUID ownerId,
@AuthenticationPrincipal User currentUser) {

if (currentUser.getRole() == Role.OWNER &&
!currentUser.getId().equals(ownerId)) {
throw new ForbiddenException("Du kan bara se dina egna ärenden");
}

return ResponseEntity.ok(
medicalRecordService.getByOwnerAllowedForUser(ownerId, currentUser)
.stream()
.map(MedicalRecordSummaryResponse::from)
.toList()
);
}

// GET /api/medical-records/pet/{petId}
// I controllern — enklare
@GetMapping("/pet/{petId}")
@Transactional(readOnly = true)
public ResponseEntity<List<MedicalRecordSummaryResponse>> getByPet(
@PathVariable UUID petId,
@AuthenticationPrincipal User currentUser) {

return ResponseEntity.ok(
medicalRecordService.getByPetAllowedForUser(petId, currentUser)
.stream()
.map(MedicalRecordSummaryResponse::from)
.toList()
);
}

// GET /api/medical-records/clinic/{clinicId}
@GetMapping("/clinic/{clinicId}")
@Transactional(readOnly = true)
public ResponseEntity<List<MedicalRecordSummaryResponse>> getByClinic(
@PathVariable UUID clinicId,
@AuthenticationPrincipal User currentUser) {

medicalRecordPolicy.canViewClinic(currentUser, clinicId);
return ResponseEntity.ok(
medicalRecordService.getByClinic(clinicId)
.stream()
.map(MedicalRecordSummaryResponse::from)
.toList()
);
}

// GET /api/medical-records/clinic/{clinicId}/status/{status}
@GetMapping("/clinic/{clinicId}/status/{status}")
@Transactional(readOnly = true)
public ResponseEntity<List<MedicalRecordSummaryResponse>> getByClinicAndStatus(
@PathVariable UUID clinicId,
@PathVariable RecordStatus status,
@AuthenticationPrincipal User currentUser) {

medicalRecordPolicy.canViewClinic(currentUser, clinicId);
return ResponseEntity.ok(
medicalRecordService.getByClinicAndStatus(clinicId, status)
.stream()
.map(MedicalRecordSummaryResponse::from)
.toList()
);
}

// PUT /api/medical-records/{id}
@PutMapping("/{id}")
@Transactional
public ResponseEntity<MedicalRecordResponse> update(
@PathVariable UUID id,
@Valid @RequestBody UpdateMedicalRecordRequest request,
@AuthenticationPrincipal User currentUser) {

MedicalRecord record = medicalRecordService.getById(id);
medicalRecordPolicy.canUpdate(currentUser, record);

return ResponseEntity.ok(
MedicalRecordResponse.from(
medicalRecordService.update(
id,
request.title(),
request.description(),
currentUser
)
)
);
}

// PUT /api/medical-records/{id}/assign-vet
@PutMapping("/{id}/assign-vet")
@Transactional
public ResponseEntity<MedicalRecordResponse> assignVet(
@PathVariable UUID id,
@Valid @RequestBody AssignVetRequest request,
@AuthenticationPrincipal User currentUser) {

MedicalRecord record = medicalRecordService.getById(id);
User vetToAssign = userService.getById(request.vetId());
medicalRecordPolicy.canAssignVet(currentUser, record, vetToAssign);

return ResponseEntity.ok(
MedicalRecordResponse.from(
medicalRecordService.assignVet(id, vetToAssign, currentUser)
)
Comment on lines +187 to +194

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

assign-vet can persist an invalid assignee and reopen closed records.

userService.getById(request.vetId()) accepts any User, while the provided MedicalRecordPolicy.canAssignVet(...) only checks access rules. Combined with MedicalRecordService.assignVet(...) always forcing IN_PROGRESS, this route can assign a non-vet and mutate a final record back out of CLOSED. Reject non-VET assignees and final records in the domain layer before saving.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/vet1177/controllers/MedicalRecordController.java`
around lines 169 - 176, The controller currently fetches any User via
userService.getById and relies on medicalRecordPolicy.canAssignVet only for
access checks, while medicalRecordService.assignVet unconditionally sets status
to IN_PROGRESS allowing non-VET assignees and reopening CLOSED records; update
the domain/service layer (MedicalRecordService.assignVet) to validate that the
assignee has role VET (reject otherwise) and to prevent changing a record whose
status is FINAL/CLOSED, throwing a domain exception; ensure
MedicalRecordController still calls medicalRecordPolicy.canAssignVet for
authorization but remove responsibility for role/status validation from the
controller and centralize checks in MedicalRecordService.assignVet (and related
domain model) before persisting.

);
}

// PUT /api/medical-records/{id}/status
@PutMapping("/{id}/status")
@Transactional
public ResponseEntity<MedicalRecordResponse> updateStatus(
@PathVariable UUID id,
@Valid @RequestBody UpdateStatusRequest request,
@AuthenticationPrincipal User currentUser) {

MedicalRecord record = medicalRecordService.getById(id);
medicalRecordPolicy.canUpdateStatus(currentUser, record, request.status());

return ResponseEntity.ok(
MedicalRecordResponse.from(
medicalRecordService.updateStatus(
id,
request.status(),
currentUser
)
)
);
}

// PUT /api/medical-records/{id}/close
@PutMapping("/{id}/close")
@Transactional
public ResponseEntity<MedicalRecordResponse> close(
@PathVariable UUID id,
@AuthenticationPrincipal User currentUser) {

MedicalRecord record = medicalRecordService.getById(id);
medicalRecordPolicy.canClose(currentUser, record);

return ResponseEntity.ok(
MedicalRecordResponse.from(
medicalRecordService.close(id, currentUser)
)
);
}
}
10 changes: 10 additions & 0 deletions src/main/java/org/example/vet1177/policy/MedicalRecordPolicy.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ public void canUpdateStatus(User user, MedicalRecord record, RecordStatus newSta
}
}


public boolean isAllowed(User user, MedicalRecord record) {
return switch (user.getRole()) {
case OWNER -> user.getId().equals(record.getOwner().getId());
case VET -> user.getClinic() != null &&
user.getClinic().getId().equals(record.getClinic().getId());
case ADMIN -> true;
};
}

public void canAssignVet(User user, MedicalRecord record, User vetToAssign) {
switch (user.getRole()) {
case OWNER ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import org.example.vet1177.entities.*;
import org.example.vet1177.exception.BusinessRuleException;
import org.example.vet1177.exception.ResourceNotFoundException;
import org.example.vet1177.policy.MedicalRecordPolicy;
import org.example.vet1177.repository.ClinicRepository;
import org.example.vet1177.repository.MedicalRecordRepository;
import org.example.vet1177.repository.PetRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -16,28 +19,44 @@
public class MedicalRecordService {

private final MedicalRecordRepository medicalRecordRepository;

public MedicalRecordService(MedicalRecordRepository medicalRecordRepository) {
private final PetRepository petRepository;
private final ClinicRepository clinicRepository;
private final MedicalRecordPolicy medicalRecordPolicy;

public MedicalRecordService(MedicalRecordRepository medicalRecordRepository,
PetRepository petRepository,
ClinicRepository clinicRepository,
MedicalRecordPolicy medicalRecordPolicy) {
this.medicalRecordRepository = medicalRecordRepository;
this.petRepository = petRepository;
this.clinicRepository = clinicRepository;
this.medicalRecordPolicy = medicalRecordPolicy;
}

// ── Skapa ────────────────────────────────────────────────

public MedicalRecord create(
String title,
String description,
Pet pet,
User owner,
Clinic clinic,
User createdBy) {
UUID petId,
UUID clinicId,
User currentUser) {

Pet pet = petRepository.findById(petId)
.orElseThrow(() -> new ResourceNotFoundException("Pet", petId));

Clinic clinic = clinicRepository.findById(clinicId)
.orElseThrow(() -> new ResourceNotFoundException("Clinic", clinicId));

medicalRecordPolicy.canCreate(currentUser, pet, clinic);

MedicalRecord record = new MedicalRecord();
record.setTitle(title);
record.setDescription(description);
record.setPet(pet);
record.setOwner(owner);
record.setOwner(pet.getOwner()); // ← hämtas från pet
record.setClinic(clinic);
record.setCreatedBy(createdBy);
record.setCreatedBy(currentUser);
record.setStatus(RecordStatus.OPEN);

return medicalRecordRepository.save(record);
Expand Down Expand Up @@ -71,12 +90,30 @@ public List<MedicalRecord> getByClinicAndStatus(UUID clinicId, RecordStatus stat
return medicalRecordRepository.findByClinicIdAndStatus(clinicId, status);
}

@Transactional(readOnly = true)
public List<MedicalRecord> getByPetAllowedForUser(UUID petId, User currentUser) {
List<MedicalRecord> all = medicalRecordRepository.findByPetId(petId);

return all.stream()
.filter(record -> medicalRecordPolicy.isAllowed(currentUser, record))
.toList();
}

@Transactional(readOnly = true)
public List<MedicalRecord> getByOwnerAllowedForUser(UUID ownerId, User currentUser) {
List<MedicalRecord> all = medicalRecordRepository.findByOwnerId(ownerId);

return all.stream()
.filter(record -> medicalRecordPolicy.isAllowed(currentUser, record))
.toList();
}

// ── Uppdatera ─────────────────────────────────────────────

public MedicalRecord update(UUID id, String title, String description, User updatedBy) {
MedicalRecord record = getById(id);

if (record.getStatus().isFinal()) { // ← mellan rad 75-76
if (record.getStatus().isFinal()) {
throw new BusinessRuleException("Stängda ärenden kan inte uppdateras");
}

Expand All @@ -86,16 +123,29 @@ public MedicalRecord update(UUID id, String title, String description, User upda
return medicalRecordRepository.save(record);
}

public MedicalRecord assignVet(UUID recordId, User vet, User updatedBy) {
public MedicalRecord assignVet(UUID recordId, User vetToAssign, User updatedBy) {
MedicalRecord record = getById(recordId);
record.setAssignedVet(vet);
if (record.getStatus().isFinal()) {
throw new BusinessRuleException(
"Kan inte tilldela handläggare till ett stängt ärende");
}

if (vetToAssign.getRole() != Role.VET) {
throw new BusinessRuleException(
"Endast veterinärer kan tilldelas som handläggare");
}

record.setAssignedVet(vetToAssign);
record.setStatus(RecordStatus.IN_PROGRESS);
record.setUpdatedBy(updatedBy);
return medicalRecordRepository.save(record);
}
Comment on lines +126 to 142

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing clinic validation for vet assignment.

The assignVet() method validates that the user has the VET role but does not verify that the vet belongs to the same clinic as the record. This could allow assigning a vet from clinic A to a record belonging to clinic B, breaking the multi-tenant clinic isolation enforced elsewhere (see canCreate() in MedicalRecordPolicy.java lines 33-35).

🛡️ Proposed fix to add clinic validation
     public MedicalRecord assignVet(UUID recordId, User vetToAssign, User updatedBy) {
         MedicalRecord record = getById(recordId);
         if (record.getStatus().isFinal()) {
             throw new BusinessRuleException(
                     "Kan inte tilldela handläggare till ett stängt ärende");
         }

         if (vetToAssign.getRole() != Role.VET) {
             throw new BusinessRuleException(
                     "Endast veterinärer kan tilldelas som handläggare");
         }

+        if (vetToAssign.getClinic() == null ||
+                !vetToAssign.getClinic().getId().equals(record.getClinic().getId())) {
+            throw new BusinessRuleException(
+                    "Veterinären måste tillhöra samma klinik som ärendet");
+        }
+
         record.setAssignedVet(vetToAssign);
         record.setStatus(RecordStatus.IN_PROGRESS);
         record.setUpdatedBy(updatedBy);
         return medicalRecordRepository.save(record);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public MedicalRecord assignVet(UUID recordId, User vetToAssign, User updatedBy) {
MedicalRecord record = getById(recordId);
record.setAssignedVet(vet);
if (record.getStatus().isFinal()) {
throw new BusinessRuleException(
"Kan inte tilldela handläggare till ett stängt ärende");
}
if (vetToAssign.getRole() != Role.VET) {
throw new BusinessRuleException(
"Endast veterinärer kan tilldelas som handläggare");
}
record.setAssignedVet(vetToAssign);
record.setStatus(RecordStatus.IN_PROGRESS);
record.setUpdatedBy(updatedBy);
return medicalRecordRepository.save(record);
}
public MedicalRecord assignVet(UUID recordId, User vetToAssign, User updatedBy) {
MedicalRecord record = getById(recordId);
if (record.getStatus().isFinal()) {
throw new BusinessRuleException(
"Kan inte tilldela handläggare till ett stängt ärende");
}
if (vetToAssign.getRole() != Role.VET) {
throw new BusinessRuleException(
"Endast veterinärer kan tilldelas som handläggare");
}
if (vetToAssign.getClinic() == null ||
!vetToAssign.getClinic().getId().equals(record.getClinic().getId())) {
throw new BusinessRuleException(
"Veterinären måste tillhöra samma klinik som ärendet");
}
record.setAssignedVet(vetToAssign);
record.setStatus(RecordStatus.IN_PROGRESS);
record.setUpdatedBy(updatedBy);
return medicalRecordRepository.save(record);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/vet1177/services/MedicalRecordService.java` around
lines 126 - 142, The assignVet method currently checks role but not clinic;
before setting assignedVet in assignVet(UUID recordId, User vetToAssign, User
updatedBy) fetch record via getById(recordId) and validate that vetToAssign
belongs to the same clinic as the record (compare record.getClinic() or clinicId
fields on record and vetToAssign), throwing a BusinessRuleException with a clear
message if they differ or if clinic info is missing; perform this clinic check
immediately after the Role.VET check and before modifying/saving the record to
preserve clinic isolation enforced elsewhere (see
MedicalRecordPolicy.canCreate).


public MedicalRecord updateStatus(UUID recordId, RecordStatus newStatus, User updatedBy) {
MedicalRecord record = getById(recordId);

if (record.getStatus().isFinal()) {
throw new BusinessRuleException("Stängda ärenden kan inte ändras");}
record.setStatus(newStatus);
record.setUpdatedBy(updatedBy);

Expand All @@ -121,4 +171,8 @@ public MedicalRecord close(UUID recordId, User closedBy) {

return medicalRecordRepository.save(record);
}

public MedicalRecord save(MedicalRecord record) {
return medicalRecordRepository.save(record);
}
}