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

Feat: #BBB-130 기술 서적 스터디 해설 영상 업로드 버튼 구현 #39

Merged
merged 3 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions components/study/button/video-upload-button.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
'use client';

import { useEffect, useState } from 'react';
import { Button } from '@/components/ui/button/button';
import { Upload } from 'lucide-react';
import {
completeMultipartUpload,
generatePresignedUrl,
generateUploadId,
uploadVideo
} from '@/lib/api/video/video-upload';
import checkVideoUploadStatus from '@/lib/api/study/check-video-upload-status';
import { toast } from 'react-toastify';
import { useRecoilState } from 'recoil';
import { userState } from '@/recoil/userAtom';
import { BookRound } from '@/types/study/study-detail';
import { VideoUploadButtonProps } from '@/types/study/video-upload-button';

export default function VideoUploadButton({
studyType,
studyId,
round
}: VideoUploadButtonProps) {
const [myData, setMyData] = useRecoilState(userState);
const [myAssignmentId, setMyAssignmentId] = useState<number>();
const [file, setFile] = useState<File | null>(null);

useEffect(() => {
if (studyType === 'BOOK' && myData) {
const BookRound = round as BookRound;
setMyAssignmentId(BookRound.users[myData.id].assignmentId);
}
}, [studyType, myData, round]);

if (myAssignmentId == null) return null;

const handleFileChange = (event: React.ChangeEvent<HTMLInputElement>) => {
if (event.target.files && event.target.files[0]) {
setFile(event.target.files[0]);
}
};

const handleUpload = async () => {
if (file == null) return;

const completedParts = [];
const respose = await generateUploadId(studyId, myAssignmentId);
const uploadId = respose.data.uploadId;

const partSize = 5 * 1024 * 1024;
const totalParts = Math.ceil(file.size / partSize);
for (let partNumber = 1; partNumber <= totalParts; partNumber++) {
const start = (partNumber - 1) * partSize;
const end = Math.min(start + partSize, file.size + 1);
const chunk = file.slice(start, end);
const response = await generatePresignedUrl(
uploadId,
partNumber,
chunk.size,
studyId,
myAssignmentId
);
const presignedUrl = response.data.presignedUrl;
const result = await uploadVideo(presignedUrl, chunk);
const eTag = result.headers.etag;
completedParts.push({ partNumber, eTag });
}

const completeResponse = await completeMultipartUpload(
uploadId,
completedParts,
studyId,
myAssignmentId
);

if (completeResponse.status === 200) {
console.log(myAssignmentId);
checkVideoUploadStatus(studyId, myAssignmentId).then((response) => {
if (response.status === 200) {
toast.success('업로드에 성공했습니다.');
} else {
toast.error('업로드에 실패했습니다.');
}
});
} else {
toast.error('업로드에 실패했습니다.');
}
setFile(null);
};

return (
<div className="flex justify-end space-x-2 flex-shrink-0">
<input
type="file"
id="assignment-upload"
className="sr-only"
onChange={handleFileChange}
accept=".mp4,.mov,.avi,.mkv,.wmv,.flv,.webm"
/>
<label htmlFor="assignment-upload" className="cursor-pointer">
<Button size="sm" variant="outline" asChild>
<span>
<Upload className="w-4 h-4 mr-2" />
{file ? file.name : '과제 업로드'}
</span>
</Button>
</label>
{file && (
<Button size="sm" onClick={handleUpload}>
업로드
</Button>
)}
</div>
);
}

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다.

