search

dnd-side-project / dnd-11th-2-backend

Run earth with 'RunUs' 🌏
https://apps.apple.com/kr/app/runus/id6689522964
0 stars 1 forks source link

Feat: 사용자 목표 저장 및 조회 구현 #99

Closed hee9841 closed 2 months ago

hee9841 commented 2 months ago

🔗 이슈 연결

🚀 구현한 API

💡 반영할 내용 및 변경 사항 요약

🔍 리뷰 요청/참고 사항

Jaewon-pro commented 2 months ago

몇 km 달성 목표인지에 대해서, meter -> 0.0km 변환하는 포메팅이 필요해서 Level.formatExp(int exp)를 com.dnd.runus.global.util.RunningMetricsFormater.java에 추가 했습니다. Level.formatExp(int exp) 부분은 수정하지 않았고 필요하면 공통으로 사용하는 것이 좋을 것 같아요. 어떻게 생각하시나요?

Level.formatExp()와 RunningMetricsFormater은 유사하지만 분리하는 게 좋을 것 같아요 레벨 exp가 meter를 기준으로 하지만 수정될 수 있다고 생각해요

그래서 RunningMetricsFormater은 GoalAcievement에서만 쓰이고 있는데, util 클래스 없이 meterToKm, secondToKoreanHHMM 메서드들을 GoalAchievement 도메인 클래스로 옮기면 어떠신가요?

GoalAchievement 테이블에 member, running_record 외래키를 2개 가지고 있어요 running_record 테이블에서 member를 외래키로 가지고 있으니, goal_achievement 테이블의 member 외래키는 없어도 될 것 같아요

그리고 GoalAchievementService의 save, findByRunningId는 GoalAchievementRepository의 save, findByRunningRecordId()를 다시 묶어서 dto로 변환만 하고 있어요. 제 생각에는 단순하게 하기 위해서 GoalAchievementService는 삭제해도 좋을 것 같은데 어떻게 생각하시나요?

hee9841 commented 2 months ago

그래서 RunningMetricsFormater은 GoalAcievement에서만 쓰이고 있는데, util 클래스 없이 meterToKm, secondToKoreanHHMM 메서드들을 GoalAchievement 도메인 클래스로 옮기면 어떠신가요?

좋습니다!

hee9841 commented 2 months ago

사실 처음에 회원탈퇴 시 편하게 하기위해 member를 외래키로 설정하지 않고 memberId로 지정했었는데, 생각해보니 runningRecord에 삭제 시 cascade 옵션을 추가하는게 좋을 것 같은데 어떻게 생각하신가요?? 그러면 Challenge_achievement 쪽도 비슷하게 member를 외래키로 설정하지 않고 runningRecord에 삭제 시 cascade 옵션을 추가하면 될 것 같네요. 그리고 추가적으로 실제 테이블에도 on delete cascade옵션을 추가하는게 좋을 까요?

hee9841 commented 2 months ago

goal_achievement는 조인하거나 repository의 findById를 service 객체 없이 사용할 것 같아요

러닝쪽에서 조인하신다는 말씀이시죠??

challenge가 테이블 형식이면 runningType에 따라 다르게 조인할 수 있지만, 지금은 조인하기가 조금 복잡할 것 같아요. GoalAchievementService는 삭제해도 괜찮을 것 같고, 러닝 서비스에서 러닝 타입에 따라 아래처럼 가는게 좋을 것 같은데 어떻게 생각하신가요?

hee9841 commented 2 months ago

그리고 챌린지 쪽은 다 되어서 챌린지 조회, 저장 쪽 러닝 서비스에 반영해도 됩니다!

Jaewon-pro commented 2 months ago

그러면 Challenge_achievement 쪽도 비슷하게 member를 외래키로 설정하지 않고 runningRecord에 삭제 시 cascade 옵션을 추가하면 될 것 같네요. 그리고 추가적으로 실제 테이블에도 on delete cascade옵션을 추가하는게 좋을 까요?

cascade 옵션도 있지만, 지금 회원 탈퇴 과정에 사용하기엔 약간 좋지 않은 것 같아요

회원 탈퇴 요청시 하나의 트랜잭션에서 너무 많은 변경이 이루어지고 있어요

  1. apple에 revoke 요청을 보냅니다. (네트워크 통신)
  2. 회원 테이터를 모두 delete합니다. (JPA repository의 delete 사용, deleteInBatch 사용X)

그래서 회원은 바로 삭제하는 것이 아니라, 지우지 않아도 되는 테이블에는 soft delete를 적용하는 방법도 있을 것 같아요

hee9841 commented 2 months ago

오 이런 방법이 있었군요! 그럼 회원은 바로 삭제 하지 않고, 계정이 연결 해제 되었을 경우에 일정 기간이 지나면 데이터 지우게 배치 처리하는 건 어떻게 생각하시나요?

Jaewon-pro commented 2 months ago

오 이런 방법이 있었군요! 그럼 회원은 바로 삭제 하지 않고, 계정이 연결 해제 되었을 경우에 일정 기간이 지나면 데이터 지우게 배치 처리하는 건 어떻게 생각하시나요?

좋아요!

soft delete로 변경하는 작업이 오래 걸릴 것 같아서, challenge 쪽 먼저 작업하고 그 이후에 진행하는 건 어떠신가요?

hee9841 commented 2 months ago

좋습니다! challenge_achievement 쪽 member 삭제하고 새로 pr 올릴게요!

hee9841 commented 2 months ago

그리고 이건 어떻게 생각하는지 궁금합니다! https://github.com/dnd-side-project/dnd-11th-2-backend/pull/99#issuecomment-2306318273 일단 pr리뷰 내용 적용하고, 서비스 단 지우고 작업한 다음 다시 커밋올릴려고 하는데 이렇게 진행하면 되나요?

hee9841 commented 2 months ago

그리고 배치 같은 경우 저희가 사용자와 데이터가 별로 없기도 하고, 마감 시간까지 완성하기 위해서 사실 이 방식이 제일 안 좋은 방식일 것 같은데 지금은 한 서비스 내에서 배치가 포함되게 구성하면 좋을 것 같아요. 시간이 된다면 배치 서비스를 별도로 구성하는 방식으로 가고요. 혹시 어떻게 생각하시나요? 제가 배치는 처음이라서 재원님 의견이 궁금합니다.

Jaewon-pro commented 2 months ago

타입이 goal이면 goal_achievement의 repository를 호출 타입이 challenge면 ChallengeAchievement 서비스 호출 타입이 normal이면 아무것도 호출하지 않고 러닝 데이터만 리턴

RunningRecordService에서 ChallengeAchievementService를 호출해서 계층 분리가 되지 않을 것 같아요

추후 리팩토링하도록 하고 goal_achievement 테이블, flyway sql, 엔티티 추가만 부탁드려요 !

Jaewon-pro commented 2 months ago

그리고 배치 같은 경우 저희가 사용자와 데이터가 별로 없기도 하고, 마감 시간까지 완성하기 위해서 사실 이 방식이 제일 안 좋은 방식일 것 같은데 지금은 한 서비스 내에서 배치가 포함되게 구성하면 좋을 것 같아요. 시간이 된다면 배치 서비스를 별도로 구성하는 방식으로 가고요. 혹시 어떻게 생각하시나요? 제가 배치는 처음이라서 재원님 의견이 궁금합니다.

지금은 단일 인스턴스 하나에 was를 띄우고 있기 때문에, 단순하게 스프링 스케줄러로 멤버 데이터 삭제를 해도 좋을 것 같아요