-
Notifications
You must be signed in to change notification settings - Fork 0
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: 멤버 미션 API를 작성한다 #21
Conversation
- 특정 (완료된) 미션에 대한 reward 조회 기능 구현 - 모든 (완료된) 미션에 대한 reward 총합 조회 기능 구현 - reward 응답 DTO 작성 - MemberMissionsRepository JPA 타입 변경
- MemberMissions 하위 예외 패키지 추가 - 예외 핸들러 추가
- API 경로의 회원 인식되도록 인터셉터 메서드 추가
- 회원 미션 목록 및 미션 등록 구현 - 관련 리포지터리 인터페이스 정의
- 회원의 특정 미션 클리어 처리 구현
- MemberMissions isExistMemberMissions 메서드 삭제
- MemberMissionsJpaRepositoryTest 작성 - 리포지터리 메서드의 반환 타입 변경
- MemberMissionsJpaRepository의 필요 없는 메서드 정의 삭제
- 서비스 단계인 MemberMissionsService 테스트 작성 - 필요한 MemberMissionsFakeRepository 생성
- MemberMissionsService 예외 테스트 Nested 추가 - 미션 클리어 테스트 추가 - given 표현 수정
- MemberMissions 컨트롤러 테스트 작성 - MemberMissions 문서화 적용 - 필요한 RewardResponse Fixture 생성
- 회원의 모든 미션 (페이징) 조회 구현 - 회원의 미션 완료 여부에 따른 결과 조회 구현
- 회원 미션 조회 테스트 작성 - API 문서화 추가
- 완료된 MemberMission 생성 시 doesGetReward도 true로 되도록 수정
- Mission 예외 핸들러 등록 - Mission 예외 패키지 분리
- MemberMissions 인수 테스트 작성
- MemberMissionPagingResponse final 인자 추가
- MemberMissionsService 주석 제거
- MemberMissionsController 메서드 인자 줄내림 추가
- MemberMissionsController, MemberMissionsService, 테스트 간의 메서드 순서 같도록 수정
- MemberMissionQueryRepository 테스트 작성 - MemberMissionQueryRepository 메서드 타입 변경
- 컨트롤러 흐름에 맞게 문서화 수정
- MemberMissionsExceptionHandler 메서드명 수정
- ignoringFields 대신 ignoringFieldsOfTypes을 사용하도록 수정
- 구현체와 같은 표현을 위해 메서드 공백 추가
- 테스트 클래스 public 제거
- MemberMissionsController의 MemberMissionsService 의존성 제거
- 회원 미션 등록, 관리가 이벤트 기반으로 되도록 수정 - 완료한 미션의 미수령 보상 수령 메서드 삭제 - 클리어 상태 삭제 (클리어 된 미션만 들어올 수 있음) - 성별 별로 가능한 최대 데일리 미션 갯수 반영 - 챌린지 미션은 1회만 등록되도록 설정 - 테스트 코드 예외 수정
- 메서드명 수정 - 검증 조건을 단축하기 위해 Map으로 초기화
- 성별에 따른 데일리 미션 추가 예외 테스트 작성 - 챌린지 미션 예외 테스트 작성 - 관련 Fixture 등록 - 빌더 사용 시 BaseEntity 사용하도록 SuperBuilder 등록
- 바뀐 도메인에 대한 서비스 테스트 추가 작성 - MethodSource를 활용하여 중복 테스트 공통 처리 진행
- MethodSource를 사용하여 비슷한 테스트 삭제 진행
- MemberMission 클리어 여부 조회 기능 삭제 - 관련 테스트 삭제 - 기타 필요없는 데이터 삭제
- 클리어 여부에 따른 회원 미션 조회 메서드 삭제
- 여러 데일리 미션이 등록되어 있도록 수정
- MemberMission 조회 DTO에 계산 로직이 들어가지 않도록 수정 - MemberMission 조회 DTO에 미션 타입, 생성 날짜 들어가도록 수정 - 페이지 처리 검증을 위해 사이즈 수정
- 회원 클리어 여부 미션 조회 API 문서 삭제
- MemberMission-Mission 관계를 ManyToOne으로 수정
- 넘겨진 미션의 타입에 따라 조기 리턴되도록 검증 메서드 수정
- MemberMissionFixture 메서드명 수정
- MemberMission 통합 조회 테스트 데이터 수정
- 미션의 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.
고생하셨습니다! 몇가지만 수정해주세요:)
if (isStatusClear) { | ||
// TODO | ||
return; | ||
private static int getNextPage(final int pageNumber, final Page<MemberMissionSimpleResponse> memberMissions) { |
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.
static 제거해주세요~
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.
제거 완료하였습니다 :)
MemberMissions memberMissions = memberMissionsRepository.findByMemberId(memberId) | ||
.orElseGet(() -> createNewMemberMissionsWithMemberId(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.
저도 좀 더 생각해 보니 지금 같은 경우에는 orElseGet으로 처리하는 것이 좋을 거 같습니다!
private Long id = 0L; | ||
|
||
@Override | ||
public MemberMissions save(final MemberMissions memberMissions) { | ||
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.
저장되는 id의 값을 1부터 하고 싶어서 이렇게 하신건가요??
그럼 private Long id = 1L;로 하는게 더 나아 보입니다
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.
수정하였습니다!
- MemberMissionsQueryService static 제거
- 데일리 미션 생성 날짜 비교 메서드 추가
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.
수고하셨습니다 몇 가지만 확인하고 수정해주시면 될 것 같습니다~
private final MemberMissionsService memberMissionsService; | ||
|
||
@EventListener | ||
public void addClearedMemberMission(final MemberMissionClearedEvent event) { |
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.
addedClearMemberMission이 맞을 것 같네요!
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.
clear에만 과거형을 붙였었네요 수정 완료하였습니다!
@@ -31,40 +31,38 @@ public class MemberMission extends BaseEntity { | |||
@GeneratedValue(strategy = GenerationType.IDENTITY) | |||
private Long id; | |||
|
|||
@OneToOne(fetch = FetchType.EAGER) | |||
@ManyToOne(fetch = FetchType.LAZY) |
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.
MemberMission에서 보상을 수령할 때 미션으로부터 받게 되고, 회원이 가지고 있는 미션이 챌린지 미션인지를 판단하는 과정에서도 미션으로부터 메서드를 호출하게 됩니다.
또한, MemberMission이 미션에 대한 id만 가지고 있다면 MemberMission을 조회할 때 제약이 너무 많이 생길 것 같다는 생각이 들었습니다. (챌린지/데일리 여부, 미션의 보상 등등)
그래서 서로 다른 애그리거트더라도, 이 경우에 대해서는 객체 참조로 하는 게 적절할 것 같다는 판단을 하였습니다.
이벤트 방식으로 하면 이벤트를 수신한 객체로부터 값을 전달받는 것
에 대해 제약이 많을 것 같은데, 재윤님께서는 이러한 상황에서 어떻게 해결하시는지 여쭤봐도 될까요?
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.
음 이 부분은 생성할 때 역정규화로 값을 저장해도 될 것 같아요
조금 어색하게 느껴지는데 일단 현준님 말도 충분히 이해는 돼서 개발하다가 문제가 생기면 그때 바꾸는 걸로 하도록 합시다!
this.memberMissions.add(memberMission); | ||
} | ||
|
||
public Integer getRewardBy(final Long missionId) { | ||
private void validateIsCanAddMission(final Gender memberGender, final Mission mission) { |
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.
isCan은 네이밍이 조금 이상한 것 같아요
validateCan~~이 어떨까요?
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.
수정하였습니다! is can 둘 다 동사였는데 함께 사용했었네요
List<MemberMission> existedMission = extractChallengeMission(mission); | ||
|
||
if (!existedMission.isEmpty()) { | ||
throw new AlreadyChallengeMissionExistedException(); | ||
} |
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.
이 부분도 extractChallengeMission 메서드에서 스트림으로 한 번에 할 수 있어보이네요
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.
ifPresent를 이용하여 수정하였습니다!
|
||
private final JPAQueryFactory jpaQueryFactory; | ||
|
||
public Page<MemberMissionSimpleResponse> findMemberMissionsWithPaging(final Long memberId, final Pageable pageable) { |
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.
해당 쿼리는 어떻게 나갈까요?
private final MemberMissionsRepository memberMissionsRepository; | ||
private final MissionRepository missionRepository; |
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.
MemberMission 쪽이 모두 Mission에 영향을 받을 거 같은데 Mission 참조는 없어도 될 것 같습니다 이벤트나 다른 걸로 풀어나가면 어떨까요?
- MemberMissionsFakeRepository id 초깃값 수정
- 이벤트 컨벤션에 맞게 과거형으로 네이밍
- 동사 중복 삭제 (IsCan -> Can)
- 데일리 미션에서 스트림을 활용하여 한 번에 표현되도록 수정 - extractTodayDailyMissions 반환 타입 수정
- MemberMissionRepository 삭제 - MemberMissionQueryRepository 삭제 - MemberMissionsRepository만 존재하도록 수정
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.
작업하시느라 수고하셨습니다!
📄 Summary
재윤님이 작성해주신 멤버 미션 API 뼈대를 기반으로 작업하였습니다.
🙋🏻 More
Mission
에 대한 예외들이ExceptionHandler
가 없어서 반영하였습니다.MemberMissionsQueryService
의findMemberMissionsByStatus
메서드에서는 페이징을 적용하지 않고 개발하였습니다. 페이징을 필요로 하는 것은findMemberMissionsWithPaging
에서만 필요한 것으로 생각하고 개발을 했었는데, 이 또한 페이징을 추가해야 한다면 반영하도록 하겠습니다. (다만, 민호 님께 여쭤보니 회원의 미션을 조회하는 API는 필요하지 않은 영역일수도 있다고 하셨습니다.)MemberMissionsQueryService
는MemberMissionRepository
의 메서드가 조회밖에 없고, 저장이MemberMissionsRepository
에서 진행되기 때문에 (cascade)MemberMissionQueryRepositoryTest
를 통합 테스트 하는 방식으로 대체하였습니다.이번에도 고쳐야 할 점이 있다면 반영하도록 하겠습니다!
2차 수정
MemberMissionClearedEvent
가 발생된다고 가정하였습니다. 회원 id, 미션 id, 회원 성별이 필요합니다.MemberMissions
에서 Map으로 미리 넣어두도록 하였습니다.피드백 주시면 다시 반영하겠습니다!
close #19