-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 아티클(가게에 담긴 이야기) 제거 #153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough아티클 관련 기능(컨트롤러, DTO, 서비스, 리포지토리, 엔티티), 관련 테스트·픽스처, DB 마이그레이션 및 시드에서 article 테이블과 시드가 제거되었고, story.description 컬럼 제약이 NULL 허용으로 변경되며 중복 마이그레이션 파일이 정리되었습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
📌 최신 ERD가 자동 생성되었습니다. 👉 ERD 보러가기 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/resources/db/migration/V3__add_story_table.sql (1)
10-10: NULL 허용 변경에 따른 도메인/검증 영향 점검 필요description이 NULL 가능해지면:
- 엔티티 매핑(@column(nullable = true) 또는 기본값)과 DTO/요청 검증(@notblank → @SiZe 등)의 정합성 확인
- 서비스 로직에서 NULL vs 빈 문자열("")의 의미를 명확히 하고 일관성 있게 처리
- 검색/정렬/인덱싱 영향(예: IS NULL 조건) 및 통계 집계 시 누락 여부 점검
- 시드/테스트 데이터에서 NOT NULL 가정 제거
필요 시, “빈 문자열은 금지하고 NULL만 허용” 같은 규칙을 어플리케이션 레벨에서 강제하세요. 원한다면 관련 테스트 보강과 검증 어노테이션/로직 패치를 제안드리겠습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/main/java/eatda/controller/article/ArticleController.java(0 hunks)src/main/java/eatda/controller/article/ArticleResponse.java(0 hunks)src/main/java/eatda/controller/article/ArticlesResponse.java(0 hunks)src/main/java/eatda/domain/article/Article.java(0 hunks)src/main/java/eatda/repository/article/ArticleRepository.java(0 hunks)src/main/java/eatda/service/article/ArticleService.java(0 hunks)src/main/resources/db/migration/V1__init.sql(0 hunks)src/main/resources/db/migration/V3__add_story_table.sql(1 hunks)src/main/resources/db/migration/V5__story_description_available_null.sql(0 hunks)src/main/resources/db/seed/dev/V2__dev_init_data.sql(0 hunks)src/main/resources/db/seed/local/V2__local_init_data.sql(0 hunks)src/test/java/eatda/controller/BaseControllerTest.java(0 hunks)src/test/java/eatda/controller/article/ArticleControllerTest.java(0 hunks)src/test/java/eatda/document/BaseDocumentTest.java(0 hunks)src/test/java/eatda/document/article/ArticleDocumentTest.java(0 hunks)src/test/java/eatda/fixture/ArticleGenerator.java(0 hunks)src/test/java/eatda/service/BaseServiceTest.java(0 hunks)src/test/java/eatda/service/article/ArticleServiceTest.java(0 hunks)
💤 Files with no reviewable changes (17)
- src/main/java/eatda/controller/article/ArticleResponse.java
- src/test/java/eatda/controller/article/ArticleControllerTest.java
- src/main/resources/db/seed/local/V2__local_init_data.sql
- src/main/resources/db/seed/dev/V2__dev_init_data.sql
- src/test/java/eatda/service/BaseServiceTest.java
- src/main/resources/db/migration/V1__init.sql
- src/main/java/eatda/controller/article/ArticlesResponse.java
- src/main/java/eatda/repository/article/ArticleRepository.java
- src/test/java/eatda/document/BaseDocumentTest.java
- src/main/java/eatda/domain/article/Article.java
- src/main/java/eatda/service/article/ArticleService.java
- src/test/java/eatda/document/article/ArticleDocumentTest.java
- src/test/java/eatda/service/article/ArticleServiceTest.java
- src/main/java/eatda/controller/article/ArticleController.java
- src/test/java/eatda/fixture/ArticleGenerator.java
- src/main/resources/db/migration/V5__story_description_available_null.sql
- src/test/java/eatda/controller/BaseControllerTest.java
| `store_lot_number_address` VARCHAR(255) NOT NULL, | ||
| `store_category` VARCHAR(50) NOT NULL, | ||
| `description` TEXT NOT NULL, | ||
| `description` TEXT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Flyway 기존 마이그레이션(V3) 파일 수정은 체크섬 불일치/이행 실패를 유발할 수 있습니다
이미 로컬/CI 등에서 V3가 적용된 상태라면, V3 내용 변경으로 Flyway 체크섬 불일치가 발생합니다. 또한 V5(ALTER) 파일을 삭제한 상태이므로, DB를 초기화하지 않으면 마이그레이션 경로가 깨질 수 있습니다. PR 설명대로 전 환경에서 DB 초기화를 확실히 수행한다면 가능하지만, 히스토리 재작성은 팀/CI 동기화가 실패할 때 리스크가 큽니다.
권장: V3는 원본대로 유지하고, 새로운 마이그레이션(V6 등)으로 NULL 허용 변경을 적용하세요. 이렇게 하면 초기화가 불필요하고, 안전하게 점진적 이행이 가능합니다.
옵션 A(권장) — V3 원복 + 신규 V6 추가
- V3 원복(diff 제안):
- `description` TEXT NULL,
+ `description` TEXT NOT NULL,- 신규 파일 예: src/main/resources/db/migration/V6__story_description_nullable.sql
ALTER TABLE `story`
MODIFY COLUMN `description` TEXT NULL;옵션 B(현 상태 유지) — 전 환경 DB 초기화 강제
- 모든 로컬/CI/스테이징 환경에서 flyway clean 또는 DB drop 후 재생성 필요
- 머지/배포 체크리스트에 “DB 초기화 완료”를 명시하고, 실패 시 롤백 플랜 포함
검증 스크립트(레포 루트에서 실행): 현 레포 상태와 Flyway 설정을 빠르게 점검합니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Migration files =="
fd -a 'V*_*.sql' src/main/resources/db/migration || true
echo
echo "== Check if V5 (story_description_available_null) exists =="
fd -a 'V5__*story*' src/main/resources/db/migration || true
echo
echo "== Flyway config candidates =="
fd -a 'application*.yml' || true
fd -a 'application*.properties' || true
echo
echo "== Inspect Flyway flags (cleanDisabled/baselineOnMigrate/locations) =="
rg -n 'flyway\.(cleanDisabled|baselineOnMigrate|locations)' || trueLength of output: 1354
Flyway 마이그레이션 히스토리 재작성 위험 — V3 복원 후 신규 마이그레이션 생성 권장
현재 src/main/resources/db/migration 에는 V1, V3만 존재하며, V5 파일은 삭제된 상태입니다. 또한 application.yml에 별도 Flyway 설정(cleanDisabled, baselineOnMigrate 등)이 없어 기본 동작으로 마이그레이션을 수행하므로, V3 마이그레이션을 직접 수정할 경우 이미 해당 스크립트가 적용된 환경에서 체크섬 불일치로 마이그레이션 실패가 발생할 수 있습니다.
권장 해결안:
- V3 마이그레이션 원본으로 복원
파일:src/main/resources/db/migration/V3__add_story_table.sql(10행)@@ -7,7 +7,7 @@ CREATE TABLE `story` ( - `description` TEXT NULL, + `description` TEXT NOT NULL,
- 새로운 마이그레이션(V4 이상) 생성
예:src/main/resources/db/migration/V4__story_description_nullable.sqlALTER TABLE `story` MODIFY COLUMN `description` TEXT NULL;
대안(위험도 높음):
모든 로컬/CI/스테이징 환경에 대해 flyway clean 또는 DB drop → 재생성을 반드시 수행하도록 강제하고, “DB 초기화 완료”를 머지/배포 체크리스트에 명시해야 합니다.
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V3__add_story_table.sql around line 10, the
migration has been edited to change `description` to TEXT NULL which risks
Flyway checksum mismatches in environments where V3 already ran; restore V3 to
its original committed content, then create a new migration file (e.g.,
src/main/resources/db/migration/V4__story_description_nullable.sql) containing
an ALTER TABLE statement to MODIFY COLUMN `description` TEXT NULL so the change
is applied as a new migration; do not modify V3 in-place unless you will enforce
a full Flyway clean/database reset across all environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- PR 머지하고 배포하기 전에 DB 테이블을 초기화할 예정
- 만약 팀원이 DB 초기화를 반대할 경우, 해당 부분 롤백하고 다시 SQL 문을 추가할 예정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leegwichan 네, 이해했습니다. DB 초기화 계획과 롤백 대안을 준비하신 점 확인했습니다.
현재 접근 방식으로 진행하실 경우 체크리스트:
- 머지 전 팀원들과 DB 초기화 일정 사전 공유
- 로컬/CI 환경에서
flyway clean또는 스키마 재생성 필요 - 초기화 시점에
flyway_schema_history테이블도 함께 제거 확인
팀원 반대 시 롤백 플랜도 명확하니 안전하게 진행 가능할 것 같습니다.
✏️ Learnings added
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#153
File: src/main/resources/db/migration/V3__add_story_table.sql:10-10
Timestamp: 2025-08-11T07:48:37.790Z
Learning: leegwichan 팀은 프로덕션 운영 시작 전 환경에서는 Flyway 마이그레이션 파일을 직접 수정하는 것을 허용하며, DB 초기화를 통해 해결하는 방식을 선호합니다. 다만 팀원 합의가 필요한 경우 롤백하고 새 마이그레이션 파일을 추가하는 대안도 고려합니다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
lvalentine6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 PR도 고생하셨습니다! 🎉
뭔가 기능들이 제거되는게 가슴아프네요..
초기화 진행하셔도 될것 같아요!
# Conflicts: # src/main/resources/db/migration/V5__story_description_available_null.sql
|
📌 최신 ERD가 자동 생성되었습니다. 👉 ERD 보러가기 |
|
📌 최신 ERD가 자동 생성되었습니다. 👉 ERD 보러가기 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/eatda/domain/story/Story.java (2)
156-170: 주소 파싱 split(" ")는 연속 공백/탭에서 빈 토큰이 생길 수 있습니다 — 공백 정규식으로 견고화 제안연속 공백이나 탭이 포함된 주소에서 현재 로직은 빈 문자열을 반환할 수 있습니다. 정규식 분리로 방어하면 안정적입니다.
아래와 같이 변경을 제안드립니다.
- String[] addressParts = storeLotNumberAddress.split(" "); + String[] addressParts = storeLotNumberAddress.trim().split("\\s+"); @@ - String[] addressParts = storeLotNumberAddress.split(" "); + String[] addressParts = storeLotNumberAddress.trim().split("\\s+");
126-130: storeRoadAddress는 null만 검증합니다 — blank 허용 의도인지 확인 필요다른 필드(storeName, storeLotNumberAddress 등)는 isBlank()까지 금지하는데, roadAddress는 null만 체크합니다. 빈 문자열이 도메인 규칙상 허용인지 확인 부탁드립니다. 불허라면 아래처럼 일관화하는 편이 좋습니다.
- private void validateStoreRoadAddress(String roadAddress) { - if (roadAddress == null) { + private void validateStoreRoadAddress(String roadAddress) { + if (roadAddress == null || roadAddress.isBlank()) { throw new BusinessException(BusinessErrorCode.INVALID_STORE_ADDRESS); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/eatda/domain/story/Story.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
src/main/java/eatda/domain/story/Story.java (1)
57-58: description 컬럼 NULL 허용 변경 검증 완료
- Flyway 마이그레이션(V3__add_story_table.sql)에서
description TEXT NULL로 일관되게 설정되어 있습니다. story 테이블에만 적용되며, cheer 등 다른 테이블 스크립트엔 영향이 없습니다.- 애플리케이션 설정에서
spring.jpa.hibernate.ddl-auto가 명시되지 않아 기본(validate/none)이 적용됩니다. 프로덕션 환경에선validate모드를 사용해 스키마 불일치 방지를 권장드립니다.- 코드 내
getDescription()호출부를 확인한 결과,.trim(),.isBlank()등 메서드 체이닝이 없고 바로 값 전달만 이루어지므로 NPE 우려가 없습니다. 단위 테스트에서도 null 허용 시나리오가 정상 작동함을 검증하고 있습니다.LGTM 👍
|
🎉 This PR is included in version 1.4.0-develop.80 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.7.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



✨ 개요
🧾 관련 이슈
closed #150
🔍 참고 사항 (선택)
Summary by CodeRabbit