prod: 수험번호 및 수험표 #382
prod: 수험번호 및 수험표 #382
Conversation
Feat/ticket: 수험번호 및 수험표 발급 기능 구현
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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은 시험 관리 시스템에 대한 중요한 개선 사항을 도입합니다. 관리자가 CSV 파일을 사용하여 게스트 신청을 대량으로 처리할 수 있도록 하여 수동 데이터 입력의 필요성을 줄입니다. 또한, 응시자에게 고유한 수험번호를 자동으로 할당하는 새로운 메커니즘을 구현하고, 신청 정보를 기반으로 맞춤형 PDF 수험표를 생성하는 기능을 추가하여 사용자 경험과 관리 효율성을 향상시킵니다. 이러한 변경 사항은 전반적인 시험 신청 및 관리 프로세스를 간소화하는 데 중점을 둡니다. 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 파일을 통한 일괄 신청 데이터 가져오기, 수험번호 생성 로직, PDF 수험표 생성 등 핵심적인 기능들이 많이 추가되었습니다. 전반적으로 기능 구현의 방향은 좋으나, 몇 가지 개선점을 제안합니다. 특히 CSV 파싱 로직의 안정성, 설정값 하드코딩 문제, 예외 처리 및 로깅 방식에 대한 검토가 필요합니다. 또한, 현재 주석 처리되어 비활성화된 것으로 보이는 알림톡 발송 로직은 프로덕션 배포 전 반드시 확인이 필요합니다.
| // 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 메소드의 로직이 주석 처리되어 있습니다. 이 코드가 프로덕션에 배포되면 알림톡이 발송되지 않는 심각한 문제가 발생할 수 있습니다. 개발 또는 테스트 목적으로 주석 처리한 것이라면, 프로덕션 배포 전에 반드시 주석을 해제해야 합니다. 만약 기능 비활성화가 의도된 것이라면, 주석 처리 대신 기능 플래그(feature flag)나 설정 값을 사용하는 것이 더 안전한 방법입니다.
| success++; | ||
| } catch (Exception e) { | ||
| log.error("게스트 신청 CSV 행 {} 처리 실패: {}", lineNo, e.getMessage(), e); | ||
| throw new RuntimeException("CSV 일괄 처리 중 오류 발생"); |
There was a problem hiding this comment.
예외를 새로 생성하여 던질 때 기존 예외 정보를 포함하지 않으면 근본적인 원인을 파악하기 어려워집니다. log.error로 기록은 되지만, 호출 스택 상위에서는 원본 예외 정보를 알 수 없습니다. 디버깅 편의성을 위해 원본 예외(e)를 함께 전달하는 것이 좋습니다.
throw new RuntimeException("CSV 일괄 처리 중 오류 발생: 행 " + lineNo, e);| } | ||
|
|
||
| try { | ||
| String[] values = line.split(CSV_DELIMITER, -1); |
There was a problem hiding this comment.
String.split(",")을 사용하여 CSV를 파싱하는 방식은 필드 값에 쉼표가 포함된 경우(예: "some, text")를 제대로 처리하지 못합니다. 이는 데이터 파싱 오류를 유발할 수 있습니다. Apache Commons CSV나 OpenCSV와 같은 CSV 전문 라이브러리를 사용하여 이러한 엣지 케이스를 안정적으로 처리하는 것을 강력히 권장합니다.
| results.add(ApplicationCsvInfo.of(values)); | ||
| } catch (Exception e) { | ||
| log.error("CSV 파싱 실패 (Line {}): {}", lineNumber, e.getMessage()); | ||
| throw new RuntimeException("CSV 파싱 오류"); |
There was a problem hiding this comment.
예외 발생 시 새로운 RuntimeException을 생성할 때 원본 예외(e)를 포함하지 않아 근본 원인 추적이 어렵습니다. throw new RuntimeException("..."); 대신 `throw new RuntimeException("...
| throw new RuntimeException("CSV 파싱 오류"); | |
| throw new RuntimeException("CSV 파싱 오류", e); | |
| 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 | ||
| ); |
There was a problem hiding this comment.
수험 번호 생성에 사용되는 회차, 지역, 학교 코드가 소스 코드에 하드코딩되어 있습니다. 이 값들이 변경될 때마다 코드를 수정하고 재배포해야 하므로 유지보수가 어렵습니다. 이 데이터를 데이터베이스 테이블이나 외부 설정 파일(e.g., application.yml)로 옮기는 것을 고려해보세요. 이렇게 하면 애플리케이션 재시작 없이도 값을 변경할 수 있어 유연성이 높아집니다.
| try { | ||
| return LocalDate.parse(dateStr.trim()); | ||
| } catch (DateTimeParseException e) { | ||
| throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다"); |
There was a problem hiding this comment.
날짜 파싱 실패 시 예외 메시지가 너무 일반적입니다. 어떤 값 때문에 오류가 발생했는지 알 수 있도록, "날짜 형식이 올바르지 않습니다: '" + dateStr + "'"와 같이 실패한 문자열 값을 메시지에 포함하면 디버깅에 큰 도움이 됩니다.
throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다: '" + dateStr + "'");| private final ExamApplicationJpaRepository examApplicationJpaRepository; | ||
| private final ExamJpaRepository examJpaRepository; | ||
| private final ExamNumberCode examNumberCode; | ||
| private final int GAP_BETWEEN_CHECKED_AND_UNCHECKED = 25; |
There was a problem hiding this comment.
수험번호 생성 로직에서 검토 여부에 따른 간격 값(25)이 하드코딩되어 있습니다. 이 값은 비즈니스 규칙에 따라 변경될 수 있으므로, @Value 어노테이션을 사용하여 application.yml 등의 설정 파일에서 값을 주입받도록 변경하는 것이 좋습니다. 이렇게 하면 코드 수정 없이 설정을 통해 값을 유연하게 변경할 수 있습니다.
| private static final List<Subject> SOCIAL_SUBJECTS = List.of( | ||
| Subject.LIFE_AND_ETHICS, | ||
| Subject.WORLD_HISTORY, | ||
| Subject.ECONOMICS, | ||
| Subject.POLITICS_AND_LAW, | ||
| Subject.SOCIETY_AND_CULTURE, | ||
| Subject.ETHICS_AND_IDEOLOGY, | ||
| Subject.KOREAN_GEOGRAPHY, | ||
| Subject.WORLD_GEOGRAPHY, | ||
| Subject.EAST_ASIAN_HISTORY | ||
| ); | ||
|
|
||
| private static final List<Subject> SCIENCE_SUBJECTS = List.of( | ||
| Subject.PHYSICS_1, | ||
| Subject.CHEMISTRY_1, | ||
| Subject.BIOLOGY_1, | ||
| Subject.EARTH_SCIENCE_1, | ||
| Subject.PHYSICS_2, | ||
| Subject.CHEMISTRY_2, | ||
| Subject.BIOLOGY_2, | ||
| Subject.EARTH_SCIENCE_2 | ||
| ); |
There was a problem hiding this comment.
사회/과학탐구 과목 목록이 코드에 하드코딩되어 있습니다. 만약 과목이 추가되거나 변경될 경우 코드를 수정해야 합니다. Subject enum에 과목 그룹(사회/과학)을 나타내는 속성을 추가하거나, 데이터베이스에서 과목 그룹 정보를 관리하는 것을 고려해보세요. 이렇게 하면 과목 체계 변경에 더 유연하게 대응할 수 있습니다.
| drawText(cs, font, 10, 198, 783, nz(request.examNumber())); | ||
| drawText(cs, font, 11, 174, 734, formatName(request.userName())); | ||
| drawText(cs, font, 10, 174, 667, formatBirth(request.birth())); | ||
|
|
||
| // 우측 | ||
| drawText(cs, font, 9, 481, 812, nz(request.examNumber())); | ||
| drawText(cs, font, 9, 481, 801, nz(request.userName())); | ||
| drawText(cs, font, 9, 481, 746, firstOrEmpty(request.subjects())); | ||
| drawText(cs, font, 9, 481, 736, lastOrEmpty(request.subjects())); | ||
| drawText(cs, font, 9, 481, 714, nz(request.schoolName())); |
There was a problem hiding this comment.
PDF에 텍스트를 그리는 위치 좌표가 매직 넘버로 하드코딩되어 있습니다. PDF 템플릿 레이아웃과 관련된 이 값들을 별도의 private static final 변수나, 좌표를 그룹화하는 내부 record/class로 추출하면 가독성과 유지보수성을 높일 수 있습니다. 예를 들어, private static final float EXAM_NUMBER_X = 198f; 와 같이 상수로 정의할 수 있습니다.
| } catch (Exception e) { | ||
| return null; | ||
| } finally { |
There was a problem hiding this comment.
이미지 URL로부터 이미지를 로드하는 중 발생하는 예외가 조용히 무시되고 있습니다. 이미지 로드 실패는 수험표에 사진이 누락되는 결과를 낳으므로, catch 블록 안에서 최소한 경고(WARN) 수준으로 로그를 남겨(e.g., log.warn("Failed to load image from URL: {}", url, e);) 문제를 추적할 수 있도록 하는 것이 좋습니다.
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타