[WTH-198] 동아리 개설 가입시 기수 입력 및 초기화#28
Conversation
📝 WalkthroughWalkthrough클럽 생성 시 초기 활동 기수(currentCardinal)를 자동 생성하고, 회원이 자신의 초기 활동 기수 목록을 설정할 수 있는 엔드포인트와 관련 비즈니스 로직(중복/존재 검사, 락 조회, 출석 초기화 등)을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ClubController
participant UseCase as ManageClubMemberUsecase
participant Policy as ClubMemberPolicy
participant CardinalReader as CardinalReader
participant MemberCardinalRepo as ClubMemberCardinalRepository
participant SessionReader as SessionReader
participant AttendanceRepo as AttendanceRepository
Client->>Controller: POST /clubs/{clubId}/members/me/cardinals\n(ClubMemberCardinalSetRequest)
Controller->>UseCase: setInitialCardinals(clubId, userId, request)
UseCase->>Policy: getActiveMemberWithLock(clubId, userId)
Policy-->>UseCase: ClubMember (locked)
alt Member already has cardinals
UseCase-->>Controller: throw CardinalAlreadySetException
else proceed
UseCase->>CardinalReader: validate requested cardinals exist
CardinalReader-->>UseCase: Cardinals found
UseCase->>MemberCardinalRepo: saveAll(ClubMemberCardinals)
MemberCardinalRepo-->>UseCase: saved
UseCase->>SessionReader: find sessions by cardinals
SessionReader-->>UseCase: sessions list
UseCase->>AttendanceRepo: saveAll(Attendances)
AttendanceRepo-->>UseCase: saved
UseCase-->>Controller: success
end
Controller-->>Client: 201 MEMBER_CARDINAL_SET_SUCCESS
sequenceDiagram
participant Client
participant Controller as ClubController
participant UseCase as ManageClubUseCase
participant ClubRepo as ClubRepository
participant MemberRepo as ClubMemberRepository
participant CardinalRepo as CardinalRepository
participant MemberCardinalRepo as ClubMemberCardinalRepository
Client->>Controller: POST /clubs (ClubCreateRequest with currentCardinal)
Controller->>UseCase: create(request)
UseCase->>ClubRepo: save(Club)
ClubRepo-->>UseCase: Club created
UseCase->>MemberRepo: save(ClubMember - LEAD)
MemberRepo-->>UseCase: LEAD member saved
loop i from 1 to currentCardinal
alt i < currentCardinal
UseCase->>CardinalRepo: save(Cardinal number i, status DONE)
else i == currentCardinal
UseCase->>CardinalRepo: save(Cardinal number i, status IN_PROGRESS)
end
end
CardinalRepo-->>UseCase: all Cardinals saved
UseCase->>MemberCardinalRepo: save(ClubMemberCardinal LEAD→latest)
MemberCardinalRepo-->>UseCase: saved
UseCase-->>Controller: created with initialized cardinals
Controller-->>Client: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75분 Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/kotlin/com/weeth/domain/club/application/dto/request/ClubCreateRequest.kt (1)
17-25: TODO 항목 처리 및 검증 불일치 확인 필요Line 23의 TODO는
contactPhoneNumber가 필수라고 명시하지만, 실제 구현은String? = null로 선택 필드입니다. 의도된 동작과 불일치합니다.🔧 contactPhoneNumber를 필수로 변경하는 수정안
- // TODO: 얘는 필수 - `@field`:Schema(description = "연락 전화번호", example = "010-1234-5678") - val contactPhoneNumber: String? = null, + `@field`:Schema(description = "연락 전화번호", example = "010-1234-5678") + `@field`:NotBlank + val contactPhoneNumber: String,TODO 항목들(길이 제한 추가, 필수/선택 필드 검증)을 구현하는 코드를 생성하거나, 별도 이슈로 등록하는 것을 도와드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/club/application/dto/request/ClubCreateRequest.kt` around lines 17 - 25, The TODOs indicate missing validation and a mismatch for contactPhoneNumber: change contactPhoneNumber from nullable String? = null to a non-null String and apply appropriate validation annotations (e.g., `@field`:NotBlank / `@field`:NotNull and a `@field`:Pattern or `@field`:Size for phone format/length) on contactPhoneNumber in ClubCreateRequest; also add length/size constraints for description and contactEmail (e.g., `@field`:Size(max=...) and `@field`:Email for contactEmail) so all three fields (description, contactEmail, contactPhoneNumber) enforce the intended required/optional and length rules at DTO level.src/main/kotlin/com/weeth/domain/club/application/usecase/command/AdminClubMemberUseCase.kt (1)
67-97: TODO 주석은 적절하나, 경쟁 조건(race condition)의 실제 심각도를 고려해야 합니다.
setInitialCardinals는getActiveMemberWithLock을 사용하여 비관적 락을 획득하지만,applyOb는getMemberInClub을 사용하여 락 없이 멤버를 조회합니다. 동일 멤버에 대해 두 메서드가 동시에 실행되면initializeAttendances가 중복 호출되어 중복 출석 레코드가 생성될 수 있습니다.MVP 이후 개선 시,
applyOb에서도 멤버 단위 락을 획득하거나,Attendance엔티티에(session_id, club_member_id)유니크 제약을 추가하는 방안을 검토하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/club/application/usecase/command/AdminClubMemberUseCase.kt` around lines 67 - 97, applyOb currently fetches members without a lock (using getMemberInClub) which allows a race with setInitialCardinals (which uses getActiveMemberWithLock) and can cause initializeAttendances to run twice; fix by fetching the member under the same pessimistic/member-level lock (replace getMemberInClub with getActiveMemberWithLock or call the existing locking method before mutate operations in applyOb) so duplicate attendance creation is prevented, and/or add a DB-level unique constraint on Attendance (session_id, club_member_id) to enforce idempotency as a safety net; ensure applyOb remains `@Transactional` and handle any lock-related exceptions consistently.
🤖 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/kotlin/com/weeth/domain/club/domain/repository/ClubMemberRepository.kt`:
- Around line 35-41: The query in findByClubIdAndUserIdWithLock can return
multiple rows because it lacks a memberStatus filter and also doesn't fetch the
user like findByIdWithLock; update the method and its `@Query` to include a
memberStatus condition (e.g., AND cm.memberStatus = :memberStatus) and add JOIN
FETCH cm.user so the locked entity includes the user, and add a corresponding
method parameter `@Param`("memberStatus") memberStatus: MemberStatus to ensure a
single, eagerly-fetched result under the pessimistic lock.
In `@src/main/kotlin/com/weeth/domain/club/presentation/ClubController.kt`:
- Line 26: Remove the unused import statement for PathVariable from
ClubController.kt: delete the line importing
org.springframework.web.bind.annotation.PathVariable since ClubController (the
class in this file) does not reference PathVariable anywhere; ensure no other
references to PathVariable remain before committing.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/club/application/dto/request/ClubCreateRequest.kt`:
- Around line 17-25: The TODOs indicate missing validation and a mismatch for
contactPhoneNumber: change contactPhoneNumber from nullable String? = null to a
non-null String and apply appropriate validation annotations (e.g.,
`@field`:NotBlank / `@field`:NotNull and a `@field`:Pattern or `@field`:Size for phone
format/length) on contactPhoneNumber in ClubCreateRequest; also add length/size
constraints for description and contactEmail (e.g., `@field`:Size(max=...) and
`@field`:Email for contactEmail) so all three fields (description, contactEmail,
contactPhoneNumber) enforce the intended required/optional and length rules at
DTO level.
In
`@src/main/kotlin/com/weeth/domain/club/application/usecase/command/AdminClubMemberUseCase.kt`:
- Around line 67-97: applyOb currently fetches members without a lock (using
getMemberInClub) which allows a race with setInitialCardinals (which uses
getActiveMemberWithLock) and can cause initializeAttendances to run twice; fix
by fetching the member under the same pessimistic/member-level lock (replace
getMemberInClub with getActiveMemberWithLock or call the existing locking method
before mutate operations in applyOb) so duplicate attendance creation is
prevented, and/or add a DB-level unique constraint on Attendance (session_id,
club_member_id) to enforce idempotency as a safety net; ensure applyOb remains
`@Transactional` and handle any lock-related exceptions consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4456792-859a-44cf-9c22-e34167dbc4fa
📒 Files selected for processing (15)
.claude/rules/api-design.mdsrc/main/kotlin/com/weeth/domain/club/application/dto/request/ClubCreateRequest.ktsrc/main/kotlin/com/weeth/domain/club/application/dto/request/ClubMemberCardinalSetRequest.ktsrc/main/kotlin/com/weeth/domain/club/application/exception/CardinalAlreadySetException.ktsrc/main/kotlin/com/weeth/domain/club/application/exception/ClubErrorCode.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/AdminClubMemberUseCase.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.ktsrc/main/kotlin/com/weeth/domain/club/domain/repository/ClubMemberReader.ktsrc/main/kotlin/com/weeth/domain/club/domain/repository/ClubMemberRepository.ktsrc/main/kotlin/com/weeth/domain/club/domain/service/ClubMemberPolicy.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubController.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubResponseCode.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt
src/main/kotlin/com/weeth/domain/club/domain/repository/ClubMemberRepository.kt
Show resolved
Hide resolved
src/main/kotlin/com/weeth/domain/club/presentation/ClubController.kt
Outdated
Show resolved
Hide resolved
soo0711
left a comment
There was a problem hiding this comment.
수고하셨습니다~~! 코멘트 하나만 확인 부탁드립니당!
| if (clubMemberCardinalRepository.findAllByClubMember(member).isNotEmpty()) { | ||
| throw CardinalAlreadySetException() | ||
| } |
There was a problem hiding this comment.
find는 모든 레코드를 가져오니까 존재여부를 확인 하기 위해서 existsByClubMember를 사용하는 건 어떨까요???
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt (1)
99-103: 기수 조회 시 N+1 쿼리 패턴 고려현재
request.cardinals의 각 요소에 대해 개별적으로findByClubIdAndCardinalNumber를 호출하고 있습니다. 기수 수가 적다면 큰 문제가 되지 않지만, 향후 요청 기수 수가 많아질 경우findAllByClubIdAndCardinalNumberIn(clubId, numbers)같은 배치 조회로 개선을 고려해볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt` around lines 99 - 103, 현재 ManageClubMemberUsecase의 request.cardinals를 순회하며 cardinalReader.findByClubIdAndCardinalNumber를 개별 호출해 N+1 쿼리 패턴을 유발하므로, request.cardinals.distinct()로 얻은 숫자 목록을 한 번에 조회하는 배치 메서드(예: cardinalReader.findAllByClubIdAndCardinalNumberIn(clubId, numbers) 또는 repository의 유사 메서드)를 구현·사용하도록 변경하고, 반환된 엔티티들을 cardinal number 기준으로 매핑하여 누락된 항목은 CardinalNotFoundException으로 처리하도록 수정하세요 (참조 심볼: request.cardinals, cardinalReader.findByClubIdAndCardinalNumber, cardinalReader.findAllByClubIdAndCardinalNumberIn).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt`:
- Around line 99-103: 현재 ManageClubMemberUsecase의 request.cardinals를 순회하며
cardinalReader.findByClubIdAndCardinalNumber를 개별 호출해 N+1 쿼리 패턴을 유발하므로,
request.cardinals.distinct()로 얻은 숫자 목록을 한 번에 조회하는 배치 메서드(예:
cardinalReader.findAllByClubIdAndCardinalNumberIn(clubId, numbers) 또는
repository의 유사 메서드)를 구현·사용하도록 변경하고, 반환된 엔티티들을 cardinal number 기준으로 매핑하여 누락된 항목은
CardinalNotFoundException으로 처리하도록 수정하세요 (참조 심볼: request.cardinals,
cardinalReader.findByClubIdAndCardinalNumber,
cardinalReader.findAllByClubIdAndCardinalNumberIn).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dad8459e-f337-4083-944d-462de5e9f9f0
📒 Files selected for processing (4)
src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.ktsrc/main/kotlin/com/weeth/domain/club/domain/repository/ClubMemberCardinalRepository.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubController.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUseCaseTest.kt
📌 Summary
동아리 개설시 가장 최신 기수를 입력받아 1기부터 초기화하는 로직을 추가했습니다.
동아리 부원이 본인의 활동 기수를 마이페이지에서 설정한 경우 기수를 저장하고, 출석을 초기화하는 로직을 추가했습니다.
📝 Changes
What
""
Why
동아리 별 기수 관리를 명확하게 하기 위함
How
동아리 개설 Form에 최신 기수 필드 추가 및 1부터 반복적으로 기수를 초기화하는 로직 추가
동아리 부원 활동정보 저장시 기수를 저장하고, 출석을 초기화하는 로직 추가 - 이때 출석 정보는 이미 0 0 0 일 것이므로 명시적으로 출석 정보를 초기화하진 않음 (기수 입력은 기능 사용을 위해 필수기 때문)
📸 Screenshots / Logs
💡 Reviewer 참고사항
getActiveMemberWithLock 메서드는 WTH-200 이슈에서 사용하고, 사용처 등록할 예정입니당
프론트 분들은 추가된 API 확인하고, 입력값 확인해주세옹
또한 기수 정보도 활동에 필수입니다. 모든 기능을 사용할 때 필수이므로 지연입력 될 수 있도록 처리해주세오
지연입력시 필요한 정보, API가 있다면 금일 회의 때 말해주세용 (프로필 정보가 완료되었음을 프론트에서 확인할 수 있는 방법)
✅ Checklist
Summary by CodeRabbit
새로운 기능
버그 수정
테스트