서비스 페이지 API 연동 by Seogeurim · Pull Request #63 · telbby/telbby-frontend

Seogeurim · 2021-12-26T15:26:25Z

개요

서비스 페이지 API 연동 작업을 진행했습니다.
새로고침 시 로그인 상태가 유지되지 않는 버그가 있는데, 이것은 이슈 새로 파서 진행하겠습니다. 따라서 테스트 과정에서 어려움이 있을 수 있습니다.

이슈 번호

closes 서비스 불러오기 & 삭제 API 연동 #60

변경사항

서비스 불러오기 & 삭제 API 연동 (useService 훅 생성)
로더 컴포넌트 생성 및 로딩 처리
에러 처리
테스트코드는 기존에서 살짝만 수정

특이사항

테스트 코드에 진심을,,, 실패했습니다 ㅠ
(테스트 케이스는 떠오르는게 많았는데
- 삭제 버튼 누르면 모달 띄우기 : 모달을 portal로 띄워서 테스트 코드가 어떻게 되는건지 ㅠ
- API 응답 시 렌더링 테스트 : API 요청을 mock 해서 원하는 UI가 잘 렌더링되는지 테스트해보고 싶었는데, API 모듈이랑 훅이랑 여러 모듈로 나뉘어 있다보니 테스트 코드를 어떻게 작성해야할지 ㅠ 모르겠더라구요,,,!)

- Service 관련 type 정의 - ServiceAPI 클래스 정의

- 로딩, 에러 처리와 관련된 상태 관리 - 렌더링 시 서비스 리스트 불러오기 및 서비스 리스트 상태 관리

- 서비스 목록에서 서비스 개수를 보여주도록 ServiceList 컴포넌트 수정 - API 응답 데이터로 서비스 목록을 보여주도록 수정

- 로딩 중일 때 로더 뜨도록 수정 - 에러 발생 시 snackbar가 나타나도록 추가

- 서비스 아이템 목록에서 삭제 버튼 클릭 후 모달 accept 를 눌렀을 때 서비스 삭제 및 페이지 새로고침되도록 기능 구현

- props 변경으로 테스트 코드 깨지는 부분 수정

Seogeurim · 2021-12-29T11:40:06Z

이제 보니 테스트 코드 API 쪽에 작성해둔게 따로 있네요...! 참고해서 이슈 따로 파서 진행하겠습니다!

pumpkiinbell

제가 저번 PR 에서 서비스 페이지 수정을 해서 충돌나는 부분이 있는 것 같습니다... ㅎㅎ...
수고많으셨어요~~

pumpkiinbell · 2021-12-30T17:43:02Z

