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

어드민 폴더 제거 #57

Merged
merged 2 commits into from
Oct 29, 2024
Merged

어드민 폴더 제거 #57

merged 2 commits into from
Oct 29, 2024

Conversation

rhehfl
Copy link
Collaborator

@rhehfl rhehfl commented Oct 28, 2024

🔗 관련 이슈

#56

📝작업 내용

어드민 폴더와 관련된 파일, 코드 등을 제거했습니다.

🔍 변경 사항

  • 어드민 폴더 제거

💬리뷰 요구사항 (선택사항)

@rhehfl rhehfl added the 🔨 Refactor 코드 리팩토링 label Oct 28, 2024
@rhehfl rhehfl self-assigned this Oct 28, 2024
@rhehfl rhehfl linked an issue Oct 28, 2024 that may be closed by this pull request
1 task
src/apis/quiz.ts Outdated
import api from './axios/instance';

const QUIZ = {
getQuizzes: (sectionId: number, part: Quiz['part']) => {
getQuizzes: (sectionId: number, part: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

히스토리를 모르는데 Quiz['part']의 타입도 string 이였을 까요?
더 좁은 타입에서 더 넓은 타입으로 옮겼다면 그 이유가 궁금하네요 🤗

Copy link
Collaborator Author

@rhehfl rhehfl Oct 28, 2024

Choose a reason for hiding this comment

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

기존 part는 enum 타입으로 part: 'EASY' | 'NORAML' | 'HARD' | 'VERY_HARD'; 의 형식을 갖고 있었는데 팀원들과 이야기를 해본 결과 변수라는 큰 타이틀에서 파트를 난이도로 나누는 것 보다는 변수 섹션의 let 파트 const 파트 등으로 좀 더 섹션에 맞게 파트를 쪼개는게 좋다구 생각해서 string 타입으로 변경했습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

어떤 의사결정이 있었는지 히스토리를 모르니까 어떤걸 의도했는지 확 와닿지가 않네요....😂
타입은 명확하게 할 수 있다면 명확하게 정의하는게 개발 단계에서의 실수를 줄이기 때문에 좋다는 마인드이긴 해서요

변수라는 큰 타이틀에서 파트를 난이도로 나누는 것 보다는 변수 섹션의 let 파트 const 파트 등으로 좀 더 섹션에 맞게 파트를 쪼갠다.

이게 어떤걸 말씀하시는건지 설명이 가능할까요? 뭔가 설명이 힘든 부분이 있다면 나중에 실제로 코드보면서 알려주세요 ㅎㅎㅎㅎㅎㅎ

Copy link
Collaborator Author

@rhehfl rhehfl Oct 28, 2024

Choose a reason for hiding this comment

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

저희 사이트는 문제를 크게 2가지로 섹션파트로 분류합니다 섹션에는 프로그래밍을 처음 배울 때 변수, 연산자, 조건문 등 책에 나와있는 챕터 등이 해당되고 파트는 기존에는 'EASY' | 'NORAML' | 'HARD' | 'VERY_HARD' 이 4가지로 분류되었습니다 .

하지만 팀원들과 이야기를 해본 결과 위 4가지로 나누는 것 보다는 명확한 단위로 나누어졌으면 좋다고 결론이 났습니다. 그렇게 나누어진 파트가 변수 섹션을 let, const, 호이스팅 파트 등등으로 조금 더 섹션별로 세부적으로 나누어서 관리하기로 했다는 의미였습니다

따라서 기존에는 변수라는 퀴즈에 'EASY' | 'NORAML' | 'HARD' | 'VERY_HARD'라는 4가지 난이도가 있었지만 이제는 변수에 let, const, 호이스팅 파트 등등 섹션별로 다양한 파트가 올 수 있기 떄문에 string 타입으로 타입을 넓혔습니다!

/quizzes?sectionId=1&part=VERY_HARD//기존 api 요청 part가 EASY' | 'NORAML' | 'HARD' | 'VERY_HARD  하나

/quizzes?sectionId=1&part=const //변경된 변수 섹션의 api 요청

/quizzes?sectionId=3&part=if //변경된 조건문 섹션의 api 요청

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rhehfl 이해했습니다! 그러면 이 const, if let, hoisting 등의 다양한 섹션이 있다고 이해했고, 그럼에도 불구하고 이런 키워드들은 타입을 특정하기가 어려운 부분일까요? 😲

Copy link
Collaborator Author

@rhehfl rhehfl Oct 28, 2024

Choose a reason for hiding this comment

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

섹션은 변수, 함수 등등 일종의 챕터가 들어 갈 수 있는데요 말씀 주신것처럼 파트는 이 섹션마다 let, if, arrowFunction 등등 너무 다양한 파트로 나눌 수 있기때문에 전처럼 타입을 특정하기는 어려울 것 같습니다

다만 리뷰를 통해 제 코드를 다시 보면서 느낀점이 있는데요

Quiz {
  id: number;
  part: 'EASY' | 'NORAML' | 'HARD' | 'VERY_HARD';
  sectionId: number;
  title: string;
  question: string;
  answer: string[];
  category: 'COMBINATION' | 'MULTIPLE_CHOICE' | 'OX_SELECTOR' | 'SHORT_ANSWER';
  answerChoice: string[];
}

위는 현재 Quiz인터페이스입니다.

const QUIZ = {
  getQuizzes: (sectionId: number, part: Quiz['part']) => {
  getQuizzes: (sectionId: number, part: string) => {

이런식으로 수정하기보단

getQuizzes: (sectionId: number, part: Quiz['part'])

이 부분을 그대로 놔두고
Quiz 인터페이스를

part:string

이런식으로 수정하는게 훨씬 좋을 것 같다는 생각이 듭니다.

Copy link
Collaborator

@ssi02014 ssi02014 Oct 28, 2024

Choose a reason for hiding this comment

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

@rhehfl

getQuizzes: (sectionId: number, part: Quiz['part'])

위와 같이 기존 코드를 유지한다는 의견 좋네요 👍

part의 이 너무 다양한 케이스의 범위가 너무 넓어서 enum 형태로 관리했을 때 오히려 가독성을 해치는 등 개발자 경험(DX) 관점에서 악영향이라고 판단된다면 string도 좋습니다! 🤗

import api from './axios/instance';

const QUIZ = {
getQuizzes: (sectionId: number, part: Quiz['part']) => {
getQuizzes: (sectionId: number, part: string) => {
return useQuery({
queryKey: ['quizzes', { sectionId, part }],
queryFn: () => api.get(`/quizzes?sectionId=${sectionId}&part=${part}`),
Copy link
Collaborator

@ssi02014 ssi02014 Oct 28, 2024

Choose a reason for hiding this comment

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

@rhehfl @bluetree7878
axios를 커스텀하게 활용하려고 api를 만든 것 같은데 쿼리스트링을 편하게 관리하는 방법이 있어요 :)
axios에는 paramsSerializer 옵션이 있는데 이를 query-string 라이브러리와 함께 활용하면 api 요청 시 쿼리를 관리하는게 편합니다. 지금과 같이 문자열로 늘어뜨려서 활용 할 필요 없어집니다.

아래는 단순 예제입니다. 😄

import queryString from 'query-string';

// 한번 더 추상화
const getAPI = ({ url, params }) => {
  const response = axiosInstance({
    method: 'GET',
    url,
    params,
    paramsSerializer: (params) => {
      // query 객체를 stringify로 변환 시 null/undefined와 빈 문자열은 제외
      return queryString.stringify(params, { skipEmptyString: true, skipNull: true })
    },
    // ... 그 외 옵션들
  });
}
// as-is
api.get(`/quizzes?sectionId=${sectionId}&part=${part}`); // 쿼리를 하나하나 작성해줘야 함
getAPI({ url: 'https://www.example.com', params: { a: 1, b: 2 }}); // params 객체로 한번에 처리
// request: GET https://www.example.com?a=1&b=2

getAPI({ url: 'https://www.example.com', params: { a: 1, b: 2, c: null, d: '', e: 3 }});
// request: GET https://www.example.com?a=1&b=2&e=3

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오.. 이런식으로 동적으로 쿼리스트링을 붙일 수 있으면 만들어놓은 커스텀 api의 재사용성이 올라갈 것 같네요
이제까지 axios 사용하면서 저 옵션이 있는 줄 몰랐습니다 좋은 기능 & 라이브러리 추천 감사힙니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rhehfl 좋아요😄👍 query-string 라이브러리는 쿼리 핸들링할때 많이 쓰이는 라이브러리라 좋은 조합이에요

@bluetree7878
Copy link
Collaborator

bluetree7878 commented Oct 28, 2024

어드민 페이지를 레포별로 따로 분리해놓는 거 정말 좋은 선택인 것 같아요. 어드민 서버까지 따로 열면 관리하기도 편하고 다른 사용자가 주소에 admin을 쳐서 나올 일까지 없애버리니 아주 좋네여!

이건 다른 얘긴데 지금 현재 저희 코코 개발서버에

[plugin:vite:import-analysis] Failed to resolve import "../pages/login/login" from "src/route/Router.tsx". Does the file exist?

라는 문구로 오류를 띄우고 있는 거 보니 폴더명이나 파일명 하나가 잘못 된 것 같은데 혹시 이거 봐주실 수 있을까요?

@rhehfl
Copy link
Collaborator Author

rhehfl commented Oct 28, 2024

아까 같이 봤던 부분이네요 대소문자때문에 오류 나는 것 같습니다.
login/login에서
login/Login
이런식으로 수정하시면 고쳐질 것 같습니다.

@rhehfl rhehfl merged commit 7d114ca into develop Oct 29, 2024
@rhehfl rhehfl deleted the refactor/#56/admin_page_remove branch October 29, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 Refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

admin page remove
3 participants