문제점 및 개선 사항

  1. 에러 처리 부족:

    • 여러 비동기 호출(예: generateUploadId, generatePresignedUrl, uploadVideo, completeMultipartUpload, checkVideoUploadStatus)에서 에러가 발생할 경우 적절한 에러 처리가 필요합니다. 현재 catch 블록이 없어서, 문제가 발생했을 때 사용자가 인지할 수 없습니다.
    try {
        // 비동기 처리 logic
    } catch (error) {
        toast.error('업로드 중 오류가 발생했습니다.');
    }
  2. 중복 코드:

    • checkVideoUploadStatus의 응답 처리 부분이 completeMultipartUpload의 응답 처리와 유사합니다. 오류 메시지가 중복되어 있으므로, 이를 함수로 뺄 수 있습니다.
  3. 미사용 변수:

    • const completedParts = [];는 비어있는 배열로 초기화되지만, 업로드가 실패할 경우 호출되지 않을 수 있습니다. 업로드가 실패해도 최종적으로 올바른 상태를 유지할 수 있도록 확인이 필요합니다.
  4. 상태 초기화:

    • 업로드가 완료된 후에 setFile(null)을 호출하고 있지만, 파일 선택기에서 여전히 이전 파일의 이름이 표시됩니다. 나중에 사용자 경험을 고려하여 상태를 명확하게 관리하는 것이 좋습니다.
  5. 파일 타입 확인:

    • 파일 업로드를 처리하기 전에 업로드할 파일 형식이 올바른지 체크하는 로직이 필요합니다. 예를 들어, 사용자가 .exe 파일과 같은 잘못된 파일을 선택했다면 경고 메시지를 출력할 수 있습니다.
  6. 스크롤 및 UI 상태 유지:

    • 업로드 진행 중에 버튼 비활성화 처리가 없으므로, 사용자 경험 요청이 있을 수 있습니다. 업로드가 진행 중일 때는 버튼을 비활성화하고 로딩 상태를 표시하는 것이 좋겠습니다.
  7. 애플리케이션 상태 관리:

    • myAssignmentId, file 상태가 변경될 때마다 조건부 렌더링을 사용하는데, 이는 효율성을 떨어뜨릴 수 있습니다. React의 useMemo 훅을 활용하여 최적화할 수 있습니다.

위의 개선 사항들을 고려하면 코드의 안정성과 사용자 경험을 크게 향상시킬 수 있습니다.

30 changes: 26 additions & 4 deletions components/study/dashboard/dashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { useParams } from 'next/navigation';
import { useRecoilState } from 'recoil';
import FeedbackDialog from '../feedback-dialog';
import { BookRow } from './book-row';
import VideoUploadButton from '@/components/study/button/video-upload-button';

