[#82] 메인 페이지 추가 요구 기능 #111

dmdmdkdkr · 2023-11-23T15:21:22Z

한 일

ContestPage
- ContestPage의 헤더 부분에 대시보드 보기 버튼 추가
- ContestPage에 문제 선택 aside 추가
MainPage
- 대회 리스트를 목업이 아닌 api응답을 받아 구성하도록 구현
- 대회 참여 버튼 클릭시
  - 로그인이 되어있는 경우 대회 참여
  - 로그인이 되어있지 않은 경우 alert창을 띄운 후 로그인 페이지로 이동
- 대회 생성 버튼 클릭시
  - 로그인이 되어있는 경우 대회 생성 페이지로 이동
  - 로그인이 되어있지 않은 경우 alert창을 띄운 후 로그인 페이지로 이동

메인 페이지

로그인이 되어있지 않은 경우(참여하기 버튼 클릭시 동일하게 동작)

대회 참여 버튼 클릭시 로그인이 되어있는 경우

이미 참여한 대회에 대해 참여하기 버튼을 클릭시

…o-with-me into 82-메인-페이지-추가-요구-기능

mahwin · 2023-11-23T15:34:19Z

frontend/src/components/Contest/Aside.tsx

+export default function Aside(props: AsideProps) {
+  return (
+    <aside>
+      <h2>문제 목록</h2>


h1,h2,...h6 태그는 SEO에서 중요하게 읽혀서 문제 목록은 span이면 충분 할 것 같습니다.

몰랐는데 감사합니다!:D

mahwin · 2023-11-23T15:37:09Z

frontend/src/components/Contest/CompetitionPageHeader.tsx

componenets 아래에 pageHeader는 어색한 것 같아요. 대회 페이지 내의 대회 정보를 담고 있는 헤더라면 CompetitionHeader 정도면 될 것 같네요.

CompetitionPage의 Header로 들어갈 부분이라서 그렇게 이름을 지었는데, 말씀하신 대로 components 아래에 들어가는 걸 고려해보니까 어색한 것 같네요
감사합니다!

dev2820 · 2023-11-23T15:29:35Z

frontend/src/components/Contest/Aside.tsx

+  setCurrentProblemIndex: (index: number) => void;
+}
+
+export default function Aside(props: AsideProps) {


단순히 Aside라는 이름은 컴포넌트의 역할이 모호하게 드러나는 것 같아요. 그리고 Aside의 기능보단 Problem을 선택할 수 있는 기능에 더 가까운 것 같습니다. ContestProblemSelector 정도가 어떨까요

확실히 나은 이름이네요, 항상 감사합니다!

dev2820 · 2023-11-23T15:33:50Z

frontend/src/components/Contest/Aside.tsx

+
+interface AsideProps {
+  problemIds: number[];
+  setCurrentProblemIndex: (index: number) => void;


Props로 내려주는 이벤트 핸들러 이름은 on으로 시작하게 해주세요.

onChangeProblemIndex: (index:number)=>void; ...

앗, 미처 신경쓰지 못했네요
감사합니다, 다음부터는 on으로 시작하게 하겠습니다!

dev2820 · 2023-11-23T15:34:38Z

frontend/src/components/Contest/ContestBreadCrumb.tsx

-    <header>
-      <div className={titleContainerStyle}>
-        {crumbs.map((crumb, index) => (
-          <span key={index} className={crumbStyle}>
-            {crumb}
-          </span>
-        ))}
-      </div>
-    </header>
+    <div className={titleContainerStyle}>
+      {crumbs.map((crumb, index) => (
+        <span key={index} className={crumbStyle}>
+          {crumb}
+        </span>
+      ))}
+    </div>


좋아요 👍

dev2820 · 2023-11-23T15:36:18Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+    try {
+      await axios.post(
+        `${apiUrl}/competitions/${props.id}/participations`,
+        {},
+        {
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        },
+      );
+
+      alert('대회에 성공적으로 참여했습니다.');
+      window.location.reload();
+    } catch (error: unknown) {
+      let errorMessage = 'Unexpected error occurred.';
+
+      if (axios.isAxiosError(error)) {
+        if (error.response) {
+          if (error.response.status === 403) {
+            errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';
+          } else if (error.response.status === 400) {
+            errorMessage = '이미 참여한 대회입니다.';
+          } else {
+            errorMessage = `HTTP Error ${error.response.status}`;
+          }
+        } else {
+          errorMessage = 'Network error occurred.';
+        }
+      }


apis 폴더로 빼는게 어떨까요?

넵, 분리하겠습니다 감사합니다!

dev2820 · 2023-11-23T15:37:01Z

frontend/src/components/Main/CompetitionList.tsx

+  const fetchCompetitions = async () => {
+    try {
+      const response = await axios.get<Competition[]>('http://101.101.208.240:3000/competitions');
+      setCompetitions(response.data);
+    } catch (error) {
+      console.error('Error fetching competitions:', (error as Error).message);
+    }
+  };
+


fetchCompetition 함수가 제 브랜치에 있어서 햇갈릴 것 같은데 fetchCompetitionList로 이름을 바꿀 수 있을까요?

그리고 우찬님도 가능하면 http://~~ 하는 부분을 .env 파일 만들고 VITE_API_URL=http://101.101.208.240:3000/ 내용 넣어서 이거 사용해주세요

함수명 변경이랑, .env파일 사용 모두 하겠습니다. 감사합니다!

dev2820 · 2023-11-23T15:40:09Z

제 쪽 브랜치 코드 때문에 나중에 충돌이 좀 날 것 같은데, 이거 먼저 머지되면 제 쪽에서 수정하겠습니다.

제 브랜치가 좀 커졌는데, 실수했네요. 머지가 오래걸릴거라면 쪼개서 PR을 먼저 올렸어야했는데

mahwin · 2023-11-23T15:40:54Z

frontend/src/components/Main/Buttons/GoToCreateCompetitionLink.tsx

+  const token = queryParams.get(TOKEN_KEY) || localStorage.getItem(TOKEN_KEY);
+  const navigate = useNavigate();
+
+  const handleNavigate = () => {
+    if (!token) {
+      alert('로그인이 필요합니다.');
+      navigate('/login');
+    } else {
+      navigate('/contest/create');
+    }
+  };


token이 아니고
import useAuth from '@/src/hooks/login/useAuth' 대충 이런 path
const { isLoggedin } = useAuth(); 이렇게
isLoggedin:boolean인데 true면 로그인된 상태 false면 안 된 상태입니다.

mahwin · 2023-11-23T15:42:19Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+  const apiUrl = import.meta.env.VITE_API_URL;
+
+  const queryParams = new URLSearchParams(location.search);
+  const token = queryParams.get(TOKEN_KEY) || localStorage.getItem(TOKEN_KEY);
+
+  const handleJoinClick = async () => {
+    if (!token) {
+      alert('로그인이 필요합니다.');
+      window.location.href = '/login';
+      return;
+    }
+


여기도 token은 사용자 검증할 때 사용되고 token으로 검증된 상태면 const { isLoggedin } = useAuth();이 true일 거예요

감사합니다!

mahwin · 2023-11-23T15:43:13Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+        {
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        },


검증된 사용자만 저 api에 응답받을 수 있나보네요?!

Swagger UI를 보면 그런 것 같아요:D

mahwin · 2023-11-23T15:47:36Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+      if (axios.isAxiosError(error)) {
+        if (error.response) {
+          if (error.response.status === 403) {
+            errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';
+          } else if (error.response.status === 400) {
+            errorMessage = '이미 참여한 대회입니다.';
+          } else {
+            errorMessage = `HTTP Error ${error.response.status}`;
+          }
+        } else {
+          errorMessage = 'Network error occurred.';
+        }


const {status} = error.response
switch문은 어떠신가요 ?!

아 정말 좋은 생각이네요, 감사합니다!

mahwin · 2023-11-23T15:49:09Z

frontend/src/components/Main/CompetitionList.tsx

+      const response = await axios.get<Competition[]>('http://101.101.208.240:3000/competitions');
+      setCompetitions(response.data);


http://101.101.208.240:3000은 .env에서 꺼내서 사용하는 게 좋겠습니다.

감사합니다!

mahwin · 2023-11-23T15:50:23Z

frontend/src/components/login/index.tsx

�제 pr을 merge 안해서 임시로 만드신 거 겠죠??

merge하기 전에 생성했는데, 삭제하면 될까요?

…o-with-me into 82-메인-페이지-추가-요구-기능

dev2820 · 2023-11-26T16:02:00Z

frontend/src/apis/joinCompetition/index.ts

+      },
+    );
+
+    alert('대회에 성공적으로 참여했습니다.');


alert도 api에서 관리할 로직이 아니라고 생각돼요. alert를 띄우는 것은 외부 데이터를 관리한 것에 대한 mainpage의 반응 이라고 생각됩니다. 즉, alert는 view를 담당하는 mainpage의 역할이라고 생각됩니다.

우찬님은 어떻게 생각하시나요?

확실히 alert도 페이지에서 관리하는 게 좋을 것 같네요, 감사합니다!

dev2820 · 2023-11-26T16:08:47Z

frontend/src/apis/joinCompetition/index.ts

+    if (axios.isAxiosError(error)) {
+      if (error.response) {
+        switch (error.response.status) {
+          case 403:


사실 지금 만으로도 충분하지만, 살짝 욕심을 내보자면,

403같은 상태 코드는 상수로 관리되는 것이 좋아보여요. 물론 상태코드는 어느정도 명시적이지만, 그럼에도 403은 Forbidden을 나타낸다는 의미를 포함하면 좋을 것 같습니다.

const STATUS = { Forbidden: 403, } as const; ... case STATUS.Forbidden: ...

이 부분은 우찬님도 욕심이 난다면 적용해주세요. 하지만 지금 상태로도 좋습니다.

감사합니다!

dev2820 · 2023-11-26T16:10:52Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+import type { CompetitionApiData } from '@/apis/joinCompetition/types';
+import useAuth from '@/hooks/login/useAuth';
+
+const TOKEN_KEY = 'accessToken';


상수화하여 관리하는 것 아주 좋아요.

개인적인 생각엔 useAuth에서 token까지 반환하는게 어떨까요?

const token = queryParams.get(TOKEN_KEY) || localStorage.getItem(TOKEN_KEY);

위 코드를 useAuth 안에 넣고 token을 꺼내 쓰면 더 깔끔할 것 같아요

리뷰 감사합니다!
이 부분은 유석님이 상수화해서 관리하던 코드를 제가 그대로 가져와서 사용하기만 한 거라서, 이번 PR에서는 넘어가도 될까요?

알겠습니다 :)

dev2820 · 2023-11-26T16:11:41Z

frontend/src/components/Main/Buttons/JoinCompetitionButton.tsx

+  const handleJoinClick = async () => {
+    if (!isLoggedin) {
+      alert('로그인이 필요합니다.');
+      window.location.href = '/login';


이 부분이 배포에서도 정상작동할지 모르겠네요??

useNavigator를 쓰는게 더 좋아보이긴 합니다.

감사합니다, useNavigator로 변경하겠습니다!

mahwin

LGTM 😎

mahwin · 2023-11-28T02:28:48Z

frontend/src/components/Login/index.tsx

충돌일어나서 생성하신 건가요?!

netlify 빌드 과정에서 대소문자 오류가 생겨서 지웠다가 다시 생성했어요!

mahwin · 2023-11-28T02:30:23Z

frontend/src/apis/joinCompetition/index.ts

+    await api.post(
+      `/competitions/${id}/participations`,
+      {},
+      {
+        headers: {
+          Authorization: `Bearer ${token}`,
+        },
+      },


api 서버가 아직 구축 안 되어서 body가 비었다 생각했는데, 오로지 검증을 위한 거라면 post 대신 get을 써도 되지않을까요

클라이언트에서 보내줄 데이터가 없어보여서요

이 부분은 일단 API가 post를 사용하게 되어있어 따르는게 좋아보이고,
제 사견으론 의미론적인 GET과 POST의 차이를 봤을 때 POST가 맞는 것 같아요.

GET은 데이터를 가져오겠다라는 의미이며 멱등성을 보장하는 것이 좋습니다.

POST는 데이터를 변경하겠다는 의미를 포함해요.

여기선 변경에 필요한 데이터를 보내주지 않지만, 내 상태를 join으로 바꾸겠다는 의미에서 POST는 이상해보이진 않습니다.

유석님은 어떻게 생각하시나요

dev2820 · 2023-11-28T02:46:42Z

frontend/src/apis/joinCompetition/index.ts

+    if (axios.isAxiosError(error)) {
+      if (error.response) {
+        switch (error.response.status) {
+          case STATUS.Forbidden:
+            errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';
+            break;
+          case 400:
+            errorMessage = '이미 참여한 대회입니다.';
+            break;
+          default:
+            errorMessage = `HTTP Error ${error.response.status}`;
+            break;
+        }
+      } else {
+        errorMessage = 'Network error occurred.';
+      }
+    }


isAxiosError 사용한 부분 아주 좋습니다. :) 제 개인적인 의견으로는 가독성을 위해

Suggested change

if (axios.isAxiosError(error)) {

if (error.response) {

switch (error.response.status) {

case STATUS.Forbidden:

errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';

break;

case 400:

errorMessage = '이미 참여한 대회입니다.';

break;

default:

errorMessage = `HTTP Error ${error.response.status}`;

break;

}

} else {

errorMessage = 'Network error occurred.';

}

}

if (!axios.isAxiosError(error)) {

return 'Unexpected error occurred';

}

if (!error.response) {

return 'Network error occurred';

}

switch(error.response.status) {

case STATUS.Forbidden:

errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';

break;

case 400:

errorMessage = '이미 참여한 대회입니다.';

break;

default:

errorMessage = `HTTP Error ${error.response.status}`;

break;

}

같은 코드가 어때요?

그리고 에러를 다양하게 핸들링한 부분 좋았어요. 기왕이면 STATUS.BadRequest도 사용하면 좋을 것 같네요

감사합니다! 확실히 가독성이 좋아졌네요:D

dev2820 · 2023-11-28T02:51:09Z

frontend/src/components/Contest/CompetitionHeader.tsx

+    <header className={headerStyle}>
+      <ContestBreadCrumb crumbs={props.crumbs} />
+      <ViewDashboardButton id={props.id} />
+    </header>


아마 로그인 정보등을 담는 그냥 헤더와 대회 헤더가 나뉘어진 것 같은데,

header 태그가 중복되는 것은 접근성에 좋아보이지 않아요.

div나 다른 태그를 사용하는게 어떨까요?

감사합니다, div태그로 수정할게요!

dev2820 · 2023-11-28T02:53:08Z

frontend/src/components/Main/CompetitionList.tsx

+  const fetchCompetitionList = async () => {
+    try {
+      const response = await axios.get<Competition[]>(`${apiUrl}competitions`);
+      setCompetitions(response.data);
+    } catch (error) {
+      console.error('Error fetching competitions:', (error as Error).message);
+    }
+  };


외부 서버로부터 정보를 받아오는 로직은 apis 폴더로 뺄 수 있을까요?
그리고 utils에 api파일을 만들어둬서 apiUrl없이 api.get과 같이 접근해서 데이터 패치 가능해요

넵, 앞으로도 api로직은 분리하겠습니다 감사합니다!

dev2820 · 2023-11-28T04:58:19Z

frontend/src/apis/competitionList/index.ts

+  try {
+    const response = await api.get<Competition[]>(`${apiUrl}competitions`);
+    return response.data;


아하 이 부분을 이해 못하셨군요.

Axios의 create 인터페이스를 활용하면 Axios 인스턴스라는 것을 만들 수 있습니다. 이 인스턴스는 baseUrl을 설정하면 다음 url에 대해 baseUrl + url 처럼 동작합니다.

const instance = axios.create({ baseUrl: 'http://root.domain.com' }) const request = instance.get('/competitions'); // http://root.domain.com/competitions 와 같이 동작합니다.

이런식으로 변경해달라는 요청이었어요. api 폴더의 다른 요청들을 보면 이해가 빠를 겁니다.

앗, 감사합니다!

dev2820 · 2023-11-28T05:02:43Z

frontend/src/apis/joinCompetition/index.ts

+    let errorMessage = 'Unexpected error occurred.';
+
+    if (!axios.isAxiosError(error)) {
+      return 'Unexpected error occurred';
+    }
+
+    if (!error.response) {
+      return 'Network error occurred';
+    }
+
+    switch (error.response.status) {
+      case STATUS.Forbidden:
+        errorMessage = '대회 참여에 실패했습니다. 서버에서 거절되었습니다.';
+        break;
+      case STATUS.BadRequest:
+        errorMessage = '이미 참여한 대회입니다.';
+        break;
+      default:
+        errorMessage = `HTTP Error ${error.response.status}`;
+        break;
+    }
+
+    return errorMessage;


27번줄과 49번줄은 더 이상 필요 없어보여요. 대신 switch문 안에서 return을 해주면 될 것 같네요

switch() { case aaa: return '대회 어쩌구' default: return '어쩌구'

감사합니다!

dev2820 · 2023-11-28T05:03:00Z

frontend/src/components/Contest/CompetitionHeader.tsx

+import ContestBreadCrumb from './ContestBreadCrumb';
+interface Props {


악 개인적인 요청사항이지만 한 칸만 띄워주세요

넵 다음부터는 미리 확인하겠습니다!

dev2820

:)

dmdmdkdkr added 12 commits November 23, 2023 14:45

feat: 대회 참여 버튼이 대회의 id값을 받도록 코드 추가

4e90bfc

feat: ContestPage의 헤더부분 분리 및 대시보드 보기 추가

9db7461

refactor: ContestPage의 헤더 컴포넌트명 변경

b9d2beb

feat: ContestPage에 문제 선택 aside 추가

1de6028

update: ContestPage에서 다음 문제로 넘어가는 버튼 삭제

79ea376

feat: 대회에 참여하는 로직 작성

5253812

Merge branch 'fe-dev' of https://github.com/boostcampwm2023/web12-alg…

0be3fa8

…o-with-me into 82-메인-페이지-추가-요구-기능

chore: login페이지 머지

1e7f3e7

feat: 로그인 여부에 따른 처리 로직 추가

c9569ad

feat: 대회 참여 api요청을 보낼 때 token을 포함하도록 수정

84b9d7f

Merge branch 'fe-dev' of https://github.com/boostcampwm2023/web12-alg…

26cd907

…o-with-me into 82-메인-페이지-추가-요구-기능

feat: CompetitionList가 api응답을 받아 출력되도록 수정

f1d491e

dmdmdkdkr added the FE fe 개발 label Nov 23, 2023

dmdmdkdkr requested review from dev2820 and mahwin November 23, 2023 15:21

dmdmdkdkr self-assigned this Nov 23, 2023

feat: 이미 참여한 대회에 대해 참여하기 시도시 이미 참여한 대회라는 alert 표시

126cbd0

mahwin reviewed Nov 23, 2023

View reviewed changes

dev2820 reviewed Nov 23, 2023

View reviewed changes

mahwin reviewed Nov 23, 2023

View reviewed changes

refactor: 컴포넌트명 변경

3b55519

mahwin reviewed Nov 23, 2023

View reviewed changes

dmdmdkdkr added 2 commits November 24, 2023 01:06

refactor: 이벤트함수명 on으로 시작하도록 변경

476fd70

대회 참여 버튼의 api로직 분리

a8ddf0f

dmdmdkdkr requested a review from mahwin November 26, 2023 05:45

dmdmdkdkr added 2 commits November 26, 2023 15:33

chore: 필요 없는 목데이터 파일 삭제

34024ab

Merge branch 'fe-dev' of https://github.com/boostcampwm2023/web12-alg…

304be51

…o-with-me into 82-메인-페이지-추가-요구-기능

dev2820 reviewed Nov 26, 2023

View reviewed changes

mahwin approved these changes Nov 27, 2023

View reviewed changes

dmdmdkdkr added 3 commits November 28, 2023 11:12

reafactor: api에서 alert분리, window.location->useNavigator, 403 상수화

26e1711

chore: Login 컴포넌트 삭제

f7cdcdd

chore: Login 컴포넌트 재작성

6b8ff0c

mahwin reviewed Nov 28, 2023

View reviewed changes

dev2820 reviewed Nov 28, 2023

View reviewed changes

dmdmdkdkr added 3 commits November 28, 2023 13:44

refactor: api요청 모듈화

f31291d

refactor: 에러 핸들링 코드 수정 및 상수 추가

62dbf9a

refactor: 중복 태그인 header태그를 div태그로 수정

7aa9d2d

dmdmdkdkr requested review from dev2820 and mahwin November 28, 2023 04:49

dev2820 reviewed Nov 28, 2023

View reviewed changes

dmdmdkdkr added 3 commits November 28, 2023 14:10

refactor: 필요 없는 코드 삭제-switch문 안에서 return하도록 수정

32b54ce

refactor: 개행문자 추가

727fdd7

refactor: api요청 코드 api모듈 사용

3c26b7f

dev2820 approved these changes Nov 28, 2023

View reviewed changes

dmdmdkdkr merged commit 50cabd8 into fe-dev Nov 28, 2023
3 checks passed

dmdmdkdkr deleted the 82-메인-페이지-추가-요구-기능 branch November 28, 2023 05:24

		const response = await axios.get<Competition[]>('http://101.101.208.240:3000/competitions');
		setCompetitions(response.data);

		import ContestBreadCrumb from './ContestBreadCrumb';
		interface Props {

[#82] 메인 페이지 추가 요구 기능 #111

[#82] 메인 페이지 추가 요구 기능 #111

Conversation

dmdmdkdkr commented Nov 23, 2023 • edited Loading

한 일

메인 페이지

로그인이 되어있지 않은 경우(참여하기 버튼 클릭시 동일하게 동작)

대회 참여 버튼 클릭시 로그인이 되어있는 경우

이미 참여한 대회에 대해 참여하기 버튼을 클릭시

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dev2820 commented Nov 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmdmdkdkr Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dev2820 left a comment

Choose a reason for hiding this comment

dmdmdkdkr commented Nov 23, 2023 •

edited

Loading

dmdmdkdkr Nov 28, 2023 •

edited

Loading