-
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
[FIX,CHORE] 방장과 관련된 로직 삭제 및 방 탈퇴 로직 수정 #107
Conversation
- 빈방일 경우 알람,룸 삭제 - 빈장이 아닐 경우 알람 삭제
|
||
} | ||
|
||
public boolean isEmptyRoom(Integer roomHeadCount) { |
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.
P3: 해당 부분은 Room 도메인 내부에 구현하는게 어떨까요? 도메인 내부에서 검증할 수 있을 것 같습니다.
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.
Room 도메인 내부에서 빈방검사를 구현하기 위해
RoomUser 리포지토리를 조회해서 방 인원을 파악하는 코드에서
Room의 roomUser로 방인원을 가져오게 수정해서 추가적인 변경사항이 있습니다.
추가로 해당방에 유저가 들어가 있는지 검사하는 것도 도메인 내부에 구현하게 수정했습니다.
(코멘트로 표시해놨습니다.)
확인하시고 이상없으면 머지하겠습니다!
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.
@05AM 머지하겠습니다.
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.
고생하셨습니다!
public void deleteUnnecessaryRoomAndAlarm(Room room, Integer userId) { | ||
Alarm alarm = alarmRepository.findByUser_IdAndRoom_Id(userId, room.getId()) | ||
.orElseThrow(() -> new BaseException(Status.ALARM_NOT_FOUND)); | ||
alarmRepository.delete(alarm); | ||
roomRepository.deleteById(roomId); | ||
if(room.isEmptyRoom()) | ||
roomRepository.delete(room); | ||
|
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.
코드 리뷰 반영한 부분
@Transactional | ||
public void leaveRoom(Integer roomId, Integer userId) { | ||
|
||
RoomUser roomUser = roomUserRepository.findByRoom_IdAndUser_Id(roomId, userId) | ||
.orElseThrow(() -> new BaseException(Status.ROOM_USER_NOT_FOUND)); | ||
if (Objects.isNull(roomUser)) { | ||
throw new BaseException(Status.ROOM_USER_NOT_FOUND); | ||
} | ||
|
||
Room room = roomRepository.findById(roomId) | ||
.orElseThrow(() -> new BaseException(Status.ROOM_NOT_FOUND)); | ||
|
||
if (roomUser.getIsHost()) { | ||
this.findNextCreatedUser(roomId, roomUser.getId()); | ||
if(!room.isUserInRoom(userId)) { | ||
throw new BaseException(Status.ROOM_USER_NOT_FOUND); | ||
} | ||
|
||
roomUserRepository.deleteByRoom_IdAndUser_Id(roomId, userId); | ||
deleteUnnecessaryRoomAndAlarm(room, userId); | ||
|
||
} |
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.
유저가 있는지 검사하는 것도 도메인 내부에 구현한 부분
확인 완료했습니다! |
☀️ 작업 사항
☀️ 관련 이슈
resolved : #101
☀️ 참고 사항
2023년 8월 17일에 삭제 여부 칼럼 이름을 is_deleted로 통일하기로 얘기를 했습니다.
다음에 작업할 때 알람의 is_activated를 is_deleted로 바꾸면 좋을 것 같습니다.
is_deleted는 1일때 삭제된 것인데 is_activated는 1일 때 삭제가 안된 거라서 헷갈리더라고요
수정할 때 참고하시면 좋을 것 같습니다.
저도 놓친 변경사항들 다시 보고 수정하겠습니다!