Skip to content
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

[1단계 - 장바구니 협업] 앨버(송상민) 미션 제출합니다. #21

Merged
merged 33 commits into from
Jun 3, 2022

Conversation

al-bur
Copy link

@al-bur al-bur commented Jun 2, 2022

안녕하세요 노스님 앨버입니다😀

또 만나뵙게 되어 반갑습니다~

실행방법

  • 이번에 MSW를 사용하였기 때문에, 데모 페이지는 존재하지 않습니다. 아래 가이드를 따라 진행 부탁 드립니다.
  1. 레포지토리를 클론한다.
git clone -b albur-step1 https://github.com/al-bur/react-shopping-cart-prod.git
  1. 폴더 이동
cd react-shopping-cart-prod
  1. 설치 후 실행
npm i
npm run start

구현한 것

기능 요구사항을 통해 전체 구현 사항을 확인할 수 있습니다.


질문

1. 로그인 후 다시 회원 정보 요청이 현업에서도 사용되는 패턴인가요??

이번에, 백엔드와의 협업을 통해 미션을 진행했습니다. 그러다 보니, 백엔드와 협의는 했지만, 대부분의 API 명세는 백엔드가 만들었는데요. 현재, 웹 사이트 상에서, 로그인을 하고 난 후 헤더에 XX님 환영합니다라는 문구를 띄워주고 있습니다. 그래서, 로그인 하자마자 사용자 정보가 필요한 상황인데, API 상으로는 response에 회원정보를 보내주고 있지 않습니다. 그래서, 로그인 성공 후에 다시 한번 회원정보를 가져오는 API 요청을 한 번 더 진행해줍니다. 프론트 입장에서는 로그인 성공 response에 회원정보가 있으면 따로 회원정보를 가져오는 API 요청을 하지 않아도 되는데, 백 입장은 resource 측면에서 로그인 성공 response에 회원정보를 담아주는 게 통상적인 방법은 아니라고 하더라고요...

현업에서는 이와같은 상황에서 어떻게 진행하는 지 궁금합니다!!


LAH1203 and others added 30 commits May 31, 2022 20:11
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
- 상품 목록 카드 스타일 수정
- global color 상수화

Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
- 로그인, 회원가입 제외, 헤더 보임
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
- 로그인, 회원가입 제외, 헤더 보임
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
- 로그인, 회원가입 제외, 헤더 보임
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
LAH1203 and others added 2 commits June 2, 2022 17:22
Co-authored-by: Seungchan On <[email protected]>
Co-authored-by: al-bur <[email protected]>
@al-bur al-bur changed the base branch from main to al-bur June 2, 2022 08:45
@al-bur al-bur changed the title Albur step1 [1단계 - 장바구니 협업] 앨버(송상민) 미션 제출합니다. Jun 2, 2022
Copy link

@pocojang pocojang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyoungnam
빠른 미션 수행을 위해 먼저 머지합니다
하지만 6/4일까지는 추가적으로 리뷰 부탁드립니다 :)

@pocojang pocojang merged commit f3a8186 into woowacourse:al-bur Jun 3, 2022
Copy link

@hyoungnam hyoungnam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 앨버님 처음에 마지막에 만나는군요:)

3단계에서 추가된 부분을 중심으로 보았고 깔끔하게 구현하셔서 코멘트가 많지 않네요. 한편 제가 회원가입하고 로그인을 시도했는데 로그인이 안되는데 한번 확인해주시거나 2단계 API 연결되면 자연스럽게 해결될것 같기도 하네요
질문 주신 부분은 백엔드분이 말씀하신게 맞습니다. 여러 다른 사이트에 회원가입을 해보며 참고해도 좋을것 같습니다.
2단계도 화이팅입니다!!

Comment on lines +22 to +27
<Route path={PATH.BASE} element={<HeaderLayout />}>
<Route path={PATH.BASE} element={<MainPage />} />
<Route path={`${PATH.PRODUCT}/:id`} element={<ProductPage />} />
<Route path={PATH.CART} element={<CartPage />} />
<Route path={PATH.EDIT_USER_INFO} element={<EditUserInfoPage />} />
</Route>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v6에서 HeaderLayout을 이렇게 합성할수있군요😀

Copy link
Author

@al-bur al-bur Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이러한 시도를 계속 해보는 것이 성장에 도움이 될 것 같았어요ㅎㅎ

일부러 더욱 해보려하고있습니다~~

Comment on lines +13 to +17
useEffect(() => {
if (isLogin()) {
navigate(-1);
}
}, []);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/questions/69923420/how-to-use-private-route-in-react-router-domv6

hook으로 만들어 할수도있겠지만 합성방식도 있으니 한번 참고하시면 좋을듯합니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step2때 같이 수정해보겠습니다!

);
const userName = useSelector((state: { user: User }) => state.user.username);

const [showUserToggle, setShowUserToggle] = useState(false);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하지 않는 변수는 살포시 지워주세용

Copy link
Author

@al-bur al-bur Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗....네 지우겠습니다!!

수정하였습니다. 15c68a0

Comment on lines +26 to +40
const renderSwitch = useCallback(() => {
switch (condition) {
case CONDITION.LOADING:
return <Loading />;
case CONDITION.COMPLETE:
return <ProductCardGrid productList={productList} />;
case CONDITION.ERROR:
return (
<Message>상품 정보를 가져오는데 오류가 발생하였습니다 😱</Message>
);
}
}, [condition, productList]);

