Skip to content

Conversation

@Shinjongyun
Copy link
Contributor

@Shinjongyun Shinjongyun commented Oct 1, 2025

Related issue 🛠

  • closed #이슈넘버

Work Description 📝

  • 마이페이지 동아리 수정 오류 수정했습니다!
  • 테스트 코드 작성후 테스트까지 완료했습니다

Screenshot 📸

Uncompleted Tasks 😅

  • Task1

To Reviewers 📢

Summary by CodeRabbit

  • Bug Fixes
    • 마이페이지에서 사용자 클럽 수정 시 존재하지 않는 클럽 요청을 차단하고, 중복·누락 없이 정확히 추가·제거되도록 정합성을 개선했습니다. 빈 입력도 안정적으로 처리하고 처리 순서로 인한 불일치를 해소했습니다.
  • Tests
    • 기존의 일부 단위 테스트 파일을 제거하여 테스트 구조를 정리했습니다.
  • Chores
    • 테스트 리소스 무시 규칙을 추가하고, 전역 UTF-8 인코딩 적용 및 테스트 로그 가시성을 강화했습니다.

@coderabbitai
Copy link

coderabbitai bot commented Oct 1, 2025

Walkthrough

.gitignore에 테스트 리소스 무시 규칙(+/src/test/*)을 추가했다. Gradle에 전역 UTF-8 인코딩 및 테스트 로깅 설정을 도입했다. UserService의 동아리 업데이트 로직을 입력 정규화·존재 검증·차집합 기반으로 재구성했고, 관련 헬퍼를 추가했다. 기존 Mockito 기반의 단위 테스트 파일이 삭제되었다.

Changes

Cohort / File(s) Summary
Ignore rules
./.gitignore
테스트 리소스 무시 규칙 추가: +/src/test/*
Build config & encoding
./build.gradle
테스트 JVM -Dfile.encoding=UTF-8 추가, 테스트 로깅 이벤트(FAILED, SKIPPED, STANDARD_ERROR) 및 exceptionFormat = FULL, JavaCompile/Javadoc 전역 UTF-8 인코딩 설정 추가
User club update flow
./src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java
updateUserClubs 입력 정규화(null→빈집합, null 필터), 대상 클럽 존재 검증 추가, 현재/목표 집합으로 toAdd/toRemove 계산, 제거 우선 수행, 추가 전 존재 재검증을 수행하는 헬퍼 메서드 도입
Test removal
./src/test/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserServiceTest.java
기존 Mockito 기반 단위 테스트 파일 삭제 (클래스 UserServiceTest 제거)

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client as 호출자
  participant US as UserService
  participant CR as ClubRepository
  participant MR as MemberRepository

  Client->>US: updateUserClubs(userId, targetClubIds)
  US->>US: 입력 정규화 (null→empty, null 항목 제거)
  US->>CR: validateClubExistence(targetClubIds)
  CR-->>US: 존재 확인 / 예외 발생 가능

  US->>MR: find current memberships by userId
  MR-->>US: currentClubIds

  US->>US: toRemove = current - target\ntoAdd = target - current

  alt 제거 대상 존재
    US->>MR: deleteMemberships(toRemove)
    MR-->>US: 삭제 완료
  end

  alt 추가 대상 존재
    US->>CR: findAllById(toAdd)
    CR-->>US: 조회된 클럽 목록
    US->>US: 누락 클럽 있으면 예외
    US->>MR: saveAll(new Members for toAdd)
    MR-->>US: 저장 완료
  end

  US-->>Client: 업데이트 결과 반환
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

UTF‑8 바람이 깃들고, 로그는 한 줄 더 밝아지고,
집합은 계산되어 먼저 비워진 자리, 다음엔 새 얼굴들,
.gitignore의 작은 문장 하나가 숨을 고르고,
사라진 테스트 파일은 다시 쓰일 이야길 남긴다. 🎐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 제목은 PR의 핵심 변경사항인 마이페이지 동아리 수정 기능의 오류를 바로잡는 내용을 명확하게 반영하고 있으며 불필요하게 장황하지 않고 간결하게 핵심을 전달하고 있습니다. 리뷰어가 PR 히스토리를 스캔할 때 즉시 변경 의도를 파악할 수 있어 목적에 부합합니다. 따라서 PR 제목으로서 충분히 목적에 맞고 이해하기 쉽습니다.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#10-myPage

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2745079 and 50f611d.

📒 Files selected for processing (2)
  • .gitignore (1 hunks)
  • src/test/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserServiceTest.java (0 hunks)
💤 Files with no reviewable changes (1)
  • src/test/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • .gitignore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (4)

24-24: 와일드카드 임포트 대신 명시적 임포트를 사용하세요.

import java.util.*; 와일드카드 임포트는 어떤 클래스들이 실제로 사용되는지 파악하기 어렵고, 네임스페이스 충돌 가능성도 있습니다. 현재 코드에서 사용 중인 Set, HashSet, LinkedHashSet, List, Objects 등을 명시적으로 임포트하는 것을 권장합니다.

-import java.util.*;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;

143-159: 클럽 추가 로직이 잘 구현되었으나, 중복 검증을 제거할 수 있습니다.

126번 라인에서 이미 모든 targetClubIds의 존재 여부를 검증했으므로, 146-148번 라인의 clubs.size() != toAdd.size() 체크는 이론적으로 불필요합니다. toAddtargetClubIds의 부분집합이기 때문입니다.

다만 145번 라인의 clubRepository.findAllById(toAdd) 호출 자체는 이후 Member 엔티티 생성에 필요하므로 유지해야 합니다.

중복 검증을 제거하려면:

         // 5) 추가할 Club 존재 여부 검증
         if (!toAdd.isEmpty()) {
             List<Club> clubs = clubRepository.findAllById(toAdd);
-            if (clubs.size() != toAdd.size()) {
-                throw new BusinessException(ErrorCode.CLUB_NOT_FOUND);
-            }

             // 6) 추가 실행
             List<Member> newMembers = clubs.stream()

또는, 검증의 명확성을 위해 유지하되 주석으로 의도를 표시할 수도 있습니다.


162-171: 재사용 가능한 검증 헬퍼 메서드가 잘 추가되었습니다.

validateClubExistence 메서드는 클럽 존재 여부 검증 로직을 깔끔하게 캡슐화했습니다. null/빈 집합 처리와 일괄 검증 로직이 명확합니다.

다만 앞서 언급했듯이, 126번 라인에서 이 메서드를 호출해 전체 targetClubIds를 검증한 후, 144-148번 라인에서 toAdd에 대해 유사한 검증을 다시 수행하고 있습니다. 중복을 제거하면 코드가 더 간결해질 것 같습니다.


117-126: 중복된 클럽 존재 검증 제거
초기 validateClubExistence(targetClubIds) 호출로 이미 모든 요청 ID 유효성을 확인하므로, 이후 if (!toAdd.isEmpty()) { … findAllById(toAdd) … } 블록의 검증 로직을 삭제하세요.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd8d652 and 2745079.

📒 Files selected for processing (4)
  • .gitignore (1 hunks)
  • build.gradle (1 hunks)
  • src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (2 hunks)
  • src/test/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserServiceTest.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (1)
  • Slf4j (27-172)
🔇 Additional comments (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (1)

128-136: 집합 기반 차집합 연산으로 추가/삭제 계산이 깔끔하게 구현되었습니다.

현재 가입된 클럽 ID를 조회한 후 HashSetremoveAll을 활용해 추가할 클럽(toAdd)과 제거할 클럽(toRemove)을 효율적으로 계산하고 있습니다. 로직이 명확하고 모든 시나리오(추가만, 삭제만, 혼합)를 정확하게 처리합니다.

.gitignore Outdated
Comment on lines 42 to 43
### test file ###
/src/test/resources/*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

src/test/resources 전체 무시는 테스트 구성 누락을 유발합니다

@ActiveProfiles("test")로 구동되는 통합 테스트가 안정적으로 돌아가려면 application-test.yml 같은 프로필 자원이 반드시 레포지터리에 포함되어야 합니다. 하지만 /src/test/resources/*를 통째로 무시하면 필수 설정 파일을 커밋할 수 없어 다른 개발자나 CI 환경에서 동일한 테스트를 실행할 수 없게 됩니다. 생성물만 제외하려면 구체적인 패턴(예: *.generated.json 등)으로 한정하거나 이 규칙을 제거해 주세요.

🤖 Prompt for AI Agents
.gitignore around lines 42-43: the rule "/src/test/resources/*" is too broad and
prevents committing necessary test profile resources like application-test.yml;
remove this blanket ignore or narrow it to specific generated artifacts (for
example patterns like /src/test/resources/*.generated.json or a specific folder
for build outputs) so that essential test config files remain in the repo while
still excluding only generated/test-output files.

Copy link
Contributor

@yskim6772 yskim6772 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨슴다!

@yskim6772 yskim6772 merged commit a97e38f into develop Oct 1, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants