Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChanged KeywordService.createNotifiedArticleStatus to run in a new transaction (Propagation.REQUIRES_NEW). Added a unit test that reflects on the method to assert the transactional propagation. Updated repository upsert SQL to set created_at/updated_at timestamps on insert and update. Changes
Sequence Diagram(s)sequenceDiagram
participant Listener as EventListener (AFTER_COMMIT)
participant Service as KeywordService.createNotifiedArticleStatus
participant TX as TransactionManager
participant Repo as UserNotificationStatusRepository
participant DB as Database
Listener->>Service: invoke createNotifiedArticleStatus()
Note right of Service: Method annotated with\n@Transactional(propagation=REQUIRES_NEW)
Service->>TX: begin new transaction
TX->>Repo: call upsertLastNotifiedArticleId(...)
Repo->>DB: execute upsert (insert timestamps / on duplicate update updated_at)
DB-->>Repo: result
Repo-->>TX: return
TX->>TX: commit
TX-->>Service: transaction committed
Service-->>Listener: return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
🧹 Nitpick comments (1)
src/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java (1)
115-123: Good annotation guard; consider one runtime transaction-behavior test too.This reflection test protects config drift, but it won’t catch cases where transactional advice is not applied at runtime. A small integration test (outer rollback + inner
createNotifiedArticleStatuscommit assertion) would verify actualREQUIRES_NEWbehavior end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java` around lines 115 - 123, Add a runtime integration test in KeywordServiceTest that verifies createNotifiedArticleStatus actually runs in a new transaction: write a test method that starts an outer `@Transactional` test transaction, call KeywordService.createNotifiedArticleStatus(...) to persist a notified-article record, then force the outer transaction to roll back (e.g., by throwing or marking rollback) and finally query the repository to assert the notified-article persisted (committed) despite the outer rollback; reference KeywordService.createNotifiedArticleStatus and the repository used to read persisted status to locate where to add this end-to-end REQUIRES_NEW behavior check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java`:
- Around line 115-123: Add a runtime integration test in KeywordServiceTest that
verifies createNotifiedArticleStatus actually runs in a new transaction: write a
test method that starts an outer `@Transactional` test transaction, call
KeywordService.createNotifiedArticleStatus(...) to persist a notified-article
record, then force the outer transaction to roll back (e.g., by throwing or
marking rollback) and finally query the repository to assert the
notified-article persisted (committed) despite the outer rollback; reference
KeywordService.createNotifiedArticleStatus and the repository used to read
persisted status to locate where to add this end-to-end REQUIRES_NEW behavior
check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e765b18b-0ed1-436f-8404-c0ff7469081c
📒 Files selected for processing (2)
src/main/java/in/koreatech/koin/domain/community/keyword/service/KeywordService.javasrc/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java
skdud0629
approved these changes
Mar 18, 2026
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
3 participants
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.
🔍 개요
no transaction is in progress오류 발생하던 문제 수정함user_notification_statusnative upsert에서 audit 컬럼 누락으로 insert 실패 가능하던 부분 함께 보완함🚀 주요 변경 내용
REQUIRES_NEW)으로 분리함user_notification_statusupsert 쿼리에created_at,updated_at반영함💬 참고 사항
✅ Checklist (완료 조건)
Summary by CodeRabbit
Bug Fixes
Data
Tests
Note: These are internal improvements with no direct user-facing UI changes.