[Feature/#84] 제어 기능별 모달 연동 - #92
Merged
Merged
Conversation
📝 WalkthroughWalkthrough캠페인·광고 제어를 모달 기반 확인 흐름으로 전환하고 공통 ModalContent 컴포넌트를 추가했으며, 라우트와 페이지 구조를 일부 재배치(CampaignGroup 도입, AdsCreatePage 제거)했습니다. 여러 리스트·상세 컴포넌트에 로컬 모달 상태와 확인 핸들러가 추가되었습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant Page as AdsListPage / CampaignDetail / AdDetailContent
participant Modal as Modal (Modal + ModalContent)
participant State as Local State (trackOpen, isStopping...)
participant Toast as Toast
User->>Page: 중단/재개/트래킹 버튼 클릭
Page->>State: set*Open(true)
State->>Modal: Modal 표시 (open)
Modal->>User: 확인/취소 UI 제공
User->>Modal: 확인 클릭
Modal->>Page: onConfirm 호출
Page->>State: setIs*ing(true)
Page->>Toast: 처리 시작(토스트)
Page->>State: setIs*ing(false)
Page->>State: set*Open(false)
Page->>Toast: 처리 완료(토스트)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📚 Storybook 배포 완료
|
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/ads/list/CampaignDetail.tsx (1)
82-96:⚠️ Potential issue | 🟡 Minor캠페인 재개 모달 트리거 누락
ControlBox에서 "중단하기" 버튼만 있고,
resumeOpen모달을 여는 트리거가 없습니다.AdDetailContent.tsx에서는ad.runStatus에 따라 중단/재개를 토글하는 패턴을 사용하는데, 여기서도 캠페인 상태에 따른 동적 UI가 필요해 보입니다.🔧 상태 기반 동적 버튼 제안
<ControlBox title="캠페인 운영 제어" description={`전체 플랫폼의 광고 운영을 한번에 제어할 수 있습니다.\n클릭 시 해당 캠페인 내 속한 모든 광고 소재의 운영이 즉시 중단됩니다.`} - buttonText="중단하기" + buttonText={data.runStatus === "stopped" ? "재개하기" : "중단하기"} onButtonClick={() => { - setStopOpen(true); + data.runStatus === "stopped" + ? setResumeOpen(true) + : setStopOpen(true); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/CampaignDetail.tsx` around lines 82 - 96, The ControlBox only triggers setStopOpen(true) for the "중단하기" action; add state-based logic to render a resume action and trigger the resume modal instead when the campaign is in a stopped state. Update the ControlBox props (buttonText, onButtonClick, buttonClassName/containerClassName as needed) to switch between "중단하기" -> onButtonClick={() => setStopOpen(true)} and "재개하기" -> onButtonClick={() => setResumeOpen(true)} based on the campaign status (e.g., campaign.runStatus or similar), and ensure setResumeOpen (or a resumeOpen state setter) exists and is used consistently alongside setStopOpen.
🧹 Nitpick comments (4)
src/constants/sidebarNav.ts (1)
49-49: URL 경로 네이밍 컨벤션 확인
/ads/campaignGroup으로 변경되었는데, 일반적으로 URL 경로는 kebab-case(/ads/campaign-group)가 웹 표준 권장 사항입니다. 하지만 기존 코드베이스 컨벤션을 따르는 것이라면 괜찮습니다.MainRoutes.tsx의 라우트 정의와 일치하는 것을 확인했습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/constants/sidebarNav.ts` at line 49, The sidebar navigation entry currently uses a camelCase path "/ads/campaignGroup"; update the path string in src/constants/sidebarNav.ts (the sidebar nav item for campaign group) to the kebab-case "/ads/campaign-group" to follow URL conventions, and ensure this value stays consistent with the corresponding route definition in MainRoutes.tsx (or adjust MainRoutes.tsx to match if the project convention is camelCase instead of kebab-case).src/pages/ads/list/CampaignDetail.tsx (1)
25-43: 코드 중복: AdDetailContent와 동일한 패턴
onStopConfirm,onResumeConfirm핸들러가AdDetailContent.tsx의 패턴과 거의 동일합니다. 커스텀 훅으로 추출하면 코드 재사용성이 높아지고 유지보수가 쉬워집니다. 앞서 제안한useModalAction훅을 공유할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/CampaignDetail.tsx` around lines 25 - 43, Extract the duplicated modal action logic from onStopConfirm and onResumeConfirm into a reusable hook (e.g., useModalAction) and replace those handlers with calls to that hook; the hook should accept parameters for the start/stop state setters (setIsStopping/setIsResuming), the open state setter (setStopOpen/setResumeOpen), and the success message to show via toast, perform the setIsX(true) → try { toast.success(message); setOpen(false);} finally { setIsX(false); } sequence, and then import and use this hook in both CampaignDetail (replacing onStopConfirm/onResumeConfirm) and AdDetailContent so both share the same implementation.src/components/ads/AdDetailContent.tsx (1)
19-30: 상태 관리 개선 권장: 커스텀 훅으로 분리4개의 모달에 대해 각각
open과loading상태를 관리하고 있어 총 8개의useState가 사용됩니다. 코딩 가이드라인에 따라 비즈니스 로직을 커스텀 훅으로 분리하면 코드 재사용성과 가독성이 향상됩니다.♻️ useModalAction 커스텀 훅 제안
// hooks/useModalAction.ts function useModalAction(onAction: () => Promise<void> | void) { const [isOpen, setIsOpen] = useState(false); const [isLoading, setIsLoading] = useState(false); const open = () => setIsOpen(true); const close = () => setIsOpen(false); const confirm = async () => { setIsLoading(true); try { await onAction(); close(); } finally { setIsLoading(false); } }; return { isOpen, isLoading, open, close, confirm }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ads/AdDetailContent.tsx` around lines 19 - 30, Extract the repeated open/loading state logic into a custom hook (e.g., useModalAction) and replace the eight useState calls (trackOpen/setTrackOpen, isTracking/setIsTracking, trackStopOpen/setTrackStopOpen, isTrackStopping/setIsTrackStopping, stopOpen/setStopOpen, isStopping/setIsStopping, resumeOpen/setResumeOpen, isResuming/setIsResuming) in AdDetailContent with four hook instances; the hook should expose isOpen, isLoading, open, close and confirm (confirm wraps the async action, sets loading, awaits the action, and closes in finally) so each modal uses one hook instance and calls confirm for the modal action.src/components/common/modal/ModalContent.tsx (1)
33-44: 접근성 개선 권장: 취소 버튼 및 로딩 상태 알림현재 확인 버튼만 있고 모달 내 취소 버튼이 없습니다. 사용자 경험 측면에서 명시적인 취소 버튼을 추가하는 것이 좋습니다. 또한 로딩 상태 변경 시 스크린 리더 사용자를 위한
aria-live영역 추가를 고려해보세요.♻️ 취소 버튼 추가 제안
interface IModalContentProps { icon?: ReactNode; title: string; description: string | ReactNode; buttonText: string; onConfirm: () => void; + onCancel?: () => void; + cancelText?: string; isLoading?: boolean; variant?: "danger" | "primary"; }- <div className="flex justify-center"> + <div className="flex justify-center gap-3"> + {onCancel && ( + <Button + type="button" + variant="outline" + size="big" + onClick={onCancel} + className="w-full md:w-auto px-12" + disabled={isLoading} + > + {cancelText || "취소"} + </Button> + )} <Button🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/modal/ModalContent.tsx` around lines 33 - 44, Add an explicit cancel control and an aria-live loading announcement in ModalContent: update the JSX around the Button group to include a secondary "Cancel" button that calls an onCancel prop (add/ensure onCancel is accepted by the ModalContent component and wired where the component is used) and is keyboard-focusable; keep the existing primary Button using onConfirm and isLoading/variant/buttonText. Also add a visually-hidden aria-live="polite" element bound to isLoading (e.g., render text "처리 중.." when isLoading is true) so screen readers are notified of the loading state; ensure both buttons have accessible labels and proper disabled handling consistent with isLoading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ads/AdDetailContent.tsx`:
- Around line 32-70: The handlers onTrackConfirm, onTrackStopConfirm,
onStopConfirm, and onResumeConfirm currently set loading flags synchronously
then immediately clear them, so loading UI will never show; change each to an
async function, set the corresponding loading state (setIsTracking,
setIsTrackStopping, setIsStopping, setIsResuming) to true, await the actual API
call (or a placeholder await) inside try, show success toast on success and
handle errors in catch (show error toast/log), and in finally set the loading
state back to false and close the modal
(setTrackOpen/setTrackStopOpen/setStopOpen/setResumeOpen) so the loading state
persists during the async operation.
- Around line 196-211: The "트래킹 중단" modal is defined using trackStopOpen and
setTrackStopOpen but has no trigger; add a UI trigger (e.g., a button or menu
item) that calls setTrackStopOpen(true) to open the modal when clicked. Locate
where other modal triggers are implemented (examples: stopOpen/resumeOpen
handlers and their onClick handlers) and mirror that pattern for the
trackStopOpen flow so the Modal with onConfirm={onTrackStopConfirm} and
isLoading={isTrackStopping} can be opened by the user.
In `@src/pages/ads/list/AdsListPage.tsx`:
- Line 33: Remove the leftover debug console.log statements in AdsListPage.tsx
(the lines logging "전체 캠페인 중단 시작" and "전체 캠페인 재개 시작"); either delete them or
replace them with the project's logging utility (e.g., use the app's logger or
processLogger) inside the functions/components that call them so production code
does not emit console.log; locate the calls in AdsListPage (search for the exact
strings) and update accordingly.
- Around line 117-134: The resume modal (Modal with isOpen={resumeOpen}) is
defined but never opened; add a UI trigger that calls setResumeOpen(true) —
either by making the existing ControlBox that currently only shows the pause
action render a resume ControlBox when campaigns are paused, or by adding a
separate ControlBox/button that appears for paused campaigns and invokes
setResumeOpen(true) and uses onResumeAll for confirmation; update any
conditional rendering logic around the ControlBox to switch between pause/resume
based on campaign state so the Modal (resumeOpen, setResumeOpen, onResumeAll)
can actually be launched.
---
Outside diff comments:
In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 82-96: The ControlBox only triggers setStopOpen(true) for the
"중단하기" action; add state-based logic to render a resume action and trigger the
resume modal instead when the campaign is in a stopped state. Update the
ControlBox props (buttonText, onButtonClick, buttonClassName/containerClassName
as needed) to switch between "중단하기" -> onButtonClick={() => setStopOpen(true)}
and "재개하기" -> onButtonClick={() => setResumeOpen(true)} based on the campaign
status (e.g., campaign.runStatus or similar), and ensure setResumeOpen (or a
resumeOpen state setter) exists and is used consistently alongside setStopOpen.
---
Nitpick comments:
In `@src/components/ads/AdDetailContent.tsx`:
- Around line 19-30: Extract the repeated open/loading state logic into a custom
hook (e.g., useModalAction) and replace the eight useState calls
(trackOpen/setTrackOpen, isTracking/setIsTracking,
trackStopOpen/setTrackStopOpen, isTrackStopping/setIsTrackStopping,
stopOpen/setStopOpen, isStopping/setIsStopping, resumeOpen/setResumeOpen,
isResuming/setIsResuming) in AdDetailContent with four hook instances; the hook
should expose isOpen, isLoading, open, close and confirm (confirm wraps the
async action, sets loading, awaits the action, and closes in finally) so each
modal uses one hook instance and calls confirm for the modal action.
In `@src/components/common/modal/ModalContent.tsx`:
- Around line 33-44: Add an explicit cancel control and an aria-live loading
announcement in ModalContent: update the JSX around the Button group to include
a secondary "Cancel" button that calls an onCancel prop (add/ensure onCancel is
accepted by the ModalContent component and wired where the component is used)
and is keyboard-focusable; keep the existing primary Button using onConfirm and
isLoading/variant/buttonText. Also add a visually-hidden aria-live="polite"
element bound to isLoading (e.g., render text "처리 중.." when isLoading is true)
so screen readers are notified of the loading state; ensure both buttons have
accessible labels and proper disabled handling consistent with isLoading.
In `@src/constants/sidebarNav.ts`:
- Line 49: The sidebar navigation entry currently uses a camelCase path
"/ads/campaignGroup"; update the path string in src/constants/sidebarNav.ts (the
sidebar nav item for campaign group) to the kebab-case "/ads/campaign-group" to
follow URL conventions, and ensure this value stays consistent with the
corresponding route definition in MainRoutes.tsx (or adjust MainRoutes.tsx to
match if the project convention is camelCase instead of kebab-case).
In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 25-43: Extract the duplicated modal action logic from
onStopConfirm and onResumeConfirm into a reusable hook (e.g., useModalAction)
and replace those handlers with calls to that hook; the hook should accept
parameters for the start/stop state setters (setIsStopping/setIsResuming), the
open state setter (setStopOpen/setResumeOpen), and the success message to show
via toast, perform the setIsX(true) → try { toast.success(message);
setOpen(false);} finally { setIsX(false); } sequence, and then import and use
this hook in both CampaignDetail (replacing onStopConfirm/onResumeConfirm) and
AdDetailContent so both share the same implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dfbae563-2aac-437e-9473-f43607c2edf3
⛔ Files ignored due to path filters (1)
src/assets/icon/workspace/message-circle-warning.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (8)
src/components/ads/AdDetailContent.tsxsrc/components/common/modal/ModalContent.tsxsrc/constants/sidebarNav.tssrc/pages/ads/list/AdsListPage.tsxsrc/pages/ads/list/CampaignDetail.tsxsrc/pages/ads/new/AdsCreatePage.tsxsrc/pages/ads/new/CampaignGroup.tsxsrc/routes/MainRoutes.tsx
💤 Files with no reviewable changes (1)
- src/pages/ads/new/AdsCreatePage.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/pages/ads/list/AdsListPage.tsx (2)
33-33:⚠️ Potential issue | 🟡 Minor디버그용 console.log 제거 필요
console.log("전체 캠페인 중단 시작")및console.log("전체 캠페인 재개 시작")가 프로덕션 코드에 남아있습니다. 이전 리뷰에서 지적된 사항으로, 제거해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` at line 33, Remove the debug console.log statements left in AdsListPage: delete the console.log("전체 캠페인 중단 시작") and the console.log("전체 캠페인 재개 시작") calls so no debug logging remains in production code (search for these exact strings in AdsListPage.tsx or references inside the methods that handle campaign pause/resume to locate and remove them).
115-130:⚠️ Potential issue | 🟡 Minor전체 캠페인 재개 모달 트리거 누락
resumeOpen상태와 "전체 캠페인 재개" 모달이 정의되어 있지만, 이를 열 수 있는 UI 요소가 없습니다. 현재는 중단용 ControlBox만 존재하고 재개를 위한 UI가 없어 모달에 접근할 수 없습니다.이전 리뷰에서 지적된 사항으로, 캠페인 상태에 따라 중단/재개 버튼을 동적으로 표시하거나 별도의 재개 ControlBox를 추가해주세요.
🔧 상태 기반 동적 렌더링 예시
{/* 캠페인 상태에 따라 중단/재개 ControlBox 표시 */} {isAllCampaignsStopped ? ( <ControlBox title="전체 캠페인을 재개할 수 있어요" description="모든 광고 노출이 즉시 재개되고, 연결된 플랫폼에서 광고가 다시 집행됩니다." buttonText="재개하기" onButtonClick={() => setResumeOpen(true)} buttonDisabled={isResuming} containerClassName="bg-status-blue/7 border-status-blue px-6 py-4 min-w-[650px] shrink-0" titleClassName="text-status-blue font-heading3" // ... /> ) : ( <ControlBox title="전체 캠페인을 완전히 중단할 수 있어요" // ... 현재 구현 /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 115 - 130, The resume modal (Modal using resumeOpen, setResumeOpen) is never reachable; add a UI trigger that calls setResumeOpen(true)—either render a dedicated "재개하기" ControlBox when isAllCampaignsStopped is true or add a second button in the existing ControlBox that conditionally shows "중단/재개" and opens the modal; ensure the trigger passes through buttonDisabled tied to isResuming and that the modal's onConfirm uses onResumeAll so the flow (resumeOpen -> Modal -> onResumeAll) is complete.
🧹 Nitpick comments (2)
src/pages/ads/list/AdsListPage.tsx (2)
29-51: 동기 코드에서 try-finally 구조 및 에러 처리 부재현재
onStopAll과onResumeAll핸들러는 동기적으로 실행되지만,try-finally구조를 사용하고 있어setIsStopping(true)직후 바로setIsStopping(false)가 호출됩니다. 이로 인해 버튼의 로딩 상태가 사용자에게 보이지 않을 수 있습니다.API 연동 시 다음 사항을 고려해주세요:
- async/await 패턴 적용: API 호출을 위해 비동기 함수로 변경
- catch 블록 추가: API 실패 시 사용자에게 에러 피드백 제공
- React Query useMutation 고려: 코딩 가이드라인에 따라 서버 상태는 React Query로 관리 권장
🔧 API 연동을 위한 구조 제안
- const onStopAll = () => { - setIsStopping(true); - - try { - console.log("전체 캠페인 중단 시작"); - toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다."); - setStopAllOpen(false); - } finally { - setIsStopping(false); - } - }; + const onStopAll = async () => { + setIsStopping(true); + + try { + // await stopAllCampaignsApi(); + toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다."); + setStopAllOpen(false); + } catch (error) { + toast.error("캠페인 중단에 실패했습니다. 다시 시도해주세요."); + } finally { + setIsStopping(false); + } + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 29 - 51, onStopAll and onResumeAll are synchronous but use try-finally which immediately clears loading; convert both handlers (onStopAll, onResumeAll) to async functions, perform the API call with await, wrap the await in try/catch/finally so setIsStopping(true)/setIsResuming(true) stays true during the request and is cleared in finally, call setStopAllOpen(false)/setResumeOpen(false) on success and show toast.success there, and in catch use toast.error (or similar) to surface server errors; preferably implement the server calls via React Query useMutation and call mutate/mutateAsync from these handlers to manage server state and loading instead of manual setIsStopping/setIsResuming.
70-93: LGTM - ControlBox 구성이 적절합니다.
buttonDisabled={isStopping}을 통해 중복 클릭 방지가 잘 구현되어 있습니다.- 스타일링이 PR 목표에 맞게 danger 상태를 명확히 표현하고 있습니다.
참고: Line 75의
buttonDisabled={false}는 기본값이므로 생략 가능합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 70 - 93, The first ControlBox instance (title "캠페인 통합 운영 제어") includes an unnecessary explicit prop buttonDisabled={false}; remove that prop from the ControlBox JSX so it relies on the default value instead (locate the ControlBox with title "캠페인 통합 운영 제어" and delete the buttonDisabled attribute), then run lint/format to ensure no trailing commas or spacing issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Line 33: Remove the debug console.log statements left in AdsListPage: delete
the console.log("전체 캠페인 중단 시작") and the console.log("전체 캠페인 재개 시작") calls so no
debug logging remains in production code (search for these exact strings in
AdsListPage.tsx or references inside the methods that handle campaign
pause/resume to locate and remove them).
- Around line 115-130: The resume modal (Modal using resumeOpen, setResumeOpen)
is never reachable; add a UI trigger that calls setResumeOpen(true)—either
render a dedicated "재개하기" ControlBox when isAllCampaignsStopped is true or add a
second button in the existing ControlBox that conditionally shows "중단/재개" and
opens the modal; ensure the trigger passes through buttonDisabled tied to
isResuming and that the modal's onConfirm uses onResumeAll so the flow
(resumeOpen -> Modal -> onResumeAll) is complete.
---
Nitpick comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Around line 29-51: onStopAll and onResumeAll are synchronous but use
try-finally which immediately clears loading; convert both handlers (onStopAll,
onResumeAll) to async functions, perform the API call with await, wrap the await
in try/catch/finally so setIsStopping(true)/setIsResuming(true) stays true
during the request and is cleared in finally, call
setStopAllOpen(false)/setResumeOpen(false) on success and show toast.success
there, and in catch use toast.error (or similar) to surface server errors;
preferably implement the server calls via React Query useMutation and call
mutate/mutateAsync from these handlers to manage server state and loading
instead of manual setIsStopping/setIsResuming.
- Around line 70-93: The first ControlBox instance (title "캠페인 통합 운영 제어")
includes an unnecessary explicit prop buttonDisabled={false}; remove that prop
from the ControlBox JSX so it relies on the default value instead (locate the
ControlBox with title "캠페인 통합 운영 제어" and delete the buttonDisabled attribute),
then run lint/format to ensure no trailing commas or spacing issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6b62390-fe93-4130-bc9e-d51ca50f8574
📒 Files selected for processing (1)
src/pages/ads/list/AdsListPage.tsx
Collaborator
|
P4: WorkspaceSetting 모달 색지정 확인했습니다! |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/pages/ads/list/AdsListPage.tsx (2)
130-130: 불필요한 빈<div />제거를 고려해주세요.
</section>직전의 빈<div />가 어떤 목적으로 사용되는지 불분명합니다. 레이아웃에 특별한 영향이 없다면 제거하는 것이 좋겠습니다.🧹 제거 제안
</Modal> - <div /> </section>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` at line 130, In AdsListPage remove the unnecessary empty <div /> found immediately before the closing </section> in the component render (the lone <div /> in AdsListPage.tsx); delete that self-closing div, verify the surrounding <section> layout and any CSS or flex/grid behavior still behave as expected, and run the app/tests to confirm no visual or functional regressions.
29-49: 동기 코드에서 try/finally와 로딩 상태 패턴이 오해를 유발할 수 있습니다.현재
onStopAll과onResumeAll함수는 동기적으로 실행되므로,setIsStopping(true)직후finally블록에서 바로false로 리셋됩니다. 실제로 로딩 상태가 UI에 반영될 시간이 없어isLoadingprop이 무의미해집니다.API 연동이 예정되어 있다면, 향후 비동기 처리를 위한 구조로 미리 준비해두는 것이 좋겠습니다.
♻️ 비동기 API 연동을 위한 구조 제안
- const onStopAll = () => { - setIsStopping(true); - - try { - toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다."); - setStopAllOpen(false); - } finally { - setIsStopping(false); - } - }; + const onStopAll = async () => { + setIsStopping(true); + + try { + // TODO: API 연동 + // await stopAllCampaigns(); + toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다."); + setStopAllOpen(false); + } catch (error) { + toast.error("캠페인 중단에 실패했습니다. 다시 시도해주세요."); + } finally { + setIsStopping(false); + } + };
onResumeAll도 동일한 패턴으로 수정하시면 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 29 - 49, The handlers onStopAll and onResumeAll reset loading flags immediately because they are synchronous; change them to async functions (e.g., const onStopAll = async () => { ... }) and perform the API call (or a placeholder await) between setIsStopping(true) and the finally block, moving setStopAllOpen(false) and toast.success(...) to after the awaited call so that setIsStopping(false) in finally correctly reflects the async operation; do the same pattern for onResumeAll, referencing onStopAll/onResumeAll, setIsStopping/setIsResuming, setStopAllOpen/setResumeOpen, and the toast.success calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Line 130: In AdsListPage remove the unnecessary empty <div /> found
immediately before the closing </section> in the component render (the lone <div
/> in AdsListPage.tsx); delete that self-closing div, verify the surrounding
<section> layout and any CSS or flex/grid behavior still behave as expected, and
run the app/tests to confirm no visual or functional regressions.
- Around line 29-49: The handlers onStopAll and onResumeAll reset loading flags
immediately because they are synchronous; change them to async functions (e.g.,
const onStopAll = async () => { ... }) and perform the API call (or a
placeholder await) between setIsStopping(true) and the finally block, moving
setStopAllOpen(false) and toast.success(...) to after the awaited call so that
setIsStopping(false) in finally correctly reflects the async operation; do the
same pattern for onResumeAll, referencing onStopAll/onResumeAll,
setIsStopping/setIsResuming, setStopAllOpen/setResumeOpen, and the toast.success
calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 69d6c4c2-02ff-41ed-85fa-33aa6d83310e
📒 Files selected for processing (1)
src/pages/ads/list/AdsListPage.tsx
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/pages/ads/list/AdsListPage.tsx (1)
113-128:⚠️ Potential issue | 🟡 Minor전체 캠페인 재개 모달 트리거 누락
resumeOpen상태와 모달이 정의되어 있지만, 이를 여는 UI 요소가 없습니다. 현재 상태에서 재개 모달은 사용할 수 없는 dead code입니다.두 가지 방향을 제안드립니다:
- 캠페인 상태 기반 조건부 렌더링: 전체 캠페인이 중단된 상태일 때 "중단하기" 대신 "재개하기" ControlBox를 표시
- API 연동 전까지 임시 제거: 실제 캠페인 상태를 알 수 없다면, 재개 모달 관련 코드를 제거하고 API 연동 시 함께 구현
💡 조건부 렌더링 예시
// 캠페인 상태를 서버에서 가져온다고 가정 const isAllCampaignsStopped = useCampaignStatus(); // 또는 적절한 상태 관리 {isAllCampaignsStopped ? ( <ControlBox title="전체 캠페인을 재개할 수 있어요" description="모든 광고 노출이 즉시 재개됩니다." buttonText="재개하기" onButtonClick={() => setResumeOpen(true)} buttonDisabled={isResuming} containerClassName="bg-status-blue/7 border-status-blue ..." // ... 기타 props /> ) : ( <ControlBox title="전체 캠페인을 완전히 중단할 수 있어요" // ... 현재 코드 /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 113 - 128, The resume modal (Modal + ModalContent) is defined and controlled by resumeOpen/setResumeOpen but there is no UI that sets resumeOpen to true, so the modal is unreachable; either add a trigger ControlBox/button that calls setResumeOpen(true) (conditionally rendered when campaigns are stopped) or remove the modal until API-driven campaign state is available. Locate the Modal/ModalContent block using symbols Modal, ModalContent, resumeOpen, setResumeOpen, onResumeAll and isResuming and implement one of the two fixes: 1) add a ControlBox or button component that calls setResumeOpen(true) (use campaign state like isAllCampaignsStopped for conditional rendering) or 2) remove the Modal and related state (resumeOpen, setResumeOpen, onResumeAll, isResuming) until backend integration.
🧹 Nitpick comments (2)
src/pages/ads/list/AdsListPage.tsx (2)
68-91: ControlBox 구현 적절함, 사소한 개선점전반적으로 ControlBox 사용이 적절합니다. 한 가지 사소한 개선점:
- Line 73:
buttonDisabled={false}는 기본값과 동일하므로 생략 가능합니다.🧹 불필요한 prop 제거
<ControlBox title="캠페인 통합 운영 제어" description={`여러 광고 플랫폼의 캠페인을 하나로 묶어 성과와 운영 상태를 통합 관리합니다.\n광고 플랫폼 로그인 후 캠페인을 불러와 연결합니다.`} buttonText="캠페인 통합 연동하기" onButtonClick={handleCampaignGroupClick} - buttonDisabled={false} containerClassName="bg-chart-3/7 border-chart-3 px-6 py-4 min-w-[650px] shrink-0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 68 - 91, Remove the redundant explicit prop on the first ControlBox: delete the unnecessary buttonDisabled={false} since ControlBox already defaults to an enabled button; update the JSX for the ControlBox instance (the one with title "캠페인 통합 운영 제어") to omit the buttonDisabled prop so the component relies on its default behavior.
29-49: API 연동 시 async/await 패턴 및 에러 처리 필요현재 핸들러가 동기적 try/finally 패턴을 사용하고 있어서, 향후 API 호출이 추가될 때 제대로 동작하지 않을 수 있습니다.
주요 문제점:
- 동기 코드에서 loading 상태가 무의미:
setIsStopping(true)직후 동기 코드가 실행되어finally에서 바로false로 변경됩니다. 실제 로딩 UI가 표시되지 않습니다.- 에러 핸들링 부재:
catch블록이 없어서 API 실패 시 사용자에게 피드백을 줄 수 없고, 모달이 닫히지 않을 수 있습니다.API 연동 시 아래와 같은 패턴을 권장합니다:
🔧 async/await 패턴으로 개선
- const onStopAll = () => { + const onStopAll = async () => { setIsStopping(true); - try { + // await stopAllCampaignsApi(); toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다."); setStopAllOpen(false); + } catch (error) { + toast.error("캠페인 중단에 실패했습니다. 다시 시도해주세요."); } finally { setIsStopping(false); } }; - const onResumeAll = () => { + const onResumeAll = async () => { setIsResuming(true); - try { + // await resumeAllCampaignsApi(); toast.success("전체 캠페인의 광고 노출이 재개되었습니다."); setResumeOpen(false); + } catch (error) { + toast.error("캠페인 재개에 실패했습니다. 다시 시도해주세요."); } finally { setIsResuming(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` around lines 29 - 49, The onStopAll and onResumeAll handlers are currently synchronous, so setIsStopping/setIsResuming flip back to false immediately and there is no error handling; convert both handlers (onStopAll, onResumeAll) to async functions, await the corresponding API calls (e.g., stopAllAds()/resumeAllAds() or the actual API helper you use), wrap the await in try/catch/finally, set the loading flag to true before the await, on success call toast.success and close the modal via setStopAllOpen/setResumeOpen, on error call toast.error with the error message, and always reset the loading flag in finally (setIsStopping(false)/setIsResuming(false)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Around line 113-128: The resume modal (Modal + ModalContent) is defined and
controlled by resumeOpen/setResumeOpen but there is no UI that sets resumeOpen
to true, so the modal is unreachable; either add a trigger ControlBox/button
that calls setResumeOpen(true) (conditionally rendered when campaigns are
stopped) or remove the modal until API-driven campaign state is available.
Locate the Modal/ModalContent block using symbols Modal, ModalContent,
resumeOpen, setResumeOpen, onResumeAll and isResuming and implement one of the
two fixes: 1) add a ControlBox or button component that calls
setResumeOpen(true) (use campaign state like isAllCampaignsStopped for
conditional rendering) or 2) remove the Modal and related state (resumeOpen,
setResumeOpen, onResumeAll, isResuming) until backend integration.
---
Nitpick comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Around line 68-91: Remove the redundant explicit prop on the first ControlBox:
delete the unnecessary buttonDisabled={false} since ControlBox already defaults
to an enabled button; update the JSX for the ControlBox instance (the one with
title "캠페인 통합 운영 제어") to omit the buttonDisabled prop so the component relies on
its default behavior.
- Around line 29-49: The onStopAll and onResumeAll handlers are currently
synchronous, so setIsStopping/setIsResuming flip back to false immediately and
there is no error handling; convert both handlers (onStopAll, onResumeAll) to
async functions, await the corresponding API calls (e.g.,
stopAllAds()/resumeAllAds() or the actual API helper you use), wrap the await in
try/catch/finally, set the loading flag to true before the await, on success
call toast.success and close the modal via setStopAllOpen/setResumeOpen, on
error call toast.error with the error message, and always reset the loading flag
in finally (setIsStopping(false)/setIsResuming(false)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7da0589a-79d9-4336-832d-54f59832ae43
📒 Files selected for processing (1)
src/pages/ads/list/AdsListPage.tsx
Seojegyeong
approved these changes
Mar 11, 2026
Collaborator
|
P4: 확인했습니다. 수고하셨습니다! |
jjjsun
approved these changes
Mar 11, 2026
This was referenced Mar 15, 2026
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 관련 이슈
close #84
✨ 변경사항
✏️ 작업 내용
ModalContent공용 컴포넌트 구현Modal컴포넌트로 감싸서 사용 가능모달 연동
상태별 controlBox/modal UI 적용
controlBox/modal에 적용😅 미완성 작업
📢 논의 사항 및 참고 사항
모달에 사용하는
@/assets/icon/workspace/message-circle-warning.svg의 색을 바꿔서 사용할 수 있게stroke="currentColor"로 수정했습니다. WorkspaceSetting의 모달 아이콘의 색을 지정해주시면 감사하겠습니다!! @jjjsunSummary by CodeRabbit
New Features
Chores