Feature/manage patients#49
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a manager-only UI for managing patients, carries personalIdentityNumber through DTOs and mapper, enforces validation on patient creation, and stops manually assigning UUIDs relying on entity ID generation. Changes
Sequence Diagram(s)sequenceDiagram
participant Manager as Manager User
participant Browser as Browser
participant Controller as PatientUiController
participant Service as PatientService
participant Mapper as PatientMapper
participant Repo as Database
Manager->>Browser: GET /ui/patients
Browser->>Controller: request
Controller->>Service: getAllPatients()
Service->>Repo: query PatientEntity list
Repo-->>Service: entities
Service->>Mapper: toDTO(entities)
Mapper-->>Controller: PatientDTO list
Controller-->>Browser: render patients/list
Manager->>Browser: GET /ui/patients/new
Browser->>Controller: request
Controller-->>Browser: render patients/new
Manager->>Browser: POST form (PatientCreateDTO)
Browser->>Controller: submit
Controller->>Service: createPatient(dto)
Service->>Service: map to PatientEntity
Service->>Repo: save(entity) (ID auto-generated)
Repo-->>Service: persisted entity
Service-->>Controller: result
Controller-->>Browser: redirect /ui/patients
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (3)
src/main/resources/templates/fragments/header.html (1)
14-19: Consider extracting the repeated role-gate expression.The same
th:ifguard is duplicated for both “Manage Patients” and “Manage Users”. A shared boolean (th:with) would reduce repetition.Refactor example
- <nav class="nav"> + <nav class="nav" + th:with="canManage=${currentActor != null && currentActor.role() != null + && (currentActor.role().name() == 'MANAGER' || currentActor.role().name() == 'ADMIN')}"> <a class="nav-link" href="/ui/cases">Cases</a> <a class="nav-link" href="/ui/cases/new">Create</a> <a class="nav-link" href="/ui/audit">Audit</a> <a class="nav-link" - th:if="${currentActor != null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'}" + th:if="${canManage}" href="/ui/patients">Manage Patients</a> <a class="nav-link" - th:if="${currentActor != null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'}" + th:if="${canManage}" href="/ui/employees">Manage Users</a></nav>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/fragments/header.html` around lines 14 - 19, Extract the duplicated role check expression (th:if="${currentActor != null && currentActor.role() != null && currentActor.role().name() == 'MANAGER'}") into a shared variable using th:with (e.g., define isManager) on a common parent element or wrapper, then replace both anchor tags' th:if with the new boolean (isManager) so the guards for "Manage Patients" and "Manage Users" are centralized and DRY; update the anchors that currently contain the long expression to reference the new symbol instead.src/main/java/org/example/projektarendehantering/presentation/dto/PatientCreateDTO.java (1)
13-14: Add format validation forpersonalIdentityNumber, not only non-blank.On Line 13,
@NotBlankallows invalid values like"abc". Since the form expectsYYYYMMDD-XXXX, add a format constraint to protect data quality at the boundary.Suggested change
import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; @@ - `@NotBlank` + `@NotBlank` + `@Pattern`(regexp = "^\\d{8}-\\d{4}$", message = "Use format YYYYMMDD-XXXX") private String personalIdentityNumber;🤖 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/dto/PatientCreateDTO.java` around lines 13 - 14, The personalIdentityNumber field in PatientCreateDTO currently only has `@NotBlank` and needs a format constraint; add a `@Pattern` annotation to PatientCreateDTO.personalIdentityNumber validating the Swedish national ID format (YYYYMMDD-XXXX) using an appropriate regex (e.g. digits for date, a dash, then four digits) and include a clear validation message; also add the corresponding import for javax.validation.constraints.Pattern if missing.src/main/resources/templates/patients/list.html (1)
27-27: Consider masking personal identity numbers in the list view.On Line 27, full identifier rendering increases unnecessary exposure. Prefer masked display (for example, only last 4 digits) unless full value is explicitly required for this screen.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/patients/list.html` at line 27, Replace the direct rendering of p.personalIdentityNumber in patients/list.html with a masked representation (e.g., show only the last 4 digits and replace preceding characters with asterisks), to avoid exposing full identifiers; implement the masking either by adding a helper/getter on the Patient model (e.g., Patient.getMaskedPersonalIdentityNumber() or getMaskedPin()) and use that in the template, or compute it safely in the template using Thymeleaf string functions while handling null/short values so you never render the full PIN (reference p.personalIdentityNumber and the patients/list.html template).
🤖 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/presentation/web/PatientUiController.java`:
- Around line 24-25: Change the `@PreAuthorize` annotations that currently require
only MANAGER to allow both MANAGER and ADMIN: replace
`@PreAuthorize`("hasRole('MANAGER')") with
`@PreAuthorize`("hasAnyRole('MANAGER','ADMIN')") on the listPatients method in
PatientUiController and on the other two controller methods in the same class
that currently have the same `@PreAuthorize` annotation (the annotations at the
other two locations referenced in the review).
In `@src/main/resources/templates/fragments/header.html`:
- Around line 15-16: The visibility conditional for the "Manage Patients" link
only checks currentActor.role().name() == 'MANAGER'; update the th:if on the
anchor element (which references currentActor, currentActor.role(), and
role().name()) to also allow the 'ADMIN' role (e.g., include ||
currentActor.role().name() == 'ADMIN' or check membership in an allowed-roles
set) so manager and admin users can see the link.
In `@src/main/resources/templates/patients/list.html`:
- Line 10: Replace the hardcoded href on the "New Patient" anchor with a
Thymeleaf URL expression so the link respects the application's context path;
specifically, update the anchor element containing class="button" that currently
uses href="/ui/patients/new" to use th:href="@{/ui/patients/new}" (remove or
replace the static href attribute) so Thymeleaf resolves the route at runtime.
In `@src/main/resources/templates/patients/new.html`:
- Line 28: The personalIdentityNumber input
(th:field="*{personalIdentityNumber}") is missing client-side required
validation; update the <input> element for personalIdentityNumber to include the
required attribute so it matches the other mandatory fields and provides
immediate browser-level validation and consistent UX.
---
Nitpick comments:
In
`@src/main/java/org/example/projektarendehantering/presentation/dto/PatientCreateDTO.java`:
- Around line 13-14: The personalIdentityNumber field in PatientCreateDTO
currently only has `@NotBlank` and needs a format constraint; add a `@Pattern`
annotation to PatientCreateDTO.personalIdentityNumber validating the Swedish
national ID format (YYYYMMDD-XXXX) using an appropriate regex (e.g. digits for
date, a dash, then four digits) and include a clear validation message; also add
the corresponding import for javax.validation.constraints.Pattern if missing.
In `@src/main/resources/templates/fragments/header.html`:
- Around line 14-19: Extract the duplicated role check expression
(th:if="${currentActor != null && currentActor.role() != null &&
currentActor.role().name() == 'MANAGER'}") into a shared variable using th:with
(e.g., define isManager) on a common parent element or wrapper, then replace
both anchor tags' th:if with the new boolean (isManager) so the guards for
"Manage Patients" and "Manage Users" are centralized and DRY; update the anchors
that currently contain the long expression to reference the new symbol instead.
In `@src/main/resources/templates/patients/list.html`:
- Line 27: Replace the direct rendering of p.personalIdentityNumber in
patients/list.html with a masked representation (e.g., show only the last 4
digits and replace preceding characters with asterisks), to avoid exposing full
identifiers; implement the masking either by adding a helper/getter on the
Patient model (e.g., Patient.getMaskedPersonalIdentityNumber() or
getMaskedPin()) and use that in the template, or compute it safely in the
template using Thymeleaf string functions while handling null/short values so
you never render the full PIN (reference p.personalIdentityNumber and the
patients/list.html template).
🪄 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: 10cda056-2934-4b87-bf94-a0ee543224eb
📒 Files selected for processing (8)
src/main/java/org/example/projektarendehantering/application/service/PatientMapper.javasrc/main/java/org/example/projektarendehantering/application/service/PatientService.javasrc/main/java/org/example/projektarendehantering/presentation/dto/PatientCreateDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/PatientDTO.javasrc/main/java/org/example/projektarendehantering/presentation/web/PatientUiController.javasrc/main/resources/templates/fragments/header.htmlsrc/main/resources/templates/patients/list.htmlsrc/main/resources/templates/patients/new.html
… in patient templates
This was referenced Apr 20, 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
2 participants
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.
closes #46
Summary by CodeRabbit