-
Notifications
You must be signed in to change notification settings - Fork 0
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
[fix] 식당에 메뉴 1개 남았을 때 메뉴 삭제 시, 식당 삭제 #216
Conversation
@@ -48,11 +51,21 @@ public MenusPostResponse createMenus(final MenusPostCommand command) { | |||
return MenusPostResponse.of(menus); | |||
} | |||
|
|||
private void deleteStore(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.
emptyMenuInStore 함수의 매개변수는 Long인데 혹시 둘이 다르게하신 이유가 있을까요? 아니면 통일하는게 좋을 것 같습니당
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.
이미 함수명이 deleteStore이라서 굳이 storeId라고 중복명시하는 것 보다는 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.
아 자료형을 말한 것이었습니다 통일성을 위해 Long/long 중에 하나로 하는게 좋지 않을까해서요
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.
아 저때는 검증을 마치고 난 후라 wrapper 타입 보다는 원시타입인 long으로 처리하는 것이 낫다고 생각했어요! 둘 다 원시 타입으로 통일하겠습니당
@Valid @RequestBody final MenuPostRequest request) { | ||
menuCommandService.modifyMenu(MenuPatchCommand.of(storeId, id, request.name(), request.price())); | ||
return HankkiResponse.success(CommonSuccessCode.OK); | ||
} | ||
|
||
@PostMapping("{storeId}/menus/bulk") | ||
public HankkiResponse<MenusPostResponse> createMenu(@PathVariable final Long storeId, @Valid @RequestBody final List<MenuPostRequest> request) { | ||
public HankkiResponse<MenusPostResponse> createMenu(@PathVariable @Min(value = 1L) final long storeId, |
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.
메뉴 여러개 만드는 거라 복수인 Menus로 네이밍해도 될 것 같아융
Related Issue 📌
close #215
Description ✔️
To Reviewers
api 테스트 완료