export default function StudyDashBoard({
details,
Expand All @@ -43,7 +44,12 @@ export default function StudyDashBoard({
if (studyType === StudyType.ALGORITHM) {
return (
<div className="mt-5 bg-background rounded-lg border p-6 w-full max-w-4xl h-full">
<DashBoardHeader round={round} setRound={setRound} details={details} />
<DashBoardHeader
studyId={studyId}
round={round}
setRound={setRound}
details={details}
/>
<AlgorithmDashBoardBody
round={round as AlgorithmRound}
studyId={studyId}
Expand All @@ -53,7 +59,12 @@ export default function StudyDashBoard({
} else if (studyType === StudyType.BOOK) {
return (
<div className="mt-5 bg-background rounded-lg border p-6 w-full max-w-4xl h-full">
<DashBoardHeader round={round} setRound={setRound} details={details} />
<DashBoardHeader
studyId={studyId}
round={round as BookRound}
setRound={setRound}
details={details}
/>
<BookDashBoardBody round={round as BookRound} studyId={studyId} />
</div>
);
Expand Down Expand Up @@ -147,18 +158,29 @@ function BookDashBoardBody({
}

function DashBoardHeader({
studyId,
round,
setRound,
details
}: {
studyId: number;
details: StudyDetails;
round: Round;
setRound: (round: Round) => void;
}) {
return (
<div className="flex items-center justify-between mb-6">
<h2 className="text-2xl font-bold">{details.name}</h2>
<SelectRound round={round} setRound={setRound} />
<div className="flex space-x-5">
{details.studyType === 'BOOK' ? (
<VideoUploadButton
studyType={details.studyType}
studyId={studyId}
round={round}
/>
) : null}
<SelectRound round={round} setRound={setRound} />
</div>
</div>
);
}
Expand Down Expand Up @@ -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}

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다.

  1. 버그 리스크:

    • DashBoardHeader 컴포넌트에서 studyIdround가 필수속성으로 추가되었는데, 이 값들이 부모 컴포넌트에서 항상 제공되는지 검증해야 합니다. 만약 이 값들이 undefined일 경우, 관련된 UI 또는 기능에 오류가 발생할 수 있습니다.
  2. 유지보수성:

    • if 조건문에서 studyType을 문자열로 직접 비교하는 대신, Enum(예: StudyType.ALGORITHM, StudyType.BOOK)을 사용하는 것이 좋습니다. 다른 곳에서도 동일한 로직이 필요할 경우 코드의 일관성을 유지하고 오류를 줄일 수 있습니다.
  3. 코드 중복:

    • StudyDashBoard 함수 내에서 DashBoardHeader를 호출하는 코드가 두 번 반복되고 있습니다. 이를 리팩토링하여 중복을 줄일 수 있을 것입니다. 예를 들어, studyId, round, setRound, details를 하나의 객체로 만들어 함수로 전달할 수 있습니다.
  4. UI 요소 조건 처리:

    • details.studyType === 'BOOK' 조건문에서 VideoUploadButton을 조건부 렌더링하고 있지만, 추가적인 경우나 다른 유형의 studyType이 생길 경우을 염두에 두고 좀 더 일반적인 방식을 고려할 수 있습니다. 즉, 이러한 조건문을 별도의 함수로 분리하는 것도 고려해볼 수 있습니다.
  5. 주석 및 문서화:

    • 코드의 가독성을 높이기 위해 주요 함수 및 컴포넌트에 대한 주석을 추가하는 것이 좋습니다. 현재 코드에서 각 속성이 어떤 역할을 하는지 간략히 설명하는 주석이 있다면 코드 이해도가 높아질 것입니다.
  6. CSS 클래스 이름:

    • 클래스 이름들이 bg-background, rounded-lg, p-6 등과 같이 정해진 스타일이지만, 스타일을 정의할 때 사용할 CSS-in-JS 라이브러리(예: styled-components) 또는 CSS 모듈을 활용하면 더욱 세밀하게 스타일을 조절할 수 있습니다.

이러한 점을 반영하여 코드 품질을 향상시킬 수 있을 것입니다. 추가 질문이 있으시면 언제든지 말씀해 주세요!

Expand Down
19 changes: 19 additions & 0 deletions lib/api/study/check-video-upload-status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import axios from 'axios';
import { getAuthenticationConfig } from '@/lib/utils';

/**
* 해설 영상 업로드 완료 요청 API
*
* @param studyId
* @param assignmentId
*/
export default function checkVideoUploadStatus(
studyId: number,
assignmentId: number
) {
return axios.post(
`${process.env.NEXT_PUBLIC_API_SERVER_URL}/api/v1/studies/${studyId}/upload-status`,
{ assignmentId: assignmentId },
getAuthenticationConfig()
);
}

Choose a reason for hiding this comment

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

코드 패치를 검토해본 결과, 몇 가지 개선점과 잠재적인 버그 리스크가 보입니다.

  1. 환경 변수 유효성 검사:

    • process.env.NEXT_PUBLIC_API_SERVER_URL이 정의되어 있는지 확인하는 코드가 없습니다. 만약 이 변수가 정의되어 있지 않다면 API 호출 시 오류가 발생할 수 있습니다. 이를 체크하는 로직을 추가하는 것이 좋습니다.
    if (!process.env.NEXT_PUBLIC_API_SERVER_URL) {
      throw new Error("API 서버 URL이 설정되지 않았습니다.");
    }
  2. 에러 처리:

    • axios의 요청은 성공적인 응답뿐만 아니라 실패할 경우도 발생할 수 있습니다. 현재 코드에서는 에러 처리 로직이 없습니다. 예외 처리(try-catch)를 통해 에러를 캡처하고 적절히 처리하는 것이 중요합니다.
    return axios.post(...)
      .then(response => response.data)
      .catch(error => {
        console.error("업로드 상태 확인 중 오류 발생:", error);
        throw error; // 호출자에게 오류 전달
      });
  3. 타입 안정성:

    • assignmentIdstudyId의 타입이 숫자로 고정되어 있는데, 이 값들이 제대로 숫자형인지 확인하는 검증이 필요할 수 있습니다. 타입 체크를 위한 로직을 추가하면 더 안전합니다.
    if (typeof studyId !== 'number' || typeof assignmentId !== 'number') {
      throw new Error("studyId와 assignmentId는 숫자여야 합니다.");
    }
  4. 함수 설명 주석:

    • 함수의 설명 주석이 기본적인 내용만 포함되어 있습니다. 반환값이나 이 함수의 사용 예시, 또는 발생할 수 있는 에러에 대한 설명을 추가하면 코드 사용에 도움이 될 수 있습니다.

이러한 사항들을 반영하면 코드의 안정성과 가독성이 높아질 것으로 보입니다.

70 changes: 70 additions & 0 deletions lib/api/video/video-upload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import axios from 'axios';
import { getAuthenticationConfig } from '@/lib/utils';

/**
* AWS S3 multipart 업로드 id 생성 API
*
* @param studyId
* @param assignmentId
*/
export async function generateUploadId(studyId: number, assignmentId: number) {
return axios.post(
`${process.env.NEXT_PUBLIC_API_SERVER_URL}/api/v1/videos/initiate-upload`,
{ studyId, assignmentId },
getAuthenticationConfig()
);
}

/**
* AWS S3 presigned url 생성 API
*
* @param uploadId
* @param partNumber
* @param partSize
* @param studyId
* @param assignmentId
*/
export async function generatePresignedUrl(
uploadId: string,
partNumber: number,
partSize: number,
studyId: number,
assignmentId: number
) {
return axios.post(
`${process.env.NEXT_PUBLIC_API_SERVER_URL}/api/v1/videos/presigned-url`,
{ uploadId, partNumber, partSize, studyId, assignmentId },
getAuthenticationConfig()
);
}

/**
* AWS S3에 비디오 업로드 API
*
* @param presignedUrl
* @param file
*/
export async function uploadVideo(presignedUrl: string, file: File | Blob) {
return axios.put(presignedUrl, file);
}

/**
* AWS S3 multipart 업로드 완료 요청 API
*
* @param uploadId
* @param parts
* @param studyId
* @param assignmentId
*/
export async function completeMultipartUpload(
uploadId: string,
parts: { partNumber: number; eTag: string }[],
studyId: number,
assignmentId: number
) {
return axios.post(
`${process.env.NEXT_PUBLIC_API_SERVER_URL}/api/v1/videos/complete-upload`,
{ uploadId, parts, studyId, assignmentId },
getAuthenticationConfig()
);
}

Choose a reason for hiding this comment

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

코드 리뷰에 대해 의견을 말씀드리겠습니다.

  1. 환경 변수 검사: process.env.NEXT_PUBLIC_API_SERVER_URL이 설정되어 있지 않은 경우, API 호출 시 문제가 발생할 수 있습니다. 이러한 경우를 방지하기 위해 코드 상단에서 해당 환경 변수가 정의되어 있는지 확인하고, 그렇지 않은 경우 예외를 발생시키는 로직을 추가하는 것이 좋습니다.

  2. 에러 핸들링: 각 Axios 요청에 대해 에러 핸들링을 추가하는 것이 좋습니다. 요청이 실패할 경우 사용자에게 적절한 피드백을 줄 수 있도록 try-catch 블록을 사용할 수 있습니다.

  3. 타입 검사: uploadVideo 함수에서 File | Blob 타입을 받는 경우, 적절한 파일인지 확인하는 추가 검사를 통해 안정성을 높일 수 있습니다. 예를 들어, 파일 크기나 형식을 검사할 수 있습니다.

  4. 직접적인 API URL 사용: 현재 API URL을 직접 문자열로 사용하고 있습니다. API 버전이나 엔드포인트가 변경될 경우 여러 곳에서 수정을 해야 하므로, 상수로 정의하고 재사용하는 것이 좋습니다.

  5. 주석 및 문서화: 함수에 주석이 잘 작성되어 있지만, 매개변수의 타입에 대한 설명이 추가되면 더 좋을 것 같습니다. 특히 숫자 타입이 어떤 값을 의미하는지 간단한 설명을 추가하면 가독성이 향상됩니다.

  6. 반환 값: 현재 각 함수는 Axios 요청의 반환 값을 그대로 반환하고 있습니다. 이를 Promise 형태로 변환하거나, 응답 데이터를 가공하여 반환하는 것도 고려해볼 수 있습니다.

이 점들을 고려하여 개선하면, 코드의 안정성 및 가독성이 높아질 것입니다.

7 changes: 7 additions & 0 deletions types/study/video-upload-button.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { Round } from '@/types/study/study-detail';

export interface VideoUploadButtonProps {
studyType: string;
studyId: number;
round: Round;
}

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

  1. 타입 안정성: studyTypestring으로 정의했는데, 가능한 선택지를 제한할 수 있다면, enum이나 union type을 사용하는 것이 좋습니다. 예를 들어, studyType이 특정 값들 중 하나여야 한다면 더 안전하게 코드를 작성할 수 있습니다.

  2. 문서화: VideoUploadButtonProps 인터페이스의 각 속성에 대한 설명 주석을 추가하는 것이 좋습니다. 이는 코드 유지 보수성과 가독성을 높이는 데 도움이 됩니다.

  3. import 경로: 경로가 절대 경로(@/types/study/study-detail)로 설정되어 있는데, 이 경로가 유효한지 확인 필요합니다. 상대 경로를 사용하는 것이 더 명확할 수 있습니다.

  4. 추후 확장성: 만약 다른 속성이 추가될 가능성이 있다면, 현재의 구조에 맞춰 향후 확장성을 고려하여 미리 설계해두는 것이 좋습니다.

이외에도 현재 코드에는 큰 버그 리스크는 없어 보입니다. 추가적인 검토 사항이나 기능적 요구사항에 따라 더 깊이 있는 분석이 필요할 수 있습니다.