Skip to content

Fix(host): 공연 수정뷰 QA 반영 #354

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.

Sign up for GitHub

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

Merged
eunkr82 merged 10 commits into from
Apr 10, 2026
Merged

Fix(host): 공연 수정뷰 QA 반영 #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8850e62
fix: 공연 수정 모달 텍스트 수정
eunkr82 Apr 1, 2026
5d46026
fix: 공연 수정 후 상세 데이터 캐시 갱신처리
eunkr82 Apr 1, 2026
59b7a06
fix: 모달 문구 수정
eunkr82 Apr 5, 2026
345df9d
fix: 래빗 리뷰 반영
eunkr82 Apr 5, 2026
19ad73b
fix: mutateAsync를 mutate로 정리
eunkr82 Apr 5, 2026
576573a
fix: 수정 요청 중 모달 취소 버튼 비활성화
eunkr82 Apr 5, 2026
b398ce2
fix: 카테고리 모달 분기 수정
eunkr82 Apr 5, 2026
324254d
refactor: handleConfirmEdit 분리
eunkr82 Apr 9, 2026
8517c1e
feat: 핸들러에 isPending 가드 추가
eunkr82 Apr 9, 2026
175e797
fix: isPending 가드 삭제
eunkr82 Apr 10, 2026
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: 8 additions & 1 deletion apps/host/src/features/event-edit/use-event-edit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { useMutation } from '@tanstack/react-query';
import { useMutation, useQueryClient } from '@tanstack/react-query';

import { putFestival } from '@entities/event-edit/api/event-edit';

import { ORGANIZERS_QUERY_KEY } from '@shared/constants/query-key';

export const useFestivalUpdateMutation = (festivalId: number) => {
const queryClient = useQueryClient();

return useMutation({
mutationKey: ORGANIZERS_QUERY_KEY.FESTIVAL_UPDATE(festivalId),
mutationFn: (formData: FormData) => putFestival(festivalId, formData),
onSuccess: async () => {
await queryClient.invalidateQueries({
queryKey: ORGANIZERS_QUERY_KEY.FESTIVAL_DETAIL(festivalId),
});
},
});
};
Comment on lines 12 to 19

@Sohyunnnn Sohyunnnn Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

처음 코드를 봤을 때는 PR에 적어주신 것처럼
수정 요청 성공 -> 관련 detail query invalidate -> 페이지 이동
흐름의 의도가 명확하다고 생각하여 리뷰 남기지 않았었습니다.

다만 수정해주신 현재 코드에서는 호출부 onSuccess에서 바로 후속 동작을 처리하고 있어서, invalidate 완료를 반드시 기다려야 하는 요구가 아니라면 useFestivalUpdateMutation 내부 onSuccess의 async/await는 제거해도 괜찮아 보입니다.

@jin-evergreen jin-evergreen Apr 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

나은님이 PR에 적어주신 흐름의 의도를 유지하려면, 현재 mutate로 변경했다 할지라도 async/await을 유지해야 의도한 것처럼 API 호출 -> invalidateQueries 실행 -> 끝날 때까지 대기 -> 호출부 onSuccess(페이지 라우팅) 실행 이라는 흐름이 유지될 것 같은데 소현님의 의견이 궁금합니다!

물론 이런 흐름의 의도를 유지하는 것이 불필요하다고 판단되면 말씀해주신 것처럼 제거하여, 불필요한 대기를 줄이는 것도 좋을 듯 합니다.

@eunkr82 eunkr82 Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

리뷰 감사합니다!

말씀해 주신 것처럼 불필요한 대기를 줄여서 더 가볍게 가져가는 것도 좋다고 생각합니다. 다만 이번 PR의 시작점이 수정 이후에 관련 detail query를 다시 무효화해서 최신 데이터가 안정적으로 반영되도록 만드는 것이었기 때문에, 수정 성공 → detail query invalidate 완료까지 대기 → 그 다음 호출부 onSuccess 실행의 흐름이 유지되도록 하는 것이 처음 목적에 더 가깝다고 생각했습니다.

우선 기획 쪽에 QA 반영사항 전달 후, 성능상 불필요한 대기가 실제로 문제되는 지점이 보이면 그때 await 제거 여부를 검토해 보도록 하겠습니다! 두 분 다 좋은 의견 감사합니다 🙇‍♀️

55 changes: 28 additions & 27 deletions apps/host/src/pages/event-edit/event-edit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useQuery } from '@tanstack/react-query';
import { overlay } from 'overlay-kit';
import { useNavigate, useParams } from 'react-router';

import { Modal, RectButton } from '@amp/ads-ui';
import { Modal, RectButton, toast } from '@amp/ads-ui';
import { Loading } from '@amp/compositions';

import EventForm from '@widgets/event-form/event-form';
Expand All @@ -19,24 +19,13 @@ import { NAV_PATH } from '@shared/constants/path';
interface ConfirmModalProps {
title: string;
description?: string;
onConfirm: () => void;
values: EventFormSubmitValues;
}

interface EventEditPageContentProps {
festivalId: number;
}

const areSameCategoryIds = (prev: number[], next: number[]) => {
if (prev.length !== next.length) {
return false;
}

const sortedPrev = [...prev].sort((a, b) => a - b);
const sortedNext = [...next].sort((a, b) => a - b);

return sortedPrev.every((id, index) => id === sortedNext[index]);
};

