Add support for case notes functionality#26
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 23 minutes and 59 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 (1)
📝 WalkthroughWalkthroughAdded CaseNote as a JPA entity with DTO and mapper, wired repository and mapper into service and mapper, extended CaseEntity with a notes relationship, added UI endpoint and template to create/display notes, and adjusted build config for Lombok annotation processing. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser as Browser
participant UiController as UiController
participant CaseService as CaseService
participant CaseRepository as CaseRepository
participant CaseNoteRepository as CaseNoteRepository
participant Database as Database
User->>Browser: Submit note form (content)
Browser->>UiController: POST /ui/cases/{id}/notes (content, principal)
UiController->>CaseService: addNote(caseId, content, author)
CaseService->>CaseRepository: findById(caseId)
CaseRepository->>Database: SELECT case WHERE id=?
Database-->>CaseRepository: CaseEntity
alt case found
CaseService->>CaseService: build CaseNoteEntity(case, content, author, now)
CaseService->>CaseNoteRepository: save(note)
CaseNoteRepository->>Database: INSERT INTO case_notes (...)
Database-->>CaseNoteRepository: success
CaseService-->>UiController: success
else not found
CaseService-->>UiController: throws NOT_FOUND
end
UiController->>Browser: Redirect /ui/cases/{id}
Browser->>User: Show case detail with notes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 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 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 (5)
src/main/java/org/example/projektarendehantering/presentation/web/UiController.java (2)
28-33: Consider failing fast whenprincipalis null instead of falling back to "Anonymous".Per
SecurityConfig(lines 18-33),/ui/cases/**requires authentication (.anyRequest().authenticated()), soprincipalshould never be null here. The "Anonymous" fallback could mask a security misconfiguration. If authentication is mandatory, throwing an exception would surface issues earlier.🛡️ Suggested defensive approach
`@PostMapping`("/ui/cases/{caseId}/notes") public String addNote(`@PathVariable` UUID caseId, `@RequestParam`("content") String content, Principal principal) { - String author = principal != null ? principal.getName() : "Anonymous"; + if (principal == null) { + throw new IllegalStateException("Authentication required"); + } + String author = principal.getName(); caseService.addNote(caseId, content, author); return "redirect:/ui/cases/" + caseId; }🤖 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/web/UiController.java` around lines 28 - 33, The addNote handler should fail fast instead of silently using "Anonymous": in the addNote method check if Principal principal is null and throw a runtime exception (e.g., IllegalStateException or AccessDeniedException) with a clear message indicating authentication is required, then call caseService.addNote(caseId, content, principal.getName()); this surfaces security misconfigurations rather than masking them by falling back to "Anonymous".
29-29: Consider validating thatcontentis not blank.The
@RequestParamaccepts any string including empty/whitespace-only values. The HTMLrequiredattribute provides client-side validation only. Server-side validation would prevent empty notes.✨ Suggested validation
`@PostMapping`("/ui/cases/{caseId}/notes") -public String addNote(`@PathVariable` UUID caseId, `@RequestParam`("content") String content, Principal principal) { +public String addNote(`@PathVariable` UUID caseId, `@RequestParam`("content") `@NotBlank` String content, Principal principal) {Or handle it explicitly:
if (content == null || content.isBlank()) { return "redirect:/ui/cases/" + caseId + "?error=empty"; }🤖 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/web/UiController.java` at line 29, The addNote method accepts the request param content without server-side validation, so update addNote to check that content is not null and not blank (e.g., content == null || content.isBlank()) before creating/saving the note; if the check fails, do not call the note creation flow and instead redirect back to the case view (use the existing caseId) with an error indicator (for example redirect:/ui/cases/{caseId}?error=empty) so empty/whitespace-only notes are rejected server-side.src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.java (1)
24-32: Consider adding column constraints for data integrity.The entity lacks explicit constraints that would enforce data integrity at the database level:
case_idFK should likely benullable = falsecontentandauthormay need length limits andnullable = false💡 Suggested constraints
`@ManyToOne`(fetch = FetchType.LAZY) -@JoinColumn(name = "case_id") +@JoinColumn(name = "case_id", nullable = false) private CaseEntity caseEntity; +@Column(nullable = false, length = 4000) private String content; +@Column(nullable = false, length = 255) private String author; +@Column(nullable = false) private Instant createdAt;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.java` around lines 24 - 32, The CaseNoteEntity is missing DB-level constraints—update the mapping: set `@JoinColumn`(name = "case_id", nullable = false) on the caseEntity relation, add `@Column`(nullable = false, length = 4000) (or appropriate length) on content, add `@Column`(nullable = false, length = 255) on author, and mark createdAt with `@Column`(nullable = false) to enforce non-null timestamps; adjust lengths to suit domain rules and update any tests/migrations accordingly.src/main/java/org/example/projektarendehantering/application/service/CaseService.java (1)
36-48: Consider validatingcontentandauthorbefore persisting.The method accepts
contentandauthorwithout validation. Null or blank values will be persisted, potentially creating invalid case notes. This could lead to confusing UI display or data integrity issues.🛡️ Proposed fix: add input validation
`@Transactional` public void addNote(UUID caseId, String content, String author) { + if (content == null || content.isBlank()) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Note content is required"); + } + if (author == null || author.isBlank()) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Author is required"); + } CaseEntity caseEntity = caseRepository.findById(caseId) .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Case not found")); CaseNoteEntity note = new CaseNoteEntity();🤖 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/CaseService.java` around lines 36 - 48, The addNote method currently persists CaseNoteEntity without validating inputs; before constructing and saving the note (in addNote), validate that content and author are non-null and non-blank (e.g., trim and check length/empty) and reject invalid input by throwing a ResponseStatusException(HttpStatus.BAD_REQUEST, "...") so caseNoteRepository.save(...) never runs with bad data; perform these checks just after resolving caseEntity and before new CaseNoteEntity() and consider enforcing a max length if desired.src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java (1)
64-64: Consider returning an unmodifiable view to preserve encapsulation.
getNotes()returns the internal mutable list. External code can inadvertently modify the collection (e.g.,getNotes().clear()), bypassing theorphanRemovalsemantics intended for managed mutations.♻️ Proposed refactor
-public List<CaseNoteEntity> getNotes() { return notes; } +public List<CaseNoteEntity> getNotes() { return Collections.unmodifiableList(notes); }You'll also need to add the import:
import java.util.Collections;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java` at line 64, Change CaseEntity.getNotes() to return an unmodifiable view of the internal list so external callers cannot mutate the collection and bypass JPA orphanRemoval; specifically, update the CaseEntity#getNotes method to return Collections.unmodifiableList(notes) (or Collections.emptyList() if notes is null) and add the import for java.util.Collections; keep CaseNoteEntity and the existing field/mapping unchanged so managed mutations still go through the entity methods.
🤖 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/infrastructure/persistence/CaseEntity.java`:
- Around line 26-41: The CaseEntity constructor directly assigns the passed-in
notes to the notes field, which can nullify the field initializer and cause
NPEs; update the CaseEntity constructor to perform a null-safe (and preferably
defensive) assignment for notes — e.g. assign this.notes = (notes != null ? new
ArrayList<>(notes) : new ArrayList<>()) — so the notes field is never null;
update references to the notes field in the CaseEntity class accordingly.
---
Nitpick comments:
In
`@src/main/java/org/example/projektarendehantering/application/service/CaseService.java`:
- Around line 36-48: The addNote method currently persists CaseNoteEntity
without validating inputs; before constructing and saving the note (in addNote),
validate that content and author are non-null and non-blank (e.g., trim and
check length/empty) and reject invalid input by throwing a
ResponseStatusException(HttpStatus.BAD_REQUEST, "...") so
caseNoteRepository.save(...) never runs with bad data; perform these checks just
after resolving caseEntity and before new CaseNoteEntity() and consider
enforcing a max length if desired.
In
`@src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java`:
- Line 64: Change CaseEntity.getNotes() to return an unmodifiable view of the
internal list so external callers cannot mutate the collection and bypass JPA
orphanRemoval; specifically, update the CaseEntity#getNotes method to return
Collections.unmodifiableList(notes) (or Collections.emptyList() if notes is
null) and add the import for java.util.Collections; keep CaseNoteEntity and the
existing field/mapping unchanged so managed mutations still go through the
entity methods.
In
`@src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.java`:
- Around line 24-32: The CaseNoteEntity is missing DB-level constraints—update
the mapping: set `@JoinColumn`(name = "case_id", nullable = false) on the
caseEntity relation, add `@Column`(nullable = false, length = 4000) (or
appropriate length) on content, add `@Column`(nullable = false, length = 255) on
author, and mark createdAt with `@Column`(nullable = false) to enforce non-null
timestamps; adjust lengths to suit domain rules and update any tests/migrations
accordingly.
In
`@src/main/java/org/example/projektarendehantering/presentation/web/UiController.java`:
- Around line 28-33: The addNote handler should fail fast instead of silently
using "Anonymous": in the addNote method check if Principal principal is null
and throw a runtime exception (e.g., IllegalStateException or
AccessDeniedException) with a clear message indicating authentication is
required, then call caseService.addNote(caseId, content, principal.getName());
this surfaces security misconfigurations rather than masking them by falling
back to "Anonymous".
- Line 29: The addNote method accepts the request param content without
server-side validation, so update addNote to check that content is not null and
not blank (e.g., content == null || content.isBlank()) before creating/saving
the note; if the check fails, do not call the note creation flow and instead
redirect back to the case view (use the existing caseId) with an error indicator
(for example redirect:/ui/cases/{caseId}?error=empty) so empty/whitespace-only
notes are rejected server-side.
🪄 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: 386edb32-3eba-4bf4-94ef-039a17df0879
📒 Files selected for processing (10)
src/main/java/org/example/projektarendehantering/application/service/CaseMapper.javasrc/main/java/org/example/projektarendehantering/application/service/CaseNoteMapper.javasrc/main/java/org/example/projektarendehantering/application/service/CaseService.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteRepository.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseNoteDTO.javasrc/main/java/org/example/projektarendehantering/presentation/web/UiController.javasrc/main/resources/templates/cases/detail.html
src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/CaseService.java`:
- Around line 44-56: The addNote method in CaseService is missing its closing
brace, causing a compilation error; add a closing '}' immediately after the
caseNoteRepository.save(note); statement to properly terminate the addNote(UUID
caseId, String content, String author) method in the CaseService class so the
class compiles correctly.
- Around line 37-42: The CaseService class has two inconsistent constructors
causing uninitialized fields (employeeRepository or caseNoteRepository) and NPEs
in addNote() or assignUsers()/requireEmployeeWithRole(); replace the two
constructors with a single unified constructor for CaseService that accepts and
assigns all dependencies (CaseRepository, CaseMapper, PatientRepository,
CaseNoteRepository, EmployeeRepository) to their corresponding fields, and then
move the addNote method to follow this unified constructor so all methods see
fully-initialized repositories.
🪄 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: 4360c3d5-b58f-41a1-b9de-adaa9419f2f2
📒 Files selected for processing (3)
src/main/java/org/example/projektarendehantering/application/service/CaseService.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.javasrc/main/java/org/example/projektarendehantering/presentation/web/UiController.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/example/projektarendehantering/presentation/web/UiController.java
- src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java
src/main/java/org/example/projektarendehantering/application/service/CaseService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/example/projektarendehantering/application/service/CaseService.java
Outdated
Show resolved
Hide resolved
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.
closes #17
Summary by CodeRabbit