-
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: #168 일정 파일 다운로드 기능 구현 #186
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
src/types/SearchCallbackType.tsx (2)
Line range hint
9-27
: 다른 타입 정의들에 대한 긍정적인 영향
FetchCallback
타입의 변경으로AllSearchCallback
,TeamSearchCallback
,ProjectSearchCallback
, 그리고SearchCallback
타입들이 더 유연해졌습니다. 이는 향후 기능 확장에 도움이 될 것입니다.이 변경사항에 대한 설명을 파일 상단에 주석으로 추가하는 것이 좋을 것 같습니다. 이렇게 하면 다른 개발자들이 이 변경의 의도를 쉽게 이해할 수 있을 것입니다.
Line range hint
1-27
: 전반적인 변경 사항 요약이 파일의 변경사항은 파일 다운로드 기능 구현이라는 PR의 목적에 잘 부합합니다.
FetchCallback
타입의 수정으로 비동기 함수가 값을 반환할 수 있게 되었고, 이는 파일 다운로드 API와의 상호작용에 필요한 유연성을 제공합니다. 또한 이 변경으로 인해 다른 관련 타입들도 더 유연해져 향후 기능 확장에 도움이 될 것입니다.이러한 타입 변경은 전체 애플리케이션의 타입 안정성을 향상시키며, 특히 API 응답 처리와 관련된 부분에서 더 강력한 타입 체크를 가능하게 합니다. 앞으로 이와 같은 접근 방식을 다른 비동기 작업을 다루는 타입에도 적용하는 것을 고려해보시기 바랍니다.
src/services/taskService.ts (1)
193-215
: 새로운 downloadTaskFile 함수가 잘 구현되었습니다.새로 추가된
downloadTaskFile
함수는 전반적으로 잘 구현되었습니다. API 엔드포인트 구조와 파라미터 사용이 적절하며, 파일 다운로드를 위해responseType: 'blob'
을 사용한 것도 좋습니다.다만, 한 가지 개선 사항을 제안드립니다:
파일 이름에 공백이나 특수 문자가 포함될 경우를 대비하여
fileName
을 URL 인코딩하는 것이 좋겠습니다. 다음과 같이 수정해 보세요:- return authAxios.get<Blob>(`/file/project/${projectId}/${taskId}/${fileName}`, { + return authAxios.get<Blob>(`/file/project/${projectId}/${taskId}/${encodeURIComponent(fileName)}`, {이렇게 하면 파일 이름에 특수 문자가 포함되어 있어도 안전하게 요청을 보낼 수 있습니다.
src/mocks/mockData.ts (1)
39-39
: 파일 정보 구조 개선에 대한 종합 의견FileInfo 타입과 FILE_DUMMY 상수의 변경으로 파일 정보 구조가 개선되었습니다. 이는 애플리케이션의 파일 처리 로직을 단순화하고 일관성을 높일 것으로 예상됩니다.
이 변경사항과 관련하여 다음 사항들을 고려해 주세요:
- 관련 문서(API 문서, 개발 가이드 등)를 업데이트하여 새로운 파일 정보 구조를 반영해 주세요.
- 파일 업로드 및 다운로드 기능을 구현할 때, 이 새로운 구조를 활용하는 방법에 대한 가이드라인을 팀원들과 공유해 주세요.
- 실제 서버 측 구현에서도 이 구조가 일관되게 사용되는지 확인해 주세요.
다음 스크립트로 프로젝트의 README 파일에서 파일 관련 문서를 확인할 수 있습니다:
#!/bin/bash # README 파일에서 파일 관련 문서 확인 rg --type markdown "file|upload|download" README.mdAlso applies to: 825-825, 831-831, 837-837
src/components/modal/task/DetailModalTask.tsx (1)
102-110
: 파일 다운로드 버튼의 접근성 및 구조 개선현재
<li>
요소에aria-label="file download"
속성을 사용하고 있는데, 이는 실제 클릭 이벤트를 가지는<button>
요소에 적용하는 것이 더 적절합니다. 또한,cursor-pointer
클래스는 클릭 가능한 요소인<button>
에 적용하는 것이 좋습니다.- <li - key={fileId} - className="flex cursor-pointer items-center gap-5 rounded-md bg-button px-4 py-2 hover:bg-sub" - aria-label="file download" - > - <button type="button" onClick={() => handleDownloadClick(fileName, uploadName)}> + <li key={fileId}> + <button + type="button" + onClick={() => handleDownloadClick(fileName, uploadName)} + className="flex items-center gap-5 rounded-md bg-button px-4 py-2 hover:bg-sub cursor-pointer" + aria-label="file download" + > {fileName} <LuDownload /> - </button> - </li> + </button> + </li>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/components/modal/task/DetailModalTask.tsx (4 hunks)
- src/hooks/useAxios.ts (1 hunks)
- src/mocks/mockData.ts (2 hunks)
- src/mocks/services/taskServiceHandler.ts (2 hunks)
- src/services/taskService.ts (1 hunks)
- src/types/SearchCallbackType.tsx (1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/mocks/services/taskServiceHandler.ts (1)
Learnt from: Seok93 PR: GU-99/grow-up-fe#155 File: src/mocks/services/taskServiceHandler.ts:107-107 Timestamp: 2024-09-24T07:02:17.215Z Learning: 최근 파일 타입 변경 사항에도 불구하고 `taskServiceHandler.ts`의 일정 파일 목록 검색 API는 업데이트가 필요하지 않습니다.
🔇 Additional comments (9)
src/types/SearchCallbackType.tsx (1)
5-7
:FetchCallback
타입 변경이 적절해 보입니다.이 변경은
FetchCallback
타입을 더 유연하게 만들어 데이터를 반환하는 함수도 처리할 수 있게 합니다. 이는 파일 다운로드 기능 구현에 필요한 변경으로 보이며, 기존의 void를 반환하는 함수들과의 호환성도 유지합니다.src/hooks/useAxios.ts (2)
45-45
: 응답 객체 반환 추가: 기능 향상이 변경사항은
fetchData
함수의 기능을 개선합니다. 전체 Axios 응답 객체를 반환함으로써, 호출자가 상태 코드, 헤더 등 응답의 모든 측면에 접근할 수 있게 됩니다. 이는 특히 파일 다운로드 기능 구현에 유용할 것입니다.이 변경사항은 기존 기능을 해치지 않으면서 훅의 유용성을 높이므로 승인됩니다.
Line range hint
1-56
: 전반적인 변경 사항 요약이 변경사항은
useAxios
훅의 기능을 효과적으로 개선합니다:
- 파일 다운로드 API 구현에 필요한 전체 응답 객체에 접근할 수 있게 되었습니다.
- 기존 기능을 유지하면서 훅의 유연성과 재사용성이 향상되었습니다.
- PR의 목표인 파일 다운로드 기능 구현을 위한 중요한 단계입니다.
이 변경사항은 잘 구현되었으며, PR의 목표를 달성하는 데 기여할 것으로 보입니다.
src/services/taskService.ts (2)
Line range hint
1-215
: 전체적인 파일 구조와 일관성이 잘 유지되었습니다.새로 추가된
downloadTaskFile
함수는 기존 파일의 구조와 스타일을 잘 따르고 있습니다. 다음과 같은 점에서 일관성이 유지되었습니다:
- 함수 명명 규칙
- 파라미터 타입 사용
- JSDoc 주석 스타일
authAxios
인스턴스 사용- 다른 파일 관련 함수들과 함께 그룹화
이러한 일관성은 코드의 가독성과 유지보수성을 높여줍니다. 좋은 작업입니다!
193-215
: PR 목표에 부합하는 구현입니다.이
downloadTaskFile
함수의 구현은 PR의 주요 목표인 "일정 파일 다운로드 기능 구현"에 정확히 부합합니다. 다음과 같은 점에서 요구사항을 잘 충족하고 있습니다:
- axios를 사용한 파일 다운로드 API 기능 구현
- 프로젝트 ID, 태스크 ID, 파일 이름을 사용하여 특정 파일 다운로드 가능
- 바이너리 응답 처리를 위한
responseType: 'blob'
설정이 구현으로 사용자들이 일정 상세 모달에서 직접 파일을 다운로드할 수 있는 기능의 백엔드 부분이 완성되었습니다. 좋은 작업입니다!
src/mocks/mockData.ts (2)
825-825
: FILE_DUMMY 상수 업데이트 승인FILE_DUMMY 상수가 FileInfo 타입의 변경사항을 올바르게 반영하고 있습니다. 'uploadName'이 파일 이름과 확장자를 모두 포함하도록 변경된 것은 적절합니다.
파일 이름 규칙이 일관성 있게 적용되었는지 확인해 주세요. 특히 'FILE_UUID_' 접두사의 사용이 의도적인지, 그리고 이 명명 규칙이 실제 프로덕션 환경에서의 파일 업로드 로직과 일치하는지 검증이 필요합니다. 다음 스크립트로 파일 이름 패턴을 확인할 수 있습니다:
#!/bin/bash # FILE_DUMMY에서 사용된 파일 이름 패턴 확인 rg --type typescript "uploadName: '.*'" src/mocks/mockData.tsAlso applies to: 831-831, 837-837
39-39
: FileInfo 타입 변경 승인FileInfo 타입의 구조가 개선되었습니다. 'fileName'과 'extension' 속성을 제거하고 'uploadName'으로 통합한 것은 좋은 변경사항입니다.
이 변경으로 인해 영향을 받을 수 있는 다른 부분의 코드를 확인해 주세요. 다음 스크립트를 실행하여 'fileName'과 'extension'을 사용하는 다른 파일들을 찾아볼 수 있습니다:
src/components/modal/task/DetailModalTask.tsx (2)
48-49
: Blob 생성 시 응답 데이터 타입 확인 필요
response.data
를 Blob으로 생성하기 전에, 응답 데이터가 올바른 파일 데이터인지 확인이 필요합니다.downloadTaskFile
API 호출 시 Axios의responseType
이'blob'
으로 설정되지 않으면 파일 다운로드에 문제가 발생할 수 있습니다.downloadTaskFile
함수에서responseType: 'blob'
설정을 확인해주세요.// downloadTaskFile 함수 내부 또는 Axios 설정 확인 필요 axios.get(url, { + responseType: 'blob', });
9-11
: 필요한 모듈들이 올바르게 임포트되었습니다파일 다운로드 기능 구현을 위해 필요한 모듈들이 정확하게 임포트되었습니다.
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.
이번 작업해 주신 내용은 앞으로도 파일 쪽 코드 짤 때 참고가 많이 될 것 같아요! 파일 업로드 다운로드 구조도 공부해 봐야겠어요.
파일 작업이 이것저것 신경 써야 할 부분이 많은 작업이었는데 마지막 다운로드까지 고생 많으셨습니다!!🥳🎉
@Yoonyesol 리뷰 감사합니다! 저도 파일 처리 관련해서 조금 더 공부해볼 생각입니다 ㅎㅎ |
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.
일정 파일 다운로드 기능 구현 고생하셨습니다~
@ice-bear98 리뷰 감사합니다! |
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
Other information
참고 자료
View