const EventEditPageContent = ({ festivalId }: EventEditPageContentProps) => {
const updateMutation = useFestivalUpdateMutation(festivalId);

Expand All @@ -58,27 +47,38 @@ const EventEditPageContent = ({ festivalId }: EventEditPageContentProps) => {
return null;
}

const initialValues = toEventEditInitialValues(data);

const submitEdit = (values: EventFormSubmitValues) => {
const handleConfirmEdit = (
values: EventFormSubmitValues,
closeModal: () => void,
) => {
const formData = serializeUpdateFestivalFormData(values);

updateMutation.mutate(formData, {
onSuccess: () => {
closeModal();
navigate(NAV_PATH.noticeList(festivalId));
},
onError: () => {
toast.show('공연 수정에 실패했어요. 잠시 후 다시 시도해 주세요.');
},
});
};

const initialValues = toEventEditInitialValues(data);

const openConfirmModal = ({
title,
description,
onConfirm,
values,
}: ConfirmModalProps) => {
overlay.open(({ isOpen, close, unmount }) => (
<Modal
open={isOpen}
onClose={() => {
if (updateMutation.isPending) {
return;
}

close();
unmount();
}}
Expand All @@ -94,6 +94,7 @@ const EventEditPageContent = ({ festivalId }: EventEditPageContentProps) => {
<Modal.Actions>
<RectButton
variant='secondary'
disabled={updateMutation.isPending}
onClick={() => {
close();
unmount();
Expand All @@ -106,9 +107,10 @@ const EventEditPageContent = ({ festivalId }: EventEditPageContentProps) => {
variant='primary'
disabled={updateMutation.isPending}
onClick={() => {
onConfirm();
close();
unmount();
handleConfirmEdit(values, () => {
close();
unmount();
});
}}
Comment thread
eunkr82 marked this conversation as resolved.
>
확인
Expand All @@ -129,19 +131,18 @@ const EventEditPageContent = ({ festivalId }: EventEditPageContentProps) => {
return;
}

const isCategoryChanged = !areSameCategoryIds(
initialValues.activeCategoryIds,
values.activeCategoryIds,
const hasDeletedCategory = initialValues.activeCategoryIds.some(
(id) => !values.activeCategoryIds.includes(id),
);
Comment on lines +134 to 136

@coderabbitai coderabbitai Bot Apr 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Suggest: 코딩 가이드라인에 따라 boolean 변수명을 is 접두사로 변경하는 것을 고려해 주세요.

현재 hasDeletedCategory는 변수인데, 코딩 가이드라인에서는 is 접두사는 boolean 변수에, has 접두사는 boolean을 반환하는 유틸리티 함수에 사용하도록 권장하고 있어요.

♻️ 수정 제안 
-        const hasDeletedCategory = initialValues.activeCategoryIds.some(
+        const isCategoryDeleted = initialValues.activeCategoryIds.some(
           (id) => !values.activeCategoryIds.includes(id),
         );

         openConfirmModal({
-          title: hasDeletedCategory
+          title: isCategoryDeleted
             ? '공지 카테고리를 수정하시겠어요?'
             : '수정하시겠어요?',
-          description: hasDeletedCategory
+          description: isCategoryDeleted
             ? '카테고리를 수정하면\n해당 카테고리로 작성된 공지가 삭제돼요.'
             : undefined,
           values,
         });

As per coding guidelines: "Use 'is' prefix for boolean variables (e.g., isActive)" and "Use 'has' prefix for utility functions that return boolean values"

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/host/src/pages/event-edit/event-edit.tsx` around lines 128 - 130, Rename
the boolean variable hasDeletedCategory to use an "is" prefix per guidelines
(e.g., isAnyCategoryDeleted or isCategoryDeleted) in the event-edit component:
update the declaration (const hasDeletedCategory = ...) and all its subsequent
references so the code continues to check
initialValues.activeCategoryIds.some(...) vs
values.activeCategoryIds.includes(...) under the new name; ensure any related
TypeScript types, props, or tests that reference hasDeletedCategory are updated
to the new identifier.


openConfirmModal({
title: isCategoryChanged
title: hasDeletedCategory
? '공지 카테고리를 수정하시겠어요?'
: '공지를 수정하시겠어요?',
description: isCategoryChanged
: '수정하시겠어요?',
description: hasDeletedCategory
? '카테고리를 수정하면\n해당 카테고리로 작성된 공지가 삭제돼요.'
: undefined,
onConfirm: () => submitEdit(values),
values,
});
}}
/>
Expand Down
9 changes: 2 additions & 7 deletions apps/host/src/widgets/home/festival-actions/festival-actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,15 @@ const FestivalActionsView = ({
<>
{children}
<OptionSheet open={isOptionSheetOpen} onClose={onCloseOptionSheet}>
<OptionSheet.Item onClick={onEdit}>
수정하기
</OptionSheet.Item>
<OptionSheet.Item onClick={onEdit}>수정하기</OptionSheet.Item>
<OptionSheet.Item onClick={onOpenDeleteModal}>
삭제하기
</OptionSheet.Item>
</OptionSheet>
<Modal open={isDeleteModalOpen} onClose={onCloseDeleteModal}>
<Modal.Panel role='alertdialog'>
<Modal.Content>
<Modal.Title>공연을 삭제할까요?</Modal.Title>
<Modal.Description>
삭제한 공연은 복구할 수 없어요.
</Modal.Description>
<Modal.Title>공연을 삭제하시겠어요?</Modal.Title>
</Modal.Content>
<Modal.Actions>
<RectButton variant='secondary' onClick={onCloseDeleteModal}>
Expand Down
Loading