-
Notifications
You must be signed in to change notification settings - Fork 2
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 #75
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.
수고하셨습니다 커찬 몇 가지 의견을 남겼습니다:)
|
||
@GetMapping("/balances/rooms/{roomId}/members") | ||
public ResponseEntity<RoomMembersResponse> getAllBalanceGameRoomMember(@Positive @PathVariable Long roomId) { | ||
RoomMembersResponse response = roomService.findAllRoomMember(roomId); | ||
|
||
return ResponseEntity.ok(response); | ||
} |
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.
의견) 이거 제가 ResponseEntity 사용 관련해서 정해진게 없을 때 구현했던 코드인데 혹시 ResponseEntity로 감싼 부분 제거해주실 수 있는지 간곡한 요청을 드립니다..
private RoomContent findCurrentRoomContent(Long roomId) { | ||
Room room = roomRepository.getById(roomId); |
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.
질문) roomRepository에서 roomId로 이미 room을 찾아오는데 여기서 또 다시 조회하는 이유가 궁금합니다~!
이미 찾은 room만 받아도 되지않을까요?
@Test | ||
void 마지막_라운드_일_경우_예외를_던진다() { | ||
// given | ||
Room room = new Room(); | ||
goToFinalRound(room); | ||
|
||
// when & then | ||
assertThatThrownBy(() -> room.moveToNextRound()) | ||
.isInstanceOf(BadRequestException.class) | ||
.hasMessage("마지막 라운드입니다."); | ||
} | ||
|
||
private void goToFinalRound(Room room) { | ||
for (int round = START_ROUND; round < TOTAL_ROUND; round++) { | ||
room.moveToNextRound(); | ||
} | ||
} | ||
} |
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.
의견) 이후에 Room의 total_round를 생성자를 통해 입력하게 되면 이 테스트는 터지게될 수도 있으니 주의가 필요할 것 같네요.
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.
아마도 지금 사용하는 생성자를 유지하지 않을까 싶어요. 결국에는 초기 방 생성시에는 TOTAL_ROUND의 기본 값이 필요할 테니까요. 그리고 updateTotalRound()
형식으로 바뀌어지지 않을까요?
미래를 생각하는 힘드니, 지금 이 형식은 유지하고, 나중에 값이 바뀔 수 있을 때 조금 다듬어야 겠네요;;
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.
굿보이 꺽찬~~ 이든의 코멘트와 의견이 같아서 먼저 approve하겠슴니돠
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.
LGTM~ 수고했어 커찬~
RoomService, init.sql 및 일부 테스트 데이터 변경
Issue Number
Closed #63
As-Is
다음 라운드 넘어가는 API 필요
To-Be
다음 라운드 넘어가는 API 구현
Check List
Test Screenshot
(Optional) Additional Description
RoomService
에 생성한 로직이BalanceVoteService
와 동일한 코드가 존재