Skip to content

[Feature/#95] 캠페인 목록 조회 및 전체 캠페인 제어 API 연동 - #99

Merged
YermIm merged 17 commits into
developfrom
feature/#95
Mar 17, 2026
Merged

[Feature/#95] 캠페인 목록 조회 및 전체 캠페인 제어 API 연동#99
YermIm merged 17 commits into
developfrom
feature/#95

Conversation

@YermIm

@YermIm YermIm commented Mar 15, 2026

Copy link
Copy Markdown
Collaborator

🚨 관련 이슈

close #95

✨ 변경사항

  • 🐞 BugFix Something isn't working
  • 💻 CrossBrowsing Browser compatibility
  • 🌏 Deploy Deploy
  • 🎨 Design Markup & styling
  • 📃 Docs Documentation writing and editing (README.md, etc.)
  • ✨ Feature Feature
  • 🔨 Refactor Code refactoring
  • ⚙️ Setting Development environment setup
  • ✅ Test Test related (storybook, jest, etc.)

✏️ 작업 내용

  • 서버 응답에 따른 인터페이스 수정

    • ICampaign 인터페이스 필드명 수정
  • 캠페인 목록 조회 API 연동

    • /api/project/{orgId}
  • 캠페인 운영 상태별 UI 처리

    • OVER(종료) : 목록에서 제외 필터링 로직 추가
    • PAUSED(중단) : 해당 캠페인의 Row 배경색/텍스트 회색 처리
    • 동기화 상태 주석 처리 후 비율 재조정
  • 전체 캠페인 통합 제어 기능

    • /api/project/{orgId}/status API 연동 (중단/재개)
    • 캠페인 운영 상태에 따른 controlBox UI 처리
스크린샷 2026-03-16 오전 5 48 41 스크린샷 2026-03-16 오전 6 00 38

😅 미완성 작업

N/A

📢 논의 사항 및 참고 사항

  • 현재 전체 캠페인 제어 PATCH 요청은 작동하지만, 상태 데이터가 다른 API에 반영되지 않는 것 같습니다.(캠페인 목록에 업데이트 X) 이 부분은 확인 후에 다시 테스트할 예정입니다.
  • 일단 종료(OVER) 상태의 경우 해당 캠페인은 목록에 표시하지 않도록 했습니다. 종료 처리에 대해서는 추후 다른 방식으로 나타내도 좋을 것 같습니다!

💬 리뷰어 가이드 (P-Rules)
P1: 필수 반영 (Critical) - 버그 가능성, 컨벤션 위반. 해결 전 머지 불가.
P2: 적극 권장 (Recommended) - 더 나은 대안 제시. 가급적 반영 권장.
P3: 제안 (Suggestion) - 아이디어 공유. 반영 여부는 드라이버 자율.
P4: 단순 확인/칭찬 (Nit) - 사소한 오타, 칭찬 등 피드백.

Summary by CodeRabbit

릴리스 노트

  • 새 기능

    • 캠페인 일괄 제어 추가: 전체 캠페인 일시중지/재개 기능
    • 상세 페이지에서 캠페인 개별 중단/재개 확인 모달 추가
  • 개선

    • 목록에 예산 소진 현황(퍼센트) 컬럼 추가
    • 플랫폼 표기를 제공사 기반으로 변경 및 중복/미연결 처리 개선
    • 로딩·빈상태 UI, 키보드 접근성(Enter/Space) 향상
    • 종료된 캠페인 자동 제외 및 워크스페이스 연동으로 초기 로드 개선

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

조직 워크스페이스 조회 후 첫 조직을 선택해 캠페인 목록을 불러오고, 전체 캠페인 상태(ON_GOING/PAUSED) 일괄 제어 API를 추가해 UI와 상태 흐름을 연동했습니다. 타입과 Campaign 컴포넌트 Props가 재설계되어 목록 렌더링이 변경되었습니다.

Changes

Cohort / File(s) Summary
API 통합 레이어
src/api/ads/ads.ts
getCampaignList(orgId)updateAllCampaignStatus(orgId, status) 신규 API 헬퍼 추가 (axios 기반).
타입 정의 및 목 데이터
src/types/ads/campaign.ts, src/pages/ads/list/campaign.mock.ts
캠페인 타입 재설계: TCampaignStatus"ON_GOING" | "PAUSED" | "OVER"; ICampaign/ICampaignDetail 추가(프로퍼티: projectId, providers, budgetUsageRate 등); 목 데이터 형식 일치화.
목록 페이지 로직 및 상태관리
src/pages/ads/list/AdsListPage.tsx
워크스페이스 → 첫 조직 선택 → 캠페인 조회 흐름 추가, isLoading, currentOrgId, campaigns 상태 도입, 전체 일시정지/재개 버튼에서 API 호출 및 로컬 상태 동기화 구현.
테이블/행 컴포넌트 변경
src/components/ads/CampaignTable.tsx, src/components/ads/CampaignRow.tsx
Props 재설계: platformsproviders, progressbudgetUsageRate, projectId 추가; OVER 상태 필터링(표에서 제외); 렌더링 및 레이아웃 조정.
상세 페이지 제어 및 표시 변경
src/pages/ads/list/CampaignDetail.tsx
상태표시 로직 변경(ON_GOING→운영 중 등), createdAt/budget 포맷 변경, 캠페인 중단·재개 모달과 핸들러 추가.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant AdsListPage as AdsListPage (UI)
    participant API as Ads API Layer
    participant Backend as Backend

    User->>AdsListPage: 페이지 진입
    AdsListPage->>API: getMyWorkspaces()
    API->>Backend: GET /workspaces
    Backend-->>API: workspaces[]
    API-->>AdsListPage: workspaces

    AdsListPage->>AdsListPage: currentOrgId = first workspace
    AdsListPage->>API: getCampaignList(currentOrgId)
    API->>Backend: GET /api/project/{orgId}
    Backend-->>API: campaigns[]
    API-->>AdsListPage: ICampaign[]

    User->>AdsListPage: "모두 일시정지" 클릭
    AdsListPage->>API: updateAllCampaignStatus(currentOrgId, "PAUSED")
    API->>Backend: PATCH /api/project/{orgId}/status?status=PAUSED
    Backend-->>API: success
    API-->>AdsListPage: void
    AdsListPage->>AdsListPage: 로컬 campaigns 상태 동기화 / 토스트 표시
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

Suggested labels

📬 API

Suggested reviewers

  • Seojegyeong
  • jjjsun
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목이 주요 변경사항(캠페인 목록 조회 및 전체 캠페인 제어 API 연동)을 명확하게 설명하고 있으며 Issue #95와의 연관성도 잘 드러나 있습니다.
Description check ✅ Passed PR 설명이 관련 이슈, 변경사항 체크리스트, 구체적인 작업 내용, 스크린샷을 포함하여 템플릿의 주요 항목들을 충분히 작성했습니다.
Linked Issues check ✅ Passed PR이 Issue #95의 두 가지 주요 요구사항(캠페인 목록 조회 API 연동, 전체 캠페인 제어 API 연동)을 모두 구현했으며, 관련된 UI 처리 및 상태 관리도 함께 포함하고 있습니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 Issue #95의 요구사항(캠페인 목록 조회, 전체 캠페인 제어, 상태별 UI 처리)과 직접 관련되어 있으며 범위를 벗어난 변경은 없습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/#95
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

@YermIm YermIm self-assigned this Mar 15, 2026
@YermIm YermIm added the ✨ Feature 기능 개발 label Mar 15, 2026
@YermIm
YermIm requested review from Seojegyeong and jjjsun March 15, 2026 21:13
@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-gbxswsytek.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=83

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

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/AdsListPage.tsx (1)

107-167: ⚠️ Potential issue | 🟠 Major

재개 CTA를 hasActiveCampaign의 반대로만 결정하면 오동작합니다.

캠페인이 하나도 없거나 전부 OVER인 경우에도 지금 분기에서는 바로 "시작하기" 박스가 노출됩니다. 이 경우 재개 대상이 없는데 잘못된 액션을 안내하게 되고, 조직이 없는 초기/에러 상태에서는 confirm이 조용히 no-op가 됩니다. ON_GOINGPAUSED를 각각 따로 계산해서, 실제로 재개 가능한 캠페인이 있을 때만 재개 CTA를 보여 주세요.

수정 예시
-  const hasActiveCampaign = campaigns.some((c) => c.status === "ON_GOING");
+  const hasPausableCampaign = campaigns.some((c) => c.status === "ON_GOING");
+  const hasResumableCampaign = campaigns.some((c) => c.status === "PAUSED");
...
-            {hasActiveCampaign ? (
+            {currentOrgId !== null && hasPausableCampaign ? (
               <ControlBox
                 title="전체 캠페인을 완전히 중단할 수 있어요"
                 description="모든 광고 노출이 즉시 멈추고, 연결된 플랫폼에서도 더 이상 광고가 집행되지 않아요."
                 buttonText="중단하기"
                 onButtonClick={() => setStopAllOpen(true)}
                 buttonDisabled={isStopping}
                 containerClassName="bg-status-red/7 border-status-red px-6 py-4 min-w-[650px] shrink-0"
                 titleClassName="text-status-red font-heading3"
                 descriptionClassName="font-body2 text-text-sub"
                 buttonSize="big"
                 buttonClassName="font-body1 bg-status-red"
               />
-            ) : (
+            ) : currentOrgId !== null && hasResumableCampaign ? (
               <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"
                 descriptionClassName="font-body2 text-text-sub"
                 buttonSize="big"
                 buttonClassName="font-body1 bg-status-blue"
               />
-            )}
+            ) : null}
🤖 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 107 - 167, The current UI
decides between "중단하기" and "시작하기" solely on hasActiveCampaign, which shows a
"시작하기" CTA even when there are no PAUSED campaigns to resume; compute a separate
flag (e.g., const hasPausedCampaign = campaigns.some(c => c.status ===
"PAUSED")) and change the render logic so: if hasActiveCampaign show the stop
ControlBox (uses setStopAllOpen, isStopping), else if hasPausedCampaign show the
resume ControlBox (uses setResumeOpen, isResuming), otherwise render nothing or
a neutral disabled box—update references to campaigns, hasActiveCampaign,
hasPausedCampaign, setResumeOpen, setStopAllOpen, isResuming, isStopping, and
ControlBox accordingly.
🤖 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/CampaignRow.tsx`:
- Around line 26-33: The component CampaignRow currently destructures projectId
from props but never uses it, causing TS6133; remove projectId from the
destructuring list in the CampaignRow function signature while leaving projectId
in the ICampaignRowProps interface (so callers still provide it), i.e., stop
extracting projectId in the CampaignRow parameter list to eliminate the unused
variable error.

In `@src/components/ads/CampaignTable.tsx`:
- Around line 31-44: Create a filtered list (e.g., const visibleCampaigns =
campaigns?.filter(c => c.status !== "OVER") ?? []) and use
visibleCampaigns.length for the conditional render instead of campaigns.length;
map visibleCampaigns to render <CampaignRow key={projectId} {...project}
onClick={() => onRowClick?.(project.projectId)} /> and render the empty state as
an <li> (not a <div>) so it remains a direct semantic child of the surrounding
<ul>, and ensure any necessary ARIA/semantic attributes are preserved on the
empty <li>.

In `@src/pages/ads/list/AdsListPage.tsx`:
- Around line 21-48: The initData useEffect currently handles API calls, loading
and error state and local cache (getMyWorkspaces -> getCampaignList,
setCampaigns, setIsLoading, setCurrentOrgId) and should be refactored to React
Query: create custom hooks like useAdsCampaigns (which calls getMyWorkspaces
then getCampaignList and returns data/loading/error) and useUpdateCampaignStatus
for mutations, replace initData and its try/catch/setIsLoading logic with
useQuery hooks in AdsListPage, remove manual setCampaigns and instead read from
the query result, and use queryClient.invalidateQueries where needed to refresh
caches instead of manual state updates.
- Around line 17-48: The component declares isLoading/setIsLoading but never
reads it, causing TS6133 and showing the "Start" CTA while data is loading;
update the AdsListPage JSX to branch on isLoading (set by initData) so you
render a loading indicator (or skeleton) while isLoading is true, compute
hasActiveCampaign only after loading completes (use campaigns once isLoading is
false) and hide/disable the "시작하기" CTA until isLoading is false and
hasActiveCampaign is determined; ensure references to isLoading are used in the
render logic so the linter error is resolved.

In `@src/types/ads/campaign.ts`:
- Around line 5-18: The ICampaign/ICampaignDetail types don't match the mock
data and UI usage: update the type definitions or introduce a UI-specific
interface and align the mock typing. Specifically, either add the missing fields
(budget, startDate, statusText, progress, runStatus, runStatusText, ads) to
ICampaign/ICampaignDetail so CampaignDetail.tsx can safely read data.budget and
data.startDate, or create a new interface (e.g., ICampaignDetailWithUI) that
includes those UI-only fields and change MOCK_CAMPAIGNS' type to that interface
and use it in CampaignDetail.tsx; ensure references to ICampaign,
ICampaignDetail, MOCK_CAMPAIGNS, and the data.budget/data.startDate reads are
updated accordingly.

---

Outside diff comments:
In `@src/pages/ads/list/AdsListPage.tsx`:
- Around line 107-167: The current UI decides between "중단하기" and "시작하기" solely
on hasActiveCampaign, which shows a "시작하기" CTA even when there are no PAUSED
campaigns to resume; compute a separate flag (e.g., const hasPausedCampaign =
campaigns.some(c => c.status === "PAUSED")) and change the render logic so: if
hasActiveCampaign show the stop ControlBox (uses setStopAllOpen, isStopping),
else if hasPausedCampaign show the resume ControlBox (uses setResumeOpen,
isResuming), otherwise render nothing or a neutral disabled box—update
references to campaigns, hasActiveCampaign, hasPausedCampaign, setResumeOpen,
setStopAllOpen, isResuming, isStopping, and ControlBox accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 64bef2b0-302a-44c5-84fb-d98886c53bdc

📥 Commits

Reviewing files that changed from the base of the PR and between f844773 and 6dd5ffe.

📒 Files selected for processing (6)
  • src/api/ads/ads.ts
  • src/api/common/common.ts
  • src/components/ads/CampaignRow.tsx
  • src/components/ads/CampaignTable.tsx
  • src/pages/ads/list/AdsListPage.tsx
  • src/types/ads/campaign.ts

Comment thread src/components/ads/CampaignRow.tsx
Comment thread src/components/ads/CampaignTable.tsx Outdated
Comment thread src/pages/ads/list/AdsListPage.tsx
Comment thread src/pages/ads/list/AdsListPage.tsx
Comment thread src/types/ads/campaign.ts
@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-rxaqwemlaq.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=84

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

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)

77-85: ⚠️ Potential issue | 🟠 Major

플랫폼 카드가 실제 연결 플랫폼을 반영하지 않습니다.

상세 타입에 providers를 올려 놓았는데 여기서는 여전히 3개 플랫폼을 하드코딩하고 있어서, 일부 플랫폼만 연결된 캠페인도 모두 연결된 것처럼 보입니다. data.providers를 그대로 넘겨야 상세 화면과 목록 화면이 일치합니다.

수정 예시
             <PlatformCard
-              platforms={["kakao", "google", "naver"]}
+              platforms={data.providers}
               className="flex-1 min-w-[320px] w-full"
             />
🤖 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 77 - 85, The PlatformCard
is being fed a hardcoded array ["kakao","google","naver"] which makes every
campaign look fully connected; update the CampaignDetail component to pass the
actual providers from the campaign data (use data.providers) into PlatformCard
instead of the hardcoded list, and if necessary adapt or map data.providers to
the exact prop shape PlatformCard expects so the detail view reflects the real
connected platforms.
🤖 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/CampaignRow.tsx`:
- Around line 11-18: The component props declare onClick as optional in
ICampaignRowProps but the rendered row always gets role="button", keyboard
handlers, and hover cursor; update the component so it either (A) makes onClick
required in ICampaignRowProps and ensure all usages pass it, or (B) keep onClick
optional but conditionally add role="button", tabIndex, keyDown/keyboard
handlers, and hover cursor styles only when onClick is provided (i.e., check the
onClick prop before attaching interactive attributes/handlers in CampaignRow);
adjust any places that use CampaignRow accordingly to satisfy the chosen
approach.

In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 21-22: 주석 처리된 상태 분기(isPaused, isOngoing)를 복원하고 data.status에 따라 하단
제어 박스의 버튼 문구·스타일·클릭 핸들러를 분기하세요: data.status === "PAUSED"일 때는 버튼을 "재개"로 표시하고
resumeOpen 모달을 여는 재개 핸들러를 연결하며, data.status === "ON_GOING"일 때는 버튼을 "중단하기"로 표시하고
중단 핸들러(pause 처리)를 연결하도록 CampaignDetail 컴포넌트의 해당 렌더링 로직과 클릭 이벤트를 수정하세요; 기타 상태는
기본/비활성 스타일로 처리해 UI와 동작이 일치하도록 만드세요.
- Line 4: Remove the unnecessary type assertion "as ICampaignDetail" and use the
actual MOCK_CAMPAIGNS-typed data; replace the hardcoded platforms array passed
to the CampaignDetail component (platforms={["kakao","google","naver"]}) with
the real provider list from the campaign data (platforms={data.providers}) so
each campaign renders its correct platforms, and implement the commented
isPaused/isOngoing checks (complete the logic where isPaused/isOngoing are
referenced) so the component derives pause/ongoing state from the campaign
object rather than leaving those lines commented out.

---

Outside diff comments:
In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 77-85: The PlatformCard is being fed a hardcoded array
["kakao","google","naver"] which makes every campaign look fully connected;
update the CampaignDetail component to pass the actual providers from the
campaign data (use data.providers) into PlatformCard instead of the hardcoded
list, and if necessary adapt or map data.providers to the exact prop shape
PlatformCard expects so the detail view reflects the real connected platforms.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d01f9e79-c4e8-40b0-be61-ea010d475bb7

📥 Commits

Reviewing files that changed from the base of the PR and between 6dd5ffe and b6c1d72.

📒 Files selected for processing (5)
  • src/components/ads/CampaignRow.tsx
  • src/pages/ads/list/AdsListPage.tsx
  • src/pages/ads/list/CampaignDetail.tsx
  • src/pages/ads/list/campaign.mock.ts
  • src/types/ads/campaign.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/pages/ads/list/AdsListPage.tsx

Comment thread src/components/ads/CampaignRow.tsx
Comment thread src/pages/ads/list/CampaignDetail.tsx
Comment thread src/pages/ads/list/CampaignDetail.tsx
@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-gqkhdzgzcq.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=85

@jjjsun jjjsun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P4: 확인했습니다! 고생하셨어요!

@jjjsun

jjjsun commented Mar 16, 2026

Copy link
Copy Markdown
Collaborator

P4: 저도 종료상태인경우 해당 캠페인을 목록에 표시하지 않는 것이 지금으로써는 충분한것같습니다. 다만, 종료된 캠페인을 사용자가 확인하는 것을 원할 수 있기에, 추후에 한번 논의해보는 것이 좋을것같습니다!

Comment thread src/api/common/common.ts Outdated
Comment thread src/pages/ads/list/AdsListPage.tsx Outdated
@Seojegyeong

Copy link
Copy Markdown
Collaborator

P4: 저는 종료된 캠페인도 확인할 수 있도록 하는게 좋을 것 같습니다. 대면 회의 때 의논해보면 좋을 것 같습니다 :) 수고하셨습니다!

@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-uobzxtmhkp.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=90

@coderabbitai coderabbitai Bot left a comment

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 comments (2)
src/pages/ads/list/AdsListPage.tsx (2)

134-137: onRowClick 핸들러 인라인 함수 고려사항

현재 onRowClick={(id) => handleCampaignClick(id)}는 매 렌더링마다 새 함수를 생성해요. CampaignTableReact.memo로 래핑되어 있다면 불필요한 리렌더링을 유발할 수 있습니다.

간소화 제안
  <CampaignTable
    campaigns={campaigns}
-   onRowClick={(id) => handleCampaignClick(id)}
+   onRowClick={handleCampaignClick}
  />

handleCampaignClick을 직접 전달하면 동일한 동작을 하면서 코드도 간결해집니다.

🤖 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 134 - 137, The inline arrow
for onRowClick creates a new function each render which can trigger unnecessary
re-renders of CampaignTable; change the prop to pass handleCampaignClick
directly (CampaignTable onRowClick={handleCampaignClick}) or, if needed, wrap
handleCampaignClick in useCallback so its identity is stable; update the
invocation sites to continue accepting the id parameter (i.e., ensure
handleCampaignClick(id) signature remains unchanged).

66-105: 상태 업데이트 로직이 API 호출 성공 후에 실행되고 있어요.

현재 구현은 API 호출 성공 후 로컬 상태를 업데이트하는 방식인데, 이는 올바른 접근입니다. 다만 몇 가지 개선점이 있습니다:

  1. 상태 문자열 타입 안전성: "PAUSED", "ON_GOING" 문자열이 하드코딩되어 있어요. ICampaignstatus 타입과 일치하는지 확인이 필요합니다.

  2. 에러 로깅 부재: catch 블록에서 에러 객체를 사용하지 않아 디버깅 시 원인 파악이 어려울 수 있어요.

개선 제안
- const onStopAll = async () => {
+ const onStopAll = async () => {
    if (!currentOrgId) return;
    setIsStopping(true);

    try {
      await updateAllCampaignStatus(currentOrgId, "PAUSED");

      setCampaigns((prev) =>
        prev.map((c) =>
          c.status === "ON_GOING" ? { ...c, status: "PAUSED" } : c,
        ),
      );
      toast.success("전체 캠페인의 모든 광고 노출이 중단되었습니다.");
      setStopAllOpen(false);
-   } catch {
-     toast.error("중단 처리에 실패하였습니다.");
+   } catch (error) {
+     console.error("캠페인 중단 실패:", 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 66 - 105, The onStopAll and
onResumeAll handlers use hardcoded status strings ("PAUSED", "ON_GOING") and
swallow errors; update them to use your canonical status enum/type (e.g.,
ICampaignStatus or a STATUS constant used across the codebase) instead of raw
strings to ensure type safety and consistency, and modify the catch blocks in
both onStopAll and onResumeAll to accept and log the caught error (e.g., catch
(err) { console.error(err); toast.error(...) }) so failures are recorded for
debugging while keeping the existing toast messages and state updates with the
enum/constant values (refer to functions onStopAll, onResumeAll and the
updateAllCampaignStatus call to locate where to change the strings and add error
logging).
🤖 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`:
- Around line 134-137: The inline arrow for onRowClick creates a new function
each render which can trigger unnecessary re-renders of CampaignTable; change
the prop to pass handleCampaignClick directly (CampaignTable
onRowClick={handleCampaignClick}) or, if needed, wrap handleCampaignClick in
useCallback so its identity is stable; update the invocation sites to continue
accepting the id parameter (i.e., ensure handleCampaignClick(id) signature
remains unchanged).
- Around line 66-105: The onStopAll and onResumeAll handlers use hardcoded
status strings ("PAUSED", "ON_GOING") and swallow errors; update them to use
your canonical status enum/type (e.g., ICampaignStatus or a STATUS constant used
across the codebase) instead of raw strings to ensure type safety and
consistency, and modify the catch blocks in both onStopAll and onResumeAll to
accept and log the caught error (e.g., catch (err) { console.error(err);
toast.error(...) }) so failures are recorded for debugging while keeping the
existing toast messages and state updates with the enum/constant values (refer
to functions onStopAll, onResumeAll and the updateAllCampaignStatus call to
locate where to change the strings and add error logging).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ee367bce-3105-4af6-93c0-bef8097601ff

📥 Commits

Reviewing files that changed from the base of the PR and between 8338fd4 and 6ba9e5c.

📒 Files selected for processing (2)
  • src/api/ads/ads.ts
  • src/pages/ads/list/AdsListPage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/api/ads/ads.ts

@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-ovxhaoskep.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=91

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/components/ads/CampaignRow.tsx (1)

36-43: ⚠️ Potential issue | 🟡 Minor

onClick가 없을 때도 인터랙티브 행으로 노출됩니다.

Line 17에서 onClick은 optional인데, Line 36/41/43에서 hover·cursor·role="button"·키보드 핸들러가 항상 적용됩니다. isInteractive = Boolean(onClick)로 조건부 적용하거나, 반대로 onClick을 필수 prop으로 고정해 주세요.

As per coding guidelines, 7. 접근성: 시맨틱 HTML, ARIA 속성 사용 확인.

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

In `@src/components/ads/CampaignRow.tsx` around lines 36 - 43, The row is being
exposed as interactive even when onClick is optional; introduce a local flag
like isInteractive = Boolean(onClick) in CampaignRow and use it to conditionally
apply interactive behavior: only add role="button", tabIndex (e.g., 0), the
onKeyDown keyboard handler, and the hover/cursor CSS classes when isInteractive
is true (leave isPaused logic intact for background styling); update the
className template to include hover:bg-bg-surface and hover:cursor-pointer only
when isInteractive, and guard the onKeyDown and tabIndex props so
non-interactive rows are not treated as buttons.
🧹 Nitpick comments (1)
src/components/ads/CampaignRow.tsx (1)

76-80: 주석 처리된 상태 배지 블록은 정리하는 게 좋습니다.

비활성화된 JSX를 파일에 남겨두면 이후 유지보수 시 의도를 추적하기 어렵습니다. 재사용 계획이 없다면 삭제하고, 필요하면 PR 설명/이슈로 의도를 남기는 쪽이 더 깔끔합니다.

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

In `@src/components/ads/CampaignRow.tsx` around lines 76 - 80, Remove the
commented-out JSX block that renders the status badge in CampaignRow.tsx (the
disabled div containing Badge, status, and statusText); if you expect to reuse
this UI later, instead create a short PR description or open an issue linking
the intended reuse and remove the dead commented code from the file to keep the
component clean and maintainable.
🤖 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/components/ads/CampaignRow.tsx`:
- Around line 36-43: The row is being exposed as interactive even when onClick
is optional; introduce a local flag like isInteractive = Boolean(onClick) in
CampaignRow and use it to conditionally apply interactive behavior: only add
role="button", tabIndex (e.g., 0), the onKeyDown keyboard handler, and the
hover/cursor CSS classes when isInteractive is true (leave isPaused logic intact
for background styling); update the className template to include
hover:bg-bg-surface and hover:cursor-pointer only when isInteractive, and guard
the onKeyDown and tabIndex props so non-interactive rows are not treated as
buttons.

---

Nitpick comments:
In `@src/components/ads/CampaignRow.tsx`:
- Around line 76-80: Remove the commented-out JSX block that renders the status
badge in CampaignRow.tsx (the disabled div containing Badge, status, and
statusText); if you expect to reuse this UI later, instead create a short PR
description or open an issue linking the intended reuse and remove the dead
commented code from the file to keep the component clean and maintainable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 71c933ae-26db-4095-b512-87f2defe2ee9

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba9e5c and b5d4f9d.

📒 Files selected for processing (2)
  • src/components/ads/CampaignRow.tsx
  • src/components/ads/CampaignTable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/ads/CampaignTable.tsx

@github-actions

Copy link
Copy Markdown

📚 Storybook 배포 완료

항목 링크
📖 Storybook https://69a147b60a56365d9e2185ef-rlurdntwob.chromatic.com/
🔍 Chromatic https://www.chromatic.com/build?appId=69a147b60a56365d9e2185ef&number=93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ Feature 기능 개발

Projects

None yet

Development

Successfully merging this pull request may close these issues.

✨ [Feature] 캠페인 목록 조회 및 전체 캠페인 제어 API 연동

3 participants