골 수락 기능 구현 #86
골 수락 기능 구현 #86
Conversation
|
|
||
| return ReadGoalDetailDto.from(goal); | ||
| } | ||
|
|
||
| private void validateUserToRead(final User user, final Goal goal) { | ||
| if (!goal.isTeam(user) || !goal.isAccepted(user)) { |
There was a problem hiding this comment.
goal에서 isTeam 자체를 수락까지 완료한 사용자만 true로 반환하면 안 될까요?
만약, 기존 isTeam의 메서드도 필요하다면, 해당 메서드의 이름을 대기 중인 팀원 등으로 변경하면 좋을 것 같아요.
There was a problem hiding this comment.
말씀하신대로 아직 골 수락을 하지 않은 초대 멤버에 대해서는 팀원으로 취급하지 않아도 될 것 같아서 isTeam 호출에서 수락한 사람들만 가져오는 것으로 변경했습니다. 추후에 초대한 팀원의 수락 여부를 받아와야할 경우가 생긴다면 그 때 메서드를 분리하면 좋을 것 같네요!
There was a problem hiding this comment.
테스트를 돌렸는데 초대한 팀원 목록을 받아와야하는 경우가 있어 오류가 나더군요.. 다시 메서드 분리하고 메서드명을 isTeamAndAccepted로 설정했습니다!
|
|
||
| public class InvalidGoalAcceptException extends BloomingException { | ||
|
|
||
| public InvalidGoalAcceptException(final ExceptionMessage exceptionMessage) { |
There was a problem hiding this comment.
해당 메서드를 클래스 외부에서 사용할 것이 아니라면 private으로 설정해줘도 좋을 것 같아요!
| private void validateUserIsManager(final User user, final Goal goal) { | ||
| if (goal.getManagerId().equals(user.getId())) { | ||
| updateAccepted(); | ||
| } |
There was a problem hiding this comment.
검증보다는, 특정 조건에 대한 전처리에 더 가까운 것 같아 validate라는 네이밍이 와닿지 않는 것 같습니다.
그리고 위 updateAccepted 메서드와 순서가 변경되면 더 좋겠네요!
There was a problem hiding this comment.
이전에 컨벤션 회의에서 값을 기본 설정해주는 메서드의 경우 processDefault를 붙이기로 했어서 컨벤션대로 수정했습니다!
| @@ -102,4 +105,15 @@ public ResponseEntity<Void> delete( | |||
| return ResponseEntity.noContent() | |||
| .build(); | |||
| } | |||
|
|
|||
| @PostMapping(value = "/{goalId}/accept", headers = "X-API-VERSION=1") | |||
There was a problem hiding this comment.
친구 수락의 경우 Patch였는데, 골 수락은 Post인 이유가 있을까요?
There was a problem hiding this comment.
patch 생각을 못하고 post로 설정했습니다.. patch로 변경하도록 하겠습니다!
| final List<GoalTeam> acceptedGoalTeam = 현재_진행중인_골1.getTeams() | ||
| .stream() | ||
| .filter(GoalTeam::isAccepted) | ||
| .toList(); |
There was a problem hiding this comment.
@Transactional을 사용하지 않기 위해선 GoalRepository 등을 @Autowired로 주입받아 조회를 통해 검증할 수 있습니다.
해당 경우 @Transactional이 없을 때 실패하는 이유는 서비스 메서드를 실행시켜 그에 대한 결과가 테스트 클래스까지 전파되기 위해선 하나의 트랜잭션이어야 하기 때문입니다.
다만, 이때 굳이 @Transactional을 사용해야 하는 가에 대한 고민이 필요할 것 같습니다.
저는 이번 프로젝트에서 @Transactional을 사용하지 않으면 좋겠다고 생각했는데, 메서드에 @Transactional을 붙이는 경우는 고려해보지 못했네요.
그래도 repository를 통해 변경된 데이터를 가져올 수 있으니, 사용하지 않는 것이 더 좋지 않을까 생각합니다.
그리고 메서드마다 매번 필요 여부를 따지는 것도 불편할 것 같아요.
사용하지 않으면 좋겠다고 처음 생각하게 된 이유의 참고 자료입니다.
효선님의 의견 알려주시면 감사하겠습니다!
There was a problem hiding this comment.
말씀주신대로 repository를 autowired로 주입시켜 repository로부터 변경내용을 가져와 테스트를 해보니 잘 돌아가네요! 다만 repository에서 findByIdAndDeletedIsFalse 메서드로 불러왔을 경우에 GoalTeam을 fetch join 해주지 않아 조건에 맞는 GoalTeam 리스트를 생성할 수 없어 fetch join 설정이 되어 있는 findByIdWithUserAndDeletedIsFalse로 불러와 테스트했습니다.
|
3/10
골 수락 기능 구현 완료했습니다. 이번주까지 시험이 있어서 진도를 많이 못나갔습니다.. 다음주부터 속도 내보겠습니다!
[주요 작업 내용]
[질문]
@Transactional을 테스트 메서드에 부여해주니 문제없이 잘 돌아가더라구요.. 이전까지 수정 기능 테스트하면서 트랜잭션 문제가 있었던 적이 없었는데 이번에 처음 발생한 문제라 정확한 원인을 좀 더 찾아보도록 하겠습니다. 우선 테스트 상으로는 문제가 없어 그대로 둘지 아니면 수정하는 것이 좋을지 질문드립니다!