Skip to content

Conversation

six-standard
Copy link
Member

@six-standard six-standard commented Nov 28, 2024

설명

Capture-2024-11-28-153914
간단한 디자인을 기반으로 로그인 페이지를 제작하였습니다.

진행한 작업

  • 페이지 퍼블리싱
  • API 연동 (우선은 path만 작성해두었습니다)
  • 테스트 제작 (5개의 테스트 케이스를 생성해두었습니다)

그 외..

Jest를 처음 사용하다 보니 조금 어색해서 오래 걸렸던 것 같습니다.
특히 jest-DOM과 mocking 작업에서 이해가 안 가는 부분이 조금 많았습니다;

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 로그인 페이지 및 대시보드 페이지에 대한 새로운 구성 요소 추가.
    • 사용자 로그인 기능을 위한 폼과 API 요청 처리 추가.
    • Jest 테스트 환경 설정 및 테스트 파일 추가.
    • Tailwind CSS 구성 파일 수정.
    • QueryProvider를 통한 데이터 페칭 관리 기능 추가.
    • 사용자 인터페이스를 위한 새로운 버튼 및 입력 컴포넌트 추가.
    • 새로운 sizeStyle 상수를 통한 버튼 및 입력 필드 크기 관리 추가.
    • 새로운 환경 변수 NEXT_PUBLIC_BASE_URL 추가.
    • README 파일에 프로젝트 설정 및 사용 방법 추가.
  • 버그 수정

    • 로그인 버튼 비활성화 및 오류 메시지 표시 기능 개선.
  • 문서화

    • .env.sample 파일에 환경 변수 추가.
  • 리팩토링

    • 기존 컴포넌트를 새로운 구조로 변경하여 코드 관리 용이성 향상.
  • 스타일

    • 버튼 및 입력 필드에 대한 스타일 개선.
  • 테스트

    • 로그인 페이지에 대한 단위 테스트 추가.
  • 기타

    • 패키지 관리 및 스크립트 정의를 위한 package.json 파일 추가.
    • ESLint 및 Prettier 설정 파일 업데이트.

놀랍게도 현재 Next 15는 RC가 아니라는거~
+ 로그인 테스트 추가 (엣지케이스만)
괄호가 붙으면 URL로써 인식되지 않는데, 이 때문에 첫 페이지가 두개인 오류 발생
+ ToastContainer 추가
@HA0N1
Copy link

HA0N1 commented Nov 28, 2024

FE의 테스트 코드는 처음 보는데 사용자 행동을 테스팅하는군요! 잘봤습니다

좋았던 점

  1. describe와 it을 사용해 테스트 케이스를 그룹화하여 테스트 구조를 잘 정리 하신 거 같습니다

개선할 점

  1. login test 파일 refrehs 오타가 난 거 같습니다
  2. 에러 메시지 같은 경우에는 상수로 정의해서 사용하시면 나중에 유지보수가 쉬울 거 같습니다

Copy link

@BDlhj BDlhj left a comment

Choose a reason for hiding this comment

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

PR 잘 확인했습니다!
코멘트 남긴 모든 부분에 대해서 수정을 요청드린 건 아니니 확인하시고 기준님 판단대로 하시면 될 것 같습니다. 그러고 re-request 날려주세요~~

좋았던 점

  1. 하온님 말씀대로 테스트를 그룹화해주셔서 이해하기 편한 것 같습니다!
  2. 자주 사용되는/될 css 속성을 tailwind에서 커스텀하신 것도 앞으로 사용성 측면에서 좋다고 생각합니다👍🏻

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

좋았던 사항

  • ㅋㅋㅋㅋ벌써 많은 전쟁을 치룬 뒤에 보게 된 것 같아, 훨씬 뭐랄까 피땀눈물이 느껴진달까..? 고생많으셨어요
  • FE 에서 테스트 쉽지않은데 정말로, 깔끔한 성공 / 실패 테스트케이스 인 것 같아요. 특히 랜더링 체크에서요 :)
  • 이제 셀레니움등을 활용한 테스트도 (지금 전혀 필요없음) 살짝씩 체크해보시면 어떨까? 라는 생각도 듭니다 :)

