-
Notifications
You must be signed in to change notification settings - Fork 6
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
[User] 친구삭제 API 구현 #194
[User] 친구삭제 API 구현 #194
Conversation
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.
고생하셨습니다!!!!!!!!!!!!!!! 👍
friendRepository.delete(userFriendRelation) | ||
friendRepository.delete(friendFriendRelation) |
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.
cascade를 통해서 친구 연관관계를 삭제하는 방식은 어떤가요?
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.
넵 수정하겠습니다!!
return friendRepository | ||
.findByUserIdAndFriendIdAndRequestStatus(userId, friendId, requestStatus) | ||
?: throw ServiceException(UserErrorCode.FRIEND_NOT_FOUND) | ||
} | ||
return friendRepository | ||
.findByUserIdAndFriendIdAndRequestStatus(userId, friendId, requestStatus) |
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.
friendRepository.findByUserIdAndFriendIdAndRequestStatus(userId, friendId, requestStatus)
를 중복해서 사용해야하는 제약이 있나요?
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.
해당 코드가 acceptFriendRequest와 deleteFriend에서 사용되는데, 각 메서드에서 필요한 값을 얻기 위해서는 requestStatus를 다르게 줘야 해서 중복되는 코드를 줄이기 위해 이와 같이 작성했습니다!
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.
사용하는 코드에서 기대하는 에러 코드가 달라지는 것을 기대하는 것이라면 이 기능은 acceptFriendRequest와 deleteFriend가 처리해야하는 책임을 가지는 것이 아닐까요?
코드 상에서 accept 상태일 때는 friend_not_found 이지만 그렇지 않은 경우에 반환하는 코드의 역할이 너무 큰 것 같다는 생각도 드네요. 존재하거나 찾지 못하는 두가지의 상태를 모두 나타내는 것이라서 그런 생각이 든 것 같아요.
단순히 중복을 줄이는 것보다는 책임을 기준으로 메서드를 분리하면 더 좋을 것 같아요. public 메서드라면 더욱 더 그렇구요
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.
넵 알겠습니다!
@@ -190,4 +190,67 @@ internal class FriendServiceImplTest : FunSpec({ | |||
} | |||
exception.errorCode.message() shouldBe "해당하는 친구신청이 없거나 이미 친구입니다." | |||
} | |||
|
|||
test("친구삭제_성공") { | |||
val user = mockk<User>(relaxed = true) |
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.
relaxed는 처음보는데 어떤 의도로 사용하신건지 물어봐도 괜찮을까요?
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.
relaxed = true 옵션을 적어주면, 설정되지 않은 값에 대해서는 기본값이 반환되도록 해 오류가 발생하는 것을 방지한다고 해서 사용했던 것입니다!
|
||
val response = | ||
friendService.deleteFriend(CommonTest.TEST_USER_ID, CommonTest.TEST_FRIEND_ID) | ||
// response.friendId shouldBe CommonTest.TEST_FRIEND_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.
해당 주석은 제거하는 것은 어떨까요?
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.
헉 주석 없앤다는 걸 깜빡했네요..!! 수정하겠습니다
#️⃣연관된 이슈
📝작업 내용