Conversation
- sortApplicantsByField1에서 원본 배열 변이 방지 ([...applicants].sort())
- 불필요한 블록 스코프 {} 제거 및 early return 정리
- confirm 메시지 한글화 (pass/non-pass -> 합격/불합격)
- 개별 발송 버튼 추가 (non-processed 상태일 때 비활성화) - 1차 합격/불합격, 최종 합격/불합격 일괄 발송 버튼 추가 - sendEmailToApplicant, sendEmailToAll API 추가
- 헤더 컬럼 5개로 맞춤 (합격/불합격, 이메일 발송 헤더 추가) - 리스트 grid-cols 4fr -> 5fr로 헤더와 동기화 - 발송 버튼 truncate 제거
Summary of ChangesHello, 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은 합격 상태 관리 페이지에 지원자 결과 이메일 발송 기능을 도입하고, 기존 코드의 여러 버그를 수정하여 사용자 경험과 시스템 안정성을 향상시키는 것을 목표로 합니다. 개별 및 일괄 이메일 발송 기능을 통해 운영 효율성을 높이고, 데이터 변이 방지 및 UI 개선을 통해 전반적인 애플리케이션의 견고함을 강화했습니다. 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. Changelog
Activity
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
This pull request introduces email sending functionality to the pass/fail status management page and includes bug fixes. A critical security vulnerability has been identified: the new /pass-state route lacks access control validation, potentially allowing unauthenticated users to access sensitive data and trigger email-sending actions. Additionally, the review highlights areas for code duplication reduction and UI improvements. Consideration for robust duplicate email sending prevention (UI-level vs. backend-driven) is also suggested.
| </Link> | ||
| )} | ||
| <div className="grid grid-cols-[8fr_8fr_4fr_3fr] gap-4"> | ||
| <div className="grid grid-cols-[8fr_8fr_4fr_3fr_3fr] gap-4"> |
There was a problem hiding this comment.
The new /pass-state route is missing from the needValidatePath list in frontend/src/constants/index.ts. This list defines the routes that require authentication and authorization (e.g., /admin, /kanban, /applicant). Since this page allows managing applicant pass states and sending sensitive result emails, it should be protected to prevent unauthorized access. An attacker could potentially navigate to /pass-state/[generation] and perform administrative actions without proper authentication.
| const onSendEmailAll = (state: EmailState) => { | ||
| const labelMap: Record<EmailState, string> = { | ||
| "first-passed": "1차 합격자", | ||
| "first-failed": "1차 불합격자", | ||
| "final-passed": "최종 합격자", | ||
| "final-failed": "최종 불합격자", | ||
| }; | ||
| const ok = confirm( | ||
| `${labelMap[state]} 전체에게 결과 이메일을 발송하시겠습니까?` | ||
| ); | ||
| if (!ok) return; | ||
| sendEmailAll({ year: Number(selectedGeneration), state }); | ||
| }; |
There was a problem hiding this comment.
labelMap 객체가 JSX 내부(140-145행)에서 다시 한 번 중복으로 정의되고 있습니다. 이 객체를 컴포넌트 상단에 상수로 정의하여 재사용하면 코드 중복을 줄이고 유지보수성을 높일 수 있습니다. 예를 들어, EMAIL_STATE_LABEL_MAP이라는 상수를 만들어 onSendEmailAll 함수와 JSX에서 모두 활용하는 것을 고려해보세요.
| <button | ||
| key={state} | ||
| type="button" | ||
| className="border px-4 py-2 rounded-lg truncate hover:bg-primary-100" |
There was a problem hiding this comment.
PR 설명에서 truncate 클래스를 제거하여 텍스트 잘림 현상을 수정했다고 하셨는데, 새로 추가된 일괄 발송 버튼에는 truncate 클래스가 여전히 존재합니다. 버튼 텍스트("1차 합격자 일괄 발송" 등)가 길어서 잘릴 가능성이 있습니다. 의도된 동작이 아니라면 truncate 클래스를 제거하는 것을 고려해보세요.
| className="border px-4 py-2 rounded-lg truncate hover:bg-primary-100" | |
| className="border px-4 py-2 rounded-lg hover:bg-primary-100" |
2yunseong
left a comment
2yunseong
left a comment
There was a problem hiding this comment.
고생하셨습니다! 한 가지 요청사항이 있는데, 버튼이 테이블 뷰를 해치는 것 같아요. 버튼들을 위로 올리는 것은 어떨까요?
| [ | ||
| "first-passed", | ||
| "first-failed", | ||
| "final-passed", | ||
| "final-failed", | ||
| ] as EmailState[] |
There was a problem hiding this comment.
배열이 너무 하드하게 인라인된 구조입니다.
별도 상수로 분리해내면 어떨까요?
타입 단언까지 있어서 가독성이나 코드 유연함이 부족하다는 생각이 드네요!
cho-in-sik
left a comment
cho-in-sik
left a comment
There was a problem hiding this comment.
고생하셨습니다~! 저도 윤성님 의견에 동의합니다. 테이블 상단에 있는 일괄 발송 버튼들의 위치 조정이 필요할 것 같아요. 작은 헤더 태그를 추가해서 "일괄 발송" 기능임을 명확히 표시하면 사용자가 기능을 더 쉽게 이해할 수 있을 것 같습니다. 추가로 현재 이메일 발송 후 성공/실패에 대한 피드백이 없어서 사용자 입장에서 혼란이 있을 수 있을 것 같아요. 일단은 간단한 alert 알림창이라도 추가해두는 게 어떨까요?
dle234
left a comment
dle234
left a comment
There was a problem hiding this comment.
위에 리뷰를 잘 달아주셔서 말씀대로 성공처리, 버튼 위치 추가로 개선해주시면 좋을 것 같습니다! 고생하셨습니다!
|
넵 알겠습니다 |
5 participants
작업 분류
PR을 통해 해결하려는 문제가 무엇인가요? 🚀
합격 상태 관리 페이지에서 지원자에게 결과 이메일을 발송하는 기능이 없었습니다.
또한 기존 코드에 배열 원본 변이, early return 블록 스코프 오류, 확인 메시지 영문 노출 등의 버그가 존재했습니다.
PR에서 핵심적으로 변경된 부분이 어떤 부분인가요? 👀
핵심 변경사항 이외 추가적으로 변경된 사항이 있나요? ➕
추가적으로, 리뷰어가 리뷰하며 알아야 할 정보가 있나요? 🙌
이런 부분을 신경써서 봐주셨으면 좋겠어요. 🙋🏻♂️
일괄 발송 시 재발송 방지 로직이 없어 중복 발송이 가능한 상태인데,
이 부분을 어떻게 처리할지 의견 부탁드립니다.
체크리스트 ✅
reviewers설정assignees설정label설정