Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
Walkthrough유효하지 않은 timetable frame ID(예: 0)를 차단하는 타입가드 Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as 브라우저 (Client)
participant NextServer as Next.js 서버 (getServerSideProps / SSR)
participant Queries as Timetable Queries (timetableQueries)
participant Service as Timetable 서비스 / DB
Browser->>NextServer: /timetable 요청 (query 포함 가능)
NextServer->>NextServer: parse frameId, call isValidTimetableFrameId
alt valid frameId
NextServer->>Queries: 요청 (authorization & frameId)
Queries->>Service: getTimetableLectureInfo(frameId)
Service-->>Queries: lecture data
Queries-->>NextServer: lecture data
else invalid or missing
NextServer-->>NextServer: set validatedFrameId = null
NextServer-->>Browser: render page with placeholder state
end
Note right of Browser: 클라이언트에서도 router.query를 검증하여<br>Timetable 또는 TimetableGridPlaceholder 렌더링 결정
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/TimetablePage/components/MainTimetable/index.tsx (1)
24-31:timetableFrameIdprop에 대한 방어적 검증 고려.이 컴포넌트는
timetableFrameId를 prop으로 받아useMyLectures(timetableFrameId)(line 31)에 직접 전달합니다. 부모 컴포넌트(src/pages/timetable/index.tsx)에서resolvedCurrentFrameIndex로 검증된 값을 전달하지만, 만약0이나 유효하지 않은 값이 전달될 경우useTotalGrades등에서 문제가 발생할 수 있습니다.현재 부모에서
mainFrameId의 기본값이0이므로, frame이 전혀 없는 경우 이 컴포넌트에0이 전달될 수 있습니다. 이 경우에 대한 방어 로직이나 early return을 고려해 보세요.function MainTimetable({ timetableFrameId }: { timetableFrameId: number }) { + if (!isValidTimetableFrameId(timetableFrameId)) { + return <div className={styles['page__timetable-wrap']}>시간표를 선택해주세요.</div>; + } + const [isModalOpen, openModal, closeModal] = useBooleanState(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TimetablePage/components/MainTimetable/index.tsx` around lines 24 - 31, MainTimetable receives timetableFrameId and passes it directly to useMyLectures, so add defensive validation for timetableFrameId (e.g., ensure it's a positive non-zero id) at the top of MainTimetable before calling hooks; if the id is invalid (0, null, undefined), return an early fallback (null, loading skeleton, or error state) or bypass hooks that require a valid frame id (useMyLectures, useTotalGrades), and ensure any downstream use of timetableFrameId is guarded to avoid passing an invalid id into useMyLectures/useTotalGrades.src/components/IndexComponents/IndexTimetable/index.tsx (1)
31-39:timetableFrameId={0}을 전달하면 불필요한 쿼리 인스턴스가 생성됩니다.
Timetable컴포넌트는 내부적으로useMyLectures(timetableFrameId)를 호출하며, 이는useTimetableInfoList를 통해timetableQueries.lectureInfo쿼리를 실행합니다.queryFn의 가드로 인해 실제 API 호출은 방지되지만,queryKey: [TIMETABLE_INFO_LIST, 0]으로 캐시 엔트리가 생성됩니다.placeholder 렌더링 시에는
Timetable대신 시간표 그리드 UI만 표시하는 별도의 presentational 컴포넌트를 사용하는 것이 더 깔끔할 수 있습니다. 다만 현재 구현도 기능적으로는 동작하므로, 향후 리팩토링 시 고려하시면 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/IndexComponents/IndexTimetable/index.tsx` around lines 31 - 39, The placeholder mounts Timetable with timetableFrameId={0}, which triggers useMyLectures → useTimetableInfoList and creates a cache key [TIMETABLE_INFO_LIST, 0]; replace this by rendering a presentational-only component (e.g., TimetableGridPlaceholder or TimetableSkeleton) that reproduces the grid UI and accepts columnWidth/firstColumnWidth/rowHeight/totalHeight props but does not call useMyLectures, or alternatively avoid passing timetableFrameId (ensure Timetable checks for undefined) so that timetableQueries.lectureInfo is not instantiated; update the placeholder to use the new presentational component instead of Timetable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/IndexComponents/IndexTimetable/index.tsx`:
- Around line 31-39: The placeholder mounts Timetable with timetableFrameId={0},
which triggers useMyLectures → useTimetableInfoList and creates a cache key
[TIMETABLE_INFO_LIST, 0]; replace this by rendering a presentational-only
component (e.g., TimetableGridPlaceholder or TimetableSkeleton) that reproduces
the grid UI and accepts columnWidth/firstColumnWidth/rowHeight/totalHeight props
but does not call useMyLectures, or alternatively avoid passing timetableFrameId
(ensure Timetable checks for undefined) so that timetableQueries.lectureInfo is
not instantiated; update the placeholder to use the new presentational component
instead of Timetable.
In `@src/components/TimetablePage/components/MainTimetable/index.tsx`:
- Around line 24-31: MainTimetable receives timetableFrameId and passes it
directly to useMyLectures, so add defensive validation for timetableFrameId
(e.g., ensure it's a positive non-zero id) at the top of MainTimetable before
calling hooks; if the id is invalid (0, null, undefined), return an early
fallback (null, loading skeleton, or error state) or bypass hooks that require a
valid frame id (useMyLectures, useTotalGrades), and ensure any downstream use of
timetableFrameId is guarded to avoid passing an invalid id into
useMyLectures/useTotalGrades.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b94d81d-46e8-44b6-925f-30584d235bc0
📒 Files selected for processing (4)
src/api/timetable/queries.tssrc/components/IndexComponents/IndexTimetable/index.tsxsrc/components/TimetablePage/components/MainTimetable/index.tsxsrc/pages/timetable/index.tsx
ff1451
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/TimetablePage/components/TimetableGridPlaceholder/index.tsx (1)
5-8: 시간 슬롯 정의를 한 곳으로 모아 주세요.
DEFAULT_TIME_STRING가 여기와src/utils/zustand/myLectures.ts에 중복돼 있고,timetable__content높이도 슬롯 개수를 다시20으로 하드코딩하고 있습니다. 기본 시간대가 바뀌면 placeholder 높이와 실제 시간표 높이가 서로 어긋날 가능성이 큽니다. 공통 상수로 분리하고timetable__content높이도columnHeight를 재사용하는 편이 안전합니다.As per coding guidelines,
src/**: 1. Next.js, React, TypeScript 팀 코드 컨벤션 및 공식 스타일 가이드(React/TS best practices)를 우선적으로 반영하여, 가독성·안정성(Null/에러 처리)·테스트/유지보수 용이성·브라우저/접근성 이슈 등을 검토해주세요.Also applies to: 23-24, 48-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TimetablePage/components/TimetableGridPlaceholder/index.tsx` around lines 5 - 8, DEFAULT_TIME_STRING is duplicated between TimetableGridPlaceholder (DEFAULT_TIME_STRING) and src/utils/zustand/myLectures.ts and timetable__content height is hardcoded to 20; extract the shared time-slot array into a common constant module (e.g., export DEFAULT_TIME_SLOTS) and import it in both TimetableGridPlaceholder and myLectures, then replace hardcoded "20"/manual slot-based heights by reusing the shared columnHeight or computing height from DEFAULT_TIME_SLOTS.length (use the existing columnHeight symbol if present) so placeholder and real timetable stay in sync; update references to DEFAULT_TIME_STRING to the new exported name and ensure imports are added where needed.src/components/TimetablePage/components/MainTimetable/index.tsx (1)
105-120: 빈 시간표 상태를 텍스트로도 전달해 주세요.여기서는
TimetableGridPlaceholder만 렌더링하는데, 그 컴포넌트 루트는aria-hidden입니다 (src/components/TimetablePage/components/TimetableGridPlaceholder/index.tsx). 그래서 스크린 리더에서는 이 영역이 왜 비어 있는지 알 수 없습니다.등록된 시간표가 없습니다같은 안내 문구를 시각적으로 노출하거나 최소한sr-only로 함께 렌더링하는 편이 좋겠습니다.As per coding guidelines,
src/components/**: 4. 접근성(a11y) 관련 속성이 적절히 사용되고 있는지 확인해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TimetablePage/components/MainTimetable/index.tsx` around lines 105 - 120, The Timetable currently renders only TimetableGridPlaceholder (which has aria-hidden) so screen readers get no indication of an empty timetable; update the JSX where MainTimetableLayout is used (the timetableContent prop) to render an explicit empty-state message alongside the placeholder—e.g., a visually visible or screen-reader-only element with text like "등록된 시간표가 없습니다"—so that either a visible hint or an sr-only <div> (or a11y utility component) is included when myLectures is empty; modify the code around MainTimetableLayout / TimetableGridPlaceholder usage to conditionally include that message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TimetablePage/components/MainTimetable/index.tsx`:
- Around line 193-198: The current MainTimetable branch only uses
isValidTimetableFrameId and can still render non-existent frames; update
MainTimetable to also verify the given timetableFrameId exists in the current
timeTableFrameList (or via the same selector/hook that provides frames), and if
it does not, replace it with the first valid frame id from timeTableFrameList;
only when no valid frames exist fall back to rendering InvalidMainTimetable;
ensure you reference and use the same source of truth used by
initialFrameId/resolvedCurrentFrameIndex so edit/download/modal flows and
useMyLectures receive a corrected id (adjust MainTimetable to validate
membership against timeTableFrameList, choose first valid id if missing, then
pass that id to ValidMainTimetable or render InvalidMainTimetable).
---
Nitpick comments:
In `@src/components/TimetablePage/components/MainTimetable/index.tsx`:
- Around line 105-120: The Timetable currently renders only
TimetableGridPlaceholder (which has aria-hidden) so screen readers get no
indication of an empty timetable; update the JSX where MainTimetableLayout is
used (the timetableContent prop) to render an explicit empty-state message
alongside the placeholder—e.g., a visually visible or screen-reader-only element
with text like "등록된 시간표가 없습니다"—so that either a visible hint or an sr-only <div>
(or a11y utility component) is included when myLectures is empty; modify the
code around MainTimetableLayout / TimetableGridPlaceholder usage to
conditionally include that message.
In `@src/components/TimetablePage/components/TimetableGridPlaceholder/index.tsx`:
- Around line 5-8: DEFAULT_TIME_STRING is duplicated between
TimetableGridPlaceholder (DEFAULT_TIME_STRING) and
src/utils/zustand/myLectures.ts and timetable__content height is hardcoded to
20; extract the shared time-slot array into a common constant module (e.g.,
export DEFAULT_TIME_SLOTS) and import it in both TimetableGridPlaceholder and
myLectures, then replace hardcoded "20"/manual slot-based heights by reusing the
shared columnHeight or computing height from DEFAULT_TIME_SLOTS.length (use the
existing columnHeight symbol if present) so placeholder and real timetable stay
in sync; update references to DEFAULT_TIME_STRING to the new exported name and
ensure imports are added where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42d5edd4-73cc-46b4-9602-6a71c27b7d8d
📒 Files selected for processing (3)
src/components/IndexComponents/IndexTimetable/index.tsxsrc/components/TimetablePage/components/MainTimetable/index.tsxsrc/components/TimetablePage/components/TimetableGridPlaceholder/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/IndexComponents/IndexTimetable/index.tsx
ff1451
force-pushed
the
codex/fix-pc-timetable-empty-frame
branch
from
b3f98fb to
d7bf3a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR? 🔍
Changes 📝
timetable_frame_id가0또는null상태일 때 시간표 상세 조회가 나가지 않도록 공통 가드를 추가했습니다./timetable페이지에서 invalid query/main frame 값을 직접 state 로 밀어넣지 않도록 정리해 클라이언트 예외 경로를 제거했습니다.ScreenShot 📷
Test CheckList ✅
yarn eslint src/api/timetable/queries.ts src/components/IndexComponents/IndexTimetable/index.tsx src/components/TimetablePage/components/MainTimetable/index.tsx src/pages/timetable/index.tsxyarn tsc --noEmit/timetable페이지가 안전하게 렌더되도록 코드 경로 점검Precaution
src/assets/images/미추적 파일은 PR에 포함하지 않았습니다.caniuse-lite is outdated경고는 기존 환경 경고로, 이번 수정 범위와는 별개입니다.✔️ Please check if the PR fulfills these requirements
developbranch unconditionally?main?yarn lintSummary by CodeRabbit
릴리스 노트
버그 수정
개선사항
신규