fix: 관심과목 분석 상세 페이지 버그 수정#348
Conversation
🚀 Preview Deployment
|
Remove console log for response status in error handling.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
hyunwoo0081
left a comment
There was a problem hiding this comment.
위험
버그, 런타임 에러, 빌드 실패 가능성이 있는 항목. 반드시 이번 PR에서 수정.
- [packages/client/src/entities/subjectAggregate/model/useDetailWishes.ts:5] FSD 레이어 위반 (Entities -> Features 참조)
- 이유:
entities레이어에서 상위 레이어인features/wish/model/useWishesInfo를 import하고 있습니다. 이는 FSD 아키텍처의 계층 구조를 위반하며 순환 의존성 문제를 야기할 수 있습니다. - 수정 방향:
WishesInfo인터페이스를shared/model로 이동시키거나, 엔티티 훅에서는 필요한 데이터(subjectId,semesterCode)만 개별 인자로 받도록 수정하세요. (useRecommendWishes.ts,useDetailRegisters.ts등 동일 패턴 수정 필요)
- 이유:
중간
컨벤션 위반, 관심사 미분리, allcll-ui 미적용 등. 이번 PR에서 수정 권장.
-
[packages/client/src/entities/semester/model/useSemesterNameBySubjectId.ts:3-7] 학기 데이터 하드코딩
- 이유: 학기 범위(startId, endId)와 같은 마스터 데이터가 파일 내부에 리터럴로 존재합니다. 컨벤션 제6조에 따라 상수는 공통 파일에서 관리해야 합니다.
- 수정 방향: 해당 배열을
shared레이어의 상수 파일이나 별도의constants파일로 분리하여 관리하세요.
-
[packages/client/src/widgets/wishlist/WishesBarChart.tsx:34] allcll-ui 레이아웃 컴포넌트 미적용
- 이유: 레이아웃 구성을 위해
div와className을 직접 사용하고 있습니다. 컨벤션 제1조에 따라allcll-ui의Flex나Grid를 우선 사용해야 합니다. - 수정 방향:
<div className="mt-4">대신 상위 컴포넌트에서gap을 주거나,Flex컴포넌트 등을 활용하여 레이아웃을 구성하세요.
- 이유: 레이아웃 구성을 위해
낮음
네이밍 개선, 리팩토링 제안 등. 추후 작업으로 미뤄도 무방.
-
[packages/client/src/pages/wishlist/WishesDetail.tsx:31] 페이지 최대 너비 컨벤션 미준수
- 이유:
max-w-5xl을 사용 중이나, 컨벤션 제11조는max-w-7xl통일을 권장합니다. - 수정 방향: 디자인 의도가 명확한 경우가 아니라면
max-w-7xl로 수정을 검토해주세요.
- 이유:
-
[packages/client/src/entities/semester/model/useSemesterNameBySubjectId.ts:7] 미사용 주석 코드 잔존
- 이유: 컨벤션 제9조에 따라 코드 자체로 의도가 명확한 경우나 완료된 코드는 주석을 제거합니다.
- 수정 방향: 주석 처리된
SPRING_26관련 코드를 삭제하세요.
중간
낮음
|
|
중간
낮음
\isPastSemester\ 네이밍 변경과 \IInlineSkeletonProps\ 인터페이스 추출 등 많은 부분이 개선되었습니다! 남은 잔여 사항들도 마저 정리 부탁드립니다. |
INSANE-P
left a comment
There was a problem hiding this comment.
이전 학기 과목이 현재 학기 데이터에 없어서 상세가 안 뜨던 문제를 과목 아이디 기반으로 학기를 추적하는 방식 너무 좋은 것 같습니다. 해당 버그가 존재하는지도 모르고 있었는데 앞으로 더 관심을 가지도록 하겠습니다. 리뷰가 너무 늦어서 죄송합니다. 바쁘실텐데 수고많이하셨습니다.🙂
리뷰가 늦어져서 학기가 지나가 버렸네요. 학기쪽 부분 주석만 수정하고 바로 머지해도 될 것 같습니다.
Co-authored-by: 박찬빈 <chanbin0626@gmail.com>
|
꼼꼼하게 리뷰 주셔서 감사합니다. 다시 PR 확인하면서 리펙토링 하는 부분이 많이 보이네요. 추후에 개선 할 포인트들은 따로 올려두겠습니다 |



작업 내용
변경 사항 및 리뷰 포인트
추후 추가하면 좋은 issue