-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat: #BBB-130 기술 서적 스터디 해설 영상 업로드 버튼 구현 #39
Conversation
…signup#BBB-137 [BBB-137] Feat: 로그인 및 회원가입 시 종단간 암호화 적용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review by ChatGPT
)} | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 진행하겠습니다.
문제점 및 개선 사항
-
에러 처리 부족:
- 여러 비동기 호출(예:
generateUploadId
,generatePresignedUrl
,uploadVideo
,completeMultipartUpload
,checkVideoUploadStatus
)에서 에러가 발생할 경우 적절한 에러 처리가 필요합니다. 현재 catch 블록이 없어서, 문제가 발생했을 때 사용자가 인지할 수 없습니다.
try { // 비동기 처리 logic } catch (error) { toast.error('업로드 중 오류가 발생했습니다.'); }
- 여러 비동기 호출(예:
-
중복 코드:
checkVideoUploadStatus
의 응답 처리 부분이completeMultipartUpload
의 응답 처리와 유사합니다. 오류 메시지가 중복되어 있으므로, 이를 함수로 뺄 수 있습니다.
-
미사용 변수:
const completedParts = [];
는 비어있는 배열로 초기화되지만, 업로드가 실패할 경우 호출되지 않을 수 있습니다. 업로드가 실패해도 최종적으로 올바른 상태를 유지할 수 있도록 확인이 필요합니다.
-
상태 초기화:
- 업로드가 완료된 후에
setFile(null)
을 호출하고 있지만, 파일 선택기에서 여전히 이전 파일의 이름이 표시됩니다. 나중에 사용자 경험을 고려하여 상태를 명확하게 관리하는 것이 좋습니다.
- 업로드가 완료된 후에
-
파일 타입 확인:
- 파일 업로드를 처리하기 전에 업로드할 파일 형식이 올바른지 체크하는 로직이 필요합니다. 예를 들어, 사용자가 .exe 파일과 같은 잘못된 파일을 선택했다면 경고 메시지를 출력할 수 있습니다.
-
스크롤 및 UI 상태 유지:
- 업로드 진행 중에 버튼 비활성화 처리가 없으므로, 사용자 경험 요청이 있을 수 있습니다. 업로드가 진행 중일 때는 버튼을 비활성화하고 로딩 상태를 표시하는 것이 좋겠습니다.
-
애플리케이션 상태 관리:
myAssignmentId
,file
상태가 변경될 때마다 조건부 렌더링을 사용하는데, 이는 효율성을 떨어뜨릴 수 있습니다. React의useMemo
훅을 활용하여 최적화할 수 있습니다.
위의 개선 사항들을 고려하면 코드의 안정성과 사용자 경험을 크게 향상시킬 수 있습니다.
@@ -187,7 +209,7 @@ function SelectRound({ | |||
}; | |||
|
|||
return ( | |||
<div className="flex items-center w-1/5 justify-end"> | |||
<div className="flex items-center justify-end"> | |||
<Select | |||
value={(round.idx + 1).toString()} | |||
onValueChange={handleRoundChange} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 진행하겠습니다.
-
버그 리스크:
DashBoardHeader
컴포넌트에서studyId
와round
가 필수속성으로 추가되었는데, 이 값들이 부모 컴포넌트에서 항상 제공되는지 검증해야 합니다. 만약 이 값들이undefined
일 경우, 관련된 UI 또는 기능에 오류가 발생할 수 있습니다.
-
유지보수성:
if
조건문에서studyType
을 문자열로 직접 비교하는 대신, Enum(예:StudyType.ALGORITHM
,StudyType.BOOK
)을 사용하는 것이 좋습니다. 다른 곳에서도 동일한 로직이 필요할 경우 코드의 일관성을 유지하고 오류를 줄일 수 있습니다.
-
코드 중복:
StudyDashBoard
함수 내에서DashBoardHeader
를 호출하는 코드가 두 번 반복되고 있습니다. 이를 리팩토링하여 중복을 줄일 수 있을 것입니다. 예를 들어,studyId
,round
,setRound
,details
를 하나의 객체로 만들어 함수로 전달할 수 있습니다.
-
UI 요소 조건 처리:
details.studyType === 'BOOK'
조건문에서VideoUploadButton
을 조건부 렌더링하고 있지만, 추가적인 경우나 다른 유형의studyType
이 생길 경우을 염두에 두고 좀 더 일반적인 방식을 고려할 수 있습니다. 즉, 이러한 조건문을 별도의 함수로 분리하는 것도 고려해볼 수 있습니다.
-
주석 및 문서화:
- 코드의 가독성을 높이기 위해 주요 함수 및 컴포넌트에 대한 주석을 추가하는 것이 좋습니다. 현재 코드에서 각 속성이 어떤 역할을 하는지 간략히 설명하는 주석이 있다면 코드 이해도가 높아질 것입니다.
-
CSS 클래스 이름:
- 클래스 이름들이
bg-background
,rounded-lg
,p-6
등과 같이 정해진 스타일이지만, 스타일을 정의할 때 사용할 CSS-in-JS 라이브러리(예: styled-components) 또는 CSS 모듈을 활용하면 더욱 세밀하게 스타일을 조절할 수 있습니다.
- 클래스 이름들이
이러한 점을 반영하여 코드 품질을 향상시킬 수 있을 것입니다. 추가 질문이 있으시면 언제든지 말씀해 주세요!
{ assignmentId: assignmentId }, | ||
getAuthenticationConfig() | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치를 검토해본 결과, 몇 가지 개선점과 잠재적인 버그 리스크가 보입니다.
-
환경 변수 유효성 검사:
process.env.NEXT_PUBLIC_API_SERVER_URL
이 정의되어 있는지 확인하는 코드가 없습니다. 만약 이 변수가 정의되어 있지 않다면 API 호출 시 오류가 발생할 수 있습니다. 이를 체크하는 로직을 추가하는 것이 좋습니다.
if (!process.env.NEXT_PUBLIC_API_SERVER_URL) { throw new Error("API 서버 URL이 설정되지 않았습니다."); }
-
에러 처리:
- axios의 요청은 성공적인 응답뿐만 아니라 실패할 경우도 발생할 수 있습니다. 현재 코드에서는 에러 처리 로직이 없습니다. 예외 처리(try-catch)를 통해 에러를 캡처하고 적절히 처리하는 것이 중요합니다.
return axios.post(...) .then(response => response.data) .catch(error => { console.error("업로드 상태 확인 중 오류 발생:", error); throw error; // 호출자에게 오류 전달 });
-
타입 안정성:
assignmentId
와studyId
의 타입이 숫자로 고정되어 있는데, 이 값들이 제대로 숫자형인지 확인하는 검증이 필요할 수 있습니다. 타입 체크를 위한 로직을 추가하면 더 안전합니다.
if (typeof studyId !== 'number' || typeof assignmentId !== 'number') { throw new Error("studyId와 assignmentId는 숫자여야 합니다."); }
-
함수 설명 주석:
- 함수의 설명 주석이 기본적인 내용만 포함되어 있습니다. 반환값이나 이 함수의 사용 예시, 또는 발생할 수 있는 에러에 대한 설명을 추가하면 코드 사용에 도움이 될 수 있습니다.
이러한 사항들을 반영하면 코드의 안정성과 가독성이 높아질 것으로 보입니다.
{ uploadId, parts, studyId, assignmentId }, | ||
getAuthenticationConfig() | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰에 대해 의견을 말씀드리겠습니다.
-
환경 변수 검사:
process.env.NEXT_PUBLIC_API_SERVER_URL
이 설정되어 있지 않은 경우, API 호출 시 문제가 발생할 수 있습니다. 이러한 경우를 방지하기 위해 코드 상단에서 해당 환경 변수가 정의되어 있는지 확인하고, 그렇지 않은 경우 예외를 발생시키는 로직을 추가하는 것이 좋습니다. -
에러 핸들링: 각 Axios 요청에 대해 에러 핸들링을 추가하는 것이 좋습니다. 요청이 실패할 경우 사용자에게 적절한 피드백을 줄 수 있도록 try-catch 블록을 사용할 수 있습니다.
-
타입 검사:
uploadVideo
함수에서File | Blob
타입을 받는 경우, 적절한 파일인지 확인하는 추가 검사를 통해 안정성을 높일 수 있습니다. 예를 들어, 파일 크기나 형식을 검사할 수 있습니다. -
직접적인 API URL 사용: 현재 API URL을 직접 문자열로 사용하고 있습니다. API 버전이나 엔드포인트가 변경될 경우 여러 곳에서 수정을 해야 하므로, 상수로 정의하고 재사용하는 것이 좋습니다.
-
주석 및 문서화: 함수에 주석이 잘 작성되어 있지만, 매개변수의 타입에 대한 설명이 추가되면 더 좋을 것 같습니다. 특히 숫자 타입이 어떤 값을 의미하는지 간단한 설명을 추가하면 가독성이 향상됩니다.
-
반환 값: 현재 각 함수는 Axios 요청의 반환 값을 그대로 반환하고 있습니다. 이를 Promise 형태로 변환하거나, 응답 데이터를 가공하여 반환하는 것도 고려해볼 수 있습니다.
이 점들을 고려하여 개선하면, 코드의 안정성 및 가독성이 높아질 것입니다.
studyType: string; | ||
studyId: number; | ||
round: Round; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 리뷰를 제공하겠습니다.
-
타입 안정성:
studyType
을string
으로 정의했는데, 가능한 선택지를 제한할 수 있다면, enum이나 union type을 사용하는 것이 좋습니다. 예를 들어,studyType
이 특정 값들 중 하나여야 한다면 더 안전하게 코드를 작성할 수 있습니다. -
문서화:
VideoUploadButtonProps
인터페이스의 각 속성에 대한 설명 주석을 추가하는 것이 좋습니다. 이는 코드 유지 보수성과 가독성을 높이는 데 도움이 됩니다. -
import 경로: 경로가 절대 경로(
@/types/study/study-detail
)로 설정되어 있는데, 이 경로가 유효한지 확인 필요합니다. 상대 경로를 사용하는 것이 더 명확할 수 있습니다. -
추후 확장성: 만약 다른 속성이 추가될 가능성이 있다면, 현재의 구조에 맞춰 향후 확장성을 고려하여 미리 설계해두는 것이 좋습니다.
이외에도 현재 코드에는 큰 버그 리스크는 없어 보입니다. 추가적인 검토 사항이나 기능적 요구사항에 따라 더 깊이 있는 분석이 필요할 수 있습니다.
작업 개요
기술 서적 스터디의 해설 영상 업로드를 위한 버튼 구현 (multipart upload 구현)
전달 사항
s3에 task/${studyId}/${assignmentId} 라는 objectName으로 영상이 업로드된다.