Closed
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
[feat] 책 이야기 상세 조회-댓글 조회
fix : build 문제 진짜 해결
4 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (86)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive data fetching and integration layer, replacing static dummy data with real API calls. It adds React Query infrastructure, multiple API services, data transformation hooks, drag-and-drop team management, infinite scrolling pagination, and client-side API client with error handling across numerous pages and components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 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 |
Contributor
Summary of ChangesHello @hongik-luke, 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! This pull request significantly advances the application's functionality by integrating numerous backend APIs, moving away from dummy data for core features. It focuses on enhancing data-driven interactions, improving user experience through dynamic content loading and interactive elements, and laying robust groundwork for future group-related features and content management. 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. Changelog
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 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
This pull request introduces significant improvements to the application's data fetching and state management by integrating TanStack Query for various data operations, including book searches, story listings, and club management. It also refactors several components to use API-driven data instead of dummy data, enhancing the application's functionality and responsiveness. New features include infinite scrolling for search results and stories, a new club creation wizard, and team management for book club meetings with drag-and-drop functionality. Several utility functions for time formatting, URL validation, and data mapping have been added, contributing to a more robust and maintainable codebase. However, there are a few areas that require attention, particularly regarding code duplication in API client implementations and hardcoded values.
Comment on lines
+1
to
+105
| "use client"; | ||
|
|
||
| import { useAuthStore } from "@/store/useAuthStore"; | ||
| import toast from "react-hot-toast"; | ||
| import { getErrorMessage, ApiError } from "../errors"; | ||
|
|
||
| interface RequestOptions extends RequestInit { | ||
| headers?: Record<string, string>; | ||
| params?: Record<string, any>; | ||
| timeout?: number; // Timeout in ms (default: 10000) | ||
| } | ||
|
|
||
| async function request<T>( | ||
| url: string, | ||
| options: RequestOptions = {} | ||
| ): Promise<T> { | ||
| const { params, timeout = 10000, ...fetchOptions } = options; | ||
|
|
||
| const defaultHeaders: Record<string, string> = { | ||
| "Content-Type": "application/json", | ||
| }; | ||
|
|
||
| // [Utility] Query String Builder | ||
| let requestUrl = url; | ||
| if (params) { | ||
| const searchParams = new URLSearchParams(); | ||
| Object.entries(params).forEach(([key, value]) => { | ||
| if (value !== undefined && value !== null) { | ||
| searchParams.append(key, String(value)); | ||
| } | ||
| }); | ||
| requestUrl += `?${searchParams.toString()}`; | ||
| } | ||
|
|
||
| // [Resilience] Timeout Controller | ||
| const controller = new AbortController(); | ||
| const id = setTimeout(() => controller.abort(), timeout); | ||
|
|
||
| const config: RequestInit = { | ||
| ...fetchOptions, | ||
| // [Security] Include credentials (cookies) for all requests | ||
| credentials: "include", | ||
| headers: { | ||
| ...defaultHeaders, | ||
| ...options.headers, | ||
| }, | ||
| signal: controller.signal, | ||
| }; | ||
|
|
||
| try { | ||
| const response = await fetch(requestUrl, config); | ||
| clearTimeout(id); | ||
|
|
||
| // [Resilience] Interceptor: 401 Unauthorized Handling | ||
| if (response.status === 401) { | ||
| console.warn("Session expired. Logging out..."); | ||
| useAuthStore.getState().logout(); | ||
| toast.error("세션이 만료되었습니다. 다시 로그인해주세요."); | ||
| } | ||
|
|
||
| // [Resilience] Safe JSON Parsing | ||
| let data: any; | ||
| const contentType = response.headers.get("content-type"); | ||
| if (contentType && contentType.includes("application/json")) { | ||
| data = await response.json(); | ||
| } else { | ||
| data = { | ||
| isSuccess: false, | ||
| message: "서버 응답 형식이 올바르지 않습니다.", | ||
| }; | ||
| } | ||
|
|
||
| // [Standardization] Response Normalization | ||
| if (!response.ok || (data && data.isSuccess === false)) { | ||
| const errorCode = data?.code || `HTTP${response.status}`; | ||
| const errorMessage = | ||
| data?.message || | ||
| getErrorMessage(errorCode) || | ||
| "요청 처리 중 오류가 발생했습니다."; | ||
|
|
||
| throw new ApiError(errorMessage, errorCode, data); | ||
| } | ||
|
|
||
| return data; | ||
| } catch (error) { | ||
| clearTimeout(id); | ||
| console.error("API Request Error:", error); | ||
| if (error instanceof DOMException && error.name === "AbortError") { | ||
| toast.error("요청 시간이 초과되었습니다."); | ||
| throw new Error("Request timeout"); | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| export const apiClient = { | ||
| get: <T>(url: string, options?: RequestOptions) => | ||
| request<T>(url, { ...options, method: "GET" }), | ||
| post: <T>(url: string, body?: any, options?: RequestOptions) => | ||
| request<T>(url, { ...options, method: "POST", body: body ? JSON.stringify(body) : undefined }), | ||
| put: <T>(url: string, body?: any, options?: RequestOptions) => | ||
| request<T>(url, { ...options, method: "PUT", body: body ? JSON.stringify(body) : undefined }), | ||
| delete: <T>(url: string, options?: RequestOptions) => | ||
| request<T>(url, { ...options, method: "DELETE" }), | ||
| }; |
Contributor
There was a problem hiding this comment.
There are two nearly identical API client implementations: src/lib/api/client.ts and src/lib/api/client/index.ts. This code duplication is a critical issue as it leads to inconsistencies (e.g., src/lib/api/client/index.ts is missing the patch method) and makes maintenance significantly harder. These files should be merged into a single, canonical API client implementation.
| subscribingCount={u.subscribingCount} | ||
| subscribersCount={u.subscribersCount} | ||
| onSubscribeClick={() => console.log("subscribe", u.id)} | ||
| key={u.nickname + idx} |
Contributor
There was a problem hiding this comment.
Using u.nickname + idx as a key can lead to issues if nicknames are not guaranteed to be unique and the list order changes. It's generally safer to use a unique identifier from the user object if available, or a more robust method to generate unique keys if nickname can be duplicated and idx is not stable across renders.
| imageUrl={isValidUrl(story.bookInfo.imgUrl) ? story.bookInfo.imgUrl : "/book_example.svg"} | ||
| authorName={story.authorInfo.nickname} | ||
| authorNickname={story.authorInfo.nickname} | ||
| authorId={story.authorInfo.nickname} |
Contributor
There was a problem hiding this comment.
The authorId prop in BookstoryDetail is currently being passed story.authorInfo.nickname. If authorId is intended to be a unique identifier for the author (e.g., a numeric ID from the backend), using a nickname might lead to unexpected behavior or incorrect data associations if nicknames are not guaranteed to be unique. Please confirm if authorId is meant for display or for internal logic requiring a unique ID.
|
|
||
| const handleAddTeam = () => { | ||
| setTeams((prev) => { | ||
| if (prev.length >= 7) return prev; |
Contributor
There was a problem hiding this comment.
The maximum number of teams is hardcoded to 7. It would be better to define this as a named constant (e.g., MAX_TEAMS) for improved readability and easier modification in the future. This constant is already defined in src/types/groups/bookcasedetail.ts as MAX_TEAMS.
Suggested change
| if (prev.length >= 7) return prev; | |
| if (prev.length >= MAX_TEAMS) return prev; |
| <ButtonWithoutImg | ||
| text="이번 모임 바로가기" | ||
| onClick={() => router.push(joinUrl)} | ||
| onClick={() => router.push(`${Number(groupId)}/notice/4`)} |
Contributor
There was a problem hiding this comment.
Hardcoding the notice ID 4 makes the application less flexible and harder to maintain. This ID should ideally be fetched dynamically, perhaps representing the latest or most relevant meeting notice, rather than a fixed value.
Comment on lines
175
to
+182
| className={[ | ||
| 'h-[28px] t:h-[40px] px-4 py-2 w-full', | ||
| 'flex items-center justify-center', | ||
| 'rounded-[8px]', | ||
| 'bg-primary-2 text-White hover:brightness-90 cursor-pointer', | ||
| 'body_2_2 t:body_1_2', | ||
| 'mb-1', | ||
| ].join(' ')} | ||
| "h-[28px] t:h-[40px] px-4 py-2 w-full", | ||
| "flex items-center justify-center", | ||
| "rounded-[8px]", | ||
| "bg-primary-2 text-White", | ||
| "body_2_2 t:body_1_2", | ||
| "mb-1", | ||
| ].join(" ")} |
Contributor
There was a problem hiding this comment.
The hover effects (hover:brightness-90 cursor-pointer) were removed from this button. Reintroducing visual feedback for interactive elements improves user experience.
Suggested change
| className={[ | |
| 'h-[28px] t:h-[40px] px-4 py-2 w-full', | |
| 'flex items-center justify-center', | |
| 'rounded-[8px]', | |
| 'bg-primary-2 text-White hover:brightness-90 cursor-pointer', | |
| 'body_2_2 t:body_1_2', | |
| 'mb-1', | |
| ].join(' ')} | |
| "h-[28px] t:h-[40px] px-4 py-2 w-full", | |
| "flex items-center justify-center", | |
| "rounded-[8px]", | |
| "bg-primary-2 text-White", | |
| "body_2_2 t:body_1_2", | |
| "mb-1", | |
| ].join(" ")} | |
| className={[ | |
| "h-[28px] t:h-[40px] px-4 py-2 w-full", | |
| "flex items-center justify-center", | |
| "rounded-[8px]", | |
| "bg-primary-2 text-White hover:brightness-90 cursor-pointer", | |
| "body_2_2 t:body_1_2", | |
| "mb-1", | |
| ].join(" ")} |
Comment on lines
+193
to
+199
| "h-[28px] t:h-[40px] px-4 py-2 w-full", | ||
| "flex items-center justify-center gap-[10px]", | ||
| "rounded-[8px]", | ||
| "border border-primary-1", | ||
| "bg-background text-primary-3", | ||
| "body_2_2 t:body_1_2", | ||
| ].join(" ")} |
Contributor
There was a problem hiding this comment.
The hover effects (hover:brightness-95 cursor-pointer) were removed from this button. Reintroducing visual feedback for interactive elements improves user experience.
className={[
"h-[28px] t:h-[40px] px-4 py-2 w-full",
"flex items-center justify-center gap-[10px]",
"rounded-[8px]",
"border border-primary-1",
"bg-background text-primary-3 hover:brightness-95 cursor-pointer",
"body_2_2 t:body_1_2",
].join(" ")}
Comment on lines
+221
to
+223
| "mt-4 w-full h-[44px] rounded-[10px] body_1_2", | ||
| reason.trim() ? "bg-primary-2 hover:bg-primary-1 text-White" : "bg-Gray-2 text-Gray-4", | ||
| ].join(" ")} |
Contributor
There was a problem hiding this comment.
The hover effects (hover:brightness-95 cursor-pointer) were removed from this button. Reintroducing visual feedback for interactive elements improves user experience.
reason.trim() ? "bg-primary-2 hover:bg-primary-1 text-White" : "bg-Gray-2 text-Gray-4",
].join(" ")}
| onTopicClick={() => { } } | ||
| onReviewClick={() => { } } | ||
| onMeetingClick={() => { } } | ||
| imageUrl={''} /> |
Contributor
There was a problem hiding this comment.
The BookcaseCard component expects a string for imageUrl. Passing an empty string ('') is semantically an invalid URL and might lead to broken image icons or unexpected behavior if the component doesn't handle empty strings gracefully. Consider passing a valid placeholder image URL or null/undefined if the prop is optional and the component has a robust fallback.
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.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores