[team6-BE] Jung, yeon 1차 PR #37
Conversation
kimnayeon0108
changed the title
There was a problem hiding this comment.
안녕하세요! Jung & Yeon!! 리뷰어 Dion입니다.
두 번째 프로젝트를 진행해주시느라 고생이 많으십니다.
객체와 DB 설계가 조금 아쉬운 부분이 있었어요. (String으로 저장해버리는 경우)
그래도 어느정도 API 설계가 되어있어서 빠르게 수정가능한 부분만 수정해주시면 좋을 것 같습니다.
OOP를 좀 더 시도해봐주시면 좋을 것 같아요.
그리고 예외 처리도 시도해주시면 좋을 것 같습니다.
RequestDishDTO 객체를 생성하는 메소드(createDish)를 Dish, DishDTO 중 어느 클래스에 넣을지 고민했습니다. 일단 Entity는 서비스, DTO와 별개로 존재해야된다는 점에서 DTO 클래스에 넣는것이 맞을 것 같아 그렇게 했습니다.
어떤 코드는 Entity에 DTO가 드러나고, 어떤 코드는 드러나지 않더라고요.
답을 드리자면, 변경되기 쉬운쪽에 위치하는 것이 맞습니다. Entity보다는 DTO가 변경가능성이 높으니 DTO에 위치하는 것이 좋겠죠?
Entity → DTO 변환, DTO → Entity 변환 모두 DTO 쪽에서 수행해주거나, 중간계층을 거치는 것이 맞겠습니다.
고생하셨습니다~
궁금하신점이나 도움이 필요한 부분, 이해가 되지 않는 부분 등은 코멘트 남겨주세요~
| HELP.md | ||
| .idea/ | ||
| .gradle | ||
| build/ | ||
| !gradle/wrapper/gradle-wrapper.jar | ||
| !**/src/main/**/build/ | ||
| !**/src/test/**/build/ | ||
|
|
There was a problem hiding this comment.
gitignore가 제대로 적용이 되지 않았는지 .idea/ 에 존재하는 파일들이 버전관리 되고 있네요.
제거해주시는 편이 좋고, 항상 버전관리 되지 않아야 하는 파일들이 커밋에 포함되는지 확인해주세요.
There was a problem hiding this comment.
또, 추가로 환경별로 제외될 수 있는 파일이 있을 수 있는데요.
gitignore.io에 가보시면 여러 템플릿이 있으니, 적용해보시는 것도 추천드립니다.
There was a problem hiding this comment.
BE 디렉토리 바깥에 있는 gitignore파일에 제거해야할 파일들을 추가해줬는데, 그래도 .idea폴더가 사라지지 않았습니다.
연이 열심히 검색하셔서 캐시 문제라는 것을 알아내셔서 삭제에 성공했습니다!
출처에서 시키는대로 하니까 되더라고요 ㅎㅎ
|
|
||
| ## 메인페이지 | ||
|
|
||
| - /banchan-code/main |
There was a problem hiding this comment.
banchan-code/main 보다는 sidedishes/main 이 더 좋지 않을까 하는 생각이 드네요.
There was a problem hiding this comment.
API는 화면의 구성을 따라가도 되지만, 따라가지 않아도 됩니다.
API를 사용하는건 일반 사용자가 아니니까, API 사용자 관점에서 디자인해보세요.
There was a problem hiding this comment.
banchan-code가 원래 저희가 합의한 서비스 이름이어서 url에 담았었습니다.
그런데 말씀해주신대로 서비스이름을 API url설계에 담을 필요가 없더라고요.
IOS분들과 합의한 내용이 있어서 이번에는 이렇게 진행하고 앞으로 말씀해주신대로 적용해보겠습니다!
| "name": "[미노리키친] 규동 250g", | ||
| "top_image": "http://public.codesquad.kr/jk/storeapp/data/detail/HBDEF/4cce011a4a352c22cd399a60271b4921.jpg", | ||
| "description": "일본인의 소울푸드! 한국인도 좋아하는 소고기덮밥", | ||
| "prices": [6500, 5200], |
There was a problem hiding this comment.
@ksundong DB설계가 아쉽다고 하셔서 Badge, ThumbImage, Price, DetailImage 클래스를 만들고 테이블도 생성해서 Dish와 관계를 맺도록 했습니다! 그리고 지금 API를 변경하면 내일 데모때 영향이 갈거 같아서.... 다음부터는 객체로 반환하도록 API 설계 해보겠습니다!
| import sidedish.service.dto.CategoryDTO; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/banchan-code") |
There was a problem hiding this comment.
위에서도 말씀드렸듯, 더 좋은 Path를 고민해주세요.
개인적으로 한글을 그대로 음으로 가져다 쓰는 것은 좋지 않다고 생각됩니다.
There was a problem hiding this comment.
위에서 말씀드렸듯이 banchan-code는 저희의 서비스 이름입니다..ㅎㅎ naver.com에서 naver 부분이고 이걸 api 설계의 url에 담을 필요가 없는데 잘못생각했네요!
| } | ||
|
|
||
| @GetMapping("/{title}") | ||
| public ResponseEntity<CategoryDTO> mainPage(@PathVariable String title) { |
There was a problem hiding this comment.
ResponseEntity를 사용해주신 이유는 무엇인가요?
There was a problem hiding this comment.
@ksundong 사실.... 이전 미션에서 계속 이렇게 해와서 ResponseEntity를 사용한거 같습니다. 이부분 더 학습이 필요할듯 합니다
There was a problem hiding this comment.
이전 미션에서 계속 사용해왔었는데, header, body, status를 전부 반환해줄 수 있다고 해서 쓰고 있었습니다!
아직 헤더에 무언가를 담아본적은 없지만, ok 리스폰스와 함께 DTO를 반환하려고 사용하고 있었습니다.
사실 다른 대체재가 무엇이 있는지 잘 몰라 계속 이것을 사용한 것 같은데, 추가적인 공부가 필요할 것 같네요..ㅎㅎ
There was a problem hiding this comment.
Q&A 할 때 잠깐 API를 만들었었는데 그걸 잘 떠올려보세요.
There was a problem hiding this comment.
CategoryDTO, DishDTO를 리턴하도록 수정했습니다!
| @JsonPropertyOrder({"id", "name","top_image", "description", "prices", "badges", | ||
| "stock" , "point", "delivery_info", "thumb_images", "detail_images"}) |
There was a problem hiding this comment.
@JsonProperty애노테이션을 추가해서 리스폰스 값들이 스네이크 케이스를 따르도록 설정하니까, 애노테이션을 추가한 값들이 맨 아래로 내려가는 문제가 생겼습니다. ios분들과 합의한 순서에 맞지 않아 순서를 지정해줬습니다.
리스폰스된 json 값에서 dish의 어떤 속성이 먼저 오는지 순서가 중요한 것 같지는 않았는데, 그래도 합의한 것을 따르는 것이 좋을 것 같아 이렇게 했습니다.
There was a problem hiding this comment.
생각해보면 순서는 별로 중요하지 않습니다.
key, value 형태이기 때문이에요.
| private Long id; | ||
| private String name; | ||
|
|
||
| @JsonProperty("top_image") |
There was a problem hiding this comment.
이렇게 일일이 정의해주는 것도 괜찮지만,
전체가 공통으로 설정을 공유한다면
Json Parser의 설정을 바꿔보세요. 힌트는 jackson, ObjectMapper입니다~
There was a problem hiding this comment.
감사합니다! application.properties에 spring.jackson.property-naming-strategy=SNAKE_CASE 추가해서 해결했습니다 ㅎㅎ
| import java.util.List; | ||
|
|
||
| @JsonPropertyOrder({"id", "name","top_image", "description", "prices", "badges", | ||
| "stock" , "point", "delivery_info", "thumb_images", "detail_images", "category"}) |
| public Dish createDish() { | ||
| return new Dish(name, topImage, description, ConvertUtils.convertToString(prices), ConvertUtils.convertToString(badges), | ||
| stock, deliveryInfo, ConvertUtils.convertToString(thumbImages), ConvertUtils.convertToString(detailImages)); | ||
| } |
| spring.datasource.schema=classpath:schema.sql | ||
| spring.datasource.data=classpath:data.sql |
There was a problem hiding this comment.
schema.sql data.sql외의 이름을 사용할 때 저 부분을 application.properties에 추가해주는 것이고,
기본설정대로 schema.sql, data.sql으로 이름을 정했으면 필요없는거군요!!
처음 알았어요 감사합니당ㅎㅎ
rename ConvertUtils to TypeConvertUtils to show the purpose of the class # Please enter the commit message for your changes. Lines starting
add NotFoundException and make DishNotFoundException and CategoryNotFoundException extend NotFoundException. add ExceptionHandler to handle NotFoundException
| if(!dishes.contains(dish)) { | ||
| throw new DishNotFoundException(); | ||
| } return true; |
There was a problem hiding this comment.
| if(!dishes.contains(dish)) { | |
| throw new DishNotFoundException(); | |
| } return true; | |
| if (!dishes.contains(dish)) { | |
| throw new DishNotFoundException(); | |
| } | |
| return true; |
| package sidedish.exception; | ||
|
|
||
| public class CategoryNotFoundException extends NotFoundException { | ||
| private static final String CATEGORY_NOT_FOUND_MESSAGE = "해당 카테고리가 존재하지 않습니다!"; |
| Category category = categoryRepository.findById(dishDTO.getCategoryId()).orElseThrow(CategoryNotFoundException::new); | ||
| Dish dish = dishDTO.createDish(); | ||
| category.addDish(dish); | ||
| categoryRepository.save(category); |
There was a problem hiding this comment.
개행을 통해서 논리의 단위를 나눠주셔야 가독성이 높아집니다.
There was a problem hiding this comment.
@ksundong 네엡 변경했습니다. 이부분도 더 신경쓰겠습니다!
| public static List<Price> convertToPriceList(List<Integer> prices) { | ||
| return prices.stream() | ||
| .map(Price::new) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<Badge> convertToBadgeList(List<String> badges) { | ||
| return badges.stream() | ||
| .map(Badge::new) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<ThumbImage> convertToThumbImageList(List<String> images) { | ||
| return images.stream() | ||
| .map(ThumbImage::new) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<DetailImage> convertToDetailImageList(List<String> images) { | ||
| return images.stream() | ||
| .map(DetailImage::new) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<Integer> convertPriceList(List<Price> prices) { | ||
| return prices.stream() | ||
| .map(Price::getPrice) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<String> convertBadgeList(List<Badge> badges) { | ||
| return badges.stream() | ||
| .map(Badge::getBadge) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<String> convertThumbImageList(List<ThumbImage> thumbImages) { | ||
| return thumbImages.stream() | ||
| .map(ThumbImage::getThumbImage) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public static List<String> convertDetailImageList(List<DetailImage> detailImages) { | ||
| return detailImages.stream() | ||
| .map(DetailImage::getDetailImage) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
@ksundong 일급 컬렉션에 대해 학습하고 Prices, Badegs, ThumbImages, DetailImages 클래스를 생성해서 적용해보았습니다!
| public RequestDishDTO() { | ||
| } |
There was a problem hiding this comment.
@ksundong 저희가 dish의 모든 데이터를 Post방식으로 한번에 날려서 데이터를 저장하고 있는데, 이때 이부분이 없으면 Cannot construct instance of 'sidedish.service.dtoRequestDishDTO' (no Creators, like default constructor, exist) 에러가 발생해서 선언했습니다!
| @JsonProperty("category") | ||
| private Long categoryId; |
There was a problem hiding this comment.
@ksundong 아예 categoryId로 속성값을 지정하는게 더 좋을까요??
|
@ksundong 말씀해주신 부분 수정했습니다! 추가적으로 추상 클래스를 추가해 중복되는 부분을 제거했습니다. |
* [#1] feat : create CRA - CRA를 설치했습니다 * [#1] feat : mediumCard index.jsx prop 추가 - 상동 * [#1] feat : node-sass 추가 - node sass를 추가했습니다 * [#1] feat : 폴더 구조 정리 - atoms, molecules, images 등 파일 구조 수정했습니다 * [#5] feat : reset scss 추가 - reset scss 추가 - 기본 글꼴 추가 * [#5] feat : button, icon, tag components 추가 - atoms의 button, icon, tag component를 재사용 가능한 형태로 생성했습니다. * [#5] feat: set moculses structure - LargeCard 구조설정 - MediumCard 구조 설정 * [#5] chore : 잘못 생성된 파일 수정 - 파일 명을 sidedish -> frontend로 바꾸는 과정에서 폴더 구성이 꼬여서 수정 * [#5] feat: molcules- LargeCard,MediumCard 구조 생성 * [#13] feat : MainDish, More 컴포넌트 생성 - 신규 컴포넌트 생성: maindish, more organisms * [#11] feat: fetch를 위한 util dir 생성 - util/loadData 생성 : fetch로 data가져오기 - util/url 생성: 기본 url주소 변수 설정 * [#11] feat: HeaderLeft 생성 * [#13] feat : MediumCard TagType 추가 - 메인요리에 medium카드를 사용하기 위해 medium카드 일부 속성을 수정했습니다. * [#13] feat : useFetch 컴포넌트 추가 - fetch기능을 분리하기 위해 util-useFetch컴포넌트를 생성 * [#11] feat: Header 생성 - HeaderLeft 구조 생성 - HeaderRight 구조 생성 - BestDish 구조 생성 중 * [#11] feat: useFatch fetch오류 해결 * [#12] feat: BestDish useTabs hook 사용 * [#13] feat : 카드 캐로셀 기능 완성 - setTimeout으로 렌더링을 지연시켜 캐로셀 기능을 완성 * [#12] feat: BestDish UI 및 기능구성 - Tab 버튼 클릭시 rendering - Header Style 일부 수정 (HeaderRight,HeaderLeft,Span,Icon) * deploy : build 210423 * [#13] feat: CSS style 수정 - innerTitle 설정 * chore: add gitginore * chore: Add basic gradlew files * [#13] feat: MainDish,SideDish,BestDish CSS style디테일 설정 - mainDish의 <Icon>,<Image> margin 설정 * [#13] fix : tag 버튼 오류, carousel 렌더링 지연 방식 변경 - tag 가 이벤트 특가만 나타나는 오류 수정 - carousel 렌더링 지연 방식을 setTimeOut -> onTransitionEnd 로 변경 * deploy : build 210424 * [#25] feat : Detail UI 컴포넌트 생성 - Detail UI 를 위한 InfoGeneral, InfoImages, InfoNumber, InfoPrice, InfoProduct molecules 컴포넌트 생성 * [#24] feat: ADD tab click event(color change) - Fix MediumCard Tag 부분 * [#25] fix : icon onClick 이벤트 실행 로직 수정 - icon이 left, right 일 경우에만 moveSlide이벤트가 실행되도록 수정 * [#24] feat: ADD Img MouseOver event - useState를 통한 isHover로 상태변경 가능 * [#24] feat: Fix CSS Detail in HoverEvent * deploy : build 210427 * [#24] feat: ADD HeaderDrop event - Header MouseOver 시 Drop event 추가 - Header DropMenu MouseOver 시 hover event 추가 - Private Component 이름 통일변경 (ex Div -> WrapDiv) - Component들의 inex.style.jsx 추가 (Styled div 분리) * [#25] fix : MainDish Carousel 스타일 적용 함수 분리 - MainDish 스타일 및 매직넘버 삭제 * [#37] refactor : 코드리뷰 반영 - useFatch 구조 변경 : loadData 컴포넌트 생성해서 데이터만 받아오고 useEffect는 각 컴포넌트에서 실행 - tag 내용 및 컨텐츠 적용 방식 수정 - span 스타일 컴포넌트 적용방식을 className 사용으로 변경 * [#25] feat : Detail UI 컴포넌트 수정 - pages 컴포넌트 생성 * [#25] feat : Modal 컴포넌트 생성 - 모달 컴포넌트 생성 및 클릭 후 모달 오픈, close 버튼 클릭시 숨김 기능 구현 * [#25] feat: Modal data fetch 연결 * deploy : build 210428 * deploy : rebuild 210428 * deploy : rebuild 210428 * deploy : rebuild 210428 * build : rebuild 210428 * [#26] feat : 상세페이지 내 이미지 이벤트 추가 - 이미지 및 썸네일 추가 - 썸네일 클릭 시 메인 이미지 변경 이벤트 추가 * [#26] feat : large 카드에 모달 이벤트 추가 - large 카드에도 모달 이벤트를 추가했으나, 아직 베스트 반찬에는 데이터 api를 못받아오고 있으므로 실행은 되지 않게끔 주석처리 했습니다 * [#27] feat: Detail - Info UI 생성 및 fetch data 연결 * deploy :build 210429 * [#27] feat: Fix syntax error * build : build 210429 * [#26] refactor : carousel 컴포넌트 생성 - 라이브러리화를 위해 carousel 컴포넌트 생성 * build : rebuild 210429 * build : rebuild 210429 * [#26] refactor : carousel 컴포넌트 완성 carousel, carousel style 컴포넌트를 mainDish 컴포넌트에서 분리 완료 * [#26] feat : OtherCard, DetailOther 컴포넌트 생성 - 디테일 Carousel 작업을 위해 상기 컴포넌트 생성 * [#27] feat: Datail CSS UI 수정 - UI: BestMenu 오른쪽마진 수정 - UI: HoverCard 가운데 정렬 수정 - UI: Mouse cursor 디테일 설정 - feat: useTabs 삭제 및 component내부로 수정 - faet: getComma 파일생성 ( 원, (,) 넣는 함수) * deploy : rebuild 210429 * deploy : build 210430 * [#27] FE API data fetch 연결 확인 * [#45] FE API data fetch 연결 확인 * [#26] feat : OtherCard 스타일 적용 완성 - Other Card의 레이아웃 완성 * [#26] feat : OtherCard 케로셀 적용 중 * [#45] feat: 디테일한 CSS UI 구성 * [#26] feat : OtherCard 케로셀 완료 - portal을 적용해서 기존 캐러셀 컴포넌트를 재활용할 수 없는 관계로 캐로셀을 중복해서 사용함 * [#45] feat: fetch Data API and 합치기 * [#45]feat: Details 파일명 변경 및 데이터 전달 * [#45] feat:Details close error 해결 * [#45]feat: CSS style 수정 * [#45] feat: Image data 수정 * fix : Change directory name to camel case Co-authored-by: ink-0 <71919983+ink-0@users.noreply.github.com> Co-authored-by: woody <woojihye2339@gmail.com> Co-authored-by: Tree <gmldbs1109@naver.com> Co-authored-by: Ubuntu <ubuntu@ip-172-31-44-162.ap-northeast-2.compute.internal>
* Update README.md * 리액트 개발환경 설정 * Update README.md * Style: 기본 레이아웃 생성 * Update README.md * docs: README파일 이미지 업데이트 * feat: [#1] 헤더 컴포넌트 작업중 (5 / 6) - React-icons 모듈 사용 - DropdownMenu 작업중.. * 리액트 개발환경 설정 * Style: 기본 레이아웃 생성 * feat: Tag 컴포넌트 생성 * npm install하면서 package-lock.json 변경됨 * feat: Button 컴포넌트 생성 * feat: 헤더의 메뉴아이템에 마우스를 올리면 DropdownMenu 생성 - 헤더 완성 (1차) * chore: .gitignore 설정 변경 - .gitignore 설정 변경 및 git cached 초기화 * chore: .gitignore 설정 변경 (issue #1) - .gitignore 설정 변경 및 git cached 초기화 * fix: 삼항연산자로 텍스트 바뀌도록 수정 * fix: 스타일 컴포넌트 변수명 수정 * feat: Title 컴포넌트 생성 * type 값에 따라서 타이틀 문자열 반환 * feat: baseURL 담긴 객체 생성, * style: 태그 간 간격 스타일 추가 * feat: useFetch 커스텀 훅 작성 * feat: ProductCard 컴포넌트 생성 * fix: 스타일 컴포넌트를 컴포넌트 함수 밑으로 이동 * feat: hover에 따라 반투명박스 생기는 기능 구현 * delete: 디렉토리 구조 변경하면서 이전 경로 파일 삭제 * refactor: 디렉토리 구조 변경 * refactor: 디렉토리 구조 변경 * fix: themeProvider 스타일 추가, 피그마에 맞춰 키값 변경 * chore: 폴더명 변경 (Header 폴더 -> header) * chore: import 관련 path 수정 * feat: 캐러셀 슬라이더 프로토타입 완성 - 추후 리팩토링 필요 (재사용성을 위함) * feat: 캐러셀 슬라이더 파일 분리 및 SubContents 완성 - SubContents 렌더 시, useFetch 사용하여 MOCK 데이터 Get후 렌더링 * feat: 캐러셀 버튼 - 캐러셀의 위치정보에 따라 색상 변경 * fix: 이미지 에러 시 나오는 이미지 변경, border-radius 추가 * feat: Carousel을 재사용 할 수 있게 리팩토링 & 상세페이지 하단에도 쓰일 수 있게 변경 * feat: Carousel - Default 설정 변경 * style: margin-bottom 추가 * chore: Carousel 컴포넌트 폴더 위치 변경 * feat: Title, TabList, MainContents를 감싸는 MainContentsContainer 컴포넌트 생성 * feat: ProductCard를 감싸는 MainContents 컴포넌트 생성 * feat: 탭 요소 생성, 탭 기능 구현 * feat: loading 기능 구현, 에러처리 추가 필요 * feat: [#37] 상세정보 창 (Detail) 작업중.. - Modal (틀, 재사용 가능한 컴포넌트) 완성 - 상세정보 창에서 쓰일 SideDishStore(context) 생성 * feat: [#37] 상세정보 창 (Detail) 작업중.. 2 - DetailTop 1차 완성 * feat: [#37] 상세정보 창 (Detail) 작업중.. 3 - DetailTopGallery 동작 방식 변경 * feat: [#37] 상세정보 창 (Detail) 데이터 핸들링 테스트 중.. 1 - 버그 많음.. 수정 중 - customHook - useFetch 통합 * feat: [#37] 상세정보 창 (Detail) - DetailTop 1차 완성 - 이미지 제대로 안뜨던 버그 수정완료 * feat: [#37] 상세정보 창 (1차 마무리) - DetailBottom 완성 - Detail 페이지 1차 완성 * feat: SubContents 카테고리 모두 보이기 / 가리기 기능 추가 및 리팩토링 - SubContents 컴포넌트 작동 방식 변경 (리팩토링) - 카테고리 모두 보이기 / 가리기 기능 추가 * feat: [#57] 상세페이지 큰 이미지 업데이트 안되는 버그 수정 - DetailTopGallery의 Ref 관련 수정 * fix: hover 버그 수정 * useRef로 classList 수정하는 방식에서 state변경하는 방식으로 변경 * feat: Mock 데이터 제거 후 API 적용 (1차) * fix: Detail & DetailBottom 페이지 렌더 방식 변경 - 이에 따라 페이지 초기 렌더시 SubContents에서 비동기 통신 시 (데이터 가져올 시) 서브콘텐츠 (슬라이더) 데이터의 구조 변경 * fix: DetailTopGallery -> top_image 데이터 핸들링하는 방식으로 변경 - DetailTopInfo의 volumne -> quantity로 변경 * feat: 주문하기 (프로토타입) 1차완성 Co-authored-by: Kyu <59721293+kyu-kim-kr@users.noreply.github.com> Co-authored-by: eve712 <62237639+eve712@users.noreply.github.com>
안녕하세요 백엔드 Jung, yeon 입니다.
URL 요청에 따른 기본적인 기능 구현까지 완료했습니다.
배포와 OAuth 구현은 아직 진행하지 않았습니다.
API 문서
구현사항
궁금했던 점