MOSU-123 refactor: 설계 변경으로 인한 관리자 기능 변경#125
Conversation
WalkthroughThis update migrates the admin, application, payment, refund, and initialization modules from a school/application-centric model to an exam/exam-application-centric model. It replaces lunch selection from an ID to a boolean, updates DTOs, queries, and entity relationships, and refactors the database initializer and admin controller/services to match the new domain structure. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminController
participant AdminService
participant ApplicationQueryRepo
participant RefundQueryRepo
participant PaymentRepo
participant ExamApplicationRepo
AdminController->>AdminService: getApplications(filter, pageable)
AdminService->>ApplicationQueryRepo: searchAllApplications(filter, pageable)
ApplicationQueryRepo->>ExamApplicationRepo: query exam applications
ExamApplicationRepo-->>ApplicationQueryRepo: exam application data
ApplicationQueryRepo-->>AdminService: application responses
AdminService-->>AdminController: application responses
AdminController->>AdminService: getLunchCounts()
AdminService->>ApplicationQueryRepo: searchAllSchoolLunches()
ApplicationQueryRepo->>ExamApplicationRepo: query lunch counts
ExamApplicationRepo-->>ApplicationQueryRepo: lunch count data
ApplicationQueryRepo-->>AdminService: lunch count responses
AdminService-->>AdminController: lunch count responses
AdminController->>AdminService: getRefundCounts(pageable)
AdminService->>RefundQueryRepo: searchAllRefunds(pageable)
RefundQueryRepo->>ExamApplicationRepo: query refund data
ExamApplicationRepo-->>RefundQueryRepo: refund data
RefundQueryRepo-->>AdminService: refund responses
AdminService-->>AdminController: refund responses
Estimated code review effort4 (~90 minutes) Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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 @wlgns12370, 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!
This pull request implements a major architectural change by refactoring the core domain model related to exam applications, payments, and refunds. The primary goal is to align the system with a new design, which involved updating database entities, query repositories, service logic, and API endpoints. This ensures that the administrative functionalities operate correctly and efficiently with the revised data structure.
Highlights
- Core Data Model Refactoring: The underlying data model for exam applications, payments, and refunds has been significantly refactored. This includes migrating from an
ApplicationSchoolcentric model to a new structure centered aroundExamApplicationandExamentities. Payments and refunds are now directly linked toApplicationandExamApplicationrespectively. - Admin Functionality Activation and Update: Previously commented-out admin service logic and API endpoints have been activated and updated to reflect the new data model. This enables full functionality for retrieving student lists, application details, school lunch counts, and refund information through the admin panel.
- Simplified Lunch Option Handling: The handling of lunch options during exam application has been simplified from using a
lunchId(presumably an ID for a specific lunch option) to a straightforward booleanisLunchChecked, indicating whether lunch is selected or not. - Data Initialization Overhaul: The database initializer has been completely rewritten to populate dummy data that aligns with the new
Exam,ExamApplication, andPaymententities, ensuring consistency with the updated design. - API Documentation and DTO Adjustments: API documentation (Swagger/OpenAPI) and various Data Transfer Objects (DTOs) have been updated to match the refactored data structures and endpoint behaviors, including changes to
ApplicationExcelDto,ApplicationListResponse, andExamTicketResponse.
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 or fill out our survey to provide feedback.
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. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring of the domain model, moving from a School-based application structure to an Exam-based one. This impacts entities like Application, Payment, and Refund, and introduces new entities like ExamApplication and ExamSubject. The admin-facing services, controllers, and queries have been updated accordingly to reflect these design changes. The changes are extensive and generally move towards a more normalized and flexible data model. However, I've found a couple of critical issues in the new QueryDSL implementations in ApplicationQueryRepositoryImpl and RefundQueryRepositoryImpl that will cause runtime errors due to incorrect join conditions or incomplete queries. There are also some medium-severity suggestions for improving code quality and ensuring input validation. Please address the critical issues before merging.
| .from(examApplication) | ||
| .leftJoin(exam).on(examApplication.examId.eq(exam.id)) | ||
| .leftJoin(application).on(examApplication.applicationId.eq(application.id)) | ||
| .leftJoin(payment).on(payment.applicationId.eq(examApplication.id)) |
There was a problem hiding this comment.
The join condition for the payment table appears to be incorrect. It's currently joining on payment.applicationId.eq(examApplication.id). Based on the data model, payment.applicationId stores the ID of an ApplicationJpaEntity, but it's being compared with examApplication.id, which is the ID of an ExamApplicationJpaEntity. This will likely result in incorrect data or no data being fetched for payments.
The join should be made against application.id.
| .leftJoin(payment).on(payment.applicationId.eq(examApplication.id)) | |
| .leftJoin(payment).on(payment.applicationId.eq(application.id)) |
| refund.createdAt, | ||
| refund.agreedAt, | ||
| // payment.paymentMethod, | ||
| refund.reason |
There was a problem hiding this comment.
The query is missing a join to the payment table, and payment.paymentMethod is commented out in the select clause. This will cause tuple.get(payment.paymentMethod) to fail. Although there is a TODO comment, this code is currently broken and should be fixed before merging. Please add the join to the payment table and include payment.paymentMethod in the selection.
return queryFactory
.select(
refund.id,
examApplication.examNumber,
profile.userName,
profile.phoneNumber,
refund.createdAt,
refund.agreedAt,
payment.paymentMethod,
refund.reason
)
.from(refund)
.leftJoin(examApplication).on(refund.examApplicationId.eq(examApplication.id))
.leftJoin(application).on(examApplication.applicationId.eq(application.id))
.leftJoin(profile).on(profile.userId.eq(application.userId))
.leftJoin(payment).on(payment.applicationId.eq(application.id));| .examNumber(String.format("MOSU-%d-%d", application.getId(), | ||
| exam.getId())) | ||
| .isLunchChecked( | ||
| ((j % 2 == 0) ? true : false) |
There was a problem hiding this comment.
This boolean assignment can be simplified for better readability. The ternary operator (condition ? true : false) is redundant and can be replaced by the condition itself.
.isLunchChecked(j % 2 == 0)| @Schema(description = "전화번호 필터", example = "010-1234-5678") | ||
| String phone, |
There was a problem hiding this comment.
The @PhoneNumberPattern validation annotation has been removed from the phone field. Was this intentional? Removing it might weaken input validation and allow incorrectly formatted phone numbers to be processed. If validation is still required, please consider re-adding it or ensuring it's handled elsewhere.
| @Schema(description = "전화번호 필터", example = "01012345678") | ||
| @PhoneNumberPattern | ||
| String phone, |
There was a problem hiding this comment.
Similar to ApplicationFilter, the @PhoneNumberPattern validation annotation has been removed from the phone field. Could you please confirm if this was intentional? This change could affect input validation for phone numbers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java (1)
25-39: Fix inconsistent variable naming and verify event parameter alignment.The variable naming suggests a mismatch with the domain model changes:
missingAppSchoolIdsshould be renamed tomissingAppIdssince it now contains application IDs, not school IDs- The event method
applicationSchoolIds()and parameter nameappSchoolIdsuggest they still reference school IDs, which may be inconsistent with the new domain modelVerify that the event structure has been updated to match the new domain model:
- List<Long> missingAppSchoolIds = event.applicationSchoolIds().stream() - .filter(appSchoolId -> !existingAppIds.contains(appSchoolId)) + List<Long> missingAppIds = event.applicationIds().stream() + .filter(appId -> !existingAppIds.contains(appId)) .toList(); // 실패 신규 엔티티 생성 ( 배치 후속 처리 필요 ) - List<PaymentJpaEntity> newPayments = missingAppSchoolIds.stream() - .map(appSchoolId -> PaymentJpaEntity.ofFailure( - appSchoolId, + List<PaymentJpaEntity> newPayments = missingAppIds.stream() + .map(appId -> PaymentJpaEntity.ofFailure( + appId, event.orderId(), event.status(), event.totalAmount())) .toList();
🧹 Nitpick comments (2)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationFilter.java (1)
12-12: LGTM - Improved phone number format, but consider consistency across DTOs.The change to hyphenated phone number format ("010-1234-5678") improves readability. However, there's an inconsistency with other DTOs like
StudentExcelDto(line 15 insrc/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java) which still uses the unhyphenated format ("01012345678").Consider standardizing the phone number format across all DTOs for consistency:
// In StudentExcelDto.java @Schema(description = "전화번호", example = "010-1234-5678")src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentFilter.java (1)
14-15: Track completion of sorting functionalityThe schema description change to "개발중" (in development) indicates the sorting feature is incomplete. Ensure this is tracked and completed as part of the refactoring effort.
Consider creating a TODO or tracking issue for completing the sorting functionality to avoid leaving it in an incomplete state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/life/mosu/mosuserver/application/admin/AdminService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java(2 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java(4 hunks)src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaEntity.java(3 hunks)src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminControllerDocs.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationFilter.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationListResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/SchoolLunchResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentFilter.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java(1 hunks)
🧬 Code Graph Analysis (3)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationFilter.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
Schema(8-45)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
Schema(8-45)
src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaEntity.java (2)
src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java (1)
Entity(18-102)src/main/java/life/mosu/mosuserver/domain/application/ApplicationJpaEntity.java (1)
Entity(15-52)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationFilter.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
Schema(8-45)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
Schema(8-45)
src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaEntity.java (2)
src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java (1)
Entity(18-102)src/main/java/life/mosu/mosuserver/domain/application/ApplicationJpaEntity.java (1)
Entity(15-52)
🔇 Additional comments (29)
src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java (1)
22-22: LGTM - Consistent with domain model changes.The change from
getApplicationSchoolId()togetApplicationId()aligns with the broader domain model refactoring from school-centric to exam-centric model.src/main/java/life/mosu/mosuserver/presentation/admin/dto/SchoolLunchResponse.java (1)
16-19: LGTM - Clean factory method implementation.The static factory method follows standard Java conventions and provides a clean way to create instances of the record. The implementation is straightforward and appropriate for a simple DTO.
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (2)
4-5: LGTM - Appropriate imports for the collection type changes.The import additions support the new Set collection type and stream operations used in the updated subject handling logic.
63-65: LGTM - Improved subject representation.The change from
List<Subject>toSet<String>provides several benefits:
- Prevents duplicate subjects in the response
- Simplifies the DTO by using string names instead of full domain objects
- Reduces coupling between service and domain layers
The mapping logic correctly extracts subject names from the domain entities.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java (2)
7-7: LGTM: Clean domain model simplificationThe change from
Long lunchIdtoboolean isLunchCheckedappropriately simplifies the lunch selection from a specific ID reference to a boolean flag, aligning with the exam-centric domain model refactoring.
10-12: LGTM: Method correctly updated for new parameterThe
toTargetExam()method properly passes the new boolean parameter, maintaining consistency with the domain model changes.src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java (3)
5-5: LGTM: Appropriate import updateImport correctly updated from
ListtoSetto support the field type change.
21-21: LGTM: Set usage is semantically correct for subjectsChanging from
List<Subject>toSet<String>is a good design decision:
- Set prevents duplicate subjects, which is semantically correct
- String representation simplifies the DTO by removing domain object dependencies
- Aligns with the exam-centric domain model refactoring
32-32: LGTM: Factory method parameter updated consistentlyThe static factory method parameter correctly matches the field type change to
Set<String>.src/main/java/life/mosu/mosuserver/application/admin/AdminService.java (2)
21-28: LGTM: Proper Spring service configurationGood use of Spring annotations and constructor-based dependency injection. The service properly declares its dependencies on the query repository implementations.
30-55: LGTM: Clean delegation patternThe service methods appropriately delegate to specialized repository implementations. While the service layer is thin, it provides a good abstraction that allows for future business logic addition without changing the controller interface.
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java (2)
11-31: LGTM: Good modernization to Java recordThe conversion from a traditional class to a record is a good modernization that:
- Eliminates boilerplate code (constructors, getters, equals, hashCode)
- Maintains all necessary annotations for schema documentation and Excel export
- Preserves field semantics and types appropriately
The inline annotations are acceptable for a DTO, though they create longer lines.
22-23: Excel generation logic verifiedThe
lunchandsubjectsfields are correctly handled inApplicationQueryRepositoryImpl.mapToExcel(...):
lunchNameis retrieved viatuple.get(exam.lunchName)and passed as the DTO’slunchString.subjectsare converted fromSet<Subject>toSet<String>viasubjects.stream().map(Subject::getSubjectName).collect(...).No further changes are required.
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java (1)
30-31: LGTM: Lunch field simplification is well-designed.The change from
Long lunchIdtoboolean isLunchCheckedsimplifies the lunch selection model and makes the intent clearer. The column mapping tolunch_checkedis appropriate.src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java (5)
36-36: LGTM: Service dependency properly injected.The AdminService dependency is correctly injected using constructor injection with
@RequiredArgsConstructor.
42-45: LGTM: Service integration and pagination defaults.The service integration is properly implemented and the default page size of 10 is reasonable for admin interfaces.
59-64: LGTM: Excel download functionality well-implemented.The Excel download implementation is clean and follows good practices:
- Proper content type and encoding handling
- Correct use of SimpleExcelFile with generic type
- Direct streaming to response output
94-99: LGTM: Consistent Excel implementation pattern.The application Excel download follows the same well-structured pattern as the student Excel download.
103-109: Complete the refund endpoint implementation.The refund endpoint still has the service call commented out and returns null data, making it incomplete compared to other endpoints.
Should this endpoint be fully implemented as part of this PR, or is it intentionally left as a stub for future implementation?
src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java (3)
17-17: LGTM: Consistent mapping of boolean lunch selection.The update to map
isLunchCheckedinstead oflunchIdis consistent with the entity changes and maintains proper data flow.
27-27: LGTM: Entity creation properly updated.The
toEntity()method correctly passes the booleanisLunchCheckedvalue to the entity creation method.
32-35: LGTM: TargetExam record updated consistently.The nested
TargetExamrecord properly reflects the change fromLong lunchIdtoboolean isLunchChecked, maintaining consistency across the domain model.src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaEntity.java (3)
27-28: LGTM: Field renaming aligns with domain model changes.The renaming from
applicationSchoolIdtoexamApplicationIdis consistent with the broader refactoring to an exam-centric domain model.
30-31: LGTM: Simplified column naming.The column name change from
refund_reasontoreasonsimplifies the naming convention while maintaining clarity.
40-50: Verify database migration scripts for column renamesI couldn’t locate any migrations that rename the old columns (
application_school_id→exam_application_id,refund_reason→reason). Please ensure your database migration includes these changes. For example, check undersrc/main/resources/db/migration(or wherever migrations live) for a script like:
- ALTER TABLE refund RENAME COLUMN application_school_id TO exam_application_id;
- ALTER TABLE refund RENAME COLUMN refund_reason TO reason;
src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java (4)
29-30: LGTM: Consistent field renaming.The renaming from
applicationSchoolIdtoapplicationIdimproves clarity and aligns with the domain model refactoring. The column mapping remaining asapplication_idmaintains database compatibility.
51-58: LGTM: Constructor properly updated.The constructor parameter and field assignment are consistently updated to use the new
applicationIdname.
67-82: LGTM: Static factory methods updated consistently.Both the parameter name and builder method call are properly updated to use
applicationIdthroughout theofmethod.
85-97: LGTM: Failure factory method updated consistently.The
ofFailuremethod is also properly updated with the new parameter name and builder method call.
| public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter, | ||
| Pageable pageable) { | ||
|
|
||
| JPAQuery<Tuple> query = baseQuery() | ||
| .where( | ||
| buildNameCondition(filter.name()), | ||
| buildPhoneCondition(filter.phone()) | ||
| ) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()); | ||
|
|
||
| List<ApplicationListResponse> content = query.fetch().stream() | ||
| .map(tuple -> { | ||
| Long appSchoolId = tuple.get(examApplication.id); | ||
| Set<Subject> subjects = appSchoolId != null | ||
| ? findSubjectsByExamApplicationId(appSchoolId) | ||
| : new HashSet<>(); | ||
| return mapToResponse(tuple, subjects); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return new PageImpl<>(content, pageable, content.size()); | ||
| } |
There was a problem hiding this comment.
Incorrect pagination total count
The total count is calculated incorrectly, using the page size instead of the actual total count from the database.
Calculate the total count properly:
public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter,
Pageable pageable) {
+ long total = baseQuery()
+ .where(
+ buildNameCondition(filter.name()),
+ buildPhoneCondition(filter.phone())
+ )
+ .fetchCount();
JPAQuery<Tuple> query = baseQuery()
.where(
buildNameCondition(filter.name()),
buildPhoneCondition(filter.phone())
)
.offset(pageable.getOffset())
.limit(pageable.getPageSize());
List<ApplicationListResponse> content = query.fetch().stream()
.map(tuple -> {
Long appSchoolId = tuple.get(examApplication.id);
Set<Subject> subjects = appSchoolId != null
? findSubjectsByExamApplicationId(appSchoolId)
: new HashSet<>();
return mapToResponse(tuple, subjects);
})
.toList();
- return new PageImpl<>(content, pageable, content.size());
+ return new PageImpl<>(content, pageable, total);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter, | |
| Pageable pageable) { | |
| JPAQuery<Tuple> query = baseQuery() | |
| .where( | |
| buildNameCondition(filter.name()), | |
| buildPhoneCondition(filter.phone()) | |
| ) | |
| .offset(pageable.getOffset()) | |
| .limit(pageable.getPageSize()); | |
| List<ApplicationListResponse> content = query.fetch().stream() | |
| .map(tuple -> { | |
| Long appSchoolId = tuple.get(examApplication.id); | |
| Set<Subject> subjects = appSchoolId != null | |
| ? findSubjectsByExamApplicationId(appSchoolId) | |
| : new HashSet<>(); | |
| return mapToResponse(tuple, subjects); | |
| }) | |
| .toList(); | |
| return new PageImpl<>(content, pageable, content.size()); | |
| } | |
| public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter, | |
| Pageable pageable) { | |
| long total = baseQuery() | |
| .where( | |
| buildNameCondition(filter.name()), | |
| buildPhoneCondition(filter.phone()) | |
| ) | |
| .fetchCount(); | |
| JPAQuery<Tuple> query = baseQuery() | |
| .where( | |
| buildNameCondition(filter.name()), | |
| buildPhoneCondition(filter.phone()) | |
| ) | |
| .offset(pageable.getOffset()) | |
| .limit(pageable.getPageSize()); | |
| List<ApplicationListResponse> content = query.fetch().stream() | |
| .map(tuple -> { | |
| Long appSchoolId = tuple.get(examApplication.id); | |
| Set<Subject> subjects = appSchoolId != null | |
| ? findSubjectsByExamApplicationId(appSchoolId) | |
| : new HashSet<>(); | |
| return mapToResponse(tuple, subjects); | |
| }) | |
| .toList(); | |
| return new PageImpl<>(content, pageable, total); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
around lines 56 to 78, the pagination total count is incorrectly set to the
current page size instead of the actual total number of records. Fix this by
executing a separate count query that matches the filtering conditions to get
the total count from the database, and use that count value when constructing
the PageImpl object instead of content.size().
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
| private ApplicationExcelDto mapToExcel(Tuple tuple, Set<Subject> subjects) { | ||
| Set<String> subjectNames = subjects.stream() | ||
| .map(Subject::getSubjectName) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| String lunchName = tuple.get(exam.lunchName); | ||
| String genderName = tuple.get(profile.gender).getGenderName(); | ||
| String gradeName = tuple.get(profile.grade).getGradeName(); | ||
| String educationName = tuple.get(profile.education).getEducationName(); | ||
| String appliedAt = tuple.get(application.createdAt) | ||
| .format(EXCEL_DT_FORMATTER); | ||
|
|
||
| return new ApplicationExcelDto( | ||
| tuple.get(payment.paymentKey), | ||
| tuple.get(examApplication.examNumber), | ||
| tuple.get(profile.userName), | ||
| genderName, | ||
| tuple.get(profile.birth), | ||
| tuple.get(profile.phoneNumber), | ||
| tuple.get(application.parentPhoneNumber), | ||
| educationName, | ||
| tuple.get(profile.schoolInfo.schoolName), | ||
| gradeName, | ||
| lunchName, | ||
| subjectNames, | ||
| tuple.get(exam.schoolName), | ||
| tuple.get(exam.examDate), | ||
| tuple.get(examTicketImage.fileName), | ||
| tuple.get(payment.paymentStatus), | ||
| tuple.get(payment.paymentMethod), | ||
| appliedAt | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing exam.lunchName in query selection
The mapToExcel method accesses exam.lunchName but it's not selected in the base query.
Add exam.lunchName to the select clause in baseQuery:
.select(
examApplication.id,
payment.paymentKey,
examApplication.examNumber,
profile.userName,
profile.gender,
profile.birth,
profile.phoneNumber,
application.parentPhoneNumber,
profile.education,
profile.schoolInfo.schoolName,
profile.grade,
exam.schoolName,
exam.examDate,
+ exam.lunchName,
examApplication.isLunchChecked,
examTicketImage.s3Key,
examTicketImage.fileName,
payment.paymentStatus,
payment.paymentMethod,
application.createdAt
)🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
around lines 217 to 249, the mapToExcel method uses exam.lunchName but this
field is not included in the base query's select clause. To fix this, add
exam.lunchName to the select clause of the baseQuery so that it is fetched and
available in the tuple passed to mapToExcel.
|
|
||
| @Override | ||
| public Page<RefundListResponse> searchAllRefunds(Pageable pageable) { | ||
| long total = baseQuery().fetch().size(); |
There was a problem hiding this comment.
Inefficient total count calculation
The current implementation fetches all records just to count them, which is inefficient for large datasets.
Use a dedicated count query instead:
- long total = baseQuery().fetch().size();
+ long total = baseQuery().fetchCount();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| long total = baseQuery().fetch().size(); | |
| long total = baseQuery().fetchCount(); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java
at line 36, replace the current total count calculation that fetches all records
and then counts them with a more efficient dedicated count query method. Use a
query that directly returns the count of records without fetching the entire
dataset to improve performance on large data sets.
| private JPAQuery<Tuple> baseQuery() { | ||
| return queryFactory | ||
| .select( | ||
| refund.id, | ||
| examApplication.examNumber, | ||
| profile.userName, | ||
| profile.phoneNumber, | ||
| refund.createdAt, | ||
| refund.agreedAt, | ||
| // payment.paymentMethod, | ||
| refund.reason | ||
| ) | ||
| .from(refund) | ||
| .leftJoin(examApplication).on(refund.examApplicationId.eq(examApplication.id)) | ||
| .leftJoin(application).on(examApplication.applicationId.eq(application.id)) | ||
| .leftJoin(profile).on(profile.userId.eq(application.userId)); | ||
| //TODO: payment method join | ||
| } |
There was a problem hiding this comment.
Missing payment join causes NullPointerException
The query doesn't join with the payment table, but mapToResponse attempts to access payment.paymentMethod from the tuple, which will result in a NullPointerException.
Either add the payment join or remove the payment method from the select and response:
.from(refund)
.leftJoin(examApplication).on(refund.examApplicationId.eq(examApplication.id))
.leftJoin(application).on(examApplication.applicationId.eq(application.id))
- .leftJoin(profile).on(profile.userId.eq(application.userId));
- //TODO: payment method join
+ .leftJoin(profile).on(profile.userId.eq(application.userId))
+ .leftJoin(payment).on(payment.applicationId.eq(application.id));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private JPAQuery<Tuple> baseQuery() { | |
| return queryFactory | |
| .select( | |
| refund.id, | |
| examApplication.examNumber, | |
| profile.userName, | |
| profile.phoneNumber, | |
| refund.createdAt, | |
| refund.agreedAt, | |
| // payment.paymentMethod, | |
| refund.reason | |
| ) | |
| .from(refund) | |
| .leftJoin(examApplication).on(refund.examApplicationId.eq(examApplication.id)) | |
| .leftJoin(application).on(examApplication.applicationId.eq(application.id)) | |
| .leftJoin(profile).on(profile.userId.eq(application.userId)); | |
| //TODO: payment method join | |
| } | |
| private JPAQuery<Tuple> baseQuery() { | |
| return queryFactory | |
| .select( | |
| refund.id, | |
| examApplication.examNumber, | |
| profile.userName, | |
| profile.phoneNumber, | |
| refund.createdAt, | |
| refund.agreedAt, | |
| // payment.paymentMethod, | |
| refund.reason | |
| ) | |
| .from(refund) | |
| .leftJoin(examApplication).on(refund.examApplicationId.eq(examApplication.id)) | |
| .leftJoin(application).on(examApplication.applicationId.eq(application.id)) | |
| .leftJoin(profile).on(profile.userId.eq(application.userId)) | |
| .leftJoin(payment).on(payment.applicationId.eq(application.id)); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java
between lines 50 and 67, the query selects paymentMethod but does not join the
payment table, causing a NullPointerException when accessing paymentMethod in
mapToResponse. Fix this by adding a left join to the payment table on the
appropriate key and include payment.paymentMethod in the select clause, or
alternatively remove payment.paymentMethod from both the select clause and the
response mapping to avoid referencing a missing join.
| private RefundListResponse mapToResponse(Tuple tuple) { | ||
| PaymentMethod paymentMethod = tuple.get(payment.paymentMethod); | ||
| return new RefundListResponse( | ||
| tuple.get(refund.id), | ||
| tuple.get(examApplication.examNumber), | ||
| tuple.get(profile.userName), | ||
| tuple.get(profile.phoneNumber), | ||
| formatDate(tuple.get(refund.createdAt)), | ||
| formatDate(tuple.get(refund.agreedAt)), | ||
| paymentMethod != null ? paymentMethod.getName() : "N/A", | ||
| tuple.get(refund.reason) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Remove unnecessary payment method retrieval
Since payment.paymentMethod is not selected in the query, this line will always return null or throw an exception.
Remove the unnecessary retrieval and directly use "N/A":
private RefundListResponse mapToResponse(Tuple tuple) {
- PaymentMethod paymentMethod = tuple.get(payment.paymentMethod);
return new RefundListResponse(
tuple.get(refund.id),
tuple.get(examApplication.examNumber),
tuple.get(profile.userName),
tuple.get(profile.phoneNumber),
formatDate(tuple.get(refund.createdAt)),
formatDate(tuple.get(refund.agreedAt)),
- paymentMethod != null ? paymentMethod.getName() : "N/A",
+ "N/A", // TODO: Implement payment method retrieval
tuple.get(refund.reason)
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private RefundListResponse mapToResponse(Tuple tuple) { | |
| PaymentMethod paymentMethod = tuple.get(payment.paymentMethod); | |
| return new RefundListResponse( | |
| tuple.get(refund.id), | |
| tuple.get(examApplication.examNumber), | |
| tuple.get(profile.userName), | |
| tuple.get(profile.phoneNumber), | |
| formatDate(tuple.get(refund.createdAt)), | |
| formatDate(tuple.get(refund.agreedAt)), | |
| paymentMethod != null ? paymentMethod.getName() : "N/A", | |
| tuple.get(refund.reason) | |
| ); | |
| } | |
| private RefundListResponse mapToResponse(Tuple tuple) { | |
| return new RefundListResponse( | |
| tuple.get(refund.id), | |
| tuple.get(examApplication.examNumber), | |
| tuple.get(profile.userName), | |
| tuple.get(profile.phoneNumber), | |
| formatDate(tuple.get(refund.createdAt)), | |
| formatDate(tuple.get(refund.agreedAt)), | |
| "N/A", // TODO: Implement payment method retrieval | |
| tuple.get(refund.reason) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java
between lines 69 and 81, the code attempts to retrieve payment.paymentMethod
from the tuple, but this field is not selected in the query, causing it to
always be null or throw an exception. Remove the retrieval of paymentMethod from
the tuple and replace the conditional expression with a direct use of the string
"N/A" in the RefundListResponse constructor.
| public ExamApplicationJpaEntity(Long applicationId, Long examId, boolean isLunchChecked, | ||
| String examNumber) { | ||
| this.applicationId = applicationId; | ||
| this.examId = examId; | ||
| this.lunchId = lunchId; | ||
| this.isLunchChecked = isLunchChecked; | ||
| this.examNumber = examNumber; | ||
| } |
There was a problem hiding this comment.
Fix constructor parameter inconsistency.
The constructor accepts an examNumber parameter, but the static create method on lines 45-52 doesn't provide this parameter, potentially leaving examNumber as null for entities created through the factory method.
Consider one of these approaches:
- Add
examNumberparameter to thecreatemethod - Make
examNumberoptional in the constructor and set a default value - Remove
examNumberfrom the constructor if it's set separately viagrantExamNumber
public static ExamApplicationJpaEntity create(Long applicationId, Long examId,
- boolean isLunchChecked) {
+ boolean isLunchChecked, String examNumber) {
return ExamApplicationJpaEntity.builder()
.applicationId(applicationId)
.examId(examId)
.isLunchChecked(isLunchChecked)
+ .examNumber(examNumber)
.build();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public ExamApplicationJpaEntity(Long applicationId, Long examId, boolean isLunchChecked, | |
| String examNumber) { | |
| this.applicationId = applicationId; | |
| this.examId = examId; | |
| this.lunchId = lunchId; | |
| this.isLunchChecked = isLunchChecked; | |
| this.examNumber = examNumber; | |
| } | |
| public static ExamApplicationJpaEntity create(Long applicationId, | |
| Long examId, | |
| boolean isLunchChecked, | |
| String examNumber) { | |
| return ExamApplicationJpaEntity.builder() | |
| .applicationId(applicationId) | |
| .examId(examId) | |
| .isLunchChecked(isLunchChecked) | |
| .examNumber(examNumber) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java
around lines 37 to 43, the constructor requires an examNumber parameter, but the
static create method does not supply it, causing examNumber to be null for
created entities. To fix this, either add an examNumber parameter to the create
method and pass it to the constructor, or modify the constructor to make
examNumber optional with a default value, or remove examNumber from the
constructor entirely if it is intended to be set later via grantExamNumber.
| boolean isLunchChecked = (exam.getLunchPrice() != null) && random.nextBoolean(); | ||
|
|
||
| totalAmount += BASE_EXAM_FEE; | ||
|
|
||
| ExamApplicationJpaEntity examApplication = examApplicationRepository.save( | ||
| ExamApplicationJpaEntity.builder() | ||
| .applicationId(application.getId()) | ||
| .examId(exam.getId()) | ||
| .examNumber(String.format("MOSU-%d-%d", application.getId(), | ||
| exam.getId())) | ||
| .isLunchChecked( | ||
| ((j % 2 == 0) ? true : false) | ||
| ) | ||
| .build() |
There was a problem hiding this comment.
Redundant lunch selection logic
The isLunchChecked value is set twice with different logic - first based on lunch availability and random selection, then overridden with index-based alternation.
Remove the redundant assignment:
boolean isLunchChecked = (exam.getLunchPrice() != null) && random.nextBoolean();
totalAmount += BASE_EXAM_FEE;
ExamApplicationJpaEntity examApplication = examApplicationRepository.save(
ExamApplicationJpaEntity.builder()
.applicationId(application.getId())
.examId(exam.getId())
.examNumber(String.format("MOSU-%d-%d", application.getId(),
exam.getId()))
- .isLunchChecked(
- ((j % 2 == 0) ? true : false)
- )
+ .isLunchChecked(isLunchChecked)
.build()
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| boolean isLunchChecked = (exam.getLunchPrice() != null) && random.nextBoolean(); | |
| totalAmount += BASE_EXAM_FEE; | |
| ExamApplicationJpaEntity examApplication = examApplicationRepository.save( | |
| ExamApplicationJpaEntity.builder() | |
| .applicationId(application.getId()) | |
| .examId(exam.getId()) | |
| .examNumber(String.format("MOSU-%d-%d", application.getId(), | |
| exam.getId())) | |
| .isLunchChecked( | |
| ((j % 2 == 0) ? true : false) | |
| ) | |
| .build() | |
| boolean isLunchChecked = (exam.getLunchPrice() != null) && random.nextBoolean(); | |
| totalAmount += BASE_EXAM_FEE; | |
| ExamApplicationJpaEntity examApplication = examApplicationRepository.save( | |
| ExamApplicationJpaEntity.builder() | |
| .applicationId(application.getId()) | |
| .examId(exam.getId()) | |
| .examNumber(String.format("MOSU-%d-%d", application.getId(), | |
| exam.getId())) | |
| .isLunchChecked(isLunchChecked) | |
| .build() | |
| ); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java
around lines 214 to 227, the isLunchChecked variable is assigned twice with
conflicting logic—first using lunch availability and randomness, then
overwritten by an index-based boolean. Remove the redundant assignment inside
the builder where isLunchChecked is set using the index check, and instead use
the previously computed isLunchChecked value to ensure consistent and intended
lunch selection logic.
| @Operation(summary = "환불 신청 수 조회 (구현중)", description = "환불 신청 수 조회 입니다") | ||
| @ApiResponses(value = { | ||
| @ApiResponse(responseCode = "200", description = "환불 신청 수 조회 성공") | ||
| }) | ||
| public ResponseEntity<ApiResponseWrapper<Page<RefundListResponse>>> getRefundCounts( | ||
| @PageableDefault(size = 15) Pageable pageable | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Complete the implementation of getRefundCounts
The endpoint is marked as "구현중" (under development). Ensure this is implemented before merging.
🏁 Script executed:
#!/bin/bash
# Check if the getRefundCounts method is implemented in AdminController
ast-grep --pattern 'public ResponseEntity<ApiResponseWrapper<Page<RefundListResponse>>> getRefundCounts($_) {
$$$
}'Length of output: 120
🏁 Script executed:
#!/bin/bash
# Search for any references or implementations of getRefundCounts across the codebase
rg -n "getRefundCounts(" --color neverLength of output: 123
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of getRefundCounts across the codebase to verify its implementation
rg -F -n "getRefundCounts" --color neverLength of output: 379
🏁 Script executed:
#!/bin/bash
# Show context around getRefundCounts in AdminController.java to confirm its implementation status
rg -n -C5 "getRefundCounts" src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.javaLength of output: 645
Implement the getRefundCounts endpoint before merging
The getRefundCounts method in AdminController currently returns a null payload and comments out the service call. Please replace the stub with a real implementation:
• File: src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java
Lines 103–108
Suggested diff:
@@ -103,7 +103,7 @@ public ResponseEntity<ApiResponseWrapper<Page<RefundListResponse>>> getRefundCoun
-// Page<RefundListResponse> result = adminService.getRefunds(pageable);
- return ResponseEntity.ok(
- ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", null));
+ Page<RefundListResponse> result = adminService.getRefunds(pageable);
+ return ResponseEntity.ok(
+ ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result));Additionally, update the OpenAPI docs in AdminControllerDocs.java to remove “(구현중)” once implemented.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java
lines 103 to 108, replace the stub implementation of getRefundCounts with a real
service call that fetches the refund counts and returns them in the
ResponseEntity. Then, in
src/main/java/life/mosu/mosuserver/presentation/admin/AdminControllerDocs.java
lines 77 to 83, update the @Operation summary to remove “(구현중)” to reflect that
the endpoint is now implemented.
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationListResponse.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java (2)
56-78: Pagination total count is still incorrect.This is the same issue previously identified - the total count uses
content.size()instead of the actual database count.
217-249: Missing exam.lunchName in query selection - same issue as before.The
mapToExcelmethod accessesexam.lunchNameon line 222, but this field is still not included in thebaseQueryselect clause.
🧹 Nitpick comments (1)
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java (1)
94-117: Consider using inner joins instead of right joins.The query logic is correct, but using
rightJoinoperations is unconventional for this aggregation. Consider usinginnerJoinsince you're filtering on conditions that require all entities to exist.- .rightJoin(exam).on(examApplication.examId.eq(exam.id)) - .rightJoin(application).on(application.id.eq(examApplication.applicationId)) - .rightJoin(payment).on(application.id.eq(payment.applicationId)) + .innerJoin(exam).on(examApplication.examId.eq(exam.id)) + .innerJoin(application).on(application.id.eq(examApplication.applicationId)) + .innerJoin(payment).on(application.id.eq(payment.applicationId))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java(1 hunks)
🔇 Additional comments (1)
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java (1)
142-149: Join conditions look correct.The payment join condition has been properly implemented using
application.id, which correctly links payments to applications.
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Style/Documentation