Conversation
# Conflicts: # src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.java
WalkthroughMeta 캠페인과 광고그룹의 예산 수정 PATCH API가 추가되었습니다. 요청 검증, Meta 연동, 예산 변경 서비스, 동기화 업서트 반영, 관련 오류 코드와 응답 DTO가 함께 확장되었습니다. ChangesMeta 예산 수정 API 전체 흐름
Sequence Diagram(s)sequenceDiagram
participant Client
participant MetaAdApiController
participant MetaAdApiService
participant MetaBudgetService
participant MetaClient
Client->>MetaAdApiController: PATCH /api/meta/campaigns/{id}/budget
MetaAdApiController->>MetaAdApiService: updateCampaignBudget(userId, campaignId, request)
MetaAdApiService->>MetaBudgetService: updateCampaignBudget(userId, campaignId, request)
MetaBudgetService->>MetaClient: updateBudget(nodeId, access_token, daily_budget/lifetime_budget)
MetaClient-->>MetaBudgetService: UpdateResponse(success, id)
MetaBudgetService-->>MetaAdApiService: BudgetUpdateResponse
MetaAdApiService-->>MetaAdApiController: BudgetUpdateResponse
MetaAdApiController-->>Client: 200 OK DataResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.java (2)
66-69: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win주석 처리된 기존 코드는 삭제해 주세요.
버전 관리 시스템(Git)에 히스토리가 남아 있으므로 주석 처리된 코드를 유지할 필요가 없습니다. 코드 가독성을 위해 삭제하는 것이 좋습니다.
🤖 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/entity/AdGroup.java` around lines 66 - 69, Remove the commented-out updateBudget method in the AdGroup class that contains the parameters budget and bidAmount. Since version control systems like Git maintain the complete history of all code changes, there is no need to keep commented-out code in the repository. Simply delete these lines entirely to improve code readability and maintainability.
71-81: 🧹 Nitpick | 🔵 Trivial | 💤 Low value분기 로직을
if-else if-else구조로 개선하면 더 안전합니다.현재 두 개의 독립적인
if문을 사용하고 있는데, Provider가 상호 배타적이므로 기능적으로는 문제없습니다. 하지만 향후 새로운 Provider가 추가될 경우 예상치 못한 동작이 발생할 수 있어요. 예를 들어Provider.GOOGLE이 추가되면 현재 코드에서는 아무 동작도 하지 않고 조용히 넘어가게 됩니다.♻️ 개선 제안
public void updateBudget(Long budget, Long bidAmount) { - if (this.adCampaign.getProvider() == Provider.NAVER) { + Provider provider = this.adCampaign.getProvider(); + + if (provider == Provider.NAVER) { if (budget != null) this.budget = budget; if (bidAmount != null) this.bidAmount = bidAmount; - } - - if (this.adCampaign.getProvider() == Provider.META) { + } else if (provider == Provider.META) { this.budget = budget; if (bidAmount != null) this.bidAmount = bidAmount; + } else { + // 지원하지 않는 Provider는 기존 로직(null 체크) 적용 + if (budget != null) this.budget = budget; + if (bidAmount != null) this.bidAmount = bidAmount; } }🤖 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/entity/AdGroup.java` around lines 71 - 81, The updateBudget method uses two independent if statements to check for different Provider types, but since these conditions are mutually exclusive, the second if statement checking for Provider.META should be converted to an else if statement. This change makes the code structure clearer and prevents unexpected behavior if new providers are added in the future, as it ensures only one branch will execute instead of silently doing nothing for unknown providers.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.java (1)
218-232: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMeta 에러 코드를 상수로 추출하면 유지보수성이 향상됩니다.
1487079,1885630같은 Magic Number는 Meta API의 특정 에러 코드인데, 상수로 추출하고 주석을 달아두면 나중에 코드를 보는 팀원들이 이해하기 쉬워요.현재 주석이 잘 달려 있어서 큰 문제는 아니지만, 참고용으로 제안드립니다.
♻️ 개선 제안 (선택사항)
// Meta Marketing API Error Codes // Reference: https://developers.facebook.com/docs/marketing-api/error-reference private static final String META_ERROR_BUDGET_TOO_LOW = "1487079"; private static final String META_ERROR_BUDGET_TYPE_MISMATCH = "1885630"; private boolean isBudgetTooLowError(FeignException e) { String body = e.contentUTF8(); if (body == null) return false; String lower = body.toLowerCase(); return body.contains(META_ERROR_BUDGET_TOO_LOW) || lower.contains("minimum") || lower.contains("budget is too low"); } private boolean isBudgetTypeMismatchError(FeignException e) { String body = e.contentUTF8(); if (body == null) return false; return body.contains(META_ERROR_BUDGET_TYPE_MISMATCH); }🤖 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/domain/service/adapi/meta/MetaBudgetService.java` around lines 218 - 232, Extract the hardcoded Meta API error codes as static final constants to improve code maintainability. Create two constants META_ERROR_BUDGET_TOO_LOW with value "1487079" and META_ERROR_BUDGET_TYPE_MISMATCH with value "1885630" at the class level, then replace the hardcoded strings in the isBudgetTooLowError method (changing body.contains("1487079") to body.contains(META_ERROR_BUDGET_TOO_LOW)) and in the isBudgetTypeMismatchError method (changing body.contains("1885630") to body.contains(META_ERROR_BUDGET_TYPE_MISMATCH)). This makes the code more maintainable and allows team members to easily understand what these error codes represent.
🤖 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/advertisement/domain/service/adapi/meta/MetaBudgetService.java`:
- Around line 66-69: The comparison in the MetaBudgetService class where
previousBudget.equals(request.amount()) is called will throw a
NullPointerException when previousBudget is null (which can occur for campaigns
with no budget set in the database). Replace the direct .equals() call with
Objects.equals(previousBudget, request.amount()) which safely handles null
values without throwing an exception. Ensure that java.util.Objects is imported
in the file.
- Around line 119-123: The code in the budget comparison logic within
MetaBudgetService.java has a null pointer exception risk because previousBudget
from adGroup.getBudget() can be null when budget is only set at the campaign
level. Replace the direct call to previousBudget.equals(request.dailyBudget())
with Objects.equals(previousBudget, request.dailyBudget()) to safely handle null
values in the comparison.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.java`:
- Around line 99-103: The `@ApiResponse` annotation with responseCode "400" in
MetaAdApiControllerDocs for the campaign budget update endpoint is missing
documentation for the ADAPI_400_9 error code. Add the ADAPI_400_9 error code
description to the 400 response description string to match the actual behavior
of MetaBudgetService.updateCampaignBudget, which throws a SAME_BUDGET_AMOUNT
error when the requested budget amount is identical to the existing budget
amount. Include this new error code documentation in the same format as the
existing error codes (ADAPI_400_2, ADAPI_400_6, ADAPI_400_8).
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/converter/MetaConverter.java`:
- Around line 273-284: The fromMetaBudget method performs integer division on
line 283 when converting non-KRW currencies, which truncates fractional values
and causes precision loss during round-trip conversions (e.g., 2550 divided by
100 gives 25 instead of 25.5, then converts back to 2500). Replace the integer
division operation with proper rounding logic that preserves precision for
non-KRW currency conversions, ensuring that when minorBudget is divided by 100
and later converted back through toMetaBudget, the original value is maintained
without budget drift.
- Around line 66-84: The budget parsing in the toAdGroup method does not handle
parsing failures properly. When parseLong fails to parse src.dailyBudget(), it
can result in a 0 value being saved to the database, which overwrites existing
budget data with an invalid value. Add error handling around the parseLong call
to ensure that if the dailyBudget cannot be parsed as a Long, the budget remains
null instead of defaulting to 0, preventing invalid budget overwrites in the
database during the MetaUpsertService.upsertAdGroups operation.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.java`:
- Around line 218-232: Extract the hardcoded Meta API error codes as static
final constants to improve code maintainability. Create two constants
META_ERROR_BUDGET_TOO_LOW with value "1487079" and
META_ERROR_BUDGET_TYPE_MISMATCH with value "1885630" at the class level, then
replace the hardcoded strings in the isBudgetTooLowError method (changing
body.contains("1487079") to body.contains(META_ERROR_BUDGET_TOO_LOW)) and in the
isBudgetTypeMismatchError method (changing body.contains("1885630") to
body.contains(META_ERROR_BUDGET_TYPE_MISMATCH)). This makes the code more
maintainable and allows team members to easily understand what these error codes
represent.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.java`:
- Around line 66-69: Remove the commented-out updateBudget method in the AdGroup
class that contains the parameters budget and bidAmount. Since version control
systems like Git maintain the complete history of all code changes, there is no
need to keep commented-out code in the repository. Simply delete these lines
entirely to improve code readability and maintainability.
- Around line 71-81: The updateBudget method uses two independent if statements
to check for different Provider types, but since these conditions are mutually
exclusive, the second if statement checking for Provider.META should be
converted to an else if statement. This change makes the code structure clearer
and prevents unexpected behavior if new providers are added in the future, as it
ensures only one branch will execute instead of silently doing nothing for
unknown providers.
🪄 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: 6f7462d7-1692-4c86-a80b-8b6e459c1561
📒 Files selected for processing (14)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/MetaAdApiService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaSyncService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaUpsertService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/MetaAdApiController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/exception/code/AdApiErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/client/MetaClient.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/config/MetaAdConfig.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/converter/MetaConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/dto/MetaDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/dto/MetaResponse.java
kingmingyu
approved these changes
Jun 23, 2026
kingmingyu
left a comment
kingmingyu
left a comment
Collaborator
There was a problem hiding this comment.
P4: 고생하셨습니다! 기존 코드에 맞춰서 잘 구현하신 것 같습니다!
jinnieusLab
approved these changes
Jun 25, 2026
jinnieusLab
left a comment
jinnieusLab
left a comment
Collaborator
There was a problem hiding this comment.
P4: 고생하셨습니다! Meta 예산 수정에 맞게 구체적으로 코드를 예외 처리와 함께 신경 써서 구현해주신 것 같네요 👍 제가 남긴 리뷰도 확인해주시면 될 것 같아요!
| return lifetimeBudget != null ? "LIFETIME" : "DAILY"; | ||
| } | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
P3: 제가 이해한 것이 맞다면, dailyBudget만 쓰기로 한 것이라면 이 DTO에서 아예 lifetimeBudget 필드를 제거하는 게 어떨까요?
현재 광고 그룹 쪽 로직을 보면 lifetimeBudget이 들어올 경우 에러를 던지도록 되어 있는데, 애초에 클라이언트 측에서 요청할 수 없도록 구조적으로 닫아버리는 게 더 안전하고 명확할 것 같습니다!
아니면 예산도 일일예산인지 총예산인지 구분하는 필드가 필요할 수도 있겠네요...
Collaborator
Author
There was a problem hiding this comment.
죄송합니다... 광고 그룹도 일일예산(dailyBudget) / 전체 예산(lifetimeBudget) 두개를 선택해서 할당 가능한데 제가 Meta 광고 관리자에서 광고 그룹에 전체 예산 할당이 계속 취소되길래 안되는 걸로 알았는데 전체 예산 할당 가능하네요... 이거에 맞춰서 로직 수정 하겠습니다.ㅠㅠ 광고 그룹도 두 예산 형식이 모두 가능하다보니 제 생각에는 DTO 는 이대로 유지하는게 좋을 것 같아요..
@jinnieusLab @kingmingyu AdCampaign 과 AdGroup 에 광고 예산 형식 필드를 두는 것도 생각은 해봤었는데(ex. enum 으로 budgetType 추가?) 필드를 추가하게 되면 메타에서는 활용 가능하더라도 네이버나 구글에서는 아예 쓰지 않는 필드가 되거나 유형명이 플랫폼마다 다를 수 있을 거 같아서 일단 추가하지 않았었는데 추가하는게 좋을까요...? 이건 다들 확인하고 진행하는게 좋을 것 같아서 언급 해봅니다..!
Collaborator
There was a problem hiding this comment.
네넵! 전 AdCampaign에 budgetType 추가하는 거 좋은 거 같아요! 구글 같은 경우에는 둘 다 가능하고(거의 다 일일예산이긴 하지만 총예산 수정도 일부 기능에 있다고 하네요..), 네이버는 일일예산만 지원하는 것 같아서 해당 필드값은 일일예산으로 고정해서 사용하면 될 듯 합니다!
There was a problem hiding this comment.
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/advertisement/domain/service/adapi/meta/MetaBudgetService.java (1)
196-198: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
access_token누락도 인증 오류로 처리해주세요.Line 197에서 map 조회 결과가
null이어도 그대로 반환되어 Meta 호출까지 진행될 수 있습니다. 토큰 생성은 성공했지만 키가 누락된 경우도INVALID_API_CREDENTIALS로 끊어주면 원인 추적이 더 명확합니다.🛡️ 수정 제안
try { - return adApiAuthUtil.generateAuthHeaders(connection.getId(), AdAuthRequest.empty()).get("access_token"); + String token = adApiAuthUtil.generateAuthHeaders(connection.getId(), AdAuthRequest.empty()).get("access_token"); + if (token == null || token.isBlank()) { + throw new AdApiHandler(AdApiErrorCode.INVALID_API_CREDENTIALS); + } + return token; } catch (Exception e) {🤖 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/domain/service/adapi/meta/MetaBudgetService.java` around lines 196 - 198, In MetaBudgetService, the auth flow in generateAuthHeaders should treat a missing access_token the same as an authentication failure instead of returning null. Update the try block around adApiAuthUtil.generateAuthHeaders(...) so you explicitly read the token from the returned map, validate it, and throw or map to INVALID_API_CREDENTIALS when the access_token key is absent or null. Keep the fix localized to the MetaBudgetService token retrieval path and preserve the existing error handling used for invalid credentials.
🤖 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/advertisement/domain/service/adapi/meta/MetaBudgetService.java`:
- Around line 106-152: The updateAdGroupBudget flow in MetaBudgetService
currently accepts and propagates lifetimeBudget even though AdGroup should only
allow dailyBudget. Update the request validation and budget conversion logic in
updateAdGroupBudget so it rejects or ignores lifetimeBudget inputs, and only
uses dailyBudget when calling callMetaBudgetUpdate, replacing the entity budget,
saving budgetHistoryRepository, and building the BudgetUpdateResponse. Keep the
fix localized around updateAdGroupBudget and the
request.amount()/request.dailyBudget()/request.lifetimeBudget() usage so the
AdGroup contract remains dailyBudget-only.
---
Outside diff comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.java`:
- Around line 196-198: In MetaBudgetService, the auth flow in
generateAuthHeaders should treat a missing access_token the same as an
authentication failure instead of returning null. Update the try block around
adApiAuthUtil.generateAuthHeaders(...) so you explicitly read the token from the
returned map, validate it, and throw or map to INVALID_API_CREDENTIALS when the
access_token key is absent or null. Keep the fix localized to the
MetaBudgetService token retrieval path and preserve the existing error handling
used for invalid credentials.
🪄 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: 79a70b32-30b4-46bc-bb2e-4764bae8e5ad
📒 Files selected for processing (3)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/code/MetaAdErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.java
There was a problem hiding this comment.
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/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.java (1)
41-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
budgetType()/amount()에 유효성 방어 로직을 추가해주세요.현재는 검증이 선행되지 않은 경로에서
dailyBudget/lifetimeBudget가 둘 다null이어도budgetType()이DAILY로 계산될 수 있습니다. 이 경우 잘못된 예산 타입으로 후속 로직이 진행될 수 있어요.isExactlyOne()조건을 메서드 내부에서도 한 번 더 보장(예: 예외 발생)해 주세요.🤖 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/application/dto/request/AdvertisementRequest.java` around lines 41 - 47, `AdvertisementRequest`의 `amount()`와 `budgetType()`는 `dailyBudget`/`lifetimeBudget`가 둘 다 null이거나 둘 다 값이 있는 경우를 방어하지 못합니다. `isExactlyOne()` 조건을 `amount()`, `budgetType()` 내부에서도 다시 확인하도록 `AdvertisementRequest`의 해당 메서드에 유효성 검사를 추가하고, 조건이 깨지면 예외를 던져 잘못된 `BudgetType` 계산과 금액 반환을 막아주세요.
🤖 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/advertisement/persistence/entity/AdCampaign.java`:
- Around line 55-57: `AdCampaign`와 관련된 `budgetType` 엔티티 변경만으로는 운영 DB에 반영되지 않으므로,
`AdCampaign`/`AdGroup`에 공통으로 추가된 `budget_type` 컬럼을 스키마에 반영하는 마이그레이션을 추가하세요.
`src/main/resources/db/migration`(또는 Liquibase changelog)에서 최신 버전에
`ad_campaign`과 `ad_group` 테이블 각각에 `budget_type` 컬럼을 추가하는 변경을 포함시키고, 마이그레이션 관리
폴더가 없다면 새 버전 SQL 파일을 생성해 함께 커밋하세요. 특히 `AdCampaign`, `AdGroup`, `budgetType` 관련
변경과 스키마가 일치하도록 맞추는 것이 핵심입니다.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.java`:
- Around line 50-52: The new AdGroup.budgetType mapping requires a matching
database schema change, but no Flyway migration exists for the ad_group
budget_type column. Add a new V*.sql migration under the existing db/migration
flow to alter the ad_group table and create budget_type with the correct
type/constraints, keeping it aligned with the AdGroup entity’s `@Column`(name =
"budget_type") field. Ensure the migration name and placement match the
project’s existing Flyway versioning conventions so the schema is applied before
JPA starts using BudgetType.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/converter/MetaConverter.java`:
- Around line 240-260: `MetaConverter.resolveBudget()` and `parseMinorBudget()`
currently let `BudgetType` be set even when parsing fails, which can produce a
`BudgetInfo(null, ...)` and overwrite existing budget values. Make parsing
success and budget-type assignment atomic by normalizing/trimming once before
parsing, only creating a `BudgetInfo` when the budget string parses
successfully, and otherwise returning no budget update so `upsert` does not
clear the existing amount. Use the existing `isActiveBudget()`,
`parseMinorBudget()`, and `BudgetInfo` flow to keep the amount and type in sync.
---
Outside diff comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.java`:
- Around line 41-47: `AdvertisementRequest`의 `amount()`와 `budgetType()`는
`dailyBudget`/`lifetimeBudget`가 둘 다 null이거나 둘 다 값이 있는 경우를 방어하지 못합니다.
`isExactlyOne()` 조건을 `amount()`, `budgetType()` 내부에서도 다시 확인하도록
`AdvertisementRequest`의 해당 메서드에 유효성 검사를 추가하고, 조건이 깨지면 예외를 던져 잘못된 `BudgetType`
계산과 금액 반환을 막아주세요.
🪄 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: 58646486-9726-4b4b-965a-6aa0fb28dffa
📒 Files selected for processing (12)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/BudgetType.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaSyncService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaUpsertService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/code/MetaAdErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdGroup.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/converter/MetaConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/dto/MetaDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/dto/MetaResponse.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/BudgetType.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/meta/dto/MetaResponse.java
- src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaSyncService.java
- src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/MetaAdApiControllerDocs.java
- src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/adapi/meta/MetaBudgetService.java
Comment on lines
+55
to
+57
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "budget_type") | ||
| private BudgetType budgetType; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: budget_type 컬럼 추가 마이그레이션 존재 여부 확인.
fd -i '(migration|changelog|schema|V[0-9].*)' . -t f -x sh -c '
printf "\n## %s\n" "$1"
rg -n "budget_type|ad_campaign|ad_group" "$1" || true
' sh {}
printf "\n## Entity mappings\n"
rg -n "budget_type|`@Table`\\(name = \"ad_campaign\"|`@Table`\\(name = \"ad_group\"" src/main/java || trueRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 748
🏁 Script executed:
#!/bin/bash
# migration 폴더 구조 및 내용 확인
fd -d 2 "migration" -t d || echo "Migration directory not found in common locations"
fd -d 2 "changelog" -t d || echo "Changelog directory not found in common locations"
# 구체적인 마이그레이션 파일 검색 시도 (src/main/resources 경로 기준)
find src/main/resources -name "V*.sql" -o -name "*.xml" -o -name "*.xml" 2>/dev/null | head -20 || trueRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 167
🏁 Script executed:
#!/bin/bash
# 상세 검색: resources 디렉토리 내 migration/changelog 파일 확인
find src/main/resources -type f \( -name "*.sql" -o -name "*.xml" -o -name "*migration*" -o -name "*changelog*" \) 2>/dev/null | head -20 || echo "File search completed (empty if none found)."
ls -la src/main/resources/db/ 2>/dev/null || echo "src/main/resources/db/ directory not found."
ls -la src/main/resources/ 2>/dev/null | grep -i "migration\|changelog" || echo "No migration/changelog files/dirs in resources root."Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 264
DB 마이그레이션 스크립트 추가가 필요합니다.
AdCampaign 및 AdGroup 엔티티에 budget_type 컬럼이 추가되었으나, 이를 적용할 DB 스크립트(Vxxx.sql 등) 가 현재 PR 에 포함て 있지 않습니다.
엔티티 클래스만 변경하고 마이그레이션 파일이 없으면, hibernate.ddl-auto 설정이 ddl-auto=none인 운영 환경에서는 열이 없는 상태로 테이블이 읽히거나 저장 오류(Column not found) 가 발생할 수 있습니다.
확인 및 조치 사항:
src/main/resources/db/migration(또는 Liquibase 사용 시changelog) 폴더의 최신 마이그레이션 파일에 다음 2 개 변경이 포함되어 있는지 확인해주세요.ad_campaign테이블에budget_type컬럼 추가 (ALTER TABLE)ad_group테이블에budget_type컬럼 추가 (ALTER TABLE)
- 만약 마이그레이션 관리 폴더가 아예 없다면, 이번 PR 에
V버전 파일 (예:V004__add_budget_type_to_campaigns.sql) 을 새로 생성하여 추가해주세요.
엔티티 코드는 깔끔하게 잘 작성되셨고, applyBudgetType 메서드를 통해 상태 변경 로직도 잘 캡슐화하셨습니다. 다만, 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/entity/AdCampaign.java`
around lines 55 - 57, `AdCampaign`와 관련된 `budgetType` 엔티티 변경만으로는 운영 DB에 반영되지
않으므로, `AdCampaign`/`AdGroup`에 공통으로 추가된 `budget_type` 컬럼을 스키마에 반영하는 마이그레이션을
추가하세요. `src/main/resources/db/migration`(또는 Liquibase changelog)에서 최신 버전에
`ad_campaign`과 `ad_group` 테이블 각각에 `budget_type` 컬럼을 추가하는 변경을 포함시키고, 마이그레이션 관리
폴더가 없다면 새 버전 SQL 파일을 생성해 함께 커밋하세요. 특히 `AdCampaign`, `AdGroup`, `budgetType` 관련
변경과 스키마가 일치하도록 맞추는 것이 핵심입니다.
Comment on lines
+50
to
+52
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "budget_type") | ||
| private BudgetType budgetType; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: ad_group budget_type 컬럼 추가 마이그레이션 확인.
fd -i '(migration|changelog|schema|V[0-9].*)' . -t f -x sh -c '
printf "\n## %s\n" "$1"
rg -n "budget_type|ad_group" "$1" || true
' sh {}Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 167
ad_group 테이블의 budget_type 컬럼 추가를 위한 마이그레이션 스크립트가 누락되었습니다.
AdGroup 엔티티에 새로 추가된 budgetType 필드를 JPA가 인식하려면, 실제 DB 스키마가 먼저 변경되어야 합니다. 하지만 현재 프로젝트의 db/migration 폴더 또는 관련 시나리오 파일에서 해당 컬럼 추가 (budget_type) 을 위한 Flyway 스크립트 (V*.sql) 를 찾을 수 없었습니다.
엔티티 코드만 변경하고 마이그레이션을 깜빡이면, 애플리케이션 실행 시 곧바로 Column "budget_type" does not exist 오류가 발생하여 서비스 전체가 중지될 수 있으니 꼭 스크립트 파일 (Vx.x.x__add_budget_type_to_ad_group.sql) 을 생성하여 배포 일정에 포함해주세요.
기존의 다른 로직 수정사항은 문제없이 적용되어 있습니다.
🤖 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/entity/AdGroup.java`
around lines 50 - 52, The new AdGroup.budgetType mapping requires a matching
database schema change, but no Flyway migration exists for the ad_group
budget_type column. Add a new V*.sql migration under the existing db/migration
flow to alter the ad_group table and create budget_type with the correct
type/constraints, keeping it aligned with the AdGroup entity’s `@Column`(name =
"budget_type") field. Ensure the migration name and placement match the
project’s existing Flyway versioning conventions so the schema is applied before
JPA starts using BudgetType.
Comment on lines
+240
to
+260
| // AdSet 예산(문자열, Meta 최소 단위) → 로컬 budget 단위 변환. 파싱 실패 시 null | ||
| private static Long parseMinorBudget(String raw, Currency currency) { | ||
| try { | ||
| long minorBudget = Long.parseLong(raw); | ||
| return fromMetaBudget(minorBudget, currency); | ||
| } catch (NumberFormatException e) { | ||
| log.warn("[META] 예산 파싱 실패 - value: {}", raw); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| // 예산 추출 결과 — 로컬 통화 단위 금액 + 예산 유형 | ||
| private record BudgetInfo(Long amount, BudgetType type) {} | ||
|
|
||
| // lifetime/daily 중 설정된 예산을 로컬 통화 단위 금액 + 유형으로 변환 (Campaign·AdSet 공통) | ||
| private static BudgetInfo resolveBudget(String lifetimeBudget, String dailyBudget, Currency currency) { | ||
| if (isActiveBudget(lifetimeBudget)) { | ||
| return new BudgetInfo(parseMinorBudget(lifetimeBudget, currency), BudgetType.TOTAL); | ||
| } | ||
| if (isActiveBudget(dailyBudget)) { | ||
| return new BudgetInfo(parseMinorBudget(dailyBudget, currency), BudgetType.DAILY); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
예산 파싱 성공 여부와 BudgetType 저장을 함께 묶어주세요.
isActiveBudget()는 trim() 후 파싱하지만 parseMinorBudget()는 원문을 그대로 파싱합니다. 예를 들어 " 2500 "은 활성 예산으로 판단된 뒤 파싱 실패하여 BudgetInfo(null, DAILY)가 되고, upsert에서 기존 예산을 null로 덮으면서 타입만 남을 수 있습니다.
🔧 제안 수정안
private static Long parseMinorBudget(String raw, Currency currency) {
try {
- long minorBudget = Long.parseLong(raw);
+ long minorBudget = Long.parseLong(raw.trim());
return fromMetaBudget(minorBudget, currency);
} catch (NumberFormatException e) {
log.warn("[META] 예산 파싱 실패 - value: {}", raw);
return null;
}
@@
private static BudgetInfo resolveBudget(String lifetimeBudget, String dailyBudget, Currency currency) {
if (isActiveBudget(lifetimeBudget)) {
- return new BudgetInfo(parseMinorBudget(lifetimeBudget, currency), BudgetType.TOTAL);
+ Long amount = parseMinorBudget(lifetimeBudget, currency);
+ if (amount != null) {
+ return new BudgetInfo(amount, BudgetType.TOTAL);
+ }
}
if (isActiveBudget(dailyBudget)) {
- return new BudgetInfo(parseMinorBudget(dailyBudget, currency), BudgetType.DAILY);
+ Long amount = parseMinorBudget(dailyBudget, currency);
+ if (amount != null) {
+ return new BudgetInfo(amount, BudgetType.DAILY);
+ }
}
return new BudgetInfo(null, null);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // AdSet 예산(문자열, Meta 최소 단위) → 로컬 budget 단위 변환. 파싱 실패 시 null | |
| private static Long parseMinorBudget(String raw, Currency currency) { | |
| try { | |
| long minorBudget = Long.parseLong(raw); | |
| return fromMetaBudget(minorBudget, currency); | |
| } catch (NumberFormatException e) { | |
| log.warn("[META] 예산 파싱 실패 - value: {}", raw); | |
| return null; | |
| } | |
| } | |
| // 예산 추출 결과 — 로컬 통화 단위 금액 + 예산 유형 | |
| private record BudgetInfo(Long amount, BudgetType type) {} | |
| // lifetime/daily 중 설정된 예산을 로컬 통화 단위 금액 + 유형으로 변환 (Campaign·AdSet 공통) | |
| private static BudgetInfo resolveBudget(String lifetimeBudget, String dailyBudget, Currency currency) { | |
| if (isActiveBudget(lifetimeBudget)) { | |
| return new BudgetInfo(parseMinorBudget(lifetimeBudget, currency), BudgetType.TOTAL); | |
| } | |
| if (isActiveBudget(dailyBudget)) { | |
| return new BudgetInfo(parseMinorBudget(dailyBudget, currency), BudgetType.DAILY); | |
| // AdSet 예산(문자열, Meta 최소 단위) → 로컬 budget 단위 변환. 파싱 실패 시 null | |
| private static Long parseMinorBudget(String raw, Currency currency) { | |
| try { | |
| long minorBudget = Long.parseLong(raw.trim()); | |
| return fromMetaBudget(minorBudget, currency); | |
| } catch (NumberFormatException e) { | |
| log.warn("[META] 예산 파싱 실패 - value: {}", raw); | |
| return null; | |
| } | |
| } | |
| // 예산 추출 결과 — 로컬 통화 단위 금액 + 예산 유형 | |
| private record BudgetInfo(Long amount, BudgetType type) {} | |
| // lifetime/daily 중 설정된 예산을 로컬 통화 단위 금액 + 유형으로 변환 (Campaign·AdSet 공통) | |
| private static BudgetInfo resolveBudget(String lifetimeBudget, String dailyBudget, Currency currency) { | |
| if (isActiveBudget(lifetimeBudget)) { | |
| Long amount = parseMinorBudget(lifetimeBudget, currency); | |
| if (amount != null) { | |
| return new BudgetInfo(amount, BudgetType.TOTAL); | |
| } | |
| } | |
| if (isActiveBudget(dailyBudget)) { | |
| Long amount = parseMinorBudget(dailyBudget, currency); | |
| if (amount != null) { | |
| return new BudgetInfo(amount, BudgetType.DAILY); | |
| } | |
| } | |
| return new BudgetInfo(null, null); | |
| } |
🤖 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/infrastructure/client/meta/converter/MetaConverter.java`
around lines 240 - 260, `MetaConverter.resolveBudget()` and `parseMinorBudget()`
currently let `BudgetType` be set even when parsing fails, which can produce a
`BudgetInfo(null, ...)` and overwrite existing budget values. Make parsing
success and budget-type assignment atomic by normalizing/trimming once before
parsing, only creating a `BudgetInfo` when the budget string parses
successfully, and otherwise returning no budget update so `upsert` does not
clear the existing amount. Use the existing `isActiveBudget()`,
`parseMinorBudget()`, and `BudgetInfo` flow to keep the amount and type in sync.
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.
📌 관련 이슈
🚀 개요
Meta 광고 플랫폼 광고 캠페인(AdCampaign), 광고 그룹(AdGroup) 예산값 변경 API 추가
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
기존 DB 의 AdCampaign 과 메타 마케팅 콘솔에서의 예산값 (id = 1 인 AdCampaign 은 AdGroup 에 예산 할당되어있음, 나머지 캠페인은 모두 일일예산 2000원)
id = 2 인 AdCampaign 의 예산을 2000원 -> 2500원 수정 요청 결과
BudgetHistory 엔티티 정상 생성
기존 DB 의 AdGroup 과 메타 마케팅 콘솔에서의 예산값 (id = 1 인 AdGroup 만 일일예산 2000원 할당 상태)
id = 1 인 AdGroup 의 예산을 2000원 -> 2500원 수정 요청 결과
BudgetHistory 엔티티 정상 생성
+++ 추가 : 캠페인 전체기간 예산 (lifetimeBudget) 정상 변경 확인
기존 전체기간 예산 광고 캠페인 (DB id = 4)
id = 4 인 캠페인에 대한 lifetimeBudget 수정 성공
BudgetHistoy 엔티티 정상 생성
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
릴리스 노트