+    try {
+      setIsLoading(true);
+
+      const { count, projectList } = await serviceApi.getListAndCount();


음... 저희 이 부분 recoil selector로 캐싱하는 거 어떻게 생각하시나요 ??

저도 동의합니다 ㅎㅎ

useAuth 도 그렇게 수정해볼까 고민 중인데.. 그렇게 되면 autoLogin 을 어떻게 처리해야 할지 고민중이에요..

그렇지만 service를 가져오는 경우는 selector를 사용하는게 더 좋아보입니다 ㅎㅎ

useRecoilValueLoadable 이라는 훅을 사용하면 해당 상태 값의 상태를 가져올 수 있습니다.

hasValue , loading, hasError 를 사용하면 로딩, 에러 등의 처리를 진행할 수 있습니다 !

@jiyong1 오... auth 는 캐싱 안해도 될것 같은 느낌이 드는데... 아닐까요??

다들 잊고 있지만 설치되어 있는 패키지, React Query......!

오호 제가 이 부분 recoil이랑 react-query 좀 더 살펴보면서 수정해보겠습니다!!!

헐 react query 가 있군요... 좋습니다 😄

pumpkiinbell · 2021-12-30T17:45:17Z

+    setConfirmModal({
+      acceptHandler: () => {
+        onDeleteServiceItem(id.toString());
+        window.location.reload();


reload 를 하면 해당 페이지 리소스를 다시 불러와야 해서 SPA의 장점을 저해시키지 않을까요...?

새로 API 요청하는 건 어떻게 생각하시나요??

window.location.reload()와 같은 방식은 종호님 말씀처럼 이미 가지고 있는 리소스까지 다시 요청하기 때문에 불필요하게 과도한 네트워크 비용이 발생하지 않을까 싶어 수정하는 게 좋다고 생각합니다 ㅎㅎ

저는 개인적으로 요런 아이템을 삭제하는 API를 요청하고 성공하면 아이템 리스트 상태(state)로부터 해당 아이템을 지워주는 방식을 선택해왔는 데 요런 방식은 어떤가요?

아이템을 삭제하는 API가 성공했다면 클라이언트 측에서는 해당 아이템이 정상적으로 지워졌다라는 것을 믿는 게 맞지 않을까 했습니다. 실패했는 데 성공 코드를 응답하는 경우가 있다면 해당 경우는 서버 측 예외 처리가 제대로 되지 않았다는 의미일테니 서버 측에서 수정해야 하는 이슈이지 않을까 싶습니다.

별 생각 없이 reload했는데 이런 부분을 고려하지 못했네요!!! 자세히 리뷰해주시니 이런 부분들을 얻어가네요 감사합니다 ㅎㅎㅎ
API 요청 후 -> 성공 시 -> state로부터 아이템 삭제하고 -> delete success 관련해 snackbar를 띄워주는 방식으로 한 번 고쳐볼게요!!

pumpkiinbell · 2021-12-30T17:46:11Z

    setConfirmModalState((prev) => ({ ...prev, next, isOpen: false }));

-  return [open, close, setConfirmModal];
+  return { open, close, setConfirmModal };


반환값 형태가 배열에서 객체로 변한 이유가 궁금하네요 😅

배열로 하면 각 순서에 맞게 사용하면서 함수의 이름을 customize 할 수 있지만

객체로 하면 각 함수의 이름을 특정지으면서 제약을 건다는 느낌일까요..? ㅎㅎ

이 modal을 open하는 함수 등의 이름을 굳이 바꿀 필요 없으니.. 차라리 객체인게 나을 수 있지 않을까 하는 저의 개인적인 생각입니다 ㅎㅎ

아 이 부분을 제가 훅 사용하면서 open, setConfirmModal만 사용하고 싶었는데 배열로 하면
[open, close, setConfirmModal] = useConfimModal();
이런식으로 불필요한 close 함수도 불러와야 하는 문제가 있어 객체 형식으로 바꿨습니다!!

아 그렇군요 ㅎㅎㅎ

jiyong1

리뷰 한번 확인 부탁드릴게요 !

jiyong1 · 2021-12-31T06:03:50Z

    setConfirmModalState((prev) => ({ ...prev, next, isOpen: false }));

-  return [open, close, setConfirmModal];
+  return { open, close, setConfirmModal };


배열로 하면 각 순서에 맞게 사용하면서 함수의 이름을 customize 할 수 있지만

객체로 하면 각 함수의 이름을 특정지으면서 제약을 건다는 느낌일까요..? ㅎㅎ

이 modal을 open하는 함수 등의 이름을 굳이 바꿀 필요 없으니.. 차라리 객체인게 나을 수 있지 않을까 하는 저의 개인적인 생각입니다 ㅎㅎ

jiyong1 · 2021-12-31T06:04:53Z

+    try {
+      setIsLoading(true);
+
+      const { count, projectList } = await serviceApi.getListAndCount();


저도 동의합니다 ㅎㅎ

useAuth 도 그렇게 수정해볼까 고민 중인데.. 그렇게 되면 autoLogin 을 어떻게 처리해야 할지 고민중이에요..

그렇지만 service를 가져오는 경우는 selector를 사용하는게 더 좋아보입니다 ㅎㅎ

jiyong1 · 2021-12-31T06:06:39Z

+    try {
+      setIsLoading(true);
+
+      const { count, projectList } = await serviceApi.getListAndCount();


useRecoilValueLoadable 이라는 훅을 사용하면 해당 상태 값의 상태를 가져올 수 있습니다.

hasValue , loading, hasError 를 사용하면 로딩, 에러 등의 처리를 진행할 수 있습니다 !

hseoy

졸업이 벌써 다음 주라 학교 노트북을 반납하고 집의 데스크탑 환경에서 확인했네요 ㅎㅎㅎ

서버를 실행할 수 있는 환경 세팅(mysql 설치 등등)이 넘 오래 걸릴 것 같아서 일단 서버 없이.... API 연동 리뷰인데 서버 없이.... 리뷰 했다는 점 먼저 사과드리겠습니다.... ㅠㅠ 🍎🍎🍎

Loader를 추가해서 로딩 때 보여주도록 설정한 부분이나 에러 메시지를 Snackbar를 사용해서 보여준 점 등 디테일한 부분들이 잘 구현되서 좋았네요 ㅎㅎ 서비스 개수 보여주는 것도 GOOD 👍

다만, 서비스 개수 등은 서버 상태로서 React Query를 사용해도 좋지 않을까 싶지만 저도 React Query를 사용해본 적이 없어서... 추후에 차차 개선하면 좋을 것 같고 window.location.reload() 정도만 수정하면 될 것 같아서 바로 approve합니당

모달이랑 API 연동 부분에 대해서 어떻게 테스트 해야 할지는 ..... ㅠㅠㅠ 더 공부를 해봐야 겠네요

hseoy · 2021-12-31T08:08:59Z

+    setConfirmModal({
+      acceptHandler: () => {
+        onDeleteServiceItem(id.toString());
+        window.location.reload();


window.location.reload()와 같은 방식은 종호님 말씀처럼 이미 가지고 있는 리소스까지 다시 요청하기 때문에 불필요하게 과도한 네트워크 비용이 발생하지 않을까 싶어 수정하는 게 좋다고 생각합니다 ㅎㅎ

저는 개인적으로 요런 아이템을 삭제하는 API를 요청하고 성공하면 아이템 리스트 상태(state)로부터 해당 아이템을 지워주는 방식을 선택해왔는 데 요런 방식은 어떤가요?

아이템을 삭제하는 API가 성공했다면 클라이언트 측에서는 해당 아이템이 정상적으로 지워졌다라는 것을 믿는 게 맞지 않을까 했습니다. 실패했는 데 성공 코드를 응답하는 경우가 있다면 해당 경우는 서버 측 예외 처리가 제대로 되지 않았다는 의미일테니 서버 측에서 수정해야 하는 이슈이지 않을까 싶습니다.

hseoy · 2021-12-31T08:12:37Z

-      'Services',
+      `Services ${serviceCount}`,


서비스 개수 보여주는 거 좋네요!!

GOOD!!! 👍👍👍

hseoy · 2021-12-31T08:14:34Z

+          <h2 css={headerStyle}>
+            Services <span>{serviceCount}</span>
+          </h2>


으음..... Emotion과 ClassName을 함께 사용하는 건 어떤가요?? 약간 컨벤션에 대한 얘기가 될 수도 있겠네요.

어떤 emotion css style 하위에 있는 요소를 select할 때 단순히 태그 선택자만 사용하니까 css 코드에서는 이게 어떤 것에 대한 스타일인지 잘 모르겠고 html 코드에서는 여기에 스타일이 적용되어 있는 건지 안되어 있는 건지 잘 모르겠어요.

저도 저희가 처음에 마크업 구조를 잘 보기 위해 css로 넣어주길 선택한건데, 이렇게 하다보니 class로 스타일을 지정하는 장점도 뺏기고, styled-component를 사용했을 때의 장점도 뺏기는 느낌이네요 ㅠㅠㅠ
지난번 마크업 PR 올렸을 때도, 어떤 스타일 하위에 h1 선택자로 스타일을 지정하고 있었는데, 이 태그가 h1에서 h2로 바뀌니까 스타일이 갑자기 풀렸는데 어디가 문제인지 바로 인식이 안 되더라구요!!
css 코드가 점점 많아질수록 관리하기 힘들어지는 문제가 있을 것 같습니다. 이 부분 다음 회의 때 컨벤션 다시 한 번 논의해봐도 좋을 것 같네요!

hseoy · 2021-12-31T08:15:43Z

+  useEffect(() => {
+    if (error) {
+      snackbar.showMessage(error, {
+        duration: 1500,
+      });
+    }
+  }, [error]);


오오오오오오오 👍👍👍👍👍🤣🤣

Seogeurim added 8 commits December 18, 2021 11:06

[#60] feat: Service API 모듈 생성

5186bd0

- Service 관련 type 정의 - ServiceAPI 클래스 정의

[#60] feat: useService hook 생성

aa4f408

- 로딩, 에러 처리와 관련된 상태 관리 - 렌더링 시 서비스 리스트 불러오기 및 서비스 리스트 상태 관리

[#60] feat: ServicePage 수정

58f22b7

- 서비스 목록에서 서비스 개수를 보여주도록 ServiceList 컴포넌트 수정 - API 응답 데이터로 서비스 목록을 보여주도록 수정

[#60] feat: Loader 컴포넌트 생성

b233855

[#60] feat: 서비스 페이지 에러 처리 추가

a29907a

- 로딩 중일 때 로더 뜨도록 수정 - 에러 발생 시 snackbar가 나타나도록 추가

[#60] refactor: useConfirmModal hook return type을 객체로 변경

ba90b85

[#60] feat: 서비스 삭제 API 연동

ee9e72d

- 서비스 아이템 목록에서 삭제 버튼 클릭 후 모달 accept 를 눌렀을 때 서비스 삭제 및 페이지 새로고침되도록 기능 구현

[#60] test: 서비스 페이지 테스트 수정

09544aa

- props 변경으로 테스트 코드 깨지는 부분 수정

Seogeurim added the feature New feature or request label Dec 26, 2021

Seogeurim added this to the 11+12주차 milestone Dec 26, 2021

Seogeurim requested review from hseoy, jiyong1 and pumpkiinbell December 26, 2021 15:26

Seogeurim self-assigned this Dec 26, 2021

Seogeurim added 3 commits December 28, 2021 22:13

[#60] refactor: 불필요한 console log 지우기

aec41c7

Merge branch 'dev' into feature/#60

6394494

[#60] fix: 불필요한 에러 throw문 삭제

b8b6cae

pumpkiinbell approved these changes Dec 30, 2021

View reviewed changes

jiyong1 reviewed Dec 31, 2021

View reviewed changes

hseoy approved these changes Dec 31, 2021

View reviewed changes

Uh oh!

Conversation

Seogeurim commented Dec 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

개요

이슈 번호

변경사항

특이사항

Uh oh!

Seogeurim commented Dec 29, 2021

Uh oh!

pumpkiinbell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hseoy Dec 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hseoy Dec 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiyong1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hseoy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hseoy Dec 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hseoy Dec 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Seogeurim commented Dec 26, 2021 •

edited

Loading

hseoy Dec 31, 2021 •

edited

Loading

hseoy Dec 31, 2021 •

edited

Loading

hseoy left a comment •

edited

Loading

hseoy Dec 31, 2021 •

edited

Loading

hseoy Dec 31, 2021 •

edited

Loading