implements #54: implements note service - #86
Conversation
w-richter
left a comment
There was a problem hiding this comment.
PR review — Note Service implementation
|
|
||
| // Specific mappers | ||
|
|
||
| Note toNote(CreateNoteRequest request, long userId); |
There was a problem hiding this comment.
Note toNote(CreateNoteRequest request, long userId) has no explicit @Mapping for userId. Relies on implicit parameter-name matching that breaks silently if the parameter is renamed.
| private final NoteMapper mapper; | ||
|
|
||
| @Override | ||
| public CreateNoteResponse createNote(CreateNoteRequest request, long userId) { |
There was a problem hiding this comment.
Write methods (createNote, updateNote, deleteNote) are not annotated with @Transactional. repository.save() commits before the subsequent mapper call, and entities loaded via getNoteEntity() may detach between findById() and save()/delete().
There was a problem hiding this comment.
I also added @Transactional(readOnly = true) to read methods. Thanks for the heads up
|
|
||
| public record IllegalAccessPair(long ownerId, long noteId) { | ||
|
|
||
| @Override |
There was a problem hiding this comment.
IllegalAccessPair.toString() is dead code. The parent constructor builds its message from pair.noteId and pair.ownerId directly, never calling toString().
| required: | ||
| - title | ||
| - content | ||
| CreateNoteResponse: |
There was a problem hiding this comment.
CreateNoteResponse, GetNoteResponse, and UpdateNoteResponse wrap IdentifiedTimestampedNote in allOf with an empty type: object that adds no constraints.
| private Long userId; | ||
|
|
||
| @Column(nullable = false) | ||
| private String title; |
There was a problem hiding this comment.
The title column is VARCHAR(255) in Liquibase and content is TEXT, but the entity has no Bean Validation annotations (@Size, @NotBlank). A title exceeding 255 characters would produce a database constraint violation (500) instead of a validation error (400). Consider adding @Size(max = 255) on title and @NotBlank on both fields in a follow-up.
| private Long userId; | ||
|
|
||
| @Column(nullable = false) | ||
| private String title; |
There was a problem hiding this comment.
The title column is VARCHAR(255) in Liquibase and content is TEXT, but the entity has no Bean Validation annotations (@Size, @NotBlank). A title exceeding 255 characters would produce a database constraint violation (500) instead of a validation error (400). Consider adding @Size(max = 255) on title and @NotBlank on both fields in a follow-up.
There was a problem hiding this comment.
description should be able to be left blank, in case someone just wants to set some title to a note
|
lgtm now |
Note Service Implementation
This PR implements the core backend features and integration tests for the Note Service, making it ready for production.
Highlights