-
Notifications
You must be signed in to change notification settings - Fork 1
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: 칸반보드 상세 페이지에서 지원 상태 확인 및 지원 상태 변경 가능하게 구현 #208
Changes from 9 commits
1fa563e
beb5efe
165b80c
79168a4
b47fb18
1656aa1
6abdd75
e860e3c
8f1095c
4bc5185
1bb9f56
e692ac4
03153e8
cc6dae1
e2d85e0
b3e062f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,115 @@ | ||
import Txt from "@/components/common/Txt.component"; | ||
import { ApplicantReq } from "@/src/apis/applicant"; | ||
import { | ||
ApplicantReq, | ||
getApplicantState, | ||
patchApplicantState, | ||
} from "@/src/apis/applicant"; | ||
import { applicantDataFinder } from "@/src/functions/finder"; | ||
import Portfolio from "./Portfolio"; | ||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; | ||
import { useSearchParams } from "next/navigation"; | ||
import { useEffect, useState } from "react"; | ||
import KanbanCardApplicantStatusLabel from "@/components/kanban/card/CardApplicantStatusLabel"; | ||
import { ApplicantPassState, getAllKanbanData } from "@/src/apis/kanban"; | ||
import { useAtomValue } from "jotai"; | ||
import { KanbanSelectedButtonNumberState } from "@/src/stores/kanban/Navbar.atoms"; | ||
import { getMyInfo } from "@/src/apis/interview"; | ||
interface ApplicantResourceProps { | ||
data: ApplicantReq[]; | ||
postId: string; | ||
generation: string; | ||
} | ||
|
||
const ApplicantResource = ({ data, postId }: ApplicantResourceProps) => { | ||
const ApplicantResource = ({ | ||
data, | ||
postId, | ||
generation, | ||
}: ApplicantResourceProps) => { | ||
const navbarId = useAtomValue(KanbanSelectedButtonNumberState); | ||
const searchParams = useSearchParams(); | ||
const applicantId = searchParams.get("applicantId"); | ||
const queryClient = useQueryClient(); | ||
|
||
const { | ||
data: initialState, | ||
isLoading, | ||
isError, | ||
} = useQuery(["applicantState", applicantId], () => | ||
getApplicantState(navbarId, `${applicantId}`, generation) | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. navbarId에 대한 의존성이 빠진 것 같네요~ |
||
|
||
const { | ||
data: myInfo, | ||
isLoading: myInfoLoading, | ||
isError: myInfoError, | ||
} = useQuery(["user"], getMyInfo); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추후 버전업에 유리하기 위해 parameter로 하나의 객체로 표현할 수 있게 하는건 어떨까요? |
||
|
||
const [passState, setPassState] = | ||
useState<ApplicantPassState>("non-processed"); | ||
|
||
const { mutate } = useMutation({ | ||
mutationFn: (afterState: "non-pass" | "pass") => | ||
patchApplicantState(`${applicantId}`, afterState), | ||
onSuccess: (data) => { | ||
queryClient.invalidateQueries(["kanbanDataArray", generation]); | ||
setPassState(data.passState); | ||
}, | ||
}); | ||
|
||
const onFailedButtonClick = () => { | ||
mutate("non-pass"); | ||
}; | ||
|
||
const onPassedButtonClick = () => { | ||
mutate("pass"); | ||
}; | ||
|
||
useEffect(() => { | ||
if (initialState) { | ||
setPassState(initialState); | ||
} | ||
}, [initialState]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 페이지를 들어왔을 때 useQuery가 작동하고 initialState가 undefined 였다가 api 요청이 끝나면 지원 상태를 받아와지기 때문에 useEffect와 setPassState를 이용해서 현재 지원 상태를 표시해주었습니다.
위와 같이 작성해도 되지만 이렇게 해버리면 KanbanCardApplicantStatusLabel 컴포넌트에 대해서도 undefined 처리를 해줘야하기 때문에 현재 코드를 선택하게 되었습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 제 질문 사항이 부족한 것 같아 더 추가적으로 댓글을 적습니다
또한 useEffect는 최대한 사용하지 말아주세요.(react의 성능 및 철학에 위배됩니다. 키워드는 함수형에 대해 찾아보면 좋습니다.) 특히 useQuery안에 사용하는 데이터와 useState는 혼용해서 사용하면 데이터를 복제하여 사용하는 효과가 있기 때문에 복잡성을 높이기 쉽습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. useEffect를 사용하지 않는 방향으로 코드를 변경해보겠습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useEffect와 passState를 사용하지 않고 queryData를 변경하는 방법으로 진행하면 어떨까요? 사실 passState라는 상태가 initialState라는 상태와 같은 느낌이라고 생각하는데, SSOT를 위배하는 코드 같아용 따라서 queryData를 조작하는 식으로 가는 느낌이 좋을듯합니다! 참고할만 한 예제 코드를 작성해봤는데 참고해보시기 바랍니당 const { mutate } = useMutation({
mutationFn: (afterState: "non-pass" | "pass") =>
patchApplicantState(`${applicantId}`, afterState),
onMutate: async (afterState) => {
// 데이터 꼬임 방지
await queryClient.cancelQueries(["applicantState", applicantId]);
const snapshotState = queryClient.getQueryData<getApplicantStateRes>([
"applicantState",
applicantId,
]);
// 적절하게 업데이트 해준다. 혹은 복잡하다면, 이부분은 무시해도 된다.
queryClient.setQueryData<getApplicantStateRes>(
["applicantState", applicantId],
(prev) => {
if (prev === "non-processed" && afterState === "pass") {
return "first-passed";
}
return prev;
}
);
return { snapshotState };
},
onSuccess: (data) => {
queryClient.invalidateQueries(["kanbanDataArray", generation]);
},
onError: (error, variables, context) => {
window.alert("상태 변경에 실패했습니다.");
// rollback한다.
if (context !== undefined) {
queryClient.setQueryData<getApplicantStateRes>(
["applicantState", applicantId],
context.snapshotState
);
}
},
onSettled: () => {
// 요청에 성공하면 새로운 passState를 가져온다.
queryClient.invalidateQueries(["applicantState", applicantId]);
},
}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @2yunseong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. onError 부분의 아래 코드와 onSettled의 내용이 중복 코드라고 생각이 되는데 아래 코드를 작성하신 이유가 있을까요?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@smb0123 이 말이 잘 이해가 가지 않는다면 swr 전략에 대해 공부해보시기 바랍니다 :) |
||
|
||
if (!initialState || isLoading || !myInfo || myInfoLoading) { | ||
return <div>로딩중...</div>; | ||
} | ||
|
||
if (isError || myInfoError) { | ||
return <div>에러 발생</div>; | ||
} | ||
|
||
return ( | ||
<> | ||
<div className="flex flex-col gap-1 mb-2"> | ||
<Txt className="text-xl text-secondary-200 font-medium"> | ||
{applicantDataFinder(data, "major")} | ||
</Txt> | ||
<Txt typography="h2">{`[${applicantDataFinder( | ||
data, | ||
"field" | ||
)}] ${applicantDataFinder(data, "name")}`}</Txt> | ||
<div className="flex justify-between items-center"> | ||
<div className="flex flex-col gap-1 mb-2"> | ||
<div className="flex justify-between items-center"> | ||
<Txt className="text-xl text-secondary-200 font-medium"> | ||
{applicantDataFinder(data, "major")} | ||
</Txt> | ||
<KanbanCardApplicantStatusLabel passState={passState} /> | ||
</div> | ||
<Txt typography="h2">{`[${applicantDataFinder( | ||
data, | ||
"field" | ||
)}] ${applicantDataFinder(data, "name")}`}</Txt> | ||
</div> | ||
{(myInfo?.role === "ROLE_OPERATION" || | ||
myInfo?.role === "ROLE_PRESIDENT") && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 부분은 윗 부분에 작성된 if 문으로 인하여 undefined가 될 수 없습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 처음에 if문에 myInfo를 조건으로 작성하지 않았다가 나중에 수정하였는데 옵셔널에 대한 수정은 하지 않았네요 ..... |
||
<div className="flex gap-5"> | ||
<button | ||
onClick={onFailedButtonClick} | ||
className="bg-zinc-200 w-20 h-20 hover:bg-sky-400 rounded-xl" | ||
> | ||
불합격 | ||
</button> | ||
<button | ||
onClick={onPassedButtonClick} | ||
className="bg-zinc-200 w-20 h-20 hover:bg-sky-400 rounded-xl" | ||
> | ||
합격 | ||
</button> | ||
</div> | ||
)} | ||
</div> | ||
<div className="flex gap-4 mb-8"> | ||
<div className="flex gap-1"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import { getAllInterviewerWithOrder } from "@/src/apis/interview"; | ||
import { APPLICANT_KEYS } from "@/src/constants"; | ||
import { https } from "@/src/functions/axios"; | ||
import { ApplicantPassState, getKanbanCards, KanbanCardReq } from "../kanban"; | ||
|
||
export interface ApplicantReq { | ||
name: string; | ||
|
@@ -131,3 +132,31 @@ export const getApplicantTimeTables = async (id: string) => { | |
|
||
return data; | ||
}; | ||
|
||
interface patchApplicantStateRes { | ||
passState: ApplicantPassState; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 타입은 Pascal Case로 작성해주세요 :) |
||
|
||
export const patchApplicantState = async ( | ||
id: string, | ||
afterState: "non-pass" | "pass" | ||
) => { | ||
const { data } = await https.patch<patchApplicantStateRes>( | ||
`/applicants/${id}/state?afterState=${afterState}` | ||
); | ||
|
||
return data; | ||
}; | ||
|
||
type getApplicantStateRes = ApplicantPassState | undefined; | ||
|
||
export const getApplicantState = async ( | ||
navigationId: string, | ||
applicantId: string, | ||
generation: string | ||
): Promise<getApplicantStateRes> => { | ||
const cardsData = await getKanbanCards(navigationId, generation); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아쉽지만 저희의 https객체에는 cache 기능이 없습니다. 그래서 지원자 수만큼 같은 요청이 backend에게 주어 부담을 줄 것입니다. 그러기에 이를 캐시를 적용하여 접근할 수 있는 방법이 있으면 더 좋지 않을까 라는 생각을 합니다.(캐시를 구현하는 것보다 구현되어 있는 것을 사용하면 좋을 듯 합니다.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개인적으로 서버 요청을 통해서 가져온 데이터를 가공해서 반환하는 함수는 api 로직들과 함께 있으면 안될 것 같다는 생각이 듭니다! 해당 방법은 채승님께서도 말씀해주셨지만, 요청을 여러번 하는 문제도 존재합니다. 이를 수정한다면 순수하게 getKanbanCard 의 반환값을 인자로 받고, getApplicantStateRes 형태를 반환하는 유틸 함수로 만드는 것도 좋아보입니다. 저의 개인적인 생각이므로 참고만 해주세요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가적으로 |
||
|
||
return cardsData.find((card) => card.applicantId === applicantId)?.state | ||
.passState; | ||
}; |
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.
getAllKanbanData는 안쓰이는 모듈입니다. 삭제 부탁드려요~~