Repository navigation
๐ซ feat: ํ ์คํธ ๊ธฐ๊ฐ ํฌ์ธํธ ์ง๊ธ ์ํ ์ ์ฑ ์ ์ฉ - #118
Conversation
|
No actionable comments were generated in the recent review. ๐ โน๏ธ Recent review infoโ๏ธ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ๐ Files selected for processing (5)
๐ง Files skipped from review as they are similar to previous changes (5)
๐ WalkthroughWalkthroughPoint awards are centralized in ChangesPoint award policy
Asset and gacha award integration
Soup reward integration
Service wiring and award tests
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RewardFlow
participant PointAwardService
participant PointHistoryRepository
participant UserRepository
RewardFlow->>PointAwardService: request point award
PointAwardService->>UserRepository: lock and load user
PointAwardService->>PointHistoryRepository: read cumulative earned amount
PointAwardService->>PointHistoryRepository: persist earning history
PointAwardService-->>RewardFlow: return actual granted amount
Possibly related PRs
๐ฅ Pre-merge checks | โ 4 | โ 1โ Failed checks (1 warning)
โ Passed checks (4 passed)
โจ Finishing Touches๐ Generate docstrings
๐งช Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
๐งน Nitpick comments (2)
src/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt (1)
47-56: ๐๏ธ Data Integrity & Integration | ๐ต Trivial | โก Quick winAssert the capped amount in persisted history.
verify(pointHistoryRepository).save(any())would still pass if the user received 100 but history recorded the requested 300. Capture the saved history and assert that its awarded amount is 100, since this value drives subsequent cumulative-cap calculations.๐ค 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/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt` around lines 47 - 56, Update the award test around PointAwardService.award to capture the PointHistory passed to pointHistoryRepository.save instead of verifying with any(). Assert the captured historyโs awarded amount is 100, while retaining the existing assertions for grantedAmount and user.point.src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt (1)
44-44: ๐๏ธ Data Integrity & Integration | ๐ต Trivial | ๐๏ธ Heavy liftExercise capped awards through every downstream flow.
All three fixtures construct
PointAwardServicewith an unset cap, so the standalone policy tests pass even if downstream services mishandle partial or exhausted grants.
src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt#L44-L44,L55-L55: add capped Gacha assertions for response, balance, and history.src/test/kotlin/com/zerost/api/soup/application/SoupBrewingServiceTest.kt#L65-L65,L74-L74: add capped soup assertions for response/base/bonus allocation, balance, and history.src/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt#L62-L62,L71-L71: add capped reroll assertions for response, balance, and history.๐ค 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/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt` at line 44, The point-award fixtures use an unset cap, so downstream flows are not tested against partial or exhausted grants. In src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt:44-44, update the PointAwardService fixture and add capped assertions covering response, balance, and history; in src/test/kotlin/com/zerost/api/soup/application/SoupBrewingServiceTest.kt:65-65, add capped assertions for response, base/bonus allocation, balance, and history; in src/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt:62-62, add capped assertions for response, balance, and history, using the existing service test structures and ensuring partial and exhausted grants are exercised.
๐ค 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/resources/application.yaml`:
- Around line 32-34: Remove the max-cumulative-earn-amount-per-user entry from
the point.policy configuration in application.yaml, relying on the existing
environment-variable binding and Kotlin default null value instead.
---
Nitpick comments:
In
`@src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt`:
- Line 44: The point-award fixtures use an unset cap, so downstream flows are
not tested against partial or exhausted grants. In
src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt:44-44,
update the PointAwardService fixture and add capped assertions covering
response, balance, and history; in
src/test/kotlin/com/zerost/api/soup/application/SoupBrewingServiceTest.kt:65-65,
add capped assertions for response, base/bonus allocation, balance, and history;
in
src/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt:62-62,
add capped assertions for response, balance, and history, using the existing
service test structures and ensuring partial and exhausted grants are exercised.
In `@src/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt`:
- Around line 47-56: Update the award test around PointAwardService.award to
capture the PointHistory passed to pointHistoryRepository.save instead of
verifying with any(). Assert the captured historyโs awarded amount is 100, while
retaining the existing assertions for grantedAmount and user.point.
๐ช 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7bc164a-960b-4a58-8a1e-3268d849b926
๐ Files selected for processing (12)
src/main/kotlin/com/zerost/api/admin/asset/application/AdminAssetGrantService.ktsrc/main/kotlin/com/zerost/api/gacha/application/GachaExecutionService.ktsrc/main/kotlin/com/zerost/api/point/application/PointAwardService.ktsrc/main/kotlin/com/zerost/api/point/application/PointPolicyConfig.ktsrc/main/kotlin/com/zerost/api/point/application/PointPolicyProperties.ktsrc/main/kotlin/com/zerost/api/point/domain/PointHistoryRepository.ktsrc/main/kotlin/com/zerost/api/soup/application/SoupRewardService.ktsrc/main/resources/application.yamlsrc/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.ktsrc/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.ktsrc/test/kotlin/com/zerost/api/soup/application/SoupBrewingServiceTest.ktsrc/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt
41f8278 to
2ed89aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
๐ค 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/kotlin/com/zerost/api/point/application/PointPolicyProperties.kt`:
- Around line 5-7: Validate maxCumulativeEarnAmountPerUser during
PointPolicyProperties configuration binding or bootstrap, accepting null/blank
as unset but rejecting malformed and negative values as configuration errors.
Ensure PointAwardService receives only a valid non-negative integer cap and no
longer relies on lazy parsing or silently clamps invalid negative values.
In `@src/main/kotlin/com/zerost/api/point/domain/PointHistoryRepository.kt`:
- Around line 4-6: Update the aggregate query method in PointHistoryRepository
to return Long, and keep the cap calculation in Long throughout its callers;
convert the final value to Int only at the boundary where an Int is required.
๐ช 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a8545e6a-acc5-4bb4-a611-559c8ab61f66
๐ Files selected for processing (12)
src/main/kotlin/com/zerost/api/admin/asset/application/AdminAssetGrantService.ktsrc/main/kotlin/com/zerost/api/gacha/application/GachaExecutionService.ktsrc/main/kotlin/com/zerost/api/point/application/PointAwardService.ktsrc/main/kotlin/com/zerost/api/point/application/PointPolicyConfig.ktsrc/main/kotlin/com/zerost/api/point/application/PointPolicyProperties.ktsrc/main/kotlin/com/zerost/api/point/domain/PointHistoryRepository.ktsrc/main/kotlin/com/zerost/api/soup/application/SoupRewardService.ktsrc/main/resources/application.yamlsrc/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.ktsrc/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.ktsrc/test/kotlin/com/zerost/api/soup/application/SoupBrewingServiceTest.ktsrc/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt
๐ง Files skipped from review as they are similar to previous changes (8)
- src/main/resources/application.yaml
- src/main/kotlin/com/zerost/api/point/application/PointPolicyConfig.kt
- src/main/kotlin/com/zerost/api/gacha/application/GachaExecutionService.kt
- src/main/kotlin/com/zerost/api/point/application/PointAwardService.kt
- src/test/kotlin/com/zerost/api/soup/application/SoupRerollServiceTest.kt
- src/test/kotlin/com/zerost/api/gacha/application/GachaExecutionServiceTest.kt
- src/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt
- src/main/kotlin/com/zerost/api/soup/application/SoupRewardService.kt
There was a problem hiding this comment.
Actionable comments posted: 1
๐งน Nitpick comments (1)
src/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt (1)
34-36: ๐๏ธ Data Integrity & Integration | ๐ต Trivial | โก Quick winAssert the persisted history contents, not only the save call.
Capture the saved
PointHistoryand verify its amount matches the granted amount (500and100) and thatsourceType/sourceIdare preserved. The cumulative cap relies on this history, sosave(any())would miss an incorrect persisted award.Also applies to: 55-57
๐ค 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/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt` around lines 34 - 36, Update the PointAwardServiceTest assertions around pointHistoryRepository.save in both affected cases to capture the persisted PointHistory argument and verify its amount matches the granted award (500 and 100), while also asserting sourceType and sourceId are preserved. Replace the save(any())-only verification with content assertions.
๐ค 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/kotlin/com/zerost/api/point/application/PointAwardService.kt`:
- Around line 53-58: Update the award flow in PointAwardService so calculating
currentEarnedAmount, enforcing remainingAmount, and persisting the balance and
earning history execute atomically per user. Use the repository/database
transaction with row locking or an equivalent conditional update, and ensure
concurrent awards for the same user cannot exceed
maxCumulativeEarnAmountPerUser.
---
Nitpick comments:
In `@src/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt`:
- Around line 34-36: Update the PointAwardServiceTest assertions around
pointHistoryRepository.save in both affected cases to capture the persisted
PointHistory argument and verify its amount matches the granted award (500 and
100), while also asserting sourceType and sourceId are preserved. Replace the
save(any())-only verification with content assertions.
๐ช 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73aaaf2e-c94f-4e79-b0d3-23891767f758
๐ Files selected for processing (4)
src/main/kotlin/com/zerost/api/point/application/PointAwardService.ktsrc/main/kotlin/com/zerost/api/point/application/PointPolicyProperties.ktsrc/main/kotlin/com/zerost/api/point/domain/PointHistoryRepository.ktsrc/test/kotlin/com/zerost/api/point/application/PointAwardServiceTest.kt
๐ง Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/com/zerost/api/point/domain/PointHistoryRepository.kt
#๏ธโฃ Issue Number
๐ ์์ฝ(Summary)
๐ ๋ฆฌ๋ทฐ ์์ฒญ์ฌํญ
APP_POINT_POLICY_MAX_CUMULATIVE_EARN_AMOUNT_PER_USER์ค์ ์ด ์์ ๋๋ ๊ธฐ์กด ๋์๊ณผ ๋์ผํ๊ฒ ์ ํ ์์ด ์ง๊ธ๋๋ ๊ตฌ์กฐ์ ๋๋ค.๐ป ํ ์คํธ ๊ฒฐ๊ณผ
./gradlew -q compileKotlinํต๊ณผ./gradlew test --tests 'com.zerost.api.point.application.PointAwardServiceTest' --tests 'com.zerost.api.gacha.application.GachaExecutionServiceTest' --tests 'com.zerost.api.soup.application.SoupBrewingServiceTest' --tests 'com.zerost.api.soup.application.SoupRerollServiceTest'ํต๊ณผ๐ ํฌ์ธํธ
โ๏ธ ํ๊ณ
Summary by CodeRabbit
New Features
app.point.policy.max-cumulative-earn-amount-per-user(environment-configurable).Bug Fixes
Tests