[WTH-468] 백엔드 패널티 관리 UI 변경에 따른 수정사항 반영 - #97
Hidden character warning
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
hyxklee
left a comment
There was a problem hiding this comment.
우선 어드민쪽 작업 먼저 리뷰를 했습니당
관련된 리뷰를 보시고, 일단 이번 PR에서 유저쪽은 다음 이슈로 분리해서 진행하는게 좋을 것 같아용
어드민 쪽에 달린 PR이 유저 쪽에서도 해당하는 경우가 많을테니 함께 수정하면서 작업을 해주시면 될 것 같습니당
고생하셨어요! 처음인데 크게 벗어나는 것 없이 작업이 되었네용
| var backgroundImageStorageKey: String? = backgroundImageStorageKey | ||
| private set | ||
|
|
||
| @Column(nullable = false) |
There was a problem hiding this comment.
DB 변경시 resources/db/migration에 마이그레이션 쿼리가 필요합니당
Flyway에 대해서 한 번 Claude와 함께 이야기해보고, 이해한 후에 추가해주세용
There was a problem hiding this comment.
아 확인했습니당 V12__add_club_warning_and_penalty_rule.sql로 파일 추가했습니다!
| var warningEnabled: Boolean = false | ||
| private set | ||
|
|
||
| @Column(length = 500) |
There was a problem hiding this comment.
넹 !! @Column(length = 500, nullable = true)로 수정했습니다!
| @RequestParam(required = false) cardinalNumber: Int?, | ||
| @RequestParam(defaultValue = "0") page: Int, | ||
| @RequestParam(defaultValue = "20") size: Int, | ||
| ): CommonResponse<PageResponse<ClubMemberResponse>> { |
There was a problem hiding this comment.
검색의 경우는 Pagination이 굳이 없어도 될 것 같아요!
추가하신 별도의 의도가 있을까요??
| val score: Int = 1, | ||
| @field:Schema(description = "페널티 사유", example = "정기모임 무단 불참") | ||
| @field:NotBlank | ||
| val penaltyDescription: String, |
There was a problem hiding this comment.
사유가 디자인 스펙에는 몇 자인지 안 나타나있는 것 같네용 한 번 확인해서 반영해주세요!
There was a problem hiding this comment.
피그마 코멘트로 한 번 여쭤보긴 했는데 일단 임의로 50자로 제한해두었습니닷 추후 스펙이 확정되면 수정하겠습니다!
There was a problem hiding this comment.
디자인 상으로는 20자가 적합할 것 같다고 하셔서 20자로 수정해두었습니다!
|
|
||
| val penalty = mapper.toEntity(request, clubMember, cardinal) | ||
| penaltyRepository.save(penalty) | ||
| request.userIds.forEach { targetUserId -> |
There was a problem hiding this comment.
이렇게 내부에서 forEach를 통해 db에서 조회를 하게 되면 request.userIds 만큼 쿼리가 날라갑니당
해당 유스케이스의 경우에는 많은 인원을 선택해서 들어오는 경우는 적을 것 같긴 하지만, 이런 점을 인지하고 BE 개발을 하시면 좋습니당
BE는 항상 DB와 엮여있기 때문이에오
There was a problem hiding this comment.
넵 감사합니다!! 혹시 현재 코드에서 bulk 조회 방식으로 개선이 필요할까용..?? 아니면 이번 PR에서는 인지하는 선에서 넘어가면 될까요??
There was a problem hiding this comment.
우선 주석 정도로 남겨놓고, 학습 후에 다음에 리팩토링을 하는 것으로 합시당
| clubMemberRepository.findByIdWithLock(clubMember.id) | ||
| ?: throw PenaltyNotFoundException() | ||
| lockedMember.incrementPenaltyCount() | ||
| if (request.penaltyType == PenaltyType.WARNING && !clubMember.club.warningEnabled) { |
There was a problem hiding this comment.
반복문 내부에서 경고 활성화 여부를 확인하고 있는 것 같네용
이렇게 되면 clubMember.club을 가져오면서 club을 조회하는 SELECT 문이 반복해서 나가게될 것 같아요 (FetchJoin을 이용하면 clubMember를 조회할 때 club도 같이 조회할 수 있습니다. 이건 FetchJoin, N+1 문제를 찾아보시면 좋아요)
디자인 스펙을 보면 경고/패널티는 한 번의 요청에서 섞이지 않는 것으로 나옵니당 WARGING인 경우는 반복문 밖 (위)에서 처리해주는게 좋아보여요
There was a problem hiding this comment.
아하... 넵! 그럼 clubReader를 통해서 반복문 진입 전에 warningEnabled를 한 번만 확인하도록 수정해두겠습니당!
| class GetPenaltyRuleQueryService( | ||
| private val clubReader: ClubReader, | ||
| ) { | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
쿼리 서비스에서는 클래스에 @transactional(readOnly = true)를 다는 것이 컨벤션입니당
클로드가 못잡아줬나보네요..
There was a problem hiding this comment.
앗 넵 !! 클래스 레벨로 이동했습니다! 하네스도 같이 업데이트해두었는데 한 번 확인 부탁드립니당!
| import org.springframework.web.bind.annotation.RestController | ||
|
|
||
| @Tag(name = "PENALTY", description = "패널티 API") | ||
| @Tag(name = "PENALTY", description = "페널티 API") |
There was a problem hiding this comment.
이번 작업은 어드민쪽 작업까지로 분리하고, 유저쪽은 PR을 나눠서 작업하면 좋을 것 같아용
BE는 FE에 비해 비교적 코드 양이 적기 때문에 500줄 정도면 큰 편으로 생각합니당
베스트는 300줄 이내, 큰 작업의 경우는 500줄 정도로 PR 볼륨을 잡고 작업을 분리하면서 하시면 좋을 것 같아요!
There was a problem hiding this comment.
넵 !! 유저쪽은 별도 PR로 분리하겠습니다! 그럼 현재 브랜치에서 유저 마이페이지 관련 파일들은 되돌리고, 지금 wth-468 어드민 브랜치를 베이스로 한 새 브랜치에 유저 코드만 체리 픽해 PR로 올리고, 어드민 PR이 dev에 머지되면 유저 피알 base를 dev로 변경하겠습니당!!
soo0711
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 👍 👍 처음인데도 백엔드 코드 엄청 잘 구현한 것 같아요 ㅎㅎ 짱짱
아래에 몇가지 피드백 남겼습니당
| ALTER TABLE club | ||
| ADD COLUMN warning_enabled BOOLEAN NOT NULL DEFAULT FALSE, | ||
| ADD COLUMN penalty_rule VARCHAR(500); |
There was a problem hiding this comment.
club_member : warning_count 필드랑 penalty : score 필드도 추가되어야할 것 같아욤!
There was a problem hiding this comment.
앗 구러네요 반영해두겠습니다!! 감사합니당
| class GetPenaltyRuleQueryService( | ||
| private val clubReader: ClubReader, | ||
| ) { |
There was a problem hiding this comment.
이건 컨트롤러에서 사용 되지 않는 것 같은데 요건 어느 곳에서 사용하는 건가용?!
There was a problem hiding this comment.
아 원래 해당 피알에서 유저용 api까지 같이 반영하고 다시 유저용 api 따로 분리하다가 해당 부분이 아직 어드민에 남아있는 것 같아요!! 유저 페널티 규칙 조회 API에서 사용하고 있어서 #98 요기서 확인해보시면 될 것 같아요!!
| @field:Schema(description = "수정할 페널티 사유 (null=변경 안 함)", example = "정기모임 무단 불참 (수정)", nullable = true) | ||
| val penaltyDescription: String?, |
There was a problem hiding this comment.
@field:Size(max = 20) 수정할 때도 20 글자수 제한 들어가야할 것 가타요!
There was a problem hiding this comment.
헙 그러네요... 꼼꼼한 리뷰 감사함니다,, 생각도 못하고 있었어요


📌 Summary
페널티 관리 UI 개편에 맞춰 페널티 관련 API를 전반적으로 수정하고, 마이페이지 및 어드민 페이지에 페널티 관련 기능을 추가합니다.
📝 Changes
1. 페널티 부여 수정
POST /api/v4/admin/clubs/{clubId}/penaltiesuserIds: List<Long>)score: Int, 기본값 1) — 한 번에 여러 점수 부여 가능penaltyDescription: @NotBlank)2. 페널티 수정 추가
PUT /api/v4/admin/clubs/{clubId}/penalties/{penaltyId}ClubMember.penaltyCount/warningCount자동 delta 보정3. 페널티 삭제 수정
DELETE /api/v4/admin/clubs/{clubId}/penalties/{penaltyId}4. 어드민 멤버 페널티 상세 조회 신규
GET /api/v4/admin/clubs/{clubId}/penalties/members/{clubMemberId}5. 페널티 규정 저장 신규
PUT /api/v4/admin/clubs/{clubId}/penalties/rule6. 어드민 멤버 목록에 페널티 정보 추가
GET /api/v4/admin/clubs/{clubId}/memberspenaltyCount,lastPenaltyAt추가7. 마이페이지 stats에 페널티 갯수 추가
GET /api/v4/clubs/{clubId}/users/me/mypagestats.penaltyCount추가8. 마이페이지 페널티 목록 조회 신규
GET /api/v4/clubs/{clubId}/users/me/mypage/penaltiesSliceResponse)9. 마이페이지 페널티 규정 조회 신규
GET /api/v4/clubs/{clubId}/users/me/mypage/penalty-rule10. 한국어 표기 일괄 수정
11. 어드민 멤버 검색 조회 신규
GET /api/v4/admin/clubs/{clubId}/members/search페널티 / 경고 분리 현황
현재
PenaltyTypeenum으로PENALTY/WARNING두 타입이 구분되어 있으며,ClubMember에penaltyCount와warningCount가 별도로 관리됩니다.현재 상태:
Club.warningEnabled의 기본값이false이므로, 프론트에서penaltyType: "WARNING"을 전송하면WarningNotEnabledException이 발생합니다. 경고 타입은 현재 실제로 사용할 수 없습니다.경고 기능 활성화를 위해 필요한 것:
PUT /api/v4/admin/clubs/{clubId}/warning-enabled예정)warningEnabled = true설정 후 프론트에서penaltyType: "WARNING"전송 시warningCount증가테스트 항목
📸 Screenshots / Logs
💡 Reviewer 참고사항
슬랙에도 적어뒀는데 페널티에서 멤버 검색을 할때 해당 기수 전체 멤버 검색 api가 필요할 것 같습니다! 페널티 도메인보다 어드민 멤버 목록 API에 검색 파라미터 추가하거나 따로 공통 검색 API를 분리하는게 나을 것 같은데 어떻게 생각하시나욤...?? 의견 주시면 해당 피알이나 다음 피알에서 함께 반영해보겠습니닷...!!-> 구현 완료✅ Checklist