Feat/ticket: 수험번호 및 수험표 발급 기능 구현 #381
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds guest CSV import flow, exam ticket generation (including PDF creation), and exam number generation via cron. Introduces supporting DTOs, processors, repositories, schema migration, whitelisting, and utilities. Removes legacy ExamNumberGenerationService. Updates ApplicationContext mappings and several repository queries. Includes new Thymeleaf admin pages and PDFBox dependencies. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant C as AdminApplicationImportController
participant S as AdminApplicationImportService
participant R as CsvReader
participant P1 as GetApplicationGuestRequestStepProcessor
participant P2 as ApplyGuestStepProcessor
participant P3 as ChangeTestPaperCheckedStepProcessor
participant P4 as RegisterVirtualAccountStepProcessor
Admin->>C: POST /admin/applications/import/guests (file)
C->>S: importGuestApplications(file)
S->>R: read(file)
R-->>S: List<ApplicationCsvInfo>
loop For each row
S->>P1: process(csvInfo)
P1-->>S: ApplicationGuestRequest
S->>P2: process(request)
P2-->>S: applicationId
alt csvInfo.isTestPaperChecked
S->>P3: process(applicationId)
P3-->>S: examApplicationIds
end
S->>P4: process(applicationId)
P4-->>S: virtualAccountLogId
end
S-->>C: ImportResultDto
C-->>Admin: View "admin/import-result"
sequenceDiagram
actor Client
participant EC as ExamTicketController
participant ES as ExamTicketService
participant IP as InfoProcessor (Partner/Member)
participant REP as Repositories/S3
participant GP as GenerateExamTicketProcessor
participant PDF as PDFBox
Client->>EC: GET /exam-ticket/(partner|member)/...
EC->>ES: get...ExamTicket(...)
ES->>IP: process(key)
IP->>REP: fetch projection, subjects, image URL
REP-->>IP: Data
IP-->>ES: ExamTicketIssueResponse
ES->>GP: process(issueResponse)
GP->>PDF: load template/font/image, draw text/image
PDF-->>GP: bytes
GP-->>ES: ExamTicketFileResponse
ES-->>EC: ExamTicketFileResponse
EC-->>Client: PDF bytes + headers
sequenceDiagram
participant Quartz as Quartz Scheduler
participant J as ExamNumberGenerationJob(R1/R2)
participant EX as ExamNumberGeneratorExecutorImpl
participant ER as ExamJpaRepository
participant AR as ExamApplicationJpaRepository
participant EC as ExamNumberCode/Util
participant EA as ExamApplication Entities
Quartz->>J: Trigger execute()
J->>EX: generate(examDate)
EX->>ER: findAllByExamDate(examDate)
ER-->>EX: exams
loop per exam
EX->>AR: findDoneAndSortByTestPaperGroup(examId, groups)
AR-->>EX: applications
EX->>EC: getRound/Area/School codes
EC-->>EX: codes
loop assign numbers
EX->>EC: formatExamNumber(...)
EC-->>EX: examNumber
EX->>EA: grantExamNumber(examNumber)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @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! 이 PR은 시험 신청 관리 시스템에 중요한 기능들을 도입합니다. 응시자에게 고유한 수험번호를 자동으로 부여하고, 개인화된 수험표를 PDF 형태로 발급할 수 있도록 하여 사용자 경험을 향상시킵니다. 또한, 관리자가 CSV 파일을 통해 대량의 게스트 신청 데이터를 효율적으로 등록할 수 있는 기능을 제공하여 운영 편의성을 크게 높였습니다. 이로써 시험 신청부터 수험표 발급까지의 전반적인 프로세스가 더욱 자동화되고 체계화되었습니다. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 수험번호 및 수험표 발급 기능과 CSV를 통한 신청 정보 일괄 등록 기능을 구현합니다. 전반적으로 기능 구현은 잘 이루어졌지만, 몇 가지 개선점을 제안합니다. CSV 가져오기 기능의 오류 처리 로직을 개선하여 실패 시에도 사용자에게 명확한 결과를 보여주도록 수정이 필요합니다. 또한, NullPointerException 발생 가능성이 있는 부분을 방어하고, 일부 하드코딩된 값들을 상수로 추출하여 코드의 안정성과 유지보수성을 높이는 것이 좋겠습니다. 알림 전송 로직이 주석 처리되어 있는데, 의도된 것인지 확인이 필요해 보입니다. 마지막으로 사용하지 않는 코드와 주석 처리된 코드를 정리하여 코드베이스를 깔끔하게 유지하는 것을 권장합니다.
|
|
||
| LocalDate examDate = csvInfo.examDate(); | ||
| String schoolName = csvInfo.examSchool(); | ||
| ExamJpaEntity exam = examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName); |
There was a problem hiding this comment.
examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName) 메서드가 null을 반환할 경우, 다음 줄에서 exam.getId()를 호출할 때 NullPointerException이 발생할 수 있습니다. 데이터베이스에 해당 시험 정보가 없는 경우를 대비하여 null 체크 로직을 추가해야 합니다.
ExamJpaEntity exam = examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName);
if (exam == null) {
throw new IllegalArgumentException("해당 시험 정보가 존재하지 않습니다. examDate=" + examDate + ", schoolName=" + schoolName);
}| public void send(LunaNotifyEventRequest request) { | ||
| LunaNotifyRequest lunaRequest = createLunaNotifyRequest(request); | ||
|
|
||
| webClient.post() | ||
| .uri(properties.getApi().getBaseUrl()) | ||
| .bodyValue(lunaRequest) | ||
| .retrieve() | ||
| .bodyToMono(String.class) | ||
| .publishOn(Schedulers.boundedElastic()) | ||
| .doOnSuccess(response -> log.debug("알림톡 응답 성공")) | ||
| .doOnError(error -> log.error("알림톡 전송 실패", error)) | ||
| .subscribe(); | ||
| // LunaNotifyRequest lunaRequest = createLunaNotifyRequest(request); | ||
| // | ||
| // webClient.post() | ||
| // .uri(properties.getApi().getBaseUrl()) | ||
| // .bodyValue(lunaRequest) | ||
| // .retrieve() | ||
| // .bodyToMono(String.class) | ||
| // .publishOn(Schedulers.boundedElastic()) | ||
| // .doOnSuccess(response -> log.debug("알림톡 응답 성공")) | ||
| // .doOnError(error -> log.error("알림톡 전송 실패", error)) | ||
| // .subscribe(); | ||
| } |
There was a problem hiding this comment.
send 메서드의 내용 전체가 주석 처리되어 있어 알림톡 전송 기능이 비활성화된 것으로 보입니다. 개발 또는 테스트 목적으로 의도된 변경이라면 괜찮지만, 만약 실수로 주석 처리된 것이라면 프로덕션 환경에서 알림이 발송되지 않는 문제가 발생할 수 있습니다. 이 변경이 의도된 것인지 확인해주세요.
| @Transactional | ||
| public ImportResultDto importGuestApplications(MultipartFile file) { | ||
| List<ApplicationCsvInfo> rows = csvReader.read(file); | ||
|
|
||
| int total = rows.size(); | ||
| int success = 0; | ||
|
|
||
| int lineNo = 1; | ||
| for (ApplicationCsvInfo row : rows) { | ||
| lineNo++; | ||
| try { | ||
| processGuestRow(row); | ||
| success++; | ||
| } catch (Exception e) { | ||
| log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e); | ||
| throw new RuntimeException("CSV 일괄 처리 중 오류 발생"); | ||
| } | ||
| } | ||
|
|
||
| int fail = total - success; | ||
| log.info("CSV Import 완료 - 총 {}건, 성공 {}, 실패 {}", total, success, fail); | ||
| return new ImportResultDto(total, success, fail); | ||
| } |
There was a problem hiding this comment.
importGuestApplications 메서드의 오류 처리 방식에 개선이 필요해 보입니다. 현재 구조에서는 CSV 처리 중 오류가 발생하면 RuntimeException을 발생시켜 트랜잭션을 롤백합니다. 이는 데이터 정합성 측면에서는 좋은 방법이지만, 예외가 컨트롤러까지 전파되어 사용자에게는 일반적인 오류 페이지만 보이게 됩니다. import-result.html에 구현된 실패 결과 화면을 사용자에게 보여주려면 서비스 계층에서 예외를 던지는 대신, 트랜잭션 롤백을 수동으로 설정하고 실패 정보를 담은 ImportResultDto를 반환하는 방식이 더 적합합니다.
또한, Spring 환경에서는 jakarta.transaction.Transactional보다 Spring이 제공하는 @Transactional 어노테이션을 사용하는 것이 일관성 및 기능 활용(TransactionAspectSupport 등) 측면에서 더 좋습니다.
@Transactional
public ImportResultDto importGuestApplications(MultipartFile file) {
List<ApplicationCsvInfo> rows = csvReader.read(file);
int total = rows.size();
int lineNo = 1;
for (ApplicationCsvInfo row : rows) {
lineNo++;
try {
processGuestRow(row);
} catch (Exception e) {
log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e);
TransactionAspectSupport.currentTransactionStatus().setRollbackOnly();
log.info("CSV Import 실패로 인해 롤백됩니다.");
return new ImportResultDto(total, 0, total);
}
}
log.info("CSV Import 완료 - 총 {}건, 성공 {}, 실패 {}", total, total, 0);
return new ImportResultDto(total, total, 0);
}| return String.format( | ||
| "%d%02d%02d%04d", | ||
| roundCode, | ||
| areaCode, | ||
| schoolCode, | ||
| personalCode | ||
| ); |
There was a problem hiding this comment.
String.format에 사용된 포맷 문자열 "%d%02d%02d%04d"가 매직 스트링으로 사용되고 있습니다. 이 문자열을 상수로 추출하면 코드의 의도를 더 명확하게 하고 유지보수성을 높일 수 있습니다.
final String EXAM_NUMBER_FORMAT = "%d%02d%02d%04d";
return String.format(
EXAM_NUMBER_FORMAT,
roundCode,
areaCode,
schoolCode,
personalCode
);| exam.getExamDate(), | ||
| subjects, | ||
| lunchName | ||
| // examApp.getIsTestPaperChecked() |
There was a problem hiding this comment.
주석 처리된 코드가 남아있습니다. 사용하지 않는 코드는 가독성을 위해 제거하는 것이 좋습니다.
| String schoolName, | ||
| AddressJpaVO address, | ||
| Boolean isLunchChecked, | ||
| // Boolean isTeacherChecked, |
There was a problem hiding this comment.
주석 처리된 코드가 남아있습니다. 사용하지 않는 코드는 가독성을 위해 제거하는 것이 좋습니다.
| package life.mosu.mosuserver.global.support; | ||
|
|
||
| import java.time.LocalDate; | ||
|
|
||
| public interface NumberGeneratorWithDate { | ||
|
|
||
| String generate(LocalDate examDate); | ||
|
|
||
| } |
There was a problem hiding this comment.
새로 추가된 NumberGeneratorWithDate 인터페이스가 현재 PR의 변경사항 내에서 사용되지 않는 것으로 보입니다. 만약 향후 사용될 예정이 아니라면, 사용하지 않는 코드는 혼란을 줄 수 있으므로 제거하는 것이 좋습니다.
| if (values.length < 11) { | ||
| throw new IllegalArgumentException("CSV 행이 필수 컬럼 수(11개)보다 부족합니다. 실제 컬럼 수: " + values.length); | ||
| } |
There was a problem hiding this comment.
of(String[] values) 메서드에서 CSV 컬럼 수를 나타내는 11이 하드코딩되어 있습니다. 이 값을 private static final int REQUIRED_COLUMN_COUNT = 11;과 같이 상수로 추출하면 코드의 가독성과 유지보수성이 향상됩니다.
| LocalDate examDate, | ||
| Set<String> subjects, | ||
| String lunchName | ||
| // Boolean isTestPaperChecked |
There was a problem hiding this comment.
주석 처리된 코드가 남아있습니다. 사용하지 않는 코드는 가독성을 위해 제거하는 것이 좋습니다.
| AddressResponse address, | ||
| Set<String> subjects, | ||
| String lunchName, | ||
| // Boolean isTestPaperChecked, |
There was a problem hiding this comment.
주석 처리된 코드가 남아있습니다. 사용하지 않는 코드는 가독성을 위해 제거하는 것이 좋습니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (1)
43-47: Critical: Register method doesn't persist entities.The
registermethod is marked@Transactionalbut only converts the event to entities without saving them to the database. This will result in exam applications not being persisted.Apply this diff to save the entities:
@Transactional public List<ExamApplicationJpaEntity> register(RegisterExamApplicationEvent event) { List<ExamApplicationJpaEntity> examApplicationEntities = event.toEntity(); - return examApplicationEntities; + return examApplicationJpaRepository.saveAll(examApplicationEntities); }
🧹 Nitpick comments (26)
src/main/java/life/mosu/mosuserver/domain/examapplication/projection/ExamApplicationInfoProjection.java (1)
16-16: Remove the commented-out field.Leaving dead/commented code in a projection bloats the API surface with noise. If the field is not part of the contract anymore, please drop the comment outright; otherwise, reintroduce it properly.
src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java (1)
16-17: Remove lingering commented-out field wiring.The commented
isTestPaperCheckedfield and factory wiring add noise and invite drift. Either bring the field back with full support or drop the commented lines altogether to keep the DTO clean.Also applies to: 30-31, 43-44
src/main/java/life/mosu/mosuserver/infra/config/QuartzAutoRegisterConfig.java (1)
6-6: Consider using explicit imports instead of wildcard.The wildcard import
org.quartz.*makes it less clear which specific Quartz classes are being used. Explicit imports improve code readability and help avoid potential naming conflicts.Apply this diff to use explicit imports:
-import org.quartz.*; +import org.quartz.CronScheduleBuilder; +import org.quartz.Job; +import org.quartz.JobBuilder; +import org.quartz.JobDetail; +import org.quartz.Trigger; +import org.quartz.TriggerBuilder;src/main/java/life/mosu/mosuserver/global/util/PhoneNumberUtil.java (1)
27-32: LGTM! Consider adding documentation.The implementation correctly removes the prefix characters (G/g/U/u) added by
formatGuestPhoneNumberandformatPhoneNumber. The case-insensitive regex pattern and null handling are appropriate for a utility method likely used in data processing pipelines.Note: This method returns
nullfor null input, whereasformatPhoneNumberWithHyphenthrows an exception. This difference appears intentional for flexibility in data pipelines, but ensure this behavior aligns with how the method is used in the CSV import feature.Consider adding JavaDoc to document the method's purpose and behavior:
+ /** + * Removes the prefix (G/g/U/u) from a phone number. + * + * @param phoneNumber the phone number with prefix, or null + * @return the phone number without prefix, or null if input is null + */ public static String removePrefix(String phoneNumber) { if (phoneNumber == null) { return null; } return phoneNumber.replaceFirst("^[GgUu]", ""); }src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java (1)
25-26: Reconsider exception type for "not found" after creation.
IllegalArgumentExceptiontypically indicates invalid input. However, if an application cannot be found immediately after creation, this suggests a data consistency or concurrency issue, not invalid input.Consider using
IllegalStateExceptionor a custom exception to better convey the nature of the problem:- .orElseThrow(() -> new IllegalArgumentException("신청을 찾을 수 없습니다. id=" + applicationId)); + .orElseThrow(() -> new IllegalStateException("방금 생성된 신청을 찾을 수 없습니다. id=" + applicationId));This makes the exceptional condition clearer for debugging and monitoring.
src/main/java/life/mosu/mosuserver/application/examapplication/code/ExamNumberCode.java (5)
15-19: Consider externalizing hardcoded exam dates.The
roundCodeMapcontains hardcoded exam dates that will require code changes for future exam rounds. Consider:
- Loading these mappings from
application.ymlor a database table for easier updates without redeployment.- Adding validation to ensure exam dates are in the future when the application starts.
Example configuration approach in
application.yml:exam: rounds: - date: 2025-10-19 code: 1 - date: 2025-10-26 code: 2 - date: 2025-11-02 code: 3Then inject via
@ConfigurationProperties:@Component @ConfigurationProperties(prefix = "exam") @Getter @Setter public class ExamNumberCode { private List<RoundMapping> rounds; private Map<Area, Integer> areas; private Map<String, Integer> schools; public Integer getRoundCode(LocalDate examDate) { return rounds.stream() .filter(r -> r.getDate().equals(examDate)) .findFirst() .map(RoundMapping::getCode) .orElseThrow(() -> new IllegalArgumentException( "해당 시험일에 해당하는 코드가 존재하지 않습니다")); } @Getter @Setter public static class RoundMapping { private LocalDate date; private Integer code; } }
28-36: Prefer database or configuration for school codes.The
schoolCodeMaphardcodes school names and codes. As the number of schools grows beyond 10 entries,Map.of()will fail (it supports maximum 10 entries). Consider:
- Storing school codes in a database table for dynamic management.
- Using
@ConfigurationPropertieswith aMap<String, Integer>(no entry limit).- Providing an admin interface to manage school codes without code deployments.
12-12: Reconsider exposing internal maps via @Getter.The
@Getterannotation exposes the internal maps publicly, which violates encapsulation. WhileMap.of()returns immutable maps, exposing them still reveals internal implementation details.Remove
@Getterand rely solely on the lookup methods for access:@Component -@Getter public class ExamNumberCode {
38-54: Improve error messages for production observability.The error messages are in Korean and lack context about the invalid input, making debugging harder in production logs.
Apply this pattern to improve observability:
public Integer getRoundCode(LocalDate examDate) { return Optional.ofNullable(roundCodeMap.get(examDate)) .orElseThrow(() -> new IllegalArgumentException( - "해당 시험일에 해당하는 코드가 존재하지 않습니다")); + String.format("No round code found for exam date: %s. Available dates: %s", + examDate, roundCodeMap.keySet()))); } public Integer getAreaCode(Area area) { return Optional.ofNullable(areaCodeMap.get(area)) .orElseThrow(() -> new IllegalArgumentException( - "해당 지역에 해당하는 코드가 존재하지 않습니다")); + String.format("No area code found for area: %s. Available areas: %s", + area, areaCodeMap.keySet()))); } public Integer getSchoolCode(String schoolName) { return Optional.ofNullable(schoolCodeMap.get(schoolName)) .orElseThrow(() -> new IllegalArgumentException( - "해당 학교명에 해당하는 코드가 존재하지 않습니다")); + String.format("No school code found for school: '%s'. Available schools: %s", + schoolName, schoolCodeMap.keySet()))); }
11-13: Add startup validation for exam number code mappings.Without validation, invalid or incomplete mappings won't be detected until runtime when an exam number is generated, potentially causing failures during critical operations.
Add a
@PostConstructvalidation method:+import jakarta.annotation.PostConstruct; +import java.util.Arrays; +import java.util.EnumSet; + @Component @Getter public class ExamNumberCode { private final Map<LocalDate, Integer> roundCodeMap = Map.of( LocalDate.of(2025, 10, 19), 1, LocalDate.of(2025, 10, 26), 2, LocalDate.of(2025, 11, 2), 3 ); private final Map<Area, Integer> areaCodeMap = Map.of( Area.DAECHI, 1, Area.MOKDONG, 2, Area.NOWON, 3, Area.DAEGU, 4 ); private final Map<String, Integer> schoolCodeMap = Map.of( "대치중학교", 7, "개원중학교", 6, "문래중학교", 5, "목운중학교", 4, "신서중학교", 3, "온곡중학교", 2, "노변중학교", 1 ); + + @PostConstruct + public void validate() { + // Validate all Area enums have mappings + EnumSet<Area> unmappedAreas = EnumSet.complementOf( + EnumSet.copyOf(areaCodeMap.keySet())); + if (!unmappedAreas.isEmpty()) { + throw new IllegalStateException( + "Missing area code mappings for: " + unmappedAreas); + } + + // Validate round codes are not empty + if (roundCodeMap.isEmpty()) { + throw new IllegalStateException("Round code map cannot be empty"); + } + + // Validate school codes are not empty + if (schoolCodeMap.isEmpty()) { + throw new IllegalStateException("School code map cannot be empty"); + } + + log.info("ExamNumberCode validation passed. Rounds: {}, Areas: {}, Schools: {}", + roundCodeMap.size(), areaCodeMap.size(), schoolCodeMap.size()); + }src/main/resources/db/migration/V4__add_test_paper_checked.sql (2)
1-2: Consider NOT NULL constraint for boolean flag.The
test_paper_checkedcolumn is defined asNULLwith aDEFAULT b'0'. This creates ambiguity:
- If
NULLis allowed, what does it represent vs.b'0'(false)?- For a boolean flag, typically
NOT NULLis preferred to avoid tri-state logic.Unless there's a specific business need for
NULLto represent "unknown" or "not applicable", apply this diff:ALTER TABLE exam_application - ADD COLUMN test_paper_checked BIT(1) NULL DEFAULT b'0'; + ADD COLUMN test_paper_checked BIT(1) NOT NULL DEFAULT b'0';Also, ensure the file ends with a newline to comply with POSIX standards and avoid git diff warnings.
1-2: LGTM! Migration adds test_paper_checked column correctly.The migration appropriately adds a nullable
BIT(1)column with a default value ofb'0'(false), ensuring backward compatibility with existing records.Minor: Consider adding a newline at the end of the file for consistency with Unix conventions.
src/main/java/life/mosu/mosuserver/domain/virtualaccount/VirtualAccountLogJpaRepository.java (1)
11-12: Add composite index on (order_id, deposit_status) to virtual_account_log
Create a new migration under src/main/resources/db/migration that addsCREATE INDEX idx_va_log_order_deposit ON virtual_account_log(order_id, deposit_status);src/main/java/life/mosu/mosuserver/application/admin/processor/RegisterVirtualAccountStepProcessor.java (1)
18-25: Document the null parameters and auto-success behavior.The processor creates a
VirtualAccountLogJpaEntitywith several null fields (bankCode, accountNumber, accountOwner, depositName) and immediately marks the deposit as successful without actual payment verification. This pattern suggests these are pre-approved guest applications where payment is handled separately from the virtual account flow.Consider adding documentation to clarify:
- Why these fields are null (e.g., guest applications don't require actual virtual accounts)
- Why deposit is auto-marked successful (e.g., payment verification happens through a different channel for CSV imports)
Example documentation:
+ /** + * Registers a virtual account log entry for admin-imported guest applications. + * For guest imports via CSV, payment is verified externally, so we create a + * placeholder log entry with deposit pre-marked as successful. + * + * @param applicationId the application ID to register + * @return the saved virtual account log ID + */ @Override public Long process(Long applicationId) {src/main/java/life/mosu/mosuserver/domain/examapplication/entity/ExamApplicationJpaEntity.java (3)
3-3: Consider using explicit imports instead of wildcard.Wildcard imports can lead to namespace pollution and make it harder to track dependencies. While this is a minor style issue, explicit imports improve code clarity.
70-72: Consider renaming the setter for clarity.The method
setTestPaperChecked()acts as a flag-setter with no parameters, which is unconventional. While the implementation is correct for marking test papers as checked, a more descriptive name likemarkTestPaperAsChecked()orcheckTestPaper()would better convey the intent.Example refactor:
- public void setTestPaperChecked() { + public void markTestPaperAsChecked() { this.isTestPaperChecked = Boolean.TRUE; }
3-3: Consider avoiding wildcard imports.The wildcard import
jakarta.persistence.*was introduced, replacing the previous individual imports. While this is not a critical issue, explicit imports are generally preferred because they:
- Make dependencies clearer
- Avoid naming conflicts
- Help IDEs and tools with refactoring
If the project's coding guidelines permit wildcard imports, you can safely ignore this suggestion.
src/main/resources/templates/admin/import-result.html (2)
145-154: Remove or implement the commented-out error details section.Lines 145-154 contain commented-out code for displaying detailed error messages. If error detail display is planned for a future iteration, consider removing this dead code to keep the template clean, or uncomment and implement it if the functionality is needed now.
145-154: Remove commented-out error details section or add TODO.The error messages section is commented out (lines 145-154). If this feature is planned for future implementation, add a TODO comment explaining what's needed. Otherwise, remove the dead code to keep the template clean.
Example TODO:
+ <!-- TODO: Implement detailed error message display when error tracking is added to ImportResultDto --> <!-- <h4>오류 상세 내용 (롤백을 유발한 오류):</h4>-->src/main/java/life/mosu/mosuserver/presentation/admin/AdminApplicationImportController.java (1)
26-31: Add file validation for CSV uploads.The
importGuestsmethod accepts aMultipartFilewithout validation. Consider adding checks for:
- File size limits (prevent DoS via large files)
- File type/MIME type (ensure it's a CSV)
- File name validation (prevent path traversal)
Example validation:
@PostMapping("/guests") public String importGuests(@RequestParam("file") MultipartFile file, Model model) { + // Validate file + if (file.isEmpty()) { + model.addAttribute("error", "파일을 선택해주세요."); + return "admin/import"; + } + + // Check file size (e.g., 10MB limit) + if (file.getSize() > 10 * 1024 * 1024) { + model.addAttribute("error", "파일 크기는 10MB를 초과할 수 없습니다."); + return "admin/import"; + } + + // Validate content type + String contentType = file.getContentType(); + if (contentType == null || !contentType.equals("text/csv")) { + model.addAttribute("error", "CSV 파일만 업로드 가능합니다."); + return "admin/import"; + } + ImportResultDto result = adminApplicationImportService.importGuestApplications(file);Note: File size can also be configured globally via
spring.servlet.multipart.max-file-sizein application properties.src/main/java/life/mosu/mosuserver/application/admin/util/CsvReader.java (1)
43-45: Consider validating CSV structure.The reader splits each line by comma and passes the raw array to
ApplicationCsvInfo.of(values), but doesn't validate the expected number of columns. If a row has too few or too many columns, the error will only surface whenApplicationCsvInfo.of()tries to access array indices.Consider adding a column count check:
+ private static final int EXPECTED_COLUMN_COUNT = 8; // Update based on actual CSV structure + try { String[] values = line.split(CSV_DELIMITER, -1); + if (values.length != EXPECTED_COLUMN_COUNT) { + throw new IllegalArgumentException( + String.format("잘못된 열 수: 예상 %d개, 실제 %d개", EXPECTED_COLUMN_COUNT, values.length) + ); + } results.add(ApplicationCsvInfo.of(values));src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java (1)
66-66: Consider caching the loaded font to improve performance.Loading the font on every request via
PDType0Font.load(doc, new ByteArrayInputStream(fontBytes))may incur unnecessary overhead. While the byte array is cached, PDFBox still parses and loads the font structure each time.Consider caching the loaded font at the class level if PDFBox supports font reuse across documents, or measure whether the current approach meets your performance requirements.
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java (1)
247-278: Complex query: verify performance and index coverage.This query performs multiple LEFT JOINs, aggregation, and a multi-level ORDER BY with randomization. Consider these points:
Performance: The
GROUP BYwith aggregation and random sorting may be slow on large datasets. Ensure appropriate indexes exist on:
ExamApplicationJpaEntity.examIdPaymentJpaEntity.examApplicationIdandpaymentStatusVirtualAccountLogJpaEntity.applicationIdanddepositStatusExamSubjectJpaEntity.examApplicationIdRandom sorting:
function('rand', 1)is database-specific (MySQL/H2). If you migrate to PostgreSQL, you'll needrandom()instead. Consider documenting this dependency or using a dialect-agnostic approach if portability matters.Correctness: Verify that
GROUP BY ea.id, ea.isTestPaperCheckedis intentional and that other aggregated columns are correctly handled.Run an
EXPLAINquery to verify index usage and performance, especially if you expect thousands of exam applications per exam.src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (1)
13-17: Consider parameterizing cron jobs for maintainability.Both
ExamNumberGenerationJobRound1andExamNumberGenerationJobRound2follow identical patterns with only the date and cron expression differing. This duplication creates maintenance burden.Consider one of these approaches:
- Make the exam date configurable via properties and use a single job class
- Use a data-driven approach where job configurations are stored in the database or configuration files
- At minimum, extract common logic into a base class or shared method
This would reduce duplication and make it easier to add future exam rounds.
src/main/java/life/mosu/mosuserver/application/admin/dto/ApplicationCsvInfo.java (2)
62-68: Boolean parsing silently treats invalid values as false.
Boolean.parseBoolean(trimmedValue)returnsfalsefor any non-"true" string (case-insensitive). This means CSV values like "yes", "1", "Y", or typos will silently becomefalse.If your CSV uses strict "true"/"false" values, the current behavior is acceptable. However, if the CSV might contain variations or you want to catch data quality issues, consider more explicit parsing:
private static Boolean parseBoolean(String bool) { if (bool == null || bool.isBlank()) { return false; } String trimmedValue = bool.trim().toLowerCase(); return switch (trimmedValue) { case "true", "1", "yes", "y" -> true; case "false", "0", "no", "n" -> false; default -> throw new IllegalArgumentException( "잘못된 boolean 값: '" + bool + "', 예상값: true/false"); }; }Or document that only "true" (case-insensitive) is accepted and other values default to false.
35-52: Enforce exact column count or document extra columns
The factory requires ≥11 fields but drops any extras without notice; either validatevalues.length == 11in ApplicationCsvInfo.of or update its Javadoc to state that additional columns are intentionally ignored.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
src/main/resources/fonts/NanumGothic-Regular.ttfis excluded by!**/*.ttfsrc/main/resources/fonts/NotoSansKR-Black.ttfis excluded by!**/*.ttfsrc/main/resources/fonts/NotoSansKR-Light.ttfis excluded by!**/*.ttfsrc/main/resources/fonts/NotoSansKR-Medium.ttfis excluded by!**/*.ttfsrc/main/resources/fonts/NotoSansKR-Regular.ttfis excluded by!**/*.ttfsrc/main/resources/fonts/NotoSansKR-Thin.ttfis excluded by!**/*.ttfsrc/main/resources/static/default-profile.pngis excluded by!**/*.pngsrc/main/resources/static/exam-ticket.pdfis excluded by!**/*.pdf
📒 Files selected for processing (46)
build.gradle(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/AdminApplicationImportService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/dto/ApplicationCsvInfo.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/dto/ImportResultDto.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/processor/ChangeTestPaperCheckedStepProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/processor/GetApplicationGuestRequestStepProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/processor/RegisterVirtualAccountStepProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/admin/util/CsvReader.java(1 hunks)src/main/java/life/mosu/mosuserver/application/application/ApplicationContext.java(2 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/code/ExamNumberCode.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/util/ExamNumberUtil.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examticket/ExamTicketService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examticket/processor/GetMemberExamTicketInfoProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examticket/processor/GetPartnerExamTicketInfoProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/application/entity/ApplicationJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/entity/ExamApplicationJpaEntity.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/projection/ExamApplicationInfoProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/projection/ExamTicketIssueProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/service/ExamNumberGenerationService.java(0 hunks)src/main/java/life/mosu/mosuserver/domain/virtualaccount/VirtualAccountLogJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(1 hunks)src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java(2 hunks)src/main/java/life/mosu/mosuserver/global/support/NumberGeneratorWithDate.java(1 hunks)src/main/java/life/mosu/mosuserver/global/util/PhoneNumberUtil.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/config/QuartzAutoRegisterConfig.java(2 hunks)src/main/java/life/mosu/mosuserver/infra/cron/annotation/CronJob.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/cron/job/ArchivingOrchestratorJob.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound1.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminApplicationImportController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/examticket/ExamTicketController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/examticket/dto/ExamTicketFileResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/examticket/dto/ExamTicketIssueResponse.java(1 hunks)src/main/resources/db/migration/V4__add_test_paper_checked.sql(1 hunks)src/main/resources/templates/admin/import-result.html(1 hunks)src/main/resources/templates/admin/import.html(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/life/mosu/mosuserver/domain/examapplication/service/ExamNumberGenerationService.java
🧰 Additional context used
🧬 Code graph analysis (11)
src/main/java/life/mosu/mosuserver/domain/examapplication/projection/ExamTicketIssueProjection.java (1)
src/main/java/life/mosu/mosuserver/domain/examapplication/projection/ExamTicketInfoProjection.java (1)
ExamTicketInfoProjection(5-14)
src/main/java/life/mosu/mosuserver/application/admin/AdminApplicationImportService.java (1)
src/main/java/life/mosu/mosuserver/application/admin/util/CsvReader.java (1)
Slf4j(14-59)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GetMemberExamTicketInfoProcessor.java (2)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java (1)
Component(30-193)src/main/java/life/mosu/mosuserver/application/examticket/processor/GetPartnerExamTicketInfoProcessor.java (1)
Component(23-72)
src/main/java/life/mosu/mosuserver/application/admin/processor/GetApplicationGuestRequestStepProcessor.java (1)
src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java (1)
Component(13-31)
src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (2)
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java (1)
Slf4j(19-82)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound1.java (1)
Slf4j(13-32)
src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java (1)
src/main/java/life/mosu/mosuserver/application/admin/processor/GetApplicationGuestRequestStepProcessor.java (1)
Component(16-49)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java (2)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GetMemberExamTicketInfoProcessor.java (1)
Component(19-54)src/main/java/life/mosu/mosuserver/application/examticket/processor/GetPartnerExamTicketInfoProcessor.java (1)
Component(23-72)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GetPartnerExamTicketInfoProcessor.java (2)
src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java (1)
Component(30-193)src/main/java/life/mosu/mosuserver/application/examticket/processor/GetMemberExamTicketInfoProcessor.java (1)
Component(19-54)
src/main/java/life/mosu/mosuserver/application/admin/util/CsvReader.java (2)
src/main/java/life/mosu/mosuserver/application/admin/AdminApplicationImportService.java (1)
Slf4j(19-66)src/main/java/life/mosu/mosuserver/application/admin/processor/GetApplicationGuestRequestStepProcessor.java (1)
Component(16-49)
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java (2)
src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound1.java (1)
Slf4j(13-32)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (1)
Slf4j(13-33)
src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound1.java (2)
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java (1)
Slf4j(19-82)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (1)
Slf4j(13-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-deploy
- GitHub Check: build-and-test
🔇 Additional comments (37)
src/main/java/life/mosu/mosuserver/infra/cron/job/ArchivingOrchestratorJob.java (1)
11-12: Import shuffle is fine.No functional impact; import grouping still consistent.
src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java (1)
67-67: LGTM! Error code addition is well-structured.The new
EXAM_TICKET_INFO_NOT_FOUNDerror code is correctly configured with:
- Appropriate
HttpStatus.NOT_FOUNDfor a missing resource scenario- Clear Korean error message consistent with other codes
CriticalLevel.LOWsuitable for a user-facing "not found" condition- Logical placement within the exam ticket errors section
src/main/java/life/mosu/mosuserver/infra/config/QuartzAutoRegisterConfig.java (1)
15-17: LGTM!The explicit imports for
java.utilclasses are clear and appropriate.src/main/java/life/mosu/mosuserver/domain/application/entity/ApplicationJpaEntity.java (1)
3-3: Import consolidation applied.The wildcard import consolidates multiple
jakarta.persistenceimports into a single statement with no functional impact.src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java (1)
20-30: Transactions correctly configured:importGuestApplicationsis annotated@Transactional, covering bothprocess()and itsapplyByGuestcall, ensuring atomicity of the create-fetch-update sequence.src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (3)
26-30: LGTM!The added imports are necessary and correctly support the time-related operations in
validateExamTicketOpenDateand collection operations ingetApplication.
44-47: Confirm cron-based exam number generation and edge-case coverage
The register() method no longer assigns exam numbers; ExamNumberGenerationJobRound1/2 invoke ExamNumberGeneratorExecutorImpl.generate(examDate) to grant numbers. Verify these jobs are scheduled at the correct times for each exam date and that any applications created after the initial run (e.g., late registrations) still receive exam numbers (via reruns or fallback logic).
44-47: Cron-based exam number generation is configured
ExamNumberGeneratorExecutorImpl is annotated @CronTarget and scheduled by ExamNumberGenerationJobRound1/ExamNumberGenerationJobRound2 via @cronjob, ensuring all registered applications receive exam numbers.src/main/java/life/mosu/mosuserver/application/examapplication/code/ExamNumberCode.java (1)
38-54: LGTM with reservation!The lookup methods correctly use
Optional.ofNullable()and throw meaningful exceptions. The Korean error messages are appropriate for the target users.However, be aware that
IllegalArgumentExceptionis an unchecked exception. Ensure callers handle these exceptions appropriately or consider using a custom checked exception for better error tracking.src/main/java/life/mosu/mosuserver/presentation/examticket/dto/ExamTicketFileResponse.java (3)
1-8: LGTM!The record design is clean and appropriate for carrying PDF file data. The three fields (
bytes,filename,contentType) are sufficient for the exam ticket file response use case.Note: Java records don't perform defensive copying of arrays by default. If the
bytesarray is mutable and shared, consider documenting that callers should not modify the array after passing it to this record, or implement a custom constructor with defensive copying if needed.
1-8: LGTM!Clean and simple DTO record for exam ticket file responses. The structure is appropriate for returning PDF content with metadata.
3-8: LGTM! Clean DTO for exam ticket file response.The record appropriately models the PDF response with byte content, filename, and content type. The structure is clear and suitable for returning generated exam ticket PDFs.
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutor.java (3)
1-10: LGTM!The interface design is clean and follows single-responsibility principle:
- Extends
CronJobExecutorto integrate with the cron infrastructure.- Single method
generate(LocalDate examDate)provides a clear contract for exam number generation.- Parameterizing by
examDateallows different cron jobs to target specific exam rounds.
1-10: LGTM!Clean interface design for exam number generation. The contract is simple and focused, extending
CronJobExecutorappropriately for scheduled execution.
6-10: LGTM! Clean interface design for exam number generation.The interface appropriately extends
CronJobExecutorand defines a clear contract for exam-date-specific number generation. TheLocalDateparameter allows flexible scheduling for different exam rounds.src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java (2)
8-10: LGTM!The import additions are necessary for the enhanced whitelist logic and are correctly used in the static methods below.
56-56: LGTM! Guest exam ticket access is appropriately restricted.The
EXAM_TICKET_GUESTwhitelist entry properly restricts access to GET requests only, which is appropriate for guest users retrieving their exam tickets.src/main/java/life/mosu/mosuserver/domain/examapplication/entity/ExamApplicationJpaEntity.java (3)
37-51: Ensure isTestPaperChecked is handled consistently in the builder.The new
isTestPaperCheckedfield has a default value ofBoolean.FALSE, but it's not included as a parameter in the builder (lines 40-46). This means entities created via the builder will have this field set toFALSEby default, which aligns with the field initialization. However, for clarity and consistency, consider whether this field should be explicitly included in the builder pattern or if the current approach is intentional.If the field should remain excluded from the builder (because it's only set via
setTestPaperChecked()after creation), consider adding a comment to document this design decision.
37-38: Field initialization pattern is correct for nullable boolean flag.Using
Boolean(boxed type) with explicitBoolean.FALSEinitialization is appropriate here since:
- The field represents an optional state that may not be set initially
- The default value prevents NPE issues
- JPA/Hibernate handles boxed Boolean types correctly for database mapping
Note: The builder (lines 40-46) doesn't include
isTestPaperCheckedas a parameter, so all new instances correctly default toFALSE.
70-72: One-way setter design is intentional but inflexible.The
setTestPaperChecked()method only allows setting the flag toTRUEwith no way to reset it toFALSE. This is a valid design choice for an immutable state transition (e.g., once a test paper is checked, it stays checked), but it limits flexibility.If there's ever a need to "un-check" a test paper (e.g., for corrections or admin overrides), this design will require modification. Consider documenting this one-way behavior in a comment or the method's Javadoc if it's a critical business rule.
src/main/java/life/mosu/mosuserver/application/admin/processor/ChangeTestPaperCheckedStepProcessor.java (4)
23-26: Verify that throwing an exception for empty results is the desired behavior.The processor throws
EXAM_APPLICATION_NOT_FOUNDwhen no exam applications exist for the givenapplicationId. In the context of CSV import with transaction rollback, this ensures data consistency. However, consider whether this is the intended behavior if:
- An application was created but no exam applications were associated yet
- An application was deleted after CSV processing started
If these are valid edge cases, consider logging a warning and returning an empty list instead of throwing an exception.
28-32: LGTM! JPA dirty checking will persist the changes.The use of
forEachto mutate entities followed by collecting IDs is a clean pattern. The changes will be automatically persisted by JPA dirty checking within the transaction, so no explicit save call is needed.
23-26: Reconsider the exception for missing exam applications.The code throws
EXAM_APPLICATION_NOT_FOUNDwhen no exam applications exist for the givenapplicationId. This might be overly strict depending on the business logic:
- If it's valid for an application to exist without exam applications (e.g., incomplete signup), throwing an exception would prevent CSV import from completing.
- If exam applications are mandatory, then this check is appropriate.
Please verify: Is it always an error when an
applicationIdhas no associated exam applications during CSV import? If not, consider logging a warning instead of throwing an exception:List<ExamApplicationJpaEntity> examApplications = examApplicationJpaRepository.findByApplicationId(applicationId); if (examApplications.isEmpty()) { - throw new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_NOT_FOUND); + log.warn("No exam applications found for applicationId: {}", applicationId); + return Collections.emptyList(); }
28-32: JPA dirty checking will persist changes; code is correct.The code mutates entities via
setTestPaperChecked()without an explicit save call. This is correct within a@Transactionalcontext (managed by the calling service), as JPA's dirty checking will automatically persist the changes when the transaction commits.Optional: For Java 16+, you could simplify the stream using method reference and
toList():return examApplications.stream() .map(ExamApplicationJpaEntity::getId) - .collect(Collectors.toList()); + .toList();src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaRepository.java (2)
66-66: LGTM! Returning List is appropriate for multi-result queries.The method
findAllByExamDatecorrectly returns aList<ExamJpaEntity>, which can be empty if no exams are found for the given date. This is the standard Spring Data JPA pattern for multi-result queries.
66-66: LGTM:findAllByExamDatesignature is safe.Returning
List<ExamJpaEntity>is correct—Spring Data JPA will return an empty list (never null) if no records match.src/main/java/life/mosu/mosuserver/application/examapplication/util/ExamNumberUtil.java (1)
8-22: LGTM! Clean utility method with proper validation.The exam number formatting logic is well-implemented:
- Validates all required parameters are non-null (lines 11-13)
- Uses clear format string with appropriate digit padding (lines 15-21)
- Throws descriptive exception for invalid input
src/main/java/life/mosu/mosuserver/application/admin/util/CsvReader.java (1)
28-29: LGTM! Proper encoding and CSV parsing.The implementation correctly:
- Uses
StandardCharsets.UTF_8for reading (line 29)- Uses
split(CSV_DELIMITER, -1)to preserve trailing empty strings (line 44), which is essential for CSV with optional trailing columnsAlso applies to: 44-44
src/main/java/life/mosu/mosuserver/application/examticket/processor/GenerateExamTicketProcessor.java (2)
51-56: LGTM: Resource initialization at startup.Loading PDF template, font, and default profile at initialization is an effective pattern for reusing resources across invocations. The approach reduces I/O overhead on each request.
187-192: Verify character spacing logic for edge cases.The
formatNamemethod applies different spacing for 2-character names versus others. Ensure this handles edge cases correctly:
- Empty string returns "" (line 188 check)
- Single-character names get 2-space spacing
- Names with 3+ characters get 2-space spacing
If this formatting is based on specific PDF layout requirements, consider adding a comment explaining the spacing rationale.
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java (2)
226-245: LGTM: Well-structured query with proper joins.The query retrieves exam ticket information with explicit LEFT JOINs and returns an Optional projection. The join structure is appropriate for the use case.
280-301: LGTM: Consistent query pattern with payment status filter.The query follows the same pattern as the partner projection query, retrieving exam ticket information with appropriate joins and filtering by payment status.
src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (1)
14-22: Incorrect mismatch alert: the cron schedule intentionally runs six days before the exam (matches Round1).Cron for Round1 is
0 0 9 13 10 ?vs exam date 2025-10-19 (6 days before) and Round2 is0 0 6 20 10 ?vs exam date 2025-10-26 (6 days before). No changes required.Likely an incorrect or invalid review comment.
src/main/java/life/mosu/mosuserver/domain/virtualaccount/VirtualAccountLogJpaRepository.java (2)
11-12: LGTM!Standard Spring Data JPA query method following naming conventions. The addition of
depositStatusparameter allows filtering by both order ID and deposit status, which aligns with the admin import workflow requirements.
11-12: LGTM! Repository method follows Spring Data JPA conventions.The
findByOrderIdAndDepositStatusmethod appropriately extends the repository's query capabilities, allowing filtering by both order ID and deposit status. The method signature follows Spring Data JPA naming conventions and returnsOptionalfor null-safety.src/main/java/life/mosu/mosuserver/application/admin/processor/RegisterVirtualAccountStepProcessor.java (1)
17-25: Validate simulated deposit logic and null handling
- Creating a “successful” deposit log without an actual transaction risks data integrity and audit discrepancies.
- Guard against null or blank
orderIdfromorderIdGenerator.generate().- Avoid passing multiple nulls into
VirtualAccountLogJpaEntity.create(...)unless downstream consumers explicitly handle them.Confirm whether marking deposits as successful here is intentional (e.g., for prepaid/guest CSV flows). If so, introduce a flag/status to distinguish simulated from real payments and document the context; otherwise, add proper validation and error handling.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java (1)
15-15: Complete or remove isTestPaperChecked in ExamApplicationResponse DTO.
- In src/main/java/.../ExamApplicationResponse.java (lines 15, 27, 38), either uncomment the field in the record, constructor, and factory method—mapping it from
ExamApplicationJpaEntity.isTestPaperChecked—or delete all commented references.- Confirm whether
testPaperCheckedis intended to be exposed in this response.
| int lineNo = 1; | ||
| for (ApplicationCsvInfo row : rows) { | ||
| lineNo++; | ||
| try { | ||
| processGuestRow(row); | ||
| success++; | ||
| } catch (Exception e) { | ||
| log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e); | ||
| throw new RuntimeException("CSV 일괄 처리 중 오류 발생"); | ||
| } |
There was a problem hiding this comment.
Do not abort the entire import on the first row error.
The catch block rethrows a RuntimeException on the first failing row, so the loop stops immediately, the transaction rolls back, and the summary logic (including the failure count) is never reached. As a result, a single bad row prevents all other valid applications from being registered—defeating the purpose of batch import and the success/fail statistics you’re computing.
Handle the exception per row (log it, continue, and let the failure counter increase) instead of propagating it so that the rest of the CSV can still be processed. For example:
- } catch (Exception e) {
- log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e);
- throw new RuntimeException("CSV 일괄 처리 중 오류 발생");
- }
+ } catch (Exception e) {
+ log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e);
+ }This keeps the loop running, allowing fail = total - success to reflect actual results while leaving successful rows committed.
📝 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.
| int lineNo = 1; | |
| for (ApplicationCsvInfo row : rows) { | |
| lineNo++; | |
| try { | |
| processGuestRow(row); | |
| success++; | |
| } catch (Exception e) { | |
| log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e); | |
| throw new RuntimeException("CSV 일괄 처리 중 오류 발생"); | |
| } | |
| int lineNo = 1; | |
| for (ApplicationCsvInfo row : rows) { | |
| lineNo++; | |
| try { | |
| processGuestRow(row); | |
| success++; | |
| } catch (Exception e) { | |
| log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/AdminApplicationImportService.java
around lines 37 to 46, the catch currently rethrows a RuntimeException which
aborts the whole import and prevents the failure/success summary from being
produced; instead remove the rethrow, log the full exception, increment a
per-row failure counter (or keep track of failedRows), and continue the loop so
remaining rows are processed; if transactions currently wrap the entire import,
ensure each row is processed in a way that a single row failure does not roll
back successful rows (e.g., use a separate transactional boundary for
processGuestRow or catch and swallow exceptions per row), and preserve existing
success/fail summary logic so fail = total - success (or use an explicit fail++
when catching).
| private static LocalDate parseDate(String dateStr) { | ||
| try { | ||
| return LocalDate.parse(dateStr.trim()); | ||
| } catch (DateTimeParseException e) { | ||
| throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve error message to include the invalid value.
The error message "날짜 형식이 올바르지 않습니다" (Date format is incorrect) doesn't indicate which value failed or what format is expected. This makes debugging difficult for users.
Apply this diff to improve the error message:
private static LocalDate parseDate(String dateStr) {
try {
return LocalDate.parse(dateStr.trim());
} catch (DateTimeParseException e) {
- throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다");
+ throw new IllegalArgumentException(
+ "날짜 형식이 올바르지 않습니다. 입력값: '" + dateStr + "', 예상 형식: yyyy-MM-dd", e);
}
}📝 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 static LocalDate parseDate(String dateStr) { | |
| try { | |
| return LocalDate.parse(dateStr.trim()); | |
| } catch (DateTimeParseException e) { | |
| throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다"); | |
| } | |
| } | |
| private static LocalDate parseDate(String dateStr) { | |
| try { | |
| return LocalDate.parse(dateStr.trim()); | |
| } catch (DateTimeParseException e) { | |
| throw new IllegalArgumentException( | |
| "날짜 형식이 올바르지 않습니다. 입력값: '" + dateStr + "', 예상 형식: yyyy-MM-dd", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/dto/ApplicationCsvInfo.java
around lines 54 to 60, the parseDate method throws a generic
IllegalArgumentException without showing the offending value or expected format;
update the catch block to include the trimmed input value and the expected
format (e.g., ISO yyyy-MM-dd) in the exception message (optionally appending the
original DateTimeParseException message) so callers know which value failed and
why.
| CreateApplicationResponse response = applicationService.applyByGuest(request); | ||
|
|
||
| Long applicationId = response.applicationId(); | ||
| ApplicationJpaEntity application = applicationJpaRepository.findById(applicationId) | ||
| .orElseThrow(() -> new IllegalArgumentException("신청을 찾을 수 없습니다. id=" + applicationId)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inefficient fetch immediately after creation.
The code fetches the application entity from the database immediately after creating it via applyByGuest(). This introduces unnecessary database round-trips and latency.
Consider one of the following approaches:
-
Preferred: Modify
applyByGuest()to return the created entity or allow setting the status during creation, eliminating the need for a separate fetch and update. -
Alternative: If the fetch-and-update pattern is required for architectural reasons (e.g., audit logging, event triggering in
changeStatus()), add a comment explaining why the fetch is necessary.
For approach 1, you would refactor the service method to accept the status as a parameter or return the entity directly:
ApplicationJpaEntity application = applicationService.applyByGuestAndReturn(request, ApplicationStatus.APPROVED);
return application.getId();This reduces database operations and improves performance for bulk imports.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java
around lines 22 to 26, the code does an unnecessary DB fetch right after calling
applicationService.applyByGuest(), causing extra round-trips; either change
applyByGuest() to return the created ApplicationJpaEntity (or accept a desired
status so it can be set on creation) and use that returned entity directly, or,
if fetch is required, add a concise comment explaining why the separate
findById() is necessary (e.g., for audit/event reasons) so future readers
understand the intentional extra query.
| ApplicationJpaEntity application = applicationJpaRepository.findById(applicationId) | ||
| .orElseThrow(() -> new IllegalArgumentException("신청을 찾을 수 없습니다. id=" + applicationId)); | ||
|
|
||
| application.changeStatus(ApplicationStatus.APPROVED); |
There was a problem hiding this comment.
Critical: Status change is not persisted to the database.
Line 28 calls changeStatus() on the entity, but the modified entity is never saved back to the repository. The status change will be lost when the transaction commits (or the persistence context is cleared).
Apply this diff to persist the status change:
application.changeStatus(ApplicationStatus.APPROVED);
+applicationJpaRepository.save(application);
return applicationId;Alternatively, if changeStatus() is designed to work within a managed transaction, ensure this method is annotated with @Transactional at the class or method level.
📝 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.
| application.changeStatus(ApplicationStatus.APPROVED); | |
| application.changeStatus(ApplicationStatus.APPROVED); | |
| applicationJpaRepository.save(application); | |
| return applicationId; |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/processor/ApplyGuestStepProcessor.java
around line 28, the code calls
application.changeStatus(ApplicationStatus.APPROVED) but does not persist the
updated entity; either call the appropriate repository.save(application) (or
repository.saveAndFlush(application) if immediate DB write is required) after
the change to persist the new status, or ensure the surrounding method/class is
annotated with @Transactional so the persistence context will automatically
flush the status change before commit.
| ExamJpaEntity exam = examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName); | ||
|
|
||
| ExamApplicationRequest examApplicationRequest = new ExamApplicationRequest( | ||
| exam.getId(), | ||
| csvInfo.isLunchChecked() | ||
| ); |
There was a problem hiding this comment.
Guard against missing exam lookups.
findByExamDateAndSchoolName returns null when no exam matches the CSV row. We immediately dereference exam.getId(), so the CSV import blows up with an NPE instead of a controlled failure. Please fail fast with a domain exception (e.g., CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND) or similar) before building the request. A minimal fix is to wrap the lookup in Optional.ofNullable(...).orElseThrow(...).
+import life.mosu.mosuserver.global.exception.CustomRuntimeException;
+import life.mosu.mosuserver.global.exception.ErrorCode;
+import java.util.Optional;
@@
- ExamJpaEntity exam = examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName);
+ ExamJpaEntity exam = Optional.ofNullable(
+ examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName))
+ .orElseThrow(() -> new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND));📝 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.
| ExamJpaEntity exam = examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName); | |
| ExamApplicationRequest examApplicationRequest = new ExamApplicationRequest( | |
| exam.getId(), | |
| csvInfo.isLunchChecked() | |
| ); | |
| // at the top of the file, alongside the other imports | |
| import life.mosu.mosuserver.global.exception.CustomRuntimeException; | |
| import life.mosu.mosuserver.global.exception.ErrorCode; | |
| import java.util.Optional; | |
| … | |
| // around lines 28–33 in GetApplicationGuestRequestStepProcessor.java | |
| ExamJpaEntity exam = Optional.ofNullable( | |
| examJpaRepository.findByExamDateAndSchoolName(examDate, schoolName)) | |
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND)); | |
| ExamApplicationRequest examApplicationRequest = new ExamApplicationRequest( | |
| exam.getId(), | |
| csvInfo.isLunchChecked() | |
| ); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/processor/GetApplicationGuestRequestStepProcessor.java
around lines 28 to 33, the code dereferences exam returned from
examJpaRepository.findByExamDateAndSchoolName(...) which can be null; wrap the
lookup with Optional.ofNullable(...).orElseThrow(...) and throw a domain
exception such as new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND) (or the
project’s equivalent) so the import fails fast with a controlled error before
constructing ExamApplicationRequest.
| LocalDate examDate, | ||
| Set<String> subjects, | ||
| String lunchName | ||
| // Boolean isTestPaperChecked |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove or uncomment isTestPaperChecked field.
The isTestPaperChecked field is commented out in three locations. Given that:
- Migration
V4__add_test_paper_checked.sqladds thetest_paper_checkedcolumn to the database. - The field appears to be part of the feature implementation.
Either:
- Uncomment the field if it's part of this PR's functionality and wire it through from the entity.
- Remove the comments entirely if the feature is deferred to a future PR.
Leaving commented code creates maintenance confusion and suggests incomplete implementation.
If the field should be active, apply this diff:
LocalDate examDate,
Set<String> subjects,
- String lunchName
-// Boolean isTestPaperChecked
+ String lunchName,
+ Boolean isTestPaperChecked
) {
public static ExamApplicationResponse of(
...
LocalDate examDate,
Set<String> subjects,
- String lunchName
-// Boolean isTestPaperChecked
+ String lunchName,
+ Boolean isTestPaperChecked
) {
return new ExamApplicationResponse(
...
subjects,
- lunchName
-// isTestPaperChecked
+ lunchName,
+ isTestPaperChecked
);
}Also applies to: 27-27, 38-38
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java
around lines 15, 27 and 38, the isTestPaperChecked field is left commented out;
either remove the commented lines entirely if this feature is deferred, or if it
should be active in this PR, uncomment the field declarations and accessors and
map it from the domain/entity (add a getter/setter or include in constructor and
set from the entity's getTestPaperChecked() value), update
equals/hashCode/toString if applicable, and ensure any JSON serialization
annotations are present so the DTO reflects the new test_paper_checked column.
🛠️ Refactor suggestion | 🟠 Major
Remove commented code.
The commented isTestPaperChecked references should be removed rather than left in the codebase. Commented code creates confusion and technical debt.
Options:
- If the feature is incomplete: Remove the comments entirely and track the work in an issue
- If ready to implement: Uncomment and ensure the field is properly populated from the entity
Apply this diff to remove the comments:
Set<String> subjects,
String lunchName
-// Boolean isTestPaperChecked
) {
public static ExamApplicationResponse of(
String lunchName
-// Boolean isTestPaperChecked
) {
return new ExamApplicationResponse(
lunchName
-// isTestPaperChecked
);Also applies to: 27-27, 38-38
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java
around lines 15, 27 and 38, remove the commented-out Boolean isTestPaperChecked
lines (do not leave commented code in the file). If the feature is incomplete,
delete those comment lines and open a ticket to track adding the field later; if
the feature should exist, instead uncomment the field and ensure it is declared,
mapped from the entity, and used consistently across
constructors/getters/setters.
| ALTER TABLE exam_application | ||
| ADD COLUMN test_paper_checked BIT(1) NULL DEFAULT b'0'; No newline at end of file |
There was a problem hiding this comment.
Column should be NOT NULL with default value.
The test_paper_checked column is defined as NULL but has a DEFAULT b'0'. Since every row should have a boolean value (true/false), the column should be NOT NULL.
Apply this diff:
ALTER TABLE exam_application
- ADD COLUMN test_paper_checked BIT(1) NULL DEFAULT b'0';
+ ADD COLUMN test_paper_checked BIT(1) NOT NULL DEFAULT b'0';Note: BIT(1) is MySQL/MariaDB-specific. If the project needs to support other databases, consider using BOOLEAN or TINYINT(1) for better portability.
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V4__add_test_paper_checked.sql around lines 1
to 2, the new column is created as NULL with a default of b'0'; change the
column to NOT NULL with the same default so every row always has a boolean
value. Update the ALTER TABLE statement to declare test_paper_checked as NOT
NULL DEFAULT b'0' (or switch to a more portable type like BOOLEAN/TINYINT(1) if
cross-DB compatibility is required).
| /*.failure-message {*/ | ||
| /* color: #ef4444; !* Red *!*/ | ||
| /* font-weight: bold;*/ | ||
| /* font-size: 18px;*/ | ||
| /* margin-top: 20px;*/ | ||
| /* padding: 10px;*/ | ||
| /* border: 1px solid #fee2e2;*/ | ||
| /* background-color: #fef2f2;*/ | ||
| /* border-radius: 8px;*/ | ||
| /* text-align: center;*/ | ||
| /*}*/ |
There was a problem hiding this comment.
Uncomment the CSS for failure-message or use an alternative class.
The .failure-message class is used at line 141 to style the failure banner, but the corresponding CSS is commented out at lines 62-72. This will result in unstyled text when import failures occur.
Apply this fix:
- /*.failure-message {*/
- /* color: #ef4444; !* Red *!*/
- /* font-weight: bold;*/
- /* font-size: 18px;*/
- /* margin-top: 20px;*/
- /* padding: 10px;*/
- /* border: 1px solid #fee2e2;*/
- /* background-color: #fef2f2;*/
- /* border-radius: 8px;*/
- /* text-align: center;*/
- /*}*/
+ .failure-message {
+ color: #ef4444; /* Red */
+ font-weight: bold;
+ font-size: 18px;
+ margin-top: 20px;
+ padding: 10px;
+ border: 1px solid #fee2e2;
+ background-color: #fef2f2;
+ border-radius: 8px;
+ text-align: center;
+ }Also applies to: 141-143
🤖 Prompt for AI Agents
In src/main/resources/templates/admin/import-result.html around lines 62-72 (and
also relevant to usage at lines 141-143), the .failure-message CSS block is
commented out so the failure banner at lines 141-143 will be unstyled; restore
or reintroduce the CSS by uncommenting the .failure-message block (or moving
equivalent styles into an existing stylesheet) so it defines color, font-weight,
font-size, margin, padding, border, background-color, border-radius and
text-align for the failure banner; ensure the restored rules are valid CSS
(remove comment markers and any template-comment artifacts) and reload the
template reference so the element at 141-143 receives the expected styles.
Remove commented-out CSS or document its purpose.
The .failure-message CSS block is commented out but the class is referenced on line 141. This creates an inconsistency where the failure banner will be unstyled.
If this styling is not needed yet, remove the reference on line 141. If it's planned for future use, add a TODO comment explaining when/why it should be enabled.
Apply this diff to remove the dead code:
- /*.failure-message {*/
- /* color: #ef4444; !* Red *!*/
- /* font-weight: bold;*/
- /* font-size: 18px;*/
- /* margin-top: 20px;*/
- /* padding: 10px;*/
- /* border: 1px solid #fee2e2;*/
- /* background-color: #fef2f2;*/
- /* border-radius: 8px;*/
- /* text-align: center;*/
- /*}*/Or uncomment and complete the failure message styling if it's needed.
📝 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.
| /*.failure-message {*/ | |
| /* color: #ef4444; !* Red *!*/ | |
| /* font-weight: bold;*/ | |
| /* font-size: 18px;*/ | |
| /* margin-top: 20px;*/ | |
| /* padding: 10px;*/ | |
| /* border: 1px solid #fee2e2;*/ | |
| /* background-color: #fef2f2;*/ | |
| /* border-radius: 8px;*/ | |
| /* text-align: center;*/ | |
| /*}*/ |
🤖 Prompt for AI Agents
In src/main/resources/templates/admin/import-result.html around lines 62 to 72
(the commented .failure-message CSS) and note the class is referenced at line
141, the file contains dead/commented-out styling which leaves the failure
banner unstyled; either remove the HTML reference to .failure-message at line
141 if the style is not required, or restore the CSS by uncommenting and
finishing the block so the class is styled, or keep the CSS commented but add a
clear TODO comment above the commented block describing when/why it should be
enabled; make whichever change keeps code consistent (no unused class
references) and update commit message accordingly.
| </div> | ||
|
|
||
| <div th:if="${result.totalFailure == 0}" class="success-message"> | ||
| ✅ 데이터 오류가 발견되지 않아 **총 <span th:text="${result.totalSuccess}">100</span>건의 데이터가 모두 성공적으로 반영되었습니다.** |
There was a problem hiding this comment.
Fix incorrect bold text syntax in HTML.
Lines 137 and 142 use **text** markdown syntax inside HTML, which won't render as bold. Use proper HTML tags instead.
Apply this diff:
- ✅ 데이터 오류가 발견되지 않아 **총 <span th:text="${result.totalSuccess}">100</span>건의 데이터가 모두 성공적으로 반영되었습니다.**
+ ✅ 데이터 오류가 발견되지 않아 <strong>총 <span th:text="${result.totalSuccess}">100</span>건의 데이터가 모두 성공적으로 반영되었습니다.</strong>- ❌ 데이터 오류가 발견되어 **전체 (<span th:text="${result.totalProcessed}">100</span>건) 작업이 롤백되었습니다.**
+ ❌ 데이터 오류가 발견되어 <strong>전체 (<span th:text="${result.totalProcessed}">100</span>건) 작업이 롤백되었습니다.</strong>Also applies to: 142-142
🤖 Prompt for AI Agents
In src/main/resources/templates/admin/import-result.html around lines 137 and
142, the text uses markdown-style bold (**...**) which will not render in HTML;
replace the **bolded** markdown with proper HTML tags (e.g., wrap the intended
bold portions in <strong>...</strong> or <b>...</b>), preserving the existing
<span th:text="${result.totalSuccess}">100</span> interpolation and any
surrounding text/spacing.
| <form th:action="@{/admin/applications/import/guests}" method="post" enctype="multipart/form-data"> | ||
| <label for="guest-file"> | ||
| CSV 파일 선택 (.csv): | ||
| </label> | ||
| <input type="file" id="guest-file" name="file" accept=".csv" required /> | ||
|
|
||
| <button type="submit" class="btn-guest"> | ||
| 비회원 데이터 업로드 시작 | ||
| </button> |
There was a problem hiding this comment.
Add CSRF hidden field to enable POST
Spring Security’s default CSRF protection will reject this multipart POST unless the _csrf token is submitted, so the admin upload will fail. Please include the hidden token field.
<form th:action="@{/admin/applications/import/guests}" method="post" enctype="multipart/form-data">
+ <input type="hidden" th:name="${_csrf.parameterName}" th:value="${_csrf.token}" />
<label for="guest-file">🤖 Prompt for AI Agents
In src/main/resources/templates/admin/import.html around lines 136 to 144, the
multipart POST form is missing the Spring Security CSRF token field so the
upload will be rejected; add a hidden input inside the form that binds to the
CSRF parameter and token (using Thymeleaf expressions: set the input's name to
${_csrf.parameterName} and its value to ${_csrf.token}) so the _csrf token is
submitted with the POST.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java (2)
src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound1.java (1)
Slf4j(13-32)src/main/java/life/mosu/mosuserver/infra/cron/job/ExamNumberGenerationJobRound2.java (1)
Slf4j(13-33)
| if (!gapInserted && Boolean.FALSE.equals(examApplication.getIsTestPaperChecked())) { | ||
| personalCode += GAP_BETWEEN_CHECKED_AND_UNCHECKED; | ||
| gapInserted = true; | ||
| } | ||
|
|
||
| personalCode++; | ||
|
|
||
| String examNumber = ExamNumberUtil.formatExamNumber( | ||
| roundCode, areaCode, schoolCode, personalCode); | ||
|
|
||
| examApplication.grantExamNumber(examNumber); | ||
| } |
There was a problem hiding this comment.
Fix gap handling when no checked papers exist.
If every application has isTestPaperChecked == FALSE, the very first iteration hits this branch, adds the gap, and assigns personal code 26 to the first candidate. That wastes numbers 1–25 even though there are no “checked” applications ahead of them. Guard the gap insertion with personalCode > 0 (or an equivalent flag) so it only triggers after at least one checked application has been numbered.
- if (!gapInserted && Boolean.FALSE.equals(examApplication.getIsTestPaperChecked())) {
+ if (!gapInserted
+ && Boolean.FALSE.equals(examApplication.getIsTestPaperChecked())
+ && personalCode > 0) {📝 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.
| if (!gapInserted && Boolean.FALSE.equals(examApplication.getIsTestPaperChecked())) { | |
| personalCode += GAP_BETWEEN_CHECKED_AND_UNCHECKED; | |
| gapInserted = true; | |
| } | |
| personalCode++; | |
| String examNumber = ExamNumberUtil.formatExamNumber( | |
| roundCode, areaCode, schoolCode, personalCode); | |
| examApplication.grantExamNumber(examNumber); | |
| } | |
| if (!gapInserted | |
| && Boolean.FALSE.equals(examApplication.getIsTestPaperChecked()) | |
| && personalCode > 0) { | |
| personalCode += GAP_BETWEEN_CHECKED_AND_UNCHECKED; | |
| gapInserted = true; | |
| } | |
| personalCode++; | |
| String examNumber = ExamNumberUtil.formatExamNumber( | |
| roundCode, areaCode, schoolCode, personalCode); | |
| examApplication.grantExamNumber(examNumber); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/examapplication/cron/ExamNumberGeneratorExecutorImpl.java
around lines 78 to 89, the gap insertion branch currently runs even when no
checked papers have been numbered, causing the first candidate to receive
personalCode 26; update the condition so the gap is only inserted after at least
one personal code has been assigned (e.g., change the if to require personalCode
> 0 in addition to !gapInserted and isTestPaperChecked == FALSE, or
alternatively set gapInserted only when you assign the first checked
application) so numbering 1–25 are not skipped when no checked applications
precede them.
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Security
Database