Skip to content

Feat/#139 광고 플랫폼 연동 해제 API 추가#145

Open
ojy0903 wants to merge 15 commits into
developfrom
feat/#139
Open

Feat/#139 광고 플랫폼 연동 해제 API 추가#145
ojy0903 wants to merge 15 commits into
developfrom
feat/#139

Conversation

@ojy0903
Copy link
Copy Markdown
Collaborator

@ojy0903 ojy0903 commented Jun 2, 2026

📌 관련 이슈

🚀 개요

이번 PR에서 변경된 핵심 내용을 요약해주세요.

광고 플랫폼 연동해제와 관련 광고 엔티티 삭제 진행 API 추가

📄 작업 내용

구체적인 작업 내용을 설명해주세요.

  • PlaformConnection, PlatformAccount 삭제 로직 추가
  • 연관된 AdCampaign, MetricFact, ClickLog 삭제 로직 추가

📸 스크린샷 / 테스트 결과 (선택)

결과물 확인을 위한 사진이나 테스트 로그를 첨부해주세요.

연동 해제 API 호출 전 상태 (Meta 와 Naver 연동 상태, AdCampaign, AdGroup, AdContent, MetricFact 에 각각 3개 데이터 존재, ClickLog 에는 존재 X)

연동 해제 전 DB 상태 캡쳐 PlatformAccount, PlatformConnection DB 상태 image image AdCampaign, AdGroup, AdContent, MetricFact, ClickLog DB 상태 image image image image image

플랫폼 연동 조회 API 를 통해 네이버, 메타 연동됨을 확인 (네이버는 세부 광고 정보 없음)
image

연동 해제 API 호출 (id 가 3번인 Meta 플랫폼 연동 해제)
image

연동 해제 후 DB 상태 캡쳐 PlaformAccount, PlaformConnection image image

AdCampaign, AdGroup, AdContent, MetricFact, ClickLog DB 상태
image
image
image
image
image

✅ 체크리스트

  • 브랜치 전략(GitHub Flow)을 준수했나요?
  • 메서드 단위로 코드가 잘 쪼개져 있나요?
  • 테스트 통과 확인
  • 서버 실행 확인
  • API 동작 확인

🔍 리뷰 포인트 (Review Points)

리뷰어가 중점적으로 확인했으면 하는 부분을 적어주세요. (P1~P4 적용 가이드)

  • MetricFact 와 ClickLog 와 같이 데이터가 많은 부분은 청크 단위로 삭제하는 것이 좋을 것 같아 PlatformDataCleanupExecutor 클래스에 삭제 로직을 별도로 구현했습니다.
  • AdCampaign, AdGroup, AdContent 가 CascadeType.ALL 로 묶여있어 AdCampaign 만 지워서 하위의 AdGroup, AdContent 가 자동 삭제되도록 했는데 괜찮을까요?
  • AdCampaign, AdGroup, AdContent 가 삭제되어서 하위에 AdCampaign 이 아예 없는 Project 엔티티도 삭제되도록 했는데 삭제되도록 두는게 좋을까요 아니면 남겨두는게 좋을까요...?
  • 그 외에 수정이나 리팩터링 필요한 부분 있으면 알려주세요...!!

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

Summary by CodeRabbit

  • New Features

    • 광고 플랫폼 연동 해제(사용자·시스템 단위) 및 대규모 데이터 청크 삭제로 안전한 계정 정리 지원.
    • 회원 탈퇴 스케줄러에 연동 자동 해제 추가 및 빈 프로젝트 자동 제거.
  • Errors / Security

    • 조직 소속·소유자 권한 관련 403 오류 코드 및 메시지 추가.
  • Documentation

    • 연동 해제 API 문서 공개 및 명세 추가.
  • Chores

    • 더 이상 사용되지 않는 사용자 연동 관련 오류 코드 제거.

@ojy0903 ojy0903 self-assigned this Jun 2, 2026
@ojy0903 ojy0903 added the ✨ Feature 새로운 기능 추가 label Jun 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

Walkthrough

