-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Feat/#453] 예매자 관리 페이지 기능 구현 #454
Conversation
….com/TEAM-BEAT/BEAT-Client into feat/#453/TicketHolderListFunction
PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶 |
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.
p4: 👍
지금은 값을 debounce를 하는데,
함수 호출 debounce 할 수도 있어요!
한 번 보면 좋을 것 같아요 ㅎ.ㅎ
// TODO : 공통컴포넌트에 svg 들어갈 수 있도록 수정하기 | ||
subText: "CSV", | ||
leftOnClick: handleNavigateBack, | ||
rightOnClick: handleEditButton, | ||
// TODO : rightOnClick CSV 다운로드로 변경 |
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.
p1: TODO 해주세용
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.
우리 신지는 정말 열심히 했구나...ㄷㄷㄷ 최고당!!!! 근데 todo는 언제해주시낭요
src/apis/domains/tickets/queries.ts
Outdated
queryFn: () => getTicketRetrieve(formData, filterList), | ||
gcTime: 1000 * 60 * 60 * 24, | ||
}); | ||
}; |
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.
src/apis/domains/tickets/queries.ts
Outdated
return useQuery({ | ||
queryKey: [QUERY_KEY.LIST, BOOKING_QUERY_KEY.BOOKING_LIST], |
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.
p3) 제가 쿼리키를 갈아엎으면서 예매한 공연 쿼리키에서 Bookinglist를 지우고 회원 기준 예매 공연목록과 비회원 기준 예매 공연 목록으로 분리했습니당! 그런데 사실 예매자가 예매한 공연 리스트를 캐싱하는거랑, 주최자가 예매자 리스트를 캐싱하는거랑 성격이 너무 다른데 같은 쿼리키를 꼭 공유해야 하는 이유를 잘 모르겠어서 지우고 sellerBookingList로 따로 관리하도록 수정했는데 이 부분 같이 이야기해보는 게 좋을 것 같습니다!!
src/apis/domains/tickets/queries.ts
Outdated
// 예매자 목록 검색 API (GET)를 위한 쿼리 작성 | ||
|
||
export const useTicketRetriveSearch = (formData: getTicketReq, searchWord, filterList) => { | ||
return useQuery({ | ||
queryKey: [QUERY_KEY.LIST, BOOKING_QUERY_KEY.BOOKING_LIST], | ||
queryFn: () => getTicketRetrieveSearch(formData, searchWord, filterList), | ||
gcTime: 1000 * 60 * 60 * 24, | ||
}); | ||
}; |
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.
p5) 이 코드가 위에서 검색하는 코드랑 많이 중복되는 것 같습니다! 그냥 인자의 차이라고 생각되는데 맞을까요??
그냥 위에 코드에 검색어도 함께 넣어줘서 값의 여부에 따라서 자동적으로 getTicketRetrieve를 요청하면 될 것 같은데 어떻게 생각하시나요??
src/apis/domains/tickets/queries.ts
Outdated
return useQuery({ | ||
queryKey: [QUERY_KEY.LIST, BOOKING_QUERY_KEY.BOOKING_LIST], |
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.
p3) filterList에 따라서 계속 목록을 받아오는 쿼리를 작성하신 것 맞을까요?? 저는 보통 이렇게 필터링 요소에 맞게 계속 요청을 보내야 하는 구조에서는 쿼리키에 넣어주는 편인데요! 현재 코드에서는 데이터가 바뀔때마다 refetch
를 해주고 있더라구용
useEffect(() => {
const fetchData = async () => {
const refetchData = await refetch();
setPaymentData(refetchData?.data?.bookingList ?? []);
};
const fetchSearchData = async () => {
const refetchSearchData = await searchRefetch();
setPaymentData(refetchSearchData?.data?.bookingList ?? []);
};
// TODO : 서버에서 검색어 2글자 이상으로 넘겨줬는데, 기-디에 화면에 어떻게 표현할지 물어보기
searchWord.length >= 2 ? fetchSearchData() : fetchData();
}, [filterList, status, debouncedQuery]);
그런데 쿼리키에 해당값을 넣어주면 값이 바뀔때마다 새로 요청이 자동으로 갈 수 있는데 굳이 이렇게 직접 구현을 해야할 필요성이 있는지 잘 모르겠습니당! 필터링(filterList
)이랑 검색어(searchWord
)를 쿼리키에 넣어서 관리해주는 것은 어떻게 생각하시나요?
src/apis/domains/tickets/queries.ts
Outdated
export const useTicketRefund = () => { | ||
const queryClient = new QueryClient(); | ||
|
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.
p3) 이미 상위의 App.tsx
에서 QueryClient
를 생성하고 있는데 이 훅에서 개별적으로 다시 생성하는 이유가 있나요?
모든 훅에서 새롭게 생성하고 있는데 이렇게 관리하면 하나의 queryClient에서 관리가 되지 않는데 제대로 동작하나요??
const queryClient = useQueryClient();
로 사용하는 것이 더 알맞는 방법 같습니다!
return css` | ||
color: ${({ theme }) => theme.colors.gray_400}; | ||
text-decoration: line-through; |
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.
p5) 여기 있는 theme을 그냥 status랑 같이 넣어주면 안되나요?
${({ $status }) => { | ||
if ($status === "취소 완료") { | ||
return css` | ||
color: ${({ theme }) => theme.colors.gray_600}; | ||
`; | ||
} | ||
}} |
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.
p5) 이거같은 경우에는 그냥 color에서 한 줄로 처리해도 될 것 같은데 나중에 변경될 가능성도 있으니까 유지보수를 위해서 이렇게 작성하신걸까용?
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.
p1) 헝헝 컴포넌트도 지워주세용
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.
p5) 여기 코드리뷰 안되는거 실화야요??????
에반데... 어떻게 코리남겨요...??
- 컴포넌트 밖으로 뺄수 있는 상수들은 분리해주는게 좋을 것 같아요!
- 업데이트를 하고
reload
를 직접하고 있는데, 리액트 쿼리를 활용하지 못한것 같습니다! 그냥 무효화시켜주면 되지 않나용??
setTimeout(() => {
window.location.reload();
}, 1000);
Quality Gate failedFailed conditions |
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes
📢 To Reviewers
📸 스크린샷