-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat: 회원탈퇴 구현 #55
Feat: 회원탈퇴 구현 #55
Conversation
… feature/#40/account-delete
… feature/#40/account-delete
- save - findById - findBySocialTypeAndOauthId - updateOauthEmail - deleteByMember
* Feat: RunningRecord member에 따라 삭제 기능 추가 * Test: RunningRecord member에 따라 삭제 테스트 * Fix: deleteByMember의 인자를 member_id로 받게 수정 * Fix: Repository Transactional 삭제
...com/dnd/runus/infrastructure/persistence/domain/running/RunningRecordRepositoryImplTest.java
Outdated
Show resolved
Hide resolved
.../com/dnd/runus/infrastructure/persistence/domain/member/SocialProfileRepositoryImplTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnd/runus/domain/oauth/service/OauthService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnd/runus/domain/oauth/dto/request/WithdrawRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnd/runus/auth/oidc/provider/OidcProviderFactory.java
Outdated
Show resolved
Hide resolved
badgeAchievementRepository.deleteByMemberId(memberId); | ||
runningRecordRepository.deleteByMemberId(memberId); | ||
memberLevelRepository.deleteByMemberId(memberId); | ||
socialProfileRepository.deleteByMemberId(memberId); |
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.
지금 변경은 어렵지만, 고려해볼 사항
지금은 apple 밖에 없지만, 다른 social profile들도 연결 끊기 작업이 필요하다고 생각해요
- 각 oidc 제공자 마다 연결 끊는 방법이 다를 수 있으므로, OidcProvider 인터페이스에
withdraw()
나unlink()
같은 함수가 있어야 할 것 같아요
그리고 현재 PR에서 withdraw 함수가 하나의 트랜잭션 내에서 외부 네트워크로 통신을 하고 있는데,
이러면 트랜잭션이 너무 길어질 수도 있고, 외부 서비스 장애시 대처하기 어려운 것 같아요.
- 지금 PR에서는 수정하기 힘들더라도, 이슈로 남겨두고 리팩토링하면 좋을 것 같아요
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.
각 oidc 제공자 마다 연결 끊는 방법이 다를 수 있으므로, OidcProvider 인터페이스에 withdraw()나 unlink() 같은 함수가 있어야 할 것 같아요
OidcProvider 인터페이스에 연결해제 함수가 있어요!
void revoke(String accessToken)
찾아보니 대표 소셜 로그인에서 공통적으로 accessToken으로 해제하는 것 같아서 인자로 accessToken을 받게 해놨습니다.
그리고 현재 PR에서 withdraw 함수가 하나의 트랜잭션 내에서 외부 네트워크로 통신을 하고 있는데,
이러면 트랜잭션이 너무 길어질 수도 있고, 외부 서비스 장애시 대처하기 어려운 것 같아요.
이런 부분에 대해 생각해보지 못했는데 👍
이슈로 남겨두고 리팩토링해봐야 겠네요!
...com/dnd/runus/infrastructure/persistence/domain/running/RunningRecordRepositoryImplTest.java
Outdated
Show resolved
Hide resolved
고생하셨습니다 ! 몇가지 생각들 적었는데, 한번 코멘트 주시면 감사하겠습니다 |
꼼꼼한 코멘트 감사합니다! |
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.
빠른 수정 감사합니다 !
어제 실수로 놓친 점이 있어서, 그것만 봐주시면 감사하겠습니다 🙏
src/main/java/com/dnd/runus/client/vo/AppleAuthTokenResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnd/runus/client/vo/AppleAuthTokenRequest.java
Outdated
Show resolved
Hide resolved
- AppleAuthTokenResponse 코드 카멜로 변경 - Requst DTO toMultiValueMap 메서드 코드 간결화
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
💡 반영할 내용 및 변경 사항 요약
badge_achievement
running_record
member_level
social_profile
member
🔍 리뷰 요청/참고 사항