플랫폼 계정 연동 해제 시 권한·소속 검증 후 AdCampaign 기반 projectId를 수집하고, ClickLog/MetricFact를 LIMIT 기반 청크로 반복 삭제한 뒤 AdCampaign·PlatformConnection·PlatformAccount를 제거하고 빈 Project를 정리합니다. API가 OpenAPI에 노출됩니다.

Changes

플랫폼 계정 연동 해제 및 데이터 정리

Layer / File(s) Summary
저장소 계층 - 배치 삭제 및 조회 메서드
src/main/java/.../advertisement/persistence/repository/AdCampaignRepository.java, src/main/java/.../advertisement/persistence/repository/MetricFactRepository.java, src/main/java/.../click/persistence/repository/ClickLogRepository.java, src/main/java/.../platform/persistence/repository/PlatformConnectionRepository.java
AdCampaign의 플랫폼계정 기준 조회·distinct projectId 수집, project별 AdCampaign 카운트, ClickLog/MetricFact의 LIMIT 기반 네이티브 배치 삭제 메서드 및 PlatformConnection 조회/집계 메서드가 추가되었습니다.
정리 오케스트레이션 - PlatformDataCleanupExecutor
src/main/java/.../platform/domain/service/PlatformDataCleanupExecutor.java
사용자·조직·계정 소속·소유자 검증 후 projectIds를 반환하고, REQUIRES_NEW 트랜잭션으로 ClickLog/MetricFact 청크 삭제를 반복 수행하며 계정·관계 삭제와 빈 Project 조건부 삭제를 조율합니다.
서비스 레이어 통합 및 에러 코드
src/main/java/.../platform/domain/service/PlatformService.java, src/main/java/.../platform/domain/service/PlatformServiceImpl.java, src/main/java/.../platform/exception/code/PlatformErrorCode.java
disconnectPlatform 계약 추가, PlatformServiceImpl에서 PlatformDataCleanupExecutor 위임 구현 및 트랜잭션 전파(NOT_SUPPORTED) 적용, 시스템용 disconnectAccountBySystem 추가, 접근 제어용 403 에러 코드(PLATFORM_403_2, PLATFORM_403_3) 2개가 추가되었습니다.
API 노출 및 문서화
src/main/java/.../platform/presentation/PlatformController.java, src/main/java/.../platform/presentation/docs/PlatformControllerDocs.java
PlatformController의 @Hidden 제거로 엔드포인트가 OpenAPI에 노출되고, PlatformControllerDocs에 disconnectPlatform의 OpenAPI 메타데이터와 시그니처가 추가되었습니다.
조직 Soft Delete (회원 탈퇴 흐름 전용)
src/main/java/.../organization/domain/service/OrgService.java, src/main/java/.../organization/domain/service/OrgServiceImpl.java
회원 탈퇴 전용 조직 소프트 삭제 계약과 구현이 추가되어 조직 멤버의 currentOrgId 초기화 후 조직을 softDelete합니다.
회원 탈퇴 흐름 · 스케줄러 통합
src/main/java/.../user/domain/service/UserService.java, src/main/java/.../user/domain/service/scheduler/UserDeleteScheduler.java
UserService에서 플랫폼 연동 검증 로직 제거 및 조직 삭제 호출 방식 변경, UserDeleteScheduler에 하드 삭제 전 플랫폼 계정 단위 연동 해제 호출을 추가해 자동 정리 흐름을 삽입했습니다.
sequenceDiagram
  actor Client
  participant PlatformController
  participant PlatformServiceImpl
  participant PlatformDataCleanupExecutor
  participant ClickLogRepository
  participant MetricFactRepository
  participant AdCampaignRepository
  participant PlatformAccountRepository
  participant ProjectRepository

  Client->>PlatformController: DELETE /orgs/{orgId}/platforms/{accountId}
  PlatformController->>PlatformServiceImpl: disconnectPlatform(userId, orgId, accountId)
  PlatformServiceImpl->>PlatformDataCleanupExecutor: verifyAndCollectProjectIds(...)
  PlatformDataCleanupExecutor->>AdCampaignRepository: findDistinctProjectIdsByPlatformAccountId(accountId)
  PlatformDataCleanupExecutor-->>PlatformServiceImpl: projectIds
  loop for each chunk
    PlatformServiceImpl->>PlatformDataCleanupExecutor: deleteClickLogChunk(accountId)
    PlatformDataCleanupExecutor->>ClickLogRepository: deleteByPlatformAccountIdInBatch(accountId, batchSize)
    ClickLogRepository-->>PlatformDataCleanupExecutor: deletedCount
    PlatformServiceImpl->>PlatformDataCleanupExecutor: deleteMetricFactChunk(accountId)
    PlatformDataCleanupExecutor->>MetricFactRepository: deleteByPlatformAccountIdInBatch(accountId, batchSize)
    MetricFactRepository-->>PlatformDataCleanupExecutor: deletedCount
  end
  PlatformServiceImpl->>PlatformDataCleanupExecutor: deleteAccountAndRelations(accountId)
  PlatformDataCleanupExecutor->>PlatformAccountRepository: delete(account)
  loop for project in projectIds
    PlatformDataCleanupExecutor->>AdCampaignRepository: countByProject_Id(projectId)
    PlatformDataCleanupExecutor->>MetricFactRepository: (count by project)
    PlatformDataCleanupExecutor->>ProjectRepository: delete(projectId) [if both counts == 0]
  end
  PlatformServiceImpl-->>Client: 200 OK
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

  • WhereYouAd/WhereYouAd-Backend#135: disconnectPlatform API 계약과 스켈레톤 구현을 도입한 PR로, 본 PR은 해당 흐름을 완전하게 연결하고 청크 삭제를 구현합니다.

