-
Notifications
You must be signed in to change notification settings - Fork 1
134 feat: 소셜 로그인 중복, 복수전공, 편입, 졸업요건 예외 처리 #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 1차 방어: studentCode 기준으로 중복 검사 - 2차 방어: race condition을 우려하여 제약 조건 기준으로 검사
- 편입생 판별법: 학법 앞 2자리, 입학년도 뒷 2자리 비교
Summary of ChangesHello @pp8817, 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은 포털 연동 및 졸업 요건 조회 과정에서 발생할 수 있는 다양한 학생 관련 시나리오에 대한 오류 처리 로직을 개선하고 확장합니다. 특히, 소셜 로그인 시 학번 중복, 복수전공 학생 처리, 편입생 졸업 요건 미지원, 그리고 졸업 요건 데이터 부재와 같은 특정 상황에 대한 명확한 예외 처리를 도입하여 시스템의 안정성과 사용자 경험을 향상시키는 데 중점을 두었습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces several enhancements and error handling improvements across the application. Key changes include updating API documentation for portal-related endpoints (login, startScraping) to reflect new error codes such as DUAL_MAJOR_COURSE_TYPE_MISSING, DUPLICATED_STUDENT_CODE, and TRANSFER_STUDENT_UNSUPPORTED. The InitializePortalConnectionService and RefreshPortalConnectionService now include checks for unsupported dual majors and duplicate student codes, with refined exception handling for DataIntegrityViolationException and generic Exception types. The GraduationService and GraduationQueryRepository were updated to handle and reject transfer students, and a new isTransferStudent() method was added to the Student entity to identify transfer students based on student code and admission year. Additionally, the ErrorCode enum was reorganized and expanded with new common, server, student, and dual major related error codes. Review comments suggest refactoring the isTransferStudent() method to use an existing stored field for consistency, consolidating duplicate @ApiResponse annotations for the same HTTP status code in API documentation, removing a redundant catch (CommonException e) block, and improving the robustness of the isStudentCodeUniqueViolation check by avoiding string-based error message parsing. The reviewer also noted that the TRANSFER_STUDENT_UNSUPPORTED error code is inconsistently placed within the 'Token related' group and should be moved to a more appropriate category.
| public boolean isTransferStudent() { | ||
| if (this.studentCode == null || this.academicInfo == null || this.academicInfo.getAdmissionYear() == null) { | ||
| return false; | ||
| } | ||
|
|
||
| if (this.studentCode.length() < 2) { | ||
| return false; | ||
| } | ||
|
|
||
| String codePrefix = this.studentCode.substring(0, 2); // 학번 앞 2자리 | ||
| String yearSuffix = String.valueOf(this.academicInfo.getAdmissionYear()).substring(2); // 입학년도 뒤 2자리 | ||
|
|
||
| return !codePrefix.equals(yearSuffix); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로 추가된 isTransferStudent() 메서드는 학번과 입학년도를 비교하여 편입생 여부를 동적으로 계산하고 있습니다. 하지만 Student 엔티티의 academicInfo 필드에는 이미 isTransferStudent라는 boolean 필드가 존재하며, 포털 데이터로부터 이 값을 받아와 저장하고 있습니다.
동일한 정보에 대해 두 가지 출처(저장된 값, 계산된 값)가 존재하면 데이터 불일치가 발생할 수 있고 혼란을 야기할 수 있습니다. 예를 들어, 학번 부여 규칙이 변경되면 이 메서드는 잘못된 결과를 반환할 수 있습니다.
따라서 이 메서드는 이미 저장된 academicInfo.getIsTransferStudent() 값을 직접 반환하도록 수정하는 것이 좋습니다. 이렇게 하면 데이터의 일관성을 보장하고 코드를 단순화할 수 있습니다.
public boolean isTransferStudent() {
return this.academicInfo != null && this.academicInfo.getIsTransferStudent() != null && this.academicInfo.getIsTransferStudent();
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
데이터 확인 결과 편입생인 경우에도 student.is_transfer_student 값이 FALSE로 처리되고 있음.
현재 방식 유지 -> 크롤링 데이터가 문제인지, 처리를 잘못하고 있는지 확인 필요
ISSUE #134
참고 사항
P1: 꼭 반영해주세요 (Request changes)
P2: 적극적으로 고려해주세요 (Request changes)
P3: 웬만하면 반영해 주세요 (Comment)
P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
P5: 그냥 사소한 의견입니다 (Approve)
🔗 Related Issue