-
Notifications
You must be signed in to change notification settings - Fork 3
Feat/139/gathering card UI - 모달 API 연결 #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough이 PR은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@minkyung5x5 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't determine the plugin "react-hooks" uniquely.
Please remove the "plugins" setting from either config or remove either plugin installation. If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/common/gathering-card/scheduled-gathering-card/container.tsx (1)
28-41: 에러 처리를 더 구체화하면 좋을 것 같습니다.현재 구현된 에러 처리는 기본적인 케이스를 잘 다루고 있지만, 다음과 같은 개선사항을 고려해보세요:
- 구체적인 에러 타입 정의
- 네트워크 오류와 같은 추가적인 에러 케이스 처리
다음과 같이 개선해볼 수 있습니다:
useEffect(() => { if (error) { if (error instanceof ApiError) { const errorData = JSON.parse(error.message); - if (errorData.status === 'NOT_FOUND') { + switch (errorData.status) { + case 'NOT_FOUND': toast.error('모임 정보를 찾을 수 없습니다.'); - } + break; + case 'UNAUTHORIZED': + toast.error('접근 권한이 없습니다.'); + break; + default: + toast.error('알 수 없는 오류가 발생했습니다.'); + } } else { - toast.error('데이터 통신에 실패했습니다.'); + if (!navigator.onLine) { + toast.error('인터넷 연결을 확인해주세요.'); + } else { + toast.error('데이터 통신에 실패했습니다.'); + } } } }, [error]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/common/gathering-card/scheduled-gathering-card/container.tsx(2 hunks)src/components/common/gathering-card/scheduled-gathering-card/presenter.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/components/common/gathering-card/scheduled-gathering-card/presenter.tsx
🔇 Additional comments (2)
src/components/common/gathering-card/scheduled-gathering-card/container.tsx (2)
Line range hint 1-21: 인터페이스와 임포트 구성이 적절합니다.
API 통합을 위한 필수 의존성들이 잘 추가되었고, ScheduledGatheringCardContainerProps 인터페이스에 crewId 필드가 적절히 추가되었습니다.
43-44: 모달 핸들링이 깔끔하게 구현되었습니다.
useCallback을 활용한 모달 상태 관리가 React의 모범 사례를 잘 따르고 있습니다.
| <div> | ||
| <ScheduledGatheringCardPresenter | ||
| data={data} | ||
| onClick={handleCardClick} | ||
| onLikeToggle={handleLikeToggle} | ||
| /> | ||
| {dummyModalData && ( | ||
| <ScheduledGatheringCardPresenter data={data} onClick={handleCardClick} /> | ||
| {gatheringData && ( | ||
| <GatheringDetailModalContainer | ||
| opened={isOpened} | ||
| close={() => { | ||
| setIsOpened(false); | ||
| }} | ||
| data={dummyModalData} | ||
| close={handleCloseModal} | ||
| data={gatheringData} | ||
| /> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
로딩 상태 처리가 필요합니다.
현재 구현에서 데이터 로딩 중일 때의 UI 처리가 누락되어 있습니다. 사용자 경험 향상을 위해 다음 사항들을 고려해보세요:
- 로딩 상태 표시
- 에러 바운더리 추가
다음과 같이 개선해볼 수 있습니다:
+ const { data: gatheringData, error, isLoading } = useGetGatheringDetailQuery(data.crewId, data.id);
return (
<div>
<ScheduledGatheringCardPresenter data={data} onClick={handleCardClick} />
+ {isLoading && <LoadingSpinner />}
{gatheringData && (
<GatheringDetailModalContainer
opened={isOpened}
close={handleCloseModal}
data={gatheringData}
/>
)}
</div>
);Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx (1)
26-26: 다양한 crewId 값으로 테스트 케이스를 보강하는 것이 좋겠습니다.현재 두 스토리 모두 동일한
crewId: 1을 사용하고 있습니다. 다양한 시나리오 테스트를 위해 서로 다른crewId값을 사용하는 것이 좋겠습니다.예시 수정안:
// LikedEvent story - crewId: 1, + crewId: 123, // NotLikedEvent story - crewId: 1, + crewId: 456,Also applies to: 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/common/gathering-card/scheduled-gathering-card/container.tsx(1 hunks)src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx(2 hunks)src/types/gathering-data.d.ts(1 hunks)
🔇 Additional comments (4)
src/components/common/gathering-card/scheduled-gathering-card/container.tsx (2)
26-27: 모달 상태 관리가 잘 구현되어 있습니다
useCallback을 사용하여 핸들러 함수를 메모이제이션한 것이 좋습니다.
30-38: 이전 리뷰에서 지적된 로딩 상태 처리가 아직 구현되지 않았습니다
데이터 페칭 중일 때 사용자에게 로딩 상태를 표시해야 합니다.
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx (1)
26-26: API 통합을 위한 crewId 속성 추가가 적절합니다.
모달 API 연동을 위해 필요한 crewId 속성이 스토리에 추가되었습니다.
Also applies to: 46-46
src/types/gathering-data.d.ts (1)
50-50: 타입 정의가 적절하게 추가되었습니다.
crewId 속성이 GatheringDetailType과 일관성 있게 추가되었으며, 모임 상세 정보를 가져오는데 필요한 타입 안전성이 확보되었습니다.
컴포넌트에서 crewId 사용을 확인하기 위해 다음 스크립트를 실행합니다:
✅ Verification successful
Let me generate additional scripts to verify the complete usage of crewId:
crewId 속성이 올바르게 구현되어 있음을 확인했습니다
crewId 속성이 다음과 같이 적절하게 사용되고 있습니다:
GatheringCardProps인터페이스에 명확하게 정의되어 있습니다useGetGatheringDetailQuery훅에서 모임 상세 정보를 가져오는데 사용됩니다- 여러 컴포넌트(
gathering-card/container.tsx,scheduled-gathering-card/container.tsx)에서 일관되게 활용되고 있습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: crewId 프로퍼티의 사용 현황을 확인합니다.
# GatheringCard 컴포넌트에서 crewId 사용 확인
ast-grep --pattern 'const GatheringCard = ({ crewId, $$$_ }: GatheringCardProps)'
# crewId를 사용하는 API 호출 확인
rg -A 3 'useGetGatheringDetailQuery.*crewId'
Length of output: 1314
Script:
#!/bin/bash
# GatheringCardProps 인터페이스의 사용처를 확인합니다
ast-grep --pattern 'interface GatheringCardProps {
$$$
}'
# crewId 사용 패턴을 더 넓게 검색
rg -A 2 'crewId.*GatheringCard'
Length of output: 793
| import { useCallback, useState } from 'react'; | ||
| import { useCallback, useEffect, useState } from 'react'; | ||
| import { toast } from 'react-toastify'; | ||
| import { ApiError } from 'next/dist/server/api-utils'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서버 사이드 유틸리티를 클라이언트에서 사용하고 있습니다
next/dist/server/api-utils의 ApiError는 서버 사이드 전용 유틸리티입니다. 클라이언트 사이드에서는 커스텀 에러 클래스를 정의하여 사용하는 것이 좋습니다.
다음과 같이 수정해보세요:
-import { ApiError } from 'next/dist/server/api-utils';
+class ApiError extends Error {
+ constructor(public status: string, message: string) {
+ super(message);
+ this.name = 'ApiError';
+ }
+}📝 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.
| import { ApiError } from 'next/dist/server/api-utils'; | |
| class ApiError extends Error { | |
| constructor(public status: string, message: string) { | |
| super(message); | |
| this.name = 'ApiError'; | |
| } | |
| } |
| if (error) { | ||
| if (error instanceof ApiError) { | ||
| const errorData = JSON.parse(error.message); | ||
| if (errorData.status === 'NOT_FOUND') { | ||
| toast.error('모임 정보를 찾을 수 없습니다.'); | ||
| } | ||
| } else { | ||
| toast.error('데이터 통신에 실패했습니다.'); | ||
| } | ||
| } | ||
| }, [error]); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
에러 처리 로직을 더 안전하게 개선해야 합니다
현재 구현에는 다음과 같은 잠재적인 문제가 있습니다:
JSON.parse가 실패할 수 있습니다errorData타입 검증이 없습니다
다음과 같이 개선해보세요:
useEffect(() => {
if (error) {
if (error instanceof ApiError) {
- const errorData = JSON.parse(error.message);
- if (errorData.status === 'NOT_FOUND') {
+ try {
+ const errorData = JSON.parse(error.message);
+ if (typeof errorData === 'object' && errorData?.status === 'NOT_FOUND') {
toast.error('모임 정보를 찾을 수 없습니다.');
+ } else {
+ toast.error('알 수 없는 에러가 발생했습니다.');
+ }
+ } catch {
+ toast.error('에러 처리 중 문제가 발생했습니다.');
}
} else {
toast.error('데이터 통신에 실패했습니다.');
}
}
}, [error]);📝 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.
| useEffect(() => { | |
| if (error) { | |
| if (error instanceof ApiError) { | |
| const errorData = JSON.parse(error.message); | |
| if (errorData.status === 'NOT_FOUND') { | |
| toast.error('모임 정보를 찾을 수 없습니다.'); | |
| } | |
| } else { | |
| toast.error('데이터 통신에 실패했습니다.'); | |
| } | |
| } | |
| }, [error]); | |
| useEffect(() => { | |
| if (error) { | |
| if (error instanceof ApiError) { | |
| try { | |
| const errorData = JSON.parse(error.message); | |
| if (typeof errorData === 'object' && errorData?.status === 'NOT_FOUND') { | |
| toast.error('모임 정보를 찾을 수 없습니다.'); | |
| } else { | |
| toast.error('알 수 없는 에러가 발생했습니다.'); | |
| } | |
| } catch { | |
| toast.error('에러 처리 중 문제가 발생했습니다.'); | |
| } | |
| } else { | |
| toast.error('데이터 통신에 실패했습니다.'); | |
| } | |
| } | |
| }, [error]); |
yulrang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~!!! 화이팅
HaeJungg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨슴니다!!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx (1)
54-54: 테스트 커버리지 개선을 위한 제안두 스토리가 동일한 crewId 값을 사용하고 있습니다. 다양한 시나리오 테스트를 위해 서로 다른 crewId 값을 사용하는 것이 좋을 것 같습니다.
- crewId: 1, + crewId: 2,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx(4 hunks)
🔇 Additional comments (2)
src/components/common/gathering-card/scheduled-gathering-card/scheduled-gathering.stories.tsx (2)
2-2: ClientProvider 통합이 적절히 구현되었습니다!
스토리북 데코레이터를 사용하여 모든 스토리에 ClientProvider를 일관되게 적용한 것이 좋습니다.
Also applies to: 16-22
34-34: crewId 값의 유효성을 확인해주세요.
API 연동 시 실제 사용될 crewId 값과 일치하는지 확인이 필요합니다.
🔖 Issue Ticket
Ticket
✍️ Description
✅ Checklist
PR
Test
Summary by CodeRabbit
신규 기능
버그 수정
문서화
GatheringCardProps인터페이스에crewId속성이 추가되었습니다.리팩토링
onLikeToggle속성이 제거되어 코드가 간소화되었습니다.