legal information - #161
Conversation
WalkthroughAdds LegalInformation CRUD support: a domain model, repository, service with admin-guarded operations, GraphQL mutations (create, update, delete), and input DTOs with validation. Mutations pass authorization context to the service and return entity or delete payload. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant GraphQL as MiscMutations (GraphQL)
participant Service as LegalInformationService
participant Repo as LegalInformationRepository
rect rgba(200,235,255,0.3)
note over Client,Repo: Create flow
Client->>GraphQL: createLegalInformation(input)
GraphQL->>Service: createLegalInformation(authCtx, input)
Service->>Service: validate + assertAdmin
Service->>Repo: save(new LegalInformation)
Repo-->>Service: saved entity
Service-->>GraphQL: LegalInformation
GraphQL-->>Client: LegalInformation
end
rect rgba(220,255,220,0.3)
note over Client,Repo: Update flow
Client->>GraphQL: updateLegalInformation(input)
GraphQL->>Service: updateLegalInformation(authCtx, input)
Service->>Service: validate + assertAdmin
Service->>Repo: findById(input.id)
Repo-->>Service: entity
Service->>Service: apply optional fields
Service->>Repo: save(entity)
Repo-->>Service: updated entity
Service-->>GraphQL: LegalInformation
GraphQL-->>Client: LegalInformation
end
rect rgba(255,230,230,0.4)
note over Client,Repo: Delete flow
Client->>GraphQL: deleteLegalInformation(input)
GraphQL->>Service: deleteLegalInformation(authCtx, input)
Service->>Service: validate + assertAdmin
Service->>Repo: findById(input.id)
Repo-->>Service: entity
Service->>Repo: delete(entity)
Service-->>GraphQL: DeleteNodePayload(id)
GraphQL-->>Client: DeleteNodePayload
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
🧹 Nitpick comments (10)
core/src/main/kotlin/gropius/dto/input/misc/CreateLegalInformationInput.kt (2)
8-13: Ensure GraphQL descriptions attach to properties (use @Property target).Without the property target, descriptions may not appear in the schema for Kotlin primary‑ctor properties.
Apply:
- @GraphQLDescription("Initial label of the LegalInformation") + @property:GraphQLDescription("Initial label of the LegalInformation") val label : String, - @GraphQLDescription("Initial text of the LegalInformation") + @property:GraphQLDescription("Initial text of the LegalInformation") val text : String, - @GraphQLDescription("Initial priority of the LegalInformation") + @property:GraphQLDescription("Initial priority of the LegalInformation") val priority : Int
16-24: Validate non‑negative priority.Prevents odd ordering and accidental negative values.
override fun validate() { super.validate() if (label.isBlank()) { throw IllegalArgumentException("Label must not be blank") } if (text.isBlank()) { throw IllegalArgumentException("Text must not be blank") } + if (priority < 0) { + throw IllegalArgumentException("Priority must not be negative") + } }core/src/main/kotlin/gropius/dto/input/misc/UpdateLegalInformationInput.kt (2)
10-15: Use @Property:GraphQLDescription for Kotlin ctor properties.Ensures field docs surface in the GraphQL schema.
- @GraphQLDescription("The new label of the LegalInformation") + @property:GraphQLDescription("The new label of the LegalInformation") val label : OptionalInput<String>, - @GraphQLDescription("The new text of the LegalInformation") + @property:GraphQLDescription("The new text of the LegalInformation") val text: OptionalInput<String>, - @GraphQLDescription("The new priority of the LegalInformation") + @property:GraphQLDescription("The new priority of the LegalInformation") val priority: OptionalInput<Int>
18-30: Validate non‑negative priority when provided.Guards updates that would break ordering semantics.
text.ifPresent { if (it.isBlank()) { throw IllegalArgumentException("Text must not be blank") } } + priority.ifPresent { + if (it < 0) { + throw IllegalArgumentException("Priority must not be negative") + } + }core/src/main/kotlin/gropius/service/misc/LegalInformationService.kt (4)
37-41: Check authorization before validating input.Avoids work and leaking validation errors to unauthorized callers.
- input.validate() - checkIsAdmin(authorizationContext, "create LegalInformation") + checkIsAdmin(authorizationContext, "create LegalInformation") + input.validate() val legalInformation = LegalInformation(input.label, input.text, input.priority) return repository.save(legalInformation).awaitSingle()
39-41: Trim label on create.Prevents labels consisting of leading/trailing whitespace.
- val legalInformation = LegalInformation(input.label, input.text, input.priority) + val legalInformation = LegalInformation(input.label.trim(), input.text, input.priority)
54-67: Check authorization before validating input and trim label on update.Minor ordering improvement plus normalization.
- input.validate() - checkIsAdmin(authorizationContext, "update LegalInformation") + checkIsAdmin(authorizationContext, "update LegalInformation") + input.validate() val legalInformation = repository.findById(input.id) input.label.ifPresent { - legalInformation.label = it + legalInformation.label = it.trim() } input.text.ifPresent { legalInformation.text = it } input.priority.ifPresent { legalInformation.priority = it }
69-83: Consider auditing create/update/delete.Given legal/compliance scope, emit structured audit events (who, when, what changed, old/new values) for these operations.
core/src/main/kotlin/gropius/model/misc/LegalInformation.kt (2)
18-20: Untrusted Markdown: ensure safe rendering downstream.Text will likely be rendered as HTML; the UI must sanitize to prevent XSS.
21-23: Ordering by priority may benefit from an index.If lists grow, add/verify a DB index on priority (and any sort combination you use).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api-public/src/main/kotlin/gropius/schema/mutation/MiscMutations.kt(1 hunks)core/src/main/kotlin/gropius/dto/input/misc/CreateLegalInformationInput.kt(1 hunks)core/src/main/kotlin/gropius/dto/input/misc/UpdateLegalInformationInput.kt(1 hunks)core/src/main/kotlin/gropius/model/misc/LegalInformation.kt(1 hunks)core/src/main/kotlin/gropius/repository/misc/LegalInformationRepository.kt(1 hunks)core/src/main/kotlin/gropius/service/misc/LegalInformationService.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/kotlin/gropius/service/misc/LegalInformationService.kt (1)
core/src/main/kotlin/gropius/service/common/NodeService.kt (1)
checkIsAdmin(57-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
core/src/main/kotlin/gropius/repository/misc/LegalInformationRepository.kt (1)
7-11: LGTM.Repository wiring matches existing patterns and enables CRUD.
api-public/src/main/kotlin/gropius/schema/mutation/MiscMutations.kt (2)
27-52: LGTM: mutations delegate and enforce admin via service.Descriptions and payloads are consistent with existing patterns.
21-23: Confirm reactive transaction semantics with coroutines and REQUIRES_NEW.Ensure a reactive transaction manager is configured and this boundary matches expectations.
chriku
left a comment
chriku
left a comment
There was a problem hiding this comment.
code is okay, but the artificial bunny PR comments suck
3 participants
Summary by CodeRabbit