Suggested Reviewers

  • kingmingyu
  • jinnieusLab
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.68% 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 추가'는 이번 PR의 핵심 목표를 잘 반영하고 있습니다.
Description check ✅ Passed PR 설명이 템플릿의 필수 섹션을 충족하고 있습니다. 관련 이슈, 개요, 작업 내용이 명확하며, 실제 동작 검증을 위한 DB 상태 스크린샷과 API 호출 결과를 포함하고 있습니다.
Linked Issues check ✅ Passed PR이 #139의 모든 주요 요구사항을 충족합니다. PlatformConnection/PlatformAccount 삭제 로직, 관련 광고 엔티티(AdCampaign, MetricFact, ClickLog) 삭제 로직, 청크 단위 최적화 삭제가 PlatformDataCleanupExecutor에 구현되어 있습니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 플랫폼 연동 해제 및 관련 데이터 정리라는 명확한 범위 내에 있습니다. 다만 UserService와 UserDeleteScheduler의 변경은 추가적으로 검토가 필요합니다.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/#139

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java (1)

152-183: 부분 실패 시 데이터 정합성에 대한 안내/대비를 권장합니다.

NOT_SUPPORTED로 전체를 하나의 트랜잭션으로 묶지 않기 때문에, 예를 들어 ClickLog·MetricFact 청크는 지워졌는데 deleteAccountAndRelations 단계에서 예외가 나면 "지표는 사라졌지만 계정/캠페인은 남아 있는" 중간 상태가 그대로 커밋된 채로 남습니다.

다행히 청크 삭제와 검증 로직이 멱등에 가까워 동일 요청을 재시도하면 복구되는 구조이긴 합니다. 다만 운영 관점에서:

  • 실패 시 재시도가 안전하다는 점을 로그/문서에 남기고,
  • 알림(모니터링)으로 중간 실패를 잡을 수 있게 해두면

