[feature] 지원서 수정 요청시에 설명필드가 비어있는지 검증을 한다#1216
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
검증 로직 추가 backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java |
title 필드에 @NotBlank 어노테이션 추가, 내부 양식(isInternalFormValid())과 외부 양식(isExternalFormValid()) 유효성 검증 메서드 구현 |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
- [feature] 관리자 지원서 페이지에 설명을 추가한다 #563: 동일한 신청 양식
description필드에 대해 프론트엔드는 텍스트 영역과 타입을 추가하고 백엔드는 설명에 대한 유효성 검증을 추가하므로 관련이 있습니다.
Suggested reviewers
- Zepelown
- SeongHoonC
- oesnuj
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경사항의 주요 내용을 명확하게 설명하고 있습니다. 지원서 수정 요청 시 설명 필드 검증 추가라는 핵심 변화가 잘 드러나 있습니다. |
✏️ 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/#1215-add-validation-application-description-field-MOA-664
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.
Test Results99 tests 99 ✅ 17s ⏱️ Results for commit d003809. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java (1)
25-25:⚠️ Potential issue | 🟡 Minor
formMode가null이면 두 검증 메서드가 모두 무조건 통과합니다.
formMode필드에@NotNull이 없기 때문에,null값이 전달되면 두!=가드가 모두true를 반환해 cross-field 검증 전체가 우회됩니다.formMode가 필수 필드라면@NotNull을 추가해야 합니다.🛡️ 제안 수정
+ `@NotNull` ApplicationFormMode formMode,Also applies to: 36-38, 48-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java` at line 25, The formMode field in ClubApplicationFormEditRequest allows null so the cross-field guards that compare against formMode (the two validation methods referenced) can be bypassed; add `@NotNull` to the formMode field declaration in ClubApplicationFormEditRequest (and the same change in the other affected request classes/fields at the other locations) and import javax.validation.constraints.NotNull so that null values are rejected before the cross-field validation logic runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java`:
- Around line 34-53: Change the two private validation methods
isInternalFormValid() and isExternalFormValid() to public so they follow
JavaBeans conventions and avoid DataBinder NotReadablePropertyException during
validation; update the access modifier in ClubApplicationFormEditRequest for
both methods (and make the same change in ClubApplicationFormCreateRequest for
the equivalent methods) so Spring's validator can access the boolean properties
without causing 500 errors.
---
Outside diff comments:
In
`@backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java`:
- Line 25: The formMode field in ClubApplicationFormEditRequest allows null so
the cross-field guards that compare against formMode (the two validation methods
referenced) can be bypassed; add `@NotNull` to the formMode field declaration in
ClubApplicationFormEditRequest (and the same change in the other affected
request classes/fields at the other locations) and import
javax.validation.constraints.NotNull so that null values are rejected before the
cross-field validation logic runs.
| @AssertTrue(message = "지원서 양식에 필요한 필드가 누락되었습니다.") | ||
| private boolean isInternalFormValid() { | ||
| if (formMode != ApplicationFormMode.INTERNAL) { | ||
| return true; | ||
| } | ||
|
|
||
| boolean hasDescription = StringUtils.hasText(description); | ||
| boolean hasQuestions = questions != null && !questions.isEmpty(); | ||
|
|
||
| return hasDescription && hasQuestions; | ||
| } | ||
|
|
||
| @AssertTrue(message = "외부 링크가 누락되었습니다.") | ||
| private boolean isExternalFormValid() { | ||
| if (formMode != ApplicationFormMode.EXTERNAL) { | ||
| return true; | ||
| } | ||
|
|
||
| return StringUtils.hasText(externalApplicationUrl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file to confirm the code snippet
cat -n backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java | head -60Repository: Moadong/moadong
Length of output: 1760
🏁 Script executed:
# Search for other `@AssertTrue` usage in the codebase to understand patterns
rg -n --type java '@AssertTrue' -A 2 -B 1Repository: Moadong/moadong
Length of output: 2125
🏁 Script executed:
# Check if there are any other validation constraint methods (private or public) in the codebase
rg -n --type java '(private|public)\s+boolean\s+is[A-Z]' -A 1 | head -50Repository: Moadong/moadong
Length of output: 2973
🌐 Web query:
Spring DataBinder @AssertTrue private method NotReadablePropertyException validation
💡 Result:
In Spring MVC / DataBinder-triggered Bean Validation (@Valid on a form/model attribute), Hibernate Validator validates bean properties (fields or JavaBeans getters). If you put @AssertTrue on a private helper method that is not a JavaBeans getter, Spring/HV won’t treat it as a constraint to execute; instead you can end up with Spring trying to resolve it as a property and failing with NotReadablePropertyException (no readable getter for that “property”). (stackoverflow.com)
Fixes
-
Make it a real JavaBeans boolean getter and annotate that
`@AssertTrue`(message = "...") public boolean isValid() { ... } // no args; name starts with is/get
This is the standard way for cross-field checks with
@AssertTruein bean validation. (stackoverflow.com) -
Or annotate a field instead of a method
`@AssertTrue`(message="...") private boolean valid; public boolean isValid() { return valid; }
(Field constraints don’t depend on getter naming; the validator can access fields directly.) (docs.hibernate.org)
-
If you truly want “method validation” (arbitrary methods), don’t use
DataBinderfor that
Use executable/method validation (e.g., Spring’s@Validatedon service methods).DataBindervalidation is about validating the bound bean, not invoking arbitrary private methods. (docs.spring.io)
Citations:
- 1: https://stackoverflow.com/questions/47995603/method-marked-with-asserttrue-is-not-invoked?utm_source=openai
- 2: https://stackoverflow.com/questions/47995603/method-marked-with-asserttrue-is-not-invoked?utm_source=openai
- 3: https://docs.hibernate.org/validator/4.2/reference/en-US/html/validator-usingvalidator.html?utm_source=openai
- 4: https://docs.spring.io/spring-framework/docs/4.1.x/spring-framework-reference/html/validation.html?utm_source=openai
🌐 Web query:
Bean Validation @AssertTrue property constraint public getter requirement specification
💡 Result:
In Bean Validation, a property constraint such as @AssertTrue may be placed on a JavaBeans getter (i.e., the read accessor). The specification’s requirements are:
- The constrained property accessor must follow JavaBeans getter conventions:
getX()(no parameters) or, for boolean properties,isX()(no parameters, returnsboolean). The property name is derived viajava.beans.Introspector.decapitalize(...). [1] - Visibility is not constrained by the specification: “The fields or methods visibility are not constrained.” So a constrained getter is not required to be
publicby the Bean Validation spec (though some frameworks/contexts may impose additional rules outside the spec). [1] - Static fields and static methods are excluded from validation. [2]
Sources:
[1] Bean Validation spec, “Field and property validation” (JavaBeans conventions + visibility not constrained) https://beanvalidation.org/latest-draft/spec/
[2] Bean Validation spec, “Requirements on classes to be validated” (static excluded) https://beanvalidation.org/latest-draft/spec/
private 접근자의 @AssertTrue 메서드는 Spring의 데이터 바인더에서 유효성 검증 실패 시 문제를 유발할 수 있습니다.
Bean Validation 사양은 제약 조건을 private 메서드에 적용하는 것을 금지하지 않지만, Spring MVC의 DataBinder는 유효성 검증 실패 시 JavaBeans 규약에 따라 프로퍼티를 조회하려 시도합니다. 이 경우 internalFormValid, externalFormValid 프로퍼티가 접근 불가능하면 NotReadablePropertyException을 발생시켜 400 오류 대신 500 오류를 반환할 수 있습니다.
접근 제어자를 public으로 변경하여 JavaBeans 컨벤션을 준수하면 이 문제가 해결되며, 코드의 의도도 명확해집니다.
제안 수정
- `@AssertTrue`(message = "지원서 양식에 필요한 필드가 누락되었습니다.")
- private boolean isInternalFormValid() {
+ `@AssertTrue`(message = "지원서 양식에 필요한 필드가 누락되었습니다.")
+ public boolean isInternalFormValid() {
- `@AssertTrue`(message = "외부 링크가 누락되었습니다.")
- private boolean isExternalFormValid() {
+ `@AssertTrue`(message = "외부 링크가 누락되었습니다.")
+ public boolean isExternalFormValid() {참고: 동일한 패턴이 ClubApplicationFormCreateRequest.java에도 존재하므로 함께 수정이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/main/java/moadong/club/payload/request/ClubApplicationFormEditRequest.java`
around lines 34 - 53, Change the two private validation methods
isInternalFormValid() and isExternalFormValid() to public so they follow
JavaBeans conventions and avoid DataBinder NotReadablePropertyException during
validation; update the access modifier in ClubApplicationFormEditRequest for
both methods (and make the same change in ClubApplicationFormCreateRequest for
the equivalent methods) so Spring's validator can access the boolean properties
without causing 500 errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/moadong/club/payload/request/ClubApplicationFormCreateRequest.java`:
- Around line 16-18: The `@NotBlank` on the description field causes an unwanted
mandatory constraint in EXTERNAL mode; decide which behavior you want and
implement accordingly: either A) keep `@NotBlank` on String description and remove
the redundant hasDescription check from isInternalFormValid(), or B) remove the
field-level `@NotBlank` so description is optional like questions and validate
presence only inside isInternalFormValid(); update the class
ClubApplicationFormCreateRequest and its methods isInternalFormValid() /
isExternalFormValid() to reflect the chosen option consistently.
backend/src/main/java/moadong/club/payload/request/ClubApplicationFormCreateRequest.java
Outdated
Show resolved
Hide resolved
2 participants
#️⃣연관된 이슈
📝작업 내용
지원서 수정시 요청에 누락되었던 검증필드를 추가하였습니다
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트