Uh oh!
There was an error while loading. Please reload this page.
feat: 어드민 인증 필요 시 알림을 보내도록 - #828
Conversation
Walkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8729a79937
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| HttpEntity<Map<String, String>> request = new HttpEntity<>(Map.of("content", buildMessage(type, applicantInfo)), headers); |
There was a problem hiding this comment.
When this notifier is called from score and mentor submissions, applicantInfo is siteUser.getNickname(), which users can control. Because the webhook payload only sends raw content, a nickname such as @everyone, @here, or a role mention will be parsed by Discord and can ping the admin channel whenever that user submits a score or mentor application; add allowed_mentions with an empty parse list or escape user-supplied text before sending.
Useful? React with 👍 / 👎.
| @Value("${spring.profiles.active:}") | ||
| private String environment; | ||
| @Async |
There was a problem hiding this comment.
When the shared async executor is full (it is bounded in AsyncConfig and also used by S3/view-count work), calling an @Async method can be rejected before notify() enters its try/catch. In that case the transactional score, mentor, or report request can fail or roll back solely because the Discord notification could not be queued; wrap the proxy call/rejection or use a dedicated best-effort executor/rejection policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/java/com/example/solidconnection/common/discord/DiscordNotifier.java`:
- Around line 28-29: Update DiscordNotifier’s message construction to omit the
environment prefix, including its brackets and spacing, when the injected
environment value is blank; retain the existing prefixed format when an
environment is configured.
- Around line 41-43: Update DiscordNotifier.notify(...) so the catch path no
longer only logs and drops failures; instead persist the failed alert or send
event to the existing outbox/queue flow, using DiscordNotifier as the entry
point and the current type/applicantInfo payload as the retry record. Add
deduplication and a bounded retry policy around the send operation, and route
exhausted retries to the failure-storage/alert path rather than swallowing the
exception.
In `@src/main/java/com/example/solidconnection/score/service/ScoreService.java`:
- Line 52: Move the Discord notification calls to transaction-commit event
handling so they execute only after the surrounding save transaction
successfully commits, preserving the existing notification types and recipients:
update ScoreService.java lines 52-52 and 65-65, MentorApplicationService.java
line 66-66, and ReportService.java line 45-45. Use an AFTER_COMMIT transaction
event or an equivalent outbox flow, and do not invoke DiscordNotifier.notify
directly within the transactional methods.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72cd2f8c-4573-4601-ab75-967e9524def3
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/common/discord/DiscordNotificationType.javasrc/main/java/com/example/solidconnection/common/discord/DiscordNotifier.javasrc/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.javasrc/main/java/com/example/solidconnection/report/service/ReportService.javasrc/main/java/com/example/solidconnection/score/service/ScoreService.java
| @Value("${spring.profiles.active:}") | ||
| private String environment; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
환경 값이 비어 있으면 접두사를 생략하세요.
spring.profiles.active의 기본값이 빈 문자열이므로 환경이 지정되지 않으면 메시지가 [] 학점 성적 검수 요청... 형식으로 시작합니다. 빈 환경을 유지하려면 환경 접두사 전체를 생략하세요.
Also applies to: 46-48
🤖 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/main/java/com/example/solidconnection/common/discord/DiscordNotifier.java`
around lines 28 - 29, Update DiscordNotifier’s message construction to omit the
environment prefix, including its brackets and spacing, when the injected
environment value is blank; retain the existing prefixed format when an
environment is configured.
| } catch (Exception e) { | ||
| log.error("Discord 검수 알림 전송 실패. type={}, applicantInfo={}", type, applicantInfo, e); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Files matching DiscordNotifier:"
fd -i 'DiscordNotifier'.||trueechoecho"Git status/stat:"
git diff --stat ||trueechoecho"DiscordNotifier outline:"
file="$(fd -i '^DiscordNotifier\.java$'.| head -n1 || true)"if [ -n"${file:-}" ];thenecho"FILE=$file"
wc -l "$file"
ast-grep outline "$file"||trueechoecho"DiscordNotifier relevant lines:"
cat -n "$file"| sed -n '1,180p'fiechoecho"Search Discord notifier usages and related outbox/queue terms:"
rg -n "DiscordNotifier|sendExam|exam|outbox|queue|retries?|retry|DeadLetter|DLQ|실패|실패한|성공" src/main/java src/test/java 2>/dev/null ||trueRepository: solid-connection/solid-connect-server
Length of output: 50394
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Project configuration files:"
git ls-files | rg '(^|/)(build\.gradle|build\.gradle\.kts|pom\.xml|gradle\.properties|settings\.gradle|docker-compose\.ya?ml|application.*\.ya?ml|application.*\.properties)$'||trueechoecho"Search notification infrastructure:"
rg -n "AsyncEventListener|ApplicationEvent.|`@Transactional`|TransactionalEventListener|`@EventListener`|Outbox|Outbox|Queue|Queue|CloudEvent|Notification|Notifier\\.|sendExam|sendExam" src/main/java 2>/dev/null ||trueRepository: solid-connection/solid-connect-server
Length of output: 17180
1. 실패한 Discord 알림을 저장하고 재시도하세요.
DiscordNotifier.notify(...)가 내부 예외를 로그에만 남기기 때문에 Discord 429, 5xx, 네트워크 오류는 알림 유실로 이어집니다. 이벤트 또는 전송 상태를 outbox/queue에 저장하고, 중복 전송 방지와 함께 제한된 재시도, 실패 보관/경보를 구현하세요.
- 1 전송 실패 시 이벤트 또는 전송 기록을 outbox/queue에 저장하세요.
- 2 중복 전송 방지와 제한된 재시도를 함께 구현하세요.
- 3 재시도 모두 실패 시 실패 보관/알림 경로로 이동시키세요.
🤖 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/main/java/com/example/solidconnection/common/discord/DiscordNotifier.java`
around lines 41 - 43, Update DiscordNotifier.notify(...) so the catch path no
longer only logs and drops failures; instead persist the failed alert or send
event to the existing outbox/queue flow, using DiscordNotifier as the entry
point and the current type/applicantInfo payload as the retry record. Add
deduplication and a bounded retry policy around the send operation, and route
exhausted retries to the failure-storage/alert path rather than swallowing the
exception.
| Gpa gpa = new Gpa(gpaScoreRequest.gpa(), gpaScoreRequest.gpaCriteria(), uploadedFile.fileUrl()); | ||
| GpaScore newGpaScore = new GpaScore(gpa, siteUser); | ||
| GpaScore savedNewGpaScore = gpaScoreRepository.save(newGpaScore); | ||
| discordNotifier.notify(DiscordNotificationType.GPA_SCORE, siteUser.getNickname()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'Find candidate service files:\n'
fd -a 'ScoreService\.java|MentorApplicationService\.java|ReportService\.java|.*Notifier.*\.java|.*Notification.*\.java|.*Listener.*\.java|.*Async.*\.java' src/main/java 2>/dev/null | sed 's#^\./##'||trueprintf'\nScoreService outline/contents:\n'
ast-grep outline src/main/java/com/example/solidconnection/score/service/ScoreService.java 2>/dev/null ||true
cat -n src/main/java/com/example/solidconnection/score/service/ScoreService.java
printf'\nMentorApplicationService outline/contents:\n'
ast-grep outline src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java 2>/dev/null ||true
cat -n src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java
printf'\nReportService outline/contents:\n'
ast-grep outline src/main/java/com/example/solidconnection/report/service/ReportService.java 2>/dev/null ||true
cat -n src/main/java/com/example/solidconnection/report/service/ReportService.java
printf'\nSearch for DiscordNotifier implementation and async/disk notification handling:\n'
rg -n "interface DiscordNotifier|class .*Discord|notify\\(|`@Async`|ApplicationEvent|AFTER_COMMIT|TransactionSynchronizationManager|TransactionManager|EventPublisher|save\\(" src/main/java src/test/java 2>/dev/null | head -n 200Repository: solid-connection/solid-connect-server
Length of output: 38630
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'DiscordNotifier implementation:\n'
cat -n src/main/java/com/example/solidconnection/common/discord/DiscordNotifier.java
printf'\nAsync transactional configuration:\n'
cat -n src/main/java/com/example/solidconnection/common/config/sync/AsyncConfig.java
printf'\nStatic verifier: direct Discord notifier calls from `@Transactional` save paths:\n'
python3 - <<'PY'from pathlib import Pathimport retargets = [ ('src/main/java/com/example/solidconnection/score/service/ScoreService.java', [52, 65]), ('src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java', [66]), ('src/main/java/com/example/solidconnection/report/service/ReportService.java', [45]),]notifier = Path('src/main/java/com/example/solidconnection/common/discord/DiscordNotifier.java').read_text()for file, lines in targets: src = Path(file).read_text().splitlines() in_transactional = False for i, line in enumerate(src, 1): if '`@Transactional`' in line: in_transactional = True if i in lines: call = line.strip() print(f'{file}:{i}: transactional_context={in_transactional} call={call}') if 'public ' in line and 'void' in line or (i in lines and in_transactional): in_transactional = 'return' not in line or 'void' not in linehas_async_notify = bool(re.search(r'@\s*Async[\s\n]*public\s+void\s+notify\b', notifier))has_spring_async_annotation = bool(re.search(r'org\.springframework\..scheduling\.\w+\.annotation\.\s*`@Async`', Path('pom.xml').read_text() if Path('pom.xml').exists() else ''))print(f'DiscordNotifier.notify is `@Async`={has_async_notify}')print(f'Service paths dispatch outside transaction commit boundary; Spring `@Async` tasks queued while current request path returns before notification completion.')PYRepository: solid-connection/solid-connect-server
Length of output: 4433
1. Discord 알림 트랜잭션 커밋과 분리하세요.
네 위치 모두 @Async DiscordNotifier.notify가 @Transactionalsave 후에 실행됩니다. 기존 등록/수정/삭제가 rollback되면 알림이 누락되거나, 실제 데이터 링크는 커밋이 완성되지 않은 상태로 전송될 수 있습니다. transaction event를 AFTER_COMMIT에서 처리하거나 outbox로 저장 후 발송하세요.
- src/main/java/com/example/solidconnection/score/service/ScoreService.java#L52: GPA 알림을 커밋 이후 이벤트로 변경하세요.
- src/main/java/com/example/solidconnection/score/service/ScoreService.java#L65: 어학 성적 알림을 커밋 이후 이벤트로 변경하세요.
- src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java#L66: 멘토 지원 알림을 커밋 이후 이벤트로 변경하세요.
- src/main/java/com/example/solidconnection/report/service/ReportService.java#L45: 신고 알림을 커밋 이후 이벤트로 변경하세요.
[low Effort_and_high_reward]
📍 Affects 3 files
src/main/java/com/example/solidconnection/score/service/ScoreService.java#L52-L52(this comment)src/main/java/com/example/solidconnection/score/service/ScoreService.java#L65-L65src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java#L66-L66src/main/java/com/example/solidconnection/report/service/ReportService.java#L45-L45
🤖 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/main/java/com/example/solidconnection/score/service/ScoreService.java` at
line 52, Move the Discord notification calls to transaction-commit event
handling so they execute only after the surrounding save transaction
successfully commits, preserving the existing notification types and recipients:
update ScoreService.java lines 52-52 and 65-65, MentorApplicationService.java
line 66-66, and ReportService.java line 45-45. Use an AFTER_COMMIT transaction
event or an equivalent outbox flow, and do not invoke DiscordNotifier.notify
directly within the transactional methods.
Uh oh!
There was an error while loading. Please reload this page.
관련 이슈
작업 내용
성적, 멘토 신청, 신고 시 디스코드로 알림이 오도록 구현했습니다.
parameter store에 웹훅 url 등록했습니다.
특이 사항
리뷰 요구사항 (선택)