장애 상황에서 데이터가 어중간하게 남는 것을 빠르게 인지·복구할 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java`
around lines 152 - 183, The disconnectPlatform flow (method disconnectPlatform)
can leave a partially-cleaned state if later steps like
platformDataCleanupExecutor.deleteAccountAndRelations(...) fail after
click/metric chunk deletions; add a defensive try-catch around the sequence of
chunk deletions + deleteAccountAndRelations + deleteEmptyProject loop to detect
partial failure, emit a structured error log including userId/orgId/accountId
and totals (totalClickLogDeleted, totalMetricFactDeleted), record a
monitoring/alert metric (e.g., PlatformCleanupPartialFailure) with those fields,
and either rethrow after logging or persist a retry marker so operators can
safely retry (document that retries are idempotent in logs). Ensure the
try-catch is placed in disconnectPlatform and that
platformDataCleanupExecutor.deleteAccountAndRelations(...) and the loop over
projectIds are included in the protected section so any exception triggers the
alert/logging path.
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java (1)

233-241: ⚡ Quick win

MySQL 기준에서는 DELETE ... LIMIT 호환성 이슈가 크지 않습니다

MetricFactRepository.deleteByPlatformAccountIdInBatch의 native DELETE FROM metric_fact ... LIMIT :batchSize현재 구성(MySQL Dialect/driver + docker-compose의 mysql:8.0) 기준이면 동작 가능성이 높습니다(src/main/resources/application.ymlcom.mysql.cj.jdbc.Driver / org.hibernate.dialect.MySQLDialect). 또한 테스트 환경에서 H2/PostgreSQL 전환 설정이 보이지 않아(별도 application-test.* / H2 의존성 없음) 기본 실행 경로에서 깨질 가능성은 낮습니다.
다만 로컬/CI에서 DB_URL로 다른 DB(H2/PostgreSQL)로 바꾸면 동일 문법이 실패할 수 있으니, 그 경우에만 DB별 쿼리로 분기하는 쪽이 안전합니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java`
around lines 233 - 241, The native query in
MetricFactRepository.deleteByPlatformAccountIdInBatch uses "DELETE ... LIMIT
:batchSize", which can fail on non-MySQL DBs; update the repository to use a
DB-agnostic batch-delete approach: implement a two-step deletion in the
service/repository layer that first selects a batch of primary keys (e.g.,
select id from metric_fact where platform_account_id = :platformAccountId order
by id limit :batchSize) using a portable select, then call deleteAllByIdInBatch
or a JPQL delete using those ids, or provide separate native queries per dialect
and choose by current Hibernate dialect; ensure you modify
MetricFactRepository.deleteByPlatformAccountIdInBatch (or add a new repository
method used by the service) so the delete works across MySQL, H2, and
PostgreSQL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/docs/PlatformControllerDocs.java`:
- Around line 81-83: The ApiResponse labels in PlatformControllerDocs.java are
inconsistent with the actual error domains; update the `@ApiResponse` entries in
the PlatformControllerDocs class so each responseCode and its description use
the correct domain prefix and actual error identifier (e.g., use PLATFORM_404_1
/ PLATFORM_CONNECTION_NOT_FOUND for platform-related 404s and USER_404_1 /
USER_NOT_FOUND for user-related 404s) to match the thrown errors (like
UserErrorCode.USER_NOT_FOUND and platform error enums).

---

Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java`:
- Around line 233-241: The native query in
MetricFactRepository.deleteByPlatformAccountIdInBatch uses "DELETE ... LIMIT
:batchSize", which can fail on non-MySQL DBs; update the repository to use a
DB-agnostic batch-delete approach: implement a two-step deletion in the
service/repository layer that first selects a batch of primary keys (e.g.,
select id from metric_fact where platform_account_id = :platformAccountId order
by id limit :batchSize) using a portable select, then call deleteAllByIdInBatch
or a JPQL delete using those ids, or provide separate native queries per dialect
and choose by current Hibernate dialect; ensure you modify
MetricFactRepository.deleteByPlatformAccountIdInBatch (or add a new repository
method used by the service) so the delete works across MySQL, H2, and
PostgreSQL.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java`:
- Around line 152-183: The disconnectPlatform flow (method disconnectPlatform)
can leave a partially-cleaned state if later steps like
platformDataCleanupExecutor.deleteAccountAndRelations(...) fail after
click/metric chunk deletions; add a defensive try-catch around the sequence of
chunk deletions + deleteAccountAndRelations + deleteEmptyProject loop to detect
partial failure, emit a structured error log including userId/orgId/accountId
and totals (totalClickLogDeleted, totalMetricFactDeleted), record a
monitoring/alert metric (e.g., PlatformCleanupPartialFailure) with those fields,
and either rethrow after logging or persist a retry marker so operators can
safely retry (document that retries are idempotent in logs). Ensure the
try-catch is placed in disconnectPlatform and that
platformDataCleanupExecutor.deleteAccountAndRelations(...) and the loop over
projectIds are included in the protected section so any exception triggers the
alert/logging path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7f4e8a49-e8eb-495f-ae48-d1347d554cd6

📥 Commits

Reviewing files that changed from the base of the PR and between 6a59971 and 19f3009.

📒 Files selected for processing (10)
  • src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/AdCampaignRepository.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/click/persistence/repository/ClickLogRepository.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformService.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/code/PlatformErrorCode.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/PlatformController.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/docs/PlatformControllerDocs.java
💤 Files with no reviewable changes (2)
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformService.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/PlatformController.java

Copy link
Copy Markdown
Collaborator

@kingmingyu kingmingyu left a comment

Choose a reason for hiding this comment

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

P2: 고생하셨습니다! metric_fact나 clicklog같은 데이터들은 커서 청크 단위로 분리해서 삭제하는 것도 좋은 것 같습니다! 근데 이렇게 트랜잭션을 분리하게 되면 어떤 데이터는 삭제되고 어떤 데이터는 삭제되지 않은 경우 데이터 정합성이 깨지는 문제가 발생할 수 있을 것 같습니다
그래서 제 생각에는 API에서 바로 처리하기보다 유저 soft delete 이후 30일이 지난 시점에 스케줄러에서 청크 단위로 정리하는 방식으로 가져가는 것도 괜찮아 보입니다. 이렇게 하면 한 번에 큰 트랜잭션을 잡지 않아도 되고 중간에 일부 청크가 실패하더라도 다음 스케줄 실행 시 재시도할 수 있을 것 같습니다. 이후 관련 데이터 정리가 모두 끝난 경우에만 유저 hard delete를 진행하면 더 안전할 것 같습니다. 추가로 만약 이렇게 한다면 회원 탈퇴시에 플랫폼 연동 해제를 요청하지 않아도 될 것 같습니다.
근데 이렇게 하면 사용자가 그냥 플랫폼 연동을 해제하고 싶은 경우에는 어떻게 처리할지 고민이 될 것 같습니다...

@ojy0903
Copy link
Copy Markdown
Collaborator Author

ojy0903 commented Jun 3, 2026

P2: 고생하셨습니다! metric_fact나 clicklog같은 데이터들은 커서 청크 단위로 분리해서 삭제하는 것도 좋은 것 같습니다! 근데 이렇게 트랜잭션을 분리하게 되면 어떤 데이터는 삭제되고 어떤 데이터는 삭제되지 않은 경우 데이터 정합성이 깨지는 문제가 발생할 수 있을 것 같습니다 그래서 제 생각에는 API에서 바로 처리하기보다 유저 soft delete 이후 30일이 지난 시점에 스케줄러에서 청크 단위로 정리하는 방식으로 가져가는 것도 괜찮아 보입니다. 이렇게 하면 한 번에 큰 트랜잭션을 잡지 않아도 되고 중간에 일부 청크가 실패하더라도 다음 스케줄 실행 시 재시도할 수 있을 것 같습니다. 이후 관련 데이터 정리가 모두 끝난 경우에만 유저 hard delete를 진행하면 더 안전할 것 같습니다. 추가로 만약 이렇게 한다면 회원 탈퇴시에 플랫폼 연동 해제를 요청하지 않아도 될 것 같습니다. 근데 이렇게 하면 사용자가 그냥 플랫폼 연동을 해제하고 싶은 경우에는 어떻게 처리할지 고민이 될 것 같습니다...

UserDeleteScheduler 쪽에서 플랫폼 연동해제 메서드를 호출하게 하는 방식으로 하면 가능할 것 같습니다! 이 방식으로 하면 회원 탈퇴 시 연동 해제를 요청하지 않아도 되고, 지금 만들어진 플랫폼 연동해제 API 엔드포인트도 그대로 유지해서 사용자가 단순히 수동으로 플랫폼 연동을 해제만하고 싶은 경우를 커버하도록 하면 어떨까요??
다만 이 수정은 시간이 조금 걸릴거 같아서... 시험 준비하면서 천천히 해야할 듯 하긴 합니다...

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java (1)

107-107: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

오타 수정이 필요합니다.

"함꼐" → "함께"로 수정해주세요.

📝 수정 제안
-            //AdCampaign 삭제 시 CascadeType.ALL 로 인해 연관된 AdGroup, AdContent 도 함꼐 제거됨
+            //AdCampaign 삭제 시 CascadeType.ALL 로 인해 연관된 AdGroup, AdContent 도 함께 제거됨
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java`
at line 107, Fix the typo in the comment inside PlatformDataCleanupExecutor:
change "함꼐" to "함께" in the comment that explains cascade deletion of AdGroup and
AdContent when deleting AdCampaign (the comment near the CascadeType.ALL note in
PlatformDataCleanupExecutor).
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java (1)

