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

[Fix] 파일 관련 에러 해결 #335

Merged
merged 30 commits into from
Aug 8, 2024
Merged

Conversation

eonseok-jeon
Copy link
Member

@eonseok-jeon eonseok-jeon commented Aug 1, 2024

Related Issue : Closes #318


🧑‍🎤 Summary

  • 파일 에러 메세지가 textarea와 겹치는 에러 해결
  • 파트 왔다갔다 했을 시 발생되는 파일과 관련된 에러 해결
  • 파일 전송 중일 때 파일 추가 버튼 disabled 만들기
  • Info component 중복된 key 제거
  • 업로드 실패 시 에러 메세지 뜨도록 수정

🧑‍🎤 Screenshot

스크린샷 2024-08-01 오후 11 37 36

🧑‍🎤 Comment

남겨주신 코리 보면서 수정을 좀 했습니다!
감사합니다 🙆🏻‍♂️🙇🏻‍♂️

수정된 부분이 좀 많아서 대댓글 달기보다 아예 새로 pr을 다시 올리는 게 더 나을 거 같다고 판단했어요! (리뷰는 다 꼼꼼히 읽어봤습니다!!)
이해 안 되는 부분 있으면 다 물어봐 주세여!

👩🏻‍🎤 파트 변경 시 기존 파일 상태 유지

일단 첫 렌더링 시에 getValues를 통해 값이 있으면 fileName만 설정하고 넘어가도록 했어요
fileName은 state라 파트 변경 시 초기화 되기 때문에 새로 설정을 해줘야 했어요
getValues가 없다면 default file 있는지 체크한 뒤 setValue 해주었습니다
임시저장이나 제출 시 해당 value를 제출해야 하니까요
(이거 없으면 빈 값 설정됨)

if (fileValue) {
  setFileName(fileValue.fileName);
  return;
}

if (defaultFileId && defaultFileUrl && defaultFileName) {
  setValue(`file${defaultFileId}`, {
    file: defaultFileUrl,
    fileName: defaultFileName,
    recruitingQuestionId: defaultFileId,
  });
}

이때 파일을 제거해서 getValues에 값이 없는 건지
애초에 파일을 업로드 하지 않아서 없는 건지 구분해줘야 했어요
만약 1번이라면 파트 변경 시 파일이 없는 상태로 유지가 되어야 하거든요

이를 위해 state를 사용하는 건 불가능 했어요 (초기화 되니까요!)
따라서 react-hook-form value에 저장시켜줬습니다
파일 제거 시 setValue('file${id}Deleted', true); 로 설정하여
이를 가지고 판단하도록 했어요

if (isFileDeleted) return;

파일 업로드 하면 file${id}Deleted를 false로 설정되게 했습니다!
(참고로 임시저장이랑 제출하기 할 때는 file${id}Deleted value를 api 전송에 넣지 않습니다!)



👩🏻‍🎤 uploadPercent 상태 -1로 통일

기존에는 -1, -2, 2개의 예외 케이스를 두어 관리했었는데
가독성도 떨어지고 헷갈리더라고요

-2를 둔 이유가 defaultFile 있을 때를 구분해 주기 위해서였는데요
이렇게 하지 않고 if 문의 조건문을 더 꼼꼼하게 설정해 구분해 줬어요

const getFileNameClass = () => {
  return (!defaultFileName && fileName === '') || (defaultFileName && isFileDeleted) || fileName === 'delete-file'
    ? 'default'
    : 'selected';
};

const getDisplayText = () => {
  if (uploadPercent === -1 && fileName === '' && defaultFileName && !isFileDeleted) return defaultFileName;
  else if (uploadPercent === -1 && (fileName === '' || fileName === 'delete-file')) return '50mb 이하 | pdf';
  else if (isFileUploading) return `업로드 중... ${uploadPercent}/100% 완료`;
  else if (isFileSending) return '파일을 전송하고 있어요... 잠시만 기다려주세요...';
  else return fileName;
};

