-
Notifications
You must be signed in to change notification settings - Fork 4
[FIx] 친구요청 취소 버그 수정 #383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIx] 친구요청 취소 버그 수정 #383
Conversation
워크스루친구 요청 삭제 기능의 버그를 수정하고, 사용자 엔티티에 JPA 설정을 추가하며, 임포트 정리 및 테스트를 업데이트합니다. 친구 요청 조회 시 요청 방향을 수정하고, User 엔티티에 테이블 매핑, 롬복 어노테이션, 소프트 삭제 기능을 도입합니다. 변경 사항
추정 코드 리뷰 노력🎯 2 (Simple) | ⏱️ ~10 minutes 시
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
1-96: PR 목표와 코드 변경 사항의 불일치이 PR은 "친구요청 취소 버그 수정"을 목표로 하고 있으며 이슈 #382("친구 요청 삭제 버그")와 연결되어 있습니다. 하지만 변경된 코드는 공지사항(Notice) 처리 로직만 포함하고 있으며, 친구 요청과 관련된 코드는 전혀 포함되어 있지 않습니다.
현재 변경 사항은 import 문 정리와 포맷팅만 포함하고 있어, 친구 요청 취소/삭제 버그를 해결하지 못합니다. 다음 중 하나의 문제로 보입니다:
- 잘못된 파일이 커밋됨
- PR 제목/설명이 잘못 작성됨
- 실제 버그 수정 코드가 누락됨
친구 요청 관련 도메인(예: FriendRequest, Friendship 등)의 변경 사항을 포함하는지 확인해 주세요.
#!/bin/bash # 친구 요청 관련 도메인 클래스 및 서비스 찾기 echo "=== 친구 요청 관련 파일 검색 ===" fd -e java -x echo {} | rg -i "friend" echo -e "\n=== 최근 커밋에서 변경된 파일 확인 ===" git diff --name-only HEAD~1 HEADsrc/main/java/ku_rum/backend/domain/user/domain/User.java (2)
32-32: 클래스 레벨 @Setter 어노테이션 제거 필요 (심각)클래스 레벨에
@Setter를 추가하면 모든 private 필드(id,roles,oauthId,password등)에 대한 setter가 자동 생성됩니다. 이는 다음과 같은 심각한 문제를 야기합니다:
- 데이터 무결성: Primary key인
id필드가 외부에서 수정 가능해집니다- 보안:
roles필드 수정으로 권한 상승 취약점,password/oauthId수정으로 인증 우회 가능- 캡슐화 위반: 이미
changePassword(),changeNickname()등의 제어된 변경 메서드가 존재하는데 이를 우회할 수 있게 됩니다엔티티의 특정 필드만 setter가 필요한 경우 해당 필드에만
@Setter를 명시적으로 추가하거나, 기존처럼 제어된 변경 메서드만 사용하시기 바랍니다.🔎 제안하는 수정
@Getter @Entity @NoArgsConstructor(access = AccessLevel.PROTECTED) @Table(name = "users") -@Setter @SQLDelete(sql = "UPDATE users SET active = false WHERE id = ?") public class User extends BaseEntity {필요시 특정 필드에만 선택적으로 추가:
@Setter private String nickname;
33-33:active필드의 의미 중복 문제 (주요 이슈)
@SQLDelete어노테이션이active필드를false로 설정하여 소프트 삭제를 구현하고 있으나, 65번 라인의 주석과 94-96번 라인의setLocationSharingActive()메서드를 보면active필드는 "위치 공유 현재 상태"를 나타내는 용도로 사용되고 있습니다.이로 인해 다음과 같은 문제가 발생할 수 있습니다:
- 사용자가 위치 공유를 비활성화(
active=false)하면 삭제된 것으로 간주될 수 있음- 사용자가 소프트 삭제되면 위치 공유가 비활성화된 것처럼 보일 수 있음
- 쿼리 및 비즈니스 로직에서 혼란 야기
소프트 삭제를 위해 별도의 필드(예:
deleted또는isDeleted)를 추가하는 것을 권장합니다.🔎 제안하는 수정
+ private boolean deleted = false; // 소프트 삭제 상태 + private boolean active = true; //위치 공유 현재 상태그리고 @SQLDelete 수정:
-@SQLDelete(sql = "UPDATE users SET active = false WHERE id = ?") +@SQLDelete(sql = "UPDATE users SET deleted = true WHERE id = ?")필요시 @where 어노테이션도 추가:
@Where(clause = "deleted = false")
🤖 Fix all issues with AI Agents
In
@src/test/java/ku_rum/backend/domain/friend/presentation/FriendManageControllerTest.java:
- Line 157: Update the API test assertion in FriendManageControllerTest that
uses fieldWithPath("receiverId") to use a clear description reflecting that
receiverId is the recipient user's ID (예: "수신자 사용자 ID" or "수신자의 사용자 ID"),
replacing the current misleading "삭제할 친구 요청 ID" text so the documentation
correctly describes the field as the recipient's user ID rather than a
request/entity ID.
🧹 Nitpick comments (2)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
3-7: import 문 정리는 적절함static import 추가와 중복 import 제거는 코드 정리 측면에서 적절합니다. 하지만 이러한 변경은 기능적인 버그 수정이 아닌 코드 포맷팅 개선에 해당합니다.
src/main/java/ku_rum/backend/domain/friend/application/FriendManageService.java (1)
77-79: 중복 검증 로직을 제거할 수 있습니다.Line 70의 쿼리가 이미
status = PENDING조건으로 필터링하므로, 이 상태 검증은 항상 참(true)입니다. 코드 간결성을 위해 제거를 고려해보세요.🔎 제안하는 리팩토링
- if (friend.getStatus() != PENDING) { - throw new GlobalException(DUPLICATE_RESPONSE); - } - friendRepository.delete(friend);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ku_rum/backend/domain/friend/application/FriendManageService.javasrc/main/java/ku_rum/backend/domain/notice/application/NoticeService.javasrc/main/java/ku_rum/backend/domain/user/domain/User.javasrc/test/java/ku_rum/backend/domain/friend/presentation/FriendManageControllerTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
src/main/java/ku_rum/backend/domain/friend/application/FriendManageService.java (1)
70-70: 핵심 버그 수정이 정확합니다!쿼리 파라미터 순서가 올바르게 수정되었습니다. 보낸 친구 요청을 삭제할 때는
currentUser가 발신자(fromUser)인 레코드를 찾아야 하는데, 이전 코드는 파라미터가 반대로 되어 있어 요청을 찾지 못했습니다. 이 수정으로 Issue #382의 친구 요청 삭제 버그가 해결됩니다.src/test/java/ku_rum/backend/domain/friend/presentation/FriendManageControllerTest.java (1)
137-138: 테스트 업데이트가 서비스 레이어 변경과 일치합니다.
requestId에서receiverId로의 변경이 변수명, DTO 생성, JSON 페이로드에서 일관되게 적용되었으며, 수정된deleteSentRequest메서드의 동작과 올바르게 정렬됩니다.Also applies to: 145-145
src/main/java/ku_rum/backend/domain/user/domain/User.java (4)
3-25: 임포트 명시화 개선 확인와일드카드 임포트를 명시적 임포트로 변경한 것은 좋은 개선입니다. 코드 가독성과 네임스페이스 관리가 향상됩니다.
31-31: 명시적 테이블 명 지정 확인
@Table(name = "users")어노테이션 추가로 테이블명을 명시적으로 지정한 것은 좋은 관행입니다.
107-107: 기본 역할 초기화 개선 확인생성자에서 기본
USER역할을 초기화하는 것은 좋은 개선입니다. 새로운 사용자가 항상 최소한의 역할을 가지도록 보장합니다.
1-148: User.java 변경사항이 친구요청 취소 버그 수정과 무관함이 PR의 목적인 "친구요청 취소 버그 수정" (#382)을 담당하는
FriendManageService의 코드를 검토한 결과,User.java파일의 변경사항들이 실제 버그 수정에 전혀 사용되지 않음을 확인했습니다.
FriendManageService에서는:
deleteSentRequest()메서드에서 친구요청 삭제 로직 구현- User 엔티티는 조회(
userUtil.getUser(),userQueryService.getUserById())만 수행- Friend 엔티티의 상태만 수정 (
friend.setStatus())- User 엔티티의 setter 메서드 호출 없음
따라서 추가된
@Setter,@SQLDelete등의 변경사항은 친구요청 취소 기능과 무관한 별도의 리팩토링입니다. 이러한 변경사항들은 별도 PR로 분리하는 것을 권장합니다.
| ) | ||
| .requestFields( | ||
| fieldWithPath("requestId").description("삭제할 친구 요청 ID") | ||
| fieldWithPath("receiverId").description("삭제할 친구 요청 ID") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API 문서 설명이 부정확합니다.
필드명은 receiverId이지만 설명은 "삭제할 친구 요청 ID"로 되어 있어 혼란을 줄 수 있습니다. receiverId는 요청 엔티티의 ID가 아닌 수신자의 사용자 ID이므로, 설명을 명확히 수정해야 합니다.
🔎 제안하는 수정
- fieldWithPath("receiverId").description("삭제할 친구 요청 ID")
+ fieldWithPath("receiverId").description("친구 요청을 받은 사용자 ID")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fieldWithPath("receiverId").description("삭제할 친구 요청 ID") | |
| fieldWithPath("receiverId").description("친구 요청을 받은 사용자 ID") |
🤖 Prompt for AI Agents
In
@src/test/java/ku_rum/backend/domain/friend/presentation/FriendManageControllerTest.java
around line 157, Update the API test assertion in FriendManageControllerTest
that uses fieldWithPath("receiverId") to use a clear description reflecting that
receiverId is the recipient user's ID (예: "수신자 사용자 ID" or "수신자의 사용자 ID"),
replacing the current misleading "삭제할 친구 요청 ID" text so the documentation
correctly describes the field as the recipient's user ID rather than a
request/entity ID.
Test Results 38 files 38 suites 11s ⏱️ Results for commit 7cac0b6. |
#️⃣ 연관된 이슈
closes #382
📝작업 내용
작업 상세 내용
상세 내용을 입력해주세요.
💬리뷰 요구사항
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.