120-127: 빈 프로젝트 삭제 로직이 안전하게 설계되었습니다.

AdCampaign과 MetricFact 카운트를 모두 체크하여 완전히 비어있을 때만 삭제하는 방어적 설계가 좋네요. 특히 Project가 여러 PlatformAccount의 캠페인을 포함할 수 있는 구조에서, count 체크로 다른 계정의 데이터가 남아있는 경우를 올바르게 처리합니다 ✓

참고로, MetricFact 청크 삭제가 중간에 실패할 경우 일부 MetricFact가 남을 수 있고, 이 경우 count > 0이므로 Project는 안전하게 보존됩니다. 다만 AdCampaign은 이미 삭제되어 orphan MetricFact가 발생할 수 있는데, 이는 PR 코멘트에서 ojy0903님이 언급하신 "청크 분리로 인한 부분 삭제 리스크"에 해당합니다.

이 트레이드오프(대용량 처리 vs. 완전한 원자성)는 이미 팀에서 논의하고 선택하신 설계 결정이므로, 향후 운영 중 orphan 데이터가 발견되면 배치 정리 스크립트로 보완하시면 될 것 같습니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java`
around lines 120 - 127, The current deleteEmptyProject method may delete a
Project if adCampaignRepository.countByProject_Id(...) and
metricFactRepository.countByProject_Id(...) return 0, but partial/async
deletions could leave orphan MetricFact after AdCampaigns were removed; to fix,
re-check both counts immediately before calling
projectRepository.deleteById(projectId) (i.e., perform a final
verification/read-after-delete within deleteEmptyProject) so you only delete
when both repositories still report zero, using the same
adCampaignRepository.countByProject_Id and
metricFactRepository.countByProject_Id calls in the deleteEmptyProject method to
locate the check and delete logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.java`:
- Around line 40-48: The scheduler currently calls
platformService.disconnectAccountBySystem(accountId) for each account then
hardDeleteSingleUser(userId), but
PlatformDataCleanupExecutor.deleteAccountAndRelations(Long accountId) throws
PLATFORM_ACCOUNT_NOT_FOUND via
platformAccountRepository.findById(accountId).orElseThrow(...) which will break
retries if an account was already removed; update deleteAccountAndRelations (or
the disconnect wrapper called by UserDeleteScheduler) to treat
PLATFORM_ACCOUNT_NOT_FOUND as a no-op (log and return/continue) instead of
rethrowing so the operation is idempotent, preserving other error behavior and
keeping deleteClickLogChunk (REQUIRES_NEW) semantics intact.

