Repository navigation
Feat-228 발제/채팅 API 우선 연결 (+ UI수정, Querykey 수정 등, 다양한 수정) - #230
Closed
hongik-luke wants to merge 11 commits into
Closed
hongik-luke wants to merge 11 commits into
hongik-luke wants to merge 11 commits into
Conversation
added 10 commits
March 22, 2026 07:04
📝 WalkthroughWalkthroughAdds infinite-scroll pagination to debate/review lists, implements a draggable meeting chat modal with infinite upward loading, refactors meeting page to use remote meeting/topic data, and introduces new endpoints, services, hooks, types, and UI components for meeting chats and topics. Changes
Sequence DiagramsequenceDiagram
participant User
participant IntersectionObserver as IntersectionObserver
participant Section as Debate/Review Section
participant QueryHook as useMeetingChatsInfiniteQuery
participant Service as meetingChatService / API
User->>User: Scrolls near bottom
User->>IntersectionObserver: Sentinel enters viewport
IntersectionObserver->>Section: inView true
Section->>Section: if hasNextPage && !isFetchingNextPage
Section->>QueryHook: onLoadMore() / fetchNextPage()
QueryHook->>Service: GET /clubs/{clubId}/meetings/{meetingId}/teams/{teamId}/chat/messages?cursor=...
Service-->>QueryHook: paginated chats + nextCursor
QueryHook-->>Section: append pages, update isFetchingNextPage
Section->>User: render additional items / hide sentinel when done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
Contributor
Summary of ChangesHello, 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은 정기모임의 발제 및 채팅 화면을 백엔드 REST API와 통합하는 것을 목표로 합니다. 모임 상세 응답을 기반으로 팀 정보, 발제 목록, 채팅 내역을 동적으로 렌더링하도록 UI를 재구성했습니다. 사용자 경험을 향상시키기 위해 팀 멤버 변경 시 실시간 업데이트를 보장하고, 발제 및 한줄평 목록에 무한 스크롤을 도입했습니다. 또한, 채팅 기능은 전용 모달과 데스크톱 환경에서의 드래그 지원으로 강화되었습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
정기모임의 발제/채팅 기능에 실제 API를 연동하는 큰 규모의 업데이트네요. 더미 데이터를 react-query 기반의 실제 데이터로 교체하고, 채팅 UI를 별도 페이지에서 모달 형태로 리팩토링하는 등 코드 구조와 사용자 경험 양쪽에서 큰 개선이 이루어졌습니다. 특히 복잡했던 채팅 모달을 조 선택과 채팅 화면으로 분리하여 컴포넌트의 단일 책임 원칙을 잘 지키도록 수정한 점이 인상적입니다. 또한, 조 편성 후 데이터 캐시를 미리 가져와서 화면에 즉시 반영되도록 한 최적화도 훌륭합니다. 전반적으로 매우 완성도 높은 변경 사항이라고 생각합니다. 한 가지 작은 개선점을 발견하여 코멘트를 남겼으니 확인 부탁드립니다.
Comment on lines
+65
to
+66
| const clubId = Number(params.id as string); | ||
| const meetingId = Number(params.bookId as string); |
Contributor
There was a problem hiding this comment.
useParams에서 가져온 값을 Number로 변환할 때 as string으로 타입 단언을 사용하셨네요. params.id가 undefined일 경우 Number(undefined)는 NaN이 되는데, as string을 사용하면 Number(undefined as string)이 되어 타입스크립트가 잠재적 오류를 발견하기 어렵게 만듭니다. 이 PR의 다른 파일(admin/.../page.tsx)에서는 toNumber 헬퍼 함수를 사용하거나 타입 단언 없이 처리하고 있습니다. 일관성과 타입 안정성을 위해 타입 단언을 제거하는 것을 제안합니다. useQuery의 enabled 옵션에서 Number.isFinite로 이미 NaN 값을 잘 처리하고 있으므로, as string 없이 Number(params.id)로 변경해도 안전하게 동작할 것입니다.
Suggested change
| const clubId = Number(params.id as string); | |
| const meetingId = Number(params.bookId as string); | |
| const clubId = Number(params.id); | |
| const meetingId = Number(params.bookId); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/groups/[id]/admin/bookcase/[meetingId]/page.tsx (1)
45-45:⚠️ Potential issue | 🟠 Major
isInitializedis declared but never set totrue, causing stale loading behavior and potential data loss.The state
isInitialized(line 45) is never updated totrue, which means:
showLoading(line 183) always treats!isInitializedastrue, showing the loading indicator on every refetch.- Without a guard, the effect resets local
teamsandmembersstate on everydatachange (including background refetches on window focus), potentially discarding unsaved drag-and-drop edits.Either restore the initialization logic or remove
isInitializedentirely if the old behavior is no longer needed.🐛 Option 1: Restore the initialization guard
useEffect(() => { if (!data) return; + if (isInitialized) return; const existingTeamNumbers = data.existingTeams?.map((t) => Number(t.teamNumber)).filter(Number.isFinite) ?? []; const normalized = normalizeTeams(existingTeamNumbers); setTeams(normalized); const mappedMembers: TeamMember[] = data.clubMembers?.map((cm) => ({ clubMemberId: cm.clubMemberId, memberInfo: { nickname: cm.memberInfo?.nickname ?? "", profileImageUrl: cm.memberInfo?.profileImageUrl ?? "", }, teamNumber: cm.teamKey?.teamNumber ?? null, })) ?? []; setMembers(mappedMembers); + setIsInitialized(true); - }, [data]); + }, [data, isInitialized]);🐛 Option 2: Remove unused isInitialized state
- const [isInitialized, setIsInitialized] = useState(false); // ... - const showLoading = isLoading || (!isInitialized && isFetching); + const showLoading = isLoading;Also applies to: 56-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/groups/`[id]/admin/bookcase/[meetingId]/page.tsx at line 45, The isInitialized state (isInitialized / setIsInitialized) is never flipped, causing showLoading to remain true and effects that reset local teams/members on every data change; fix by either (A) restoring the initialization guard: in the effect that currently consumes data (the useEffect that assigns teams and members from data) detect the first successful load (e.g., data present and not previously initialized) and call setIsInitialized(true) after applying initial teams/members so subsequent refetches don't clobber local state, or (B) remove isInitialized entirely and update showLoading to rely only on query flags (isLoading/isFetching) and modify the data effect to merge updates instead of resetting local teams/members on background refetches; reference the variables isInitialized, setIsInitialized, showLoading, teams, members, and the data-handling useEffect when making the change.
🧹 Nitpick comments (6)
src/hooks/mutations/useMeetingMutations.ts (1)
18-39: Redundant concurrent invalidation and prefetch for the same query keys.Running
invalidateQueriesandprefetchQueryconcurrently for the same keys (detailKey,membersKey) causes redundant network requests.invalidateQueriesalready triggers a background refetch for active queries, making the parallelprefetchQuerycalls wasteful.Consider either:
- Just invalidate and let React Query handle refetching when data is accessed.
- Use only
prefetchQuery(without invalidation) if you want to guarantee fresh data is cached before proceeding.♻️ Simplified approach using only invalidation
onSuccess: async (_, vars) => { const detailKey = meetingQueryKeys.detail(vars.clubId, vars.meetingId); const membersKey = meetingQueryKeys.members(vars.clubId, vars.meetingId); await Promise.all([ qc.invalidateQueries({ queryKey: detailKey }), qc.invalidateQueries({ queryKey: membersKey }), - - qc.prefetchQuery({ - queryKey: detailKey, - queryFn: () => - meetingService.getMeetingDetail({ - clubId: vars.clubId, - meetingId: vars.meetingId, - }), - }), - - qc.prefetchQuery({ - queryKey: membersKey, - queryFn: () => - meetingService.getMeetingMembers({ - clubId: vars.clubId, - meetingId: vars.meetingId, - }), - }), ]); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/mutations/useMeetingMutations.ts` around lines 18 - 39, The block currently calls qc.invalidateQueries and qc.prefetchQuery for the same keys (detailKey, membersKey) causing duplicate fetches; pick one strategy—here remove the qc.prefetchQuery calls and only await the two qc.invalidateQueries for detailKey and membersKey so React Query will handle background refetching for active queries; update the Promise.all to only include qc.invalidateQueries({ queryKey: detailKey }) and qc.invalidateQueries({ queryKey: membersKey }) and delete the prefetchQuery calls that call meetingService.getMeetingDetail / getMeetingMembers using vars.clubId and vars.meetingId.src/components/base-ui/Bookcase/MeetingChatModal.tsx (2)
100-136: Consider extracting stable callback references for observer dependencies.The effect recreates the
IntersectionObserverwhenever any ofchatsQuery.hasNextPage,isFetchingNextPage, orfetchNextPagechanges. WhilefetchNextPageis stable from React Query, the boolean flags can change frequently during pagination.This is acceptable for correctness, but if you notice performance issues with rapid observer teardown/setup, consider reading flags inside the callback via a ref.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/Bookcase/MeetingChatModal.tsx` around lines 100 - 136, The effect recreates the IntersectionObserver frequently because it depends on changing booleans (chatsQuery.hasNextPage, chatsQuery.isFetchingNextPage) and fetchNextPage; extract stable callback refs so the observer only depends on stable values: create refs (e.g., hasNextPageRef, isFetchingNextPageRef, fetchNextPageRef) and update them in a small effect when chatsQuery changes, then use a single stable intersection handler (handleIntersect) that reads those refs and calls fetchNextPageRef.current(); keep the observer effect dependent only on isOpen, selectedTeamId, topSentinelRef, and scrollRef so it doesn't teardown/recreate on every flag flip.
39-50: Redundant condition in sort comparator.After deduplication by
messageId, all IDs are unique. The conditionif (a.messageId !== b.messageId)is always true, so thesendAtfallback comparison never executes.♻️ Simplified sort
function dedupeAndSortMessages(list: MeetingChatMessageItem[]) { const map = new Map<number, MeetingChatMessageItem>(); for (const item of list) { map.set(item.messageId, item); } - return Array.from(map.values()).sort((a, b) => { - if (a.messageId !== b.messageId) return a.messageId - b.messageId; - return new Date(a.sendAt).getTime() - new Date(b.sendAt).getTime(); - }); + return Array.from(map.values()).sort((a, b) => a.messageId - b.messageId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/Bookcase/MeetingChatModal.tsx` around lines 39 - 50, The sort comparator in dedupeAndSortMessages contains a redundant branch: after deduplication by messageId (Map keyed on item.messageId) IDs are unique so the if (a.messageId !== b.messageId) check and the sendAt fallback never runs; simplify the comparator in dedupeAndSortMessages to sort only by messageId (e.g., return a.messageId - b.messageId) or, if you prefer to keep a deterministic fallback, use a single expression that compares messageId then sendAt when equal (but the equality case should be unreachable). Update the comparator in dedupeAndSortMessages to remove the unreachable condition while keeping map, messageId and sendAt references intact.src/components/base-ui/Bookcase/ChatTeamSelectModal.tsx (1)
5-9: Consider aligningteamIdtype with downstream consumers.
ChatTeam.teamIdis typed asstring, butMeetingChatModalexpectsselectedTeamId: numberand usesNumber(team.teamId)for comparison. This implicit coercion could cause subtle bugs ifteamIdcontains non-numeric strings.If the API returns numeric IDs, consider typing
teamIdasnumberfor consistency, or document that string-to-number conversion is expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/Bookcase/ChatTeamSelectModal.tsx` around lines 5 - 9, Change the ChatTeam.teamId type to number (instead of string) so it aligns with MeetingChatModal's selectedTeamId and avoids Number(team.teamId) coercion; update the ChatTeam type definition (ChatTeam) and any places constructing/consuming team objects to use numeric IDs, and remove the implicit Number(...) conversion in MeetingChatModal where it compares selectedTeamId to team.teamId.src/app/groups/[id]/bookcase/[bookId]/meeting/page.tsx (2)
272-283: Extract magic numbers for panel dimensions as constants.The hardcoded
panelWidth = 426andpanelHeight = 716could drift out of sync with the actualMeetingChatModaldimensions. Consider extracting these as shared constants or deriving them from the modal component.💡 Suggested refactor
+const CHAT_PANEL_WIDTH = 426; +const CHAT_PANEL_HEIGHT = 716; +const MOBILE_BREAKPOINT = 768; // In handleChatDragStart: - if (window.innerWidth < 768) return; + if (window.innerWidth < MOBILE_BREAKPOINT) return; // In handlePointerMove: - const panelWidth = 426; - const panelHeight = 716; + const panelWidth = CHAT_PANEL_WIDTH; + const panelHeight = CHAT_PANEL_HEIGHT;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx around lines 272 - 283, Replace the magic numbers for panel dimensions by extracting them into shared constants or deriving them from the modal component: remove the inline panelWidth = 426 and panelHeight = 716 and instead reference a single exported constant (e.g., MEETING_CHAT_MODAL_WIDTH and MEETING_CHAT_MODAL_HEIGHT) or a getter on MeetingChatModal so the nextX/nextY calculations use those symbols; update the references in the drag logic that use dragRef.current.originX and originY to use the new constants to keep sizing consistent across the component and the modal.
189-199: Consider aligningChatTeam.teamIdtype to avoid repeated conversions.The
ChatTeamtype expectsteamId: string, but the internal team data usesnumber. This leads toString()conversions here andNumber()conversions elsewhere (lines 211, 213, 237, 514). While functional, this is error-prone.If the
ChatTeamtype inChatTeamSelectModalcan be updated to usenumber, it would eliminate these conversions and reduce potential bugs from type coercion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx around lines 189 - 199, The ChatTeam type and related uses should use number for teamId to avoid String/Number coercion: update the ChatTeam interface (and any props in ChatTeamSelectModal) to declare teamId: number, change the chatSelectableTeams mapping in useMemo to assign teamId: team.teamId (no String()), and update all comparisons/filters (e.g., the filter that compares team.teamId to myTeamId and any places expecting string ids) to use numeric comparisons (no Number()). Also update any state, prop types, and event handlers that consume ChatTeam.teamId to accept a number.
🤖 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/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx:
- Line 77: The initial chatPosition ({ x: 980, y: 56 }) can place the modal
off-screen on narrower viewports; update the initialization of chatPosition in
the useState call so x is computed from the current viewport width (e.g.,
window.innerWidth) minus the panel width and a margin, then clamp x to a minimum
value so it never starts off-screen; implement this either by using a lazy
initializer for useState that reads window.innerWidth (guarded for SSR) or by
setting chatPosition on first render in a useEffect that calls setChatPosition,
referencing the chatPosition and setChatPosition identifiers and ensuring the
panel width (426) and a minimal margin are used when computing/clamping x.
In `@src/components/base-ui/Bookcase/Admin/bookdetailgrouping/MemberPool.tsx`:
- Around line 95-107: The submit flow allows modal closes while an in-flight
submit is running; fix by blocking every modal-close path while isSubmitting is
true: in handleConfirmYes keep setIsSubmitting/use await as-is but ensure all
close handlers (e.g., onCancel, onClose, any handleClose or close-button
handlers) early-return when isSubmitting and do not call setIsConfirmOpen(false)
or router.back; also disable backdrop/escape-close via the modal props while
isSubmitting and only call router.back after onSubmit resolves and when not
interrupted. Apply the same guard to the other close handlers referenced (the
handlers around the other modal-close sites in the file).
---
Outside diff comments:
In `@src/app/groups/`[id]/admin/bookcase/[meetingId]/page.tsx:
- Line 45: The isInitialized state (isInitialized / setIsInitialized) is never
flipped, causing showLoading to remain true and effects that reset local
teams/members on every data change; fix by either (A) restoring the
initialization guard: in the effect that currently consumes data (the useEffect
that assigns teams and members from data) detect the first successful load
(e.g., data present and not previously initialized) and call
setIsInitialized(true) after applying initial teams/members so subsequent
refetches don't clobber local state, or (B) remove isInitialized entirely and
update showLoading to rely only on query flags (isLoading/isFetching) and modify
the data effect to merge updates instead of resetting local teams/members on
background refetches; reference the variables isInitialized, setIsInitialized,
showLoading, teams, members, and the data-handling useEffect when making the
change.
---
Nitpick comments:
In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx:
- Around line 272-283: Replace the magic numbers for panel dimensions by
extracting them into shared constants or deriving them from the modal component:
remove the inline panelWidth = 426 and panelHeight = 716 and instead reference a
single exported constant (e.g., MEETING_CHAT_MODAL_WIDTH and
MEETING_CHAT_MODAL_HEIGHT) or a getter on MeetingChatModal so the nextX/nextY
calculations use those symbols; update the references in the drag logic that use
dragRef.current.originX and originY to use the new constants to keep sizing
consistent across the component and the modal.
- Around line 189-199: The ChatTeam type and related uses should use number for
teamId to avoid String/Number coercion: update the ChatTeam interface (and any
props in ChatTeamSelectModal) to declare teamId: number, change the
chatSelectableTeams mapping in useMemo to assign teamId: team.teamId (no
String()), and update all comparisons/filters (e.g., the filter that compares
team.teamId to myTeamId and any places expecting string ids) to use numeric
comparisons (no Number()). Also update any state, prop types, and event handlers
that consume ChatTeam.teamId to accept a number.
In `@src/components/base-ui/Bookcase/ChatTeamSelectModal.tsx`:
- Around line 5-9: Change the ChatTeam.teamId type to number (instead of string)
so it aligns with MeetingChatModal's selectedTeamId and avoids
Number(team.teamId) coercion; update the ChatTeam type definition (ChatTeam) and
any places constructing/consuming team objects to use numeric IDs, and remove
the implicit Number(...) conversion in MeetingChatModal where it compares
selectedTeamId to team.teamId.
In `@src/components/base-ui/Bookcase/MeetingChatModal.tsx`:
- Around line 100-136: The effect recreates the IntersectionObserver frequently
because it depends on changing booleans (chatsQuery.hasNextPage,
chatsQuery.isFetchingNextPage) and fetchNextPage; extract stable callback refs
so the observer only depends on stable values: create refs (e.g.,
hasNextPageRef, isFetchingNextPageRef, fetchNextPageRef) and update them in a
small effect when chatsQuery changes, then use a single stable intersection
handler (handleIntersect) that reads those refs and calls
fetchNextPageRef.current(); keep the observer effect dependent only on isOpen,
selectedTeamId, topSentinelRef, and scrollRef so it doesn't teardown/recreate on
every flag flip.
- Around line 39-50: The sort comparator in dedupeAndSortMessages contains a
redundant branch: after deduplication by messageId (Map keyed on item.messageId)
IDs are unique so the if (a.messageId !== b.messageId) check and the sendAt
fallback never runs; simplify the comparator in dedupeAndSortMessages to sort
only by messageId (e.g., return a.messageId - b.messageId) or, if you prefer to
keep a deterministic fallback, use a single expression that compares messageId
then sendAt when equal (but the equality case should be unreachable). Update the
comparator in dedupeAndSortMessages to remove the unreachable condition while
keeping map, messageId and sendAt references intact.
In `@src/hooks/mutations/useMeetingMutations.ts`:
- Around line 18-39: The block currently calls qc.invalidateQueries and
qc.prefetchQuery for the same keys (detailKey, membersKey) causing duplicate
fetches; pick one strategy—here remove the qc.prefetchQuery calls and only await
the two qc.invalidateQueries for detailKey and membersKey so React Query will
handle background refetching for active queries; update the Promise.all to only
include qc.invalidateQueries({ queryKey: detailKey }) and qc.invalidateQueries({
queryKey: membersKey }) and delete the prefetchQuery calls that call
meetingService.getMeetingDetail / getMeetingMembers using vars.clubId and
vars.meetingId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f787f25-e036-41f4-ab72-2f3862ab40c6
📒 Files selected for processing (18)
src/app/groups/[id]/admin/bookcase/[meetingId]/page.tsxsrc/app/groups/[id]/bookcase/[bookId]/DebateSection.tsxsrc/app/groups/[id]/bookcase/[bookId]/ReviewSection.tsxsrc/app/groups/[id]/bookcase/[bookId]/meeting/page.tsxsrc/app/groups/[id]/bookcase/[bookId]/page.tsxsrc/components/base-ui/Bookcase/Admin/bookdetailgrouping/MemberPool.tsxsrc/components/base-ui/Bookcase/ChatTeamSelectModal.tsxsrc/components/base-ui/Bookcase/MeetingChatModal.tsxsrc/components/base-ui/Bookcase/MeetingTeamMemberPopover.tsxsrc/hooks/mutations/useMeetingMutations.tssrc/hooks/queries/useMeetingChatQueries.tssrc/hooks/queries/useMeetingTopicQueries.tssrc/lib/api/endpoints/Clubs.tssrc/services/meetingChatService.tssrc/services/meetingTopicService.tssrc/types/groups/meetingChats.tssrc/types/groups/meetingDetail.tssrc/types/groups/meetingTopics.ts
|
|
||
| const selectedTeamName = selectedTeam?.teamName ?? ""; | ||
| const teamNames = useMemo(() => teams.map((t) => t.teamName), [teams]); | ||
| const [chatPosition, setChatPosition] = useState({ x: 980, y: 56 }); |
There was a problem hiding this comment.
Hardcoded initial position may place the chat modal off-screen on narrower desktops.
The initial x: 980 combined with a panel width of 426px means the modal requires at least ~1406px viewport width to be fully visible. On smaller desktop screens, the modal will start partially off-screen.
Consider initializing based on viewport dimensions or clamping on first render.
💡 Suggested approach
- const [chatPosition, setChatPosition] = useState({ x: 980, y: 56 });
+ const [chatPosition, setChatPosition] = useState(() => {
+ if (typeof window === "undefined") return { x: 980, y: 56 };
+ const panelWidth = 426;
+ const x = Math.min(980, window.innerWidth - panelWidth - 16);
+ return { x: Math.max(0, x), y: 56 };
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [chatPosition, setChatPosition] = useState({ x: 980, y: 56 }); | |
| const [chatPosition, setChatPosition] = useState(() => { | |
| if (typeof window === "undefined") return { x: 980, y: 56 }; | |
| const panelWidth = 426; | |
| const x = Math.min(980, window.innerWidth - panelWidth - 16); | |
| return { x: Math.max(0, x), y: 56 }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx at line 77, The
initial chatPosition ({ x: 980, y: 56 }) can place the modal off-screen on
narrower viewports; update the initialization of chatPosition in the useState
call so x is computed from the current viewport width (e.g., window.innerWidth)
minus the panel width and a margin, then clamp x to a minimum value so it never
starts off-screen; implement this either by using a lazy initializer for
useState that reads window.innerWidth (guarded for SSR) or by setting
chatPosition on first render in a useEffect that calls setChatPosition,
referencing the chatPosition and setChatPosition identifiers and ensuring the
panel width (426) and a minimal margin are used when computing/clamping x.
Comment on lines
+95
to
+107
| const handleConfirmYes = async () => { | ||
| if (isSubmitting) return; | ||
|
|
||
| try { | ||
| setIsSubmitting(true); | ||
| await onSubmit(); | ||
| setIsConfirmOpen(false); | ||
| router.back(); | ||
| } catch (error) { | ||
| console.error(error); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } |
There was a problem hiding this comment.
Block all modal-close paths while submitting.
Line 173 and Line 195 still allow closing during in-flight submit. That can hide ongoing work and then navigate unexpectedly when router.back() runs after completion.
Suggested fix
@@
- <div
- className="fixed inset-0 z-40 bg-black/50 backdrop-blur-xs px-4 py-4"
- onClick={() => setIsConfirmOpen(false)}
- />
+ <div
+ className="fixed inset-0 z-40 bg-black/50 backdrop-blur-xs px-4 py-4"
+ onClick={() => {
+ if (!isSubmitting) setIsConfirmOpen(false);
+ }}
+ />
@@
<button
type="button"
onClick={() => setIsConfirmOpen(false)}
+ disabled={isSubmitting}
className={[
"flex-1 h-[44px] rounded-[8px]",
"border border-Subbrown-4",
"text-Gray-7 body_1_2",
"cursor-pointer hover:brightness-90 active:brightness-90",
+ isSubmitting ? "opacity-60 cursor-not-allowed" : "",
].join(" ")}
>Also applies to: 183-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/base-ui/Bookcase/Admin/bookdetailgrouping/MemberPool.tsx`
around lines 95 - 107, The submit flow allows modal closes while an in-flight
submit is running; fix by blocking every modal-close path while isSubmitting is
true: in handleConfirmYes keep setIsSubmitting/use await as-is but ensure all
close handlers (e.g., onCancel, onClose, any handleClose or close-button
handlers) early-return when isSubmitting and do not call setIsConfirmOpen(false)
or router.back; also disable backdrop/escape-close via the modal props while
isSubmitting and only call router.back after onSubmit resolves and when not
interrupted. Apply the same guard to the other close handlers referenced (the
handlers around the other modal-close sites in the file).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/groups/[id]/bookcase/[bookId]/meeting/page.tsx (1)
77-77:⚠️ Potential issue | 🟡 MinorHardcoded initial chat position can still start off-screen.
Line 77 still initializes
chatPosition.xto a fixed980, so narrower desktop widths can render the modal partially outside the viewport on first open.Suggested fix
- const [chatPosition, setChatPosition] = useState({ x: 980, y: 56 }); + const [chatPosition, setChatPosition] = useState(() => { + if (typeof window === "undefined") return { x: 980, y: 56 }; + const panelWidth = 426; + const x = Math.min(980, window.innerWidth - panelWidth - 16); + return { x: Math.max(0, x), y: 56 }; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx at line 77, The initial chatPosition is hardcoded to x:980 which can place the modal off-screen on narrow viewports; update the initialization to compute a safe x on the client (or defer to client) and clamp it inside the viewport: in page.tsx replace the hardcoded useState({ x: 980, y: 56 }) logic for chatPosition/setChatPosition with a client-aware approach (e.g., initialize to a neutral value or 0 and in a useEffect run on mount compute window.innerWidth, subtract the modal width and a margin, then setChatPosition({ x: Math.max(minX, Math.min(desiredX, window.innerWidth - modalWidth - margin)), y: 56 }) so the modal always starts fully visible; reference the chatPosition and setChatPosition state and the useEffect that runs after mount to perform this clamping.
🤖 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/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx:
- Around line 133-138: The current parsing uses
Number(searchParams.get("teamId")) which converts null to 0 and therefore can't
distinguish "missing" from an actual 0 teamId; change the logic to first read
the raw param (searchParams.get("teamId")) and only convert to Number when it's
non-null, then guard matchedById using a finite-number check (e.g., ensure
teamIdFromQuery is not undefined and Number.isFinite(teamIdFromQuery)) before
calling teams.find; update the identifiers teamIdFromQuery, matchedById, and the
searchParams.get("teamId") usage accordingly.
- Around line 110-123: myTeamId currently picks a team by matching
user?.nickname against meetingData.teamMembers which is ambiguous if nicknames
are duplicated; change the logic in the myTeamId useMemo to fail-closed: compute
all team matches for the trimmed myNickname across meetingData.teamMembers (use
group.members/member.memberInfo.nickname), and if zero matches return null, if
exactly one return that group.teamKey.teamId, but if more than one (duplicate
nickname across teams) return null to avoid misattributing team membership until
a stable member id is available.
---
Duplicate comments:
In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx:
- Line 77: The initial chatPosition is hardcoded to x:980 which can place the
modal off-screen on narrow viewports; update the initialization to compute a
safe x on the client (or defer to client) and clamp it inside the viewport: in
page.tsx replace the hardcoded useState({ x: 980, y: 56 }) logic for
chatPosition/setChatPosition with a client-aware approach (e.g., initialize to a
neutral value or 0 and in a useEffect run on mount compute window.innerWidth,
subtract the modal width and a margin, then setChatPosition({ x: Math.max(minX,
Math.min(desiredX, window.innerWidth - modalWidth - margin)), y: 56 }) so the
modal always starts fully visible; reference the chatPosition and
setChatPosition state and the useEffect that runs after mount to perform this
clamping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e80ec19-d6cf-4ce8-994a-a3ac2ccc9982
📒 Files selected for processing (1)
src/app/groups/[id]/bookcase/[bookId]/meeting/page.tsx
Comment on lines
+110
to
+123
| const myTeamId = useMemo(() => { | ||
| // TODO: | ||
| // 현재 응답에는 "내 clubMemberId"가 없어서 nickname 기반 임시 판별만 가능. | ||
| // 추후 BE가 내 clubMemberId를 내려주면 그 값 기준으로 교체할 것. | ||
| const myNickname = user?.nickname?.trim(); | ||
| if (!myNickname || !meetingData) return null; | ||
|
|
||
| for (const group of meetingData.teamMembers ?? []) { | ||
| const matched = group.members.some( | ||
| (member) => member.memberInfo.nickname === myNickname | ||
| ); | ||
| if (matched) return group.teamKey.teamId; | ||
| } | ||
|
|
There was a problem hiding this comment.
myTeamId nickname matching is ambiguous and can resolve to the wrong team.
At Line 119, nickname-only matching can produce incorrect team identity when nicknames are duplicated, which then affects topic-action gating and chat team filtering.
Safer fail-closed fallback (until immutable member id is available)
- for (const group of meetingData.teamMembers ?? []) {
- const matched = group.members.some(
- (member) => member.memberInfo.nickname === myNickname
- );
- if (matched) return group.teamKey.teamId;
- }
-
- return null;
+ const matchedTeamIds = (meetingData.teamMembers ?? [])
+ .filter((group) =>
+ group.members.some((member) => member.memberInfo.nickname?.trim() === myNickname)
+ )
+ .map((group) => group.teamKey.teamId);
+
+ return matchedTeamIds.length === 1 ? matchedTeamIds[0] : null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const myTeamId = useMemo(() => { | |
| // TODO: | |
| // 현재 응답에는 "내 clubMemberId"가 없어서 nickname 기반 임시 판별만 가능. | |
| // 추후 BE가 내 clubMemberId를 내려주면 그 값 기준으로 교체할 것. | |
| const myNickname = user?.nickname?.trim(); | |
| if (!myNickname || !meetingData) return null; | |
| for (const group of meetingData.teamMembers ?? []) { | |
| const matched = group.members.some( | |
| (member) => member.memberInfo.nickname === myNickname | |
| ); | |
| if (matched) return group.teamKey.teamId; | |
| } | |
| const myTeamId = useMemo(() => { | |
| // TODO: | |
| // 현재 응답에는 "내 clubMemberId"가 없어서 nickname 기반 임시 판별만 가능. | |
| // 추후 BE가 내 clubMemberId를 내려주면 그 값 기준으로 교체할 것. | |
| const myNickname = user?.nickname?.trim(); | |
| if (!myNickname || !meetingData) return null; | |
| const matchedTeamIds = (meetingData.teamMembers ?? []) | |
| .filter((group) => | |
| group.members.some((member) => member.memberInfo.nickname?.trim() === myNickname) | |
| ) | |
| .map((group) => group.teamKey.teamId); | |
| return matchedTeamIds.length === 1 ? matchedTeamIds[0] : null; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx around lines 110 -
123, myTeamId currently picks a team by matching user?.nickname against
meetingData.teamMembers which is ambiguous if nicknames are duplicated; change
the logic in the myTeamId useMemo to fail-closed: compute all team matches for
the trimmed myNickname across meetingData.teamMembers (use
group.members/member.memberInfo.nickname), and if zero matches return null, if
exactly one return that group.teamKey.teamId, but if more than one (duplicate
nickname across teams) return null to avoid misattributing team membership until
a stable member id is available.
Comment on lines
+133
to
+138
| const teamIdFromQuery = Number(searchParams.get("teamId")); | ||
| const teamLabelFromQuery = searchParams.get("team"); | ||
|
|
||
| setTeams(res); | ||
| const matchedById = Number.isFinite(teamIdFromQuery) | ||
| ? teams.find((team) => team.teamId === teamIdFromQuery) | ||
| : undefined; |
There was a problem hiding this comment.
teamId query parsing treats missing value as 0.
At Line 133, Number(searchParams.get("teamId")) converts null to 0, so “no teamId in URL” is not clearly distinguished from an actual numeric value.
Suggested fix
- const teamIdFromQuery = Number(searchParams.get("teamId"));
+ const rawTeamId = searchParams.get("teamId");
+ const teamIdFromQuery = rawTeamId === null ? Number.NaN : Number(rawTeamId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const teamIdFromQuery = Number(searchParams.get("teamId")); | |
| const teamLabelFromQuery = searchParams.get("team"); | |
| setTeams(res); | |
| const matchedById = Number.isFinite(teamIdFromQuery) | |
| ? teams.find((team) => team.teamId === teamIdFromQuery) | |
| : undefined; | |
| const rawTeamId = searchParams.get("teamId"); | |
| const teamIdFromQuery = rawTeamId === null ? Number.NaN : Number(rawTeamId); | |
| const teamLabelFromQuery = searchParams.get("team"); | |
| const matchedById = Number.isFinite(teamIdFromQuery) | |
| ? teams.find((team) => team.teamId === teamIdFromQuery) | |
| : undefined; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/groups/`[id]/bookcase/[bookId]/meeting/page.tsx around lines 133 -
138, The current parsing uses Number(searchParams.get("teamId")) which converts
null to 0 and therefore can't distinguish "missing" from an actual 0 teamId;
change the logic to first read the raw param (searchParams.get("teamId")) and
only convert to Number when it's non-null, then guard matchedById using a
finite-number check (e.g., ensure teamIdFromQuery is not undefined and
Number.isFinite(teamIdFromQuery)) before calling teams.find; update the
identifiers teamIdFromQuery, matchedById, and the searchParams.get("teamId")
usage accordingly.
This branch was successfully deployed
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
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.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
상세 변경 내용
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
New Features
Improvements