refactor: /presentations DTO 구현 및 API 연동 (#111)#113
Conversation
Summary of ChangesHello @AndyH0ng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 API 응답 처리 방식과 데이터 전송 객체(DTO) 구조를 전반적으로 개선하고, 프론트엔드 코드 전반의 필드명 일관성을 강화하는 리팩토링을 포함합니다. 주요 변경 사항으로는 ApiResponse 타입을 Discriminated Union으로 재정의하여 API 응답의 안정성을 높이고, 요청 DTO를 별도 디렉토리로 분리하여 코드의 모듈성과 유지보수성을 향상시켰습니다. 또한, id와 thumb 필드명을 각각 slideId와 imageUrl로 통일하여 데이터 모델의 명확성을 확보하고, 에러 처리 로직을 개선하여 사용자 경험을 향상시켰습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
이 PR은 API 명세 변경에 맞춰 대규모 리팩토링을 진행한 점이 인상적입니다. 특히 DTO를 도입하고 ApiResponse 타입을 Discriminated Union으로 개선하여 타입 안정성과 코드 유지보수성을 크게 향상시킨 점이 좋습니다.
리뷰 결과, 몇 가지 개선점을 발견했습니다.
- 여러 API 엔드포인트 함수에서 응답을 처리하는 로직이 중복되고 있습니다. 이를 헬퍼 함수로 추출하여 코드 중복을 줄이고 일관성을 높일 수 있습니다.
- MSW(Mock Service Worker) 핸들러 중 일부가 새로운 API 응답 형식(
ApiResponse)을 따르지 않아, 모킹 환경에서 런타임 에러를 유발할 수 있습니다. 특히GET및DELETE요청에 대한 핸들러 수정이 필요합니다.
자세한 내용은 각 파일에 남긴 개별 코멘트를 참고해주세요. 전반적으로 코드 품질을 크게 향상시키는 훌륭한 작업입니다.
|
@claude 코드 리뷰해줘 |
|
Claude finished @AndyH0ng's task in 4m 22s —— View job 코드 리뷰 완료 ✅대규모 리팩토링 작업으로 DTO 계층 도입, API 응답 타입 개선, 필드명 표준화 등 전반적인 코드 품질 향상이 이루어졌습니다. 전체적으로 좋은 개선이지만, 몇 가지 중요한 이슈와 개선 제안이 있습니다. ✅ 긍정적인 변경사항1. 타입 안정성 대폭 향상
2. DTO 계층 분리
3. 필드명 일관성
4. 에러 처리 구조 개선
🔴 Critical Issues (반드시 수정 필요)1. MSW 핸들러의 DELETE 응답이 API 인터페이스와 불일치위치: DELETE 엔드포인트들이 // handlers.ts (Line 198, 370)
return new HttpResponse(null, { status: 204 });
// presentations.ts (Line 96)
const response = await apiClient.delete<ApiResponse<null>>(`/presentations/${projectId}`);
if (response.data.resultType === 'FAILURE') { ... }204 응답은 body가 없어서 수정 방안: // handlers.ts에서
return HttpResponse.json(wrapResponse(null));이 문제는 다음 핸들러들에 영향을 미칩니다:
2. GET 슬라이드 상세 조회 핸들러가 ApiResponse 래퍼 없음위치: // Line 259-275: GET /presentations/slides/:slideId
return HttpResponse.json(slide); // ❌ Should be: wrapResponse(slide)API 엔드포인트 수정 방안: // Line 274를 다음과 같이 변경
return HttpResponse.json(wrapResponse(slide));단, 이 경우 3. 슬라이드 수정/생성 핸들러도 ApiResponse 래퍼 없음위치: // Line 319: PATCH /presentations/slides/:slideId
return HttpResponse.json(slides[slideIndex]); // ❌
// Line 346: POST /presentations/:projectId/slides
return HttpResponse.json(newSlide, { status: 201 }); // ❌이들도 API 함수가
|
| 구분 | 개수 | 상세 |
|---|---|---|
| Critical | 3 | MSW DELETE 응답 불일치, GET 슬라이드 래퍼 누락, POST/PATCH 래퍼 누락 |
| Major | 4 | API 응답 로직 중복, 타입 중복, CodeQL 경고 |
| Minor | 6 | Legacy 주석, deprecated 타입 사용, 타입 일관성 등 |
🎯 우선순위 제안
High Priority (배포 전 필수):
- MSW DELETE 핸들러 수정 (chore: 프로젝트 세팅 #1)
- MSW GET/POST/PATCH 슬라이드 핸들러 수정 (chore/프로젝트 세팅 #2, Closed #3)
unwrapApiResponse헬퍼 도입 (design: 디자인 시스템 구축 #4)
Medium Priority (다음 릴리스):
4. CodeQL 경고 수정 (#6)
5. 중복 타입 정리 (#5, #9)
6. 에러 처리 일관성 (#10)
Low Priority (점진적 개선):
7. Legacy 주석 정리 (#7)
8. Deprecated 타입 마이그레이션 (#8)
💡 전체 평가
긍정적 평가:
- 타입 시스템 개선으로 런타임 에러 방지 효과 ✅
- DTO 계층 분리로 관심사 분리 명확 ✅
- 50개 파일에 걸친 일관된 필드명 변경 ✅
- 에러 구조 표준화 ✅
개선 필요 사항:
- MSW 핸들러가 새 API 인터페이스를 따르지 않아 개발 환경 런타임 에러 가능성
⚠️ - API 응답 처리 로직 중복으로 유지보수성 저하
⚠️ - 정적 분석 도구 경고 존재
⚠️
전체적으로 잘 설계된 리팩토링이지만, MSW 핸들러 수정과 헬퍼 함수 도입이 완료되면 훨씬 견고해질 것으로 보입니다.
kimyw1018
left a comment
There was a problem hiding this comment.
네이ㅣ밍 수정response, request
- ScriptResponse → GetScriptResponseDto - ScriptVersion → GetScriptVersionHistoryResponseDto - UpdateScriptRequest → UpdateScriptRequestDto - RestoreScriptRequest → RestoreScriptRequestDto - types/api.ts에서 중복 타입 제거 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- SlideDetail → GetSlideResponseDto - SlideUpdateResponse → UpdateSlideResponseDto - UpdateSlideDto → UpdateSlideTitleRequestDto - deprecated UpdateSlideRequest 타입 별칭 제거 - types/slide.ts에서 중복 타입 제거 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- CommentResponse → CommentResponseDto - CommentListResponse → GetSlideCommentsResponseDto - CommentWithUser → CommentWithUserDto - ReplyListResponse → GetReplyListResponseDto - getReplies 에러 핸들링 패턴 통일 - types/comment.ts에서 API 응답 타입 제거 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- useSlides.ts: UpdateSlideRequest → UpdateSlideTitleRequestDto - dto/index.ts: CreateOpinionDto export 추가 - presentations.dto.ts: UpdateProjectDto 정의 및 export 추가 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…G/Web into refactor/setting-type-111
- Response DTO에서 ApiResponse 래퍼 중복 제거 - StartVideoResponse → StartVideoResponseDto (success 데이터만) - FinishVideoResponse → FinishVideoResponseDto (success 데이터만) - ChunkUploadResponse → ChunkUploadResponseDto (success 데이터만) - videos.ts endpoint에서 ApiResponse<T> 래핑 적용 - useVideoUpload.ts 타입 수정 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…G/Web into refactor/setting-type-111
|
|
@claude 코드리뷰 하셈 |
|
Claude finished @kimyw1018's task in 2m 58s —— View job 코드 리뷰 완료 ✅대규모 리팩토링으로 API 타입 시스템과 DTO 계층을 크게 개선한 작업입니다. 전반적으로 코드 품질 향상에 기여하는 좋은 변경이지만, 몇 가지 개선이 필요한 부분이 있습니다. 작업 목록
✅ 긍정적인 변경사항1. 타입 안정성 대폭 향상
2. DTO 계층 분리
3. 필드명 일관성 확보
4. 에러 처리 구조 개선
5. 비디오 API도 공통 래퍼 적용
🔴 Critical Issues (반드시 수정 필요)1. MSW DELETE 핸들러가 ApiResponse 형식 미준수위치: DELETE 엔드포인트가 // handlers.ts Line 198, 370
return new HttpResponse(null, { status: 204 });
// presentations.ts Line 96, slides.ts Line 104
const response = await apiClient.delete<ApiResponse<null>>(...);
if (response.data.resultType === 'FAILURE') { ... }문제: 204 응답은 body가 없어서 수정 방안: // handlers.ts에서
return HttpResponse.json(wrapResponse(null));영향받는 핸들러:
2. GET 슬라이드 상세 조회 핸들러가 ApiResponse 래퍼 누락위치: // Line 259-274: GET /presentations/slides/:slideId
const slide = slides.find((s) => s.id === slideId);
// ...
return HttpResponse.json(slide); // ❌ Should be: wrapResponse(slide)문제: 수정 방안: // Line 274
return HttpResponse.json(wrapResponse(slide));주의: 3. 슬라이드 수정/생성 핸들러도 ApiResponse 래퍼 누락위치: // Line 319: PATCH /presentations/slides/:slideId
return HttpResponse.json(slides[slideIndex]); // ❌
// Line 346: POST /presentations/:projectId/slides
return HttpResponse.json(newSlide, { status: 201 }); // ❌문제: API가 각각 수정 방안: 두 핸들러 모두 Fix PATCH/POST slide handlers →
|
| 구분 | 개수 | 우선순위 |
|---|---|---|
| Critical (배포 전 필수) | 3 | High |
| Major (다음 릴리스) | 4 | Medium |
| Minor (점진적 개선) | 5 | Low |
🎯 권장 수정 순서
배포 전 필수 (High Priority):
- ✅ MSW DELETE 핸들러 수정 (chore: 프로젝트 세팅 #1)
- ✅ MSW GET/POST/PATCH 슬라이드 핸들러 수정 (chore/프로젝트 세팅 #2, Closed #3)
다음 릴리스 (Medium Priority):
3. unwrapApiResponse 헬퍼 도입 (#4)
4. DTO 네이밍 일관성 개선 (#5)
5. CodeQL 경고 수정 (#6)
점진적 개선 (Low Priority):
6. Legacy 주석 정리 (#7)
7. Deprecated 타입 마이그레이션 (#8)
8. CreateSlideRequestDto 추가 (#9)
9. DELETE 함수 일관성 (#10)
10. analytics.dto.ts 주석 제거 (#11)
💡 전체 평가
✅ 강점:
- Discriminated Union으로 타입 안전성 대폭 향상
- DTO 계층 분리로 관심사 분리 명확
- 67개 파일에 걸친 일관된 필드명 마이그레이션
- 에러 구조 표준화 (
errorCode,reason) - 비디오 API도 공통
ApiResponse래퍼 적용
- MSW 핸들러가 새 API 인터페이스 미준수 → 개발 환경 런타임 에러 가능성
- API 응답 처리 로직 중복 → 유지보수성 저하
- DTO 네이밍 규칙 불일치
- 정적 분석 경고 존재
종합: 잘 설계된 리팩토링이지만, MSW 핸들러 수정이 최우선입니다. 이후 unwrapApiResponse 헬퍼 도입으로 코드 품질을 한층 더 높일 수 있습니다.
📌 관련 이슈
✨ 변경 내용
src/types/api.ts)ApiResponse를 Discriminated Union으로 변경ApiError구조 변경:code/message→errorCode/reason-ConversionStatus,ConversionStatusResponse타입 추가src/types/slide.ts)SlideListItem: API 목록 조회 응답 (slideId,imageUrl,slideNum,createdAt,updatedAt)SlideDetail: API 상세 조회 응답 (prevSlideId,nextSlideId추가)SlideUpdateResponse: API 수정 응답 타입Slide: 프론트엔드 스토어용 (임시)DTO 계층 분리: projects, slides, scripts, opinions, reactions 등 도메인별 요청/응답 DTO를 전용 파일로 분리하여 관리함.
슬라이드 모델 세분화: 목록용(SlideListItem), 상세용(SlideDetail), 수정 응답용(SlideUpdateResponse) 등으로 용도에 맞게 타입을 쪼개서 안정성 확보함.
ApiResponse<T>래퍼 + 타입 가드 적용presentations.ts- getConversionStatus() 신규 함수 추가client.ts- 에러 인터셉터에서 reason 필드 사용queryClient.ts-ApiFailureResponse타입으로 에러 처리useUpload.ts-error.reason필드 사용client.ts-response.data.error.reason구조 지원💡 참고 사항
스크린샷, 참고 링크, 추가 설명 등 (없으면 생략 가능)