feat : 생협 운영시간 API 구현 #643
Conversation
| String location, | ||
|
|
||
| @Schema(example = "공휴일 휴무", description = "생협 매장 특이사항") | ||
| String remarks |
There was a problem hiding this comment.
C
remarks 라는 변수명이 뭔가 알기 어려운것 같아요
보통 비고란에 휴무일만 나오면 아예 restDay 로 바꿔도 될것 같아요
There was a problem hiding this comment.
휴무 뿐만 아니라 다른 특이사항들도 올 수 있어서 remakrs가 맞는거 같아요.
| } | ||
| ) | ||
| @Operation(summary = "모든 생협 매장 정보 조회") | ||
| @GetMapping("/coop") |
There was a problem hiding this comment.
A
@RequestMapping("/coop") 빼도 될것 같아요
|
|
||
| private final CoopShopService coopShopService; | ||
|
|
||
| @GetMapping("/coop") |
There was a problem hiding this comment.
A
@RequestMapping("/coop") 빼도 될것 같아요
그런데 CoopController도 이렇게 빼놔서 두 컨트롤러를 분리하는게 맞나 궁금해요
CoopController는 식단쪽이긴한데 다른 분들 의견이 궁금해요
There was a problem hiding this comment.
사용처를 생각해보면 일반 사용자들이 학식당 정보를 조회하는 경우잖아요?
저는 DiningContoller로 넘기는 것이 적절해 보입니다.
CoopController는 영양사님께서 사용하시는 기능이 들어가는 게 좋아 보여요
+) 혹시 CoopApi에서 아래 부분 수정해주실 수 있나요?
There was a problem hiding this comment.
학식당 정보도 있지만 생협의 여러 매장 정보 조회에 대한 API라 Coop이나 Dining이랑은 분리하는게 맞다고 생각합니다!
coop이 원래는 생협을 뜻하는 단어인데 영양사님 페이지에 써버려서.. 혹시 /coopshop 로 바꿔서 따로 유지하는건 어떻게 생각하시나요?
오너 부분은 수정할게용
There was a problem hiding this comment.
이거 지금 플라이웨이에 파일 많이 생겨서 올리기전에 버전 확인하고 올려야할것 같아요
| String location, | ||
|
|
||
| @Schema(example = "공휴일 휴무", description = "생협 매장 특이사항") | ||
| String remarks |
There was a problem hiding this comment.
휴무 뿐만 아니라 다른 특이사항들도 올 수 있어서 remakrs가 맞는거 같아요.
| private final CoopShopRepository coopShopRepository; | ||
|
|
||
| public List<CoopShopResponse> getCoopsShops() { | ||
| return coopShopRepository.findAll().stream() |
There was a problem hiding this comment.
C
이렇게 반환하면 isDeleted가 true인 값들도 반환되는거 아닌가요?
| @GetMapping("/admin/coop") | ||
| public ResponseEntity<AdminCoopShopsResponse> getCoopsShops( | ||
| @RequestParam(name = "page", defaultValue = "1") Integer page, | ||
| @RequestParam(name = "limit", defaultValue = "10", required = false) Integer limit, | ||
| @RequestParam(name = "is_deleted", defaultValue = "false") Boolean isDeleted, | ||
| @Auth(permit = {ADMIN}) Integer adminId | ||
| ) { | ||
| AdminCoopShopsResponse coopShops = adminCoopShopService.getCoopsShops(page, limit, isDeleted); | ||
| return ResponseEntity.ok(coopShops); | ||
| } |
There was a problem hiding this comment.
A
페이징은 컨트롤러 파라미터단에서부터 쉽게 받을 수 있도록 제공했던 것 같은데 파라미터 네이밍같은게 어긋나나요? 사용하지 않은 이유가 있는지 궁금합니다.
There was a problem hiding this comment.
찾아보니 admin도 Pageable을 이용해서 구현할 수는 있는데
기존의 admin에서 인자들을 따로 받아 사용하는 이유는 Criteria 라는 클래스를 이용해 페이지나 표시 크기의 범위를 검증하는 단계를 추가하기 위함인 것 같습니다!
Criteria criteria = Criteria.of(page, limit, total);
public static Criteria of(Integer page, Integer limit, Integer total) {
return new Criteria(validateAndCalculatePage(page, limit, total), validateAndCalculateLimit(limit));
}
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class AdminCoopShopController implements AdminCoopShopApi { |
There was a problem hiding this comment.
C
admin api의 수요가 있었나요? 별도로 만든 이유가 궁금합니다.
There was a problem hiding this comment.
이전 회의때 운영시간 관리를 어드민으로 하자는 의견이 나와서 만들었습니다!
|
|
||
| private final CoopShopService coopShopService; | ||
|
|
||
| @GetMapping("/coop") |
There was a problem hiding this comment.
사용처를 생각해보면 일반 사용자들이 학식당 정보를 조회하는 경우잖아요?
저는 DiningContoller로 넘기는 것이 적절해 보입니다.
CoopController는 영양사님께서 사용하시는 기능이 들어가는 게 좋아 보여요
+) 혹시 CoopApi에서 아래 부분 수정해주실 수 있나요?
| private boolean isDeleted = false; | ||
|
|
||
| @Builder | ||
| public CoopOpen( |
| @OneToMany(mappedBy = "coopShop", orphanRemoval = true, cascade = {PERSIST, REFRESH, MERGE, REMOVE}) | ||
| private List<CoopOpen> coopOpens = new ArrayList<>(); |
There was a problem hiding this comment.
A
cascade를 ALL로 두지 않은 이유가 있는 건가요??
There was a problem hiding this comment.
사용하는 type들만 명시적으로 하려고 따로 적었습니다!
| private List<CoopOpen> coopOpens = new ArrayList<>(); | ||
|
|
||
| @Builder | ||
| public CoopShop( |
🔥 연관 이슈
🚀 작업 내용
💬 리뷰 중점사항