Skip to content
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: product tag 생성 #72

Open
wants to merge 3 commits into
base: week7
Choose a base branch
from
Open

feat: product tag 생성 #72

wants to merge 3 commits into from

Conversation

canyos
Copy link
Contributor

@canyos canyos commented Oct 22, 2024

해결하려는 문제가 무엇인가요?

어떻게 해결했나요?

  • 간단하게 만들어봣습니다

코드 리뷰시 요청 사항

  • enum select가 안되길래 native로 짰는데 jpql로 하는 방법 아시면 알려주세욥

더 하고 싶은 말

@canyos canyos self-assigned this Oct 22, 2024
@stopmin stopmin added 🎉 feature 기능 개발 😐 P2 우선순위 보통 labels Oct 28, 2024
Copy link
Contributor

@stopmin stopmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 product tag는 그냥 product에 tag를 enum으로 두도록 하는건 어떤지 싶습니다!!! 이것까지 Entity로 관리하면 조금 오버 엔지니어링 같은 느낌이 드네요😢

@canyos
Copy link
Contributor Author

canyos commented Oct 29, 2024

@stopmin enum으로 관리하게 바꿔봤습니다

@@ -64,6 +70,12 @@ public class Product {
@JoinColumn(name = "entityId")
List<Review> reviewList = new ArrayList<>();

@ElementCollection(fetch = FetchType.EAGER)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 Lazy대신 Eager로 설정하신 이유가 궁금합니다!!!

@@ -4,7 +4,7 @@

public record ReviewResponse
(Long id,
Long productId,
Long entityId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

product, farm 둘다 쓸 ㅇ예정이라 Entity로 네이밍을 수정하신건지 궁금합니다!

Copy link
Contributor

@jjt4515 jjt4515 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 product랑 farm이랑 id값이 중복될 수 있으니 따로 type을 받아야겠다 생각했는데
ReviewRepository에서
@query("select r from Review r where r.entityId = :productId and r.entityType = 'PRODUCT'")
와 같은 쿼리문 써서 type 받을 필요도 없도록 잘 구현해주셨네요 하나 배웠습니다~!

다만 리뷰를 가지는 엔티티가 많을 경우에는 type을 받으면 하나의 로직으로 공통화하기 좋을 것 같기는 하네여


public enum ProductTagEnum {
ORGANIC("유기농"),
NonPesticide("무농약");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum이니깐
NON_PESTICIDE("무농약"); 으로 쓰는 것은 어떨까요?

}

private Product CheckProduct(ProductTagRequest productTagRequest) {
return productRepository.findById(productTagRequest.productId())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findByIdAndDeletedAtIsNull로 바꾸는 것은 어떨까요?
혹시 제가 틀린 거라면 바로 피드백 주시면 감사하겠습니당

Product product = CheckProduct(productTagRequest);

ProductTagEnum productTagEnum = null;
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 addTag에 있는 로직과 동일해서 공통화시키면 좋을 것 같다는 생각이 듭니당

@@ -4,7 +4,7 @@

public record ReviewResponse
(Long id,
Long productId,
Long entityId,
Copy link
Contributor

@jjt4515 jjt4515 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 product랑 farm이랑 id값이 중복될 수 있으니 따로 type을 받아야겠다 생각했는데
ReviewRepository에서
@query("select r from Review r where r.entityId = :productId and r.entityType = 'PRODUCT'")
와 같은 쿼리문 써서 type 받을 필요도 없도록 잘 구현해주셨네요 하나 배웠습니다~!

다만 리뷰를 가지는 엔티티가 많을 경우에는 type을 받으면 하나의 로직으로 공통화하기 좋을 것 같기는 하네여

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 feature 기능 개발 😐 P2 우선순위 보통
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants