Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough광고 플랫폼 연동을 위해 새로운 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
📝 추가 검토 의견✅ 잘 설계된 부분
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | 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 연동 시 필요한 엔티티 설계'라는 구체적인 내용을 포함하고 있습니다. |
| Description check | ✅ Passed | PR 설명이 템플릿의 모든 주요 섹션을 포함하고 있습니다: 관련 이슈(#94), 개요(엔티티 추가 설명), 작업 내용(상세 목록), 테스트 결과(테이블 스크린샷), 체크리스트(완료 표시), 리뷰 포인트(상세한 피드백 요청)이 모두 기재되어 있습니다. |
| Linked Issues check | ✅ Passed | PR이 링크된 이슈 #94의 요구사항을 대부분 충족합니다. PlatformAccount, PlatformConnection 엔티티 설계 완료, JPA 저장소 인터페이스 추가, MetricFact/AdCampaign의 연관관계 주석 처리 등 이슈의 목표가 구현되었습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 링크된 이슈 #94의 범위 내에 있습니다. 광고 플랫폼 엔티티 설계, 저장소 인터페이스 추가, enum 상수 정의, 기존 엔티티의 연관관계 주석 처리로 모두 이슈의 목표와 일치합니다. |
✏️ 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/#94
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 help to get the list of available commands and usage tips.
![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 (4)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.java (1)
71-72: 양방향 연관관계 활성화 시 편의 메서드를 같이 넣어주세요.Line [71]-[72]를 실제로 열 때
addCampaignPlatform/removeCampaignPlatform메서드로 양쪽(AdCampaign↔CampaignPlatform.adCampaign)을 함께 세팅하지 않으면, 영속성 컨텍스트에서 관계 불일치가 발생할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.java` around lines 71 - 72, Enable the OneToMany relationship (the commented campaignPlatforms field) and add bidirectional convenience methods on AdCampaign: implement addCampaignPlatform(CampaignPlatform cp) to initialize campaignPlatforms if null, add cp to the list and call cp.setAdCampaign(this); and removeCampaignPlatform(CampaignPlatform cp) to remove from the list and call cp.setAdCampaign(null); ensure these methods keep both sides in sync to avoid persistence inconsistencies with CampaignPlatform.adCampaign.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java (1)
66-69: 연관관계 활성화는 “단계적 마이그레이션” 기준을 먼저 고정해두는 게 좋습니다.Line [66] TODO처럼 보류한 판단은 안전합니다. 다만 실제 활성화 시에는
platform_account_id백필 → FK/인덱스 추가 → 서비스 코드 전환 순서로 배포 단계를 문서화해 두세요. 이 순서를 안 잡으면 배포 시점 장애가 나기 쉽습니다.As per coding guidelines,
JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지, FetchType.LAZY가 잘 적용되었는지 체크하라.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java` around lines 66 - 69, Un-commenting the PlatformAccount relation in MetricFact must be done with a documented phased migration: add the field annotated with `@ManyToOne`(fetch = FetchType.LAZY) and `@JoinColumn`(name = "platform_account_id") in class MetricFact, then perform a data backfill of platform_account_id for existing rows, add the DB foreign key and index in a separate migration, and only after those are in place switch service code to use the relation; also run/verify JPA queries to ensure FetchType.LAZY is honored and no N+1 queries occur (use joins or fetch graphs where needed) before final deployment.src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java (2)
58-60: 연관관계 동기화 헬퍼 메서드를 추가하면 누락 저장을 줄일 수 있습니다.
campaignPlatforms만 조작하고 owning side(CampaignPlatform.platformAccount)를 안 맞추면 영속화가 어긋날 수 있습니다.add/remove헬퍼를 두고 양쪽을 같이 세팅하는 패턴을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java` around lines 58 - 60, Add bidirectional sync helper methods to PlatformAccount: implement addCampaignPlatform(CampaignPlatform cp) to set cp.setPlatformAccount(this) and add cp to campaignPlatforms if not present, and removeCampaignPlatform(CampaignPlatform cp) to remove cp from campaignPlatforms and call cp.setPlatformAccount(null); ensure null checks and idempotency so the owning side CampaignPlatform.platformAccount is always kept in sync with the PlatformAccount.campaignPlatforms collection to avoid missing persistence updates.
58-60: 연관관계 활성화는 단계적으로 여는 전략이 안전합니다.현재처럼 중간 엔티티를 먼저 도입한 건 매우 좋은 접근입니다. 다음 단계는
- FK/UNIQUE 제약 반영,
- 백필/정합성 체크,
- 그 다음에
AdCampaign/MetricFact역방향 연관 오픈
순서로 가면 기존 mock/로직 영향 최소화에 유리합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java` around lines 58 - 60, The PlatformAccount->CampaignPlatform one-to-many mapping (campaignPlatforms in PlatformAccount and CampaignPlatform entity) should remain the single active association for now; do not open the AdCampaign or MetricFact inverse relationships yet. Instead, add the corresponding FK and UNIQUE constraints in the schema for CampaignPlatform, implement a backfill/consistency migration to populate and validate those FKs, and only after successful validation enable the inverse mappings on AdCampaign and MetricFact; keep cascade/orphanRemoval behavior on campaignPlatforms as-is but avoid adding bidirectional getters/setters or `@OneToMany/`@ManyToOne on AdCampaign/MetricFact until the DB constraints and backfill checks are completed.
🤖 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/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Timezone.java`:
- Around line 3-4: Timezone enum currently declares ASIA and SEOUL which don't
represent a concrete IANA timezone and can break time calculations; update the
Timezone enum (replace ASIA and SEOUL) to concrete entries (e.g., ASIA_SEOUL)
that carry the canonical timezone ID/String (like "Asia/Seoul"), add an accessor
(e.g., getZoneId() or toZoneId()) that returns the ZoneId/ID string for
API/storage use, and adjust any persistence/serialization logic that
reads/writes Timezone so it uses the canonical ID accessor instead of the raw
enum name.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/CampaignPlatform.java`:
- Around line 8-27: Add non-null and uniqueness constraints to the N:M join:
update the CampaignPlatform entity to mark the FK join columns non-nullable (set
nullable=false on the `@JoinColumn` for adCampaign and platformAccount) and add a
table-level unique constraint for the combination (e.g.,
`@Table`(name="campaign_platform",
uniqueConstraints=@UniqueConstraint(columnNames={"ad_campaign_id","platform_account_id"})))
to prevent duplicate mappings; also add a DB migration to enforce the NOT NULL
and UNIQUE constraints at the database level.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`:
- Around line 46-48: The PlatformAccount.status column currently declares
length=10 which is too short for values like PlatformStatus.DISCONNECTED (12
chars); update the JPA mapping on the PlatformAccount.status field to increase
the length to at least 12 (or a safe larger value) and add nullable = false to
the `@Column` to prevent nulls, while keeping the `@ColumnDefault`("'ACTIVE'")
unchanged; locate the PlatformAccount class and modify the annotations on the
status field accordingly.
- Around line 35-43: Add NOT NULL constraints to the PlatformAccount entity for
the enum fields by updating the currency and timezone column mappings: for the
fields named currency and timezone in class PlatformAccount, add nullable=false
to their `@Column` annotations (you can keep the existing `@ColumnDefault`("'KRW'")
and `@ColumnDefault`("'ASIA'") if desired) so the database enforces non-null
values like the provider field does.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.java`:
- Around line 25-28: The PlatformConnection entity is missing DB-level not-null
constraints: add nullable = false to the authType column and to the foreign key
join columns for the user and platformAccount associations; specifically update
the `@Column` on authType (with `@ColumnDefault`("'API_KEY'")) to include nullable =
false, and add nullable = false to the `@JoinColumn` annotations for the user
(user_id) and platformAccount (platform_account_id) fields so that authType,
user_id and platform_account_id are enforced as NOT NULL at the database level.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.java`:
- Around line 71-72: Enable the OneToMany relationship (the commented
campaignPlatforms field) and add bidirectional convenience methods on
AdCampaign: implement addCampaignPlatform(CampaignPlatform cp) to initialize
campaignPlatforms if null, add cp to the list and call cp.setAdCampaign(this);
and removeCampaignPlatform(CampaignPlatform cp) to remove from the list and call
cp.setAdCampaign(null); ensure these methods keep both sides in sync to avoid
persistence inconsistencies with CampaignPlatform.adCampaign.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java`:
- Around line 66-69: Un-commenting the PlatformAccount relation in MetricFact
must be done with a documented phased migration: add the field annotated with
`@ManyToOne`(fetch = FetchType.LAZY) and `@JoinColumn`(name = "platform_account_id")
in class MetricFact, then perform a data backfill of platform_account_id for
existing rows, add the DB foreign key and index in a separate migration, and
only after those are in place switch service code to use the relation; also
run/verify JPA queries to ensure FetchType.LAZY is honored and no N+1 queries
occur (use joins or fetch graphs where needed) before final deployment.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`:
- Around line 58-60: Add bidirectional sync helper methods to PlatformAccount:
implement addCampaignPlatform(CampaignPlatform cp) to set
cp.setPlatformAccount(this) and add cp to campaignPlatforms if not present, and
removeCampaignPlatform(CampaignPlatform cp) to remove cp from campaignPlatforms
and call cp.setPlatformAccount(null); ensure null checks and idempotency so the
owning side CampaignPlatform.platformAccount is always kept in sync with the
PlatformAccount.campaignPlatforms collection to avoid missing persistence
updates.
- Around line 58-60: The PlatformAccount->CampaignPlatform one-to-many mapping
(campaignPlatforms in PlatformAccount and CampaignPlatform entity) should remain
the single active association for now; do not open the AdCampaign or MetricFact
inverse relationships yet. Instead, add the corresponding FK and UNIQUE
constraints in the schema for CampaignPlatform, implement a backfill/consistency
migration to populate and validate those FKs, and only after successful
validation enable the inverse mappings on AdCampaign and MetricFact; keep
cascade/orphanRemoval behavior on campaignPlatforms as-is but avoid adding
bidirectional getters/setters or `@OneToMany/`@ManyToOne on AdCampaign/MetricFact
until the DB constraints and backfill checks are completed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c28280ce-acf9-4e08-a2dc-3d48b68f8c72
📒 Files selected for processing (19)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/application/dto/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/application/mapper/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/AuthType.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Currency.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/PlatformStatus.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Timezone.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/code/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/CampaignPlatform.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/CampaignPlatformRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformAccountRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/docs/.gitkeep
Comment on lines
+3
to
+4
| public enum Timezone { | ||
| ASIA, SEOUL |
There was a problem hiding this comment.
Timezone 값이 실제 타임존 의미를 정확히 담지 못합니다.
Line [4]의 ASIA, SEOUL은 단일 타임존(Asia/Seoul)을 표현하기 어려워, 저장값/조회값 해석이 깨질 가능성이 큽니다. 플랫폼 API 연동 시 시간 집계 오차로 이어질 수 있어 지금 단계에서 정리하는 게 안전합니다.
🔧 제안 수정안
public enum Timezone {
- ASIA, SEOUL
+ ASIA_SEOUL("Asia/Seoul");
+
+ private final String zoneId;
+
+ Timezone(String zoneId) {
+ this.zoneId = zoneId;
+ }
+
+ public String zoneId() {
+ return zoneId;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Timezone.java`
around lines 3 - 4, Timezone enum currently declares ASIA and SEOUL which don't
represent a concrete IANA timezone and can break time calculations; update the
Timezone enum (replace ASIA and SEOUL) to concrete entries (e.g., ASIA_SEOUL)
that carry the canonical timezone ID/String (like "Asia/Seoul"), add an accessor
(e.g., getZoneId() or toZoneId()) that returns the ZoneId/ID string for
API/storage use, and adjust any persistence/serialization logic that
reads/writes Timezone so it uses the canonical ID accessor instead of the raw
enum name.
Comment on lines
+8
to
+27
| @Table(name = "campaign_platform") | ||
| @Getter | ||
| public class CampaignPlatform { | ||
|
|
||
| //AdCampaign - PlatformAccount 간 N:M 매핑을 위한 중간 테이블 | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "campaign_platform_id") | ||
| private Long id; | ||
|
|
||
| // AdCampaign과의 N:1 관계 | ||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "ad_campaign_id") | ||
| private AdCampaign adCampaign; | ||
|
|
||
| // PlatformAccount와의 N:1 관계 | ||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "platform_account_id") | ||
| private PlatformAccount platformAccount; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src/main/java -type f -name "CampaignPlatform.java" | head -5Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 170
🏁 Script executed:
find . -type f -name "*.java" -path "*/platform/persistence/entity/*" | head -20Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 373
🏁 Script executed:
find . -type f \( -name "*Migration*" -o -name "*Schema*" -o -name "*.sql" \) | head -20Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/CampaignPlatform.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1063
🏁 Script executed:
find src/main/java -type f -name "AdCampaign.java"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 169
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.java | head -100Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 2973
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2527
🏁 Script executed:
find src/main/java -type f -name "*CampaignPlatform*Repository*" -o -name "*CampaignPlatform*Service*"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 184
🏁 Script executed:
rg "CampaignPlatform" src/main/java --type java -A 3 -B 1 | head -100Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 3548
🏁 Script executed:
rg "CampaignPlatform" src/main/java/com/whereyouad/WhereYouAd/domains/platform --type java -A 5 | grep -E "(save|create|add|where|find)" -iRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1887
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/CampaignPlatformRepository.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 441
N:M 중간 테이블의 데이터 무결성 제약을 추가해야 합니다.
현재 FK 컬럼들(ad_campaign_id, platform_account_id)이 기본값으로 nullable 상태이며 중복 방지 제약이 없어서, 같은 조합의 매핑이 여러 번 저장될 수 있습니다. 예를 들어 AdCampaign(1번)과 PlatformAccount(10번)의 매핑이 실수로 2개 저장되면, 집계나 조회 시 잘못된 결과가 나올 수 있습니다.
PlatformAccount에 cascade와 orphanRemoval이 잘 설정되어 있는 것은 좋으나, 테이블 수준의 제약도 필요합니다:
제안 수정안
-@Table(name = "campaign_platform")
+@Table(
+ name = "campaign_platform",
+ uniqueConstraints = `@UniqueConstraint`(
+ name = "uk_campaign_platform_campaign_account",
+ columnNames = {"ad_campaign_id", "platform_account_id"}
+ )
+)
`@Getter`
public class CampaignPlatform {
@@
- `@JoinColumn`(name = "ad_campaign_id")
+ `@JoinColumn`(name = "ad_campaign_id", nullable = false)
private AdCampaign adCampaign;
@@
- `@JoinColumn`(name = "platform_account_id")
+ `@JoinColumn`(name = "platform_account_id", nullable = false)
private PlatformAccount platformAccount;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/CampaignPlatform.java`
around lines 8 - 27, Add non-null and uniqueness constraints to the N:M join:
update the CampaignPlatform entity to mark the FK join columns non-nullable (set
nullable=false on the `@JoinColumn` for adCampaign and platformAccount) and add a
table-level unique constraint for the combination (e.g.,
`@Table`(name="campaign_platform",
uniqueConstraints=@UniqueConstraint(columnNames={"ad_campaign_id","platform_account_id"})))
to prevent duplicate mappings; also add a DB migration to enforce the NOT NULL
and UNIQUE constraints at the database level.
Comment on lines
+35
to
+43
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "currency", length = 10) | ||
| @ColumnDefault("'KRW'") | ||
| private Currency currency; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "timezone", length = 64) | ||
| @ColumnDefault("'ASIA'") | ||
| private Timezone timezone; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "PlatformAccount.java" | head -5Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 171
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2527
🏁 Script executed:
# Check how currency and timezone are used in the codebase
rg "currency|timezone" -t java src/main/java/com/whereyouad/WhereYouAd/domains/platform/ -A 2 -B 2Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1545
🏁 Script executed:
# Check if there are constructors or builders that always initialize these fields
ast-grep --pattern $'class PlatformAccount {
$$$
PlatformAccount($$$) {
$$$
}
$$$
}'Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Check for any migration files or database schema definitions
find . -type f -name "*.sql" -o -name "*migration*" | grep -i platform | head -10Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
find . -type f -name "*.java" -path "*platform*service*" | head -10Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Search for PlatformAccount creation patterns
rg "PlatformAccount" -t java --max-count=20 -B 2 -A 5 | grep -E "\.builder|new PlatformAccount|\.currency|\.timezone" | head -30Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Check the Currency and Timezone enums to understand their values
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Currency.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 214
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/constant/Timezone.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 217
플랫폼 계정의 핵심 속성 currency, timezone에 데이터베이스 제약 추가 필요
@ColumnDefault만으로는 NULL 삽입을 방지할 수 없습니다. 데이터베이스 레벨에서 기본값이 설정되지만, 명시적으로 NULL을 INSERT하면 그대로 저장됩니다. 같은 엔티티의 provider 필드처럼 nullable=false를 추가해 데이터 무결성을 보장하세요.
- `@Column`(name = "currency", length = 10)
+ `@Column`(name = "currency", length = 10, nullable = false)
`@ColumnDefault`("'KRW'")
private Currency currency;
- `@Column`(name = "timezone", length = 64)
+ `@Column`(name = "timezone", length = 64, nullable = false)
`@ColumnDefault`("'ASIA'")
private Timezone timezone;예를 들어, 현재 상태라면 INSERT INTO platform_account(currency) VALUES(NULL)이 성공하지만, nullable=false를 추가하면 DB 제약으로 차단됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`
around lines 35 - 43, Add NOT NULL constraints to the PlatformAccount entity for
the enum fields by updating the currency and timezone column mappings: for the
fields named currency and timezone in class PlatformAccount, add nullable=false
to their `@Column` annotations (you can keep the existing `@ColumnDefault`("'KRW'")
and `@ColumnDefault`("'ASIA'") if desired) so the database enforces non-null
values like the provider field does.
...main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java
Outdated
Show resolved
Hide resolved
Comment on lines
+25
to
+28
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "auth_type") | ||
| @ColumnDefault("'API_KEY'") | ||
| private AuthType authType; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PlatformConnection.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 174
🏁 Script executed:
cat -n ./src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1822
🏁 Script executed:
# Check if there are any validation annotations in the file
rg "@NotNull|@NotBlank|@Valid" ./src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Check User and PlatformAccount entities to understand the relationships
cat -n ./src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java | head -50Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1861
🏁 Script executed:
# Check PlatformAccount entity
find . -name "PlatformAccount.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 171
🏁 Script executed:
cat -n ./src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2527
필수 도메인/FK 컬럼에 nullable = false를 명시해주세요.
Line 26, Line 43, Line 47의 authType, user_id, platform_account_id 컬럼에 현재 nullable = false 제약이 없습니다.
PlatformConnection 엔티티의 특성상 이 세 필드는 반드시 필요한 데이터입니다:
authType:@ColumnDefault("'API_KEY'")지정되어 있어 항상 값이 있어야 하고user_id,platform_account_id: 외래키로서 연결이 성립하려면 필수적입니다
같은 코드베이스의 User, PlatformAccount 엔티티처럼 명시적으로 nullable = false를 선언하면, DB 레벨에서도 데이터 무결성이 보장됩니다. 현재는 애플리케이션 단에서만 검증되고 있어 DB 직접 조작 시 잠재적 위험이 있습니다.
제안 수정안
- `@Column`(name = "auth_type")
+ `@Column`(name = "auth_type", nullable = false)
`@ColumnDefault`("'API_KEY'")
private AuthType authType;
- `@JoinColumn`(name = "user_id")
+ `@JoinColumn`(name = "user_id", nullable = false)
private User user;
- `@JoinColumn`(name = "platform_account_id")
+ `@JoinColumn`(name = "platform_account_id", nullable = false)
private PlatformAccount platformAccount;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.java`
around lines 25 - 28, The PlatformConnection entity is missing DB-level not-null
constraints: add nullable = false to the authType column and to the foreign key
join columns for the user and platformAccount associations; specifically update
the `@Column` on authType (with `@ColumnDefault`("'API_KEY'")) to include nullable =
false, and add nullable = false to the `@JoinColumn` annotations for the user
(user_id) and platformAccount (platform_account_id) fields so that authType,
user_id and platform_account_id are enforced as NOT NULL at the database level.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java (1)
50-52:provider컬럼에length명시 권장다른 enum 필드들(
currency,timezone,status)은 모두length를 명시했는데,provider만 누락되어 있습니다. 현재Providerenum의 값들(KAKAO,NAVER)이 모두 6자 이하이므로,length = 20으로 설정하면 충분하고status필드와의 일관성도 유지할 수 있습니다.♻️ 수정 제안
`@Enumerated`(EnumType.STRING) -@Column(name = "provider", nullable = false) +@Column(name = "provider", length = 20, nullable = false) private Provider provider;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java` around lines 50 - 52, The provider column on PlatformAccount is missing a length attribute; update the `@Column` annotation on the private Provider provider field in class PlatformAccount to include length = 20 (e.g., `@Column`(name = "provider", nullable = false, length = 20)) so it matches other enum fields (currency, timezone, status) and accommodates values like GOOGLE/KAKAO/NAVER; keep the `@Enumerated`(EnumType.STRING) as-is.
🤖 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/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`:
- Around line 19-21: The PlatformAccount class is vulnerable to NPEs because
fields initialized with new ArrayList<>() are ignored by Lombok's `@Builder`;
update each such field (e.g., campaignPlatforms and the other fields noted at
lines 58-60) to use `@Builder.Default` on the field declaration so the default
initializers are preserved when created via PlatformAccount.builder().build();
keep the existing initializer (new ArrayList<>()) and add the `@Builder.Default`
annotation immediately above each field.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`:
- Around line 50-52: The provider column on PlatformAccount is missing a length
attribute; update the `@Column` annotation on the private Provider provider field
in class PlatformAccount to include length = 20 (e.g., `@Column`(name =
"provider", nullable = false, length = 20)) so it matches other enum fields
(currency, timezone, status) and accommodates values like GOOGLE/KAKAO/NAVER;
keep the `@Enumerated`(EnumType.STRING) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a60a65dd-91e1-4520-a6d5-e22e20e55cb6
📒 Files selected for processing (1)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java
Comment on lines
+19
to
+21
| @AllArgsConstructor | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Builder |
There was a problem hiding this comment.
Lombok @Builder와 필드 초기화 값 충돌 - NPE 위험
@Builder를 사용할 때 필드 초기화 값(new ArrayList<>())은 무시됩니다. 예를 들어:
// 이렇게 빌더로 생성하면 campaignPlatforms가 null이 됩니다
PlatformAccount account = PlatformAccount.builder()
.externalAccountId("test")
.provider(Provider.NAVER)
.build();
account.getCampaignPlatforms().add(...); // NPE 발생!@Builder.Default 어노테이션을 추가해 빌더 사용 시에도 기본값이 적용되도록 해주세요.
🐛 수정 제안
//AdCampaign 과의 다대다 매핑을 위한 중간 테이블과 1:N 매핑
`@OneToMany`(mappedBy = "platformAccount", cascade = CascadeType.ALL, orphanRemoval = true)
+@Builder.Default
private List<CampaignPlatform> campaignPlatforms = new ArrayList<>();Also applies to: 58-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java`
around lines 19 - 21, The PlatformAccount class is vulnerable to NPEs because
fields initialized with new ArrayList<>() are ignored by Lombok's `@Builder`;
update each such field (e.g., campaignPlatforms and the other fields noted at
lines 58-60) to use `@Builder.Default` on the field declaration so the default
initializers are preserved when created via PlatformAccount.builder().build();
keep the existing initializer (new ArrayList<>()) and add the `@Builder.Default`
annotation immediately above each field.
kingmingyu
approved these changes
Mar 26, 2026
...main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java
Outdated
Show resolved
Hide resolved
...main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java
Outdated
Show resolved
Hide resolved
...main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/AdCampaign.java
Outdated
Show resolved
Hide resolved
...main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java
Outdated
Show resolved
Hide resolved
jinnieusLab
requested changes
Mar 26, 2026
Collaborator
jinnieusLab
left a comment
jinnieusLab
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! 검토하다보니 저희 짜놓은 ERD가 좀 수정되어야할 듯 하네요...
정리해보자면 제 의견은 일단 이런 식으로 수정하면 괜찮을 것 같아요..!
Organization ➔ Project (1:N) (유지)
Organization ➔ PlatformAccount (1:N) (변경)
PlatformAccount ➔ PlatformConnection (1:N) (유지)
PlatformAccount ➔ AdCampaign (1:N) (변경, CampaignPlatform 삭제)
Project ➔ AdCampaign (1:N) (유지)
PlatformAccount ➔ MetricFact (1:N) (주석 해제)
AdCampaign ➔ MetricFact (1:N) (추가)
...main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformAccount.java
Show resolved
Hide resolved
...ain/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/CampaignPlatform.java
Outdated
Show resolved
Hide resolved
...main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java
Outdated
Show resolved
Hide resolved
...main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java
Show resolved
Hide resolved
Collaborator
Author
|
@kingmingyu @jinnieusLab 피드백 감사합니다!! 종합적으로 반영해서 다음과 같이 수정해봤습니다. 한번 확인해보시고 괜찮으면 Approve 남겨주시면 머지 하겠습니다!
|
jinnieusLab
approved these changes
Mar 27, 2026
Collaborator
jinnieusLab
left a comment
jinnieusLab
left a comment
There was a problem hiding this comment.
P4: 네 확인했습니다 깔끔하게 잘 마무리해주신 것 같아요! 고생하셨습니다!
Collaborator
|
P4: 확인했습니다!! 고생하셨습니다!!🔥🔥 |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
🚀 개요
PlatformAccount, PlatformConnection, CampaignPlatform 엔티티 코드 추가 & 기존 MetricFact, AdCampaign 내부 연관관계 주석처리로 추가
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
PlatformAccount 테이블
PlatformConnection 테이블
CampaignPlatform 테이블
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
릴리스 노트