-
Notifications
You must be signed in to change notification settings - Fork 4
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
이미지url에 id 대신 파일 이름을 사용하도록 변경 #717
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.
엔초 자잘한 수정 사항이 많았을텐데 정말 고생 많으셨습니다!
제 눈에 별다른 수정 사항은 보이지 않아 approve 합니다.
다만, 궁금한 점이 있어 질문 남겨두었으니 해당 부분만 확이해주시면 감사하겠습니다.
.leftJoin(chatRoom.buyer).fetchJoin() | ||
.leftJoin(chatRoom.auction, auction).fetchJoin() | ||
.leftJoin(auction.seller).fetchJoin() | ||
.join(chatRoom.buyer).fetchJoin() | ||
.join(chatRoom.auction, auction).fetchJoin() | ||
.join(auction.seller).fetchJoin() | ||
.leftJoin(auction.seller.profileImage).fetchJoin() | ||
.leftJoin(auctionImage).on(auctionImage.id.eq( |
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.
칭찬
👍👍👍
// TODO: 10/13/23 앞으로 id가 아닌 store name으로 진행하기로 했는데, 임시로 해둡니다. 추후 삭제해주시면 감사하겠습니다. | ||
public static final String DEFAULT_PROFILE_IMAGE_ID = "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.
칭찬
잊지않고 해주셨군요 👍
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.
칭찬
훌륭합니다 👍
JOIN FETCH q.writer | ||
JOIN FETCH q.writer w | ||
LEFT JOIN FETCH w.profileImage | ||
LEFT JOIN FETCH q.answer | ||
JOIN FETCH q.auction a | ||
JOIN FETCH a.seller | ||
JOIN FETCH a.seller s | ||
JOIN FETCH s.profileImage |
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.
칭찬
storename으로 변경되면서 fetch join을 해줘야 하게 됐군요..!
수정하시느라 고생 많으셨습니다 👍👍👍
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.
질문
제이미가 남겨주신 칭찬에 대한 질문입니다!!
이해한 게 맞는지 궁금해서.. 남겨요
storename은 id가 아니고, id가 아닌 다른 필드를 조회하기 위해 추가쿼리가 발생하기 때문에 fetch join을 해야한다고 이해했는데 맞나요?!
근데 엔초 진짜 꼼꼼하고 똑똑하네요
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.
넵 맞습니다!!
@Query(""" | ||
SELECT COUNT(auction_image) > 0 | ||
FROM AuctionImage auction_image | ||
WHERE auction_image.image.storeName = :storeName | ||
""") | ||
boolean existsByStoreName(final String storeName); |
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.
질문
네이밍만으로 충분해보이는데 @Query
로 해줘야 하나요?
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.
넵 AuctionImage 안에 Image가 임베디드로 있어서 JPQL 없이 메서드 네이밍만으로는 불가능합니다.
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 하겠습니다
제이미의 문서 최신화랑 충돌날 것 같은데 문서 충돌이야 뭐...괜찮겠죠
// TODO: 10/13/23 앞으로 id가 아닌 store name으로 진행하기로 했는데, 임시로 해둡니다. 추후 삭제해주시면 감사하겠습니다. | ||
public static final String DEFAULT_PROFILE_IMAGE_ID = "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.
칭찬
훌륭합니다 👍
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.
엔초 진짜 자잘한 수정사항이 많았는데 역시 꼼꼼하게 잘 해주셨군요
감사합니다
고생하셨습니다!!
@@ -9,6 +9,7 @@ public interface UserRepository { | |||
User save(final User user); | |||
|
|||
Optional<User> findById(final Long id); | |||
Optional<User> findByIdWithProfileImage(final Long 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.
필수
개행이 한 줄 필요해보입니당
public ResponseEntity<Resource> downloadProfileImage(@PathVariable Long id) throws MalformedURLException { | ||
final Resource resource = imageService.readProfileImage(id); | ||
@GetMapping("/users/images/{storeName}") | ||
public ResponseEntity<Resource> downloadProfileImage(@PathVariable String storeName) throws MalformedURLException { |
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.
필수
final이 누락되어버렸어요..!
public ResponseEntity<Resource> downloadAuctionImage(@PathVariable Long id) throws MalformedURLException { | ||
final Resource resource = imageService.readAuctionImage(id); | ||
@GetMapping("/auctions/images/{storeName}") | ||
public ResponseEntity<Resource> downloadAuctionImage(@PathVariable String storeName) throws MalformedURLException { |
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.
필수
final이 누락되어버렸어요..! 22
assertThat(actual.getId()).isPositive(); | ||
} | ||
|
||
|
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.
필수
이번엔 개행이 두 번 추가되었어요!
# Conflicts: # backend/ddang/src/main/java/com/ddang/ddang/user/presentation/dto/ReadUserResponse.java # backend/ddang/src/main/java/com/ddang/ddang/user/presentation/dto/response/ReadAuctionDetailResponse.java # backend/ddang/src/main/java/com/ddang/ddang/user/presentation/dto/response/ReadUserResponse.java # backend/ddang/src/main/resources/static/docs/docs.html
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.
확인 완료했어요~~ 고생하셨습니다 엔초
# Conflicts: # backend/ddang/src/main/java/com/ddang/ddang/chat/infrastructure/persistence/QuerydslChatRoomAndMessageAndImageRepository.java # backend/ddang/src/main/java/com/ddang/ddang/user/domain/repository/UserRepository.java # backend/ddang/src/main/java/com/ddang/ddang/user/infrastructure/persistence/UserRepositoryImpl.java # backend/ddang/src/test/java/com/ddang/ddang/chat/presentation/fixture/ChatRoomControllerFixture.java
📄 작업 내용 요약
🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
📎 Issue 번호