덕분에 좀 더 코드가 깔끔해 질 수 있었어요

이 덕분에 리뷰에 남겨주신 에러 사항들도 다 수정할 수 있었습니다 :)



👩🏻‍🎤 업로드 실패 시 에러 메세지 뜨도록 수정

기존에는 error 발생 시 throw error; 되도록 구현 되어 있었어요
이렇게 하면 사용자 입장에서 갑자기 500 페이지로 이동하게 되어 불편할 수 있을 거라 생각이 들었어요
그래서 error 메세지를 띄워주는 걸로 수정했씁니다

(error) => {
  setError(`file${id}`, { type: 'unknownError', message: 
  VALIDATION_CHECK.fileInput.errorTextUnknownError });
},
스크린샷 2024-08-07 오후 9 06 44



👩🏻‍🎤 테스트 완료

임시저장 및 제출 잘 되는지 + placeholder 맞게 뜨는지 + x 버튼 기능 잘 동작하는지

  • 처음 파일 업로드 시
  • 파일 삭제 시
  • 파일 업로드 하고 하나 더 업로드 시
  • defaultFile 있을 때
  • defaultFile 제거 시
  • defaultFile 위에 새로운 파일 겹쳐 올릴 때
  • defaultFile 위에 새로운 파일 겹쳐 올린 후 파일 삭제 시
  • 파트 왔다갔다 할 시



👩🏻‍🎤 추가 코멘트

해당 문항의 value가 파일 선택인데, 첨부된 파일이 없는지? => 를 체크하여
맞지 않을 경우 alert 띄우는 등으로 처리할 수 있지 않을까 생각이 드네요

이게 조금 애매한게 파일 선택 자체가 필수가 아닐 수도 있어요
리서치 같은 경우도 파일 선택 질문이 있지만 파일과 답변을 제출 안 해도 되거든요!
그래서 일단 그냥 냅뒀습니다,,
이제는 파트 변경 했다고 파일 날아가는 것도 아니고
제출 안 했다면 본인 잘못이라 본인이 체크를 잘 해야하지 않나 싶네요,,,


이전 Comment

너무 복잡했어요
이건 각 잡고 싹 리팩토링 가야할 듯 합니다 언젠간,,

IsError state 제거

react hook form의 error가 아닌 별도의 state로 에러 관리 중이었더라고요
그럴 필요가 없어서 제거 해줬습니다


file state와 fileName 변수 합치기

file state의 역할이 fileName을 만들어주는 것 밖에는 없더라고요
그래서 합쳐줬습니다

const [fileName, setFileName] = useState('');

파일 에러 메세지가 textarea와 겹치는 에러 해결

고정된 높이가 있어서 그랬습니다
height을 제거함으로써 해결해주었어요


임시 저장된 파일 있을 때 x 버튼으로 파일 삭제해도 기존 값 나타나는 에러 해결

x 버튼을 눌러 파일을 제거하면 setFileName('delete-file')로 만듭니다
이후 fileName === 'delete-file' 이면 placeholder를 띄게 하였습니다

{uploadPercent === -1 && defaultFileName && (
  <span className={fileNameVar[fileName === 'delete-file' ? 'default' : 'selected']}>
    {fileName === 'delete-file' ? '50mb 이하 | pdf' : defaultFileName}
  </span>
)}

uploadPercent -1과 -2 차이

-1

파일 업로드 시 퍼센트가 0부터 시작을 해요
근데 이러면 파일이 0% 업로드 되었는지 파일 업로드가 안 되어서 0인 상태인지 구분이 안돼서
파일 업로드 안 하면 -1을 띄게 했어요

-2

임시저장된 default file과 새로 업로드한 파일 사이에
삭제 및 렌더링 관련해서 로직의 차이가 있어요
새로 업로드된 파일이 기존 default file을 덮어써야 하기도 하고요
이를 구분해주기 위해
새 파일을 추가하면 -2로 설정되도록 했습니다


