Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughGoogle Sheets 동기화가 기존의 동아리 회원에 더해 사전 회원을 포함하도록 확장되었습니다. 리포지토리(countByClubId), 서비스(총합 계산), 실행자(통합 행 매핑·정렬·쓰기)와 관련 테스트 및 Swagger 문서가 업데이트되었습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
actor User
participant ClubMemberSheetService
participant ClubMemberRepository
participant ClubPreMemberRepository
participant SheetSyncExecutor
participant GoogleSheetsAPI as "Google Sheets API"
User->>ClubMemberSheetService: syncMembersToSheet(clubId, sortKey, includePreMembers)
ClubMemberSheetService->>ClubMemberRepository: count(clubId)
ClubMemberRepository-->>ClubMemberSheetService: memberCount
ClubMemberSheetService->>ClubPreMemberRepository: countByClubId(clubId)
ClubPreMemberRepository-->>ClubMemberSheetService: preMemberCount
Note over ClubMemberSheetService: totalCount = memberCount + preMemberCount
ClubMemberSheetService->>SheetSyncExecutor: executeWithSort(clubId, sortKey, includePreMembers)
SheetSyncExecutor->>ClubMemberRepository: findAllByClubId(clubId)
ClubMemberRepository-->>SheetSyncExecutor: clubMembers
SheetSyncExecutor->>ClubPreMemberRepository: findAllByClubId(clubId)
ClubPreMemberRepository-->>SheetSyncExecutor: preMembers
Note over SheetSyncExecutor: map -> SheetSyncRow[] -> sort -> build ValueRange
SheetSyncExecutor->>GoogleSheetsAPI: clear(spreadsheetId) + update(values)
GoogleSheetsAPI-->>SheetSyncExecutor: updateResponse
SheetSyncExecutor-->>ClubMemberSheetService: success
ClubMemberSheetService-->>User: ClubMemberSheetSyncResponse(totalCount, sheetUrl)
예상 코드 리뷰 난이도🎯 4 (Complex) | ⏱️ ~45 minutes 관련 PR
개요Google Sheets 동기화 기능을 확장하여 일반 동아리 회원("동아리 회원")에 더하여 사전 회원("사전 회원 인명부")도 포함하도록 업데이트했습니다. 리포지토리, 서비스, 실행자 클래스에서 사전 회원을 처리하기 위한 로직을 추가하고, 관련 Swagger 문서를 수정하였으며, 새로운 기능에 대한 포괄적인 테스트를 추가했습니다. 변경 사항 (요약 한국어)
시 🐰
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Pull request overview
POST /clubs/{clubId}/members/sheet-sync 실행 시 시트에 **사전 회원(ClubPreMember)**까지 함께 기록되어, 동기화 과정에서 사전 회원 인명부가 사라지던 문제를 해결하는 PR입니다.
Changes:
SheetSyncExecutor가club_member+club_pre_member를 통합(Row 모델로 변환)해 정렬/기록하도록 변경ClubMemberSheetService동기화 응답의syncedMemberCount에 사전 회원 수를 합산하도록 보정- 사전 회원 카운트용
ClubPreMemberRepository.countByClubId추가 및 Swagger 문구/테스트 보강
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | 회원/사전회원 통합 Row로 변환 후 동일 정렬 규칙으로 시트에 기록하도록 수정 |
| src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java | 응답 카운트에 사전 회원 수를 합산하도록 변경 |
| src/main/java/gg/agit/konect/domain/club/repository/ClubPreMemberRepository.java | 사전 회원 수 집계를 위한 countByClubId 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncResponse.java | syncedMemberCount 설명을 “회원 및 사전 회원” 기준으로 정리 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.java | 시트 동기화 API 설명을 실제 동작(회원+사전회원)과 일치시킴 |
| src/test/java/gg/agit/konect/domain/club/service/SheetSyncExecutorTest.java | 회원+사전회원이 함께 시트에 기록되는 케이스 테스트 추가 |
| src/test/java/gg/agit/konect/domain/club/service/ClubMemberSheetServiceTest.java | 응답 카운트 보정(회원+사전회원 합산) 검증 테스트 추가 |
| sheetSyncExecutor.executeWithSort(clubId, sortKey, ascending); | ||
|
|
||
| return ClubMemberSheetSyncResponse.of((int)memberCount, spreadsheetId); | ||
| return ClubMemberSheetSyncResponse.of((int)(memberCount + preMemberCount), spreadsheetId); |
There was a problem hiding this comment.
memberCount/preMemberCount가 long인데 (int)(memberCount + preMemberCount)로 캐스팅하면 범위 초과 시 값이 잘려서 syncedMemberCount가 잘못될 수 있습니다. 최소한 Math.toIntExact(memberCount + preMemberCount)로 변환해 오버플로우를 즉시 감지하거나, 응답 DTO 타입을 long으로 올려 안전하게 처리하는 쪽을 검토해 주세요.
| return ClubMemberSheetSyncResponse.of((int)(memberCount + preMemberCount), spreadsheetId); | |
| return ClubMemberSheetSyncResponse.of(Math.toIntExact(memberCount + preMemberCount), spreadsheetId); |
| } | ||
|
|
||
| private void setCreatedAt(Object target, LocalDateTime createdAt) throws Exception { | ||
| Field createdAtField = target.getClass().getSuperclass().getDeclaredField("createdAt"); |
There was a problem hiding this comment.
테스트의 setCreatedAt()이 target.getClass().getSuperclass()에 createdAt 필드가 있다고 가정하고 있어(직접 상속 구조가 바뀌면 즉시 깨짐) 리팩터링에 취약합니다. BaseEntity.class에서 필드를 가져오거나, 상위 클래스를 따라 올라가며 createdAt을 찾는 방식으로 의존을 줄이는 편이 안전합니다.
| Field createdAtField = target.getClass().getSuperclass().getDeclaredField("createdAt"); | |
| Class<?> current = target.getClass(); | |
| Field createdAtField = null; | |
| while (current != null) { | |
| try { | |
| createdAtField = current.getDeclaredField("createdAt"); | |
| break; | |
| } catch (NoSuchFieldException e) { | |
| current = current.getSuperclass(); | |
| } | |
| } | |
| if (createdAtField == null) { | |
| throw new NoSuchFieldException( | |
| "Field 'createdAt' not found in class hierarchy of " + target.getClass() | |
| ); | |
| } |
| @@ -80,7 +84,12 @@ public void executeWithSort(Integer clubId, ClubSheetSortKey sortKey, boolean as | |||
| clearAndWriteAll(spreadsheetId, sorted); | |||
| applyFormat(spreadsheetId); | |||
| } | |||
There was a problem hiding this comment.
현재 추가된 테스트는 club.getSheetColumnMapping()이 null인 경우(전체 clear 후 values.update로 덮어쓰기)만 검증합니다. 실제 운영에서는 시트 등록 시 sheetColumnMapping이 저장되어 updateMappedColumns(values.batchUpdate 경로)로 동작하는 경우가 많으니, 해당 경로에서도 사전 회원이 함께 반영되는지(예: BatchUpdateValuesRequest에 preMember row가 포함되는지) 테스트를 추가하는 것이 좋겠습니다.
|
|
||
| public record ClubMemberSheetSyncResponse( | ||
| @Schema(description = "동기화된 회원 수", example = "42") | ||
| @Schema(description = "동기화된 회원 및 사전 회원 수", example = "42") |
There was a problem hiding this comment.
syncMembersToSheet는 @async로 시트 동기화를 비동기로 실행하지만, 응답 필드/Swagger 설명은 '동기화된' 것으로 표기되어 실제로는 '요청 시점 기준 동기화 대상(또는 예상) 인원'에 가깝습니다. 비동기 실행 사실을 반영해 description 문구를 조정하거나(예: "동기화 요청된 회원 및 사전 회원 수"), 동기화 완료 후의 결과를 반환하도록 동작을 맞추는 쪽 중 하나로 정합성을 맞춰주세요.
| @Schema(description = "동기화된 회원 및 사전 회원 수", example = "42") | |
| @Schema(description = "동기화 요청된 회원 및 사전 회원 수", example = "42") |
🔍 개요
POST /clubs/{clubId}/members/sheet-sync실행 시club_member만 기준으로 시트를 다시 써서, 기존club_pre_member인명부가 시트에서 사라지던 문제를 수정했습니다.🚀 주요 변경 내용
SheetSyncExecutor가club_member와club_pre_member를 함께 읽어 공통 row 모델로 정렬 후 시트에 기록하도록 수정했습니다.ClubMemberSheetService가 동기화 응답의syncedMemberCount에 사전 회원 수까지 포함하도록 보정했습니다.ClubPreMemberRepository에countByClubId를 추가하고, API/응답 Swagger 문구를 실제 동작에 맞게 회원+사전 회원 기준으로 정리했습니다.SheetSyncExecutorTest에 회원과 사전 회원이 함께 시트에 기록되는 테스트를 추가했고,ClubMemberSheetServiceTest를 추가해 응답 카운트 보정을 검증했습니다.💬 참고 사항
sortKey규칙을 유지하되, 사전 회원도 동일 기준으로 함께 정렬됩니다../gradlew.bat test --tests "gg.agit.konect.domain.club.service.SheetSyncExecutorTest" --tests "gg.agit.konect.domain.club.service.ClubMemberSheetServiceTest"와./gradlew.bat checkstyleMain으로 확인했습니다.✅ Checklist (완료 조건)