---

Outside diff comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java`:
- Line 107: Fix the typo in the comment inside PlatformDataCleanupExecutor:
change "함꼐" to "함께" in the comment that explains cascade deletion of AdGroup and
AdContent when deleting AdCampaign (the comment near the CascadeType.ALL note in
PlatformDataCleanupExecutor).

---

Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java`:
- Around line 120-127: The current deleteEmptyProject method may delete a
Project if adCampaignRepository.countByProject_Id(...) and
metricFactRepository.countByProject_Id(...) return 0, but partial/async
deletions could leave orphan MetricFact after AdCampaigns were removed; to fix,
re-check both counts immediately before calling
projectRepository.deleteById(projectId) (i.e., perform a final
verification/read-after-delete within deleteEmptyProject) so you only delete
when both repositories still report zero, using the same
adCampaignRepository.countByProject_Id and
metricFactRepository.countByProject_Id calls in the deleteEmptyProject method to
locate the check and delete logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2afd04b8-3a18-49a6-997b-8e9c86bfe5cf

📥 Commits

Reviewing files that changed from the base of the PR and between ac597b0 and a8ce99a.

📒 Files selected for processing (9)
  • src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgService.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformDataCleanupExecutor.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformService.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.java
  • src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java
💤 Files with no reviewable changes (1)
  • src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgService.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java