파트 왔다갔다 하면 파일 제거 되도록 수정

디자인 파트에서 파일 업로드 후
임시저장 하지 않고
웹 파트 갔다가 다시 디자인 파트로 돌아왔을 때
기존 업로드 한 파일 없어지도록 만들었습니다

사실 살리고 싶었는데 그러질 못했습니다
이유는 아래와 같은데요

[알고 있어야 할 내용]
파일이 임시저장 되어 있는 경우 defaultFile이 생기게 됩니다

[트러블 슈팅]
파트를 변경했다 다시 돌아오는 순간 모든 state들이 초기화가 되어,
해당 파일을 삭제했는지 아닌지 알 수가 없습니다

getValues('file${id}') 값이 있는지 없는지로 판단하려고 하였으나
defaultFile이 있는 순간 useEffect로 setValue('file${id}') 하기 때문에
자동으로 value가 채워지게 됩니다

useEffect로 setValue 하는 이유는,
파일 임시저장 이후 새로고침하면 react hook form은 해당 value가 있는지 모르게 됩니다
이 상태에서 임시저장이나 최종 제출을 하게 되면 빈 값이 제출되기에
이를 막고자 defaultFile을 setValue를 하는 것이지요

따라서 방금 파일을 지웠는지 getValues로 판단을 못하고 state로도 판단을 못해
파트 변경 시 파일을 지우는 쪽을 택하게 되었습니다

그래서 작동 방식이 다음과 같아요

  1. 첫 렌더링 시 useEffect를 통해 임시저장 값 setValue (임시저장 없으면 빈 input 렌더링)
  2. 파트 변경 시 clean up 함수를 통해 저장된 value clear
  3. 다시 돌아왔을 때, useEffect를 통해 임시저장된 값 setValue
useEffect(() => {
  if (defaultFileId && defaultFileUrl && defaultFileName) {
    setValue(`file${defaultFileId}`, {
      file: defaultFileUrl,
      fileName: defaultFileName,
      recruitingQuestionId: defaultFileId,
    });
  }

  if (getValues(`file${id}`) && getValues(`${section}${id}`) === '') setValue(`${section}${id}`, '파일 제출');
  // 임시저장된 값 있어서 '파일 제출' 이란 텍스트가 채워져 있었는데 파일 삭제 시 '파일 제출'도 같이 없어지게 구현되어 있음
  // 이후 임시저장하지 않고 파트 변경 후 다시 돌아오면 파일은 기존 파일 그대로 살아나지만
  // '파일 제출' 이란 텍스트는 돌아오지 않음
  // 다시 살려주기 위한 코드

  return () => {
    setValue(`file${id}`, undefined);
    getValues(`${section}${id}`) === '파일 제출' && setValue(`${section}${id}`, '');
  };
  // 파일을 업로드 후 임시저장 하지 않고 파트 변경 시
  // 기존 업로드 한 파일 제거 및 추가된 '파일 제출' 이란 텍스트도 제거
}, [section, id, defaultFileId, defaultFileUrl, defaultFileName, getValues, setValue]);

이때 두 가지 문제점이 생겨요

  1. 다른 문항 텍스트들은 파트 왔다리갔다리 해도 기존 값을 그대로 유지합니다. 파일만 없어져요
  2. 파트 왔다리갔다리 하면 파일이 제거되기 때문에
    디자인 파트에서 파일 업로드 후 임시저장 하고
    웹 파트가서 글 작성 후 임시저장하면 기존 디자인 파트에서 업로드 한 파일이 제거돼요
    (기존 답변은 그대로 남아있어요 파일만 제거됩니다)
    왜냐면 파트 파트 변경 시 clean up 함수를 통해 파일이 제거된 상태거든요

