sihyun10 / FE_PS_Final_Refactor

몰리턴 기업 연계 프로젝트 코드리뷰 및 리팩토링
0 stars 0 forks source link

업로드 모듈 코드리뷰 #17

Closed 05Castle closed 1 year ago

05Castle commented 1 year ago

PdfUploadModule 코드리뷰 및 분석하여 작동 방식을 이해하고, 개선할 사항을 고려하여 리팩토링 시도해보기.

분석한 내용, 알게 된 내용, 그리고 리팩토링 시도한 부분 코멘트로 남기기.

sihyun10 commented 1 year ago

'PdfUploadModule' 파일 코드의 작동방식에 대해 설명하겠습니다.

사용자가 파일을 선택하면 'onFileChange' 함수가 호출됩니다. 이 함수를 호출함으로써 pdf 파일이 맞는지 틀린지와 파일크기 제한을 초과하지 않는지 확인을 해줍니다. 그렇지 않은 경우 ErrorModal을 표시하고, 파일 상태를 초기화 해줍니다.

  // 인풋태그에 파일이 변경되었을때 체크
  const onFileChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    const file = event.target.files ? event.target.files[0] : null;
    // 100m 초과하는지 , pdf파일이 맞는지 아니라면 리턴 둘중하나라도 false 값일경우
    if (!isPdfFileType(file) || !isFileSizeValid(file)) {
      ErrorModal(!isPdfFileType(file), !isFileSizeValid(file));
      resetFileState();
      event.target.value = '';
const ErrorModal = (PdfType: boolean, PdfSize: boolean) => {
    if (PdfType) {
      setModalMessage('파일이 pdf 파일이 아닙니다.');
      setErorrModalOpen(true);
      return;
    }
    if (PdfSize) {
      setModalMessage(`파일 사이즈가 ${MAX_FILE_SIZE}MB 보다 큽니다.`);
      setErorrModalOpen(true);
      return;
    }
    setModalMessage('파일이 잘못되었거나, 알 수 없는 오류가 발생했습니다.');
    setErorrModalOpen(true);
  };
sihyun10 commented 1 year ago

onFileUpload함수는 파일을 서버에 업로드 하는 코드입니다. 업로드 중인 상태를 표시하고, 업로드 진행상태를 표시해줍니다. 업로드가 완료된다면 서버의 응답을 처리해주고, 필요한 경우 데이터를 저장해줍니다. (이 업로드 과정에서 발생한 에러들도 처리해주고있다)

    try {
      const response = await uploadFileToServer(formData);

      if (response) {
        const customData = await ApartData(response.data.summary.newAddress);
        if (customData) {
          const lastId = addResponseItem(fileName, { ...response.data, customData });
          setDataStoreId(lastId);
        } else {
          const lastId = addResponseItem(fileName, { ...response.data });
          setDataStoreId(lastId);
        }
      }
      setDownloadProgress(100);
    } catch (error) {
      setDownloadProgress(0);
      console.error('파일 업로드 실패:', error);
      UploadErrorModal();
    } finally {
      // 다시 업로드 가능하도록 버튼잠금 해제 및 모달 끄기
      setIsUploading(false);
    }

uploadFileToServer함수는 파일을 서버 전송하는 코드입니다.

      const response = await instance.post('api/pdfupload', formData, {
        cancelToken: source.token,
        onUploadProgress: (progressEvent: AxiosProgressEvent) => {
          if (progressEvent.loaded && progressEvent.total) {
            const progress = Math.round((progressEvent.loaded * 100) / progressEvent.total);
            setUploadProgress(progress);
          }
        },
        onDownloadProgress: (progressEvent?: AxiosProgressEvent) => {
          if (progressEvent?.loaded && progressEvent?.total) {
            const progress = Math.round((progressEvent.loaded * 100) / progressEvent.total);
            setDownloadProgress(progress);
          }
        },
      });

      return response.data;

위의 코드를 보며 알수있는게 onUploadProgress를 사용해, 업로드 진행 상태를 파악해 Math.round((progressEvent.loaded * 100) / progressEvent.total); 이렇게 진행을 계산하여 업로드 진행 상태를 업데이트합니다. onDownloadProgress 또한 마찬가지이며, 다운로드 진행 상태를 업데이트 해주고 있습니다. 그 다음 서버로부터 받은 응답인 response 데이터를 반환해줍니다. 만약, 업로드나 통신실패가 된다면 그것에 대한 처리도 코드에서 작성하였습니다.

sihyun10 commented 1 year ago

handleCancelUpload함수는 업로드를 cancel하고 상태를 초기화시켜줍니다.

handledDeletePDFfile함수는 파일 상태를 초기화합니다.

const handledDeletePDFfile = (): void => {
    resetFileState();
  };

ViewChange함수는 pdfsummary(등기부요약) 페이지로 이동합니다.

const ViewChange = () => {
    navigate(`/review/${dataStoreId}/pdfsummary`);
  };
sihyun10 commented 1 year ago

개선하면 좋을 것 같은 부분 (1)

현재 ErrorModal과 UploadErrorModal 함수에서 아래와 같이 메시지를 작성하고 모달을 여는 코드가 중복되고 있다. 따라서 하나의 함수로 만들어 중복을 제거한다.

setModalMessage('~~~');
setErorrModalOpen(true);
sihyun10 commented 1 year ago

개선하면 좋을 것 같은 부분 (2) _ 타입선언

타입선언이 안되어있는 부분이 있어, 타입 선언을 추가하였다.

05Castle commented 1 year ago

모달이 차지하는 영역이 너무 많다. 모달을 컴포넌트 분리하는 것이 좋을 것 같다

05Castle commented 1 year ago

삭제하면 좋을 것 같은 부분들

handledDeletePDFfile 함수는 그저 resetFileState실행할 뿐이다. 굳이 필요할 것 같지 않다.

파일선택의 input 태그와 업로드 버튼의 disabled 속성은 필요가 없다. disabled에 boolean 타입인 isUploading 상태를 전달해서 속성을 조절하는데, isUploading가 true가 될때(즉 disabled 될때)가 업로드 버튼을 눌렀을 때 뿐이다. 그런데 이 때는 이미 모달창이 열리기 때문에 파일 선택이나 X 버튼, 업로드 버튼이랑 상호작용할 일이 없음. 모달을 닫으면 isUploading 상태는 어차피 false로 돌아가기 때문에 의미가 없다.

DropZone 스타일 컴포넌트에 굳이 fileName을 prop으로 받을 필요가 없음

05Castle commented 1 year ago

현재 업로드 모듈은 모든 파일을 선택할 수 있게 되어 있는데, inpurt 태그에 accept 속성으로 .pdf 확장자 파일만 허용되도록 하는 것이 좋을 것 같다.

05Castle commented 1 year ago

드래그 앤 드롭 이하 부분도 이미 파일이 있으니 그쪽으로 옮겨서 분리하는 것을 시도해봤다. 이렇게 적용하자 작동에 문제가 생겼음.

  1. 드래그 드롭할 때 파일 선택 버튼이 사라지지 않음. → 아래 코드에서 더 이상 children 요소가 아니게 되어서 발생하는 이슈.

    {isDragging ? '업로드할 파일을 여기에 끌어다 주세요' : children}

    직접 children 부분에 기입하니 문제가 해결되었다. 하지만 이로인해 또 들여쓰기 depth가 깊어지는 문제가 발생. → 아예 다른 컴포넌트로 분리를 할까? 해당 구성 요소는 파일을 선택하는 input에 관련된 요소들이라 드래그 앤 드롭 컴포넌트에 속해있는게 어울리지 않아 보이기도 하고.

  2. X 버튼이 작동하지 않음. → handledDeletePDFfile이 아닌 resetFileState을 prop으로 가져오니 해결되었음. 해결은 되었으나 정확한 원인을 모르니, 원인도 파악해두기.