Merged
Conversation
2 tasks
WalkthroughAdvertisement 및 Project 도메인의 초기 계층(엔티티, 레포지토리, 서비스, 컨트롤러, DTO, 예외 등)과 외부 광고 클라이언트(Google/Kakao/Naver)의 기본 변환기/DTO 스텁을 추가합니다. 일부 엔티티(MetricFact, Project, Advertisement)와 관련 리포지토리가 영구 저장소 매핑을 포함해 새로 도입되었습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 6
🧹 Nitpick comments (12)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.java (1)
1-4: 컨버터 계약(메서드 시그니처)을 먼저 정의하는 편이 안전합니다.현재는 빈 클래스라 호출부가 생겨도 컴파일 타임 계약을 확인할 수 없습니다.
toResponse(...),toEntity(...)같은 최소 시그니처만 먼저 잡아두면 레이어 경계가 명확해집니다.🤖 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/application/mapper/AdvertisementConverter.java` around lines 1 - 4, Add clear converter method signatures to AdvertisementConverter so callers get compile-time contracts: declare public static AdvertisementResponse toResponse(Advertisement advertisement) and public static Advertisement toEntity(CreateAdvertisementRequest request) (and overloads as needed, e.g., toEntity(UpdateAdvertisementRequest request, Long id)) inside the AdvertisementConverter class without implementations yet; reference the AdvertisementConverter class and these method names so the layer boundary and expected parameters/return types are defined for compilation and future implementation.src/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.java (1)
1-4: 상태 없는 컨버터라면 유틸리티화도 고려해볼 만합니다.변환 메서드를 static으로 둘 계획이라면 private 생성자로 인스턴스 생성을 막아 의도를 명확히 할 수 있어요.
🤖 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/project/application/mapper/ProjectConverter.java` around lines 1 - 4, ProjectConverter is currently an empty, stateless converter—if you intend its conversion methods to be static, make the class a utility by declaring the conversion methods static, add a private constructor (e.g., private ProjectConverter()) to prevent instantiation, and consider marking the class final to express non-instantiability; update any usages to call ProjectConverter.staticMethod(...) as needed.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java (1)
1-5: OAuth Google 응답과 구분되는 네이밍을 권장합니다.
GoogleDTO는 전역 OAuth2의GoogleResponse와 의미가 달라 혼동될 여지가 있어요. Ads/통계 응답이라면GoogleAdsResponse/GoogleMetricResponse처럼 역할이 드러나는 이름이 더 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java` around lines 1 - 5, The class name GoogleDTO is ambiguous versus global OAuth2 GoogleResponse; rename the class to a descriptive, role-specific name (e.g., GoogleAdsResponse or GoogleMetricResponse) to avoid confusion—update the class declaration GoogleDTO to the new name and adjust all references/usages across the codebase (imports, constructors, method signatures, DTO mappings) to the new identifier so compilation and semantics remain correct.src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.java (1)
1-4: 빈 서비스 인터페이스는 계약을 명확히 해주세요.DI 목적이라면 CRUD 등 최소 시그니처를 먼저 선언하는 편이 좋고, 그렇지 않다면 실제 사용 시점에 도입해도 충분합니다.
🤖 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/project/domain/service/ProjectService.java` around lines 1 - 4, ProjectService currently empty; if it's meant for DI define a minimal CRUD contract: add method signatures such as createProject(Project project): Project, getProjectById(Long id): Optional<Project>, updateProject(Project project): Project, deleteProject(Long id): void and listProjects(): List<Project> (or use DTO types your domain uses) in the ProjectService interface so callers and implementations have a clear contract; if the interface isn't needed for DI, remove it and introduce it later when actual usage requires it.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.java (1)
3-5: TODO 구현 시 Java 17record문법을 활용해주세요.이 프로젝트는 이미
ExampleRequest처럼record를,EmailRequest·SmsRequest처럼 내부record타입을 적극 사용하고 있습니다.AdvertisementResponse에 필드를 추가할 때도 동일한 패턴을 따르면getter,equals,hashCode,toString을 자동으로 얻을 수 있어 보일러플레이트를 크게 줄일 수 있습니다.예시 (응답 케이스가 여러 개인 경우):
♻️ 내부 Record 활용 예시
-public class AdvertisementResponse { - // TODO: 대시보드 공통 응답값 정의 -} +public class AdvertisementResponse { + public record Dashboard( + Long advertisementId, + String campaignName, + String platform + // 필드 추가 + ) {} + + public record Detail( + Long advertisementId, + String campaignName + // 필드 추가 + ) {} +}🤖 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/application/dto/response/AdvertisementResponse.java` around lines 3 - 5, The AdvertisementResponse class is a TODO — replace the plain class with a Java 17 record (use the record keyword for AdvertisementResponse) and model any variant payloads as nested records inside it (for example, define AdvertisementResponse as a record with fields you need and create internal records for different response cases), so you get automatic getters, equals, hashCode, and toString; update any usages/constructors to use the record canonical constructor and rename fields accordingly.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java (1)
3-5:@Component누락 — 스프링 빈으로 등록되지 않으면 주입 시 런타임 오류 발생현재는 스텁 상태라 문제없지만, 나중에 서비스 레이어에서
@Autowired또는 생성자 주입으로GoogleConverter를 사용할 경우 스프링 컨테이너에 등록되지 않아NoSuchBeanDefinitionException이 발생합니다.컨버터를 스프링 빈으로 관리할 계획이라면
@Component를 추가하고, 상태를 갖지 않는 순수 변환 유틸리티로 쓸 계획이라면 변환 메서드를static으로 설계하는 방향도 고려해보세요.예시 — 스프링 빈 방식:
♻️ `@Component` 추가 예시
+import org.springframework.stereotype.Component; + +@Component public class GoogleConverter { // GoogleDTO -> MetricFactResponse 변환 }(
KakaoConverter,NaverConverter도 동일하게 적용 필요합니다.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java` around lines 3 - 5, GoogleConverter is not registered as a Spring bean which will cause NoSuchBeanDefinitionException when injected; either annotate the class GoogleConverter with `@Component` (and add import org.springframework.stereotype.Component) so Spring can autowire it, or convert its instance methods to static pure-conversion methods if you intend it as a stateless utility; apply the same change consistently to KakaoConverter and NaverConverter and ensure any callers use constructor injection or static calls accordingly.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/AdvertisementControllerDocs.java (1)
3-5: Swagger 문서화 어노테이션 미작성 — 컨트롤러 메서드 추가 시 함께 채워주세요.
ExampleControllerDocs,UserControllerDocs,OrgControllerDocs모두@Tag+@Operation+@ApiResponses를 인터페이스에 정의하고 있습니다.AdvertisementController에 엔드포인트를 추가할 때 아래와 같은 형태를 함께 작성해주세요:♻️ Swagger 어노테이션 작성 예시
+import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import org.springframework.http.ResponseEntity; + +@Tag(name = "Advertisement", description = "광고 운영 관리 API") public interface AdvertisementControllerDocs { + + `@Operation`(summary = "광고 대시보드 조회 API", description = "광고 대시보드 공통 응답값을 반환합니다.") + `@ApiResponses`({ + `@ApiResponse`(responseCode = "200", description = "성공"), + `@ApiResponse`(responseCode = "404", description = "광고 없음") + }) + ResponseEntity<?> getDashboard(...); }도움이 필요하면 메서드 시그니처에 맞는 Swagger 어노테이션 작성을 도와드릴게요. 새 이슈로 개설할까요?
🤖 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/presentation/docs/AdvertisementControllerDocs.java` around lines 3 - 5, The AdvertisementControllerDocs interface is empty and needs Swagger documentation annotations similar to ExampleControllerDocs/UserControllerDocs/OrgControllerDocs; add class-level `@Tag` on AdvertisementControllerDocs and for each endpoint in AdvertisementController add corresponding method signatures in AdvertisementControllerDocs annotated with `@Operation` and `@ApiResponses` (including response codes and descriptions) matching the controller method names and signatures (e.g., methods corresponding to AdvertisementController#createAdvertisement, getAdvertisement, updateAdvertisement, deleteAdvertisement) so the OpenAPI docs include tags, operation summaries and response schemas.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryServiceImpl.java (1)
10-11:@RequiredArgsConstructor(access = AccessLevel.PROTECTED)— 동작은 하지만 비관례적Spring의 CGLIB 프록시는 Objenesis를 통해 인스턴스를 생성하므로 생성자를 직접 호출하지 않습니다. 따라서
protected생성자로도@Transactional이 올바르게 동작합니다.하지만 Lombok의
@RequiredArgsConstructor는 기본적으로public생성자를 생성합니다.PROTECTED로 지정하면 팀 내 다른 개발자가 의도를 오해하거나, 단위 테스트 시 같은 패키지 외부에서 직접 생성이 어려워질 수 있습니다.현재 클래스에 주입받을
final필드가 없으므로@RequiredArgsConstructor자체도 불필요합니다. 필드가 추가되면 아래처럼 관례적인 방식으로 수정하는 것을 권장합니다:♻️ 개선 예시
-@RequiredArgsConstructor(access = AccessLevel.PROTECTED) +@RequiredArgsConstructor public class AdvertisementQueryServiceImpl implements AdvertisementQueryService { }🤖 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/domain/service/AdvertisementQueryServiceImpl.java` around lines 10 - 11, The class AdvertisementQueryServiceImpl is annotated with `@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which is unnecessary (there are no final injected fields) and non-idiomatic; remove the `@RequiredArgsConstructor` annotation from AdvertisementQueryServiceImpl to avoid confusion and tests issues, and if you later add final dependency fields, add `@RequiredArgsConstructor` without an access override (i.e., default public constructor) so constructor visibility follows project conventions.src/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.java (1)
8-13:@AllArgsConstructor의 접근 제어 수준을PRIVATE으로 제한하는 것을 권장합니다현재
@AllArgsConstructor는 기본값인public으로 생성됩니다. 즉, 외부에서new Project(id, name, ...)식으로 빌더를 완전히 우회해서 객체를 만들 수 있어요.Lombok 공식 문서에 따르면 "
@Builder를 클래스에 적용하면@AllArgsConstructor(access = AccessLevel.PACKAGE)를 적용하는 것과 같다"고 명시되어 있습니다. JPA 엔티티에서@NoArgsConstructor가 이미 있으므로 명시적@AllArgsConstructor가 필요한데, 빌더 패턴을 통해서만 객체를 생성하도록 강제하려면@AllArgsConstructor(access = AccessLevel.PRIVATE)으로 선언하는 것이 좋습니다.🔒 권장 수정
-@AllArgsConstructor +@AllArgsConstructor(access = AccessLevel.PRIVATE) `@Builder` public class Project extends BaseEntity {🤖 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/project/persistence/entity/Project.java` around lines 8 - 13, The Project entity currently uses `@AllArgsConstructor` with default public access which allows bypassing the `@Builder`; change the `@AllArgsConstructor` on the Project class to use access = AccessLevel.PRIVATE so construction is restricted to the builder (update the annotation on the Project class where `@AllArgsConstructor` is declared and ensure imports/reference to AccessLevel remain correct).src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java (1)
1-7: 기본 구조 잘 잡았어요! 👍 — 향후 조회 쿼리 및 인덱스 전략을 미리 고려해두세요
JpaRepository<MetricFact, Long>확장 자체는 완전히 맞습니다.PR 설명에서 "MetricFact의 다양한 조회 API" 를 언급하고 있는데,
metric_fact는 시계열 집계 팩트 테이블이므로 실제 쿼리가 추가될 때 아래 사항들을 미리 고려하면 좋습니다.
- 자주 사용될 조회 패턴 예시 —
grain,timeBucket범위,provider,project_id조합의 쿼리가 예상됩니다.- 복합 인덱스 — 예를 들어
(project_id, provider, grain, time_bucket)조합의 복합 인덱스 없이 팩트 테이블을 풀스캔하면 데이터 증가 시 심각한 성능 문제가 발생합니다.// 향후 필요할 메서드 예시 List<MetricFact> findByProjectIdAndProviderAndGrainAndTimeBucketBetween( Long projectId, Provider provider, Grain grain, LocalDateTime from, LocalDateTime to );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/advertisement/persistence/repository/MetricFactRepository.java` around lines 1 - 7, The repository currently just extends JpaRepository for MetricFact; add planned query methods and DB index guidance: implement query method(s) such as findByProjectIdAndProviderAndGrainAndTimeBucketBetween in MetricFactRepository to support common access patterns (projectId, provider, grain, timeBucket range) and add a DB migration that creates a composite index on (project_id, provider, grain, time_bucket) to prevent full table scans as the fact table grows; ensure the timeBucket column type aligns with the query (e.g., timestamp) and consider additional indexes for other frequent filter combos.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java (2)
13-18:@AllArgsConstructor접근 수준을PRIVATE으로 제한하세요 (Project 엔티티와 동일한 이슈)
@Builder를 클래스에 적용하면@AllArgsConstructor(access = AccessLevel.PACKAGE)가 자동으로 추가된 것과 같고,@NoArgsConstructor와 함께 사용 시 명시적@AllArgsConstructor가 필요합니다. 하지만 현재public으로 열려 있어 빌더를 우회한 직접 생성이 가능합니다.🔒 권장 수정
-@AllArgsConstructor +@AllArgsConstructor(access = AccessLevel.PRIVATE) `@Builder` public class MetricFact extends BaseEntity {🤖 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 13 - 18, The class MetricFact is annotated with `@Builder` and a public `@AllArgsConstructor` which allows bypassing the builder; change the `@AllArgsConstructor` on MetricFact to use access = AccessLevel.PRIVATE (matching Project entity) so only the builder can create instances while keeping `@NoArgsConstructor`(access = AccessLevel.PROTECTED) intact; update the annotation on the MetricFact class (the `@AllArgsConstructor` declaration) to use AccessLevel.PRIVATE.
4-4:Provider이름 변경으로 향후 네이밍 충돌 방지를 권장합니다광고 도메인의
Provider(com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.Provider)와 사용자 도메인의Provider(com.whereyouad.WhereYouAd.domains.user.domain.constant.Provider)가 동일한 이름을 사용하고 있습니다.현재로는 각 도메인이 분리되어 있어 문제가 없지만, 광고 플랫폼 연동 서비스를 확장할 때 두
Provider를 동시에 임포트하게 되면 컴파일 에러가 발생합니다. 같은 이름이지만 다른 개념(광고 플랫폼 vs 인증 제공자)을 나타내므로, 광고 도메인의 enum을 **AdProvider또는AdPlatform**으로 명확히 개명하는 것을 권장합니다.이렇게 하면 코드의 의도가 더 명확해지고 향후 유지보수도 훨씬 수월해집니다.
🤖 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` at line 4, Rename the advertisement domain enum Provider to a more specific name (e.g., AdProvider or AdPlatform) and update all references in the advertisement package: change the enum declaration identifier in com.whereyouad.WhereYouAd.domains.advertisement.domain.constant (rename file/class from Provider to AdProvider or AdPlatform), update imports such as the one in MetricFact (remove import of Provider and import the new AdProvider/AdPlatform), and replace all usages of the enum identifier (e.g., method signatures, fields, switch/case, comparisons) in advertisement classes to the new name to avoid collisions with the user-domain Provider.
🤖 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/advertisement/application/dto/request/AdvertisementRequest.java`:
- Around line 3-4: Add a TODO comment to AdvertisementRequest noting "대시보드 요청값
정의" (matching the style in AdvertisementResponse) and refactor the class to
follow the established pattern by declaring one or more internal Java 17 record
types for request variants (e.g., AdvertisementRequest.Create,
AdvertisementRequest.Update or AdvertisementRequest.Payload) so future fields
can be added as records consistent with EmailRequest and SmsRequest; ensure the
top-level AdvertisementRequest remains a holder for those internal records and
include the TODO above or inside the class to remind implementers to define
request fields later.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java`:
- Around line 36-43: The numeric fact fields in MetricFact (impressions, clicks,
conversions) should be non-nullable and default to 0: update the `@Column`
annotations on MetricFact for impressions, clicks and conversions to include
nullable = false and a columnDefinition/default value of 0 (or use
`@Column`(nullable = false) plus initialize the fields to 0L), and add a DB
migration to alter the underlying table to set NOT NULL and default 0 so
existing rows are backfilled; ensure any constructors or builders for MetricFact
also initialize these fields to 0L to avoid NPEs.
- Around line 33-34: The MetricFact class contains a commented-out field
dimensionType; decide whether this field is required and then either remove the
commented lines entirely or restore the field by uncommenting and adding proper
`@Column`(name = "dimension_type") private String dimensionType; (and update any
constructors/getters/setters or mapping code such as in MetricFact) so the
codebase does not contain orphaned commented code; ensure the choice is
reflected in the PR description.
- Around line 30-31: Change the MetricFact.timeBucket field from LocalDateTime
to Instant to avoid JVM-local timezone issues: update the declaration in
MetricFact (timeBucket) and any getters/setters or mappings that reference it,
and likewise consider changing BaseEntity.createdAt and BaseEntity.updatedAt
from LocalDateTime to Instant to keep timestamps consistent; also ensure
JPA/Hibernate stores UTC by adding the hibernate.jdbc.time_zone=UTC property to
application.yml so DB persistence uses UTC.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/exception/code/ProjectErrorCode.java`:
- Around line 10-16: ProjectErrorCode currently defines no enum constants, so
ProjectException cannot map to any error codes; add representative enum
constants (e.g., NOT_FOUND, FORBIDDEN, BAD_REQUEST, INTERNAL_ERROR) to
ProjectErrorCode that supply the required HttpStatus, code string and human
message, ensure the enum includes a constructor to populate the existing fields
(httpStatus, code, message) and exposes the accessor methods required by
BaseErrorCode so ProjectException can use them for exception mapping.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.java`:
- Around line 31-33: Project's organization association allows nulls at the JPA
level; add a non-null constraint on the `@JoinColumn` for the organization field
in the Project entity so JPA prevents persisting a Project without an
Organization. Locate the Project class and update the organization mapping (the
field named "organization" annotated with `@ManyToOne` and `@JoinColumn`) to include
nullable = false on `@JoinColumn`, and ensure any existing code that creates/saves
Project instances supplies a valid Organization (and adjust
tests/migrations/schema as needed).
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.java`:
- Around line 3-5: The AdvertisementResponse class is a TODO — replace the plain
class with a Java 17 record (use the record keyword for AdvertisementResponse)
and model any variant payloads as nested records inside it (for example, define
AdvertisementResponse as a record with fields you need and create internal
records for different response cases), so you get automatic getters, equals,
hashCode, and toString; update any usages/constructors to use the record
canonical constructor and rename fields accordingly.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.java`:
- Around line 1-4: Add clear converter method signatures to
AdvertisementConverter so callers get compile-time contracts: declare public
static AdvertisementResponse toResponse(Advertisement advertisement) and public
static Advertisement toEntity(CreateAdvertisementRequest request) (and overloads
as needed, e.g., toEntity(UpdateAdvertisementRequest request, Long id)) inside
the AdvertisementConverter class without implementations yet; reference the
AdvertisementConverter class and these method names so the layer boundary and
expected parameters/return types are defined for compilation and future
implementation.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryServiceImpl.java`:
- Around line 10-11: The class AdvertisementQueryServiceImpl is annotated with
`@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which is unnecessary
(there are no final injected fields) and non-idiomatic; remove the
`@RequiredArgsConstructor` annotation from AdvertisementQueryServiceImpl to avoid
confusion and tests issues, and if you later add final dependency fields, add
`@RequiredArgsConstructor` without an access override (i.e., default public
constructor) so constructor visibility follows project conventions.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java`:
- Around line 13-18: The class MetricFact is annotated with `@Builder` and a
public `@AllArgsConstructor` which allows bypassing the builder; change the
`@AllArgsConstructor` on MetricFact to use access = AccessLevel.PRIVATE (matching
Project entity) so only the builder can create instances while keeping
`@NoArgsConstructor`(access = AccessLevel.PROTECTED) intact; update the annotation
on the MetricFact class (the `@AllArgsConstructor` declaration) to use
AccessLevel.PRIVATE.
- Line 4: Rename the advertisement domain enum Provider to a more specific name
(e.g., AdProvider or AdPlatform) and update all references in the advertisement
package: change the enum declaration identifier in
com.whereyouad.WhereYouAd.domains.advertisement.domain.constant (rename
file/class from Provider to AdProvider or AdPlatform), update imports such as
the one in MetricFact (remove import of Provider and import the new
AdProvider/AdPlatform), and replace all usages of the enum identifier (e.g.,
method signatures, fields, switch/case, comparisons) in advertisement classes to
the new name to avoid collisions with the user-domain Provider.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java`:
- Around line 1-7: The repository currently just extends JpaRepository for
MetricFact; add planned query methods and DB index guidance: implement query
method(s) such as findByProjectIdAndProviderAndGrainAndTimeBucketBetween in
MetricFactRepository to support common access patterns (projectId, provider,
grain, timeBucket range) and add a DB migration that creates a composite index
on (project_id, provider, grain, time_bucket) to prevent full table scans as the
fact table grows; ensure the timeBucket column type aligns with the query (e.g.,
timestamp) and consider additional indexes for other frequent filter combos.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/AdvertisementControllerDocs.java`:
- Around line 3-5: The AdvertisementControllerDocs interface is empty and needs
Swagger documentation annotations similar to
ExampleControllerDocs/UserControllerDocs/OrgControllerDocs; add class-level `@Tag`
on AdvertisementControllerDocs and for each endpoint in AdvertisementController
add corresponding method signatures in AdvertisementControllerDocs annotated
with `@Operation` and `@ApiResponses` (including response codes and descriptions)
matching the controller method names and signatures (e.g., methods corresponding
to AdvertisementController#createAdvertisement, getAdvertisement,
updateAdvertisement, deleteAdvertisement) so the OpenAPI docs include tags,
operation summaries and response schemas.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.java`:
- Around line 1-4: ProjectConverter is currently an empty, stateless
converter—if you intend its conversion methods to be static, make the class a
utility by declaring the conversion methods static, add a private constructor
(e.g., private ProjectConverter()) to prevent instantiation, and consider
marking the class final to express non-instantiability; update any usages to
call ProjectConverter.staticMethod(...) as needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.java`:
- Around line 1-4: ProjectService currently empty; if it's meant for DI define a
minimal CRUD contract: add method signatures such as createProject(Project
project): Project, getProjectById(Long id): Optional<Project>,
updateProject(Project project): Project, deleteProject(Long id): void and
listProjects(): List<Project> (or use DTO types your domain uses) in the
ProjectService interface so callers and implementations have a clear contract;
if the interface isn't needed for DI, remove it and introduce it later when
actual usage requires it.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.java`:
- Around line 8-13: The Project entity currently uses `@AllArgsConstructor` with
default public access which allows bypassing the `@Builder`; change the
`@AllArgsConstructor` on the Project class to use access = AccessLevel.PRIVATE so
construction is restricted to the builder (update the annotation on the Project
class where `@AllArgsConstructor` is declared and ensure imports/reference to
AccessLevel remain correct).
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java`:
- Around line 3-5: GoogleConverter is not registered as a Spring bean which will
cause NoSuchBeanDefinitionException when injected; either annotate the class
GoogleConverter with `@Component` (and add import
org.springframework.stereotype.Component) so Spring can autowire it, or convert
its instance methods to static pure-conversion methods if you intend it as a
stateless utility; apply the same change consistently to KakaoConverter and
NaverConverter and ensure any callers use constructor injection or static calls
accordingly.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java`:
- Around line 1-5: The class name GoogleDTO is ambiguous versus global OAuth2
GoogleResponse; rename the class to a descriptive, role-specific name (e.g.,
GoogleAdsResponse or GoogleMetricResponse) to avoid confusion—update the class
declaration GoogleDTO to the new name and adjust all references/usages across
the codebase (imports, constructors, method signatures, DTO mappings) to the new
identifier so compilation and semantics remain correct.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Grain.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Provider.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/AdvertisementException.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/code/AdvertisementErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/AdvertisementController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/AdvertisementControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/request/ProjectRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/response/ProjectResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/exception/ProjectException.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/exception/code/ProjectErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/repository/ProjectRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/ProjectController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/docs/ProjectControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/converter/KakaoConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/dto/KakaoDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/converter/NaverConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.java
Comment on lines
+3
to
+4
| public class AdvertisementRequest { | ||
| } |
There was a problem hiding this comment.
TODO 주석 누락 및 Java 17 record 패턴 미적용
AdvertisementResponse에는 // TODO: 대시보드 공통 응답값 정의 주석이 있는데, AdvertisementRequest에는 없습니다. 나중에 요청 필드를 정의해야 한다는 사실을 잊기 쉬우니 TODO를 추가해주세요.
또한 기존 코드베이스의 EmailRequest, SmsRequest처럼 요청 케이스별로 내부 record를 사용하는 패턴이 정착되어 있습니다. 필드를 추가할 때 동일한 패턴을 따라주세요.
♻️ 개선 예시
public class AdvertisementRequest {
+ // TODO: 광고 요청 DTO 필드 정의
+
+ // 예시 — 요청 케이스별 내부 record:
+ // public record Search(String platform, String campaignName) {}
+ // public record Create(String campaignName, ...) {}
}📝 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
| public class AdvertisementRequest { | |
| } | |
| public class AdvertisementRequest { | |
| // TODO: 광고 요청 DTO 필드 정의 | |
| // 예시 — 요청 케이스별 내부 record: | |
| // public record Search(String platform, String campaignName) {} | |
| // public record Create(String campaignName, ...) {} | |
| } |
🤖 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/application/dto/request/AdvertisementRequest.java`
around lines 3 - 4, Add a TODO comment to AdvertisementRequest noting "대시보드 요청값
정의" (matching the style in AdvertisementResponse) and refactor the class to
follow the established pattern by declaring one or more internal Java 17 record
types for request variants (e.g., AdvertisementRequest.Create,
AdvertisementRequest.Update or AdvertisementRequest.Payload) so future fields
can be added as records consistent with EmailRequest and SmsRequest; ensure the
top-level AdvertisementRequest remains a holder for those internal records and
include the TODO above or inside the class to remind implementers to define
request fields later.
Comment on lines
+30
to
+31
| @Column(name = "time_bucket", nullable = false) | ||
| private LocalDateTime timeBucket; // 집계 단위 시작 시간 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 먼저 MetricFact.java 파일 확인
find . -name "MetricFact.java" -type f | head -5Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 171
🏁 Script executed:
# Java 파일 구조 파악
git ls-files "src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 169
🏁 Script executed:
# 전체 파일 목록으로 경로 확인
fd -t f "MetricFact.java"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 169
🏁 Script executed:
# MetricFact.java 파일 내용 확인
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2201
🏁 Script executed:
# 다른 엔티티에서 LocalDateTime 또는 Instant 사용 패턴 확인
rg "private (LocalDateTime|Instant)" src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/ -A 2Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 456
🏁 Script executed:
# application.properties 또는 application.yml 파일 확인 (현재 설정 상태)
find . -name "application*.yml" -o -name "application*.properties" | head -10Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 109
🏁 Script executed:
# application.yml 파일 내용 확인
cat -n ./src/main/resources/application.ymlRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 3899
🏁 Script executed:
# BaseEntity 확인 (createdAt, updatedAt 등이 어떻게 정의되어 있는지)
find . -name "BaseEntity.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 144
🏁 Script executed:
# 코드베이스에서 Instant 사용 여부 확인
rg "import java.time.Instant" src/main/java/ -l | head -10Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Spring Boot 버전 확인
find . -name "pom.xml" -o -name "build.gradle" | head -3Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 87
🏁 Script executed:
# BaseEntity 파일 내용 확인
cat -n ./src/main/java/com/whereyouad/WhereYouAd/global/common/BaseEntity.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1002
🏁 Script executed:
# build.gradle에서 Spring Boot 버전 확인
cat -n ./build.gradle | head -30Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 856
🏁 Script executed:
# 코드베이스에서 타임존 관련 설정이 있는지 확인
rg "timezone|TimeZone|UTC" src/main/java/ -iRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 375
🏁 Script executed:
# LocalDateTime이 다른 엔티티에서도 사용되고 있는지 확인
rg "private LocalDateTime" src/main/java/com/whereyouad/WhereYouAd/domains/ --type javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 496
🏁 Script executed:
# MySQL TIMESTAMP 타입의 타임존 처리 확인 및 Spring Boot 3.5.9 호환성 검증
cat build.gradle | grep -A 20 "dependencies"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 835
🏁 Script executed:
# 기존 데이터 구조 확인: DB 스키마에서 time_bucket이 어떤 타입인지
rg "CREATE TABLE|time_bucket" src/ --type javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 221
팩트 테이블의 시간 컬럼으로 LocalDateTime 대신 Instant 사용을 권장합니다
LocalDateTime은 타임존 정보가 없어서, 서버 JVM의 로컬 타임존에만 의존하게 됩니다. 현재 application.yml에 타임존 설정이 없으므로, Google, Kakao, Naver 등 서로 다른 플랫폼에서 수집한 광고 지표를 집계할 때 문제가 발생할 수 있습니다:
- 서버 JVM 타임존이 변경되면 동일한 UTC 시각의
time_bucket이 다른 값으로 저장됨 - DST(일광절약시간) 변경 시 데이터 손상 위험
- 다중 리전 환경에서 같은 광고 이벤트가 다른 시간으로 기록될 수 있음
⏱️ 권장 수정 — Instant 사용
-import java.time.LocalDateTime;
+import java.time.Instant;
- `@Column`(name = "time_bucket", nullable = false)
- private LocalDateTime timeBucket; // 집계 단위 시작 시간
+ `@Column`(name = "time_bucket", nullable = false)
+ private Instant timeBucket; // 집계 단위 시작 시간 (UTC 기준)그리고 application.yml에 아래 설정을 추가하면 JPA/Hibernate가 항상 UTC 기준으로 DB에 저장합니다:
spring:
jpa:
properties:
hibernate:
jdbc:
time_zone: UTC참고: BaseEntity의 createdAt, updatedAt도 같은 문제를 안고 있으니 함께 Instant로 변경하는 것을 검토해보세요.
📝 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
| @Column(name = "time_bucket", nullable = false) | |
| private LocalDateTime timeBucket; // 집계 단위 시작 시간 | |
| `@Column`(name = "time_bucket", nullable = false) | |
| private Instant timeBucket; // 집계 단위 시작 시간 (UTC 기준) |
🤖 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 30 - 31, Change the MetricFact.timeBucket field from LocalDateTime
to Instant to avoid JVM-local timezone issues: update the declaration in
MetricFact (timeBucket) and any getters/setters or mappings that reference it,
and likewise consider changing BaseEntity.createdAt and BaseEntity.updatedAt
from LocalDateTime to Instant to keep timestamps consistent; also ensure
JPA/Hibernate stores UTC by adding the hibernate.jdbc.time_zone=UTC property to
application.yml so DB persistence uses UTC.
Comment on lines
+33
to
+34
| // @Column(name = "dimension_type") | ||
| // private String dimensionType; |
There was a problem hiding this comment.
주석 처리된 dimensionType 필드는 머지 전 정리가 필요합니다
PR 설명에서 dimensionType 필드의 필요성을 논의 중이라고 언급했는데, 결론이 나오면 완전히 삭제하거나 주석 없이 추가하는 방향으로 정리하는 것이 좋습니다. 주석 처리된 코드가 그대로 머지되면 나중에 누가 추가한 건지, 왜 있는지 파악하기 어려워집니다.
🗑️ 제거 예시
-// `@Column`(name = "dimension_type")
-// private String dimensionType;
-📝 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
| // @Column(name = "dimension_type") | |
| // private String dimensionType; |
🤖 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 33 - 34, The MetricFact class contains a commented-out field
dimensionType; decide whether this field is required and then either remove the
commented lines entirely or restore the field by uncommenting and adding proper
`@Column`(name = "dimension_type") private String dimensionType; (and update any
constructors/getters/setters or mapping code such as in MetricFact) so the
codebase does not contain orphaned commented code; ensure the choice is
reflected in the PR description.
Comment on lines
+36
to
+43
| @Column(name = "impressions") | ||
| private Long impressions; // 노출수 | ||
|
|
||
| @Column(name = "clicks") | ||
| private Long clicks; // 클릭수 | ||
|
|
||
| @Column(name = "conversions") | ||
| private Long conversions; // 전환수 |
There was a problem hiding this comment.
팩트 테이블의 수치 컬럼은 nullable = false로 강제하고 기본값 0을 권장합니다
impressions, clicks, conversions 모두 nullable로 정의되어 있는데, 광고 지표 팩트 테이블에서 null과 0은 의미가 완전히 다릅니다.
null: "데이터가 수집되지 않음 / 알 수 없음"0: "측정됐지만 수치가 없음"
현재 설계에서 집계 쿼리(SUM, AVG 등)를 실행하면 null 행이 있을 경우 예상과 다른 결과가 나올 수 있습니다. 비즈니스 규칙상 "항상 측정값이 있어야 한다"면 아래처럼 처리하세요.
🛡️ 제약 추가 예시
- `@Column`(name = "impressions")
- private Long impressions;
+ `@Column`(name = "impressions", nullable = false)
+ private Long impressions; // 기본값 0 처리는 서비스 레이어에서 담당
- `@Column`(name = "clicks")
- private Long clicks;
+ `@Column`(name = "clicks", nullable = false)
+ private Long clicks;
- `@Column`(name = "conversions")
- private Long conversions;
+ `@Column`(name = "conversions", nullable = false)
+ private Long conversions;🤖 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 36 - 43, The numeric fact fields in MetricFact (impressions,
clicks, conversions) should be non-nullable and default to 0: update the `@Column`
annotations on MetricFact for impressions, clicks and conversions to include
nullable = false and a columnDefinition/default value of 0 (or use
`@Column`(nullable = false) plus initialize the fields to 0L), and add a DB
migration to alter the underlying table to set NOT NULL and default 0 so
existing rows are backfilled; ensure any constructors or builders for MetricFact
also initialize these fields to 0L to avoid NPEs.
Comment on lines
+10
to
+16
| public enum ProjectErrorCode implements BaseErrorCode { | ||
|
|
||
| ; | ||
|
|
||
| private final HttpStatus httpStatus; | ||
| private final String code; | ||
| private final String message; |
There was a problem hiding this comment.
ProjectErrorCode에 enum 상수 정의가 필요합니다.
Line 10-12처럼 상수가 비어 있으면 ProjectException에서 사용할 오류 코드가 없어 예외 매핑이 불가능합니다. 최소한 대표 오류(예: NOT_FOUND, FORBIDDEN 등)를 정의해 주세요.
🔧 예시 추가안
public enum ProjectErrorCode implements BaseErrorCode {
-
- ;
+ PROJECT_NOT_FOUND(HttpStatus.NOT_FOUND, "PROJECT_404", "프로젝트를 찾을 수 없습니다."),
+ PROJECT_FORBIDDEN(HttpStatus.FORBIDDEN, "PROJECT_403", "프로젝트 접근 권한이 없습니다.");📝 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
| public enum ProjectErrorCode implements BaseErrorCode { | |
| ; | |
| private final HttpStatus httpStatus; | |
| private final String code; | |
| private final String message; | |
| public enum ProjectErrorCode implements BaseErrorCode { | |
| PROJECT_NOT_FOUND(HttpStatus.NOT_FOUND, "PROJECT_404", "프로젝트를 찾을 수 없습니다."), | |
| PROJECT_FORBIDDEN(HttpStatus.FORBIDDEN, "PROJECT_403", "프로젝트 접근 권한이 없습니다."); | |
| private final HttpStatus httpStatus; | |
| private final String code; | |
| private final String message; |
🤖 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/project/exception/code/ProjectErrorCode.java`
around lines 10 - 16, ProjectErrorCode currently defines no enum constants, so
ProjectException cannot map to any error codes; add representative enum
constants (e.g., NOT_FOUND, FORBIDDEN, BAD_REQUEST, INTERNAL_ERROR) to
ProjectErrorCode that supply the required HttpStatus, code string and human
message, ensure the enum includes a constructor to populate the existing fields
(httpStatus, code, message) and exposes the accessor methods required by
BaseErrorCode so ProjectException can use them for exception mapping.
Comment on lines
+31
to
+33
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "org_id") | ||
| private Organization organization; |
There was a problem hiding this comment.
@JoinColumn에 nullable = false 제약이 없습니다
모든 Project는 반드시 Organization에 속해야 한다면, JPA 레벨에서도 이를 강제해야 합니다. 현재는 null organization을 가진 Project가 저장될 수 있습니다.
🛡️ 제약 추가 예시
`@ManyToOne`(fetch = FetchType.LAZY)
-@JoinColumn(name = "org_id")
+@JoinColumn(name = "org_id", nullable = false)
private Organization organization;만약 비즈니스 규칙상 organization 없는 Project가 유효하다면 현재 코드가 맞습니다. 확인 후 필요 없다면 그대로 두세요.
📝 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
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "org_id") | |
| private Organization organization; | |
| `@ManyToOne`(fetch = FetchType.LAZY) | |
| `@JoinColumn`(name = "org_id", nullable = false) | |
| private Organization organization; |
🤖 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/project/persistence/entity/Project.java`
around lines 31 - 33, Project's organization association allows nulls at the JPA
level; add a non-null constraint on the `@JoinColumn` for the organization field
in the Project entity so JPA prevents persisting a Project without an
Organization. Locate the Project class and update the organization mapping (the
field named "organization" annotated with `@ManyToOne` and `@JoinColumn`) to include
nullable = false on `@JoinColumn`, and ensure any existing code that creates/saves
Project instances supplies a valid Organization (and adjust
tests/migrations/schema as needed).
ojy0903
reviewed
Feb 24, 2026
| @@ -0,0 +1,5 @@ | |||
| package com.whereyouad.WhereYouAd.infrastructure.client.google.converter; | |||
|
|
|||
Collaborator
There was a problem hiding this comment.
P4: infrastructure 에 추후 API 연동 위한 확장 구조 해놓은 거 좋은거 같습니다!!
ojy0903
approved these changes
Feb 24, 2026
Collaborator
ojy0903
left a comment
ojy0903
left a comment
There was a problem hiding this comment.
P4: 전체적으로 구조 잘 만들어주신거 같아요!! 고생하셨습니다
제 생각에는 dimensionType 은 굳이 필요하진 않은 것 같긴 합니다...
그런데 제가 조금 헷갈리는게 저희 그 60일치 데이터 만드는 INSERT 문 해오는게 여기서 MetricFact 에 대해서 provider 를 각자 플랫폼에 맞춰서 만드는거가 맞을까요...?
Collaborator
Author
감사합니다!! 저는 MetricFact 테이블에 맞춰서 저장하는 거라고 생각했습니다..!(저는 전체적인 흐름이 infra 도메인에서 api 통신하고 받은 응답 원본을 converter로 metricfact에 맞게 저장이라고 생각했습니다) 근데 제가 추가로 고민 중인건 여기에 개별 광고?랑도 연관관계를 넣어줘야 조회 로직 짤 때 이 플랫폼이랑 연결된 광고가 몇개인지 조회할 수 있을 것 같은데 어떻게 생각하시나요?? 지금은 정말 광고 통계 숫자밖에 없어서 조회 로직을 어떻게 짤지 고민입니다..! |
Collaborator
음 생각해보니 확실히 MetricFact 테이블 만으로는 메인보드에서 "각 플랫폼마다 몇개의 광고를 진행중인지" 와 같은 정보를 나타내기 힘들거 같네요.... 저희 기존 ERD 에서 AD_CONTENT 테이블과 연결하는 것도 고려해봤는데 저희 1차 MVP 에서는 캠페인-그룹-광고 개체 형태의 계층 구조를 반영하기엔 한계가 있을 거 같아서, 제 생각에는 아래 사진처럼 ADVERTISEMENT 라는 객체를 하나 추가해보면 어떨까 하는데 괜찮을까요? 각 플랫폼당 광고 갯수를 구할땐 ADVERTISEMENT 에서 Provider 기준으로 갯수를 집계해서 구하고, 나머지 데이터들은 MetricFact 테이블에서 조회해오는 방식입니다. 추가로 만약 이렇게 한다면 저희 INSERT 문 만들때 해당 ADVERTISEMENT 에 대한 INSERT 문도 함께 만들어야 할 것 같긴 합니다... |
Collaborator
Author
좋은 것 같습니다! 이거 추가하면 활성 상태인 광고만 조회할 수 있을 것 같긴 한데 그럼 모든 광고에 대해서 또 join을 해야하는 것 같아서 성능 상 괜찮을까요..? 일단 뭔가 테이블을 metricfact 테이블에 광고 정보에 대한 테이블을 만들어야할 것 같긴 합니다! 제가 목요일 저녁 이후부터 작업이 가능해서 그때 추가해보겠습니다..! 아니면 시간 괜찮으시면 추가해주셔도 됩니다..! |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
src/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.java (1)
3-4:AdvertisementConverter와 동일한 문제입니다 — 유틸리티 클래스 또는 Spring 빈으로 명확히 정의해주세요.
AdvertisementConverter관련 코멘트와 동일하게,ProjectConverter역시 정적 유틸리티 클래스라면private생성자를, Spring 빈이라면@Component를 추가해주세요.🤖 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/project/application/mapper/ProjectConverter.java` around lines 3 - 4, ProjectConverter is currently an empty class like AdvertisementConverter and should be clearly defined as either a static utility or a Spring bean; either add a private no-arg constructor to make it a proper utility class (prevent instantiation) or annotate the class with `@Component` (and import org.springframework.stereotype.Component) to register it as a Spring bean—apply the same approach you used for AdvertisementConverter so the intent is explicit.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.java (1)
3-4: 이전 리뷰 코멘트가 아직 반영되지 않았습니다 — TODO 주석 및 Java 17 record 패턴 적용이 필요합니다.
AdvertisementResponse에는// TODO: 대시보드 공통 응답값 정의가 있는데,AdvertisementRequest에는 없습니다. 또한 Java 17record패턴(이미 코딩 가이드라인에서 권장)을 나중에 필드 추가 시 적용해주세요.🤖 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/application/dto/request/AdvertisementRequest.java` around lines 3 - 4, AdvertisementRequest is an empty class and should mirror the documented TODO pattern from AdvertisementResponse and adopt the Java 17 record style when fields are added; add a TODO comment in AdvertisementRequest similar to AdvertisementResponse (e.g., "// TODO: 대시보드 공통 요청값 정의") and when you introduce request fields refactor the class into a record (record AdvertisementRequest(...)) to follow the coding guideline, ensuring constructors/getters are removed and tests/usage sites are updated to use the record components.src/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.java (1)
31-33:⚠️ Potential issue | 🟡 MinorOrganization이 필수 관계라면
org_idnull 저장을 막아주세요.현재 매핑은
organization없이도Project가 저장될 수 있습니다. 모든 프로젝트가 조직에 속해야 하는 규칙이라면 JPA/DB 레벨에서nullable = false로 막는 게 안전합니다.🔒 제안 수정안
`@ManyToOne`(fetch = FetchType.LAZY) -@JoinColumn(name = "org_id") +@JoinColumn(name = "org_id", nullable = false) private Organization organization;As per coding guidelines
src/main/java/**/*.java: "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/project/persistence/entity/Project.java` around lines 31 - 33, The Project entity's organization association is currently optional, allowing org_id to be null; make this relationship required by updating the organization field mapping in the Project class: set `@JoinColumn`(name = "org_id", nullable = false) and (optionally) add a javax.validation constraint like `@NotNull` on the organization field to enforce non-null at the bean validation level; keep FetchType.LAZY as-is to avoid eager loading and ensure the DB column is created non-nullable on schema generation/migration.src/main/java/com/whereyouad/WhereYouAd/domains/project/exception/code/ProjectErrorCode.java (1)
10-16:⚠️ Potential issue | 🟠 MajorProject 도메인 에러코드가 비어 있어 예외 매핑이 불가능합니다.
ProjectErrorCode에 상수가 하나도 없어ProjectException을 도메인 코드로 일관되게 사용할 수 없습니다. 최소한 조회 실패/권한/검증 오류 코드는 바로 추가해두는 게 안전합니다.🔧 제안 수정안
public enum ProjectErrorCode implements BaseErrorCode { - - ; + PROJECT_NOT_FOUND(HttpStatus.NOT_FOUND, "PROJECT_404", "프로젝트를 찾을 수 없습니다."), + PROJECT_FORBIDDEN(HttpStatus.FORBIDDEN, "PROJECT_403", "프로젝트 접근 권한이 없습니다."), + PROJECT_BAD_REQUEST(HttpStatus.BAD_REQUEST, "PROJECT_400", "잘못된 프로젝트 요청입니다."); private final HttpStatus httpStatus; private final String code; private final String message; }As per coding guidelines
src/main/java/**/*.java: "SOLID 원칙, 의존성 주입(DI), 예외 처리(GlobalExceptionHandler)가 적절한지 보라."🤖 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/project/exception/code/ProjectErrorCode.java` around lines 10 - 16, ProjectErrorCode enum is empty so ProjectException can't map domain errors; add meaningful constants (e.g., PROJECT_NOT_FOUND, PROJECT_FORBIDDEN, PROJECT_VALIDATION_FAILED) that provide HttpStatus, unique code string and user-facing message, implement the enum constructor and required BaseErrorCode accessors (getHttpStatus/getCode/getMessage) and any toString if needed so ProjectException can consistently use ProjectErrorCode values for error mapping.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java (3)
36-43:⚠️ Potential issue | 🟠 Major팩트 수치 컬럼 nullable 허용은 집계 결과를 왜곡할 수 있습니다.
Line 36-43에서
impressions/clicks/conversions가 nullable이면0과미수집(null)의미가 섞여서 합계/평균 처리 시 예외 케이스가 늘어납니다. 기존 코멘트와 같은 이슈지만, 데이터 품질 측면에서 머지 전 확정이 필요합니다.🔧 제안 수정안
- `@Column`(name = "impressions") - private Long impressions; // 노출수 + `@Builder.Default` + `@Column`(name = "impressions", nullable = false) + private Long impressions = 0L; // 노출수 - `@Column`(name = "clicks") - private Long clicks; // 클릭수 + `@Builder.Default` + `@Column`(name = "clicks", nullable = false) + private Long clicks = 0L; // 클릭수 - `@Column`(name = "conversions") - private Long conversions; // 전환수 + `@Builder.Default` + `@Column`(name = "conversions", nullable = false) + private Long conversions = 0L; // 전환수🤖 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 36 - 43, The impressions, clicks, and conversions fields on MetricFact are currently nullable which conflates 0 and missing data; make these metrics non-nullable by updating the MetricFact entity: mark `@Column`(nullable = false) for impressions, clicks, and conversions and ensure they default to 0 (initialize fields or set in constructors/builders like the MetricFact(...) and any factory methods) so DB schema and JPA mapping enforce non-null numeric values and existing creation paths populate zeros when no data is collected.
33-34:⚠️ Potential issue | 🟡 Minor주석 처리된
dimensionType필드는 머지 전에 정리해주세요.Line 33-34는 유지/삭제 중 하나로 명확히 정리하는 게 좋습니다. 현재 상태는 의도 파악과 유지보수에 혼선을 줍니다. (기존 코멘트와 동일 이슈입니다.)
🤖 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 33 - 34, Remove the commented-out dimensionType field in the MetricFact entity: delete the two commented lines that reference "@Column(name = "dimension_type") private String dimensionType;" in the MetricFact class so the code is not left in ambiguous commented state; if the field is still required, instead uncomment and properly declare the field with the `@Column` annotation and any necessary getters/setters in MetricFact, then run formatting/compilation to ensure no references break.
30-31:⚠️ Potential issue | 🟠 Major
timeBucket의LocalDateTime은 타임존 해석 차이를 만들 수 있습니다.여러 광고 플랫폼 데이터를 합치는 팩트 테이블이면 Line 31은 UTC 기준 타입(
Instant등)으로 통일하는 편이 안전합니다. 이 항목은 이전 리뷰 포인트와 동일한 재확인입니다.🤖 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 30 - 31, The timeBucket field in MetricFact uses LocalDateTime which can cause timezone interpretation issues; change its type to a timezone-safe type (e.g., java.time.Instant or java.time.OffsetDateTime) in the MetricFact entity, update the JPA mapping (`@Column`) and any AttributeConverter or `@Convert` used for persistence so the DB stores and retrieves UTC-based instants consistently, and then update any DTOs, builders, repository queries, and tests that reference MetricFact.timeBucket to use the new type.
🧹 Nitpick comments (20)
src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.java (1)
3-5: 💡GoogleDTO,KakaoDTO와 동일하게 Javarecord도입을 검토해 보세요.세 플랫폼 DTO(
GoogleDTO,KakaoDTO,NaverDTO) 모두 같은 패턴으로 설계하는 것이 일관성 측면에서도 중요합니다. Spring 6 / Spring Boot 3 출시와 함께 Java 17+가 기본 버전이 되었으므로, 이제 Java Record를 DTO로 활용하기 좋은 시점입니다.As per coding guidelines, "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.java` around lines 3 - 5, Convert the plain class NaverDTO into a Java record to match the pattern used by GoogleDTO and KakaoDTO: replace the class declaration "class NaverDTO" with a record definition that declares the same immutable fields used in GoogleDTO/KakaoDTO (mirror their field names and types), remove any explicit getters/setters/constructors, and ensure any JSON/mapping annotations (if present in GoogleDTO/KakaoDTO) are copied onto the record components or the record itself so serialization/deserialization behavior remains the same.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java (1)
3-5:@Component누락 및 공통 인터페이스 부재 — 구현 전에 설계해 두면 좋습니다.현재 세 Converter(
GoogleConverter,KakaoConverter,NaverConverter)가 모두 평범한 Java 클래스로 선언되어 있습니다. 실제 변환 로직을 구현할 때 두 가지를 함께 챙기시면 좋겠습니다.①
@Component누락
@Component(또는@Service)가 없으면 Spring 컨테이너가 빈으로 관리하지 않아, 나중에@Autowired/생성자 주입 시NoSuchBeanDefinitionException이 발생합니다.② 공통 인터페이스 부재
PR 리뷰 포인트에서도 언급하듯이 추후 Adapter 패턴 적용 계획이 있다면, 지금 공통 인터페이스를 정의해 두는 것이 SOLID(OCP·DIP) 측면에서 훨씬 유리합니다. 새로운 플랫폼(예: Meta, TikTok)이 추가될 때 기존 코드를 수정하지 않고 새 구현체만 추가할 수 있게 됩니다.♻️ 추천 설계 예시
공통 인터페이스 (
PlatformConverter.java):public interface PlatformConverter<S, T> { T convert(S source); }각 Converter 구현체:
+import org.springframework.stereotype.Component; + +@Component -public class GoogleConverter { - // GoogleDTO -> MetricFactResponse 변환 +public class GoogleConverter implements PlatformConverter<GoogleDTO, MetricFactResponse> { + + `@Override` + public MetricFactResponse convert(GoogleDTO source) { + // TODO: 변환 로직 구현 + throw new UnsupportedOperationException("Not yet implemented"); + } }As per coding guidelines, "SOLID 원칙, 의존성 주입(DI)이 적절한지 보라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java` around lines 3 - 5, GoogleConverter 등 세 컨버터(GoogleConverter, KakaoConverter, NaverConverter)는 Spring 빈으로 관리되지 않아 주입 시 NoSuchBeanDefinitionException이 발생할 수 있고 공통 인터페이스가 없어 확장·유지보수가 어렵습니다; 따라서 PlatformConverter<S,T> 같은 제네릭 인터페이스를 정의하고 각 컨버터 클래스에 해당 인터페이스를 구현하도록 변경한 뒤(예: implements PlatformConverter<GoogleDTO, MetricFactResponse>) GoogleConverter 클래스에 `@Component` 또는 `@Service` 애노테이션을 추가해 Spring이 빈으로 관리하도록 만드세요.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/converter/KakaoConverter.java (1)
3-5:GoogleConverter와 동일하게@Component및PlatformConverter인터페이스 구현이 필요합니다.
KakaoConverter도 Spring 빈으로 등록되지 않아 주입이 불가한 상태입니다.GoogleConverter에 제안드린 공통 인터페이스(PlatformConverter<KakaoDTO, MetricFactResponse>) 방식을 동일하게 적용해 주세요.♻️ 제안 예시
+import org.springframework.stereotype.Component; + +@Component -public class KakaoConverter { - // KakaoDTO -> MetricFactResponse 변환 +public class KakaoConverter implements PlatformConverter<KakaoDTO, MetricFactResponse> { + + `@Override` + public MetricFactResponse convert(KakaoDTO source) { + // TODO: 변환 로직 구현 + throw new UnsupportedOperationException("Not yet implemented"); + } }As per coding guidelines, "SOLID 원칙, 의존성 주입(DI)이 적절한지 보라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/converter/KakaoConverter.java` around lines 3 - 5, KakaoConverter is not registered as a Spring bean and does not implement the common converter interface; modify the KakaoConverter class to implement PlatformConverter<KakaoDTO, MetricFactResponse> and annotate it with `@Component`, then implement the required convert/convertAll (or similarly named) methods to map KakaoDTO to MetricFactResponse consistent with GoogleConverter's contract so it can be injected via DI.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/dto/KakaoDTO.java (1)
3-5: 💡GoogleDTO와 동일하게 Javarecord도입을 검토해 보세요.
KakaoDTO도 외부 API 응답을 담는 불변 객체이므로, 필드 추가 시class대신record를 사용하면 불필요한 getter, setter, 생성자 오버헤드 없이 예측 가능한 구조를 만들 수 있습니다.As per coding guidelines, "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/dto/KakaoDTO.java` around lines 3 - 5, KakaoDTO is a simple immutable response holder and should be converted from a plain class to a Java record like GoogleDTO; replace the class declaration for KakaoDTO with a record declaration that lists the same response fields, remove manual getters/setters/constructors, and update any call sites to use the record's canonical constructor and accessor methods; if JSON (de)serialization is used (e.g., Jackson/Gson), ensure the record is compatible by adding the necessary annotations or modules so deserialization still works.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/converter/NaverConverter.java (1)
3-5:GoogleConverter,KakaoConverter와 동일하게@Component및 공통 인터페이스 구현이 필요합니다.
NaverConverter역시 같은 구조적 문제를 가집니다. 세 Converter 모두 동일한PlatformConverter<S, T>인터페이스를 구현하도록 설계하면, 추후 Adapter 패턴 전환 시 변경 범위를 최소화할 수 있습니다.♻️ 제안 예시
+import org.springframework.stereotype.Component; + +@Component -public class NaverConverter { - // NaverDTO -> MetricFactResponse 변환 +public class NaverConverter implements PlatformConverter<NaverDTO, MetricFactResponse> { + + `@Override` + public MetricFactResponse convert(NaverDTO source) { + // TODO: 변환 로직 구현 + throw new UnsupportedOperationException("Not yet implemented"); + } }As per coding guidelines, "SOLID 원칙, 의존성 주입(DI)이 적절한지 보라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/converter/NaverConverter.java` around lines 3 - 5, NaverConverter currently lacks the same DI and interface contract as GoogleConverter and KakaoConverter; update NaverConverter to be a Spring-managed bean by adding the `@Component` annotation and have it implement the common PlatformConverter<S,T> interface (same generic types used by GoogleConverter and KakaoConverter) while implementing the required conversion method(s); ensure method signatures match the interface so it can be swapped with Adapter-style implementations with minimal changes.src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java (1)
3-5: 💡 외부 API 응답 DTO는 Javarecord로 설계 시 이점이 큽니다.현재는 빈 스텁이지만, 필드를 추가하실 때
class대신record를 고려해 보세요. Java Records는 DTO를 간결하고 불변으로 만들어 줍니다. Google API 응답은 서버 측에서 절대 수정하지 않는 읽기 전용 데이터이므로 불변성이 특히 유용합니다. Records는 생성자, getter,equals(),hashCode(),toString()을 자동으로 생성해 보일러플레이트 코드를 줄여 줍니다.예시:
// 필드 추가 시 이렇게 설계하면 좋습니다 public record GoogleDTO( String campaignId, String campaignName, long impressions, long clicks, double spend ) {}As per coding guidelines, "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java` around lines 3 - 5, The GoogleDTO is currently a placeholder class; when adding fields convert it to a Java record by replacing the class declaration with a record declaration (e.g., change GoogleDTO to a record GoogleDTO(...) ) so the DTO is immutable and gets auto-generated constructor/getters/equals/hashCode/toString; add the response fields as record components (campaignId, campaignName, impressions, clicks, spend or other API fields), remove any setters/mutable state, and if you need validation use a compact canonical constructor inside the record (or factory methods) to keep the DTO concise and compliant with Java 17 style guidelines.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.java (1)
3-4: Converter 클래스의 역할을 명확히 해주세요 — 유틸리티 클래스인지, Spring 빈인지 구분이 필요합니다.현재
AdvertisementConverter는 빈public class이어서, 외부에서new AdvertisementConverter()로 인스턴스를 생성할 수 있습니다.
- 정적 유틸리티 메서드 방식을 선택할 경우 →
private생성자를 추가해 불필요한 인스턴스화를 막아주세요.- Spring 빈 방식을 선택할 경우 →
@Component(혹은@Service)를 달아 DI 컨테이너가 관리하도록 해주세요.- MapStruct 적용도 고려해볼 만합니다 (보일러플레이트 대폭 감소).
♻️ 정적 유틸리티 클래스 패턴 예시
public class AdvertisementConverter { + private AdvertisementConverter() { + // 유틸리티 클래스 — 인스턴스화 금지 + } + + // 예시: public static AdvertisementResponse toResponse(MetricFact fact) { ... } }🤖 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/application/mapper/AdvertisementConverter.java` around lines 3 - 4, AdvertisementConverter is currently a plain public class that can be instantiated directly; decide whether it should be a static utility or a Spring-managed bean and implement accordingly: if you want a utility class, make AdvertisementConverter’s constructor private to prevent instantiation and expose only static conversion methods; if you want DI, annotate the AdvertisementConverter class with `@Component` (or `@Service`) and convert instance methods to be non-static so Spring can inject it where needed (alternatively consider replacing it with a MapStruct mapper for boilerplate reduction).src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.java (1)
3-5: TODO 주석 잘 남겨주셨네요! 👍 필드 정의 시 Java 17record패턴도 함께 고려해주세요.코딩 가이드라인에서 Java 17의
record를 적극 활용하도록 권장하고 있습니다. DTO처럼 불변 데이터 컨테이너에record를 사용하면equals,hashCode,toString, 생성자가 자동 생성돼 보일러플레이트를 크게 줄일 수 있습니다.♻️ 개선 예시 — Java 17 내부 record 방식
public class AdvertisementResponse { - // TODO: 대시보드 공통 응답값 정의 + // TODO: 대시보드 공통 응답값 정의 + + // 예시 — 응답 케이스별 내부 record: + // public record Dashboard(String platform, String campaignName, double budgetUsageRate) {} + // public record Summary(Long id, String status) {} }As per coding guidelines, "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
🤖 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/application/dto/response/AdvertisementResponse.java` around lines 3 - 5, The AdvertisementResponse class is an empty TODO but should be implemented as an immutable Java 17 record: replace the class declaration with a record AdvertisementResponse(...) that lists the DTO fields (e.g., id, title, content, status, createdAt — use the actual fields your service expects) so equals/hashCode/toString and constructor are auto-generated; ensure all places that construct or reference AdvertisementResponse are updated to use the record's compact constructor and field accessors, and add javadoc or comments for each component if needed to document semantics.src/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/docs/ProjectControllerDocs.java (1)
3-5: Swagger@Tag어노테이션을 미리 추가해두면 좋겠습니다.메서드를 추가하기 전이라도
@Tag를 달아두면 Swagger UI에서 API 그룹이 자동으로 표시됩니다.AdvertisementControllerDocs와 같은 패턴을 유지해주세요.♻️ 개선 예시
+import io.swagger.v3.oas.annotations.tags.Tag; + +@Tag(name = "Project", description = "프로젝트 관리 API") public interface ProjectControllerDocs { }🤖 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/project/presentation/docs/ProjectControllerDocs.java` around lines 3 - 5, ProjectControllerDocs 인터페이스에 Swagger `@Tag` 어노테이션을 추가해 API 그룹이 Swagger UI에 표시되도록 하세요: ProjectControllerDocs 인터페이스(현재 비어있는 인터페이스)에 io.swagger.v3.oas.annotations.tags.Tag를 import하고 `@Tag`(name = "Project", description = "프로젝트 관련 API")와 같이 AdvertisementControllerDocs와 동일한 패턴으로 명명/설명 템플릿을 적용합니다.src/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/response/ProjectResponse.java (1)
3-4:AdvertisementResponse와의 일관성 유지를 위해 TODO 주석과 Java 17 record 패턴을 준비해주세요.
AdvertisementResponse에는// TODO: 대시보드 공통 응답값 정의주석이 있지만,ProjectResponse에는 없어서 나중에 필드 정의가 누락될 위험이 있습니다.또한 코딩 가이드라인에 따르면 Java 17의
record문법을 적극 활용해야 합니다. 필드를 추가할 시점에 아래 방식으로 전환하면 불변 DTO를 훨씬 간결하게 표현할 수 있습니다.♻️ 개선 예시 (Java 17 record 패턴)
public class ProjectResponse { + // TODO: 프로젝트 응답 DTO 필드 정의 + + // 예시 — 내부 record 방식: + // public record Detail(Long id, String name, LocalDate endDate) {} + // public record Summary(Long id, String name) {} }🤖 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/project/application/dto/response/ProjectResponse.java` around lines 3 - 4, Add the same TODO comment used in AdvertisementResponse to ProjectResponse (e.g., "// TODO: 대시보드 공통 응답값 정의") and prepare the file for Java 17 record usage by replacing the empty class declaration with a note that it should be converted to a record when fields are added (or directly change it to a record skeleton like "public record ProjectResponse(...) {}" when you know the fields); reference ProjectResponse and AdvertisementResponse so reviewers see the intended consistency and the plan to use the record pattern for an immutable DTO.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryService.java (1)
3-4: 인터페이스 구조 자체는 깔끔합니다! ✅ 다만, 메서드 설계 시 PR 토론 내용을 참고해주세요.PR 코멘트에서 논의된 것처럼,
MetricFact만으로는 "플랫폼별 광고 개수"와 같은 집계 쿼리가 어렵습니다.ADVERTISEMENT엔티티와의 연관관계가 추가됐으니, 이 서비스에 메서드를 추가할 때 아래 두 역할을 명확히 분리해서 설계해주시면 좋겠습니다:
MetricFact기반 → 예산 소진율, 노출/클릭 등 집계 지표 조회Advertisement기반 → 광고 개수, 플랫폼별 광고 목록 조회🤖 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/domain/service/AdvertisementQueryService.java` around lines 3 - 4, The AdvertisementQueryService interface needs explicit separation of responsibilities: add MetricFact-based query methods (e.g., methods such as getAggregatedMetricsByCampaign, getBudgetConsumptionByCampaign, or fetchExposureClickMetrics) for aggregated KPI retrieval, and add Advertisement-based query methods (e.g., methods such as countAdvertisementsByPlatform, listAdvertisementsByPlatform, or findAdvertisementsByCampaign) for entity/listing counts and platform-specific lists; ensure method signatures accept appropriate filter/criteria DTOs (campaignId, timeRange, platform, pagination) and return clearly typed DTOs for aggregates vs. advertisement lists so callers can distinguish aggregation vs. entity queries.src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.java (1)
3-4: 빈 서비스 인터페이스는 계약이 생길 때 도입하는 편이 유지보수에 유리합니다.지금 상태에선 타입만 늘고 이점이 거의 없습니다.
- 곧 메서드가 추가될 예정이면 인터페이스에 계약 시그니처를 먼저 정의하고,
- 아니라면 당장은 구현체만 두는 쪽이 단순합니다.
As per coding guidelines
src/main/java/**/*.java: "SOLID 원칙, 의존성 주입(DI), 예외 처리(GlobalExceptionHandler)가 적절한지 보라."🤖 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/project/domain/service/ProjectService.java` around lines 3 - 4, 현재 빈 인터페이스 ProjectService는 실제 계약이 없다면 불필요하니 결정하세요: 만약 곧 공개될 서비스 계약(예: createProject, getProjectById, updateProject, deleteProject 등)이 예정되어 있으면 ProjectService 인터페이스에 해당 메서드 시그니처를 먼저 정의하고 구현체(ProjectServiceImpl 등)가 그 인터페이스를 구현하도록 변경하되 DI(생성자 주입)와 예외 처리 흐름(GlobalExceptionHandler 연동)을 고려해 시그니처에 필요한 예외/반환 타입을 명시하세요; 반대로 계약이 아직 없고 당장은 단일 구현체만 존재한다면 인터페이스를 제거하고 구현 클래스만 유지해 복잡도를 낮추세요.src/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/request/ProjectRequest.java (1)
3-4: 향후 필드 추가 시 Java 17Record를 활용하면 더 이상적입니다.현재는 스캐폴딩 단계라 빈 클래스로 두는 것은 이해할 수 있지만, 요청 DTO처럼 불변(immutable) 데이터 운반 객체는 Java 17의
Record로 선언하면equals,hashCode,toString이 자동으로 생성되고 Lombok 없이도 깔끔하게 유지됩니다.예를 들어 필드가 추가될 때:
// 기존 class 방식 public class ProjectRequest { private String name; private String description; // getter, equals, hashCode, toString... } // Java 17 Record 방식 (훨씬 간결!) public record ProjectRequest( String name, String description ) {}🤖 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/project/application/dto/request/ProjectRequest.java` around lines 3 - 4, The ProjectRequest is currently an empty mutable class; when you add request fields convert it to a Java 17 record to make it immutable and auto-generate equals/hashCode/toString instead of using Lombok—replace the class declaration ProjectRequest { ... } with a record declaration ProjectRequest(<fields>) and move all request fields into the record component list so getters and canonical constructor are implicitly provided.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java (1)
6-7: 기본 리포지토리 구조 깔끔합니다. ✅ 하지만 향후 핵심 쿼리 메서드 추가가 필요합니다.PR 코멘트에서 논의된 주요 조회 시나리오(플랫폼별 집계, 기간별 필터링 등)를 고려하면, 다음과 같은 커스텀 쿼리 메서드들이 필요하게 됩니다:
// 예시: 특정 프로바이더 + 기간으로 MetricFact 조회 List<MetricFact> findByProviderAndTimeBucketBetween( Provider provider, LocalDateTime start, LocalDateTime end ); // 예시: 프로젝트별 MetricFact 집계 (N+1 방지를 위해 `@Query` + fetch join 권장) `@Query`("SELECT m FROM MetricFact m JOIN FETCH m.project WHERE m.provider = :provider") List<MetricFact> findByProviderWithProject(`@Param`("provider") Provider provider);지금 당장 추가하지 않아도 되지만, 서비스 레이어 구현 시 N+1 문제 방지를 위한
JOIN FETCH또는@EntityGraph설계를 미리 고려해두시면 좋겠습니다.🤖 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/repository/MetricFactRepository.java` around lines 6 - 7, Add query methods to MetricFactRepository to support common lookup patterns: implement signatures like findByProviderAndTimeBucketBetween(Provider provider, LocalDateTime start, LocalDateTime end) and a provider-with-project query such as findByProviderWithProject(Provider provider) using `@Query` with JOIN FETCH or annotate with `@EntityGraph` to avoid N+1; update the MetricFactRepository interface (which extends JpaRepository<MetricFact, Long>) to include these methods so service-layer aggregation/filtering can use them.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/Advertisement.java (2)
31-36: 주석 처리된AdStatus코드를 그대로 두는 것은 코드 스멜입니다.성능 우려로 인해
AdStatus필드를 보류한 결정 자체는 합리적이지만, 주석 코드를 소스에 남겨두면 리뷰어와 미래의 개발자가 혼란을 겪습니다. 결정 사항과 이유를 코드 주석 대신 이슈 트래커나 ADR(Architecture Decision Record)로 관리하고, 관련 코드는 삭제하는 것이 좋습니다.-// `@Enumerated`(EnumType.STRING) -// `@Column`(name = "status", nullable = false) -// `@ColumnDefault`("'ON_GOING'") -// AdStatus 는 ON_GOING, PAUSED, OVER 가 존재 -// join 연산으로 인한 성능 저하 우려 -> 일단은 주석 처리 -// private AdStatus adStatus;🤖 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/Advertisement.java` around lines 31 - 36, Remove the commented-out AdStatus field and its annotations from the Advertisement entity (the commented lines referencing `@Enumerated`, `@Column`, `@ColumnDefault` and "private AdStatus adStatus") to eliminate code smell; instead document the decision and rationale in your project tracker or an ADR (link/reference ID) so future reviewers know why the status field was deferred; if you need to preserve the decision in-code, replace the commented block with a single-line TODO pointing to the ADR/issue ID.
27-29:startDate,endDate에@Column어노테이션이 누락되었습니다.
@Column없이 선언하면 JPA 네이밍 전략에 완전히 의존하게 됩니다. Spring Boot 3.x의 기본SpringPhysicalNamingStrategy는 camelCase를 snake_case로 변환(start_date,end_date)하므로 현재는 동작하지만,nullable = false제약이나columnDefinition등을 명시하려면 어노테이션이 필요합니다. 명시적으로 선언하는 것이 더 안전합니다.+ `@Column`(name = "start_date") private LocalDateTime startDate; + `@Column`(name = "end_date") private LocalDateTime endDate;🤖 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/Advertisement.java` around lines 27 - 29, In the Advertisement entity add explicit JPA mapping for the fields startDate and endDate: annotate the LocalDateTime fields startDate and endDate in class Advertisement with `@Column` and set desired attributes (at minimum nullable = false if they must be required; optionally name = "start_date"/"end_date" and a columnDefinition or length/precision as needed) so you don't rely solely on the physical naming strategy and can enforce constraints and SQL type details.src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectServiceImpl.java (1)
10-11:@RequiredArgsConstructor(access = AccessLevel.PROTECTED)의 의도를 재확인해주세요.현재 클래스에
final필드가 없으므로, 이 어노테이션은protected접근 제한자를 가진 no-arg 생성자를 생성합니다. Spring은 리플렉션을 통해protected생성자로도 빈을 생성할 수 있지만, 이 패턴은 다소 비관용적입니다.코드베이스 내
EmailService처럼 기본적으로@RequiredArgsConstructor만 사용하고, 주입받을final필드가 추가될 때 자연스럽게 생성자 주입이 이루어지도록 하는 것이 더 일관적입니다.-@RequiredArgsConstructor(access = AccessLevel.PROTECTED) +@RequiredArgsConstructor public class ProjectServiceImpl implements ProjectService {🤖 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/project/domain/service/ProjectServiceImpl.java` around lines 10 - 11, The class-level annotation `@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) on ProjectServiceImpl currently generates a protected no-arg constructor because there are no final fields; change this to the default `@RequiredArgsConstructor` (remove the access = AccessLevel.PROTECTED) so constructor generation behaves consistently with other services like EmailService and will create the proper constructor when final dependency fields are later added for constructor injection.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/AdvertisementController.java (1)
8-11:@RequiredArgsConstructor(access = AccessLevel.PROTECTED)가 컨트롤러에는 어울리지 않습니다.
ProjectServiceImpl과 동일하게, 현재final필드가 없는 상태에서 이 어노테이션은protectedno-arg 생성자만 생성합니다. 컨트롤러는 Spring이 직접 인스턴스화하는 빈이기 때문에protected생성자는 더욱 불필요합니다.서비스(
AdvertisementService)나 유스케이스 클래스를 주입받을final필드가 추가될 때,AccessLevel.PROTECTED없이 표준 패턴을 사용하세요:-@RequiredArgsConstructor(access = AccessLevel.PROTECTED) +@RequiredArgsConstructor public class AdvertisementController implements AdvertisementControllerDocs {🤖 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/presentation/AdvertisementController.java` around lines 8 - 11, The controller is annotated with `@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which currently generates only a protected no-arg constructor and is inappropriate for a Spring `@RestController`; remove the access attribute (or remove the annotation until you add final injection fields) so that Lombok will generate a public constructor when you add final dependencies like AdvertisementService or other use-case fields in AdvertisementController, ensuring proper Spring bean instantiation.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Provider.java (1)
3-5:Provider이름이 두 개의 다른 도메인에서 중복 선언되어 있습니다.검증 결과,
com.whereyouad.WhereYouAd.domains.user.domain.constant.Provider와com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.Provider두 개의 별도Providerenum이 존재합니다. 현재는 각 클래스가 명시적 import로 올바른 패키지에서 가져오고 있어 런타임 충돌은 발생하지 않지만, IDE 자동 완성과 코드 가독성 측면에서 혼란을 야기할 수 있습니다.광고 도메인의 명확성을 위해
AdProvider또는AdvertisementProvider로 변경하는 것을 강력히 권장합니다. 특히 향후 새로운 기능 추가 시 개발자가 어느 Provider를 사용해야 하는지 한눈에 파악할 수 있게 됩니다.-public enum Provider { +public enum AdProvider { GOOGLE, KAKAO, NAVER // 추후 값 추가 }변경 후
Advertisement.java와MetricFact.java의 import 문도 함께 업데이트하면 됩니다.🤖 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/domain/constant/Provider.java` around lines 3 - 5, Rename the enum com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.Provider to AdvertisementProvider (or AdProvider) to avoid name collision with the user-domain Provider; update the enum declaration name in Provider.java to AdvertisementProvider and adjust all usages/imports in Advertisement.java and MetricFact.java to import and reference AdvertisementProvider instead of the duplicate Provider; ensure package remains com.whereyouad.WhereYouAd.domains.advertisement.domain.constant and run a build to fix any remaining references.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java (1)
15-15: 팩트 테이블 조회 패턴에 맞는 인덱스를@Table에 명시하는 것을 권장합니다.
MetricFact는 시간축 조회가 많을 가능성이 높아서,ad_id + time_bucket,provider + time_bucket같은 복합 인덱스가 없으면 데이터 증가 시 풀스캔 가능성이 커집니다.⚙️ 권장 리팩터링
-@Table(name = "metric_fact") +@Table( + name = "metric_fact", + indexes = { + `@Index`(name = "idx_metric_fact_ad_time", columnList = "ad_id,time_bucket"), + `@Index`(name = "idx_metric_fact_provider_time", columnList = "provider,time_bucket") + } +)🤖 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` at line 15, The `@Table` on the MetricFact entity needs composite indexes for common query patterns; update the MetricFact class annotation to add `@Table`(indexes = { `@Index`(name = "idx_metric_ad_time", columnList = "ad_id,time_bucket"), `@Index`(name = "idx_metric_provider_time", columnList = "provider,time_bucket") }) (or equivalent column names used in the entity) so ad_id + time_bucket and provider + time_bucket queries use DB indexes instead of full scans; ensure the columnList matches the MetricFact field-to-column mappings and choose index names that follow your naming conventions.
🤖 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/advertisement/exception/code/AdvertisementErrorCode.java`:
- Around line 10-17: The AdvertisementErrorCode enum is empty causing
AdvertisementException and global error handling to lack domain-specific codes;
populate AdvertisementErrorCode with a minimal set of common error entries
(e.g., NOT_FOUND, INVALID_REQUEST, CONFLICT, INTERNAL_ERROR) each carrying the
HttpStatus, a unique String code (like "AD_0001"), and a human message, and
implement the BaseErrorCode methods (getHttpStatus, getCode, getMessage) so
AdvertisementException and the GlobalExceptionHandler can read standardized
values; update any constructors or usages in AdvertisementException that
reference AdvertisementErrorCode to use the new enum constants.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/Advertisement.java`:
- Around line 16-21: The Advertisement entity lacks a field to store the
external platform ad identifier; add a new String field externalAdId to the
Advertisement class (with a `@Column`(name="external_ad_id", nullable=false) and a
unique index/constraint together with platform identifier if you support
multiple platforms) and update its getters/setters and constructors; ensure DB
mapping by adding the column annotation and, if needed, a uniqueness constraint
(e.g., unique on (platform, external_ad_id)) so synchronization routines can
reliably find or upsert records by Advertisement.externalAdId.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/ProjectController.java`:
- Around line 8-11: ProjectController currently uses
`@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which will generate a
protected constructor if a private final service (e.g. private final
ProjectService projectService) is added later; change this to the default public
constructor by removing the access attribute so the annotation is simply
`@RequiredArgsConstructor` to match other controllers (OrgController,
UserController, AuthController) and ensure consistent public constructor
behavior.
---
Duplicate comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.java`:
- Around line 3-4: AdvertisementRequest is an empty class and should mirror the
documented TODO pattern from AdvertisementResponse and adopt the Java 17 record
style when fields are added; add a TODO comment in AdvertisementRequest similar
to AdvertisementResponse (e.g., "// TODO: 대시보드 공통 요청값 정의") and when you
introduce request fields refactor the class into a record (record
AdvertisementRequest(...)) to follow the coding guideline, ensuring
constructors/getters are removed and tests/usage sites are updated to use the
record components.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java`:
- Around line 36-43: The impressions, clicks, and conversions fields on
MetricFact are currently nullable which conflates 0 and missing data; make these
metrics non-nullable by updating the MetricFact entity: mark `@Column`(nullable =
false) for impressions, clicks, and conversions and ensure they default to 0
(initialize fields or set in constructors/builders like the MetricFact(...) and
any factory methods) so DB schema and JPA mapping enforce non-null numeric
values and existing creation paths populate zeros when no data is collected.
- Around line 33-34: Remove the commented-out dimensionType field in the
MetricFact entity: delete the two commented lines that reference "@Column(name =
"dimension_type") private String dimensionType;" in the MetricFact class so the
code is not left in ambiguous commented state; if the field is still required,
instead uncomment and properly declare the field with the `@Column` annotation and
any necessary getters/setters in MetricFact, then run formatting/compilation to
ensure no references break.
- Around line 30-31: The timeBucket field in MetricFact uses LocalDateTime which
can cause timezone interpretation issues; change its type to a timezone-safe
type (e.g., java.time.Instant or java.time.OffsetDateTime) in the MetricFact
entity, update the JPA mapping (`@Column`) and any AttributeConverter or `@Convert`
used for persistence so the DB stores and retrieves UTC-based instants
consistently, and then update any DTOs, builders, repository queries, and tests
that reference MetricFact.timeBucket to use the new type.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.java`:
- Around line 3-4: ProjectConverter is currently an empty class like
AdvertisementConverter and should be clearly defined as either a static utility
or a Spring bean; either add a private no-arg constructor to make it a proper
utility class (prevent instantiation) or annotate the class with `@Component` (and
import org.springframework.stereotype.Component) to register it as a Spring
bean—apply the same approach you used for AdvertisementConverter so the intent
is explicit.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/exception/code/ProjectErrorCode.java`:
- Around line 10-16: ProjectErrorCode enum is empty so ProjectException can't
map domain errors; add meaningful constants (e.g., PROJECT_NOT_FOUND,
PROJECT_FORBIDDEN, PROJECT_VALIDATION_FAILED) that provide HttpStatus, unique
code string and user-facing message, implement the enum constructor and required
BaseErrorCode accessors (getHttpStatus/getCode/getMessage) and any toString if
needed so ProjectException can consistently use ProjectErrorCode values for
error mapping.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.java`:
- Around line 31-33: The Project entity's organization association is currently
optional, allowing org_id to be null; make this relationship required by
updating the organization field mapping in the Project class: set
`@JoinColumn`(name = "org_id", nullable = false) and (optionally) add a
javax.validation constraint like `@NotNull` on the organization field to enforce
non-null at the bean validation level; keep FetchType.LAZY as-is to avoid eager
loading and ensure the DB column is created non-nullable on schema
generation/migration.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.java`:
- Around line 3-5: The AdvertisementResponse class is an empty TODO but should
be implemented as an immutable Java 17 record: replace the class declaration
with a record AdvertisementResponse(...) that lists the DTO fields (e.g., id,
title, content, status, createdAt — use the actual fields your service expects)
so equals/hashCode/toString and constructor are auto-generated; ensure all
places that construct or reference AdvertisementResponse are updated to use the
record's compact constructor and field accessors, and add javadoc or comments
for each component if needed to document semantics.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.java`:
- Around line 3-4: AdvertisementConverter is currently a plain public class that
can be instantiated directly; decide whether it should be a static utility or a
Spring-managed bean and implement accordingly: if you want a utility class, make
AdvertisementConverter’s constructor private to prevent instantiation and expose
only static conversion methods; if you want DI, annotate the
AdvertisementConverter class with `@Component` (or `@Service`) and convert instance
methods to be non-static so Spring can inject it where needed (alternatively
consider replacing it with a MapStruct mapper for boilerplate reduction).
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Provider.java`:
- Around line 3-5: Rename the enum
com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.Provider to
AdvertisementProvider (or AdProvider) to avoid name collision with the
user-domain Provider; update the enum declaration name in Provider.java to
AdvertisementProvider and adjust all usages/imports in Advertisement.java and
MetricFact.java to import and reference AdvertisementProvider instead of the
duplicate Provider; ensure package remains
com.whereyouad.WhereYouAd.domains.advertisement.domain.constant and run a build
to fix any remaining references.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryService.java`:
- Around line 3-4: The AdvertisementQueryService interface needs explicit
separation of responsibilities: add MetricFact-based query methods (e.g.,
methods such as getAggregatedMetricsByCampaign, getBudgetConsumptionByCampaign,
or fetchExposureClickMetrics) for aggregated KPI retrieval, and add
Advertisement-based query methods (e.g., methods such as
countAdvertisementsByPlatform, listAdvertisementsByPlatform, or
findAdvertisementsByCampaign) for entity/listing counts and platform-specific
lists; ensure method signatures accept appropriate filter/criteria DTOs
(campaignId, timeRange, platform, pagination) and return clearly typed DTOs for
aggregates vs. advertisement lists so callers can distinguish aggregation vs.
entity queries.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/Advertisement.java`:
- Around line 31-36: Remove the commented-out AdStatus field and its annotations
from the Advertisement entity (the commented lines referencing `@Enumerated`,
`@Column`, `@ColumnDefault` and "private AdStatus adStatus") to eliminate code
smell; instead document the decision and rationale in your project tracker or an
ADR (link/reference ID) so future reviewers know why the status field was
deferred; if you need to preserve the decision in-code, replace the commented
block with a single-line TODO pointing to the ADR/issue ID.
- Around line 27-29: In the Advertisement entity add explicit JPA mapping for
the fields startDate and endDate: annotate the LocalDateTime fields startDate
and endDate in class Advertisement with `@Column` and set desired attributes (at
minimum nullable = false if they must be required; optionally name =
"start_date"/"end_date" and a columnDefinition or length/precision as needed) so
you don't rely solely on the physical naming strategy and can enforce
constraints and SQL type details.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.java`:
- Line 15: The `@Table` on the MetricFact entity needs composite indexes for
common query patterns; update the MetricFact class annotation to add
`@Table`(indexes = { `@Index`(name = "idx_metric_ad_time", columnList =
"ad_id,time_bucket"), `@Index`(name = "idx_metric_provider_time", columnList =
"provider,time_bucket") }) (or equivalent column names used in the entity) so
ad_id + time_bucket and provider + time_bucket queries use DB indexes instead of
full scans; ensure the columnList matches the MetricFact field-to-column
mappings and choose index names that follow your naming conventions.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.java`:
- Around line 6-7: Add query methods to MetricFactRepository to support common
lookup patterns: implement signatures like
findByProviderAndTimeBucketBetween(Provider provider, LocalDateTime start,
LocalDateTime end) and a provider-with-project query such as
findByProviderWithProject(Provider provider) using `@Query` with JOIN FETCH or
annotate with `@EntityGraph` to avoid N+1; update the MetricFactRepository
interface (which extends JpaRepository<MetricFact, Long>) to include these
methods so service-layer aggregation/filtering can use them.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/AdvertisementController.java`:
- Around line 8-11: The controller is annotated with
`@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which currently
generates only a protected no-arg constructor and is inappropriate for a Spring
`@RestController`; remove the access attribute (or remove the annotation until you
add final injection fields) so that Lombok will generate a public constructor
when you add final dependencies like AdvertisementService or other use-case
fields in AdvertisementController, ensuring proper Spring bean instantiation.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/request/ProjectRequest.java`:
- Around line 3-4: The ProjectRequest is currently an empty mutable class; when
you add request fields convert it to a Java 17 record to make it immutable and
auto-generate equals/hashCode/toString instead of using Lombok—replace the class
declaration ProjectRequest { ... } with a record declaration
ProjectRequest(<fields>) and move all request fields into the record component
list so getters and canonical constructor are implicitly provided.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/response/ProjectResponse.java`:
- Around line 3-4: Add the same TODO comment used in AdvertisementResponse to
ProjectResponse (e.g., "// TODO: 대시보드 공통 응답값 정의") and prepare the file for Java
17 record usage by replacing the empty class declaration with a note that it
should be converted to a record when fields are added (or directly change it to
a record skeleton like "public record ProjectResponse(...) {}" when you know the
fields); reference ProjectResponse and AdvertisementResponse so reviewers see
the intended consistency and the plan to use the record pattern for an immutable
DTO.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.java`:
- Around line 3-4: 현재 빈 인터페이스 ProjectService는 실제 계약이 없다면 불필요하니 결정하세요: 만약 곧 공개될
서비스 계약(예: createProject, getProjectById, updateProject, deleteProject 등)이 예정되어
있으면 ProjectService 인터페이스에 해당 메서드 시그니처를 먼저 정의하고 구현체(ProjectServiceImpl 등)가 그
인터페이스를 구현하도록 변경하되 DI(생성자 주입)와 예외 처리 흐름(GlobalExceptionHandler 연동)을 고려해 시그니처에 필요한
예외/반환 타입을 명시하세요; 반대로 계약이 아직 없고 당장은 단일 구현체만 존재한다면 인터페이스를 제거하고 구현 클래스만 유지해 복잡도를
낮추세요.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectServiceImpl.java`:
- Around line 10-11: The class-level annotation `@RequiredArgsConstructor`(access
= AccessLevel.PROTECTED) on ProjectServiceImpl currently generates a protected
no-arg constructor because there are no final fields; change this to the default
`@RequiredArgsConstructor` (remove the access = AccessLevel.PROTECTED) so
constructor generation behaves consistently with other services like
EmailService and will create the proper constructor when final dependency fields
are later added for constructor injection.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/docs/ProjectControllerDocs.java`:
- Around line 3-5: ProjectControllerDocs 인터페이스에 Swagger `@Tag` 어노테이션을 추가해 API 그룹이
Swagger UI에 표시되도록 하세요: ProjectControllerDocs 인터페이스(현재 비어있는 인터페이스)에
io.swagger.v3.oas.annotations.tags.Tag를 import하고 `@Tag`(name = "Project",
description = "프로젝트 관련 API")와 같이 AdvertisementControllerDocs와 동일한 패턴으로 명명/설명
템플릿을 적용합니다.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.java`:
- Around line 3-5: GoogleConverter 등 세 컨버터(GoogleConverter, KakaoConverter,
NaverConverter)는 Spring 빈으로 관리되지 않아 주입 시 NoSuchBeanDefinitionException이 발생할 수 있고
공통 인터페이스가 없어 확장·유지보수가 어렵습니다; 따라서 PlatformConverter<S,T> 같은 제네릭 인터페이스를 정의하고 각 컨버터
클래스에 해당 인터페이스를 구현하도록 변경한 뒤(예: implements PlatformConverter<GoogleDTO,
MetricFactResponse>) GoogleConverter 클래스에 `@Component` 또는 `@Service` 애노테이션을 추가해
Spring이 빈으로 관리하도록 만드세요.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.java`:
- Around line 3-5: The GoogleDTO is currently a placeholder class; when adding
fields convert it to a Java record by replacing the class declaration with a
record declaration (e.g., change GoogleDTO to a record GoogleDTO(...) ) so the
DTO is immutable and gets auto-generated
constructor/getters/equals/hashCode/toString; add the response fields as record
components (campaignId, campaignName, impressions, clicks, spend or other API
fields), remove any setters/mutable state, and if you need validation use a
compact canonical constructor inside the record (or factory methods) to keep the
DTO concise and compliant with Java 17 style guidelines.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/converter/KakaoConverter.java`:
- Around line 3-5: KakaoConverter is not registered as a Spring bean and does
not implement the common converter interface; modify the KakaoConverter class to
implement PlatformConverter<KakaoDTO, MetricFactResponse> and annotate it with
`@Component`, then implement the required convert/convertAll (or similarly named)
methods to map KakaoDTO to MetricFactResponse consistent with GoogleConverter's
contract so it can be injected via DI.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/dto/KakaoDTO.java`:
- Around line 3-5: KakaoDTO is a simple immutable response holder and should be
converted from a plain class to a Java record like GoogleDTO; replace the class
declaration for KakaoDTO with a record declaration that lists the same response
fields, remove manual getters/setters/constructors, and update any call sites to
use the record's canonical constructor and accessor methods; if JSON
(de)serialization is used (e.g., Jackson/Gson), ensure the record is compatible
by adding the necessary annotations or modules so deserialization still works.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/converter/NaverConverter.java`:
- Around line 3-5: NaverConverter currently lacks the same DI and interface
contract as GoogleConverter and KakaoConverter; update NaverConverter to be a
Spring-managed bean by adding the `@Component` annotation and have it implement
the common PlatformConverter<S,T> interface (same generic types used by
GoogleConverter and KakaoConverter) while implementing the required conversion
method(s); ensure method signatures match the interface so it can be swapped
with Adapter-style implementations with minimal changes.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.java`:
- Around line 3-5: Convert the plain class NaverDTO into a Java record to match
the pattern used by GoogleDTO and KakaoDTO: replace the class declaration "class
NaverDTO" with a record definition that declares the same immutable fields used
in GoogleDTO/KakaoDTO (mirror their field names and types), remove any explicit
getters/setters/constructors, and ensure any JSON/mapping annotations (if
present in GoogleDTO/KakaoDTO) are copied onto the record components or the
record itself so serialization/deserialization behavior remains the same.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/request/AdvertisementRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/dto/response/AdvertisementResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/application/mapper/AdvertisementConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Grain.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/constant/Provider.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/AdvertisementQueryServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/AdvertisementException.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/exception/code/AdvertisementErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/Advertisement.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/entity/MetricFact.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/persistence/repository/MetricFactRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/AdvertisementController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/AdvertisementControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/request/ProjectRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/dto/response/ProjectResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/application/mapper/ProjectConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/domain/service/ProjectServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/exception/ProjectException.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/exception/code/ProjectErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/entity/Project.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/persistence/repository/ProjectRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/ProjectController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/project/presentation/docs/ProjectControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/converter/GoogleConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/google/dto/GoogleDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/converter/KakaoConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/kakao/dto/KakaoDTO.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/converter/NaverConverter.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.java
Comment on lines
+10
to
+17
| public enum AdvertisementErrorCode implements BaseErrorCode { | ||
|
|
||
| // TODO: 에러 코드 추가 | ||
| ; | ||
|
|
||
| private final HttpStatus httpStatus; | ||
| private final String code; | ||
| private final String message; |
There was a problem hiding this comment.
광고 도메인 에러코드가 비어 있어 예외 표준화가 동작하지 않습니다.
TODO 상태로 두면 AdvertisementException이 실사용 시 도메인 식별 가능한 에러코드를 제공하지 못합니다. 최소 공통 코드부터 채워두는 걸 권장합니다.
🔧 제안 수정안
public enum AdvertisementErrorCode implements BaseErrorCode {
-
- // TODO: 에러 코드 추가
- ;
+ ADVERTISEMENT_NOT_FOUND(HttpStatus.NOT_FOUND, "ADVERTISEMENT_404", "광고를 찾을 수 없습니다."),
+ ADVERTISEMENT_BAD_REQUEST(HttpStatus.BAD_REQUEST, "ADVERTISEMENT_400", "잘못된 광고 요청입니다."),
+ ADVERTISEMENT_FORBIDDEN(HttpStatus.FORBIDDEN, "ADVERTISEMENT_403", "광고 접근 권한이 없습니다.");
private final HttpStatus httpStatus;
private final String code;
private final String message;
}As per coding guidelines src/main/java/**/*.java: "SOLID 원칙, 의존성 주입(DI), 예외 처리(GlobalExceptionHandler)가 적절한지 보라."
🤖 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/exception/code/AdvertisementErrorCode.java`
around lines 10 - 17, The AdvertisementErrorCode enum is empty causing
AdvertisementException and global error handling to lack domain-specific codes;
populate AdvertisementErrorCode with a minimal set of common error entries
(e.g., NOT_FOUND, INVALID_REQUEST, CONFLICT, INTERNAL_ERROR) each carrying the
HttpStatus, a unique String code (like "AD_0001"), and a human message, and
implement the BaseErrorCode methods (getHttpStatus, getCode, getMessage) so
AdvertisementException and the GlobalExceptionHandler can read standardized
values; update any constructors or usages in AdvertisementException that
reference AdvertisementErrorCode to use the new enum constants.
Comment on lines
+16
to
+21
| public class Advertisement extends BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "ad_id") | ||
| private Long id; |
There was a problem hiding this comment.
외부 광고 플랫폼의 고유 ID(externalAdId)가 없으면 동기화가 불가능합니다.
현재 Advertisement 엔티티에는 Google/Kakao/Naver 각 플랫폼에서 부여한 고유 광고 ID를 저장하는 필드가 없습니다. 향후 외부 API로부터 광고 데이터를 동기화할 때, 어떤 플랫폼 광고가 어떤 DB 레코드에 해당하는지 매핑할 수 없게 됩니다.
예를 들어 Google Ads API는 각 광고에 고유한 ad_id를 제공하는데, 이 값이 없으면 중복 insert나 업데이트 대상 특정이 불가능합니다.
+ `@Column`(name = "external_ad_id", nullable = false)
+ private String externalAdId; // 외부 플랫폼(Google/Kakao/Naver)에서 부여한 광고 ID🤖 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/Advertisement.java`
around lines 16 - 21, The Advertisement entity lacks a field to store the
external platform ad identifier; add a new String field externalAdId to the
Advertisement class (with a `@Column`(name="external_ad_id", nullable=false) and a
unique index/constraint together with platform identifier if you support
multiple platforms) and update its getters/setters and constructors; ensure DB
mapping by adding the column annotation and, if needed, a uniqueness constraint
(e.g., unique on (platform, external_ad_id)) so synchronization routines can
reliably find or upsert records by Advertisement.externalAdId.
Comment on lines
+8
to
+11
| @RestController | ||
| @RequiredArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @RequestMapping("/api/project") | ||
| public class ProjectController implements ProjectControllerDocs { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 프로젝트 내 모든 Java 컨트롤러의 `@RequiredArgsConstructor` 패턴 확인
echo "=== Searching for `@RequiredArgsConstructor` patterns in controllers ==="
rg -n "@RequiredArgsConstructor" --type java -A 2 -B 0
echo -e "\n=== Searching in presentation layer specifically ==="
find . -path "*/presentation/*" -name "*Controller.java" -type f | head -20Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 9900
@RequiredArgsConstructor(access = AccessLevel.PROTECTED) — 팀 컨트롤러와 일관성 맞춰주세요.
현재는 final 필드가 없어서 문제가 없지만, 나중에 서비스 필드(private final ProjectService projectService)가 추가되면 Lombok이 protected 접근자를 가진 생성자를 생성합니다.
코드베이스를 살펴보니 대부분의 컨트롤러(OrgController, UserController, AuthController)는 @RequiredArgsConstructor 기본값(PUBLIC)을 사용하고 있는데, ProjectController만 PROTECTED를 적용했네요. 일관성 측면에서:
- Spring Framework 6(Boot 3.x)은 protected 생성자도 리플렉션으로 주입 가능합니다.
- 하지만 CGLIB 기반 프록시가 필요한 일부 시나리오에서는 예기치 않은 문제가 발생할 수 있습니다.
- 팀 표준(공개 생성자)에 맞춰서
@RequiredArgsConstructor기본값으로 변경하거나, 팀 전체 컨트롤러에서 명시적으로 PROTECTED를 사용하도록 통일해야 합니다.
🤖 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/project/presentation/ProjectController.java`
around lines 8 - 11, ProjectController currently uses
`@RequiredArgsConstructor`(access = AccessLevel.PROTECTED) which will generate a
protected constructor if a private final service (e.g. private final
ProjectService projectService) is added later; change this to the default public
constructor by removing the access attribute so the annotation is simply
`@RequiredArgsConstructor` to match other controllers (OrgController,
UserController, AuthController) and ensure consistent public constructor
behavior.
Collaborator
|
우선은 domain/advertisement/persistence/entity 디렉터리 내부에 Advertisement 엔티티 추가해봤습니다. 찾아보니 join 연산으로 인한 성능저하는 조회 로직을 짤 때 Redis 활용해서 status = ON_GOING 인 광고의 ad_id 를 미리 캐싱해두거나, 반정규화로 MetricFact 테이블에 status 를 넣는 방법도 있는 것 같은데, 이 부분은 다른 분들 추후에 여유 될때 의견 듣고 해봐야 할 것 같아서 일단 Advertisement 엔티티에서 status 필드는 주석처리 해놨습니다. |
jinnieusLab
approved these changes
Feb 26, 2026
Collaborator
There was a problem hiding this comment.
P4: 두 분 다 너무 고생하셨습니다..! 코드도 깔끔하게 작성해주셔서 크게 수정해야할 부분은 없는 듯 합니다!
제 생각에도 개별 광고 조회를 위해 Advertisement 엔티티가 필요할 듯해, 작성해주신대로 해당 엔티티 추가하여 MetricFact와 join하는 것 좋다고 생각합니다. 또한 mock 데이터만 사용하는 지금으로서는 join으로 인한 성능 저하는 그렇게까지 클 것 같지는 않아서, 일단 작성해두신 대로 status 필드는 주석 처리하여 작업 진행하면 좋을 것 같아요!
추후 2차 MVP에서 실시간 지표 도입하면서 이 부분 다같이 고려해보면 될 듯합니다!
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.
📌 관련 이슈
🚀 개요
관련 피그마 화면
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
새로운 기능
오류 처리