-
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] Soft Delete 기능 구현 + 과거 문답 리스트 조회 로직 수정 #109
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.
후딱 해버리셨네요 수고하셨습니다 ~~
int doneIndex = parentchild.getCount() - 1; | ||
if (todayQnA.isChildAnswer() && todayQnA.isParentAnswer()) { | ||
doneIndex += 1; | ||
} |
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.
오오 이거 변수로 뺀 거 넘 좋다
for (User findUser : findUsers) { | ||
if (!(findUser.getSocialPlatform().equals(SocialPlatform.WITHDRAW))){ | ||
allUsersDeleted = false; | ||
break; | ||
} | ||
} |
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.
boolean allUsersDeleted = findUsers.stream()
.noneMatch(findUser -> !findUser.getSocialPlatform().equals(SocialPlatform.WITHDRAW));
이런식으로 써볼 수 있겠네욥
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.
우와 좋다!! 아직 스트림 사용이 익숙하지 않다보니 깜빡 했네요,,,
findUsers.forEach(u -> userRepository.deleteById(u.getId())); | ||
parentchildRepository.deleteById(parentChild.getId()); | ||
parentChild.getQnaList().forEach(qna -> qnARepository.deleteById(qna.getId())); | ||
} |
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.
부모, 자식 모두 탈퇴하면 삭제하는 거 넘 좋은 것 같습니다 !!!
@SQLDelete(sql = "UPDATE parentchild SET deleted=true WHERE parentchild_id=?") | ||
@Where(clause = "deleted=false") |
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.
오오 soft delete 이렇게 쓰는거구나! 배우고 갑니다 👍
private boolean deleted = Boolean.FALSE; | ||
|
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.
테스트 결과 확인했습니당
📌 관련 이슈
closed #105
✨ 어떤 이유로 변경된 내용인지
변경 전
두 유저가 모두 탈퇴한 경우에도 socialPlatform 필드가 WITHDRAW로 바뀌는 것 외에는 삭제 로직이 이루어지지 않았음
이에 따라 findAll() 등의 함수가 호출될 때 탈퇴된 유저까지 검색하기 때문에, 탈퇴한 유저가 누적될수록 탐색 효율이 낮아지게되는 문제 발생
변경 후
두 유저가 모두 탈퇴한 경우에 관련된 user 테이블, parentchild 테이블, qna 테이블을 모두 삭제하는 로직을 추가하였음
삭제할 때 hard delete가 아닌 soft delete 전략을 사용함으로써 탐색 효율은 증가시킴과 동시에, 비즈니스적으로 유용할 수 있는 정보를 DB에 보관하는 전략을 택하였음
테스트 결과
@ElementCollection은 그냥 깔끔하게 삭제됨
한계점
한명만 탈퇴할 경우에 해당 user 테이블만이라도 soft delete 하고싶었으나, 탈퇴하지 않은 사용자가 이 user 테이블의 정보를 원하는 경우가 다수 존재하는 상황
따라서 어떤 경우에는 탈퇴한 user를 조회하도록 하고, 어떤 경우에는 탈퇴한 user는 제외하고 조회하도록 함수를 다 나눠야 했으며 WITHDRAW라는 enum에 의존적이지 않도록 바꾸기 위해 대공사가 필요했음
너무 대공사임 + 대공사인데에 비해 얻을 수 있는 효용은 크지 않음 -> 안하기로 판단
✅ 앞으로 DB 설계시에 이러한 부분을 고려한다면 참 좋을 것 같음...
추가 변경점
현재 섹션별 과거의 문답리스트 조회하기 API에서 전날의 질문까지만 조회하도록 하였었음
하지만 릴리즈 후 피드백을 통해, 오늘의 질문에 부모와 자식이 둘다 답변 완료했을 경우도 포함시키도록 개선
🙏 검토 혹은 리뷰어에게 남기고 싶은 말