-
Notifications
You must be signed in to change notification settings - Fork 152
[4기 고범준] Springboot-jpa weekly 미션 1차 PR입니다. #322
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: K-jun98
Are you sure you want to change the base?
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.
테스트 코드도 작성해보면 좋을 것 같아요~
@Configuration | ||
@EnableJpaAuditing | ||
public class JpaConfig { | ||
} |
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.
👍
import static org.springframework.http.HttpStatus.OK; | ||
|
||
@RestController | ||
@RequestMapping("/api") |
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.
/api/customers
까지 해줘도 좋을 것 같네요
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.
수정 완료했습니다!
cf1bb4c
@PatchMapping("/customers/{id}") | ||
@ResponseStatus(OK) | ||
public ResponseEntity<Void> updateCustomer(@PathVariable Long id, @RequestBody CustomerUpdateRequest request) { | ||
customerService.update(id, request); | ||
return ResponseEntity.noContent().build(); | ||
} |
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.
이렇게되면 상태코드가 200, 204 중에 어느 것이 응답되나요?
명확하게 정해주는 것이 좋을 것 같아요
그리고 ResponseEntity
모두 사용거나 그렇지 않거나 통일하면 좋을 것 같아요
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.
변경 완료했습니다..!
3e5790d
|
||
@PatchMapping("/customers/{id}") | ||
@ResponseStatus(OK) | ||
public ResponseEntity<Void> updateCustomer(@PathVariable Long id, @RequestBody CustomerUpdateRequest 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.
@Vaild
가 필요해 보이네요
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.
변경 완료했습니다!
0e18423
public void update(String firstName, String lastName) { | ||
this.firstName = firstName; | ||
this.lastName = lastName; | ||
} |
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.
Request DTO에서 @blank 를 통해 공백역시 통과되지 않게 하고있습니다. 도메인에서의 검증도 레이어가 다르기 때문에 하는게 좋다고는 생각되나 프로젝트가 작아서 생략했습니다,,
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.
길이 값에 대한 검증도 진행되나요?
src/main/resources/application.yml
Outdated
properties: | ||
hibernate: | ||
format_sql: true | ||
jdbc: |
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.
변경 완료했습니다!
c395fda
@PatchMapping("/{id}") | ||
public ResponseEntity<Void> updateCustomer(@PathVariable Long id, | ||
@RequestBody @Valid CustomerUpdateRequest request) { | ||
customerService.update(id, request); | ||
return ResponseEntity.noContent().build(); | ||
} |
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.
@ResponseStatus(NO_CONTENT)
에 void
타입을 리턴하는 것이 다른 메서드들과의 일관성이 있는 것 같은데 어떻게 생각하시나요?
public void update(String firstName, String lastName) { | ||
this.firstName = firstName; | ||
this.lastName = lastName; | ||
} |
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.
길이 값에 대한 검증도 진행되나요?
private Customer getCustomerObject(Long id) { | ||
Customer customer = customerRepository.findById(id) | ||
.orElseThrow(CustomerNotFoundException::new); | ||
return customer; | ||
} |
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.
바로 반환해도 될 것 같아요!
📌 과제 설명