return <StyledPage>{renderSwitch()}</StyledPage>;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loading과 error부분을 early return 해도 충분할것 같은데 render함수를 활용한 이유가 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 이번 미션이 저번 장바구니 미션의 코드를 가지고 와서 인증인가 기능만 추가하는 것이었는데요. 그래서 이 코드는 이번 미션전에 페어 크루가 작성했던 코드들입니다! 저도 이 부분이 궁금했어서 페어한테 물어봤었는데, switch문을 통해 삼항 연산자를 중첩해서 사용하는 것을 피할 수 있었다고 했습니다.

Copy link
Author

@al-bur al-bur Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

삼항 연산자 2번 중첩해서 사용하기 vs renderSwitch 사용하기

제가 생각했을 때도 후자가 가독성이 더 좋아보여서 (LOADING, SUCCESS, ERROR일 때 명확하게 어떤 UI가 그려는 지 알아보기가 더 쉽다는 의미입니다.) 나쁘지 않은 방법이라고 생각했습니다!

노스님의 의견은 어떤가요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 삼항연산자 없이 아래와 같이 작성하면 어떤가하고 생각했습니닿

if(condition === CONDITION.LOADING) {
<Loading />;
}
if(condition === CONDITION.ERROR) {
    <Message>상품 정보를 가져오는데 오류가 발생하였습니다 😱</Message>
}
return <StyledPage>{<ProductCardGrid productList={productList} />}</StyledPage>;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 방법으로 하면 더 좋을 것 같네요!! 굳이 함수로 나눠서 해줄 필요는 없을 것 같습니다~!
이번 미션은 페어의 코드에 기능을 추가하는데 우선순위가 있는 것 같아서 이는 참고해서 추후 프로젝트에서 적용해보겠습니다!

}
};

const onSubmitEditUserInfoForm = async (e: React.FormEvent) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 두 폼은 handle prefix를 사용하였는데 이곳은 on 인데 일관성있게 prefix를 사용해도 좋을것 같네요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중요한 일관성을 지키지 못했네요ㅠㅠ

on prefix로 통일해서 수정해주었습니다! b877149

al-bur added a commit to al-bur/react-shopping-cart-prod that referenced this pull request Jun 5, 2022
al-bur added a commit to al-bur/react-shopping-cart-prod that referenced this pull request Jun 5, 2022
handle prefix -> on prefix로 수정

관련 피드백: woowacourse#21 (comment)
@al-bur
Copy link
Author

al-bur commented Jun 5, 2022

안녕하세요 노스님 앨버입니다!!

꼼꼼하게 피드백 해주셔서 감사합니다~ 로그인이 안되는 부분은 step2때는 발생하지 않도록 수정해서 PR 날리겠습니다😁
남겨주신 피드백 기반해서 수정 하였고 이 질문에 대해서만 답변해주시면 감사하겠습니다😊

hyoungnam pushed a commit that referenced this pull request Jun 17, 2022
* refactor: 사용하지 않는 useState 삭제

관련 피드백: #21 (comment)

* refactor: prefix 컨벤션에 따라 수정

handle prefix -> on prefix로 수정

관련 피드백: #21 (comment)

* chore: 자동정렬 설정 추가

* refactor: 추가한 prettier 전체 파일에 적용

- import 자동 정렬
- , 설정

* feat: 비로그인시 회원정보수정 페이지 접근 불가능하도록 구현

* fix: msw 작동 오류 해결

* refactor: mocking 시작 함수 추출

- index.tsx에서 mocking을 시작하는 로직에 대해서 정확히 알필요없을 것 같아 추상화

* fix: 무한로딩 일어나는 문제 해결

- msw 내부 설정 문제이기 때문에 공식문서를 참고하여 해당 문제 해결

* feat: 장바구니 기능 관련하여 인가 적용

- 비로그인 상황에서 장바구니 페이지에 접속하려하거나 장바구니 기능 이용시 로그인 페이지로 이동

* refactor: 로그인 상태유지 관련 예상되는 버그로 인하여 기능 삭제

- localStroage를 통해 로그인 상태를 유지하게 되면, 10분의 유효기간을 가진 토큰이 유효하지 않을 경우에도 로그인 되었다고 판단이 될 것입니다. 현재의 상황에서는 유효한 토큰인지를 검사해줄 수가 없기 때문에 로그인 상태유지 기능을 삭제하였습니다

* refactor: 상품 재고에 대한 정보 제거 및 관련 기능 삭제

- 메인 페이지내 상품 재고에 대한 정보 제거 (백엔드와의 회의 이후 결정)로 인해 해당 정보 제거 및 관련 기능 삭제
- 카트에 담겨져있는 상품의 양을 나타내는 변수의 네이밍을 stock으로 했던 것을 quantity로 변경

* refactor: 백엔드 api 명세에 따른 변수명 변경

- image -> imageUrl

* refactor: thunk 로직 수정

* chore: redux logger 설정

* feat: 로컬 서버 -> api 연결

* fix: 장바구니 전체 선택시 무한으로 장바구니에 담기는 문제 해결

- 현재 장바구니에 담겨있으면 전체 선택시 checkedCartItems에 담기지 않도록 수정

* feat: 장바구니 상품들 전체 삭제 api 연결

* chore: 기본 백엔드 API 변경

* fix: 장바구니 상품 전체 삭제시 문제 해결

* refactor: 불필요한 console.log 제거

* feat: 이미지 불러오기 실패할 시 기본 이미지 보여주도록 설정

관련 피드백: #63 (comment)

* feat: 메인페이지 및 상품 상세 페이지에서 장바구니 기능 이용시 로그인 페이지 이동 여부 물어보도록 기능 구현

관련 피드백: #63 (comment)

* refactor: 메인페이지내 장바구니 추가 로직 리팩토링

- then -> try catch + await 사용으로 변경
- 에러 핸들링을 더 깔끔하게 해주기 위해 try catch문 사용

관련 피드백: #63 (comment)

* refactor: 컬러 상수화
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants