-
Notifications
You must be signed in to change notification settings - Fork 152
[4기 나영경] Springboot-jpa weekly 미션 1차 PR입니다. #323
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
base: na-yk
Are you sure you want to change the base?
Conversation
…1/CustomerRepositoryTest.java Co-authored-by: SR <[email protected]>
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.
리뷰 남겼습니다~
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/Customer.java
Outdated
Show resolved
Hide resolved
@Builder | ||
public Customer(String firstName, String lastName) { | ||
this.firstName = firstName; | ||
this.lastName = lastName; | ||
} | ||
|
||
public static CustomerResponse from(Customer customer){ | ||
return new CustomerResponse(customer.getFirstName(), customer.getLastName()); | ||
} |
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.
이건 별다른 이유는 없고, 페어프로그래밍을 하면서 각각을 사용해보는 연습을 하고 남겨둔 것이었습니다. 불필요한 부분은 삭제했습니다!
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/Customer.java
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/Customer.java
Outdated
Show resolved
Hide resolved
...ly/src/main/java/com/example/jpaweekly/domain/customer/controller/CustomerApiController.java
Outdated
Show resolved
Hide resolved
.build(); | ||
orderProducts.add(orderProduct); | ||
}); | ||
orderProductRepository.saveAll(orderProducts); |
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.
save를 한 번씩 호출하는 것보다 saveAll을 호출해주는 것이 더 좋은 것 같아요.
ID 생성 타입이 IDENTITY인 경우에는 bulk insert가 안된다고하니 참고하셔도 좋을 것 같습니다.
테스트 코드를 작성해서 쿼리가 어떻게 생성되는지 확인해보면 좋을 것 같아요
|
||
public Long createUser(UserCreateRequest request){ |
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 Long createUser(UserCreateRequest request){ | |
@Transactional | |
public Long createUser(UserCreateRequest request){ |
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.
제거해주세요~
...kly/src/test/java/com/example/jpaweekly/domain/customer/service/CustomerServiceImplTest.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/user/service/UserServiceImpl.java
Outdated
Show resolved
Hide resolved
refactor: name 변경 메서드 하나로 통일
jpa-weekly/src/main/java/com/example/jpaweekly/domain/BaseEntity.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/Customer.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/CustomerMapper.java
Outdated
Show resolved
Hide resolved
...ly/src/main/java/com/example/jpaweekly/domain/customer/controller/CustomerApiController.java
Outdated
Show resolved
Hide resolved
...ly/src/main/java/com/example/jpaweekly/domain/customer/controller/CustomerApiController.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/customer/dto/CustomerResponse.java
Show resolved
Hide resolved
...eekly/src/main/java/com/example/jpaweekly/domain/customer/repository/CustomerRepository.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/order/OrderStatus.java
Outdated
Show resolved
Hide resolved
jpa-weekly/src/main/java/com/example/jpaweekly/domain/order/service/OrderServiceImpl.java
Outdated
Show resolved
Hide resolved
...eekly/src/test/java/com/example/jpaweekly/domain/product/service/ProductServiceImplTest.java
Show resolved
Hide resolved
style: single class import로 변경
style: 불필요한 import 제거
} | ||
|
||
@PostMapping | ||
public ResponseEntity<Long> customerCreate(@RequestBody CustomerRequest request) { |
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.
이렇게하면 파라미터 validation이 이루어지나요?
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface CustomerRepository extends JpaRepository<Customer, Long> { | ||
String findByFirstName(String firstName); |
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.
이건 왜 String을 반환하나요?
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.
고생하셨습니다.
📌 과제 설명
👩💻 요구 사항과 구현 내용
Customer
엔티티로 미션1을 수행하였고, 추후Order
,Product
,User
엔티티로 미션3을 수행했습니다.orderId
,productId
,quantity
로 구성된OrderProduct
엔티티를 추가하여Order
생성 시 주문한 제품과 그 수를 저장할 수 있도록 했습니다.✅ PR 포인트 & 궁금한 점
createOrder
을 할 때 관련OrderProduct
를 생성하는데,save()
를 반복 호출하는 것이 좋지 않다고 생각해서saveAll()
을 이용해보았습니다. 적절한 사용인지 궁금합니다!