Comment on lines +40 to 48
// 광고 플랫폼 연동 자동 해제 (계정 + 연관 광고 엔티티/ClickLog/MetricFact/비어있는 Project 정리)
// PlatformAccount → Organization FK 위반 방지를 위해 조직 Hard Delete 전에 수행
List<Long> accountIds = platformConnectionRepository.findDistinctAccountIdsByUserId(userId);
for (Long accountId : accountIds) {
platformService.disconnectAccountBySystem(accountId);
}

// 회원/조직/멤버 Hard Delete
userDeleteExecutor.hardDeleteSingleUser(userId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: PlatformDataCleanupExecutor에서 이미 삭제된 계정 처리 로직 확인

# collectProjectIds와 deleteAccountAndRelations에서 account가 없을 때 처리 확인
rg -n -A 10 'collectProjectIds|deleteAccountAndRelations' --type=java -g '**PlatformDataCleanupExecutor*'

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 3600


플랫폼 연동 해제 멱등성(재실행 내구성) 보강 필요

  • UserDeleteScheduler에서 disconnectAccountBySystem(accountId)로 플랫폼 정리 후 hardDeleteSingleUser(userId)를 호출하는 순서는 적절합니다.
  • 다만 PlatformDataCleanupExecutor.deleteAccountAndRelations(Long accountId)platformAccountRepository.findById(accountId).orElseThrow(...)PLATFORM_ACCOUNT_NOT_FOUND가 발생하면 예외를 던집니다. 그래서 스케줄러 재실행/부분 실패 시 “이미 삭제된 accountId”가 섞이면 해당 시도 전체가 중단될 수 있어요.
  • PLATFORM_ACCOUNT_NOT_FOUND는 “이미 정리됨”으로 보고 no-op(또는 로깅 후 continue) 처리하도록 예외 흐름을 조정해 멱등성을 확보하는 걸 권장합니다. (반대로 deleteClickLogChunkREQUIRES_NEW로 청크 커밋을 분리한 설계라 타임아웃 위험 저감 의도는 좋습니다.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.java`
around lines 40 - 48, The scheduler currently calls
platformService.disconnectAccountBySystem(accountId) for each account then
hardDeleteSingleUser(userId), but
PlatformDataCleanupExecutor.deleteAccountAndRelations(Long accountId) throws
PLATFORM_ACCOUNT_NOT_FOUND via
platformAccountRepository.findById(accountId).orElseThrow(...) which will break
retries if an account was already removed; update deleteAccountAndRelations (or
the disconnect wrapper called by UserDeleteScheduler) to treat
PLATFORM_ACCOUNT_NOT_FOUND as a no-op (log and return/continue) instead of
rethrowing so the operation is idempotent, preserving other error behavior and
keeping deleteClickLogChunk (REQUIRES_NEW) semantics intact.

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.

feat: 광고 플랫폼 연동 해제 API - PlatformConnection, PlatformAccount 와 관련 광고 엔티티 삭제

2 participants