-
Notifications
You must be signed in to change notification settings - Fork 1
[SRLT-129] 가격을 프로모션에 맞춰 수정한다 #78
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
base: develop
Are you sure you want to change the base?
The head ref may contain hidden characters: "SRLT-129-\uAC00\uACA9-\uBC0F-cors-\uC218\uC815"
Conversation
|
Note
|
| 코호트 / 파일(들) | 변경 요약 |
|---|---|
서브모듈 업데이트 .gitmodules |
서브모듈 커밋 참조가 1d8d33f에서 229ee0d로 업데이트됨 (기능적 동작 변화 없음) |
CORS 설정 확장 src/main/java/starlight/bootstrap/SecurityConfig.java |
개발 환경 기반 URL(${cors.origin.develop})을 @Value 주입을 통해 추가하고, CORS 허용 origins 목록에 devBaseUrl 포함 |
상품 가격 조정 src/main/java/starlight/domain/order/enumerate/UsageProductType.java |
AI_REPORT_1, AI_REPORT_2 enum 상수의 가격을 각각 49,000원, 89,000원에서 0원으로 수정 |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
- [SRLT-80] Chore: 서브모듈 지칭 해시를 변경한다 #55: 서브모듈 커밋 참조 업데이트를 포함하고 있어 이번 변경과 연관됨
Suggested labels
✂️ CHORE
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
|
| Title check | PR 제목은 가격 수정에 대해서만 언급하고 있지만, 실제 변경사항은 가격 수정(UsageProductType enum)과 CORS 설정 추가(SecurityConfig)를 포함하고 있습니다. | 제목을 'SRLT-129 가격 수정 및 CORS 개발 주소 추가'와 같이 변경하여 두 주요 변경사항을 모두 반영하도록 권장합니다. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Test Results249 tests 249 ✅ 10s ⏱️ Results for commit e83f14a. ♻️ This comment has been updated with latest results. |
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: 3
🤖 Fix all issues with AI agents
In @config:
- Line 1: SecurityConfig.java currently injects devBaseUrl into CORS
allowedOrigins—verify the config module's properties file to ensure the property
cors.origin.develop contains only development hosts (e.g.,
http://localhost:3000) and does not include any production domain; if it does,
remove the prod domain or separate dev/prod properties and update
SecurityConfig's allowedOrigins accordingly. For UsageProductType.java,
AI_REPORT_1 and AI_REPORT_2 are set to 0L in the new enum/constants—confirm
whether that is intentional (promotion vs permanent free); if not, set the
constants to the intended non-zero price values or add a clear
comment/feature-flag for promotional pricing, and add/adjust validation in the
billing/payment logic (references: UsageProductType, and whatever payment
validation class/method handles product pricing) to correctly handle zero-priced
items. Ensure tests or a simple assertion verify cors.origin.develop value and
that payment logic treats 0-priced products as intended.
In @src/main/java/starlight/bootstrap/SecurityConfig.java:
- Around line 102-106: The CORS origin properties injected for
configuration.setAllowedOrigins (clientBaseUrl, ServerBaseUrl, devBaseUrl) are
missing from the repo and cause startup failures; add an application.yml (or
.properties) defining cors.origin.server, cors.origin.client and
cors.origin.develop, and split into environment-specific files
(application-dev.yml, application-stg.yml, application-prod.yml) so that
devBaseUrl (cors.origin.develop) exists only in the dev profile while production
profiles only define cors.origin.server and cors.origin.client; ensure the
property names match the @Value injections used where
configuration.setAllowedOrigins is called.
In @src/main/java/starlight/domain/order/enumerate/UsageProductType.java:
- Around line 12-13: The AI report enum items AI_REPORT_1/AI_REPORT_2 currently
use 0L prices which breaks Toss PG; update handling so zero-priced products
never get sent to TossClient.confirm — either change the enum prices to a
non-zero promotional price or add a bypass in OrderPaymentService that detects
productPrice == 0 (or UsageProductType for AI_REPORT_*) and creates a completed
free payment record without calling TossClient.confirm (and ensure Money VO and
payment state transitions accept and handle zero-priced free payments
accordingly).
🧹 Nitpick comments (1)
src/main/java/starlight/bootstrap/SecurityConfig.java (1)
42-42: 변수 명명 규칙 통일 권장기존 변수들과 네이밍 컨벤션이 일관되지 않습니다:
ServerBaseUrl(대문자 S 시작)clientBaseUrl(소문자 c 시작)devBaseUrl(소문자 d 시작)Java 컨벤션에 따라 모두 소문자로 시작하도록 통일하는 것을 권장합니다.
♻️ 네이밍 통일 제안
- @Value("${cors.origin.server}") String ServerBaseUrl; + @Value("${cors.origin.server}") String serverBaseUrl; @Value("${cors.origin.client}") String clientBaseUrl; @Value("${cors.origin.develop}") String devBaseUrl;그리고 102-105번 라인에서도 변경:
configuration.setAllowedOrigins(List.of( clientBaseUrl, - ServerBaseUrl, + serverBaseUrl, devBaseUrl ));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
configsrc/main/java/starlight/bootstrap/SecurityConfig.javasrc/main/java/starlight/domain/order/enumerate/UsageProductType.java
⏰ 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
| configuration.setAllowedOrigins(List.of( | ||
| clientBaseUrl, | ||
| ServerBaseUrl | ||
| ServerBaseUrl, | ||
| devBaseUrl | ||
| )); |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the SecurityConfig.java file and examine the relevant code
find . -name "SecurityConfig.java" -type f | head -20Repository: StartUpLight/STARLIGHT_BE
Length of output: 124
🏁 Script executed:
# Find all application configuration files
fd -e yml -e yaml -e properties | grep -i application | head -30Repository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# Search for cors-related properties and devBaseUrl
rg -n 'cors\.origin|devBaseUrl|dev-base-url' --type-list | head -30Repository: StartUpLight/STARLIGHT_BE
Length of output: 458
🏁 Script executed:
# Read the SecurityConfig.java file to understand devBaseUrl usage
cat -n ./src/main/java/starlight/bootstrap/SecurityConfig.java | sed -n '90,120p'Repository: StartUpLight/STARLIGHT_BE
Length of output: 1366
🏁 Script executed:
# Search for application configuration files in src/main/resources
find ./src/main/resources -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" 2>/dev/nullRepository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# Search for cors.origin.develop property across configuration files
rg 'cors\.origin\.develop|devBaseUrl|dev-base-url' -t yaml -t propertiesRepository: StartUpLight/STARLIGHT_BE
Length of output: 103
🏁 Script executed:
# Search for @Value annotation or injection of devBaseUrl
rg -B 2 -A 2 'devBaseUrl' ./src/main/java/starlight/bootstrap/SecurityConfig.javaRepository: StartUpLight/STARLIGHT_BE
Length of output: 385
🏁 Script executed:
# Find all yml and yaml files in the repository
find . -name "*.yml" -o -name "*.yaml" | grep -v node_modules | head -20Repository: StartUpLight/STARLIGHT_BE
Length of output: 210
🏁 Script executed:
# Broader search for cors and origin properties
rg 'cors|origin|devBaseUrl|dev' -t yamlRepository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# List all files in src/main/resources directory
ls -la ./src/main/resources/ 2>/dev/null || echo "No src/main/resources directory found"Repository: StartUpLight/STARLIGHT_BE
Length of output: 290
🏁 Script executed:
# Search for any yml/yaml files with cors or origin properties
rg -l 'cors|origin' --type yaml . 2>/dev/null || rg 'cors|origin' . --include="*.yml" --include="*.yaml" 2>/dev/nullRepository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# Check if application.yml exists anywhere
find . -name "application.yml" -o -name "application-*.yml" -o -name "application-*.yaml" 2>/dev/nullRepository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# Search for all yml files in entire repository
fd '\\.ya?ml$' . | head -30Repository: StartUpLight/STARLIGHT_BE
Length of output: 51
🏁 Script executed:
# Search for any .yml/.yaml files in entire repository
find . -type f \( -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | head -30Repository: StartUpLight/STARLIGHT_BE
Length of output: 210
🏁 Script executed:
# Check deploy directory for configuration files
ls -la ./deploy/ 2>/dev/null || echo "No deploy directory"Repository: StartUpLight/STARLIGHT_BE
Length of output: 308
🏁 Script executed:
# Search for @ConfigurationProperties annotation
rg '@ConfigurationProperties' ./src/main/java --type javaRepository: StartUpLight/STARLIGHT_BE
Length of output: 186
🏁 Script executed:
# Check if properties are defined anywhere in codebase
rg 'cors\.origin\.(server|client|develop)' . --include="*.java" --include="*.yml" --include="*.yaml" --include="*.properties"Repository: StartUpLight/STARLIGHT_BE
Length of output: 510
🏁 Script executed:
# Search for cors.origin properties using correct rg syntax
rg 'cors\.origin\.' -g '*.java' -g '*.yml' -g '*.yaml' -g '*.properties'Repository: StartUpLight/STARLIGHT_BE
Length of output: 534
🏁 Script executed:
# Check docker-compose.yaml for environment variable definitions
cat ./deploy/docker-compose.yamlRepository: StartUpLight/STARLIGHT_BE
Length of output: 1146
🏁 Script executed:
# Search for environment variable declarations for cors
rg -i 'CORS|cors\.origin' ./deploy/Repository: StartUpLight/STARLIGHT_BE
Length of output: 51
CORS 설정 속성 파일 필수 구성 필요 - 애플리케이션 구동 불가
devBaseUrl 외에도 cors.origin.server, cors.origin.client 등 CORS 허용 origin 설정이 @Value 어노테이션으로 주입되고 있으나, 이들 속성을 정의하는 설정 파일이 저장소 어디에도 없습니다. 현재 상태로는 애플리케이션 구동 시 PropertySourceNotFoundException 또는 NullPointerException이 발생합니다.
필수 조치:
-
src/main/resources/application.yml(또는.properties)을 생성하여 최소한 다음 속성들을 정의:cors.origin.servercors.origin.clientcors.origin.develop
-
프로파일 분리:
application-dev.yml,application-stg.yml,application-prod.yml을 작성하여 각 환경에 맞는 URL만 설정. 특히cors.origin.develop은 개발 프로파일에만 포함되어야 하며, 운영 환경에서는 제거해야 합니다. -
예시:
# application.yml (공통) cors: origin: server: "https://api.starlight.com" client: "https://app.starlight.com" # application-dev.yml (개발 환경만) cors: origin: develop: "http://localhost:3000"
이는 보안 문제뿐만 아니라 기능 구현의 기초입니다. StarLight 백엔드가 제대로 구동될 수 없습니다.
🤖 Prompt for AI Agents
In @src/main/java/starlight/bootstrap/SecurityConfig.java around lines 102 -
106, The CORS origin properties injected for configuration.setAllowedOrigins
(clientBaseUrl, ServerBaseUrl, devBaseUrl) are missing from the repo and cause
startup failures; add an application.yml (or .properties) defining
cors.origin.server, cors.origin.client and cors.origin.develop, and split into
environment-specific files (application-dev.yml, application-stg.yml,
application-prod.yml) so that devBaseUrl (cors.origin.develop) exists only in
the dev profile while production profiles only define cors.origin.server and
cors.origin.client; ensure the property names match the @Value injections used
where configuration.setAllowedOrigins is called.
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.
지금 스테이징 서버를 개발에서도 공유하고 있어서 스테이징 서버에서 localhost:3000도 포함시켜줘야해 그래서 이렇게 했어
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.
@SeongHo5356, 스테이징 환경에서 개발 프론트엔드(localhost:3000)를 함께 사용해야 하는 상황은 이해했습니다! 👍
제가 언급한 주요 우려사항은 cors.origin.develop, cors.origin.server, cors.origin.client 속성 값들이 어디서 정의되는지에 관한 것이었습니다.
혹시 다음 중 어떤 방식으로 이 속성들을 관리하고 계신가요?
- 환경 변수로 주입 (docker-compose, k8s configmap 등)
- 외부 설정 서버 또는 서브모듈
- application.yml 파일이 별도로 관리됨 (gitignore 되어 있거나)
현재 방식이 확인되면 제 리뷰 코멘트를 조정하겠습니다!
참고 제안:
스테이징에서 개발 localhost를 허용해야 한다면, 프로파일 기반 설정보다는 환경 변수로 여러 origin을 쉼표로 구분해 주입하는 방식도 고려해볼 수 있습니다:
@Value("${cors.origin.allowed}") String allowedOrigins;
// "https://api.starlight.com,https://app.starlight.com,http://localhost:3000"
configuration.setAllowedOrigins(List.of(allowedOrigins.split(",")));
🚀 Why - 해결하려는 문제가 무엇인가요?
✅ What - 무엇이 변경됐나요?
🛠️ How - 어떻게 해결했나요?
🖼️ Attachment
화면 이미지, 결과 캡처 등 첨부💬 기타 코멘트
리뷰어에게 전하고 싶은 말, 테스트 방법, 주의할 점 등Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.