-
Notifications
You must be signed in to change notification settings - Fork 0
Ari pick #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Ari pick #27
Conversation
WalkthroughAdds a proposal feature: new JPA entities (Proposal, ProposalLike, VO, status enum), domain and persistence repositories (including QueryDSL-backed impl), services (command/query), REST controllers, DTOs, mappers, domain exceptions, API spec and architecture docs, and a .gitignore entry. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CmdCtrl as ProposalCommandController
participant CmdSvc as ProposalCommandService
participant Mapper as ProposalMapper
participant Repo as ProposalRepository
participant DB as Database
Note right of CmdCtrl: Create proposal
Client->>CmdCtrl: POST /api/proposals
CmdCtrl->>CmdSvc: createProposal(request)
CmdSvc->>Mapper: toEntity(request)
Mapper-->>CmdSvc: Proposal
CmdSvc->>Repo: save(proposal)
Repo->>DB: INSERT proposal
DB-->>Repo: persisted Proposal
Repo-->>CmdSvc: Proposal
CmdSvc->>Mapper: toProposalResponse(proposal)
Mapper-->>CmdSvc: ProposalResponse
CmdSvc-->>CmdCtrl: ProposalResponse
CmdCtrl-->>Client: 201 Created + body
sequenceDiagram
participant Client
participant CmdCtrl as ProposalCommandController
participant CmdSvc as ProposalCommandService
participant LikeRepo as ProposalLikeRepository
participant Repo as ProposalRepository
participant DB as Database
participant Err as Error
Note right of CmdCtrl: Like flow
Client->>CmdCtrl: POST /api/proposals/{id}/like?userId=X
CmdCtrl->>CmdSvc: likeProposal(id, userId)
CmdSvc->>Repo: findById(id)
Repo->>DB: SELECT proposal
DB-->>Repo: Proposal
Repo-->>CmdSvc: Proposal
CmdSvc->>LikeRepo: existsByProposalIdAndUserId(id,userId)
alt already liked
CmdSvc->>Err: throw ProposalAlreadyLikedException
Err-->>Client: 409 Conflict
else not liked
CmdSvc->>LikeRepo: save(ProposalLike)
LikeRepo->>DB: INSERT proposal_like
DB-->>LikeRepo: persisted Like
CmdSvc->>Repo: save(proposal.incrementLikeCount())
Repo->>DB: UPDATE proposal.like_count
DB-->>Repo: Proposal
Repo-->>CmdSvc: Proposal
CmdSvc-->>CmdCtrl: ProposalLikeResponse(liked=true, likeCount=N)
CmdCtrl-->>Client: 200 OK + body
end
sequenceDiagram
participant Client
participant QueryCtrl as ProposalQueryController
participant QuerySvc as ProposalQueryService
participant Repo as ProposalRepository
participant Mapper as ProposalMapper
participant DB as Database
Note right of QueryCtrl: List / pagination
Client->>QueryCtrl: GET /api/proposals?status=&page=&size=
QueryCtrl->>QuerySvc: queryProposals(status,page,size)
QuerySvc->>Repo: findAll(status, pageable)
Repo->>DB: SELECT ... WHERE is_deleted=false [AND status=?] ORDER BY created_at DESC LIMIT/OFFSET
DB-->>Repo: rows + total
Repo-->>QuerySvc: Page<Proposal>
loop map results
QuerySvc->>Mapper: toProposalResponse(proposal)
Mapper-->>QuerySvc: ProposalResponse
end
QuerySvc-->>QueryCtrl: ProposalListResponse
QueryCtrl-->>Client: 200 OK + body
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (3)
ari_pick_api_spec.md (1)
13-19: Add language specifiers to all fenced code blocks.All JSON code blocks throughout this specification should include the
jsonlanguage identifier for proper syntax highlighting and tooling support.Apply this pattern to all code blocks:
-``` +```json { "title": "상품명", ... }</blockquote></details> <details> <summary>prompt.md (1)</summary><blockquote> `22-44`: **Consider adding language specifiers for clarity.** The directory structure diagrams could specify `text` as the language identifier for consistency with documentation best practices. </blockquote></details> <details> <summary>src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikeRepository.kt (1)</summary><blockquote> `12-12`: **Consider returning a Boolean or count from the delete method.** The `deleteByProposalIdAndUserId` method returns `Unit`, which means callers cannot distinguish between successful deletion and the case where the like didn't exist. This could complicate unlike flows where you need to verify the operation succeeded. Consider one of these alternatives: ```diff - fun deleteByProposalIdAndUserId(proposalId: Long, userId: Long) + fun deleteByProposalIdAndUserId(proposalId: Long, userId: Long): Int // returns count of deleted rowsOr check existence before deleting in the service layer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.gitignore(1 hunks)ari_pick_api_spec.md(1 hunks)prompt.md(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalCreateRequest.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalStatusUpdateRequest.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalLikeResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalListResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalStatsResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalAlreadyLikedException.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalLikeNotFoundException.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalNotFoundException.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/mapper/ProposalMapper.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/service/ProposalCommandService.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/service/ProposalQueryService.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/Proposal.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/ProposalLike.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/type/ProposalStatus.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/vo/ProposalInfo.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalCommandController.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalQueryController.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikePersistenceRepository.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikeRepository.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikeRepositoryImpl.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalPersistenceRepository.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepository.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepositoryImpl.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/global/error/ErrorMessage.kt(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
prompt.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (20)
src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/type/ProposalStatus.kt (1)
3-7: LGTM!The enum correctly models the proposal lifecycle states and follows the architectural guidelines for domain types.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalStatusUpdateRequest.kt (1)
5-7: LGTM!The request DTO is correctly structured with immutable property and follows the naming conventions outlined in the guidelines.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalStatsResponse.kt (1)
3-8: LGTM!The response DTO correctly uses
Longfor count fields, preventing potential overflow issues with large datasets, and follows architectural conventions.src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalNotFoundException.kt (1)
6-6: LGTM!The exception correctly extends
BusinessBaseExceptionand leverages the centralized error message system, following the architectural guidelines.src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalAlreadyLikedException.kt (1)
6-6: LGTM!The exception correctly extends
BusinessBaseExceptionand maintains consistency with the error handling patterns established in the codebase.src/main/kotlin/bssm/devcoop/occount/global/error/ErrorMessage.kt (1)
16-19: LGTM!The new proposal-related error codes follow the existing pattern with appropriate HTTP status codes and clear Korean messages.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/vo/ProposalInfo.kt (1)
6-16: LGTM!The embeddable value object is well-structured with appropriate JPA annotations. The
reasonfield correctly usescolumnDefinition = "TEXT"for potentially longer content.src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/mapper/ProposalMapper.kt (1)
8-30: LGTM!The mapper functions correctly transform between request/response DTOs and domain entities, properly using the
ProposalInfovalue object for embedded fields.src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/exception/ProposalLikeNotFoundException.kt (1)
6-6: LGTM!The exception class follows the existing pattern and properly integrates with the error handling framework.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalPersistenceRepository.kt (1)
7-8: LGTM!Standard Spring Data JPA repository interface correctly configured for the Proposal entity.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalListResponse.kt (1)
3-8: LGTM!The paginated response structure includes all necessary fields for proper pagination support.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalResponse.kt (1)
6-14: LGTM!The DTO is well-structured with appropriate immutable fields for a response object.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/service/ProposalQueryService.kt (1)
40-52: LGTM!The statistics aggregation is straightforward and correct.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikeRepositoryImpl.kt (1)
7-26: LGTM!Clean delegation pattern that properly separates the domain repository interface from the persistence layer.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalQueryController.kt (1)
22-26: LGTM!The endpoint implementations are clean and correctly delegate to the service layer.
Also applies to: 38-42
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepositoryImpl.kt (2)
26-49: LGTM! QueryDSL pagination implemented correctly.The
findAllmethod properly:
- Filters non-deleted proposals
- Applies optional status filtering
- Orders by
createdAtdescending- Paginates with offset/limit
- Uses a separate count query with matching filters
55-72: LGTM!Count methods correctly filter non-deleted proposals and provide safe defaults for null results.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalLikePersistenceRepository.kt (1)
8-14: LGTM!Spring Data JPA repository correctly defines query methods following naming conventions. Methods will be auto-implemented by Spring Data.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/service/ProposalCommandService.kt (1)
24-28: Mapper-based create flow looks good.Keeping the conversion logic in
ProposalMapperkeeps this service thin and focused on orchestration.src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalCommandController.kt (1)
25-44: HTTP status mapping is on point.The 201/200/204 responses match the semantics of each mutation, which will simplify client handling.
...kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalCreateRequest.kt
Show resolved
Hide resolved
...kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalLikeResponse.kt
Show resolved
Hide resolved
...main/kotlin/bssm/devcoop/occount/domain/proposal/application/service/ProposalQueryService.kt
Show resolved
Hide resolved
src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/ProposalLike.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/ProposalLike.kt
Show resolved
Hide resolved
src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalQueryController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepository.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalCreateRequest.kt (1)
6-14: Consider tightening string validation with@NotBlank.You’ve added length and ID validation, which addresses the earlier concern. If you also want to reject whitespace‑only input (e.g.
" "), consider:import jakarta.validation.constraints.NotBlank data class ProposalCreateRequest( @field:NotBlank(message = "제목은 비어 있을 수 없습니다") @field:Size(min = 1, max = 100, message = "제목은 1자 이상 100자 이하여야 합니다") val title: String, @field:NotBlank(message = "제안 이유는 비어 있을 수 없습니다") @field:Size(min = 1, max = 1000, message = "제안 이유는 1자 이상 1000자 이하여야 합니다") val reason: String, @field:Positive(message = "작성자 ID는 양수여야 합니다") val writerId: Long )If whitespace‑only values are acceptable in your domain, the current version is fine.
src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalQueryController.kt (1)
19-40: Pagination and validation setup look good.Class‑level
@Validatedplus@Min/@Maxonpageandsizeaddress the earlier risk of invalid pagination hittingPageRequest.of. Status is nullable and optional as expected; controller cleanly delegates to the query service.src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/Proposal.kt (1)
17-73: Entity design and optimistic locking for likes look solid.
likeCount: LongwithincrementLikeCount/ guardeddecrementLikeCountis safe against underflow and consistent with response DTOs.- Adding
@Versiongives you optimistic locking, so concurrent like/unlike operations will no longer silently lose updates.As a follow‑up, make sure service code that performs like/unlike either:
- lets
OptimisticLockExceptionbubble to a well‑defined 4xx/5xx error, or- wraps these operations in a simple retry strategy,
so clients get a clear signal rather than an opaque failure if concurrent updates collide.
🧹 Nitpick comments (2)
src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepositoryImpl.kt (1)
26-68: QueryDSL pagination and counts are correct; consider minor DRY helper.
findAllcorrectly filters onisDeleted = falseand optionalstatus, orders bycreatedAt DESC, and uses matching predicates for the count query.countByStatus/countAllfollow the same rules, so paging and totals should stay in sync.If you want to reduce duplication, you could extract a small helper to build the base predicate (e.g.
isDeleted = false+ optional status) and reuse it acrossfindAll,countByStatus, andcountAll, but the current version is already clear.src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalCommandController.kt (1)
47-63: ValidateuserIdand consider deriving it from authentication context.Functionally the like/unlike endpoints are wired correctly, but two improvements are worth considering:
- Input validation for
userIdTo align with
writerIdvalidation on the request side and avoid negative/zero IDs:import jakarta.validation.constraints.Positive @PostMapping("/{proposalId}/like") @ResponseStatus(HttpStatus.OK) fun likeProposal( @PathVariable proposalId: Long, @RequestParam @Positive userId: Long, ): ProposalLikeResponse { ... } @DeleteMapping("/{proposalId}/like") @ResponseStatus(HttpStatus.OK) fun unlikeProposal( @PathVariable proposalId: Long, @RequestParam @Positive userId: Long, ): ProposalLikeResponse { ... }
- Authorization / identity spoofing
If your system uses (or will use) authentication, consider resolving the acting user from the security context (e.g.,
@AuthenticationPrincipal) instead of accepting auserIdquery parameter. Allowing clients to supply arbitrary user IDs makes it easy to like/unlike on behalf of other users unless the service layer enforces additional checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalCreateRequest.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalStatusUpdateRequest.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalLikeResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalResponse.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/Proposal.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/ProposalLike.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalCommandController.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/presentation/ProposalQueryController.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepository.kt(1 hunks)src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepositoryImpl.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/request/ProposalStatusUpdateRequest.kt
- src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalResponse.kt
- src/main/kotlin/bssm/devcoop/occount/domain/proposal/repository/ProposalRepository.kt
- src/main/kotlin/bssm/devcoop/occount/domain/proposal/domain/ProposalLike.kt
🔇 Additional comments (1)
src/main/kotlin/bssm/devcoop/occount/domain/proposal/application/dto/response/ProposalLikeResponse.kt (1)
3-6: DTO looks consistent and future‑proof.Using
LongforlikeCountaligns with stats DTOs and avoids overflow; immutable structure is appropriate for responses.
aripick
Summary by CodeRabbit
New Features
Bug Fixes / UX
Documentation
Chores