Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.BudgetFieldType;
import com.whereyouad.WhereYouAd.domains.advertisement.domain.constant.Provider;
import com.whereyouad.WhereYouAd.domains.timeline.domain.constant.ComparisonPeriodType;
import com.whereyouad.WhereYouAd.domains.timeline.domain.constant.MetricType;
import com.whereyouad.WhereYouAd.domains.timeline.domain.constant.PerformanceStatus;

Expand Down Expand Up @@ -38,6 +39,7 @@ public record TimelineDetailDTO(
LocalDate startDate,
LocalDate endDate,
PerformanceStatus performanceStatus,
ComparisonPeriodType comparisonPeriodType,
List<MetricType> metrics,
String summary,
List<DailyMetricDTO> dailyTrend,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public static Timeline toTimeline(
.useRoas(dto.metrics().contains(MetricType.ROAS))
.comparisonStartDate(comparisonStartDate)
.comparisonEndDate(comparisonEndDate)
.comparisonPeriodType(dto.comparisonPeriodType())
.createdBy(userId)
.organization(organization)
.build();
Expand Down Expand Up @@ -67,6 +68,7 @@ public static TimelineResponse.TimelineDetailDTO toTimelineDetailDTO(
timeline.getStartDate(),
timeline.getEndDate(),
timeline.getPerformanceStatus(),
timeline.getComparisonPeriodType(),
metrics,
timeline.getSummary(),
dailyTrend,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ public TimelineResponse.CreateResponseDTO updateTimeline(Long userId, Long orgId
// 9. 엔티티 업데이트
timeline.update(dto.name(), dto.startDate(), dto.endDate(),
useClick, useConversion, useImpression, useRoas,
comparisonDates.start(), comparisonDates.end());
comparisonDates.start(), comparisonDates.end(),
dto.comparisonPeriodType());
// AI 요약도 초기화
timeline.updateSummary(null);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.whereyouad.WhereYouAd.domains.timeline.persistence.entity;

import com.whereyouad.WhereYouAd.domains.organization.persistence.entity.Organization;
import com.whereyouad.WhereYouAd.domains.timeline.domain.constant.ComparisonPeriodType;
import com.whereyouad.WhereYouAd.domains.timeline.domain.constant.PerformanceStatus;
import com.whereyouad.WhereYouAd.global.common.BaseEntity;
import jakarta.persistence.*;
Expand Down Expand Up @@ -53,6 +54,10 @@ public class Timeline extends BaseEntity {
@Column(name = "comparison_end_date", nullable = false)
private LocalDate comparisonEndDate;

@Enumerated(EnumType.STRING)
@Column(name = "comparison_period_type", nullable = false)
private ComparisonPeriodType comparisonPeriodType;

Comment on lines +57 to +60

@coderabbitai coderabbitai Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# comparison_period_type 관련 DB 마이그레이션 스크립트 존재 여부 확인
fd -e sql -e yml -e yaml . | xargs rg -l 'comparison_period_type' 2>/dev/null
fd -t d migration

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 167


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[1] timeline entity and references\n'
rg -n "comparison_period_type|comparisonPeriodType" src/main/java src/main/resources db . 2>/dev/null || true

printf '\n[2] locate migration directories/files\n'
fd -t d migration .
fd -t f . src/main/resources db . | sed -n '1,200p'

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 23325


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== tracked migration/config files ==="
git ls-files | rg '(^|/)(migration|migrations|db/migration|flyway|liquibase|V[0-9].*\.sql|.*\.sql|.*\.yml|.*\.yaml)$' || true

echo
echo "=== comparison_period_type occurrences ==="
rg -n "comparison_period_type|comparisonPeriodType" --glob '!target/**' --glob '!build/**' . || true

echo
echo "=== Timeline.java context ==="
sed -n '1,140p' src/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/entity/Timeline.java

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 5574


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== top-level tracked paths that look like migrations ==="
git ls-files | rg '(^|/)(db/migration|migration|migrations|flyway|liquibase)(/|$)|(^|/)V[0-9].*\.sql$|(^|/)R__.*\.sql$' || true

echo
echo "=== files mentioning timeline table or comparison period ==="
git ls-files | while read -r f; do
  case "$f" in
    *.java|*.sql|*.yml|*.yaml|*.xml|*.properties) rg -n "timeline|comparison_period_type|comparisonPeriodType" "$f" && echo "--- $f" ;;
  esac
done

echo
echo "=== Timeline.java excerpt ==="
nl -ba src/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/entity/Timeline.java | sed -n '45,75p'

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 18748


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== application.yml ==="
nl -ba src/main/resources/application.yml | sed -n '1,220p'

echo
echo "=== build.gradle migration-related deps ==="
rg -n "flyway|liquibase|ddl-auto|hibernate\.hbm2ddl|schema" build.gradle src/main/resources/application.yml .github/workflows -n || true

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 231


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== application.yml ==="
sed -n '1,220p' src/main/resources/application.yml

echo
echo "=== build.gradle migration-related deps/config ==="
rg -n "flyway|liquibase|ddl-auto|hibernate\.hbm2ddl|schema|jakarta\.persistence|org\.flywaydb|liquibase" build.gradle src/main/resources/application.yml .github/workflows || true

Repository: WhereYouAd/WhereYouAd-Backend

Length of output: 5500


Timeline.java:58-59 — comparison_period_type는 백필 후 NOT NULL로 전환해야 합니다.
spring.jpa.hibernate.ddl-auto: update만으로 스키마를 맞추는 구조라서, 기존 timeline 데이터가 있는 상태에서 이 컬럼을 바로 nullable = false로 추가하면 반영 단계에서 실패할 수 있습니다. DEFAULT를 두거나, 먼저 nullable로 추가한 뒤 기존 row를 채우고 NOT NULL로 올려 주세요.

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

In
`@src/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/entity/Timeline.java`
around lines 57 - 60, The new comparison_period_type column in Timeline should
not be introduced as NOT NULL immediately because existing timeline rows may
cause schema update failures. Update the Timeline entity and migration flow so
comparisonPeriodType is added with a temporary default or nullable state first,
backfill existing data, and only then enforce NOT NULL; use the Timeline and
comparisonPeriodType symbols to locate the field and related schema handling.

@kingmingyu kingmingyu Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@coderabbitai 배포 DB에 타임라인 데이터가 없는 경우는 괜찮아?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The PR description image shows an API documentation panel with Code: 200 and Details: Response body. It displays a JSON response with a dark-themed code block:

{
  "status": "OK",
  "data": {
    "timelineId": 1,
    "name": "지난주 광고 성과 요약",
    "startDate": "2026-05-13",
    "endDate": "2026-05-13",
    "performanceStatus": null,
    "comparisonPeriodType": "LAST_MONTH",
    "metrics": ["CLICK"],
    "summary": null,
    "dailyTrend": [
      {
        "date": "2026-05-13",
        "clicks": 0,
        "conversions": null,
        "impressions": null,
        "roas": null
      }
    ],
    "platformContributions": [],
    "budgetHistories": []
  }
}

There's a "Copy" icon and a "Download" button in the bottom-right of the panel.

@kingmingyu 네, 배포 DB의 timeline 테이블에 기존 row가 하나도 없다면 문제없습니다.

이유:
spring.jpa.hibernate.ddl-auto: update는 새 컬럼을 추가할 때 대략 아래와 같은 DDL을 생성합니다.

ALTER TABLE timeline ADD COLUMN comparison_period_type VARCHAR(255) NOT NULL;

NOT NULL 제약이 문제가 되는 이유는 기존 row에는 새 컬럼 값이 없어서 ALTER 시점에 제약 조건을 위반하기 때문입니다. 하지만 테이블에 row 자체가 없다면 위반할 데이터가 없으므로 이 ALTER는 그대로 성공합니다.

다만 확인하고 넘어가면 좋은 부분:

  1. 배포 직전에 실제로 row 수를 확인하는 게 안전합니다 (SELECT COUNT(*) FROM timeline;). "괜찮을 것 같다"는 가정보다 실측이 낫습니다.
  2. 운영 DB는 비어있어도, 스테이징/QA 등 다른 환경에 이미 데이터가 쌓여있다면 그 환경에서는 동일한 배포 시 실패할 수 있습니다. 다른 환경도 함께 점검해 주세요.
  3. 지금은 운영 DB가 비어있어 넘어가더라도, 다음에 유사하게 nullable 컬럼을 NOT NULL로 추가할 일이 생기면 이번처럼 "데이터 없음"을 가정하기보다 nullable로 추가 → 백필 → NOT NULL 전환의 패턴을 기본값으로 가져가는 게 안전합니다.

결론적으로 지금 배포 시점에 운영 DB의 timeline 테이블이 비어 있는 게 확인된다면, 현재 코드 그대로 배포해도 괜찮습니다. 👍


🧠 Learnings used
Learnt from: ojy0903
Repo: WhereYouAd/WhereYouAd-Backend PR: 163
File: src/main/java/com/whereyouad/WhereYouAd/domains/notification/domain/service/NotificationServiceImpl.java:160-201
Timestamp: 2026-07-04T07:01:43.103Z
Learning: For Spring-managed services, remember that a class-level `Transactional` applies to all methods unless overridden at the method level. Removing a method-level `Transactional(readOnly = true)` does not eliminate the transaction boundary—it will fall back to the class-level default (typically a non-readOnly transaction). If you need a specific method to run with *no* active transaction (e.g., to avoid holding a DB connection during synchronous outbound HTTP calls such as Slack/Discord webhook dispatch), explicitly annotate that method with `Transactional(propagation = Propagation.NOT_SUPPORTED)` to suspend any active transaction and override the class-level `Transactional`.

@Enumerated(EnumType.STRING)
@Column(name = "performance_status")
private PerformanceStatus performanceStatus;
Expand Down Expand Up @@ -84,7 +89,8 @@ public void update(String name,
boolean useImpression,
boolean useRoas,
LocalDate comparisonStartDate,
LocalDate comparisonEndDate
LocalDate comparisonEndDate,
ComparisonPeriodType comparisonPeriodType
) {
this.name = name;
this.startDate = startDate;
Expand All @@ -95,5 +101,6 @@ public void update(String name,
this.useRoas = useRoas;
this.comparisonStartDate = comparisonStartDate;
this.comparisonEndDate = comparisonEndDate;
this.comparisonPeriodType = comparisonPeriodType;
}
}
Loading