-
Notifications
You must be signed in to change notification settings - Fork 4
[#102] 카페 메뉴 조회하기 및 삭제하기 #109
base: develop
Are you sure you want to change the base?
Conversation
return ResponseEntity.ok().body(AllMenu); | ||
} else { | ||
LOGGER.info("해당 카페의 전체 메뉴를 조회할 수 없습니다. cafeId = {}", cafeId); | ||
return ResponseEntity.badRequest().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.
실제로 메뉴가 없는경우도 있을수 있는데 400에러를 내리는건 안될것 같습니다.
@@ -24,4 +34,19 @@ public boolean updateMenu(MenuDTO menuDTO) { | |||
int updateMenu = menuMapper.updateMenu(menuDTOToMenuConverter.convert(menuDTO)); | |||
return updateMenu == 1; | |||
} | |||
|
|||
public List<MenuDTO> getAllMenu(long cafeId) { | |||
List<MenuDTO> selectAllMenu = menuMapper.selectAllMenu(cafeId); |
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.
cafeId 의 유효성 검증이 필요해보입니다.
(존재하는 카페인지 등의)
MenuDTO menuDTO = menuService.getMenu(menuId); | ||
if (menuDTO == null) { | ||
LOGGER.info("해당 메뉴를 조회할 수 없습니다. cafeId = {}, menuId = {}", cafeId, menuId); | ||
return ResponseEntity.badRequest().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.
역시 이곳에서도 메뉴가 없다고 해서 400 에러를 내면 안될것 같습니다.
이런경우는 리소스를 찾을수 없는 404에러가 적합합니다.
return ResponseEntity.ok().body(AllOption); | ||
} else { | ||
LOGGER.info("해당 메뉴의 전체 옵션을 조회할 수 없습니다. cafeId = {}, menuId = {}", cafeId, menuId); | ||
return ResponseEntity.badRequest().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.
여기도 위와 동일
} | ||
|
||
public boolean deleteMenu(long menuId) { | ||
int deleteMenu = menuMapper.deleteMenu(menuId); |
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.
해당 메뉴가 존재하는지 확인후 없다면 404를 내리고 있는데 삭제를 실패한 경우면 500에러를 내리는게 더 좋을것 같습니다.
@@ -24,4 +25,19 @@ public boolean updateOption(OptionDTO optionDTO) { | |||
int updateOption = optionMapper.updateOption(optionDTOToOptionConverter.convert(optionDTO)); | |||
return updateOption == 1; | |||
} | |||
|
|||
public List<OptionDTO> getAllOption(long menuId) { | |||
List<OptionDTO> selectAllOption = optionMapper.selectAllOption(menuId); |
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.
menuId에 대한 검증부터 하는것이 좋을것 같습니다.
Fixes #102
Fixes #86
Fixes #83
개요
작업사항