Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
요약
변경 사항ScheduleRoomClient 결과 화면 열기 리팩터
CreateScheduleClient 캘린더 바쁜 슬롯 실제 조회
시퀀스 다이어그램sequenceDiagram
participant User
participant SubmissionDonePanel
participant ScheduleRoomClient
participant ScheduleAPI as /api/schedules/:id
User->>SubmissionDonePanel: "결과 화면 열기" 클릭
SubmissionDonePanel->>ScheduleRoomClient: onOpenResult()
ScheduleRoomClient->>ScheduleRoomClient: setError(null), setIsOpeningResult(true)
ScheduleRoomClient->>ScheduleAPI: fetch({ cache: "no-store" })
ScheduleAPI-->>ScheduleRoomClient: resultSchedule JSON
ScheduleRoomClient->>ScheduleRoomClient: schedule/serverSaysHost/hasSubmittedAvailability/submittedName 갱신
ScheduleRoomClient->>ScheduleRoomClient: setIsOpeningResult(false)
ScheduleRoomClient->>SubmissionDonePanel: 결과 화면 렌더링
코드 리뷰 난이도🎯 3 (Moderate) | ⏱️ ~25 minutes 연관 PR
제안 레이블
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/schedule/`[id]/ScheduleRoomClient.test.tsx:
- Around line 116-117: The test is failing because line 117 uses getByRole to
query the "결과 화면 열기" button immediately after waiting for the submission text,
but the SubmissionDonePanel component's useEffect hook asynchronously calls
Supabase getUser() to set the isLoggedIn state. At the time of the query,
isLoggedIn may still be false, rendering the non-logged-in UI and the button
doesn't exist. Replace the getByRole call with findByRole to wait for the button
to appear in the DOM after the asynchronous login state update completes and the
logged-in UI is rendered.
In `@src/app/schedule/`[id]/ScheduleRoomClient.tsx:
- Around line 322-350: In the openSubmittedResultView function, the
setHasSubmittedAvailability state update uses an OR operation that allows a
client-side true value to override the server's false value, violating server
authority. Remove the OR operator and the existing hasSubmittedAvailability
reference, replacing it with just Boolean(result.hasSubmittedAvailability) to
make the server response the authoritative source of truth. If this OR pattern
is intentionally needed for a specific use case, add an explanatory comment
documenting why the client-side value should persist even when the server
indicates otherwise.
In `@src/app/schedule/create/CreateScheduleClient.tsx`:
- Around line 1043-1072: The pure utility functions fetchConnectedBusySlotKeys,
buildQueryWindow, addEventBusyKeys, and toDayCode are currently defined in the
CreateScheduleClient.tsx component file, which makes them difficult to unit test
and violates the MOIM architecture guidelines. Extract these pure functions and
their related type definitions (CalendarBusyLoadResult, GoogleCalendarSummary,
ICloudCalendarSummary, GoogleEventPayload, ICloudEventPayload) from
CreateScheduleClient.tsx and move them to a new utility module in
lib/scheduling/. Create corresponding unit test files in __tests__/ using
colocated testing patterns, then import the functions back into the component
file to maintain its current behavior.
- Around line 1221-1225: The conditional check on hour in
CreateScheduleClient.tsx is unnecessary dead code. Since
Date.prototype.getHours() always returns a value between 0 and 23, the condition
hour >= 0 && hour < 24 will always be true. Remove the if statement and directly
call keys.add() with the day and hour values without the conditional check.
- Around line 1106-1114: The sourceCount is being incremented by
calendars.length before the Promise.allSettled check, which means failed
calendar retrievals are still counted in the total, creating inaccurate
user-facing messages. Move the sourceCount increment logic to only count
calendars where the promise was fulfilled (result.status === "fulfilled"),
incrementing by 1 for each successful result instead of adding the total
calendars.length upfront. Apply this same fix to the iCloud calendar handling
code that contains the same pattern, ensuring both Google and iCloud calendar
sources only count successfully retrieved calendars in their respective
sourceCount calculations.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: afc84605-ffe1-40eb-9605-52b91f51718e
📒 Files selected for processing (3)
src/app/schedule/[id]/ScheduleRoomClient.test.tsxsrc/app/schedule/[id]/ScheduleRoomClient.tsxsrc/app/schedule/create/CreateScheduleClient.tsx
Comment on lines
+116
to
+117
| await screen.findByText(/시간 제출 완료/); | ||
| await user.click(screen.getByRole("button", { name: "결과 화면 열기" })); |
There was a problem hiding this comment.
테스트 실패: SubmissionDonePanel의 비동기 로그인 상태 반영 전에 버튼 조회.
Line 117에서 getByRole("button", { name: "결과 화면 열기" })가 실패하는 이유는 SubmissionDonePanel 내부 useEffect(원본 파일 Lines 1009-1016)에서 Supabase getUser()를 호출해 isLoggedIn을 비동기로 설정하기 때문. Line 116에서 "시간 제출 완료"만 기다린 시점엔 아직 isLoggedIn=false일 수 있어 비로그인 UI가 렌더링되고, "결과 화면 열기" 버튼이 존재하지 않음.
Why: 비동기 상태 반영이 완료되기 전에 DOM 쿼리를 수행하면 요소를 찾을 수 없음.
How: 로그인 전용 요소가 렌더링될 때까지 findBy*로 대기.
🔧 수정안
await screen.findByText(/시간 제출 완료/);
+ await screen.findByRole("button", { name: "결과 화면 열기" });
- await user.click(screen.getByRole("button", { name: "결과 화면 열기" }));
+ await user.click(screen.getByRole("button", { name: "결과 화면 열기" }));또는 더 명확하게:
await screen.findByText(/시간 제출 완료/);
- await user.click(screen.getByRole("button", { name: "결과 화면 열기" }));
+ const resultButton = await screen.findByRole("button", { name: "결과 화면 열기" });
+ await user.click(resultButton);🧰 Tools
🪛 GitHub Actions: CI Pipeline / 0_code_quality.txt
[error] 117-117: TestingLibraryElementError in test '제출 완료 화면의 결과 버튼이 같은 URL 이동 대신 결과 데이터를 불러와 현황 화면을 연다': Unable to find an accessible element with the role "button" and name "결과 화면 열기". Command/step: user.click(screen.getByRole("button", { name: "결과 화면 열기" }))
🪛 GitHub Actions: CI Pipeline / code_quality
[error] 117-117: TestingLibraryElementError: Unable to find an accessible element with the role "button" and name "결과 화면 열기" (failed test: "제출 완료 화면의 결과 버튼이 같은 URL 이동 대신 결과 데이터를 불러와 현황 화면을 연다").
🪛 GitHub Check: code_quality
[failure] 117-117: src/app/schedule/[id]/ScheduleRoomClient.test.tsx > ScheduleRoomClient 결과 화면 열기 > 제출 완료 화면의 결과 버튼이 같은 URL 이동 대신 결과 데이터를 불러와 현황 화면을 연다
TestingLibraryElementError: Unable to find an accessible element with the role "button" and name "결과 화면 열기"
Here are the accessible roles:
main:
Name "":
banner:
Name "":
link:
navigation:
Name "":
button:
Name "메뉴 토글":
Name "적용":
Name "캘린더 연동 화면 보기":
complementary:
Name "":
paragraph:
Name "":
Name "":
Comment on lines
+322
to
+350
| async function openSubmittedResultView() { | ||
| setError(""); | ||
| setIsOpeningResult(true); | ||
|
|
||
| try { | ||
| const response = await fetch(`/api/schedules/${scheduleId}`, { | ||
| cache: "no-store", | ||
| }); | ||
| const result = await response.json(); | ||
| if (!response.ok) { | ||
| throw new Error(result.error ?? "결과 화면을 열 수 없습니다."); | ||
| } | ||
|
|
||
| setSchedule(result.schedule); | ||
| setServerSaysHost(Boolean(result.isHost)); | ||
| setHasSubmittedAvailability( | ||
| Boolean(result.hasSubmittedAvailability) || hasSubmittedAvailability, | ||
| ); | ||
| if (result.participantName) { | ||
| setSubmittedName(result.participantName); | ||
| } | ||
| } catch (caught) { | ||
| setError( | ||
| caught instanceof Error ? caught.message : "요청에 실패했습니다.", | ||
| ); | ||
| } finally { | ||
| setIsOpeningResult(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
Line 338 상태 병합 로직이 서버 권위를 침해할 수 있음.
setHasSubmittedAvailability(
Boolean(result.hasSubmittedAvailability) || hasSubmittedAvailability,
);서버가 hasSubmittedAvailability: false를 반환해도 클라이언트가 이미 true면 계속 true로 유지된다. 이는 서버 상태를 source of truth로 보지 않는 패턴.
Why: 서버가 "제출 기록 삭제됨" 같은 상황을 명시적으로 알려도 클라이언트가 무시하면 UI와 실제 상태가 불일치.
How: 서버 응답을 신뢰하도록 OR 제거.
🔧 제안 수정
- setHasSubmittedAvailability(
- Boolean(result.hasSubmittedAvailability) || hasSubmittedAvailability,
- );
+ setHasSubmittedAvailability(Boolean(result.hasSubmittedAvailability));의도적으로 "한 번 true면 항상 유지"가 필요하다면 주석으로 Why를 명시해야 함.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/schedule/`[id]/ScheduleRoomClient.tsx around lines 322 - 350, In the
openSubmittedResultView function, the setHasSubmittedAvailability state update
uses an OR operation that allows a client-side true value to override the
server's false value, violating server authority. Remove the OR operator and the
existing hasSubmittedAvailability reference, replacing it with just
Boolean(result.hasSubmittedAvailability) to make the server response the
authoritative source of truth. If this OR pattern is intentionally needed for a
specific use case, add an explanatory comment documenting why the client-side
value should persist even when the server indicates otherwise.
Comment on lines
+1043
to
+1072
| type CalendarBusyLoadResult = { | ||
| busySlotKeys: string[]; | ||
| sourceCount: number; | ||
| errors: string[]; | ||
| }; | ||
|
|
||
| type GoogleCalendarSummary = { | ||
| id: string; | ||
| }; | ||
|
|
||
| type ICloudCalendarSummary = { | ||
| calendarUrl: string; | ||
| }; | ||
|
|
||
| type GoogleEventPayload = { | ||
| start?: { | ||
| dateTime?: string; | ||
| date?: string; | ||
| }; | ||
| end?: { | ||
| dateTime?: string; | ||
| date?: string; | ||
| }; | ||
| }; | ||
|
|
||
| type ICloudEventPayload = { | ||
| startAt?: string; | ||
| endAt?: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
캘린더 파싱 유틸 함수를 lib/scheduling/으로 분리 권장.
fetchConnectedBusySlotKeys, buildQueryWindow, addEventBusyKeys, toDayCode 등은 React와 무관한 순수 함수. 현재 컴포넌트 파일 하단에 있어 단위 테스트 작성이 어려움.
MOIM 아키텍처 가이드에 따르면 순수 함수는 lib/scheduling/에 배치하고 __tests__/에 코로케이션 테스트를 두어야 함.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/schedule/create/CreateScheduleClient.tsx` around lines 1043 - 1072,
The pure utility functions fetchConnectedBusySlotKeys, buildQueryWindow,
addEventBusyKeys, and toDayCode are currently defined in the
CreateScheduleClient.tsx component file, which makes them difficult to unit test
and violates the MOIM architecture guidelines. Extract these pure functions and
their related type definitions (CalendarBusyLoadResult, GoogleCalendarSummary,
ICloudCalendarSummary, GoogleEventPayload, ICloudEventPayload) from
CreateScheduleClient.tsx and move them to a new utility module in
lib/scheduling/. Create corresponding unit test files in __tests__/ using
colocated testing patterns, then import the functions back into the component
file to maintain its current behavior.
Source: Coding guidelines
Comment on lines
+1106
to
+1114
| sourceCount += calendars.length; | ||
| for (const result of results) { | ||
| if (result.status === "fulfilled") { | ||
| for (const event of result.value.events ?? []) { | ||
| addEventBusyKeys(busyKeys, getGoogleEventRange(event)); | ||
| } | ||
| } else { | ||
| errors.push("google"); | ||
| } |
There was a problem hiding this comment.
sourceCount가 실패한 캘린더까지 포함하여 메시지가 부정확해질 수 있음.
Promise.allSettled 전에 calendars.length를 더하므로, 개별 캘린더 이벤트 조회가 실패해도 sourceCount에 포함됨. "3개 캘린더에서 0개 바쁜 시간대를 반영했습니다" 같은 오해의 소지가 있는 메시지가 출력될 수 있음.
🐛 성공한 캘린더만 카운트하도록 수정
- sourceCount += calendars.length;
for (const result of results) {
if (result.status === "fulfilled") {
+ sourceCount += 1;
for (const event of result.value.events ?? []) {
addEventBusyKeys(busyKeys, getGoogleEventRange(event));
}
} else {
errors.push("google");
}
}iCloud 쪽(Line 1143)도 동일하게 수정 필요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/schedule/create/CreateScheduleClient.tsx` around lines 1106 - 1114,
The sourceCount is being incremented by calendars.length before the
Promise.allSettled check, which means failed calendar retrievals are still
counted in the total, creating inaccurate user-facing messages. Move the
sourceCount increment logic to only count calendars where the promise was
fulfilled (result.status === "fulfilled"), incrementing by 1 for each successful
result instead of adding the total calendars.length upfront. Apply this same fix
to the iCloud calendar handling code that contains the same pattern, ensuring
both Google and iCloud calendar sources only count successfully retrieved
calendars in their respective sourceCount calculations.
Comment on lines
+1221
to
+1225
| const day = toDayCode(cursor); | ||
| const hour = cursor.getHours(); | ||
| if (hour >= 0 && hour < 24) { | ||
| keys.add(`${day}-${hour}`); | ||
| } |
There was a problem hiding this comment.
hour >= 0 && hour < 24 조건은 항상 참이므로 dead code.
Date.prototype.getHours()는 항상 0–23 범위를 반환함. 불필요한 조건 분기.
🧹 불필요한 조건 제거
if (next > range.start && cursor < range.end) {
const day = toDayCode(cursor);
const hour = cursor.getHours();
- if (hour >= 0 && hour < 24) {
- keys.add(`${day}-${hour}`);
- }
+ keys.add(`${day}-${hour}`);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/schedule/create/CreateScheduleClient.tsx` around lines 1221 - 1225,
The conditional check on hour in CreateScheduleClient.tsx is unnecessary dead
code. Since Date.prototype.getHours() always returns a value between 0 and 23,
the condition hour >= 0 && hour < 24 will always be true. Remove the if
statement and directly call keys.add() with the day and hour values without the
conditional check.
Collaborator
Author
|
Superseded by the updated branch fix/calendar-result-and-imports-v2, which includes the CI stability fix for the result button test. |
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
1 participant
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.
요약
관련 이슈
Closes #114
Closes #115
테스트
체크리스트