Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Summary of ChangesHello @shinwokkang, 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! 이 풀 리퀘스트는 사용자 참여를 증진시키기 위한 핵심적인 상호작용 기능인 북 스토리 좋아요 및 회원 팔로우 기능을 시스템에 통합합니다. 사용자가 콘텐츠와 다른 사용자에게 더 쉽게 반응할 수 있도록 하여 플랫폼의 소셜 요소를 강화하고, 즉각적인 피드백을 제공하는 낙관적 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. Changelog
Activity
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}
There was a problem hiding this comment.
Code Review
This pull request introduces API integration and UI changes for adding like and follow features. While the implementation generally utilizes react-query's useMutation for optimistic updates and API request throttling effectively, a high-severity path traversal vulnerability was identified. Specifically, the nickname parameter in the member follow endpoint is used in a URL path without proper encoding, which could allow an attacker to manipulate API requests. It is recommended to use encodeURIComponent for all dynamic path parameters. Furthermore, there are opportunities to improve type safety and reduce code duplication within the onMutate logic of the mutation hook.
| RECOMMEND: `${API_BASE_URL}/members/me/recommend`, | ||
| UPDATE_PROFILE: `${API_BASE_URL}/members/me`, | ||
| UPDATE_PASSWORD: `${API_BASE_URL}/members/me/update-password`, | ||
| FOLLOW: (nickname: string) => `${API_BASE_URL}/members/${nickname}/following`, |
There was a problem hiding this comment.
The FOLLOW endpoint constructor directly concatenates the nickname parameter into the URL path without any encoding or sanitization. If a user's nickname contains path traversal sequences like ../, the resulting URL will point to an unintended endpoint. For example, a nickname of ../me/update-password would result in a POST request to ${API_BASE_URL}/members/../me/update-password/following, which resolves to ${API_BASE_URL}/me/update-password/following. This could allow an attacker to trigger actions on different API endpoints by manipulating their nickname.
| FOLLOW: (nickname: string) => `${API_BASE_URL}/members/${nickname}/following`, | |
| FOLLOW: (nickname: string) => `${API_BASE_URL}/members/${encodeURIComponent(nickname)}/following`, |
| <div | ||
| key={story.bookStoryId} | ||
| onClick={() => handleCardClick(story.bookStoryId)} | ||
| className="cursor-pointer shrink-0" |
There was a problem hiding this comment.
BookStoryCardLarge 컴포넌트 자체에 cursor-pointer 스타일이 적용되어 있으므로, 이 div의 cursor-pointer 클래스는 중복됩니다. 코드를 더 깔끔하게 유지하기 위해 제거하는 것이 좋습니다.
| className="cursor-pointer shrink-0" | |
| className="shrink-0" |
| <div | ||
| key={story.bookStoryId} | ||
| onClick={() => handleCardClick(story.bookStoryId)} | ||
| className="cursor-pointer shrink-0" |
There was a problem hiding this comment.
BookStoryCardLarge 컴포넌트 자체에 cursor-pointer 스타일이 적용되어 있으므로, 이 div의 cursor-pointer 클래스는 중복됩니다. 코드를 더 깔끔하게 유지하기 위해 제거하는 것이 좋습니다.
| className="cursor-pointer shrink-0" | |
| className="shrink-0" |
| onMutate: async ({ nickname, isFollowing }) => { | ||
| // Cancel outgoing refetches | ||
| await queryClient.cancelQueries({ queryKey: storyKeys.all }); | ||
| await queryClient.cancelQueries({ queryKey: memberKeys.recommended() }); | ||
|
|
||
| // Snapshot previous values | ||
| const previousRecommendations = queryClient.getQueryData(memberKeys.recommended()); | ||
| const previousInfiniteStories = queryClient.getQueryData(storyKeys.infiniteList()); | ||
| const previousStories = queryClient.getQueryData(storyKeys.list()); | ||
|
|
||
| // 1. Optimistically update recommendations | ||
| if (previousRecommendations) { | ||
| queryClient.setQueryData(memberKeys.recommended(), (old: any) => { | ||
| if (!old || !old.friends) return old; | ||
| return { | ||
| ...old, | ||
| friends: old.friends.map((member: any) => | ||
| member.nickname === nickname | ||
| ? { ...member, isFollowing: !isFollowing } | ||
| : member | ||
| ) | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // 2. Optimistically update infinite stories | ||
| if (previousInfiniteStories) { | ||
| queryClient.setQueryData(storyKeys.infiniteList(), (old: any) => { | ||
| if (!old || !old.pages) return old; | ||
| return { | ||
| ...old, | ||
| pages: old.pages.map((page: any) => ({ | ||
| ...page, | ||
| basicInfoList: page.basicInfoList.map((story: any) => | ||
| story.authorInfo.nickname === nickname | ||
| ? { ...story, authorInfo: { ...story.authorInfo, following: !isFollowing } } | ||
| : story | ||
| ) | ||
| })) | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // 3. Optimistically update stories list | ||
| if (previousStories) { | ||
| queryClient.setQueryData(storyKeys.list(), (old: any) => { | ||
| if (!old || !old.basicInfoList) return old; | ||
| return { | ||
| ...old, | ||
| basicInfoList: old.basicInfoList.map((story: any) => | ||
| story.authorInfo.nickname === nickname | ||
| ? { ...story, authorInfo: { ...story.authorInfo, following: !isFollowing } } | ||
| : story | ||
| ) | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| return { previousRecommendations, previousInfiniteStories, previousStories }; | ||
| }, |
There was a problem hiding this comment.
낙관적 업데이트 로직의 타입 안정성과 유지보수성을 높이는 것을 제안합니다.
-
타입 명시: 현재
onMutate콜백에서old데이터와map내부의member,story등이any타입으로 되어 있습니다.@tanstack/react-query에서 제공하는InfiniteData와 직접 정의하신RecommendResponse,BookStoryListResponse등의 타입을 명시적으로 사용하면 타입 안정성을 높이고 예기치 않은 오류를 방지할 수 있습니다. -
로직 중복 제거: 여러 쿼리 캐시(
recommendations,infinite stories,stories list)를 업데이트하는 로직이 중복되고 있습니다. 이 로직을 별도의 헬퍼 함수로 추출하면 코드가 더 간결해지고 유지보수가 용이해집니다.
아래는 개선 예시입니다. (BookStory 등 필요한 타입을 types 디렉토리에서 가져와야 할 수 있습니다.)
onMutate: async ({ nickname, isFollowing }) => {
// 필요한 타입을 import 해주세요.
// import { InfiniteData } from "@tanstack/react-query";
// import { BookStory, BookStoryListResponse } from "@/types/story";
// import { RecommendResponse } from "@/types/member";
// Cancel outgoing refetches
await queryClient.cancelQueries({ queryKey: storyKeys.all });
await queryClient.cancelQueries({ queryKey: memberKeys.recommended() });
// Snapshot previous values
const previousRecommendations = queryClient.getQueryData<RecommendResponse>(memberKeys.recommended());
const previousInfiniteStories = queryClient.getQueryData<InfiniteData<BookStoryListResponse>>(storyKeys.infiniteList());
const previousStories = queryClient.getQueryData<BookStoryListResponse>(storyKeys.list());
const updateAuthorInStory = (story: any) => {
if (story.authorInfo.nickname === nickname) {
return {
...story,
authorInfo: { ...story.authorInfo, following: !isFollowing },
};
}
return story;
};
// 1. Optimistically update recommendations
if (previousRecommendations) {
queryClient.setQueryData<RecommendResponse>(memberKeys.recommended(), (old) => {
if (!old || !old.friends) return old;
return {
...old,
friends: old.friends.map((member) =>
member.nickname === nickname
? { ...member, isFollowing: !isFollowing }
: member
),
};
});
}
// 2. Optimistically update infinite stories
if (previousInfiniteStories) {
queryClient.setQueryData<InfiniteData<BookStoryListResponse>>(storyKeys.infiniteList(), (old) => {
if (!old || !old.pages) return old;
return {
...old,
pages: old.pages.map((page) => ({
...page,
basicInfoList: page.basicInfoList.map(updateAuthorInStory),
})),
};
});
}
// 3. Optimistically update stories list
if (previousStories) {
queryClient.setQueryData<BookStoryListResponse>(storyKeys.list(), (old) => {
if (!old || !old.basicInfoList) return old;
return {
...old,
basicInfoList: old.basicInfoList.map(updateAuthorInStory),
};
});
}
return { previousRecommendations, previousInfiniteStories, previousStories };
},| onMutate: async (bookStoryId) => { | ||
| // Cancel any outgoing refetches | ||
| await queryClient.cancelQueries({ queryKey: storyKeys.all }); | ||
|
|
||
| // Snapshot the previous values | ||
| const previousInfiniteStories = queryClient.getQueryData(storyKeys.infiniteList()); | ||
| const previousMyStories = queryClient.getQueryData(storyKeys.myList()); | ||
| const previousStories = queryClient.getQueryData(storyKeys.list()); | ||
| const previousStoryDetail = queryClient.getQueryData(storyKeys.detail(bookStoryId)); | ||
|
|
||
| // Optimistically update the infinite list | ||
| if (previousInfiniteStories) { | ||
| queryClient.setQueryData(storyKeys.infiniteList(), (old: any) => { | ||
| if (!old) return old; | ||
| return { | ||
| ...old, | ||
| pages: old.pages.map((page: any) => ({ | ||
| ...page, | ||
| basicInfoList: page.basicInfoList.map((story: any) => { | ||
| if (story.bookStoryId === bookStoryId) { | ||
| const nextLiked = !story.likedByMe; | ||
| return { | ||
| ...story, | ||
| likedByMe: nextLiked, | ||
| likes: nextLiked ? story.likes + 1 : story.likes - 1, | ||
| }; | ||
| } | ||
| return story; | ||
| }), | ||
| })), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // Optimistically update my stories | ||
| if (previousMyStories) { | ||
| queryClient.setQueryData(storyKeys.myList(), (old: any) => { | ||
| if (!old) return old; | ||
| return { | ||
| ...old, | ||
| pages: old.pages.map((page: any) => ({ | ||
| ...page, | ||
| basicInfoList: page.basicInfoList.map((story: any) => { | ||
| if (story.bookStoryId === bookStoryId) { | ||
| const nextLiked = !story.likedByMe; | ||
| return { | ||
| ...story, | ||
| likedByMe: nextLiked, | ||
| likes: nextLiked ? story.likes + 1 : story.likes - 1, | ||
| }; | ||
| } | ||
| return story; | ||
| }), | ||
| })), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // Optimistically update the regular list (if used) | ||
| if (previousStories) { | ||
| queryClient.setQueryData(storyKeys.list(), (old: any) => { | ||
| if (!old) return old; | ||
| return { | ||
| ...old, | ||
| basicInfoList: old.basicInfoList.map((story: any) => { | ||
| if (story.bookStoryId === bookStoryId) { | ||
| const nextLiked = !story.likedByMe; | ||
| return { | ||
| ...story, | ||
| likedByMe: nextLiked, | ||
| likes: nextLiked ? story.likes + 1 : story.likes - 1, | ||
| }; | ||
| } | ||
| return story; | ||
| }), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // Optimistically update the detail view | ||
| if (previousStoryDetail) { | ||
| queryClient.setQueryData(storyKeys.detail(bookStoryId), (old: any) => { | ||
| if (!old) return old; | ||
| const nextLiked = !old.likedByMe; | ||
| return { | ||
| ...old, | ||
| likedByMe: nextLiked, | ||
| likes: nextLiked ? old.likes + 1 : old.likes - 1, | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| previousInfiniteStories, | ||
| previousMyStories, | ||
| previousStories, | ||
| previousStoryDetail, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
이 뮤테이션의 onMutate 로직도 useToggleFollowMutation과 유사하게 타입 안정성 및 코드 중복 관점에서 개선할 수 있습니다.
- 타입 명시:
old데이터와story객체에any대신 구체적인 타입을 지정하여 타입 안정성을 강화할 수 있습니다. - 로직 중복 제거: 여러 종류의 스토리 목록(무한 스크롤, 내 스토리, 일반 목록)과 상세 정보에 대한 낙관적 업데이트 로직이 중복됩니다. 좋아요 상태를 토글하는 로직을 헬퍼 함수로 분리하면 코드를 더 깔끔하게 만들 수 있습니다.
아래는 개선 예시입니다. (BookStory 등 필요한 타입을 types 디렉토리에서 가져와야 할 수 있습니다.)
onMutate: async (bookStoryId) => {
// 필요한 타입을 import 해주세요.
// import { InfiniteData } from "@tanstack/react-query";
// import { BookStory, BookStoryListResponse, BookStoryDetail } from "@/types/story";
// Cancel any outgoing refetches
await queryClient.cancelQueries({ queryKey: storyKeys.all });
// Snapshot the previous values
const previousInfiniteStories = queryClient.getQueryData<InfiniteData<any>>(storyKeys.infiniteList());
const previousMyStories = queryClient.getQueryData<InfiniteData<any>>(storyKeys.myList());
const previousStories = queryClient.getQueryData<any>(storyKeys.list());
const previousStoryDetail = queryClient.getQueryData<any>(storyKeys.detail(bookStoryId));
const updateStoryLike = (story: any) => {
const nextLiked = !story.likedByMe;
return {
...story,
likedByMe: nextLiked,
likes: nextLiked ? story.likes + 1 : story.likes - 1,
};
};
const updateStoriesInPage = (page: any) => ({
...page,
basicInfoList: page.basicInfoList.map((story: any) =>
story.bookStoryId === bookStoryId ? updateStoryLike(story) : story
),
});
// Optimistically update the infinite list
if (previousInfiniteStories) {
queryClient.setQueryData<InfiniteData<any>>(storyKeys.infiniteList(), (old) => {
if (!old) return old;
return {
...old,
pages: old.pages.map(updateStoriesInPage),
};
});
}
// Optimistically update my stories
if (previousMyStories) {
queryClient.setQueryData<InfiniteData<any>>(storyKeys.myList(), (old) => {
if (!old) return old;
return {
...old,
pages: old.pages.map(updateStoriesInPage),
};
});
}
// Optimistically update the regular list (if used)
if (previousStories) {
queryClient.setQueryData<any>(storyKeys.list(), (old) => {
if (!old) return old;
return {
...old,
basicInfoList: old.basicInfoList.map((story: any) =>
story.bookStoryId === bookStoryId ? updateStoryLike(story) : story
),
};
});
}
// Optimistically update the detail view
if (previousStoryDetail) {
queryClient.setQueryData<any>(storyKeys.detail(bookStoryId), (old) => {
if (!old) return old;
return updateStoryLike(old);
});
}
return {
previousInfiniteStories,
previousMyStories,
previousStories,
previousStoryDetail,
};
},1 participant
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)