-
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
✨ [Feature] 마신 음료에 대한 당 기록 기능을 구현했어요. #50
Conversation
@Builder | ||
public record DrinkRecordSaveRequest( | ||
|
||
@NotNull | ||
@Schema(description = "음료 ID", example = "1") Long id, | ||
|
||
@NotNull |
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.
Notion 명세서에서는 drinkId로 나와있는데 통일하면 좋을것 같아요~
@NotNull | ||
@Min(0) | ||
@Schema(description = "1잔 기준 최종 섭취 당 함량", example = "10") Integer intakeSugar | ||
|
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.
해당 부분도 Notion 에는 finalSugar 로 나와있네요~
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.
아 넵 노션에 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.
id 는 drinkId 가 더 직관적인 것 같아서 이렇게 수정했고, 섭취한 당의 경우 RecordEntity 의 필드값과 통일하는 것이 좋을듯 하여 intakeSugar 로 변경했습니다.
@Override | ||
@Transactional(readOnly = true) | ||
public RecordEntity findRecordById(Long id) { | ||
return queryFactory.selectFrom(recordEntity) | ||
.join(recordEntity.drink, drinkEntity).fetchJoin() | ||
.join(recordEntity.drink.franchise, franchiseEntity).fetchJoin() | ||
.where(recordEntity.member.id.eq(id)) | ||
.fetchOne(); | ||
} | ||
} |
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.
Optional 처리는 필요가 없을까요??
@Override | |
@Transactional(readOnly = true) | |
public RecordEntity findRecordById(Long id) { | |
return queryFactory.selectFrom(recordEntity) | |
.join(recordEntity.drink, drinkEntity).fetchJoin() | |
.join(recordEntity.drink.franchise, franchiseEntity).fetchJoin() | |
.where(recordEntity.member.id.eq(id)) | |
.fetchOne(); | |
} | |
} | |
@Override | |
@Transactional(readOnly = true) | |
public Optional<RecordEntity> findRecordById(Long id) { | |
RecordEntity recordEntity = queryFactory.selectFrom(QRecordEntity.recordEntity) | |
.join(QRecordEntity.recordEntity.drink, QDrinkEntity.drinkEntity).fetchJoin() | |
.join(QRecordEntity.recordEntity.drink.franchise, QFranchiseEntity.franchiseEntity).fetchJoin() | |
.where(QRecordEntity.recordEntity.member.id.eq(id)) | |
.fetchOne(); | |
return Optional.ofNullable(recordEntity); | |
} | |
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.
넵넵 당장은 비즈니스 로직에 사용하지 않지만 추후 활용을 위해 Optional 처리를 해주는 것이 좋을 것 같습니다.
수정하겠습니다!
❗️관련 이슈번호
Close #41
⚙️ 어떤 기능을 개발했나요?
🙋🏻 어떤 부분에 집중하여 리뷰해야 할까요?