개선 사항

  1. 개별 코멘트 한 번 확인해 주세요!
  2. 테스트 코드에서 "서버가 문제가 있을때" 에 대한 케이스가 없는 것 같습니다!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
src/app/(login)/Content.tsx (2)

10-13: 인터페이스 네이밍 컨벤션 개선이 필요합니다

인터페이스 이름은 파스칼 케이스를 사용하는 것이 일반적입니다.

다음과 같이 수정을 제안드립니다:

-interface formVo {
+interface FormData {
   access_token: string;
   refresh_token: string;
 }

42-67: 코드 구조 개선이 필요합니다

현재 구현에서 다음과 같은 개선 사항을 제안드립니다:

  1. Tailwind 클래스명이 너무 길어 가독성이 떨어집니다
  2. 문자열이 하드코딩되어 있어 유지보수가 어려울 수 있습니다

다음과 같은 방식으로 개선할 수 있습니다:

const FORM_STYLES = {
  container: "w-full h-full flex items-center justify-center",
  form: "w-fit h-fit flex flex-col gap-[30px] items-center p-[30px] bg-bg-2 rounded-[4px] shadow-[0_4px_16px_0_rgba(0,0,0,.04)]",
  title: "font-medium text-[32px] text-text-1"
};

const STRINGS = {
  title: "Velog Dashboard",
  accessToken: "Access Token을 입력하세요",
  refreshToken: "Refresh Token을 입력하세요",
  login: "로그인"
};

이렇게 분리된 상수를 컴포넌트에서 사용하면 코드의 가독성과 유지보수성이 향상됩니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff0a7 and a76c825.

📒 Files selected for processing (3)
  • .env.sample (1 hunks)
  • src/app/(login)/Content.tsx (1 hunks)
  • src/components/Input.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .env.sample
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Input.tsx
🔇 Additional comments (2)
src/app/(login)/Content.tsx (2)

38-40: 폼 제출 로직이 깔끔합니다

폼 제출 로직이 간단하고 명확하게 구현되어 있습니다.


24-36: 🛠️ Refactor suggestion

토큰 전송 방식 개선이 필요합니다

현재 구현에서 몇 가지 개선이 필요해 보입니다:

  1. 쿠키를 직접 헤더에 설정하는 것보다 서버에서 Set-Cookie 헤더를 통해 설정하는 것이 더 안전합니다.
  2. 에러 메시지가 사용자 친화적이지 않습니다.

다음과 같이 수정을 제안드립니다:

  const { mutate } = useMutation({
    mutationFn: async (data: formVo) =>
      await instance('/login', {
        method: 'POST',
-       headers: {
-         cookie: `access_token=${data.access_token};refresh_token=${data.refresh_token}`,
-       },
+       data: {
+         access_token: data.access_token,
+         refresh_token: data.refresh_token,
+       },
      }),
    onSuccess: () => replace('/main'),
    onError: (res: Response) => {
-     toast.error(`${res.statusText} (${res.status})`);
+     toast.error('로그인에 실패했습니다. 토큰을 확인해 주세요.');
    },
  });

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 69ac83e and 6959548.

📒 Files selected for processing (2)
  • eslint.config.mjs (1 hunks)
  • tsconfig.json (1 hunks)
🔇 Additional comments (2)
eslint.config.mjs (2)

27-29: ⚠️ Potential issue

TypeScript 규칙 설정 검토 필요

TypeScript 규칙 중 일부는 강화되었지만, naming-convention 규칙이 비활성화된 것이 우려됩니다. 네이밍 컨벤션은 코드 일관성과 가독성에 중요한 역할을 합니다.

다음과 같이 수정하는 것을 제안드립니다:

  '@typescript-eslint/promise-function-async': 'error',
  '@typescript-eslint/consistent-type-assertions': 'error',
- '@typescript-eslint/naming-convention': 'off',
+ '@typescript-eslint/naming-convention': 'error',

31-35: TypeScript 프로젝트 설정 검증 필요

parserOptions의 설정이 올바르게 되어있는지 확인이 필요합니다.

다음 스크립트를 실행하여 TypeScript 설정을 검증해보세요:

✅ Verification successful

Let me gather more information about the ESLint configuration and TypeScript setup.


TypeScript와 ESLint 설정이 올바르게 구성되어 있습니다

TypeScript와 ESLint 설정이 다음과 같이 적절하게 구성되어 있음을 확인했습니다:

  • tsconfig.json이 존재하며 올바른 컴파일러 옵션이 설정되어 있습니다
  • ESLint 설정에서 TypeScript 관련 플러그인과 규칙이 제대로 구성되어 있습니다
  • parserOptionsproject: truetsconfigRootDir 설정이 TypeScript 프로젝트 구조에 맞게 올바르게 설정되어 있습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: TypeScript 설정 파일 존재 여부와 설정 확인

# tsconfig.json 파일 존재 여부 확인
echo "TypeScript 설정 파일 확인:"
if [ -f "tsconfig.json" ]; then
    cat tsconfig.json | jq '.compilerOptions'
else
    echo "tsconfig.json 파일을 찾을 수 없습니다."
fi

# ESLint 설정에서 참조하는 파일들의 존재 여부 확인
echo -e "\nESLint 플러그인 의존성 확인:"
cat package.json | jq '.dependencies + .devDependencies | with_entries(select(.key | startswith("@typescript-eslint")))'

Length of output: 730


Script:

#!/bin/bash
# Check for ESLint related dependencies and configurations
echo "ESLint 관련 의존성 확인:"
cat package.json | jq '.dependencies + .devDependencies | with_entries(select(.key | contains("eslint") or .key | contains("typescript")))'

echo -e "\nESLint 설정 파일 내용:"
cat eslint.config.mjs

echo -e "\n프로젝트 루트 디렉토리의 파일 구조:"
ls -la

Length of output: 3129

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
src/app/(login)/Content.tsx (3)

38-40: 사용자 피드백 개선이 필요합니다

로딩 상태를 표시하여 사용자 경험을 개선할 수 있습니다.

+ const { mutate, isLoading } = useMutation({
  // ... mutation 설정

  const onSubmit = (data: formVo) => {
+   if (isLoading) return;
    mutate(data);
  };

51-62: 입력 필드 피드백 개선이 필요합니다

사용자에게 더 명확한 피드백을 제공하기 위해 다음과 같은 개선을 제안드립니다:

  <Input
    size="large"
    type="password"
    placeholder="Access Token을 입력하세요"
-   {...register('access_token', { required: true })}
+   {...register('access_token', { 
+     required: '액세스 토큰을 입력해주세요' 
+   })}
+   error={errors.access_token?.message}
  />

63-65: 버튼 상태 표시 개선이 필요합니다

로딩 상태를 버튼에 반영하여 사용자 경험을 개선할 수 있습니다:

- <Button size="large" form="large" type="submit" disabled={!isValid}>
+ <Button 
+   size="large" 
+   form="large" 
+   type="submit" 
+   disabled={!isValid || isLoading}
+ >
-   로그인
+   {isLoading ? '로그인 중...' : '로그인'}
  </Button>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 6959548 and ab6f0cf.

📒 Files selected for processing (5)
  • src/app/(login)/Content.tsx (1 hunks)
  • src/app/layout.tsx (2 hunks)
  • src/components/Button.tsx (1 hunks)
  • src/components/Input.tsx (1 hunks)
  • tailwind.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/app/layout.tsx
  • src/components/Button.tsx
  • src/components/Input.tsx
  • tailwind.config.ts

import { Button, Input } from '@/components';
import { useForm } from 'react-hook-form';
import { useMutation } from '@tanstack/react-query';
import { instance } from '../../api';
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

인터페이스 네이밍 컨벤션 개선이 필요합니다

TypeScript 네이밍 컨벤션에 따라 다음과 같이 수정을 제안드립니다:

-interface formVo {
+interface LoginFormData {
   access_token: string;
   refresh_token: string;
 }

또한 상대 경로 대신 절대 경로를 사용하는 것이 좋습니다:

-import { instance } from '../../api';
+import { instance } from '@/api';

Also applies to: 10-13

Comment on lines +24 to +36
const { mutate } = useMutation({
mutationFn: async (data: formVo) =>
await instance('/login', {
method: 'POST',
headers: {
cookie: `access_token=${data.access_token};refresh_token=${data.refresh_token}`,
},
}),
onSuccess: () => replace('/main'),
onError: (res: Response) => {
toast.error(`${res.statusText} (${res.status})`);
},
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

보안 및 에러 처리 개선이 필요합니다

  1. 쿠키를 직접 헤더에 포함시키는 것은 보안상 위험할 수 있습니다.
  2. 사용자에게 기술적인 에러 메시지를 직접 노출하는 것은 좋지 않습니다.

다음과 같이 수정을 제안드립니다:

  const { mutate } = useMutation({
    mutationFn: async (data: formVo) =>
      await instance('/login', {
        method: 'POST',
-       headers: {
-         cookie: `access_token=${data.access_token};refresh_token=${data.refresh_token}`,
-       },
+       data: {
+         access_token: data.access_token,
+         refresh_token: data.refresh_token,
+       },
      }),
    onSuccess: () => replace('/main'),
    onError: (res: Response) => {
-     toast.error(`${res.statusText} (${res.status})`);
+     toast.error('로그인에 실패했습니다. 다시 시도해 주세요.');
    },
  });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { mutate } = useMutation({
mutationFn: async (data: formVo) =>
await instance('/login', {
method: 'POST',
headers: {
cookie: `access_token=${data.access_token};refresh_token=${data.refresh_token}`,
},
}),
onSuccess: () => replace('/main'),
onError: (res: Response) => {
toast.error(`${res.statusText} (${res.status})`);
},
});
const { mutate } = useMutation({
mutationFn: async (data: formVo) =>
await instance('/login', {
method: 'POST',
data: {
access_token: data.access_token,
refresh_token: data.refresh_token,
},
}),
onSuccess: () => replace('/main'),
onError: (res: Response) => {
toast.error('로그인에 실패했습니다. 다시 시도해 주세요.');
},
});

Copy link

@BDlhj BDlhj left a comment

Choose a reason for hiding this comment

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

힘든 싸움을 하셨군요..!! 수고하셨습니다👏🏻

@six-standard
Copy link
Member Author

좋았던 사항

  • ㅋㅋㅋㅋ벌써 많은 전쟁을 치룬 뒤에 보게 된 것 같아, 훨씬 뭐랄까 피땀눈물이 느껴진달까..? 고생많으셨어요
  • FE 에서 테스트 쉽지않은데 정말로, 깔끔한 성공 / 실패 테스트케이스 인 것 같아요. 특히 랜더링 체크에서요 :)
  • 이제 셀레니움등을 활용한 테스트도 (지금 전혀 필요없음) 살짝씩 체크해보시면 어떨까? 라는 생각도 듭니다 :)

개선 사항

  1. 개별 코멘트 한 번 확인해 주세요!
  2. 테스트 코드에서 "서버가 문제가 있을때" 에 대한 케이스가 없는 것 같습니다!

"서버에 문제가 있을때" 케이스에 대해서는 리팩토링 브랜치에서 진행하도록 하겠습니다.
해당 부분이 커스텀 에러 핸들링과 겹칠 것으로 보이는데, 커스텀 에러 핸들링을 리팩토링 브랜치에서 작업할 예정이어서요.

import { Button, Input } from '@/components';
import { useForm } from 'react-hook-form';
import { useMutation } from '@tanstack/react-query';
import { instance } from '../../api';
Copy link
Member

Choose a reason for hiding this comment

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

아 이건 진짜 바꾸면 좋긴한데 ㅋㅋㅋㅋㅋㅋㅋ next config 에서 세팅해놓고 절대 경로로 하는게 진짜! 크,, 다음 브랜치 기약 오케이 합니다~

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

고생 너무많으셨어요! 리펙토링 브랜치를 기약합시다! 아자아자!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants