-
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
[Feature/45] 보틀의 핑퐁 조회하기 #51
Conversation
|
||
var name: String? = null, | ||
var height: Int, |
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.
키가 추가되어서 슬쩍 끼워넣었습니다
이거부터 답변드리면, 보틀 수락할 때 질문 3개 뽑아서 넣도록 했어요!! 보틀 생성할 때 편지를 넣기에는 거절하는 경우도 많을 것 같아서 불필요한 데이터가 생성되는 것 같아서, 수락할 때 생성되는게 좋다고 생각했어요! |
오 좋아요! |
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.
고생하셨습니다!! 👏
수정할 부분이 살짝 있어서 RC 할게요!
@@ -38,4 +39,10 @@ class BottleController( | |||
fun refuseBottle(@PathVariable bottleId: Long) { | |||
bottleFacade.refuseBottle(bottleId) | |||
} | |||
|
|||
@ApiOperation("보틀의 핑퐁 조회하기") | |||
@GetMapping("/ping-pong") |
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.
여기에 path parameter가 빠졌어요!
|
||
fun getBottlePingPong(bottleId: Long): BottlePingpongResponseDto { | ||
val me = User(1L, LocalDate.of(2000, 1, 1), "보틀즈") // TODO: 회원 기능 갖추고 수정 | ||
val bottle = bottleService.getBottle(bottleId) |
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.
bottleSevice.getBottle
이 메서드는 status가 None이고 만료되지 않은 보틀을 가져와서 핑퐁 조회할 때는 적합하지 않아요!
메서드명이 명확하지 않아서 헷갈리긴 하네요 😅
return myLetter.letters.zip(otherLetter.letters).mapIndexed { index,(mySingleLetter, otherSingleLetter) -> | ||
|
||
PingPongLetter( | ||
order = index + 1, | ||
question = mySingleLetter.question, | ||
canshow = isFirstLetterOrPreviousBothAnswered(index, myLetter, otherLetter), | ||
myAnswer = mySingleLetter.answer, | ||
otherAnswer = otherSingleLetter.answer, | ||
shouldAnswer = mySingleLetter.answer == null, | ||
isDone = mySingleLetter.answer != null && otherSingleLetter.answer != null | ||
) | ||
} | ||
} |
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.
오 이부분 저는 엄청 헷갈리던데 잘하셨네요 👍👍
fun getKoreanAge(): Int { | ||
return LocalDate.now().year - birthdate.year + 1 | ||
} |
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.
좋아요 👍
fun findLetter(bottle: Bottle, user: User): Letter? { | ||
return letterRepository.findByBottleIdAndUserId(bottle.id, user.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.
보틀 수락할 때 letter가 무조건 생성되니까 return값이 Nullable하지 않아도 될 것 같아요. null이라면 예외처리를 해주는건 어떤가요?
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.
좋습니다!
val targetUserImageUrl: String? = null, | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "source_user_id") | ||
val sourceUser: User, | ||
|
||
val sourceUserImageUrl: String? = null, | ||
|
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.
엇 사진은 Letter에 있는데 일부러 여기로 옮기신건가요?!
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.
요거 돌려놓을게요!
data class BottlePingpongResponseDto( | ||
val userProfile: PingPongUserProfile, | ||
val introduction: List<QuestionAndAnswer>? = emptyList(), | ||
val letters: List<PingPongLetter> = emptyList(), | ||
val photo: Photo, | ||
val matchResult: MatchResult, | ||
) |
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.
API 명세서에 현재 종료된 핑퐁인지에 대한 여부를 반환하는 isStopped
가 있는데 추가해야 할 것 같아요!
그리고 누구에 의해서 종료됐는지도 화면에 표시가 되어서 그것도 추가해야할 것 같아요.
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.
넵 이거 추가할게요!
NONE, // 아직 결정하지 않은 상황 | ||
ACTIVE, // 보틀 핑퐁이 진행 중인 상황 | ||
STOPPED, // 보틀을 한쪽이 거절한 상황 | ||
MATCHED, // 매칭이 된 상황 |
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.
오 맞아요!!
@@ -20,7 +20,7 @@ class BottleController( | |||
@ApiOperation("홈 - 받은 보틀 목록 조회하기") | |||
@GetMapping | |||
fun getBottlesList(): BottleListResponseDto { | |||
return bottleFacade.getBottles() | |||
return bottleFacade.getNewBottles() |
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.
여긴 정확하게 말하면 NONE인 상태, 즉 받을지 말지 결정 안한애들만 보여주려고 하는거죠?
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.
넵 맞아요!
@@ -34,7 +34,7 @@ class BottleFacade( | |||
} | |||
|
|||
fun getBottle(bottleId: Long): BottleDetailResponseDto { | |||
val bottle = bottleService.getBottle(bottleId) | |||
val bottle = bottleService.getNotExpiredBottle(bottleId, setOf(PingPongStatus.NONE)) |
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.
오 너무 좋아요! 한번에 이해하기 좋네요 👍
코드 리뷰 반영했습니다! 아무래도 도메인이 조금씩 복잡해지고 시간관계상 디자인이 모든 영역을 다루지는 못하니 서로 다르게 생각하는 모델이 나올 수 있을거 같아요! 오프라인 때 같이 맞춰봐요! |
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.
더 이해하기 좋은 코드로 변경되어서 너무 좋네요 👍
고생하셨습니다!!
val me = User(1L, LocalDate.of(2000, 1, 1), "보틀즈") // TODO: 회원 기능 갖추고 수정 | ||
val bottle = bottleService.getNotExpiredBottle( | ||
bottleId, | ||
setOf(PingPongStatus.NONE, PingPongStatus.ACTIVE, PingPongStatus.STOPPED, PingPongStatus.MATCHED) |
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.
여기서 NONE인 상태는 가져오지 않아도 되지 않나요?
핑퐁을 하게되면 무조건 NONE은 아니기 때문에요!
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.
엇 근데 생각해보니까 expiredAt은 보틀 매칭되고 24시간 이후로 설정되어서 핑퐁 조회할 때는 expiredAt을 조건에 넣으면 안될 것 같아요!
cef8a3c
to
1226229
Compare
feat: 편지를 하나씩 공개하게 변경 feat: 코드리뷰 반영
💡 이슈 번호
close: #45
✨ 작업 내용
보틀의 핑퐁 조회하기를 구현했습니다
🚀 전달 사항
보틀 생성(매칭)시 편지를 넣어줄지 정해야 할거 같아요! 질문리스트는 미리 정해야해서