Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/api/admin/customerService/inquiry.api.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import type { ApiCommonBasicType } from '../../../models/apiCommon';
import type {
AdminInquiryChangeSearchParams,
ApiAdminInquiry,
ApiAdminInquiryDetail,
InquiryAnswerBody,
} from '../../../models/inquiry';
import { httpClient } from '../../http.api';

export const getAllInquiries = async () => {
export const getAllInquiries = async (
childSearchParams: AdminInquiryChangeSearchParams
) => {
try {
const response = await httpClient.get<ApiAdminInquiry>(`/inquiry`);
const response = await httpClient.get<ApiAdminInquiry>(`/inquiry`, {
params: childSearchParams,
});
return response.data.data;
} catch (e) {
console.error('전체 문의 조회 에러', e);
Expand Down
12 changes: 12 additions & 0 deletions src/api/mypage.api.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ApiCommonBasicType } from '../models/apiCommon';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type

import type {
ApiUserInfo,
ApiUserInfoImg,
Expand Down Expand Up @@ -53,6 +54,17 @@ export const patchMyProfileImg = async (file: File) => {
}
};

export const patchGithubLink = async (githubUrl: string) => {
try {
await httpClient.patch<ApiCommonBasicType>('/user/github', {
params: { githubUrl },
});
} catch (error) {
console.error('프로필 깃허브 업데이트: ', error);
throw error;
}
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

API 요청 구조 및 반환값 처리 개선 필요

현재 구현에서 몇 가지 문제점이 발견됩니다:

  1. params 속성 사용이 부적절할 수 있습니다 - PATCH 요청에서는 일반적으로 request body 또는 query parameter로 데이터를 전송합니다
  2. 응답 데이터를 반환하지 않아 호출부에서 성공 여부를 확인하기 어렵습니다

다음과 같이 수정을 권장합니다:

-export const patchGithubLink = async (githubUrl: string) => {
-  try {
-    await httpClient.patch<ApiCommonBasicType>('/user/github', {
-      params: { githubUrl },
-    });
-  } catch (error) {
-    console.error('프로필 깃허브 업데이트: ', error);
-    throw error;
-  }
-};
+export const patchGithubLink = async (githubUrl: string) => {
+  try {
+    const response = await httpClient.patch<ApiCommonBasicType>(
+      '/user/github',
+      { githubUrl }
+    );
+    return response.data;
+  } catch (error) {
+    console.error('프로필 깃허브 업데이트: ', error);
+    throw error;
+  }
+};

또는 query parameter로 전송하려면:

-    await httpClient.patch<ApiCommonBasicType>('/user/github', {
-      params: { githubUrl },
-    });
+    const response = await httpClient.patch<ApiCommonBasicType>(
+      `/user/github?githubUrl=${encodeURIComponent(githubUrl)}`
+    );
+    return response.data;
📝 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.

Suggested change
export const patchGithubLink = async (githubUrl: string) => {
try {
await httpClient.patch<ApiCommonBasicType>('/user/github', {
params: { githubUrl },
});
} catch (error) {
console.error('프로필 깃허브 업데이트: ', error);
throw error;
}
};
export const patchGithubLink = async (githubUrl: string) => {
try {
const response = await httpClient.patch<ApiCommonBasicType>(
'/user/github',
{ githubUrl }
);
return response.data;
} catch (error) {
console.error('프로필 깃허브 업데이트: ', error);
throw error;
}
};
🤖 Prompt for AI Agents
In src/api/mypage.api.ts lines 57 to 66, the patchGithubLink function
incorrectly uses the 'params' property to send data in a PATCH request, which
should instead send the githubUrl in the request body or as query parameters.
Also, the function does not return the response, making it hard for callers to
verify success. Fix this by moving githubUrl into the request body (e.g., {
githubUrl }) and returning the awaited response from httpClient.patch so the
caller can handle the result.


export const getMyJoinedProjectList = async () => {
try {
const response = await httpClient.get<ApiJoinedProject>(
Expand Down
28 changes: 27 additions & 1 deletion src/components/admin/adminInquiry/AdminInquiry.styled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,33 @@ export const AdminInquiryCategory = styled.span`
font-weight: 500;
`;

export const AdminInquiryUser = styled.div``;
export const AdminInquiryUserWrapper = styled.div`
position: relative;
`;

export const AdminInquiryUser = styled.button`
width: fit-content;
font-size: 1.1rem;
z-index: 1;
transition: color 0.1s ease-in-out;
&:hover {
color: ${({ theme }) => theme.color.lightnavy};
}
`;

export const AdminInquiryUserCheckDropdown = styled.div`
position: absolute;
top: 0.8rem;
right: 0;
background: ${({ theme }) => theme.color.white};
border-radius: ${({ theme }) => theme.borderRadius.primary};
border: 1px solid ${({ theme }) => theme.color.lightgrey};
padding: 0.3rem 0.5rem;
z-index: 100000;
box-shadow: 3px 2px 19px -6px rgba(0, 0, 0, 0.75);
-webkit-box-shadow: 3px 2px 19px -6px rgba(0, 0, 0, 0.75);
-moz-box-shadow: 3px 2px 19px -6px rgba(0, 0, 0, 0.75);
`;

export const AdminInquiryDate = styled.span`
font-size: 0.9rem;
Expand Down
24 changes: 23 additions & 1 deletion src/components/admin/adminInquiry/AdminInquiry.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useSearchParams } from 'react-router-dom';
import { ADMIN_ROUTE } from '../../../constants/routes';
import type { AdminInquiry as TAdminInquiry } from '../../../models/inquiry';
import ContentBorder from '../../common/contentBorder/ContentBorder';
Expand All @@ -8,12 +9,33 @@ interface AdminInquiryProps {
}

export default function AdminInquiry({ list }: AdminInquiryProps) {
const [searchParams, setSearchParams] = useSearchParams();

const handleClickLookupUser = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
e.preventDefault();

const id = String(list.user.id);
const userId = id || '';
const nickname = list.user.nickname;

const newParams = new URLSearchParams(searchParams);
newParams.set('userId', userId);
newParams.set('nickname', nickname);

setSearchParams(newParams);
};

return (
<S.AdminInquiryContainer to={`${ADMIN_ROUTE.detail}/${list.id}`}>
<S.AdminInquiryWrapper>
<S.AdminInquiryCategory>[{list.category}]</S.AdminInquiryCategory>
<S.AdminInquiryTitle>{list.title}</S.AdminInquiryTitle>
<S.AdminInquiryUser>{list.user.nickname}</S.AdminInquiryUser>
<S.AdminInquiryUserWrapper>
<S.AdminInquiryUser onClick={handleClickLookupUser}>
{list.user.nickname}
</S.AdminInquiryUser>
</S.AdminInquiryUserWrapper>
<S.AdminInquiryDate>{list.createdAt}</S.AdminInquiryDate>
<S.AdminInquiryState $hasAnswer={list.state}>
{list.state ? '답변완료' : '확인중'}
Expand Down
3 changes: 2 additions & 1 deletion src/components/admin/adminInquiry/AdminInquiryList.styled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ export const SpinnerWrapper = styled(SpinnerWrapperStyled)``;
export const AdminInquiryListContainer = styled.section`
width: 100%;
display: flex;
justify-content: center;
flex-direction: column;
align-items: center;
`;

export const AdminInquiryListWrapper = styled.div`
Expand Down
15 changes: 14 additions & 1 deletion src/components/admin/adminInquiry/AdminInquiryList.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
import { useSearchParams } from 'react-router-dom';
import { useGetAllInquiries } from '../../../hooks/admin/useGetAllInquiries';
import Spinner from '../../user/mypage/Spinner';
import AdminInquiry from './AdminInquiry';
import * as S from './AdminInquiryList.styled';
import AdminInquiryListLookup from './AdminInquiryListLookup';
import type { AdminInquiryChangeSearchParams } from '../../../models/inquiry';

export type SearchParamsInquiryKeyType = keyof AdminInquiryChangeSearchParams;
export default function AdminInquiryList() {
const { allInquiriesData, isLoading } = useGetAllInquiries();
const [searchParams, setSearchParams] = useSearchParams();
const userId = searchParams.get('userId') || '';
const startDate = searchParams.get('startDate') || '';
const endDate = searchParams.get('endDate') || '';
const { allInquiriesData, isLoading } = useGetAllInquiries({
userId,
startDate,
endDate,
});

if (isLoading) {
return (
Expand All @@ -18,6 +30,7 @@ export default function AdminInquiryList() {

return (
<S.AdminInquiryListContainer>
<AdminInquiryListLookup />
<S.AdminInquiryListWrapper>
{allInquiriesData.map((list) => (
<AdminInquiry key={list.id} list={list} />
Expand Down
67 changes: 67 additions & 0 deletions src/components/admin/adminInquiry/AdminInquiryListLookup.styled.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import styled from 'styled-components';

export const LookupContainer = styled.nav`
width: 80%;
margin-bottom: 1rem;

input[type='date'] {
position: relative;
padding: 14px;
width: 150px;
height: 30px;
font-size: 14px;
color: ${({ theme }) => theme.color.placeholder};
border: none;
border-bottom: 1px solid ${({ theme }) => theme.color.grey};
}
input[type='date']::-webkit-calendar-picker-indicator {
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
width: auto;
height: auto;
color: transparent;
background: transparent;
}
`;

export const LookupWrapper = styled.form`
display: flex;
justify-content: space-between;
`;

export const LookupUser = styled.input`
border-bottom: 1px solid ${({ theme }) => theme.color.placeholder};
width: fit-content;
`;

export const LookupDateWrapper = styled.div`
display: flex;
gap: 1rem;
`;

export const LookupStartDate = styled.input``;

export const LookupJoinDate = styled.span``;

export const LookupEndDate = styled.input``;

export const LookupIconWrapper = styled.div`
display: flex;
gap: 2rem;

svg {
width: 1.5rem;
height: 1.5rem;
}
`;

export const IconDefault = styled.button`
color: ${({ theme }) => theme.color.deepGrey};
`;

export const IconSearch = styled.button`
color: ${({ theme }) => theme.color.deepGrey};
`;
119 changes: 119 additions & 0 deletions src/components/admin/adminInquiry/AdminInquiryListLookup.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import React, { useState } from 'react';
import type { SearchParamsInquiryKeyType } from './AdminInquiryList';
import * as S from './AdminInquiryListLookup.styled';
import { MagnifyingGlassIcon, XMarkIcon } from '@heroicons/react/24/outline';
import { useSearchParams } from 'react-router-dom';
import { AdminInquiryChangeSearchParams } from '../../../models/inquiry';
import Modal from '../../common/modal/Modal';
import { useModal } from '../../../hooks/useModal';
import { MODAL_MESSAGE } from '../../../constants/user/modalMessage';

export default function AdminInquiryListLookup() {
const [searchParams, setSearchParams] = useSearchParams();
const userId = searchParams.get('userId') || '';
const startDate = searchParams.get('startDate') || '';
const endDate = searchParams.get('endDate') || '';
const nickname = searchParams.get('nickname') || '';
const { isOpen, message, handleModalOpen, handleModalClose } = useModal();
const [searchFilters, setSearchFilters] = useState<
Omit<AdminInquiryChangeSearchParams, 'userId'>
>({
startDate,
endDate,
});

const handleSubmitChangeParams = (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();

const { startDate, endDate } = searchFilters;

const newParams = new URLSearchParams(searchParams);

if (startDate && !endDate) {
return handleModalOpen(MODAL_MESSAGE.endDateEmpty);
} else if (!startDate && endDate) {
return handleModalOpen(MODAL_MESSAGE.startDateEmpty);
} else if (startDate && endDate) {
newParams.set('startDate', startDate);
newParams.set('endDate', endDate);
} else {
newParams.delete('startDate');
newParams.delete('endDate');
}

setSearchParams(newParams);
};

const handleChangeDate = (
e: React.ChangeEvent<HTMLInputElement>,
key: SearchParamsInquiryKeyType
) => {
const value = e.currentTarget.value;

setSearchFilters((prev) => {
switch (key) {
case 'startDate': {
if (prev.endDate !== '' && prev.endDate < value) {
handleModalOpen(MODAL_MESSAGE.startDateInvalid);
return prev;
}
return { ...prev, startDate: value };
}
case 'endDate': {
if (prev.startDate !== '' && prev.startDate > value) {
handleModalOpen(MODAL_MESSAGE.endDateInvalid);
return prev;
}
return { ...prev, endDate: value };
}
default: {
return prev;
}
}
});
};

const handleClickInit = () => {
setSearchParams({});
setSearchFilters({ startDate: '', endDate: '' });
};

return (
<S.LookupContainer>
<S.LookupWrapper onSubmit={handleSubmitChangeParams}>
<S.LookupUser
type='text'
value={nickname}
placeholder='닉네임을 클릭하세요.'
readOnly
/>
<S.LookupDateWrapper>
<S.LookupStartDate
type='date'
value={searchFilters.startDate}
onChange={(e) => handleChangeDate(e, 'startDate')}
/>
<S.LookupJoinDate> ~ </S.LookupJoinDate>
<S.LookupEndDate
type='date'
value={searchFilters.endDate}
onChange={(e) => handleChangeDate(e, 'endDate')}
/>
</S.LookupDateWrapper>
<S.LookupIconWrapper>
{(userId || startDate || endDate) && (
<S.IconDefault type='button' onClick={handleClickInit}>
<XMarkIcon />
</S.IconDefault>
)}
<S.IconSearch type='submit'>
<MagnifyingGlassIcon />
</S.IconSearch>
</S.LookupIconWrapper>
</S.LookupWrapper>
<Modal isOpen={isOpen} onClose={handleModalClose}>
{message}
</Modal>
</S.LookupContainer>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,16 @@ import Avatar from '../../../common/avatar/Avatar';
import { ADMIN_ROUTE } from '../../../../constants/routes';
import arrow_right from '../../../../assets/ArrowRight.svg';
import Spinner from '../../../user/mypage/Spinner';
import { AdminInquiryChangeSearchParams } from '../../../../models/inquiry';

const InquiresPreview = () => {
const { allInquiriesData, isLoading, isFetching } = useGetAllInquiries();
const childSearchParams: AdminInquiryChangeSearchParams = {
userId: '',
startDate: '',
endDate: '',
};
const { allInquiriesData, isLoading, isFetching } =
useGetAllInquiries(childSearchParams);

if (isLoading || isFetching) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ import { ROUTES } from '../../../../../constants/routes';
import { useModal } from '../../../../../hooks/useModal';
import Modal from '../../../../common/modal/Modal';
import { MODAL_MESSAGE } from '../../../../../constants/user/modalMessage';
import { useGithubLink } from '../../../../../hooks/user/useMyInfo';

export default function ProfileGithubSuccess() {
const navigate = useNavigate();
const [searchParams] = useSearchParams();
const { isOpen, message, handleModalOpen, handleModalClose } = useModal();
const { patchGithubLinkMutate } = useGithubLink();

useEffect(() => {
(async () => {
const githubUrl = searchParams.get('githubUrl');
console.log(githubUrl);

if (githubUrl) {
patchGithubLinkMutate(githubUrl);
Copy link

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

비동기 처리 및 에러 핸들링 개선 필요

현재 구현에서 몇 가지 개선이 필요합니다:

  1. patchGithubLinkMutate는 비동기 함수이지만 await 없이 호출되고 있습니다
  2. mutation의 성공/실패 상태를 처리하지 않아 실제 API 호출 결과와 관계없이 성공 모달이 표시됩니다

다음과 같이 수정을 권장합니다:

-      if (githubUrl) {
-        patchGithubLinkMutate(githubUrl);
-        handleModalOpen(MODAL_MESSAGE.githubProfileSuccess);
-        setTimeout(() => {
-          navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`, {
-            state: { githubUrl },
-          });
-        }, 1000);
-      }
+      if (githubUrl) {
+        patchGithubLinkMutate(githubUrl, {
+          onSuccess: () => {
+            handleModalOpen(MODAL_MESSAGE.githubProfileSuccess);
+            setTimeout(() => {
+              navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`, {
+                state: { githubUrl },
+              });
+            }, 1000);
+          },
+          onError: () => {
+            handleModalOpen(MODAL_MESSAGE.githubProfileFail);
+            setTimeout(() => {
+              navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`);
+            }, 1000);
+          }
+        });
+      }
📝 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.

Suggested change
if (githubUrl) {
patchGithubLinkMutate(githubUrl);
if (githubUrl) {
patchGithubLinkMutate(githubUrl, {
onSuccess: () => {
handleModalOpen(MODAL_MESSAGE.githubProfileSuccess);
setTimeout(() => {
navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`, {
state: { githubUrl },
});
}, 1000);
},
onError: () => {
handleModalOpen(MODAL_MESSAGE.githubProfileFail);
setTimeout(() => {
navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`);
}, 1000);
},
});
}
🤖 Prompt for AI Agents
In src/components/user/mypage/myProfile/editProfile/ProfileGithubSuccess.tsx
around lines 18 to 20, the patchGithubLinkMutate function is called without
awaiting its asynchronous result and lacks error handling. Modify the code to
await the patchGithubLinkMutate call and handle its promise properly by adding
try-catch or using then-catch to manage success and failure states, ensuring the
success modal only shows on successful mutation and errors are handled
gracefully.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProfileGithubSuccess.tsx에서 useEffect를 불러올 때 async를 사용중인데 문제가 되니?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YouD0313 useEffect에서 async를 사용하는 현재 패턴은 전혀 문제가 되지 않습니다!

useEffect(() => {
  (async () => {
    // 비동기 코드
  })();
}, []);

이런 즉시 실행 함수 표현식(IIFE) 패턴은 useEffect 내에서 비동기 코드를 사용할 때 매우 일반적이고 권장되는 방법입니다. useEffect 자체는 async 함수를 직접 받을 수 없기 때문에 이런 패턴을 사용하는 것이 정상입니다.

제가 이전 코멘트에서 지적한 문제는 async 사용이 아니라 patchGithubLinkMutate의 에러 핸들링 부족이었습니다. 현재 코드는 API 호출의 성공/실패 여부와 관계없이 항상 성공 모달을 표시하고 있어서, 실제로 API 호출이 실패하더라도 사용자에게는 성공했다고 잘못된 피드백을 주게 됩니다.

async 패턴 자체는 완전히 올바르게 사용하고 계십니다! 👍


🧠 Learnings used
Learnt from: layout-SY
PR: devpalsPlus/frontend#246
File: src/hooks/useAlarmList.ts:1-21
Timestamp: 2025-04-23T13:39:08.960Z
Learning: React Query의 useQuery 훅에서 queryFn으로 Promise를 반환하는 함수를 사용할 때는 불필요한 async/await을 사용하지 않는 것이 좋습니다. 예를 들어 `queryFn: async () => await getAlarmList()` 대신에 `queryFn: () => getAlarmList()`와 같이 직접 Promise를 반환하는 형태로 작성하는 것이 더 효율적입니다.

Learnt from: layout-SY
PR: devpalsPlus/frontend#246
File: src/hooks/useAlarmList.ts:1-21
Timestamp: 2025-04-23T13:39:08.960Z
Learning: React Query의 useQuery 훅에서 queryFn으로 Promise를 반환하는 함수를 사용할 때는 불필요한 async/await을 사용하지 않는 것이 좋습니다. 예를 들어 `queryFn: async () => await getAlarmList()` 대신에 `queryFn: () => getAlarmList()`와 같이 직접 Promise를 반환하는 형태로 작성하는 것이 더 효율적입니다.

Learnt from: layout-SY
PR: devpalsPlus/frontend#260
File: src/components/common/header/Header.tsx:30-32
Timestamp: 2025-04-30T06:28:07.990Z
Learning: testLiveAlarm 함수는 이미 내부적으로 에러 처리를 하고 Promise를 반환하므로, 컴포넌트에서 호출할 때 async/await과 try-catch 구문 없이 직접 호출해도 충분하다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#334
File: src/components/common/admin/title/AdminTitle.styled.ts:4-10
Timestamp: 2025-06-05T15:59:56.230Z
Learning: YouD0313 prefers using position: fixed for admin headers due to layout requirements, even if it requires viewport-relative width calculations like calc(100vw - 20rem).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그럼 githubUrl 조건문처리와 다르다고 생각하는거니? 커스텀훅에서 처리하는게 맞다고 얘기하는거야?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YouD0313 좋은 질문입니다! 두 가지는 서로 다른 레벨의 검증이라고 생각합니다.

현재 상황 분석

1. githubUrl 조건문 처리 (클라이언트 사이드 검증)

  • URL 파라미터가 존재하는지 확인
  • 사용자가 올바른 경로로 왔는지 검증

2. API 호출 성공/실패 처리 (서버 통신 검증)

  • 실제 서버와의 통신 결과 확인
  • 네트워크 오류, 서버 오류 등을 처리

둘 다 필요한 이유

// 현재: githubUrl이 있어도 API 실패시 성공 모달이 표시됨
if (githubUrl) {
  patchGithubLinkMutate(githubUrl); // API 실패해도
  handleModalOpen(MODAL_MESSAGE.githubProfileSuccess); // 성공 모달 표시
}

처리 위치에 대한 견해

컴포넌트에서 처리하는 것이 더 적절하다고 생각합니다:

  • ✅ 각 컴포넌트마다 다른 성공/실패 UI 로직 구현 가능
  • ✅ 현재 Modal과 Navigation 로직이 컴포넌트에 종속됨
  • ✅ React Query의 onSuccess/onError 패턴과 일치

커스텀훅에서 처리하면 모든 컴포넌트가 동일한 에러 처리를 강제받게 되어 유연성이 떨어집니다.

결론: githubUrl 체크와 API 에러 처리는 모두 필요하며, 현재 구조에서는 컴포넌트의 onSuccess/onError 콜백 사용이 적절합니다.


🧠 Learnings used
Learnt from: layout-SY
PR: devpalsPlus/frontend#246
File: src/hooks/useAlarmList.ts:1-21
Timestamp: 2025-04-23T13:39:08.960Z
Learning: React Query의 useQuery 훅에서 queryFn으로 Promise를 반환하는 함수를 사용할 때는 불필요한 async/await을 사용하지 않는 것이 좋습니다. 예를 들어 `queryFn: async () => await getAlarmList()` 대신에 `queryFn: () => getAlarmList()`와 같이 직접 Promise를 반환하는 형태로 작성하는 것이 더 효율적입니다.

Learnt from: layout-SY
PR: devpalsPlus/frontend#246
File: src/hooks/useAlarmList.ts:1-21
Timestamp: 2025-04-23T13:39:08.960Z
Learning: React Query의 useQuery 훅에서 queryFn으로 Promise를 반환하는 함수를 사용할 때는 불필요한 async/await을 사용하지 않는 것이 좋습니다. 예를 들어 `queryFn: async () => await getAlarmList()` 대신에 `queryFn: () => getAlarmList()`와 같이 직접 Promise를 반환하는 형태로 작성하는 것이 더 효율적입니다.

Learnt from: layout-SY
PR: devpalsPlus/frontend#260
File: src/components/common/header/Header.tsx:30-32
Timestamp: 2025-04-30T06:28:07.990Z
Learning: testLiveAlarm 함수는 이미 내부적으로 에러 처리를 하고 Promise를 반환하므로, 컴포넌트에서 호출할 때 async/await과 try-catch 구문 없이 직접 호출해도 충분하다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#278
File: src/pages/login/LoginSuccess.tsx:13-24
Timestamp: 2025-05-11T09:39:02.041Z
Learning: useEffect 내에서 사용하는 모든 외부 변수(예: searchParams, storeLogin, navigate)는 의존성 배열에 포함되어야 합니다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#334
File: src/components/common/admin/title/AdminTitle.styled.ts:4-10
Timestamp: 2025-06-05T15:59:56.230Z
Learning: YouD0313 prefers using position: fixed for admin headers due to layout requirements, even if it requires viewport-relative width calculations like calc(100vw - 20rem).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커스텀훅에서 처리된다면 컴포넌트에서 조건문이 필요할까?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YouD0313 정말 좋은 지적입니다! 맞습니다. 커스텀훅에서 전체 플로우를 처리한다면 컴포넌트의 조건문이 불필요해집니다.

현재 구조의 문제점

// 컴포넌트가 너무 많은 책임을 가짐
if (githubUrl) {
  patchGithubLinkMutate(githubUrl);
  handleModalOpen(MODAL_MESSAGE.githubProfileSuccess);
  // navigation logic...
} else {
  handleModalOpen(MODAL_MESSAGE.githubProfileFail);
  // navigation logic...
}

개선된 구조 제안

커스텀훅에서 전체 처리:

// useGithubProcess.ts
export const useGithubProcess = (onModalOpen, navigate) => {
  const { patchGithubLinkMutate } = useGithubLink();
  
  const handleGithubProcess = useCallback((githubUrl) => {
    if (!githubUrl) {
      onModalOpen(MODAL_MESSAGE.githubProfileFail);
      setTimeout(() => navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`), 1000);
      return;
    }
    
    patchGithubLinkMutate(githubUrl, {
      onSuccess: () => {
        onModalOpen(MODAL_MESSAGE.githubProfileSuccess);
        setTimeout(() => navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`, { state: { githubUrl } }), 1000);
      },
      onError: () => {
        onModalOpen(MODAL_MESSAGE.githubProfileFail);
        setTimeout(() => navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`), 1000);
      }
    });
  }, []);
  
  return { handleGithubProcess };
};

컴포넌트는 단순해짐:

// ProfileGithubSuccess.tsx
export default function ProfileGithubSuccess() {
  const navigate = useNavigate();
  const [searchParams] = useSearchParams();
  const { isOpen, message, handleModalOpen, handleModalClose } = useModal();
  const { handleGithubProcess } = useGithubProcess(handleModalOpen, navigate);

  useEffect(() => {
    const githubUrl = searchParams.get('githubUrl');
    handleGithubProcess(githubUrl);
  }, [searchParams, handleGithubProcess]);

  return <Modal isOpen={isOpen} onClose={handleModalClose}>{message}</Modal>;
}

장점:

  • ✅ 관심사의 분리: 훅은 비즈니스 로직, 컴포넌트는 UI
  • ✅ 테스트 용이성: 각각 독립적으로 테스트 가능
  • ✅ 재사용성: 다른 컴포넌트에서도 동일한 로직 사용 가능
  • ✅ 컴포넌트 단순화: 조건문 제거

훌륭한 아키텍처 인사이트입니다! 👍


🧠 Learnings used
Learnt from: layout-SY
PR: devpalsPlus/frontend#260
File: src/components/common/header/Header.tsx:30-32
Timestamp: 2025-04-30T06:28:07.990Z
Learning: testLiveAlarm 함수는 이미 내부적으로 에러 처리를 하고 Promise를 반환하므로, 컴포넌트에서 호출할 때 async/await과 try-catch 구문 없이 직접 호출해도 충분하다.

Learnt from: YouD0313
PR: devpalsPlus/frontend#334
File: src/components/common/admin/title/AdminTitle.styled.ts:4-10
Timestamp: 2025-06-05T15:59:56.230Z
Learning: YouD0313 prefers using position: fixed for admin headers due to layout requirements, even if it requires viewport-relative width calculations like calc(100vw - 20rem).

handleModalOpen(MODAL_MESSAGE.githubProfileSuccess);
setTimeout(() => {
navigate(`${ROUTES.mypage}/${ROUTES.mypageEdit}`, {
Expand All @@ -28,7 +31,7 @@ export default function ProfileGithubSuccess() {
}, 1000);
}
})();
}, [searchParams, handleModalOpen, navigate]);
}, [searchParams, handleModalOpen, navigate, patchGithubLinkMutate]);

return (
<Modal isOpen={isOpen} onClose={handleModalClose}>
Expand Down
Loading