-
Notifications
You must be signed in to change notification settings - Fork 2
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
[DDING-000] 지원하기 요청 dto 필드 추가 및 폼지 상세 조회 응답에 지원자 수 필드 추가 #268
Conversation
Caution Review failedThe pull request is closed. """ Walkthrough이 PR은 UserForm 관련 응답 및 쿼리 객체에 새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant FacadeUserFormService
participant FormStatisticService
participant DataStore
Client->>Controller: getUserForm(formId, section) 호출
Controller->>FacadeUserFormService: getUserForm(formId, section) 전달
FacadeUserFormService->>FormStatisticService: getTotalApplicationCountByForm(form) 호출
FormStatisticService-->>FacadeUserFormService: applicationCount 반환
FacadeUserFormService->>DataStore: form 및 필드 조회 요청
DataStore-->>FacadeUserFormService: form, 필드 반환
FacadeUserFormService->>Controller: UserFormQuery (applicationCount 포함) 반환
Controller->>Client: UserFormResponse 전송
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/UserFormResponse.java (1)
61-61
: 음수 값 검증 추가를 고려해보세요.지원자 수는 음수가 될 수 없으므로, UserFormQuery에서 applicationCount가 음수인 경우에 대한 검증 로직 추가를 고려해보시면 좋을 것 같습니다.
예시 코드:
private static void validateApplicationCount(int count) { if (count < 0) { throw new IllegalArgumentException("지원자 수는 음수가 될 수 없습니다."); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/UserFormResponse.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImpl.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/UserFormQuery.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/api/UserFormApplicationApi.java
(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/UserFormApplicationController.java
(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/dto/request/CreateFormApplicationRequest.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/dto/response/FormApplicationCountResponse.java
(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.java
(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java
(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/dto/command/CreateFormApplicationCommand.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/dto/query/FormApplicationCountQuery.java
(0 hunks)
💤 Files with no reviewable changes (6)
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.java
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/dto/query/FormApplicationCountQuery.java
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/dto/response/FormApplicationCountResponse.java
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/api/UserFormApplicationApi.java
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/UserFormApplicationController.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (3)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImpl.java (1)
19-19
: 구현이 깔끔하고 적절합니다!
FormStatisticService
의존성 주입과applicationCount
통합이 잘 되어있습니다. 단일 책임 원칙을 잘 지키면서 기능을 확장했습니다.Also applies to: 31-32
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/UserFormQuery.java (1)
10-28
: 잘 구현되었습니다!
UserFormQuery
레코드에applicationCount
필드 추가와 빌더 패턴 활용이 깔끔합니다. 불변성을 잘 유지하면서 기능이 확장되었습니다.src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/UserFormResponse.java (1)
17-18
: 필드 추가가 잘 구현되었습니다!Schema 어노테이션을 통해 API 문서화가 잘 되어있고, 예시 값도 적절하게 제공되어 있습니다.
String email, | ||
String phoneNumber, |
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.
🛠️ Refactor suggestion
이메일 형식 검증 추가를 고려해보세요.
이메일 필드가 추가되었지만, 이메일 형식에 대한 검증이 없습니다. 잘못된 이메일 형식이 입력될 수 있습니다.
다음과 같이 이메일 형식 검증을 추가하는 것을 추천드립니다:
public record CreateFormApplicationCommand(
Long formId,
String name,
String studentNumber,
String department,
+ @Email(message = "올바른 이메일 형식이 아닙니다.")
String email,
String phoneNumber,
FormApplicationStatus status,
List<CreateFormAnswerCommand> formAnswerCommands
)
Also applies to: 43-44
@NotNull(message = "지원자 이메일은 필수 입력 사항입니다.") | ||
@Schema(description = "이메일", example = "[email protected]") | ||
String email, | ||
|
||
@NotNull(message = "지원자 전화번호는 필수 입력 사항입니다.") | ||
@Schema(description = "전화번호", example = "010-0000-0000") | ||
String phoneNumber, |
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.
🛠️ Refactor suggestion
전화번호 형식 검증 추가를 고려해보세요.
전화번호 필드에 @NotNull
검증은 있지만, 전화번호 형식에 대한 검증이 없습니다. 잘못된 형식의 전화번호가 입력될 수 있습니다.
다음과 같이 정규식을 사용한 전화번호 형식 검증을 추가하는 것을 추천드립니다:
+ @Pattern(
+ regexp = "^\\d{3}-\\d{3,4}-\\d{4}$",
+ message = "전화번호 형식이 올바르지 않습니다. (예: 010-0000-0000)"
+ )
@NotNull(message = "지원자 전화번호는 필수 입력 사항입니다.")
@Schema(description = "전화번호", example = "010-0000-0000")
String phoneNumber,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@NotNull(message = "지원자 이메일은 필수 입력 사항입니다.") | |
@Schema(description = "이메일", example = "[email protected]") | |
String email, | |
@NotNull(message = "지원자 전화번호는 필수 입력 사항입니다.") | |
@Schema(description = "전화번호", example = "010-0000-0000") | |
String phoneNumber, | |
@NotNull(message = "지원자 이메일은 필수 입력 사항입니다.") | |
@Schema(description = "이메일", example = "[email protected]") | |
String email, | |
+ @Pattern( | |
+ regexp = "^\\d{3}-\\d{3,4}-\\d{4}$", | |
+ message = "전화번호 형식이 올바르지 않습니다. (예: 010-0000-0000)" | |
+ ) | |
@NotNull(message = "지원자 전화번호는 필수 입력 사항입니다.") | |
@Schema(description = "전화번호", example = "010-0000-0000") | |
String phoneNumber, |
|
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.
확인하였습니다. 코멘트 1개 남긴거만 고치고 머지하면 될 것 같아요!
List<FormField> formFieldList = formFieldService.getAllByFormAndSection(form, section); | ||
return UserFormQuery.from(form, formFieldList); | ||
int applicationCount = formStatisticService.getTotalApplicationCountByForm(form); | ||
return UserFormQuery.from(form, applicationCount, formFieldList); |
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.
p3) formFieldList -> formFields 로 컨벤션 맞추는 게 좋을 것 같아요
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.
수정했습니다!
🚀 작업 내용
프론트 요청사항에 따라
지원하기 요청 dto에 이메일, 전화번호를 추가하였습니다.
(User)폼지 상세 조회 응답에서 지원자 수를 내려주도록 변경하였습니다.
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
신규 기능
버그 수정