여러 경우의 수 확인해서 다른 에러는 없는 것 확인했습니다

일단 기존 상태보단 에러들이 많이 개선되었고, 남아있는 에러의 심각도 또한 기존 보다 나아진 거 같아
위와 같이 구현 하는 걸로 하였습니다
(기존: 파일이 존재하는데 존재하지 않은 걸로 표시, 현재: 파일이 실제로 존재하지 않게 됨)
(개선된 점: 에러 메세지 위치, placeholder 관련 에러들 해결)

대신 위와 같은 방법으로 가려면 pm, pd와 상의해서 파트 변경시 파일이 날아갈 수 있다는 문구를 추가시켜야 할 거 같긴 해요

이들을 개선시킬 좋은 방법이 있을까요?? 끙,,, 😓

@eonseok-jeon eonseok-jeon linked an issue Aug 1, 2024 that may be closed by this pull request
Copy link

height bot commented Aug 1, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@lydiacho lydiacho left a comment

Choose a reason for hiding this comment

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

코드리뷰를 끝까지 하지 않고 중단했는데요!
이해가 잘 안되는 코드들을 파악 및 수정해보려고 했으나 유기적으로 얽힌 변수들이 너무 많아서 대공사가 될 것 같아.. 언석님 말대로 리크루팅 기간 끝나고 안전한 시기에 추가 리팩토링하는게 좋다는 생각이 들기도 하네요 ㅠㅡㅠ
뭔가 제가 중간에 작업 참여하지 않았던 간극 때문에 크게 이해하지 못하고 놓치고 있는 부분이 있나 싶기도 하고요 (-2 값의 의미 같은 것 ,,)

우선 delete-file 보이는 이슈만 먼저 해결해주시고 다른 코멘트는 선택적으로 봐주셔도 됩니다


아 그리고 파트 변경시 파일이 날아가는 문제에 대해선,
필수 항목에 대해서는 어차피 제출할 때 focus 해주니까 문제 안될 것 같고,
다만 디자인에서는 파일 업로드가 선택 질문이더라고요?
이건 좀 크리티컬한 문제가 될 수 있을 것 같아서 (파일만 날라갔는데 그냥 제출할 수 있으니까)

저희가 value파일 선택을 넣어주고 있잖아요,
text value들은 문제가 되지 않는데, input에 걸리는 파일이 문제가 되는거니까,
파트 이동 시에 파일 선택 value는 날라가지 않게, 다른 일반 텍스트 값들 처럼 유지 시키고
만약 사용자가 제출 버튼을 클릭 했을 때
해당 문항의 value가 파일 선택인데, 첨부된 파일이 없는지? => 를 체크하여
맞지 않을 경우 alert 띄우는 등으로 처리할 수 있지 않을까 생각이 드네요

src/views/ApplyPage/components/FileInput/index.tsx Outdated Show resolved Hide resolved
src/views/ApplyPage/components/FileInput/index.tsx Outdated Show resolved Hide resolved
@eonseok-jeon eonseok-jeon marked this pull request as draft August 2, 2024 04:38
@eonseok-jeon eonseok-jeon marked this pull request as ready for review August 7, 2024 12:07
@eonseok-jeon eonseok-jeon requested review from lydiacho and removed request for lydiacho August 8, 2024 04:38
Copy link
Member

@lydiacho lydiacho left a comment

Choose a reason for hiding this comment

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

PR꼼꼼히 이해했고,

이친구도 DevTool 켜서 작동하는거 테스트해봤는데 모두 다 잘되네요!!
너무 관리해줘야할 데이터가 많아 복잡한데 ㅠㅠ 고생 너무 많으셨습니다!! 🚀🚀🚀🚀

@eonseok-jeon eonseok-jeon merged commit 81d4312 into develop Aug 8, 2024
@eonseok-jeon eonseok-jeon deleted the fix/#318_after-draft branch August 8, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 파일 관련 에러 해결
2 participants