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

[4기 박은지] Springboot-jpa weekly 미션 1차 PR입니다. #307

Open
wants to merge 92 commits into
base: Eunji
Choose a base branch
from

Conversation

1o18z
Copy link

@1o18z 1o18z commented Jul 30, 2023

📌 과제 설명

미션1 : 2. JPA 소개(단일 엔티티를 이용한 CRUD를 구현)
미션2 : 3. 영속성컨텍스트(customer 엔티티를 이용하여 생명주기 실습)
미션3 : 4-2. 연관관계매핑(order, order_item, item의 연관관계 매핑 실습)

wisehero and others added 30 commits May 9, 2022 15:29
src/main/java/com/programmers/week/base/BaseEntity.java Outdated Show resolved Hide resolved
src/main/java/com/programmers/week/Message.java Outdated Show resolved Hide resolved
@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class OrderItem extends BaseEntity {
Copy link
Member

Choose a reason for hiding this comment

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

@ManyToMany 가 아닌 중간 관계 테이블을 엔티티로 직접 구현하는 이유가 있으신가요?

jpa:
open-in-view: false
hibernate:
ddl-auto: create
Copy link
Member

Choose a reason for hiding this comment

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

create 말고 다른 옵션들은 어떻게 있나요? create 쓴 이유가 뭘까요?

src/test/java/com/programmers/jpa/CustomerTest.java Outdated Show resolved Hide resolved
src/test/java/com/programmers/jpa/CustomerTest.java Outdated Show resolved Hide resolved
src/test/java/com/programmers/jpa/OrderTest.java Outdated Show resolved Hide resolved
@1o18z
Copy link
Author

1o18z commented Aug 4, 2023

JPA를 공부하고 적용하는 데에 생각보다 오래 걸리면서 시간을 맞춰 제출을 하려다 보니 많은 부분을 신경쓰지 않고 작성했던 것 같습니다 🥲
반성하면서 여러 번 검토하며 수정하고, 피드백 반영하였습니다 ❕👨🏻‍💻

@CreatedDate
private LocalDateTime createdAt;

@Column(columnDefinition = "TIMESTAMP")
Copy link
Member

Choose a reason for hiding this comment

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

timestamp precision에 대해서 알아보시면 좋을 것 같아요

Comment on lines 16 to 17
private static final int MIN_POWER = 0;
private static final long MAX_POWER = 1000000;
Copy link
Member

Choose a reason for hiding this comment

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

long 타입으로 통일해도 될 것 같아요!

src/main/java/com/programmers/week/item/domain/Item.java Outdated Show resolved Hide resolved
src/test/java/com/programmers/jpa/ItemTest.java Outdated Show resolved Hide resolved
Comment on lines 15 to 20
@DataJpaTest
public class CustomerRepositoryTest {

@Autowired
EntityManagerFactory emf;

Choose a reason for hiding this comment

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

@DataJpaTest를 사용하시는데 수동으로 트랜잭션을 시작하고 커밋할 필요가 있나요?

public CustomerResponse findById(Long id) {
return customerRepository.findById(id)
.map(CustomerResponse::from)
.orElseThrow(() -> new IllegalArgumentException(Message.CUSTOMER_IS_NO_EXIST));

Choose a reason for hiding this comment

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

어떤 id가 들어왔는지도 함께 남겨주면 좋을 것 같아요

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertFalse;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK)

Choose a reason for hiding this comment

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

웹 계층에 대한 테스트가 아닌데 이렇게 설정한 이유가 있나요?

Copy link
Member

@dojinyou dojinyou left a comment

Choose a reason for hiding this comment

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

승훈님 코멘트 챙겨주세요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants