[WTH-431] 대시보드 기수 기반으로 변경#87
Merged
Hidden character warning
The head ref may contain hidden characters: "refactor/WTH-431-\ub300\uc2dc\ubcf4\ub4dc-\uc870\ud68c-API-\uae30\uc218-\uae30\ubc18\uc73c\ub85c-\ubcc0\uacbd\ud558\uae30"
Merged
Conversation
📝 WalkthroughWalkthroughAccount 엔티티에 status 기반 isActive 파생 프로퍼티를 추가하고, 여러 커맨드/쿼리 서비스가 상태 비교 대신 isActive를 검사하여 비활성 계정에 대해 AccountNotActiveException을 던지도록 변경했다. 대시보드 조회는 accountId 대신 clubId+cardinal로 계정을 조회하도록 변경되었고, 응답에 accountId 필드가 추가되었다. Changes계정 활성 검증 및 대시보드 조회 변경
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AccountManageController
participant GetAccountDashboardQueryService
participant AccountRepository
participant Account
AccountManageController->>GetAccountDashboardQueryService: getDashboard(clubId, cardinal, userId)
GetAccountDashboardQueryService->>AccountRepository: findByClubIdAndCardinal(clubId, cardinal)
AccountRepository-->>GetAccountDashboardQueryService: Account 반환
GetAccountDashboardQueryService->>Account: validateOwnedBy(userId)
GetAccountDashboardQueryService->>Account: isActive 확인
alt 비활성
GetAccountDashboardQueryService-->>AccountManageController: AccountNotActiveException
else 활성
GetAccountDashboardQueryService-->>AccountManageController: AccountDashboardResponse(accountId 포함)
end
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt (1)
24-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win비활성 계정 "숨김" 처리까지 차단하는 회귀 가능성
isActive가드가visible분기 이전에 위치해visible=false(숨김) 요청도 예외로 막습니다. 기존에는hideFromMembers()에 활성 상태 검증이 없어 비활성 계정도 숨김 처리가 가능했는데, 이제는 아예 시도조차 못 하게 됩니다. 계정이 비활성화된 후 관리자가 노출을 끄려는 정상 시나리오를 막을 수 있습니다.가드를
visible=true(공개) 분기로 한정하는 것이 의도에 맞아 보입니다.🐛 제안 수정안
clubPermissionPolicy.requireAdmin(clubId, userId) val account = getAccountWithLock(clubId, accountId) - if (!account.isActive) throw AccountNotActiveException() if (visible) { + if (!account.isActive) throw AccountNotActiveException() account.showToMembers() } else { account.hideFromMembers() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt` around lines 24 - 35, The active-state guard in ManageAccountUseCase.apply currently blocks both show and hide paths, but the regression only affects the hide flow. Move the AccountNotActiveException check so it applies only when visible is true, and leave the visible=false branch able to call hideFromMembers() for inactive accounts; use the visible condition in the ManageAccountUseCase method and keep markModifiedBy(userId) unchanged.
🧹 Nitpick comments (2)
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
isActive비활성 검증 로직 3곳 중복동일한
if (!account.isActive) throw AccountNotActiveException()패턴이 이 파일 외GetAccountPaymentTargetQueryService.findPaymentStatus,GetAccountDashboardQueryService.getDashboard에도 그대로 반복됩니다. 공용 확장 함수(예:Account.requireActive())로 추출하면 중복을 줄일 수 있습니다.Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt` at line 3, The `if (!account.isActive) throw AccountNotActiveException()` check is duplicated across `ManageAccountUseCase` and the other account services mentioned in the review. Extract this validation into a shared helper or extension such as `Account.requireActive()` and replace each inline check in `ManageAccountUseCase`, `GetAccountPaymentTargetQueryService.findPaymentStatus`, and `GetAccountDashboardQueryService.getDashboard` with the common method.src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountPaymentUseCase.kt (1)
128-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
isActive검증 로직 중복 - 공통화 검토이
getActiveAccountWithLockprivate 함수는ManageAccountTransactionUseCase.kt(107-117)와 완전히 동일하며,if (!account.isActive) throw AccountNotActiveException()검증 자체도GetAccountTransactionQueryService.kt(132),GetAccountDashboardQueryService.kt등 최소 4~5곳에 반복되고 있습니다. Account 엔티티에 이미validateOwnedBy(clubId)같은 도메인 검증 메서드가 존재하므로, 활성 검증도 엔티티/도메인 서비스 메서드(예:account.requireActive()) 또는 공통 리더 컴포넌트로 추출해 각 UseCase/QueryService는 호출만 하도록 정리하는 것을 권장합니다.♻️ 제안 방향 예시
- private fun getActiveAccountWithLock( - clubId: Long, - accountId: Long, - ): Account { - val account = accountRepository.findByIdWithLock(accountId) ?: throw AccountNotFoundException() - account.validateOwnedBy(clubId) - if (!account.isActive) throw AccountNotActiveException() - return account - } + private fun getActiveAccountWithLock( + clubId: Long, + accountId: Long, + ): Account { + val account = accountRepository.findByIdWithLock(accountId) ?: throw AccountNotFoundException() + account.validateOwnedBy(clubId) + account.validateActive() // 공통 도메인 검증 메서드로 이동 + return account + }As per coding guidelines,
**/*UseCase.kt: UseCase classes orchestrate only; business rules belong in entities or domain services.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountPaymentUseCase.kt` around lines 128 - 136, The active-account check is duplicated across multiple use cases/query services, and `ManageAccountPaymentUseCase.getActiveAccountWithLock` repeats the same `if (!account.isActive) throw AccountNotActiveException()` logic seen elsewhere. Move the “active” business rule into a shared domain-level method on `Account` such as `requireActive()` or into a common domain service, then update `getActiveAccountWithLock` and the other duplicated call sites to use that shared validation instead of inlining the check.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt`:
- Around line 24-35: The active-state guard in ManageAccountUseCase.apply
currently blocks both show and hide paths, but the regression only affects the
hide flow. Move the AccountNotActiveException check so it applies only when
visible is true, and leave the visible=false branch able to call
hideFromMembers() for inactive accounts; use the visible condition in the
ManageAccountUseCase method and keep markModifiedBy(userId) unchanged.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountPaymentUseCase.kt`:
- Around line 128-136: The active-account check is duplicated across multiple
use cases/query services, and
`ManageAccountPaymentUseCase.getActiveAccountWithLock` repeats the same `if
(!account.isActive) throw AccountNotActiveException()` logic seen elsewhere.
Move the “active” business rule into a shared domain-level method on `Account`
such as `requireActive()` or into a common domain service, then update
`getActiveAccountWithLock` and the other duplicated call sites to use that
shared validation instead of inlining the check.
In
`@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt`:
- Line 3: The `if (!account.isActive) throw AccountNotActiveException()` check
is duplicated across `ManageAccountUseCase` and the other account services
mentioned in the review. Extract this validation into a shared helper or
extension such as `Account.requireActive()` and replace each inline check in
`ManageAccountUseCase`, `GetAccountPaymentTargetQueryService.findPaymentStatus`,
and `GetAccountDashboardQueryService.getDashboard` with the common method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efdbf5c7-72cf-4fd5-a33f-47a83f5bc30b
📒 Files selected for processing (14)
src/main/kotlin/com/weeth/domain/account/application/dto/response/AccountDashboardResponse.ktsrc/main/kotlin/com/weeth/domain/account/application/mapper/AccountDashboardMapper.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountPaymentUseCase.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountTransactionUseCase.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountDashboardQueryService.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountPaymentTargetQueryService.ktsrc/main/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountTransactionQueryService.ktsrc/main/kotlin/com/weeth/domain/account/domain/entity/Account.ktsrc/main/kotlin/com/weeth/domain/account/presentation/AccountManageController.ktsrc/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountDashboardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountPaymentTargetQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/account/application/usecase/query/GetAccountTransactionQueryServiceTest.kt
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.
📌 Summary
대시보드 조회 시 기수를 기반으로 조회하도록 수정했습니다.
📝 Changes
What
대시보드 조회 시 accountId가 아닌 기수를 기반으로 조회하도록 수정했습니다.
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
대시보드에서 조회한 accountId로 타 API들 사용할 때 쓰면 될 것 같아용
✅ Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests