Fix/#168 타임라인 상세조회 - comparisonPeriodType 추가#169
Merged
Conversation
1 task
WalkthroughTimeline 엔티티에 comparisonPeriodType 필드를 추가하고, EnumType.STRING으로 DB 컬럼에 매핑했습니다. update 메서드 시그니처를 확장하고, TimelineConverter와 TimelineServiceImpl, TimelineResponse의 TimelineDetailDTO에 comparisonPeriodType 값을 전파하여 생성 및 상세 조회 응답에서 노출되도록 변경했습니다. ChangescomparisonPeriodType 전파
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant TimelineController
participant TimelineServiceImpl
participant Timeline
participant TimelineConverter
Client->>TimelineController: 타임라인 상세 조회 요청
TimelineController->>TimelineServiceImpl: getTimelineDetail(...)
TimelineServiceImpl->>Timeline: getComparisonPeriodType()
Timeline-->>TimelineServiceImpl: comparisonPeriodType 값 반환
TimelineServiceImpl->>TimelineConverter: toTimelineDetailDTO(timeline)
TimelineConverter-->>TimelineServiceImpl: TimelineDetailDTO(comparisonPeriodType 포함)
TimelineServiceImpl-->>TimelineController: TimelineDetailDTO
TimelineController-->>Client: 응답(comparisonPeriodType 포함)
Possibly related PRs
Suggested reviewers: 시니어 개발자 입장에서 리뷰해보면, 이번 PR은 딱 필요한 부분만 깔끔하게 건드린 좋은 예시예요! 👍
전반적으로 스코프가 명확하고 변경 범위가 작아서 리뷰 부담이 크지 않은 PR입니다. 🎉 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/timeline/application/dto/response/TimelineResponse.java (1)
16-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
CreateResponseDTO에도comparisonPeriodType을 포함하는 게 어떨까요?
TimelineDetailDTO에는comparisonPeriodType이 잘 추가됐는데,CreateResponseDTO(Line 16-26)는comparisonStartDate/comparisonEndDate는 갖고 있으면서 정작 그 기준이 되는comparisonPeriodType은 빠져있어요. 생성/수정 응답에서 클라이언트가 "어떤 기준으로 비교 기간이 계산됐는지" 바로 알 수 없는 비일관성이 생깁니다. 이번 이슈(#168) 범위는 상세 조회에 한정되어 있어 필수 수정은 아니지만, 응답 일관성 측면에서 함께 추가하면 좋을 것 같습니다.💡 제안 diff
public record CreateResponseDTO( Long timelineId, String name, LocalDate startDate, LocalDate endDate, List<MetricType> metrics, LocalDate comparisonStartDate, LocalDate comparisonEndDate, + ComparisonPeriodType comparisonPeriodType, PerformanceStatus performanceStatus, LocalDateTime createdAt ) {}Also applies to: 42-42
🤖 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/application/dto/response/TimelineResponse.java` around lines 16 - 26, `CreateResponseDTO` is missing `comparisonPeriodType`, which makes the create/update response inconsistent with `TimelineDetailDTO` even though it already returns the derived comparison dates. Update the `CreateResponseDTO` record in `TimelineResponse` to include `comparisonPeriodType`, and make sure the mapping code that builds this DTO populates it from the timeline entity or request-derived comparison settings so clients can see the basis for the comparison period.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/entity/Timeline.java`:
- Around line 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.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/timeline/application/dto/response/TimelineResponse.java`:
- Around line 16-26: `CreateResponseDTO` is missing `comparisonPeriodType`,
which makes the create/update response inconsistent with `TimelineDetailDTO`
even though it already returns the derived comparison dates. Update the
`CreateResponseDTO` record in `TimelineResponse` to include
`comparisonPeriodType`, and make sure the mapping code that builds this DTO
populates it from the timeline entity or request-derived comparison settings so
clients can see the basis for the comparison period.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7df0bd07-08ab-49bc-96d3-02773de981a5
📒 Files selected for processing (4)
src/main/java/com/whereyouad/WhereYouAd/domains/timeline/application/dto/response/TimelineResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/timeline/application/mapper/TimelineConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/timeline/domain/service/TimelineServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/entity/Timeline.java
Comment on lines
+57
to
+60
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "comparison_period_type", nullable = false) | ||
| private ComparisonPeriodType comparisonPeriodType; | ||
|
|
There was a problem hiding this comment.
🗄️ 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 migrationRepository: 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.javaRepository: 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 || trueRepository: 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 || trueRepository: 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.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai 배포 DB에 타임라인 데이터가 없는 경우는 괜찮아?
There was a problem hiding this comment.
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는 그대로 성공합니다.
다만 확인하고 넘어가면 좋은 부분:
- 배포 직전에 실제로 row 수를 확인하는 게 안전합니다 (
SELECT COUNT(*) FROM timeline;). "괜찮을 것 같다"는 가정보다 실측이 낫습니다. - 운영 DB는 비어있어도, 스테이징/QA 등 다른 환경에 이미 데이터가 쌓여있다면 그 환경에서는 동일한 배포 시 실패할 수 있습니다. 다른 환경도 함께 점검해 주세요.
- 지금은 운영 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`.
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
1 participant
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
New Features
Bug Fixes