MOSU-39 feat: 신청 관련 기능 수정 및 관리자 신청 조회 관련 기능 구현#60
Conversation
WalkthroughThis update introduces new administrative features for managing and exporting application data, including endpoints for retrieving school lunch counts and application lists with filtering and Excel export. It refactors DTOs and domain models to support additional fields and validation, improves phone number validation, and changes user identification in controllers to use security context rather than request parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as AdminController
participant Service as AdminService
participant Repo as ApplicationQueryRepositoryImpl
participant Excel as ExcelExport
participant DB as Database
Admin->>Service: getLunchCounts()
Service->>Repo: searchAllSchoolLunches()
Repo->>DB: Query lunch counts grouped by school
DB-->>Repo: Lunch count data
Repo-->>Service: List<SchoolLunchResponse>
Service-->>Admin: ApiResponseWrapper<List<SchoolLunchResponse>>
Admin->>Service: getApplications(filter, pageable)
Service->>Repo: searchAllApplications(filter, pageable)
Repo->>DB: Query applications with filters and pagination
DB-->>Repo: Application data
Repo-->>Service: Page<ApplicationListResponse>
Service-->>Admin: ApiResponseWrapper<Page<ApplicationListResponse>>
Admin->>Service: getApplicationExcelData()
Service->>Repo: searchAllApplicationsForExcel()
Repo->>DB: Query all applications for export
DB-->>Repo: Application data
Repo-->>Service: List<ApplicationExcelDto>
Service->>Excel: Generate Excel file
Excel-->>Admin: Excel file response
Poem
✨ Finishing Touches
🪧 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 @chominju02, 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 significantly expands the administrative capabilities for managing applications, introduces new data fields for applications, and refines existing user-facing application and profile features. Key changes include the implementation of new admin API endpoints for application inquiry and export, enhanced application submission logic to capture more detailed school information, and a refactoring of user ID handling across several controllers for improved consistency and security.
Highlights
- New Admin Application Management Features: Implemented comprehensive administrator functionalities for application management, including paginated listing of applications, Excel export of application data, and a new endpoint to query school-specific lunch application counts. All new and existing admin endpoints are secured with
@PreAuthorize("isAuthenticated() and hasRole('ADMIN')"). - Enhanced Application Submission Logic: The application submission process (
ApplicationService.apply) now incorporates the guardian's phone number and fetches school address details from theSchoolJpaRepositoryto ensure more complete and accurate application records. Validation for application school requests has also been refined. - Refactored User ID Handling and Authorization: Standardized the extraction of
userIdin various controllers (ApplicationController,ApplicationSchoolController,ProfileController) by replacing@RequestParam Long userIdwith a custom@UserIdannotation. This improves consistency and potentially simplifies authentication integration. User-facing application endpoints are now explicitly secured with@PreAuthorize("isAuthenticated() and hasRole('USER')"). - Improved Data Representation and Validation: Enums for
Education,Gender, andGradenow include descriptive names for better display. Phone number validation has been broadened to support more formats using a custom@PhoneNumberPatternannotation.ObjectMapperconfiguration was updated to correctly handle Java 8 Date/Time API objects, and subject lists in various DTOs are now represented asSet<String>for clearer presentation.
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 is currently in preview and 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 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
The pull request includes several new features related to application management and admin functionalities. It also incorporates modifications based on previous reviews and applies @UserId annotation and phone number validation. The changes include new DTOs, repository methods, and controller endpoints for managing applications, school lunches, and generating Excel reports. Security annotations have been added to restrict access to admin-related endpoints.
| public enum Education { | ||
| ENROLLED, // 재학생 | ||
| GRADUATED // 졸업생 | ||
| ENROLLED("재학생"), | ||
| GRADUATED("졸업생"); |
There was a problem hiding this comment.
Consider adding a JavaDoc comment to explain what this enum represents.
/**
* Represents the education level of a student.
*/
@Getter
@RequiredArgsConstructor
public enum Education {| @Getter | ||
| @RequiredArgsConstructor | ||
| public enum Gender { |
There was a problem hiding this comment.
Consider adding a JavaDoc comment to explain what this enum represents.
/**
* Represents the gender of a person.
*/
@Getter
@RequiredArgsConstructor
public enum Gender {| @Getter | ||
| @RequiredArgsConstructor | ||
| public enum Grade { |
There was a problem hiding this comment.
Consider adding a JavaDoc comment to explain what this enum represents.
/**
* Represents the grade level of a student.
*/
@Getter
@RequiredArgsConstructor
public enum Grade {| public ObjectMapper mapper() { | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| objectMapper.registerModule(new JavaTimeModule()); | ||
| objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); |
There was a problem hiding this comment.
Consider extracting the object mapper configuration to a separate method for better readability and maintainability.
@Bean
public ObjectMapper mapper() {
ObjectMapper objectMapper = createObjectMapper();
return objectMapper;
}
private ObjectMapper createObjectMapper() {
ObjectMapper objectMapper = new ObjectMapper();
objectMapper.registerModule(new JavaTimeModule());
objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
return objectMapper;
}| @JsonFormat(pattern = "yyyy-MM-dd") | ||
| @NotNull(message = "생년월일은 필수입니다.") | ||
| LocalDate birth, |
There was a problem hiding this comment.
Consider adding a description to the JsonFormat annotation to explain the date format being used.
| @JsonFormat(pattern = "yyyy-MM-dd") | |
| @NotNull(message = "생년월일은 필수입니다.") | |
| LocalDate birth, | |
| @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd", timezone = "Asia/Seoul") | |
| @NotNull(message = "생년월일은 필수입니다.") | |
| LocalDate birth, |
| ) { | ||
| ApplicationSchoolResponse response = applicationSchoolService.updateSubjects(userId, applicationSchoolId, request); | ||
| ApplicationSchoolResponse response = applicationSchoolService.updateSubjects( | ||
| applicationSchoolId, request); |
There was a problem hiding this comment.
The updateSubjects method in ApplicationSchoolService does not use the userId parameter. Remove the parameter from the controller method signature to avoid confusion.
ApplicationSchoolResponse response = applicationSchoolService.updateSubjects(
applicationSchoolId, request);![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java (1)
47-63: Add error handling for Excel file generation.While the Excel download implementation is functional, consider adding try-catch blocks to handle potential errors during file generation and provide appropriate error responses to clients.
@GetMapping("/excel/students") @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") public void downloadStudentInfo( HttpServletResponse response) throws IOException { - String fileName = URLEncoder.encode("학생정보목록.xlsx", StandardCharsets.UTF_8) - .replaceAll("\\+", "%20"); - - response.setContentType( - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"); - response.setHeader("Content-Disposition", "attachment; filename*=UTF-8''" + fileName); - - List<StudentExcelDto> data = adminService.getStudentExcelData(); - SimpleExcelFile<StudentExcelDto> excelFile = new SimpleExcelFile<>(data, - StudentExcelDto.class); - - excelFile.write(response.getOutputStream()); + try { + String fileName = URLEncoder.encode("학생정보목록.xlsx", StandardCharsets.UTF_8) + .replaceAll("\\+", "%20"); + + response.setContentType( + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"); + response.setHeader("Content-Disposition", "attachment; filename*=UTF-8''" + fileName); + + List<StudentExcelDto> data = adminService.getStudentExcelData(); + SimpleExcelFile<StudentExcelDto> excelFile = new SimpleExcelFile<>(data, + StudentExcelDto.class); + + excelFile.write(response.getOutputStream()); + } catch (Exception e) { + log.error("Failed to generate student Excel file", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + response.getWriter().write("Failed to generate Excel file"); + } }
♻️ Duplicate comments (4)
src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java (1)
83-99: Apply the same error handling as suggested for the student Excel download.This method has the same error handling issue as the
downloadStudentInfomethod.src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java (1)
84-95: Same N+1 query issue for Excel export - potentially more critical.This method has the same N+1 query problem, but it's potentially more critical here since Excel exports typically involve larger datasets without pagination.
src/main/java/life/mosu/mosuserver/presentation/applicationschool/ApplicationSchoolController.java (2)
39-48: Remove unused @userid parameter.Similar to the previous method, the
@UserIdparameter is declared but not used in the service call.
50-58: Remove unused @userid parameter.The
@UserIdparameter is not used in the service call.
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/domain/user/UserJpaEntity.java (1)
34-36: Column length for Enum may be over-provisionedAdding
nullable = falseis spot-on.
However, specifyinglength = 50on anEnumType.STRINGcolumn is usually unnecessary—JPA will size the column to the longest enum constant name (often far below 50). Consider trimming this to a tighter bound (e.g.,length = 20) or omitting it altogether to keep DDL concise.- @Column(name = "user_role", nullable = false, length = 50) + @Column(name = "user_role", nullable = false, length = 20) // or remove `length`src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java (1)
37-45: Security annotation is correct but consider using method-level security configuration.The security annotation correctly enforces admin role authentication. For consistency and to avoid repetition, consider using a class-level
@PreAuthorizeannotation or creating a custom security annotation.src/main/java/life/mosu/mosuserver/presentation/applicationschool/ApplicationSchoolController.java (1)
29-37: Good refactoring to use @userid annotation.The removal of explicit
userIdparameter from the service call aligns with the security context-based user identification pattern. The@UserIdparameter is still declared but not used - consider removing it if it's not needed.@DeleteMapping("/{applicationSchoolId}/cancel") public ApiResponseWrapper<Void> cancelApplicationSchool( @PathVariable("applicationSchoolId") Long applicationSchoolId, - @UserId Long userId, @RequestBody RefundRequest request ) { applicationSchoolService.cancelApplicationSchool(applicationSchoolId, request); return ApiResponseWrapper.success(HttpStatus.OK, "신청 학교 취소 및 환불 처리 완료"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.gitignore(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/AdminService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(4 hunks)src/main/java/life/mosu/mosuserver/application/applicationschool/ApplicationSchoolService.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/application/ApplicationJpaEntity.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/profile/Education.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/profile/Gender.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/profile/Grade.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/school/SchoolJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/user/UserJpaEntity.java(2 hunks)src/main/java/life/mosu/mosuserver/global/annotation/PhoneNumberPattern.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/ObjectMapperConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java(5 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/SchoolLunchResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationSchoolRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationSchoolResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/applicationschool/ApplicationSchoolController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/applicationschool/dto/AdmissionTicketResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/dto/EditProfileRequest.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/dto/ProfileRequest.java(2 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/life/mosu/mosuserver/domain/profile/Grade.java (1)
src/main/java/life/mosu/mosuserver/domain/profile/SchoolInfoJpaVO.java (1)
Getter(10-25)
src/main/java/life/mosu/mosuserver/domain/profile/Education.java (2)
src/main/java/life/mosu/mosuserver/domain/school/AddressJpaVO.java (1)
Getter(10-34)src/main/java/life/mosu/mosuserver/domain/profile/SchoolInfoJpaVO.java (1)
Getter(10-25)
🔇 Additional comments (49)
src/main/java/life/mosu/mosuserver/domain/user/UserJpaEntity.java (1)
3-10: Explicit JPA imports – nice cleanupReplacing the wildcard with explicit imports makes the dependency list clearer and avoids accidental inclusion of unused classes. 👍
.gitignore (1)
40-40: LGTM! Good security practice.Adding
.env.localto the ignore rules prevents accidental commits of local environment configurations, which is a good security practice.src/main/java/life/mosu/mosuserver/domain/school/SchoolJpaRepository.java (1)
1-7: LGTM! Standard JPA repository implementation.This is a clean, minimal JPA repository following Spring Data JPA conventions. The interface provides all necessary CRUD operations for
SchoolJpaEntity.src/main/java/life/mosu/mosuserver/presentation/applicationschool/dto/AdmissionTicketResponse.java (2)
11-11: LGTM! Good architectural improvement.Changing the
subjectsfield fromSet<Subject>toSet<String>reduces coupling between the presentation layer and domain model, which is a good architectural practice.
15-31: LGTM! Factory method properly updated.The factory method signature and implementation are correctly updated to match the new
Set<String>type for subjects.src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationSchoolResponse.java (3)
18-18: LGTM! Consistent with architectural improvements.The field type change from
Set<Subject>toSet<String>is consistent with similar changes inAdmissionTicketResponseand maintains good architectural separation.
22-24: LGTM! Clean conversion logic.The conversion from
Subjectentities to their names using streams is clean and efficient. The use ofCollectors.toSet()ensures proper collection type conversion.
26-34: LGTM! Factory method properly updated.The factory method correctly uses the converted subject names and maintains consistency with the new field type.
src/main/java/life/mosu/mosuserver/domain/profile/Gender.java (2)
3-7: Well-structured enum enhancement for internationalization.The addition of Lombok annotations and Korean labels follows good practices for internationalization. The
@RequiredArgsConstructorwill generate a constructor that initializes thegenderNamefield properly.
9-12: Clean enum constant definitions with descriptive labels.The enum constants with Korean labels ("남자", "여자") provide clear localization support. The private final field ensures immutability and proper encapsulation.
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepository.java (1)
13-18: Well-defined repository interface methods.The method signatures are clear and follow Spring Data conventions. The separation of concerns between paginated results, Excel export, and school lunch data is appropriate.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationSchoolRequest.java (4)
3-4: Good addition of validation imports.The Jakarta validation constraints are properly imported for enhanced input validation.
17-31: Comprehensive validation annotations with Korean messages.The validation annotations are appropriately applied to required fields. The Korean validation messages are consistent with the internationalization approach seen in other parts of the codebase.
34-46: Improved method signature for better separation of concerns.The updated
toEntitymethod now acceptsAddressJavaVOas a parameter instead of deriving it internally. This improves separation of concerns and makes the method more explicit about its dependencies.
51-53: Minor formatting improvement in stream pipeline.The stream pipeline formatting is cleaner and more readable.
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (5)
3-3: Appropriate import for ArrayList initialization.The ArrayList import is correctly added for the new school entities list initialization.
13-15: Proper imports for enhanced school entity handling.The imports for
AddressJavaVO,SchoolJavaEntity, andSchoolJavaRepositoryare correctly added to support the enhanced application logic.
36-36: Appropriate dependency injection for school repository.The
SchoolJavaRepositorydependency is correctly injected to support school entity fetching during application processing.
42-42: Good initialization of school entities list.The explicit initialization of the
schoolEntitieslist is clear and appropriate for the enhanced logic.
60-69: Enhanced logic with proper error handling.The enhanced logic properly fetches school entities and handles the case where a school is not found by throwing a
CustomRuntimeException. The address extraction and passing to thetoEntitymethod is correctly implemented.src/main/java/life/mosu/mosuserver/presentation/admin/dto/SchoolLunchResponse.java (1)
3-8: LGTM! Well-designed response DTO.The record implementation is appropriate for this use case. Using a record for immutable response data is a good choice, and the field types are suitable for representing school lunch statistics.
src/main/java/life/mosu/mosuserver/global/config/ObjectMapperConfig.java (1)
13-18: Excellent configuration for consistent date/time handling.The addition of JavaTimeModule and disabling WRITE_DATES_AS_TIMESTAMPS ensures consistent ISO-8601 date formatting across the application. This is particularly important for the new admin DTOs that contain LocalDate and LocalDateTime fields.
src/main/java/life/mosu/mosuserver/presentation/profile/dto/EditProfileRequest.java (1)
25-25: Good refactoring to use custom validation annotation.The change from @pattern to @PhoneNumberPattern promotes consistency across the application and centralizes phone number validation logic. This makes the code more maintainable and reusable.
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationFilter.java (1)
7-14: Well-designed filter DTO with appropriate validation.The record implementation is clean and appropriate for filtering application data. The use of @PhoneNumberPattern maintains consistency with other DTOs, and the optional nature of all fields provides flexibility for admin queries.
src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java (1)
51-51: Excellent security improvement using @userid annotation.The change from @RequestParam to @userid annotation is a significant security improvement. It prevents user ID manipulation by extracting the user ID from the authenticated security context rather than relying on client-provided parameters.
src/main/java/life/mosu/mosuserver/domain/profile/Grade.java (1)
6-13: LGTM! Good enum enhancement with descriptive Korean names.The addition of Korean descriptive names to the Grade enum improves internationalization and display capabilities. The use of Lombok annotations follows best practices for reducing boilerplate code.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java (2)
11-12: LGTM! Good addition of guardian phone number with consistent validation.The addition of the
guardianPhoneNumberfield enhances the application data model, and both phone number fields now use the consistent@PhoneNumberPatternvalidation annotation.
20-20: LGTM! Properly integrated the new field in entity conversion.The
toEntitymethod correctly includes the newguardianPhoneNumberfield in the builder pattern.src/main/java/life/mosu/mosuserver/domain/profile/Education.java (1)
6-12: LGTM! Consistent enum enhancement pattern.The Education enum follows the same enhancement pattern as Grade.java, adding appropriate Korean descriptive names ("재학생" for enrolled, "졸업생" for graduated) with proper Lombok annotations.
src/main/java/life/mosu/mosuserver/presentation/profile/dto/ProfileRequest.java (2)
19-21: LGTM! Good addition of JSON date formatting.The
@JsonFormat(pattern = "yyyy-MM-dd")annotation on thebirthfield ensures consistent date formatting in JSON serialization/deserialization.
27-27: LGTM! Consistent phone number validation.Switching from direct
@Patternto the custom@PhoneNumberPatternannotation provides consistent phone number validation across the application.src/main/java/life/mosu/mosuserver/global/annotation/PhoneNumberPattern.java (2)
12-13: LGTM! Improved phone number validation pattern.The updated regex pattern
^01[016789]-\d{3,4}-\d{4}$now supports more Korean mobile prefixes (010, 011, 016, 017, 018, 019) while maintaining proper validation structure. The message formatting improvement with proper spacing is also good.
20-20: LGTM! Consistent message formatting.The default message method matches the annotation parameter message with proper spacing.
src/main/java/life/mosu/mosuserver/application/applicationschool/ApplicationSchoolService.java (3)
4-5: LGTM! Clean addition of necessary imports.The added imports for
SetandCollectorssupport the new subject name conversion logic appropriately.
107-109: Excellent refactoring of subject data representation.The conversion from
Subjectentities toSet<String>of subject names is well-implemented using streams. This simplifies the data structure for response DTOs and improves consistency across the application.
45-48: Proper removal of userId parameter aligns with security context usage.The method signature simplification by removing the
userIdparameter is consistent with the @userid annotation usage in the controller layer, indicating proper security context integration.src/main/java/life/mosu/mosuserver/application/admin/AdminService.java (2)
25-25: Good addition of repository dependency.The addition of
ApplicationQueryRepositoryImpldependency follows proper dependency injection patterns and supports the new administrative features.
35-47: Clean implementation of administrative service methods.The three new methods properly delegate to the repository layer, maintaining good separation of concerns:
getLunchCounts()for school lunch aggregationgetApplications()for paginated application queriesgetApplicationExcelData()for Excel export functionalityThe method signatures are clear and follow established patterns.
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationListResponse.java (1)
13-33: Well-designed comprehensive record for application data.The
ApplicationListResponserecord effectively captures all necessary application information with:
- Proper use of Java types (
LocalDate,LocalDateTime,Set<String>)- Complete coverage of personal, educational, payment, and examination details
- Consistent use of domain enums (
Gender,Education,Grade,PaymentStatus,PaymentMethod)- The
guardianPhoneNumberfield addition enhances data completenessThe immutable record approach is ideal for this data transfer object.
src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java (3)
30-30: Excellent addition of role-based security.The
@PreAuthorize("isAuthenticated() and hasRole('USER')")annotation properly secures the application endpoint, ensuring only authenticated users with the USER role can access the functionality.
32-32: Great refactoring to use security context for user identification.The replacement of
@RequestParam Long userIdwith@UserId Long userIdin both methods demonstrates proper security context usage, eliminating the need for explicit user ID parameters in requests.Also applies to: 42-42
41-41: Better method naming for clarity.The method rename from
getAlltogetApplicationsprovides better semantic clarity about what the endpoint returns.src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java (1)
15-67: Well-structured Excel DTO with appropriate type choices.The
ApplicationExcelDtoclass is well-designed for Excel export with:
- Proper use of
@ExcelColumnannotations with Korean headers for localization- Comprehensive field coverage matching the application data structure
- Appropriate use of String types for display fields (gender, educationLevel, grade, applicationDate) which is suitable for Excel formatting
- Consistent use of
Set<String>for subjects matching the broader refactoring- Final fields ensuring immutability
The class effectively bridges domain objects and Excel export requirements.
src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java (2)
65-72: Well-implemented endpoint for lunch counts.The implementation is clean and follows the established pattern with proper security and response wrapping.
73-82: Properly implemented paginated endpoint with filtering.Good use of
@Validannotation for request validation and proper pagination support.src/main/java/life/mosu/mosuserver/presentation/applicationschool/ApplicationSchoolController.java (1)
60-68: Correct usage of @userid annotation.This method correctly uses both the
userIdfrom the security context and theapplicationSchoolIdfrom the path variable.src/main/java/life/mosu/mosuserver/domain/application/ApplicationJpaEntity.java (3)
3-8: LGTM: Explicit imports improve code clarity.The change from wildcard imports to explicit Jakarta Persistence imports is a good practice that improves code readability and reduces potential naming conflicts.
29-30: LGTM: Proper JPA field mapping.The new
guardianPhoneNumberfield is properly mapped with the correct column name and appropriate data type. The field placement follows logical ordering within the entity.
44-48: LGTM: Constructor properly updated.The constructor has been correctly updated to include the new
guardianPhoneNumberparameter and field initialization. Parameter ordering is logical and consistent with the field declarations.Also applies to: 52-52
| host: ${REDIS_HOST} | ||
| port: ${REDIS_EXPORTER} | ||
| password: ${REDIS_PASSWORD} | ||
| port: ${VELKEY_PORT} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify environment variable consistency across deployment configurations.
The Redis port configuration was changed from ${REDIS_EXPORTER} to ${VELKEY_PORT}. Ensure that all deployment environments (development, staging, production) have the corresponding environment variable updated to prevent runtime configuration errors.
🏁 Script executed:
#!/bin/bash
# Description: Check if VELKEY_PORT environment variable is referenced elsewhere in the codebase
# Expected: Find other references to ensure consistency
echo "Searching for VELKEY_PORT references:"
rg "VELKEY_PORT" --type yaml --type properties --type sh --type dockerfile
echo -e "\nSearching for old REDIS_EXPORTER references that might need updating:"
rg "REDIS_EXPORTER" --type yaml --type properties --type sh --type dockerfileLength of output: 462
🏁 Script executed:
#!/bin/bash
# Re-check references for VELKEY_PORT and REDIS_EXPORTER without invalid --type filters
echo "Searching for VELKEY_PORT references across the repository:"
rg "VELKEY_PORT"
echo -e "\nSearching for REDIS_EXPORTER references across the repository:"
rg "REDIS_EXPORTER"Length of output: 1102
Update Docker-Compose configurations to use VELKEY_PORT
The change from ${REDIS_EXPORTER} to ${VELKEY_PORT} in application.yml needs to be mirrored in your Docker Compose files. Our search shows remaining ${REDIS_EXPORTER} mappings in both prod and local stacks:
-
docker-compose/docker-compose.prod.yml
– Replace
- ${REDIS_EXPORTER}:${REDIS_EXPORTER}
with
- ${VELKEY_PORT}:${VELKEY_PORT} -
docker-compose/docker-compose.local.yml
– Replace
- ${REDIS_EXPORTER}:${REDIS_EXPORTER}
with
- ${VELKEY_PORT}:${VELKEY_PORT}
Ensuring all references use VELKEY_PORT will keep your service port mappings and REDIS_ADDR settings consistent across environments.
🤖 Prompt for AI Agents
In docker-compose/docker-compose.prod.yml and
docker-compose/docker-compose.local.yml, replace all occurrences of the
environment variable mapping from ${REDIS_EXPORTER}:${REDIS_EXPORTER} to
${VELKEY_PORT}:${VELKEY_PORT} to align with the updated port configuration in
src/main/resources/application.yml line 44. This ensures consistent port
mappings and environment variable usage across your Docker Compose
configurations.
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationExcelDto; | ||
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationFilter; | ||
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationListResponse; | ||
| import life.mosu.mosuserver.presentation.admin.dto.SchoolLunchResponse; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Architecture concern: Domain layer importing presentation DTOs.
The domain layer is importing DTOs from the presentation layer (lines 4-7), which violates the typical layered architecture where domain should not depend on presentation. This creates a reverse dependency that makes the domain layer less reusable and harder to test.
Consider moving these DTOs to a shared package or creating separate domain-specific DTOs:
- import life.mosu.mosuserver.presentation.admin.dto.ApplicationExcelDto;
- import life.mosu.mosuserver.presentation.admin.dto.ApplicationFilter;
- import life.mosu.mosuserver.presentation.admin.dto.ApplicationListResponse;
- import life.mosu.mosuserver.presentation.admin.dto.SchoolLunchResponse;
+ import life.mosu.mosuserver.domain.admin.dto.ApplicationExcelDto;
+ import life.mosu.mosuserver.domain.admin.dto.ApplicationFilter;
+ import life.mosu.mosuserver.domain.admin.dto.ApplicationListResponse;
+ import life.mosu.mosuserver.domain.admin.dto.SchoolLunchResponse;📝 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.
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationExcelDto; | |
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationFilter; | |
| import life.mosu.mosuserver.presentation.admin.dto.ApplicationListResponse; | |
| import life.mosu.mosuserver.presentation.admin.dto.SchoolLunchResponse; | |
| import life.mosu.mosuserver.domain.admin.dto.ApplicationExcelDto; | |
| import life.mosu.mosuserver.domain.admin.dto.ApplicationFilter; | |
| import life.mosu.mosuserver.domain.admin.dto.ApplicationListResponse; | |
| import life.mosu.mosuserver.domain.admin.dto.SchoolLunchResponse; |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepository.java
around lines 4 to 7, the domain layer is importing DTO classes from the
presentation layer, which breaks the layered architecture by creating a reverse
dependency. To fix this, refactor by moving these DTO classes to a shared or
common package accessible by both domain and presentation layers, or create new
domain-specific DTOs within the domain layer itself, then update the imports
accordingly to remove any direct dependency on the presentation layer.
| List<ApplicationListResponse> content = query.fetch().stream() | ||
| .map(tuple -> { | ||
| Long appSchoolId = tuple.get(applicationSchool.id); | ||
| Set<Subject> subjects = appSchoolId != null | ||
| ? findSubjectsByApplicationSchoolId(appSchoolId) | ||
| : new HashSet<>(); | ||
| return mapToResponse(tuple, subjects); | ||
| }) | ||
| .toList(); |
There was a problem hiding this comment.
Performance: N+1 query problem when fetching subjects.
The current implementation executes a separate query for each application to fetch subjects, creating an N+1 query problem. This will significantly impact performance with large datasets.
Consider fetching all subjects in a single query and then mapping them:
// Fetch all application school IDs first
List<Long> appSchoolIds = query.fetch().stream()
.map(t -> t.get(applicationSchool.id))
.filter(Objects::nonNull)
.toList();
// Fetch all subjects for all application schools in one query
Map<Long, Set<Subject>> subjectsMap = fetchSubjectsForApplicationSchools(appSchoolIds);
// Then map the results
List<ApplicationListResponse> content = query.fetch().stream()
.map(tuple -> {
Long appSchoolId = tuple.get(applicationSchool.id);
Set<Subject> subjects = subjectsMap.getOrDefault(appSchoolId, new HashSet<>());
return mapToResponse(tuple, subjects);
})
.toList();🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
around lines 70 to 78, the current code fetches subjects individually for each
application, causing an N+1 query problem. To fix this, first collect all
application school IDs from the query results, then fetch all subjects for these
IDs in a single query and store them in a map. Finally, update the mapping logic
to retrieve subjects from this map instead of querying per application,
eliminating the N+1 queries and improving performance.
| public List<SchoolLunchResponse> searchAllSchoolLunches() { | ||
| return queryFactory | ||
| .select( | ||
| school.schoolName, | ||
| applicationSchool.lunch.count() | ||
| ) | ||
| .from(applicationSchool) | ||
| .rightJoin(school).on(applicationSchool.schoolId.eq(school.id)) | ||
| .where(applicationSchool.lunch.ne(Lunch.NONE)) | ||
| .groupBy(school.id, school.schoolName) | ||
| .fetch() | ||
| .stream() | ||
| .map(t -> new SchoolLunchResponse( | ||
| t.get(school.schoolName), | ||
| t.get(applicationSchool.lunch.count()) | ||
| )) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
Potential logic issue: RIGHT JOIN with WHERE clause may exclude schools.
The combination of rightJoin with where(applicationSchool.lunch.ne(Lunch.NONE)) might not work as intended. The WHERE clause will filter out schools that have no applications, defeating the purpose of the RIGHT JOIN.
If you want to include all schools (even those with 0 lunch applications), move the condition to the ON clause:
return queryFactory
.select(
school.schoolName,
applicationSchool.lunch.count()
)
.from(applicationSchool)
- .rightJoin(school).on(applicationSchool.schoolId.eq(school.id))
- .where(applicationSchool.lunch.ne(Lunch.NONE))
+ .rightJoin(school).on(
+ applicationSchool.schoolId.eq(school.id)
+ .and(applicationSchool.lunch.ne(Lunch.NONE))
+ )
.groupBy(school.id, school.schoolName)
.fetch()📝 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 List<SchoolLunchResponse> searchAllSchoolLunches() { | |
| return queryFactory | |
| .select( | |
| school.schoolName, | |
| applicationSchool.lunch.count() | |
| ) | |
| .from(applicationSchool) | |
| .rightJoin(school).on(applicationSchool.schoolId.eq(school.id)) | |
| .where(applicationSchool.lunch.ne(Lunch.NONE)) | |
| .groupBy(school.id, school.schoolName) | |
| .fetch() | |
| .stream() | |
| .map(t -> new SchoolLunchResponse( | |
| t.get(school.schoolName), | |
| t.get(applicationSchool.lunch.count()) | |
| )) | |
| .toList(); | |
| } | |
| public List<SchoolLunchResponse> searchAllSchoolLunches() { | |
| return queryFactory | |
| .select( | |
| school.schoolName, | |
| applicationSchool.lunch.count() | |
| ) | |
| .from(applicationSchool) | |
| .rightJoin(school).on( | |
| applicationSchool.schoolId.eq(school.id) | |
| .and(applicationSchool.lunch.ne(Lunch.NONE)) | |
| ) | |
| .groupBy(school.id, school.schoolName) | |
| .fetch() | |
| .stream() | |
| .map(t -> new SchoolLunchResponse( | |
| t.get(school.schoolName), | |
| t.get(applicationSchool.lunch.count()) | |
| )) | |
| .toList(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
around lines 98 to 115, the use of rightJoin combined with a where clause
filtering on applicationSchool.lunch excludes schools without lunch
applications, negating the right join effect. To fix this, move the lunch
condition from the where clause into the on clause of the join so that all
schools are included and the lunch filter applies only to joined records.
| 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(applicationSchool.id); | ||
| Set<Subject> subjects = appSchoolId != null | ||
| ? findSubjectsByApplicationSchoolId(appSchoolId) | ||
| : new HashSet<>(); | ||
| return mapToResponse(tuple, subjects); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return new PageImpl<>(content, pageable, content.size()); | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect pagination implementation will cause data inconsistency.
The pagination implementation has a critical issue. You're using content.size() as the total count, which will always return the page size or less, not the actual total count of all records. This will break pagination in the UI.
@Override
public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter,
Pageable pageable) {
JPAQuery<Tuple> query = baseQuery()
.where(
buildNameCondition(filter.name()),
buildPhoneCondition(filter.phone())
)
+ ;
+
+ long total = query.fetchCount();
+
+ query = query
.offset(pageable.getOffset())
.limit(pageable.getPageSize());
List<ApplicationListResponse> content = query.fetch().stream()
.map(tuple -> {
Long appSchoolId = tuple.get(applicationSchool.id);
Set<Subject> subjects = appSchoolId != null
? findSubjectsByApplicationSchoolId(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(applicationSchool.id); | |
| Set<Subject> subjects = appSchoolId != null | |
| ? findSubjectsByApplicationSchoolId(appSchoolId) | |
| : new HashSet<>(); | |
| return mapToResponse(tuple, subjects); | |
| }) | |
| .toList(); | |
| return new PageImpl<>(content, pageable, content.size()); | |
| } | |
| @Override | |
| public Page<ApplicationListResponse> searchAllApplications(ApplicationFilter filter, | |
| Pageable pageable) { | |
| // build base query and count total before pagination | |
| JPAQuery<Tuple> query = baseQuery() | |
| .where( | |
| buildNameCondition(filter.name()), | |
| buildPhoneCondition(filter.phone()) | |
| ); | |
| long total = query.fetchCount(); | |
| // apply pagination | |
| query = query | |
| .offset(pageable.getOffset()) | |
| .limit(pageable.getPageSize()); | |
| List<ApplicationListResponse> content = query.fetch().stream() | |
| .map(tuple -> { | |
| Long appSchoolId = tuple.get(applicationSchool.id); | |
| Set<Subject> subjects = appSchoolId != null | |
| ? findSubjectsByApplicationSchoolId(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 59 to 81, the pagination implementation incorrectly uses
content.size() as the total count, which only reflects the current page size and
not the total number of records. To fix this, add a separate count query that
calculates the total number of matching records without pagination limits, then
use this count value as the third argument in the PageImpl constructor instead
of content.size().
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores
.gitignoreto exclude additional environment files.