Skip to content

Dev#281

Merged
ms-0o0 merged 6 commits into
mainfrom
dev
Mar 18, 2026
Merged

Dev#281
ms-0o0 merged 6 commits into
mainfrom
dev

Conversation

@Misu0616
Copy link
Copy Markdown
Contributor

@Misu0616 Misu0616 commented Mar 18, 2026

개요

관련 BackLog

Resolves: (Backlog Number, ...)

PR 유형

  • 새로운 기능 추가
  • 버그 수정
  • CSS 등 사용자 UI 디자인 변경
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
  • 코드 리팩토링
  • 주석 추가 및 수정
  • 문서 수정
  • 테스트 추가, 테스트 리팩토링
  • 빌드 부분 혹은 패키지 매니저 수정
  • 파일 혹은 폴더명 수정
  • 파일 혹은 폴더 삭제

PR Checklist

  • 커밋 메시지 컨벤션에 맞게 작성했습니다.
  • 변경 사항에 대한 테스트를 했습니다.(버그 수정/기능에 대한 테스트).

Summary by CodeRabbit

  • New Features

    • Async notifications now respect database transaction boundaries, ensuring notifications are sent after commit operations complete.
  • Performance

    • Added database index to optimize query performance on family line records.
  • Chores

    • Updated build environment configuration.

@Misu0616 Misu0616 requested a review from ms-0o0 March 18, 2026 05:49
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5e688b84-d498-49f4-9d54-3c5a6901e89d

📥 Commits

Reviewing files that changed from the base of the PR and between 6f943ed and 06a801c.

⛔ Files ignored due to path filters (1)
  • bootrun.log is excluded by !**/*.log
📒 Files selected for processing (8)
  • .gitignore
  • src/main/java/com/pooli/notification/mapper/AlarmHistoryMapper.java
  • src/main/java/com/pooli/notification/service/AlarmHistoryServiceImpl.java
  • src/main/java/com/pooli/policy/service/AdminPolicyServiceImpl.java
  • src/main/resources/db/migration/V2603181135__add_family_line_role_index.sql
  • src/main/resources/mapper/notification/AlarmHistoryMapper.xml
  • src/test/java/com/pooli/notification/service/AlarmHistoryServiceImplTest.java
  • src/test/java/com/pooli/policy/service/AdminPolicyServiceImplTest.java

📝 Walkthrough

Walkthrough

This pull request modifies the notification system to use transaction-aware asynchronous notification delivery, removes DIRECT target handling from AlarmHistoryServiceImpl, adds a database index on the family_line table, and updates related tests and configuration files.

Changes

Cohort / File(s) Summary
Build Configuration
.gitignore
Added .gradle-home/ ignore rule.
Database Migrations
src/main/resources/db/migration/V2603181135__add_family_line_role_index.sql
Created non-unique index on (role, line_id) columns in FAMILY_LINE table.
Notification Service Core
src/main/java/com/pooli/notification/service/AlarmHistoryServiceImpl.java
Removed DIRECT target handling (targetLineIds assignment and lookup call) and final batchInsert invocation from sendNotification method.
Notification Service Integration
src/main/java/com/pooli/policy/service/AdminPolicyServiceImpl.java
Replaced synchronous sendNotification with transaction-aware async pattern: registers after-commit callback when within active transaction; invokes immediately otherwise.
Notification Mapper
src/main/java/com/pooli/notification/mapper/AlarmHistoryMapper.java, src/main/resources/mapper/notification/AlarmHistoryMapper.xml
Added trailing newlines (formatting only).
Test Updates
src/test/java/com/pooli/notification/service/AlarmHistoryServiceImplTest.java, src/test/java/com/pooli/policy/service/AdminPolicyServiceImplTest.java
Updated test verifications to use sendNotificationAsync method; added trailing newline in service test.

Sequence Diagram

sequenceDiagram
    participant PolicyService as AdminPolicyService
    participant TransactionMgr as TransactionSynchronizationManager
    participant AlarmService as AlarmHistoryService
    participant Database
    
    PolicyService->>TransactionMgr: Check if in active transaction
    alt Within Transaction
        TransactionMgr->>TransactionMgr: Register after-commit callback
        TransactionMgr-->>PolicyService: Callback registered
        PolicyService->>Database: Commit transaction
        Note over Database: After commit
        TransactionMgr->>AlarmService: sendNotificationAsync()
        AlarmService->>Database: Persist notifications
    else No Active Transaction
        PolicyService->>AlarmService: sendNotificationAsync()
        AlarmService->>Database: Persist notifications
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • Dev #246: Modifies notification persistence logic in AlarmHistoryServiceImpl, directly related to the removal of batchInsert and DIRECT target handling.
  • Dev #244: Introduces the async notification API (sendNotificationAsync) that this PR builds upon for transaction-aware execution.

Suggested labels

feature, test

Suggested reviewers

  • ms-0o0
  • hyuuuun
  • hyeonRS

Poem

🐰 Hoppy notification hops so fast,
Now waiting for transactions to pass,
With async calls after commit's done,
The alarm service hops and runs!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ms-0o0 ms-0o0 merged commit 6bbdbc1 into main Mar 18, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants