Merged
Conversation
- validateApplicationForm 함수로 제목/설명 필수 및 최대 길이, 질문 제목, 외부 URL 검증
- 검증 실패 시 항목별 에러 메시지를 alert으로 표시
- FormTitle에 maxLength={50} 추가
- 기존 인라인 URL 검증 로직을 validateApplicationForm으로 통합
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedThe pull request is closed. Warning
|
| Cohort / File(s) | Summary |
|---|---|
폼 제출 검증 통합 frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx |
제출 전 validateApplicationForm 호출 추가 및 에러 집계/alert 처리 추가. 기존의 useRef import 및 인라인 외부 URL 허용체크(화이트리스트) 제거. 제목 입력에 maxLength(50) 설정. 제출(create/update) 흐름은 검증 통과 후에만 진행. |
폼 검증 모듈 추가 frontend/src/pages/AdminPage/validation/validateApplicationForm.ts |
새 파일 추가: ApplicationFormErrors 인터페이스, validateApplicationForm(formData, mode, externalUrl) 구현(제목·설명·질문·외부URL 검증), hasErrors(errors) 유틸 및 내부 ALLOWED_EXTERNAL_URLS 허용목록 정의. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- [feature] 외부 지원서 링크를 추가할 수 있게한다 #881: ApplicationEditTab의 외부 URL/폼 모드 관련 로직 변경과 연관 — 외부 URL 처리 흐름이 겹침
- [feature & refactor] 객관식 항목 1개 허용 · 필수 입력 UX 개선 · Application 구조 리팩토링 #558: ApplicationEditTab 파일 경로/구조 변경과 연관 — 동일 컴포넌트 수정 내역이 있음
Suggested reviewers
- oesnuj
- lepitaaar
- suhyun113
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | 제목은 PR의 주요 변경 사항인 '지원서 저장 시 검증 추가'를 명확하게 요약하고 있으며, 변경 내용과 완전히 관련이 있습니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1219-application-edit-validation-MOA-666
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx (2)
189-195:maxLength={50}이 검증 로직의 50자 제한과 일치하여 좋습니다.입력 단에서의 제한과 검증 로직의 제한이 동기화되어 있어 일관성이 보장됩니다. 다만 이 매직 넘버(50)가 두 곳에 분산되어 있으므로, 상수로 추출하면 유지보수가 더 쉬워질 수 있습니다.
♻️ 상수 추출 제안
validateApplicationForm.ts에서:+export const APPLICATION_TITLE_MAX_LENGTH = 50; +export const APPLICATION_DESCRIPTION_MAX_LENGTH = 3000;두 파일 모두에서 이 상수를 참조하면 변경 시 한 곳만 수정하면 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx` around lines 189 - 195, Extract the magic number 50 into a shared constant (e.g. MAX_TITLE_LENGTH) and use it both in the input component and in the validation module: replace the hardcoded maxLength in Styled.FormTitle (where handleFormTitleChange is used) with MAX_TITLE_LENGTH and update validateApplicationForm (the validation function) to import and use the same MAX_TITLE_LENGTH constant so both input-level restriction and validation logic remain synchronized; add the constant to a common constants file or export it from the validation module and update imports accordingly.
119-134: 검증-에러 표시 흐름이 잘 구현되어 있습니다. 단, UX 관련 개선 여지가 있습니다.검증 실패 시
alert로 모든 에러를 한꺼번에 보여주는 방식은 기능적으로 동작하지만, 에러가 많을 경우 사용자 경험이 좋지 않을 수 있습니다. 향후에는 각 필드 옆에 인라인 에러 메시지를 표시하는 방식을 고려해 보세요.ApplicationFormErrors인터페이스가 이미 필드별 에러를 지원하는 구조이므로 확장이 용이합니다.또한 Line 129의
Object.values(validationErrors.questions ?? {})는 객체 key 순서에 의존하므로, 질문 ID가 비순차적일 경우 에러 메시지 순서가 화면 상 질문 순서와 다를 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx` around lines 119 - 134, The current flow uses alert(...) to show all validationErrors (from validateApplicationForm + hasErrors) and flattens question errors with Object.values(validationErrors.questions ?? {}), which can misorder messages; replace the alert UX by storing validationErrors in component state (e.g., setValidationErrors) and render per-field inline messages in the form using the ApplicationFormErrors shape (show title/description/externalUrl errors next to their inputs and map question errors to the matching question row by question id), and when you still need a consolidated list, build it by iterating formData.questions in display order and collecting validationErrors.questions[question.id] to preserve ordering instead of Object.values(...).frontend/src/pages/AdminPage/validation/validateApplicationForm.ts (1)
25-29: 제목 길이 검증이trim()전 원본 길이를 기준으로 합니다.Line 25에서
trim()으로 빈 값을 확인하지만, Line 27에서는formData.title.length로 원본(공백 포함) 길이를 검사합니다." a "(길이 51)처럼 앞뒤 공백이 많은 경우, 실제 내용은 짧지만 길이 초과 에러가 발생할 수 있습니다.
ApplicationEditTab.tsx에서maxLength={50}을 input에 적용하고 있어 실질적 위험은 낮지만, 서버에서 trim 후 저장한다면 검증 로직도trim()후 길이를 체크하는 것이 일관적입니다. Description(Line 33)도 동일합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/validation/validateApplicationForm.ts` around lines 25 - 29, The title (and description) length checks use the raw formData.title.length which counts surrounding whitespace; change validation in validateApplicationForm.ts to trim first (e.g., compute a trimmedTitle = formData.title?.trim()) and use trimmedTitle for both the empty check and the >50 length check, and do the same for formData.description so errors.title/errors.description reflect trimmed content length consistently with ApplicationEditTab.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/pages/AdminPage/validation/validateApplicationForm.ts`:
- Around line 3-9: The current startsWith-based whitelist
(ALLOWED_EXTERNAL_URLS) allows host-prefix bypasses like
"https://forms.gle.evil.com"; replace the naive check with a robust URL/hostname
check: parse the input with the URL constructor, ensure protocol === 'https:',
then compare parsed.hostname against a whitelist of hostnames (e.g.,
ALLOWED_HOSTNAMES) using exact equality or .endsWith(`.${host}`) to allow
subdomains only of the intended domains; update the validation function that
currently uses ALLOWED_EXTERNAL_URLS (and the checks around lines ~59-61) to use
this isAllowedExternalUrl logic and return false on URL parse errors.
---
Nitpick comments:
In `@frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx`:
- Around line 189-195: Extract the magic number 50 into a shared constant (e.g.
MAX_TITLE_LENGTH) and use it both in the input component and in the validation
module: replace the hardcoded maxLength in Styled.FormTitle (where
handleFormTitleChange is used) with MAX_TITLE_LENGTH and update
validateApplicationForm (the validation function) to import and use the same
MAX_TITLE_LENGTH constant so both input-level restriction and validation logic
remain synchronized; add the constant to a common constants file or export it
from the validation module and update imports accordingly.
- Around line 119-134: The current flow uses alert(...) to show all
validationErrors (from validateApplicationForm + hasErrors) and flattens
question errors with Object.values(validationErrors.questions ?? {}), which can
misorder messages; replace the alert UX by storing validationErrors in component
state (e.g., setValidationErrors) and render per-field inline messages in the
form using the ApplicationFormErrors shape (show title/description/externalUrl
errors next to their inputs and map question errors to the matching question row
by question id), and when you still need a consolidated list, build it by
iterating formData.questions in display order and collecting
validationErrors.questions[question.id] to preserve ordering instead of
Object.values(...).
In `@frontend/src/pages/AdminPage/validation/validateApplicationForm.ts`:
- Around line 25-29: The title (and description) length checks use the raw
formData.title.length which counts surrounding whitespace; change validation in
validateApplicationForm.ts to trim first (e.g., compute a trimmedTitle =
formData.title?.trim()) and use trimmedTitle for both the empty check and the
>50 length check, and do the same for formData.description so
errors.title/errors.description reflect trimmed content length consistently with
ApplicationEditTab.tsx.
seongwon030
deleted the
feature/#1219-application-edit-validation-MOA-666
branch
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트