Conversation
WalkthroughUpdates a JPQL query in ExamApplicationJpaRepository to broaden selection logic and exclude deleted applications. Adds a bean validation constraint limiting inquiry attachments to a maximum of three files. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller
participant Repository
participant DB
Client->>Controller: findByApplicationIdIn(applicationIds)
Controller->>Repository: Query with (payment DONE OR refund DONE) AND ea.deleted = false
Repository->>DB: Execute JPQL with joins (Payment, Refund)
DB-->>Repository: Results
Repository-->>Controller: Applications with status projection
Controller-->>Client: Response
sequenceDiagram
actor Client
participant Controller
participant Validator
participant Service
Client->>Controller: POST InquiryCreateRequest
Controller->>Validator: Bean validation (attachments @Size <= 3)
alt Invalid (>3)
Validator-->>Controller: ConstraintViolation
Controller-->>Client: 400 with message
else Valid
Controller->>Service: Create inquiry
Service-->>Controller: OK
Controller-->>Client: 201
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @jbh010204, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 기존 코드의 리팩토링을 통해 MOSU-333 이슈를 해결합니다. 주요 변경 사항은 시험 신청서 조회 로직을 개선하고 문의 생성 요청에 대한 첨부 파일 유효성 검사를 추가하는 것입니다.
Highlights
- 시험 신청서 조회 로직 개선: ExamApplicationJpaRepository.java 파일에서 시험 신청서 조회 쿼리에 deleted 플래그 조건을 추가하고, 결제 및 환불 상태 조건의 논리를 조정했습니다.
- 문의 첨부 파일 유효성 검사 추가: InquiryCreateRequest.java 파일에 문의 생성 시 첨부 파일의 최대 개수를 3개로 제한하는 유효성 검사 규칙을 추가했습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
...ava/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryCreateRequest.java (1)
21-22: Cascade-validate each FileRequest and clarify nullability
- If each file’s fields should be validated, add container element validation to trigger nested validation on list elements.
- If attachments must not be null, also add
@NotNull. Otherwise, null is allowed and bypasses@Size.Apply this diff to validate elements:
- @Size(max = 3, message = "첨부파일은 최대 3개까지 첨부할 수 있습니다.") - List<FileRequest> attachments + @Size(max = 3, message = "첨부파일은 최대 3개까지 첨부할 수 있습니다.") + List<@Valid FileRequest> attachmentsAnd add the missing import (outside the selected lines):
import jakarta.validation.Valid;Optionally enforce non-null if desired:
- @Size(max = 3, message = "첨부파일은 최대 3개까지 첨부할 수 있습니다.") - List<@Valid FileRequest> attachments + @NotNull + @Size(max = 3, message = "첨부파일은 최대 3개까지 첨부할 수 있습니다.") + List<@Valid FileRequest> attachments
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryCreateRequest.java(1 hunks)
🔇 Additional comments (3)
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java (2)
94-94: Good hard-delete guardAdding
ea.deleted = falsealigns this query with the soft-delete policy used elsewhere. Nice catch.
92-92: Please confirm the widened “payment-done OR refund-done” filter and its impact on duplicatesWe found that RefundJpaEntity has no unique constraint on examApplicationId, so multiple refund rows per application are possible. With the new OR-based join in ExamApplicationJpaRepository, this could surface duplicate exam applications if more than one refund record exists.
Key location to review:
• src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java:
WHERE (p.paymentStatus = 'DONE' OR r.refundStatus = 'DONE')Verify:
- That including applications with a DONE refund but non-DONE payment is intended.
- Whether the domain guarantees at most one success refund per examApplicationId; if not, consider adding DISTINCT, grouping, or enforcing uniqueness at the data/model layer.
src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryCreateRequest.java (1)
21-22: Cap attachments to 3 files: LGTMThe new
@Size(max = 3, ...)constraint correctly enforces the attachments limit at the DTO boundary.
| WHERE (p.paymentStatus = 'DONE' OR r.refundStatus = 'DONE') | ||
| AND ea.applicationId IN :applicationIds | ||
| AND ea.deleted = false | ||
| """) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid OR on a LEFT-joined table to prevent row multiplication; add p.deleted filter for consistency
Using OR with a column from the LEFT join (Line 92) will include all rows where p.paymentStatus = 'DONE' even when there are multiple Refund rows with non-DONE statuses, multiplying results per application. It also broadens semantics to include cases where payment is not DONE but refund is DONE (intentional?), and misses the common p.deleted = false guard used elsewhere in this repository.
Refactor to use EXISTS for the refund condition and restore the p.deleted = false consistency. This prevents duplicates and keeps semantics explicit.
Apply this diff within the current query:
- WHERE (p.paymentStatus = 'DONE' OR r.refundStatus = 'DONE')
- AND ea.applicationId IN :applicationIds
- AND ea.deleted = false
+ WHERE ea.applicationId IN :applicationIds
+ AND ea.deleted = false
+ AND (
+ (p.paymentStatus = 'DONE' AND p.deleted = false)
+ OR EXISTS (
+ SELECT 1 FROM RefundJpaEntity r2
+ WHERE r2.examApplicationId = ea.id
+ AND r2.refundStatus = 'DONE'
+ )
+ )Additionally, since the refund condition is evaluated via EXISTS, the LEFT JOIN to Refund is no longer needed and can be removed to simplify the query and avoid accidental fan-out:
// Outside the selected lines (optional cleanup)
JOIN PaymentJpaEntity p ON p.examApplicationId = ea.id
-- remove: LEFT JOIN RefundJpaEntity r ON r.examApplicationId = ea.idIf the broadened inclusion of “refund-done even if payment not done” is intentional, the EXISTS keeps that behavior without duplicating rows. If not intentional, consider reverting to the stricter semantics.
🤖 Prompt for AI Agents
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java
around lines 92-95: the query uses "WHERE (p.paymentStatus = 'DONE' OR
r.refundStatus = 'DONE')" with a LEFT JOIN to Refund, which can multiply rows
and omits the p.deleted guard. Replace the OR with an EXISTS subquery that
checks for a Refund row with refundStatus = 'DONE' and deleted = false for the
same exam application, restore the p.deleted = false condition and make the
Payment join an inner JOIN (JOIN PaymentJpaEntity p ON p.examApplicationId =
ea.id) so the payment filter applies consistently, and remove the LEFT JOIN to
Refund entirely to avoid fan-out.
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Bug Fixes