patient entity-class and functionality implemented#23
Merged
LinusWestling merged 8 commits intomainfrom Mar 31, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis pull request introduces complete patient management functionality across the application stack. New components include a JPA entity layer ( Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as PatientController
participant Service as PatientService
participant Mapper as PatientMapper
participant Repo as PatientRepository
participant DB as Database
Client->>Controller: POST /api/patients (PatientDTO)
Controller->>Service: createPatient(patientDTO)
Service->>Mapper: toEntity(patientDTO)
Mapper-->>Service: PatientEntity
Service->>Service: Assign UUID & Instant.now()
Service->>Repo: findByPersonalIdentityNumber(...)
Repo->>DB: Query for existing patient
DB-->>Repo: Result (or empty)
alt Duplicate Found
Service-->>Controller: ResponseStatusException (CONFLICT)
Controller-->>Client: 409 Conflict
else Unique
Service->>Repo: save(entity)
Repo->>DB: INSERT patient
DB-->>Repo: Persisted entity
Repo-->>Service: Saved entity
Service->>Mapper: toDTO(entity)
Mapper-->>Service: PatientDTO
Service-->>Controller: PatientDTO
Controller-->>Client: 200 OK (PatientDTO)
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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 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: 2
🧹 Nitpick comments (3)
src/main/java/org/example/projektarendehantering/presentation/rest/PatientController.java (2)
23-25: Prefer201 Createdfor successful POST creation.Returning
200 OKhere makes create semantics less explicit for API consumers.Suggested fix
`@PostMapping` public ResponseEntity<PatientDTO> createPatient(`@RequestBody` `@Valid` PatientDTO patientDTO) { - return ResponseEntity.ok(patientService.createPatient(patientDTO)); + PatientDTO created = patientService.createPatient(patientDTO); + return ResponseEntity + .created(java.net.URI.create("/api/patients/" + created.getId())) + .body(created); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/presentation/rest/PatientController.java` around lines 23 - 25, The createPatient method in PatientController currently returns 200 OK; change it to return 201 Created by using ResponseEntity.created(...) or ResponseEntity.status(HttpStatus.CREATED).body(...) and include the created PatientDTO in the response body; call patientService.createPatient(patientDTO) as before and build a Location header pointing to the new resource (e.g., using the new patient's id) so API consumers receive the URI and the 201 status (update the createPatient method and any imports accordingly).
34-36:GET /api/patientsshould be paginated.Returning all patients in one response can degrade latency and memory usage as the dataset grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/presentation/rest/PatientController.java` around lines 34 - 36, The current PatientController.getAllPatients returns all patients at once (ResponseEntity<List<PatientDTO>> patientService.getAllPatients()), which must be paginated; change the controller method signature to accept pagination (either a Pageable parameter or `@RequestParam` int page, int size), update the controller to call a paginated service method (e.g., patientService.getPatients(Pageable) or patientService.getPatients(page, size)) that returns a Page<PatientDTO> (or a paginated DTO), and return ResponseEntity<Page<PatientDTO>> (or ResponseEntity with pagination headers and a List body) so the endpoint supports page/size inputs and limits payloads; also update the PatientService and its repository calls to use pageable queries (e.g., repository.findAll(Pageable)) and adjust tests accordingly.src/main/java/org/example/projektarendehantering/application/service/PatientService.java (1)
39-41: Consider throwing a domain-specific exception instead ofResponseStatusException.
ResponseStatusExceptioncouples the service layer to Spring Web/HTTP concerns. For better separation, throw a custom domain exception (e.g.,PatientAlreadyExistsException) and handle the HTTP mapping in a@ControllerAdviceor the controller itself.♻️ Proposed refactor
Create a domain exception:
public class PatientAlreadyExistsException extends RuntimeException { public PatientAlreadyExistsException(String personalIdentityNumber) { super("Patient with personalIdentityNumber " + personalIdentityNumber + " already exists"); } }Then in the service:
- throw new ResponseStatusException(HttpStatus.CONFLICT, "Patient with personalIdentityNumber already exists"); + throw new PatientAlreadyExistsException(entity.getPersonalIdentityNumber());Handle in a
@ControllerAdvice:`@ExceptionHandler`(PatientAlreadyExistsException.class) public ResponseEntity<String> handleDuplicate(PatientAlreadyExistsException ex) { return ResponseEntity.status(HttpStatus.CONFLICT).body(ex.getMessage()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/application/service/PatientService.java` around lines 39 - 41, Replace the ResponseStatusException thrown in PatientService (the lambda inside the .ifPresent call) with a domain-specific unchecked exception PatientAlreadyExistsException that accepts the personalIdentityNumber and constructs the message; add a new PatientAlreadyExistsException class extending RuntimeException with a constructor that formats "Patient with personalIdentityNumber X already exists"; and add a `@ControllerAdvice` (e.g., GlobalExceptionHandler) with an `@ExceptionHandler`(PatientAlreadyExistsException.class) method (e.g., handleDuplicate) that returns ResponseEntity.status(HttpStatus.CONFLICT).body(ex.getMessage()) to map the domain error to HTTP 409.
🤖 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/projektarendehantering/application/service/PatientMapper.java`:
- Around line 24-28: The mapping currently copies server-managed fields from DTO
(entity.setId(dto.getId()) and entity.setCreatedAt(dto.getCreatedAt())) which
allows clients to inject/override id and audit timestamps; remove these
assignments from the PatientMapper method that converts DTO→entity (the method
containing entity.setId and entity.setCreatedAt) so that id and createdAt are
not taken from inbound DTOs — for create flows leave id null and createdAt unset
so the persistence layer or database assigns them, and for update flows load the
existing Patient entity first and update only mutable fields (firstName,
lastName, personalIdentityNumber) while preserving the existing id and
createdAt.
In
`@src/main/java/org/example/projektarendehantering/presentation/dto/PatientDTO.java`:
- Line 18: PatientDTO currently exposes a raw personalIdentityNumber field which
leaks sensitive PII; remove or stop returning personalIdentityNumber from the
general PatientDTO used for read responses and instead introduce a dedicated DTO
(e.g., PatientSensitiveDTO or PatientRequestDTO) for payloads that must include
the full identity number, or return a masked version (e.g., show only last 4
digits) when needed; update usages of PatientDTO across service/controller
layers to use the non-PII DTO for normal responses and map to the sensitive DTO
only in controlled flows where full PII is required.
---
Nitpick comments:
In
`@src/main/java/org/example/projektarendehantering/application/service/PatientService.java`:
- Around line 39-41: Replace the ResponseStatusException thrown in
PatientService (the lambda inside the .ifPresent call) with a domain-specific
unchecked exception PatientAlreadyExistsException that accepts the
personalIdentityNumber and constructs the message; add a new
PatientAlreadyExistsException class extending RuntimeException with a
constructor that formats "Patient with personalIdentityNumber X already exists";
and add a `@ControllerAdvice` (e.g., GlobalExceptionHandler) with an
`@ExceptionHandler`(PatientAlreadyExistsException.class) method (e.g.,
handleDuplicate) that returns
ResponseEntity.status(HttpStatus.CONFLICT).body(ex.getMessage()) to map the
domain error to HTTP 409.
In
`@src/main/java/org/example/projektarendehantering/presentation/rest/PatientController.java`:
- Around line 23-25: The createPatient method in PatientController currently
returns 200 OK; change it to return 201 Created by using
ResponseEntity.created(...) or
ResponseEntity.status(HttpStatus.CREATED).body(...) and include the created
PatientDTO in the response body; call patientService.createPatient(patientDTO)
as before and build a Location header pointing to the new resource (e.g., using
the new patient's id) so API consumers receive the URI and the 201 status
(update the createPatient method and any imports accordingly).
- Around line 34-36: The current PatientController.getAllPatients returns all
patients at once (ResponseEntity<List<PatientDTO>>
patientService.getAllPatients()), which must be paginated; change the controller
method signature to accept pagination (either a Pageable parameter or
`@RequestParam` int page, int size), update the controller to call a paginated
service method (e.g., patientService.getPatients(Pageable) or
patientService.getPatients(page, size)) that returns a Page<PatientDTO> (or a
paginated DTO), and return ResponseEntity<Page<PatientDTO>> (or ResponseEntity
with pagination headers and a List body) so the endpoint supports page/size
inputs and limits payloads; also update the PatientService and its repository
calls to use pageable queries (e.g., repository.findAll(Pageable)) and adjust
tests accordingly.
🪄 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: 67e54858-3a93-4452-8806-8f14da99bea9
📒 Files selected for processing (9)
src/main/java/org/example/projektarendehantering/application/service/PatientMapper.javasrc/main/java/org/example/projektarendehantering/application/service/PatientService.javasrc/main/java/org/example/projektarendehantering/common/Actor.javasrc/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientRepository.javasrc/main/java/org/example/projektarendehantering/presentation/dto/PatientDTO.javasrc/main/java/org/example/projektarendehantering/presentation/rest/PatientController.javasrc/test/resources/application.properties
src/main/java/org/example/projektarendehantering/application/service/PatientMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/example/projektarendehantering/presentation/dto/PatientDTO.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.template:
- Around line 3-4: The .env.template currently contains committed OAuth
credentials (GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET) and a trailing semicolon
on GITHUB_CLIENT_ID; remove the real values from the template and replace them
with neutral placeholders (e.g., GITHUB_CLIENT_ID=your_github_client_id and
GITHUB_CLIENT_SECRET=your_github_client_secret) and ensure no trailing
characters on the ID; after replacing the values, revoke and rotate the exposed
GitHub OAuth credentials in GitHub Developer Settings to prevent misuse and
update any runtime config to use secure secrets storage instead of committed
files.
🪄 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: 81246c8b-60cc-47f9-8355-5801eda55665
📒 Files selected for processing (2)
.env.templatesrc/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientEntity.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientEntity.java
.env.template
Outdated
Comment on lines
+3
to
+4
| GITHUB_CLIENT_ID=Ov23liH2WD75Mu6yIMRq; | ||
| GITHUB_CLIENT_SECRET=63c07ed3f40021ed9a877c06ca09be0a6ef55883 |
There was a problem hiding this comment.
Remove committed OAuth credentials from template and rotate them immediately.
GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRET should never be committed with concrete values, even in .env.template. Also, the GITHUB_CLIENT_ID includes a trailing ;, which can invalidate the value at runtime.
🔧 Proposed fix
-GITHUB_CLIENT_ID=Ov23liH2WD75Mu6yIMRq;
-GITHUB_CLIENT_SECRET=63c07ed3f40021ed9a877c06ca09be0a6ef55883
+GITHUB_CLIENT_ID=
+GITHUB_CLIENT_SECRET=Please also revoke/rotate these GitHub OAuth credentials in GitHub Developer Settings, since they are now exposed in git history.
📝 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
| GITHUB_CLIENT_ID=Ov23liH2WD75Mu6yIMRq; | |
| GITHUB_CLIENT_SECRET=63c07ed3f40021ed9a877c06ca09be0a6ef55883 | |
| GITHUB_CLIENT_ID= | |
| GITHUB_CLIENT_SECRET= |
🧰 Tools
🪛 Betterleaks (1.1.1)
[high] 4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.template around lines 3 - 4, The .env.template currently contains
committed OAuth credentials (GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET) and a
trailing semicolon on GITHUB_CLIENT_ID; remove the real values from the template
and replace them with neutral placeholders (e.g.,
GITHUB_CLIENT_ID=your_github_client_id and
GITHUB_CLIENT_SECRET=your_github_client_secret) and ensure no trailing
characters on the ID; after replacing the values, revoke and rotate the exposed
GitHub OAuth credentials in GitHub Developer Settings to prevent misuse and
update any runtime config to use secure secrets storage instead of committed
files.
…licts and removed personnr field
This was referenced Mar 31, 2026
Merged
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
/api/patientsfor creating, retrieving by ID, and listing all patients