Conversation
- Tooltip 컴포넌트를 추가하여 사용자에게 UI 설명 기능 제공. - 약속 승인 목록에 스켈레톤 로딩 UI를 추가하여 UX 개선. - useUpdateMeeting hook을 추가하여 약속 수정 로직을 분리하고, MeetingCreatePage와 MeetingSettingPage의 폼 검증 및 사용자 경험 개선. - 약속 관련 API 엔드포인트 및 타입 정의를 확장하여 기능 지원 강화.
Closed
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRadix 기반 Tooltip을 전역 도입하고, 회의 생성/수정 폼을 중앙화된 formData로 리팩터링했으며 승인 리스트를 prop 기반으로 전환했습니다. Kakao Maps SDK를 동적 로드하도록 추가하고 지도/검색 훅에 에러 상태 및 비동기 초기화를 도입했습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant MeetingCreatePage as MeetingCreatePage
participant useMeetingDetail as useMeetingDetail
participant useMeetingForm as useMeetingForm
participant useUpdateMeeting as useUpdateMeeting
participant API as API
participant Router as Router
User->>MeetingCreatePage: 수정 페이지 접근 (/gatherings/:gatheringId/meetings/:meetingId/update)
MeetingCreatePage->>useMeetingDetail: meetingId로 상세 조회
useMeetingDetail->>API: GET /api/meetings/:id
API-->>useMeetingDetail: meeting 데이터
useMeetingDetail-->>MeetingCreatePage: 초기 데이터 반환
MeetingCreatePage->>useMeetingForm: initialData로 폼 초기화
useMeetingForm-->>MeetingCreatePage: formData, handlers 반환
User->>MeetingCreatePage: 폼 수정 후 제출
MeetingCreatePage->>useUpdateMeeting: UpdateMeetingRequest 전송
useUpdateMeeting->>API: PATCH /api/meetings/:id
API-->>useUpdateMeeting: 업데이트 응답
useUpdateMeeting->>useUpdateMeeting: 관련 캐시 무효화 (detail, approvals)
useUpdateMeeting-->>MeetingCreatePage: 성공 콜백
MeetingCreatePage->>Router: 상세 페이지로 이동
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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: 8
🤖 Fix all issues with AI agents
In `@src/features/meetings/components/MeetingDetailInfo.tsx`:
- Around line 123-129: The onClick opens a Kakao map URL by interpolating
location.name directly, which can break for spaces or special chars; update the
onClick in MeetingDetailInfo.tsx to URL-encode the place name (and
stringify/encode the lat/lng if desired) before building the URL (e.g., use
encodeURIComponent on location.name and on location.latitude/location.longitude)
and then call window.open with the encoded values so the generated URL is valid
and safe.
In `@src/pages/Meetings/MeetingCreatePage.tsx`:
- Around line 155-167: The current unified guard in MeetingCreatePage.tsx blocks
submission in edit mode because it always requires bookThumbnail, bookAuthors,
and bookPublisher even though UpdateMeetingRequest doesn't include book data;
split the validation into two branches: when creating (e.g., !isEdit) require
bookId, bookName, bookThumbnail, bookAuthors, bookPublisher, startDate,
startTime, endDate, endTime, but when updating (e.g., isEdit or when building an
UpdateMeetingRequest) only validate the fields that UpdateMeetingRequest needs
(startDate/startTime/endDate/endTime and whatever update-specific fields are
required) and allow missing book* fields; update the conditional that currently
references bookId/bookThumbnail/bookAuthors/bookPublisher to check mode and
validate accordingly so edit submissions no longer silently return.
- Around line 57-90: Replace broad Zustand subscription and three separate
useEffect handlers with a selector-based subscription for just the modal action
and a single consolidated effect that enforces error priority: use const
openError = useGlobalModalStore(state => state.openError) to avoid redundant
re-renders, then combine the gatheringId check and API error handling into one
useEffect that first validates gatheringId (call openError and navigate as
currently done), else checks gatheringError before meetingError and calls
openError once with the appropriate message and navigation logic; keep
references to navigate, openError, gatheringError, meetingError, and gatheringId
in the effect dependencies.
- Around line 187-199: The code awaits openAlert which returns void so
navigation runs immediately; update the onSuccess handler in
updateMutation.mutate (and the parallel createMutation handler around the noted
create-mode block) to either call openConfirm and await its promise or move the
navigate(ROUTES.GATHERING_DETAIL(gatheringId), { replace: true }) into the
confirmation callback of the modal UI; specifically replace await openAlert('약속
수정 완료', ...) with an awaited call to openConfirm(...) (or call openAlert(...)
and perform navigate inside the modal’s onConfirm handler) so navigation happens
only after user confirmation, keeping updateMutation, createMutation,
openAlert/openConfirm and navigate as the referenced symbols.
In `@src/pages/Meetings/MeetingSettingPage.tsx`:
- Around line 53-64: The effect currently calls openError twice when both
isPendingError and isConfirmedError are true; change the logic in the useEffect
so it only opens a single modal by selecting one error to show (e.g., prefer
pendingError over confirmedError or merge messages) and call openError once with
the chosen error; update the dependency references (useEffect uses
isPendingError, isConfirmedError, pendingError, confirmedError, openError,
navigate) and ensure the navigate callback uses the same chosen error handling
path.
- Around line 18-19: The component currently converts gatheringIdParam to Number
which yields NaN for undefined and is passed into useMeetingApprovals; add
validation so queries aren't fired with an invalid id: compute an isValidId
boolean from useParams (e.g., const isValidId = typeof gatheringIdParam ===
'string' && /^\d+$/.test(gatheringIdParam)), then either (A) pass enabled:
isValidId into useMeetingApprovals call or (B) early-return/navigate when
!isValidId (e.g., navigate('/', { replace: true }) and return null). Update
occurrences of gatheringId usage to rely on the validated
Number(gatheringIdParam) only when isValidId is true. Ensure the option name
enabled is provided to useMeetingApprovals so the hook doesn't run with NaN.
- Around line 101-126: When loading flags are false but data is undefined
(error) the tab shows blank; update both the PENDING and CONFIRMED TabsContent
blocks to render a fallback UI when data is missing instead of rendering
nothing: i.e., change the conditional that currently checks isPendingLoading ?
MeetingApprovalListSkeleton : (pendingData && <MeetingApprovalList ...>) to
render either MeetingApprovalList when pendingData exists, otherwise render
MeetingApprovalListSkeleton or a dedicated error/fallback component (same for
isConfirmedLoading/confirmedData). Ensure you keep the existing props
(data={...}, currentPage={...}, onPageChange={...}) and update both PENDING and
CONFIRMED branches so the UI shows a skeleton/fallback during error states until
navigate runs.
In `@src/shared/ui/Tooltip.tsx`:
- Around line 158-164: handleDismiss currently only calls onDismiss and skips
close(), which leaves the Tooltip open when a consumer supplies onDismiss;
change handleDismiss so it always calls close() first and then, if onDismiss
exists, invoke onDismiss() as a follow-up callback (preserve order and any
args), ensuring the Tooltip closes in dismissable mode even when onDismiss is
provided; update the handleDismiss function and any references to it to reflect
this behavior.
🧹 Nitpick comments (7)
src/features/meetings/hooks/useCreateMeeting.ts (1)
30-30: 주석 일관성 개선 제안동일한 패턴을 사용하는 다른 훅들(
useRejectMeeting,useConfirmMeeting,useDeleteMeeting)은(리스트 + 카운트)상세 정보를 주석에 포함하고 있습니다. 의도적으로 제네릭하게 변경한 것이라면 다른 훅들도 함께 업데이트하는 것이 일관성 측면에서 좋을 것 같습니다.일관성을 위한 제안
옵션 1: 이 파일을 다른 훅들과 동일하게 상세 명시
- // 약속 승인 관련 모든 캐시 무효화 + // 약속 승인 관련 모든 캐시 무효화 (리스트 + 카운트)옵션 2: 다른 훅들(
useRejectMeeting,useConfirmMeeting,useDeleteMeeting)도 이 파일처럼 제네릭하게 변경// 약속 승인 관련 모든 캐시 무효화src/features/meetings/meetings.types.ts (1)
103-104: TODO 코멘트가 남아있습니다.
Todo:실제 응답값이랑 비교해봐야 함— 실제 API 연동 시 응답 타입 검증 후 제거가 필요합니다.이 TODO를 추적할 이슈를 생성해드릴까요?
src/shared/ui/Tooltip.tsx (1)
44-95: non-dismissable 모드에서 불필요한internalOpen상태 초기화
internalOpen은 dismissable 모드에서만 사용되지만, 모든 인스턴스에서useState(true)로 초기화됩니다. 현재 동작에 문제는 없지만, 의도를 명확히 하려면 dismissable일 때만 초기화하는 것이 좋습니다.src/features/meetings/hooks/useMeetingForm.ts (1)
203-212:updateField에서errors참조가 stale할 가능성
updateField내부에서errors를 직접 읽고 있습니다.errors는 렌더 스코프 변수이므로 현재 렌더 시점 값을 참조하지만, 만약 향후useCallback으로 감싸면서 의존성에errors를 빠뜨리면 stale closure 버그로 이어질 수 있습니다.현재는 매 렌더마다 재생성되므로 문제없지만, 방어적으로
clearError만 호출하고 조건 검사를clearError내부로 위임하는 방식도 고려해볼 수 있습니다.src/pages/Meetings/MeetingCreatePage.tsx (3)
92-92:||대신??사용 권장
totalMembers가0일 때||는1로 폴백되지만??는0을 유지합니다.null/undefined만 폴백하려는 의도라면??가 더 정확합니다.- const gatheringMaxCount = gathering?.totalMembers || 1 + const gatheringMaxCount = gathering?.totalMembers ?? 1
276-281:errorMessageprop을 안내 메시지 용도로 사용 — 의미/스타일 불일치 가능
errorMessage에'도서는 수정이 불가합니다.'를 전달하면,Container.Title내부에서 에러 스타일(빨간색 텍스트 등)로 렌더링될 수 있습니다. 안내 메시지라면 별도 prop이나 helper text 방식이 더 적절합니다.
142-144: 불필요한 래퍼 함수 —setBook을 직접 전달 가능
handleBookSelection은setBook을 그대로 호출하는 래퍼입니다.onSelectBook={setBook}으로 직접 전달하면 됩니다.- const handleBookSelection = (book: SearchBookItem) => { - setBook(book) - }Line 461에서:
- onSelectBook={handleBookSelection} + onSelectBook={setBook}
- 약속 생성/수정 전 confirm 모달을 노출하도록 처리 추가. - useDeleteMeeting, useRejectMeeting에 gatheringId를 전달하여 약속 거부/삭제 후 모임 약속 목록 캐시를 함께 무효화. - openAlert에 onClose 콜백을 추가하여 모달 닫힘 후 동작 지원. - MeetingCreatePage 에러 처리 useEffect 통합 및 핸들러 분리 리팩토링. - MeetingSettingPage gatheringId 파싱 안전성 개선.
index.html의 정적 스크립트 태그를 제거하고 loadKakaoSdk.ts 싱글톤 로더를 추가하여 지도 사용 시점에만 SDK를 동적으로 로드. - SDK 로드 실패 시 HTTP 상태 코드별 에러 메시지 표시 - useKakaoMap, useKakaoPlaceSearch에 error 상태 추가 - PlaceSearchModal에 SDK 로드 오류 및 검색 오류 UI 오버레이 추가 - MeetingDetailButton의 isEnabled 조건 반전 버그 수정 - MeetingCreatePage 약속명 24자 제한(slice) 적용
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/meetings/hooks/useKakaoMap.ts (1)
62-109:⚠️ Potential issue | 🟠 MajorSDK 로드 성공 시
error상태가 초기화되지 않습니다.첫 시도 실패 →
setError(message)→ 재시도 성공 시error가 이전 에러 메시지를 유지합니다.
UI에서error를 표시하고 있다면 성공했음에도 에러 메시지가 남아 있게 됩니다.🐛 수정 제안: 성공 경로에서 error 초기화
mapRef.current = map infowindowRef.current = new kakao.maps.InfoWindow({ zIndex: 1 }) + setError(null) setIsInitialized(true)
🤖 Fix all issues with AI agents
In `@src/features/meetings/hooks/useKakaoPlaceSearch.ts`:
- Around line 51-56: In useKakaoPlaceSearch, remove the hardcoded "(400 Bad
Request)" from the error message and instead surface the actual error/status
details (e.g., include the Status value or error message) when Status.ERROR
occurs; also stop calling onSearchSuccess in the error path—either add and call
an onSearchError callback (new prop) or simply avoid invoking onSearchSuccess
when setting setError/setPlaces([]); update references in that hook
(Status.ERROR handling, setError, setPlaces, onSearchSuccess) accordingly so
logs and UI reflect the real error cause.
In `@src/features/meetings/loadKakaoSdk.ts`:
- Line 34: Remove the unused parameter from the onerror handler in
loadKakaoSdk.ts: locate the script.onerror assignment (the handler currently
declared as script.onerror = (_scriptEvent) => { ... }) and change it to a
no-arg arrow function (script.onerror = () => { ... }) so the unused
`_scriptEvent` parameter is eliminated and the ESLint no-unused-vars failure is
resolved.
- Around line 30-32: The Promise created in loadKakaoSdk uses script.onload →
window.kakao.maps.load(() => resolve()) but if the maps.load callback throws the
Promise never settles; update the callback in loadKakaoSdk.ts to wrap
window.kakao.maps.load's callback body in try/catch and call reject(err) on
exceptions (and optionally call resolve only on success), and also add a safe
timeout fallback that rejects the Promise after a reasonable delay to avoid
infinite waiting; reference the script.onload handler, window.kakao.maps.load,
and the resolve/reject closures when making the change.
In `@src/pages/Meetings/MeetingCreatePage.tsx`:
- Around line 57-82: When gatheringId is 0 the first useEffect opens an error
modal and the second useEffect can also open one if useGatheringDetail(0) fails;
update the second useEffect (the API error handler that checks gatheringError
and meetingError) to early-return when gatheringId === 0 so it does nothing for
that invalid-id case—i.e., add a check for gatheringId (the same gatheringId
used above) before evaluating gatheringError/meetingError and calling
openError/navigate to prevent duplicate modals.
🧹 Nitpick comments (3)
src/features/meetings/loadKakaoSdk.ts (1)
38-56:onerror후 동일 URL을fetch로 재요청하여 상태 코드를 확인하는 접근.스크립트 태그 로드와
fetch는 CORS/CSP 정책 적용이 다를 수 있어, 실제 실패 원인과 다른 상태 코드를 받을 수 있습니다. 진단용으로는 괜찮지만 이 점을 인지하고 계시면 됩니다.src/features/meetings/components/MeetingApprovalItem.tsx (1)
45-48: TODO 코멘트 — 동시간대 승인 충돌 UX 개선 필요동시간대 약속 승인 불가 시 서버에서 특정 에러 코드를 내려줄 경우,
onError에서 해당 코드를 분기하여 안내 메시지를 표시하면 좋겠습니다. 이슈로 트래킹할까요?src/pages/Meetings/MeetingCreatePage.tsx (1)
229-231: 생성 모드에서isMeetingLoading이 불필요하게 버튼을 비활성화할 수 있음
isLoading = isGatheringLoading || isMeetingLoading인데, 생성 모드에서도useMeetingDetail(0)이 실행되면isMeetingLoading이 잠시true가 될 수 있습니다. 이는 위의enabled조건 이슈와 연관됩니다.- const isLoading = isGatheringLoading || isMeetingLoading + const isLoading = isGatheringLoading || (isEditMode && isMeetingLoading)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/features/meetings/loadKakaoSdk.ts`:
- Around line 38-61: The current fetch-based diagnostic in loadKakaoSdk.ts
(inside the fetch(script.src) handler used after script.onerror) can produce
misleading messages if the fetch returns 200 while the original script load
failed (TOCTOU); update the fetch.then branch to special-case a 200 response and
produce a distinct, non-misleading message (e.g., indicate the script tag
errored but fetch returned 200—possible CDN/cache/CORS discrepancy) instead of
treating it like a generic success or showing "HTTP 200" failure; keep the
existing kakaoStatusMessages mapping for non-200 codes, log the clearer
200-specific message with console.error, and reject the same Promise (reject(new
Error(...))) so callers still receive an error.
🧹 Nitpick comments (1)
src/features/meetings/loadKakaoSdk.ts (1)
23-67: 타임아웃 없이 Promise가 영구 pending될 수 있습니다.스크립트 로드가
onload/onerror어느 쪽도 발생하지 않는 극단적 상황(네트워크 hang 등)에서kakaoSdkPromise가 영원히 pending → 이후 모든 호출도 같은 pending Promise를 받아 지도 기능 전체가 먹통됩니다.안전장치로 타임아웃을 추가하면 재시도 경로가 열립니다.
💡 타임아웃 제안
kakaoSdkPromise = new Promise((resolve, reject) => { + const TIMEOUT_MS = 15_000 + const timer = setTimeout(() => { + kakaoSdkPromise = null + reject(new Error('카카오 지도 SDK 로드 시간이 초과되었습니다.')) + }, TIMEOUT_MS) + const script = document.createElement('script') // ... script.onload = () => { + clearTimeout(timer) window.kakao.maps.load(() => resolve()) } script.onerror = () => { + clearTimeout(timer) kakaoSdkPromise = null // ... } })
mgYang53
approved these changes
Feb 11, 2026
| const newError: ValidationErrors = {} | ||
|
|
||
| if (!bookId || !bookName) { | ||
| if ( |
Contributor
There was a problem hiding this comment.
bookThumbnail이 없는 경우는 없나요??
Contributor
Author
There was a problem hiding this comment.
먼가 과거 커밋 같은데요!! 코드가 달라여
Contributor
There was a problem hiding this comment.
현재 버전에서 책 썸네일을 필수값으로 검증하는데 만약에 썸네일이 없는 책 데이터가 있을 수도 있을까봐 여쭤본거에요!
그런 경우가 없다면 상관없습니다~
Contributor
Author
There was a problem hiding this comment.
생각 못해봤는데 확인해보니까 썸네일이 없는경우 대체이미지가 있어서 괜찮습니다!
This was referenced Feb 13, 2026
Merged
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.
🚀 풀 리퀘스트 제안
약속 생성 - 도서 검색 기능 연결, 모임 상세 API 조회하여 최대 인원 가져오기
약속 수정 - 기능 구현
약속 상세 - 장소 클릭시 카카오 지도로 이동, 툴팁
약속 설정 - 탭 카운터 로직 수정, 스켈레톤 적용
📋 작업 내용
신규 컴포넌트
신규 훅
API 레이어
기존 컴포넌트 변경 사항
삭제
📄 기타
장소 선택 지도 모달은 디자인 확정되면 새로 이슈파서 작업하겠습니다
Summary by CodeRabbit
새로운 기능
개선사항