[DDING-85] 동아리 지원폼지 생성 API 구현 #230
Merged
Conversation
This file contains 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
KoSeonJe
requested review from
wonjunYou,
5uhwann and
Seooooo24
as code owners
|
""" 워크스루이 풀 리퀘스트는 중앙 양식(Form) 생성을 위한 새로운 API와 관련 컴포넌트를 도입합니다. 애플리케이션의 양식 생성 프로세스를 지원하기 위해 API 인터페이스, 컨트롤러, 서비스, DTO 등 여러 계층에 걸친 새로운 클래스와 인터페이스가 추가되었습니다. 주요 목적은 사용자가 클럽 지원 양식을 생성할 수 있는 기능을 구현하는 것입니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant CentralFormController
participant FacadeCentralFormService
participant FormService
participant FormFieldService
participant ClubService
Client->>CentralFormController: createForm(request, principalDetails)
CentralFormController->>FacadeCentralFormService: createForm(command)
FacadeCentralFormService->>ClubService: 클럽 조회
FacadeCentralFormService->>FormService: 양식 생성
FacadeCentralFormService->>FormFieldService: 양식 필드 생성
FormFieldService-->>FacadeCentralFormService: 양식 필드 저장 완료
관련 가능한 풀 리퀘스트
제안된 레이블
제안된 리뷰어
✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java (2)
10-13: 클래스 수준의 문서화가 필요합니다.클래스의 책임과 역할을 명확히 하기 위해 Javadoc 문서화를 추가하는 것이 좋겠습니다.
다음과 같이 문서화를 추가해보세요:
+/** + * 폼 필드 생성을 담당하는 서비스 구현체입니다. + * 폼 필드들을 일괄적으로 저장하는 기능을 제공합니다. + */ @Service @RequiredArgsConstructor @Transactional(readOnly = true) public class GeneralFormFieldService implements FormFieldService {
17-21: 메서드 구현에 대한 개선 제안사항이 있습니다.현재 구현은 기능적으로는 문제가 없으나, 다음 사항들을 고려해보시면 좋겠습니다:
- 입력 파라미터 검증이 필요합니다
- 예외 처리 전략이 명확하지 않습니다
다음과 같이 개선해보세요:
@Transactional @Override public void createAll(List<FormField> formFields) { + if (formFields == null || formFields.isEmpty()) { + throw new IllegalArgumentException("폼 필드 목록은 비어있을 수 없습니다."); + } + try { formFieldRepository.saveAll(formFields); + } catch (Exception e) { + throw new RuntimeException("폼 필드 저장 중 오류가 발생했습니다.", e); + } }src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java (1)
6-9: Javadoc 문서화 추가를 제안드립니다.인터페이스와 메서드에 대한 문서화를 추가하면 좋을 것 같습니다. 특히:
- 인터페이스의 전반적인 목적
- createAll 메서드의 동작 방식
- 매개변수 설명
- 발생 가능한 예외 상황
다음과 같이 문서화를 추가하는 것을 제안드립니다:
+/** + * 폼 필드 생성을 담당하는 서비스 인터페이스입니다. + */ public interface FormFieldService { + /** + * 여러 폼 필드를 일괄 생성합니다. + * + * @param formFields 생성할 폼 필드 목록 + * @throws IllegalArgumentException formFields가 null이거나 비어있는 경우 + */ void createAll(List<FormField> formFields); }src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (1)
1-8: 인터페이스 설계가 깔끔하고 명확합니다.단일 책임 원칙(SRP)을 잘 준수하고 있으며, 메서드 시그니처가 명확합니다. 다만 다음 사항들을 고려해보시면 좋을 것 같습니다:
- Form 생성 실패 시의 예외 처리 방식을 JavaDoc으로 문서화하면 좋을 것 같습니다.
- Form 엔티티의 유효성 검증 책임이 어디에 있는지 명확히 하면 좋을 것 같습니다.
아래와 같이 JavaDoc을 추가하는 것을 제안드립니다:
package ddingdong.ddingdongBE.domain.form.service; import ddingdong.ddingdongBE.domain.form.entity.Form; +/** + * 동아리 지원 폼 생성을 위한 서비스 인터페이스 + */ public interface FormService { + /** + * 새로운 동아리 지원 폼을 생성합니다. + * + * @param form 생성할 폼 엔티티 + * @return 저장된 폼 엔티티 + * @throws IllegalArgumentException 폼 데이터가 유효하지 않은 경우 + */ Form create(Form form); }src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (1)
25-28: API 문서화 개선이 필요합니다.현재 API 명세에 다음 사항들이 누락되어 있습니다:
- 요청 본문(Request Body)에 대한 상세 설명
- 가능한 오류 응답에 대한 명세
- 입력값 제약조건 (@Valid 어노테이션)
다음과 같이 수정을 제안드립니다:
void createForm( - @RequestBody CreateFormRequest createFormRequest, + @RequestBody @Valid CreateFormRequest createFormRequest, @AuthenticationPrincipal PrincipalDetails principalDetails );추가로 다음 어노테이션들을 추가하는 것을 권장드립니다:
@ApiResponse(responseCode = "400", description = "잘못된 요청") @ApiResponse(responseCode = "401", description = "인증 실패") @ApiResponse(responseCode = "403", description = "권한 없음")src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/CreateFormRequest.java (1)
70-83: toCommand 메서드의 방어적 프로그래밍이 필요합니다.formFields가 null인 경우에 대한 처리가 필요합니다. 또한 user 파라미터에 대한 null 체크도 추가되어야 합니다.
다음과 같이 수정하는 것을 제안드립니다:
public CreateFormCommand toCommand(User user) { + if (user == null) { + throw new IllegalArgumentException("사용자 정보는 null이 될 수 없습니다."); + } + + List<CreateFormFieldCommand> createFormFieldCommands = formFields == null + ? List.of() + : formFields.stream() - List<CreateFormFieldCommand> createFormFieldCommands = formFields.stream() .map(CreateFormFieldRequest::toCommand) .toList();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/CreateFormRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/command/CreateFormCommand.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (15)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java (2)
1-9: 패키지 구조와 임포트가 잘 구성되어 있습니다!필요한 모든 의존성이 적절하게 임포트되어 있습니다.
15-15: 의존성 주입이 적절하게 구현되어 있습니다!생성자 주입 방식을 사용하여 불변성을 보장하고 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java (2)
1-9: 인터페이스 설계가 깔끔합니다!단일 책임 원칙을 잘 준수하고 있으며, 메서드 명이 목적을 명확하게 전달합니다.
8-8: 트랜잭션 처리 확인이 필요합니다.구현체에서
@Transactional어노테이션을 통해 트랜잭션 처리가 되어있는지 확인이 필요합니다. 여러 폼 필드를 일괄 생성하는 과정에서 부분 실패가 발생할 수 있기 때문입니다.다음 스크립트로 구현체의 트랜잭션 처리를 확인해보겠습니다:
✅ Verification successful
트랜잭션이 올바르게 처리되어 있습니다.
GeneralFormFieldService 구현체에서 createAll 메소드에
@Transactional어노테이션이 적용되어 있어 일괄 생성 작업의 원자성이 보장됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # FormFieldService 구현체에서 @Transactional 어노테이션 사용 여부 확인 rg -l "class.*implements.*FormFieldService" | xargs rg "@Transactional"Length of output: 122
Script:
#!/bin/bash # Get more context around the @Transactional annotations in the FormFieldService implementation rg -l "class.*implements.*FormFieldService" | xargs rg -B 2 -A 5 "@Transactional"Length of output: 421
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (1)
7-7: 구현 클래스에서의 saveAll() 사용을 확인해주세요.PR 설명에서 언급하신
saveAll()사용에 대한 구현 세부사항을 확인하기 위해 구현 클래스를 검토할 필요가 있습니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (3)
1-8: 패키지 구조와 임포트가 깔끔합니다!필요한 모든 의존성이 잘 정의되어 있습니다.
9-11: readOnly 기본값 설정이 적절한지 확인해주세요.클래스 레벨에서
@Transactional(readOnly = true)를 설정하면 모든 메서드가 기본적으로 읽기 전용 트랜잭션으로 동작합니다. 현재는create메서드에 별도의@Transactional을 지정했지만, 향후 다른 쓰기 메서드가 추가될 때 실수로 누락될 수 있습니다.
16-20: 🛠️ Refactor suggestion메서드 구현을 보완해주세요.
현재 구현에서 개선이 필요한 부분들입니다:
- 입력값 검증이 없습니다
- 예외 처리가 없습니다
- 로깅이 없습니다
다음과 같이 수정하는 것을 제안드립니다:
@Transactional @Override public Form create(Form form) { + validateForm(form); + try { + log.info("폼 생성 시작: {}", form); return formRepository.save(form); + } catch (Exception e) { + log.error("폼 생성 실패: {}", e.getMessage(), e); + throw new FormCreationException("폼 생성에 실패했습니다", e); + } } + +private void validateForm(Form form) { + if (form == null) { + throw new IllegalArgumentException("폼이 null일 수 없습니다"); + } + // 추가적인 유효성 검사 로직 +}또한, 테스트 코드 작성도 꼭 진행해주시기 바랍니다.
✅ Verification successful
리뷰 의견이 타당합니다.
다음과 같은 이유로 코드 개선이 필요합니다:
- Form 엔티티는
@Column(nullable = false)로 지정된 필수 필드들이 있어 검증이 필요합니다.- 프로젝트의 다른 서비스들은 대부분 더 견고한 create 메서드를 구현하고 있습니다.
- 로깅 설정이
logback-${spring.profiles.active}.xml로 구성되어 있으나 활용되지 않고 있습니다.- 다른 서비스들과 달리 예외 처리가 누락되어 있습니다.
리뷰어가 제안한 개선사항들이 프로젝트의 전반적인 패턴과 일치하며, 코드의 안정성을 높이는데 도움이 될 것입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Check Form class definition and any existing validation echo "=== Form Class Definition ===" ast-grep --pattern 'class Form { $$$ }' echo -e "\n=== Logging Configuration ===" # Look for logging configuration in application properties and logging imports rg -l "logging\." --type properties rg "import.*log" --type java echo -e "\n=== Similar Service Patterns ===" # Find similar service implementations rg "class \w+Service" --type java -A 5 echo -e "\n=== Exception Classes ===" # Look for custom exception classes fd "Exception.java$"Length of output: 58172
Script:
#!/bin/bash # Check Form entity and its validation echo "=== Form Entity ===" fd "Form.java$" --type f --exec cat {} echo -e "\n=== Application Properties ===" fd "application.*properties$|application.*yml$" --type f --exec cat {} echo -e "\n=== Similar Create Methods ===" ast-grep --pattern 'create($$$) { $$$ }'Length of output: 5441
Script:
#!/bin/bash # Check other service implementations echo "=== Similar Service Methods ===" rg "public \w+ create\(" --type java -B 1 -A 5 echo -e "\n=== Service Base Classes ===" rg "extends BaseEntity" --type java -B 2 -A 2Length of output: 27741
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (1)
5-8: 인터페이스 설계가 깔끔합니다!단일 책임 원칙을 잘 따르고 있으며, 메서드 시그니처가 명확합니다.
src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java (1)
17-24: 🛠️ Refactor suggestion요청 유효성 검증 로직이 필요합니다.
현재 구현에서는 다음 사항들에 대한 처리가 누락되어 있습니다:
- createFormRequest에 대한 유효성 검증 (@Valid 어노테이션)
- 예외 처리 로직
다음과 같이 수정을 제안드립니다:
@Override public void createForm( - CreateFormRequest createFormRequest, + @Valid CreateFormRequest createFormRequest, PrincipalDetails principalDetails ) { User user = principalDetails.getUser(); - facadeCentralFormService.createForm(createFormRequest.toCommand(user)); + try { + facadeCentralFormService.createForm(createFormRequest.toCommand(user)); + } catch (Exception e) { + throw new FormCreationException("폼 생성 중 오류가 발생했습니다", e); + } }✅ Verification successful
@Valid 어노테이션 추가가 필요합니다.
코드베이스의 다른 컨트롤러들과 일관성을 유지하기 위해 다음과 같이 수정해 주세요:
@Override public void createForm( - CreateFormRequest createFormRequest, + @Valid CreateFormRequest createFormRequest, PrincipalDetails principalDetails ) { User user = principalDetails.getUser(); facadeCentralFormService.createForm(createFormRequest.toCommand(user)); }🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Check for @Valid usage and exception handling patterns in controllers echo "=== Searching for @Valid usage ===" rg "@Valid" -t java echo -e "\n=== Searching for FormCreationException ===" fd "FormCreationException.java" || rg "FormCreationException" echo -e "\n=== Searching for try-catch blocks in controllers ===" rg "try\s*\{.*catch.*Exception" -U --multiline -t javaLength of output: 2753
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)
34-38: 성능 최적화가 필요할 수 있습니다.Stream 연산을 통한 엔티티 변환 과정에서 N+1 쿼리가 발생할 가능성이 있습니다. 현재는 데이터 양이 적어 문제가 되지 않을 수 있으나, 향후 확장성을 고려하여 모니터링이 필요합니다.
1-39: 테스트 코드 작성이 필요합니다.PR 설명에 언급된 대로 테스트 코드가 누락되어 있습니다. 다음 시나리오들에 대한 테스트 작성을 권장드립니다:
- 정상적인 폼 생성 케이스
- 권한이 없는 사용자의 폼 생성 시도
- 잘못된 폼 필드 데이터로 인한 실패
- 트랜잭션 롤백 시나리오
테스트 코드 작성을 도와드릴까요?
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/command/CreateFormCommand.java (3)
33-43: 엔티티 생성 시 추가 유효성 검증이 필요할 수 있습니다.Form과 FormField 엔티티 생성 시 null 체크나 기타 비즈니스 규칙에 대한 검증이 필요할 수 있습니다. 특히 Form 엔티티의 경우 club이 null이 아닌지 확인이 필요합니다.
Also applies to: 46-55
17-18: 🛠️ Refactor suggestion시작일자와 종료일자 유효성 검증이 필요합니다.
시작일자가 종료일자보다 이후일 수 없도록 유효성 검증 로직이 필요합니다.
다음과 같이 검증 로직을 추가하는 것을 제안드립니다:
@Builder public record CreateFormCommand( User user, String title, String description, LocalDate startDate, LocalDate endDate, boolean hasInterview, List<CreateFormFieldCommand> formFieldCommands ) { + public CreateFormCommand { + if (startDate != null && endDate != null && startDate.isAfter(endDate)) { + throw new IllegalArgumentException("시작일자는 종료일자보다 이후일 수 없습니다."); + } + }✅ Verification successful
날짜 유효성 검증 누락이 확인되었습니다.
코드베이스 전체를 검토한 결과, 시작일자와 종료일자 간의 비교 검증이 구현되어 있지 않습니다. 리뷰 코멘트에서 제안된 CreateFormCommand 레코드 생성자에 유효성 검증을 추가하는 방안이 적절해 보입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for date validation related to forms rg -l "startDate|endDate" --type java # Look for validation logic in form related files ast-grep --pattern 'if ($startDate.isAfter($endDate)) { $$$ }' # Search for form validation classes fd -e java validator fd -e java validation # Check form service implementations rg -l "FormService|FormValidator" --type javaLength of output: 2473
Script:
#!/bin/bash # Check Form entity implementation cat src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java # Check CreateFormRequest implementation cat src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/CreateFormRequest.java # Check service implementations cat src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java cat src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.javaLength of output: 6582
24-31: 🛠️ Refactor suggestion필드 타입에 따른 options 유효성 검증이 필요합니다.
체크박스나 라디오 버튼과 같은 필드 타입의 경우 options가 비어있으면 안 되지만, 텍스트 입력 필드의 경우 options가 불필요합니다. 필드 타입에 따른 options의 유효성 검증이 필요합니다.
다음과 같이 검증 로직을 추가하는 것을 제안드립니다:
public record CreateFormFieldCommand( String question, FieldType type, List<String> options, boolean required, int order, String section ) { + public CreateFormFieldCommand { + if ((type == FieldType.CHECK_BOX || type == FieldType.RADIO) && + (options == null || options.isEmpty())) { + throw new IllegalArgumentException( + "선택형 필드는 최소 하나 이상의 옵션이 필요합니다."); + } + if (type == FieldType.TEXT && options != null && !options.isEmpty()) { + throw new IllegalArgumentException( + "텍스트 필드는 옵션을 가질 수 없습니다."); + } + }✅ Verification successful
필드 타입에 따른 options 유효성 검증이 필요합니다.
현재 코드베이스에서 FieldType에 따른 options 필드의 유효성 검증이 구현되어 있지 않습니다. CHECK_BOX와 RADIO 타입은 options가 필수이며, TEXT 타입은 options가 불필요하므로 리뷰 코멘트에서 제안된 검증 로직 추가가 필요합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find FieldType enum definition echo "=== Searching for FieldType enum ===" ast-grep --pattern 'enum FieldType' echo -e "\n=== Searching for FieldType usage with validation ===" rg "FieldType.*options" -A 5 -B 5 echo -e "\n=== Checking for existing validation patterns ===" ast-grep --pattern 'if.*FieldType.*options'Length of output: 440
Script:
#!/bin/bash # Find files containing FieldType echo "=== Finding files with FieldType ===" fd -e java FieldType echo -e "\n=== Searching for FieldType definition ===" rg "enum.*FieldType|FieldType.*enum" --type java echo -e "\n=== Searching for form validation patterns ===" rg "validate|validation" --type java -A 5 -B 5 echo -e "\n=== Looking for form field related classes ===" fd -e java "Form.*Field|Field.*Form"Length of output: 62435
Script:
#!/bin/bash # Check FieldType enum definition echo "=== FieldType enum definition ===" cat src/main/java/ddingdong/ddingdongBE/domain/form/entity/FieldType.java echo -e "\n=== FormField entity implementation ===" cat src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java echo -e "\n=== FormField service implementation ===" cat src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.javaLength of output: 2627
Comment on lines
+23
to
+32
| @Transactional | ||
| @Override | ||
| public void createForm(CreateFormCommand createFormCommand) { | ||
| Club club = clubService.getByUserId(createFormCommand.user().getId()); | ||
| Form form = createFormCommand.toEntity(club); | ||
| Form savedForm = formService.create(form); | ||
|
|
||
| List<FormField> formFields = toFormFields(savedForm, createFormCommand.formFieldCommands()); | ||
| formFieldService.createAll(formFields); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션 내 검증 로직 추가가 필요합니다.
현재 구현에서는 엔티티 저장 전 비즈니스 규칙 검증이 누락되어 있습니다. 다음 사항들의 검증이 필요합니다:
- 클럽의 폼 생성 권한 확인
- 폼 필드의 유효성 검증
다음과 같이 수정을 제안드립니다:
@Transactional
@Override
public void createForm(CreateFormCommand createFormCommand) {
Club club = clubService.getByUserId(createFormCommand.user().getId());
+ validateFormCreationPermission(club);
+ validateFormFields(createFormCommand.formFieldCommands());
Form form = createFormCommand.toEntity(club);
Form savedForm = formService.create(form);
List<FormField> formFields = toFormFields(savedForm, createFormCommand.formFieldCommands());
formFieldService.createAll(formFields);
}Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
38
to
56
| record CreateFormFieldRequest( | ||
| @Schema(description = "폼지 질문", example = "우리 동아리 들어올겁니까?") | ||
| String question, | ||
|
|
||
| @Schema(description = "질문 종류", example = "CHECK_BOX") | ||
| FieldType type, | ||
|
|
||
| @Schema(description = "질문의 선택리스트", example = "[지문1이다., 지문2이다., 지문3이다.]") | ||
| List<String> options, | ||
|
|
||
| @Schema(description = "필수여부", example = "true") | ||
| boolean required, | ||
|
|
||
| @Schema(description = "질문 순서", example = "1") | ||
| int order, | ||
|
|
||
| @Schema(description = "질문 섹션 종류", example = "공통") | ||
| String section | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CreateFormFieldRequest의 필드 제약조건이 부족합니다.
다음 필드들에 대한 추가 제약조건이 필요합니다:
다음과 같이 제약조건을 추가하는 것을 제안드립니다:
record CreateFormFieldRequest(
@Schema(description = "폼지 질문", example = "우리 동아리 들어올겁니까?")
+ @NotNull(message = "질문은 null이 될 수 없습니다.")
+ @Size(min = 1, max = 200, message = "질문은 1자 이상 200자 이하여야 합니다.")
String question,
@Schema(description = "질문 종류", example = "CHECK_BOX")
+ @NotNull(message = "질문 종류는 null이 될 수 없습니다.")
FieldType type,
@Schema(description = "질문의 선택리스트", example = "[지문1이다., 지문2이다., 지문3이다.]")
List<String> options,
@Schema(description = "필수여부", example = "true")
boolean required,
@Schema(description = "질문 순서", example = "1")
+ @Min(value = 1, message = "질문 순서는 1 이상이어야 합니다.")
int order,
@Schema(description = "질문 섹션 종류", example = "공통")
+ @NotNull(message = "섹션 종류는 null이 될 수 없습니다.")
+ @Size(min = 1, max = 50, message = "섹션 종류는 1자 이상 50자 이하여야 합니다.")
String section
)📝 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.
Suggested change
| record CreateFormFieldRequest( | |
| @Schema(description = "폼지 질문", example = "우리 동아리 들어올겁니까?") | |
| String question, | |
| @Schema(description = "질문 종류", example = "CHECK_BOX") | |
| FieldType type, | |
| @Schema(description = "질문의 선택리스트", example = "[지문1이다., 지문2이다., 지문3이다.]") | |
| List<String> options, | |
| @Schema(description = "필수여부", example = "true") | |
| boolean required, | |
| @Schema(description = "질문 순서", example = "1") | |
| int order, | |
| @Schema(description = "질문 섹션 종류", example = "공통") | |
| String section | |
| ) { | |
| record CreateFormFieldRequest( | |
| @Schema(description = "폼지 질문", example = "우리 동아리 들어올겁니까?") | |
| @NotNull(message = "질문은 null이 될 수 없습니다.") | |
| @Size(min = 1, max = 200, message = "질문은 1자 이상 200자 이하여야 합니다.") | |
| String question, | |
| @Schema(description = "질문 종류", example = "CHECK_BOX") | |
| @NotNull(message = "질문 종류는 null이 될 수 없습니다.") | |
| FieldType type, | |
| @Schema(description = "질문의 선택리스트", example = "[지문1이다., 지문2이다., 지문3이다.]") | |
| List<String> options, | |
| @Schema(description = "필수여부", example = "true") | |
| boolean required, | |
| @Schema(description = "질문 순서", example = "1") | |
| @Min(value = 1, message = "질문 순서는 1 이상이어야 합니다.") | |
| int order, | |
| @Schema(description = "질문 섹션 종류", example = "공통") | |
| @NotNull(message = "섹션 종류는 null이 될 수 없습니다.") | |
| @Size(min = 1, max = 50, message = "섹션 종류는 1자 이상 50자 이하여야 합니다.") | |
| String section | |
| ) { |
Comment on lines
+14
to
+36
| public record CreateFormRequest( | ||
| @Schema(description = "폼지 제목", example = "폼지제목입니다") | ||
| @NotNull(message = "폼지 제목은 null이 될 수 없습니다.") | ||
| String title, | ||
|
|
||
| @Schema(description = "폼지 설명", example = "우리 동아리는 띵동입니다.") | ||
| String description, | ||
|
|
||
| @Schema(description = "폼지 시작일자", example = "2001-01-01") | ||
| @NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.") | ||
| LocalDate startDate, | ||
|
|
||
| @Schema(description = "폼지 종료일자", example = "2001-01-02") | ||
| @NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.") | ||
| LocalDate endDate, | ||
|
|
||
| @Schema(description = "면접여부", example = "true") | ||
| @NotNull(message = "면접여부는 null이 될 수 없습니다.") | ||
| boolean hasInterview, | ||
|
|
||
| @ArraySchema(schema = @Schema(implementation = CreateFormFieldRequest.class)) | ||
| List<CreateFormFieldRequest> formFields | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
추가적인 필드 유효성 검증이 필요합니다.
다음과 같은 추가 검증이 필요합니다:
- title의 최소/최대 길이 제한
- description의 최대 길이 제한
- formFields의 최소 개수 제한 (@SiZe 어노테이션 사용)
다음과 같이 검증을 추가하는 것을 제안드립니다:
public record CreateFormRequest(
@Schema(description = "폼지 제목", example = "폼지제목입니다")
@NotNull(message = "폼지 제목은 null이 될 수 없습니다.")
+ @Size(min = 1, max = 100, message = "폼지 제목은 1자 이상 100자 이하여야 합니다.")
String title,
@Schema(description = "폼지 설명", example = "우리 동아리는 띵동입니다.")
+ @Size(max = 500, message = "폼지 설명은 500자 이하여야 합니다.")
String description,
@Schema(description = "폼지 시작일자", example = "2001-01-01")
@NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.")
LocalDate startDate,
@Schema(description = "폼지 종료일자", example = "2001-01-02")
@NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.")
LocalDate endDate,
@Schema(description = "면접여부", example = "true")
@NotNull(message = "면접여부는 null이 될 수 없습니다.")
boolean hasInterview,
@ArraySchema(schema = @Schema(implementation = CreateFormFieldRequest.class))
+ @NotNull(message = "폼 필드 목록은 null이 될 수 없습니다.")
+ @Size(min = 1, message = "최소 하나 이상의 폼 필드가 필요합니다.")
List<CreateFormFieldRequest> formFields
)📝 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.
Suggested change
| public record CreateFormRequest( | |
| @Schema(description = "폼지 제목", example = "폼지제목입니다") | |
| @NotNull(message = "폼지 제목은 null이 될 수 없습니다.") | |
| String title, | |
| @Schema(description = "폼지 설명", example = "우리 동아리는 띵동입니다.") | |
| String description, | |
| @Schema(description = "폼지 시작일자", example = "2001-01-01") | |
| @NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.") | |
| LocalDate startDate, | |
| @Schema(description = "폼지 종료일자", example = "2001-01-02") | |
| @NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.") | |
| LocalDate endDate, | |
| @Schema(description = "면접여부", example = "true") | |
| @NotNull(message = "면접여부는 null이 될 수 없습니다.") | |
| boolean hasInterview, | |
| @ArraySchema(schema = @Schema(implementation = CreateFormFieldRequest.class)) | |
| List<CreateFormFieldRequest> formFields | |
| ) { | |
| public record CreateFormRequest( | |
| @Schema(description = "폼지 제목", example = "폼지제목입니다") | |
| @NotNull(message = "폼지 제목은 null이 될 수 없습니다.") | |
| @Size(min = 1, max = 100, message = "폼지 제목은 1자 이상 100자 이하여야 합니다.") | |
| String title, | |
| @Schema(description = "폼지 설명", example = "우리 동아리는 띵동입니다.") | |
| @Size(max = 500, message = "폼지 설명은 500자 이하여야 합니다.") | |
| String description, | |
| @Schema(description = "폼지 시작일자", example = "2001-01-01") | |
| @NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.") | |
| LocalDate startDate, | |
| @Schema(description = "폼지 종료일자", example = "2001-01-02") | |
| @NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.") | |
| LocalDate endDate, | |
| @Schema(description = "면접여부", example = "true") | |
| @NotNull(message = "면접여부는 null이 될 수 없습니다.") | |
| boolean hasInterview, | |
| @ArraySchema(schema = @Schema(implementation = CreateFormFieldRequest.class)) | |
| @NotNull(message = "폼 필드 목록은 null이 될 수 없습니다.") | |
| @Size(min = 1, message = "최소 하나 이상의 폼 필드가 필요합니다.") | |
| List<CreateFormFieldRequest> formFields | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java (1)
27-29: 사용되지 않는 의존성이 있습니다.
FormService가 주입되어 있지만 테스트에서 사용되지 않고 있습니다. 불필요한 의존성은 제거하는 것이 좋습니다.- @Autowired - private FormService formService; -
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/CreateFormRequest.java(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/CreateFormRequest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (1)
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java (1)
24-25: TestContainerSupport 설정을 검증해주세요.
TestContainerSupport를 상속받고 있는데, 컨테이너 설정이 올바르게 되어있는지 확인이 필요합니다.✅ Verification successful
TestContainerSupport 설정이 올바르게 구성되어 있습니다.
- MySQL 컨테이너가 싱글톤 패턴으로 적절히 구현되어 있습니다
- 테스트 데이터베이스 속성이 동적으로 올바르게 설정되어 있습니다
- 각 테스트 실행 전 데이터 초기화가 정상적으로 수행됩니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check TestContainerSupport configuration rg -A 10 "class TestContainerSupport"Length of output: 1166
Script:
#!/bin/bash # Get complete TestContainerSupport implementation and DataInitializer echo "=== TestContainerSupport Implementation ===" cat src/test/java/ddingdong/ddingdongBE/common/support/TestContainerSupport.java echo -e "\n=== DataInitializer Implementation ===" rg -l "class DataInitializer" | xargs catLength of output: 3639
Comment on lines
+47
to
+76
| @DisplayName("폼지와 폼지 질문을 생성할 수 있다.") | ||
| @Test | ||
| void createForm() { | ||
| // given | ||
| User user = fixtureMonkey.giveMeBuilder(User.class) | ||
| .set("id", 1L) | ||
| .set("Role", Role.CLUB) | ||
| .set("deletedAt", null) | ||
| .sample(); | ||
| User savedUser = userRepository.save(user); | ||
| Club club = fixtureMonkey.giveMeBuilder(Club.class) | ||
| .set("id", 1L) | ||
| .set("user", savedUser) | ||
| .set("score", null) | ||
| .set("clubMembers", null) | ||
| .set("deletedAt", null) | ||
| .sample(); | ||
| clubRepository.save(club); | ||
| CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class) | ||
| .set("user", savedUser) | ||
| .sample(); | ||
| // when | ||
| facadeCentralFormService.createForm(createFormCommand); | ||
| // then | ||
| List<Form> form = formRepository.findAll(); | ||
| List<FormField> formFields = formFieldRepository.findAll(); | ||
|
|
||
| assertThat(form).isNotEmpty(); | ||
| assertThat(formFields).isNotEmpty(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 케이스와 검증 로직을 보강해주세요.
현재 테스트에서 개선이 필요한 부분들이 있습니다:
- 테스트 데이터 설정이 테스트 메소드 내에 직접 구현되어 있어 재사용이 어렵습니다.
- 단순히 리스트가 비어있지 않은지만 확인하고 있어, 실제 데이터의 정확성을 검증하지 않습니다.
- 실패 케이스에 대한 테스트가 없습니다.
- 엔티티 간의 관계가 올바르게 설정되었는지 검증하지 않습니다.
다음과 같이 개선해보세요:
+ private User createTestUser() {
+ User user = fixtureMonkey.giveMeBuilder(User.class)
+ .set("id", 1L)
+ .set("Role", Role.CLUB)
+ .set("deletedAt", null)
+ .sample();
+ return userRepository.save(user);
+ }
+
+ private Club createTestClub(User user) {
+ Club club = fixtureMonkey.giveMeBuilder(Club.class)
+ .set("id", 1L)
+ .set("user", user)
+ .set("score", null)
+ .set("clubMembers", null)
+ .set("deletedAt", null)
+ .sample();
+ return clubRepository.save(club);
+ }
+
@DisplayName("폼지와 폼지 질문을 생성할 수 있다.")
@Test
void createForm() {
// given
- User user = fixtureMonkey.giveMeBuilder(User.class)
- .set("id", 1L)
- .set("Role", Role.CLUB)
- .set("deletedAt", null)
- .sample();
- User savedUser = userRepository.save(user);
- Club club = fixtureMonkey.giveMeBuilder(Club.class)
- .set("id", 1L)
- .set("user", savedUser)
- .set("score", null)
- .set("clubMembers", null)
- .set("deletedAt", null)
- .sample();
- clubRepository.save(club);
+ User savedUser = createTestUser();
+ Club savedClub = createTestClub(savedUser);
CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class)
.set("user", savedUser)
.sample();
+
// when
facadeCentralFormService.createForm(createFormCommand);
+
// then
- List<Form> form = formRepository.findAll();
+ List<Form> forms = formRepository.findAll();
List<FormField> formFields = formFieldRepository.findAll();
- assertThat(form).isNotEmpty();
- assertThat(formFields).isNotEmpty();
+ assertThat(forms).hasSize(1);
+ Form createdForm = forms.get(0);
+ assertThat(createdForm.getClub()).isEqualTo(savedClub);
+
+ assertThat(formFields)
+ .isNotEmpty()
+ .allSatisfy(field -> {
+ assertThat(field.getForm()).isEqualTo(createdForm);
+ assertThat(field.getQuestion()).isNotBlank();
+ });
}
+
+ @DisplayName("권한이 없는 사용자는 폼지를 생성할 수 없다.")
+ @Test
+ void createForm_WithoutPermission() {
+ // given
+ User unauthorizedUser = fixtureMonkey.giveMeBuilder(User.class)
+ .set("id", 1L)
+ .set("Role", Role.USER)
+ .set("deletedAt", null)
+ .sample();
+ User savedUser = userRepository.save(unauthorizedUser);
+ CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class)
+ .set("user", savedUser)
+ .sample();
+
+ // when & then
+ assertThatThrownBy(() -> facadeCentralFormService.createForm(createFormCommand))
+ .isInstanceOf(UnauthorizedException.class);
+ }📝 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.
Suggested change
| @DisplayName("폼지와 폼지 질문을 생성할 수 있다.") | |
| @Test | |
| void createForm() { | |
| // given | |
| User user = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 1L) | |
| .set("Role", Role.CLUB) | |
| .set("deletedAt", null) | |
| .sample(); | |
| User savedUser = userRepository.save(user); | |
| Club club = fixtureMonkey.giveMeBuilder(Club.class) | |
| .set("id", 1L) | |
| .set("user", savedUser) | |
| .set("score", null) | |
| .set("clubMembers", null) | |
| .set("deletedAt", null) | |
| .sample(); | |
| clubRepository.save(club); | |
| CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class) | |
| .set("user", savedUser) | |
| .sample(); | |
| // when | |
| facadeCentralFormService.createForm(createFormCommand); | |
| // then | |
| List<Form> form = formRepository.findAll(); | |
| List<FormField> formFields = formFieldRepository.findAll(); | |
| assertThat(form).isNotEmpty(); | |
| assertThat(formFields).isNotEmpty(); | |
| } | |
| private User createTestUser() { | |
| User user = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 1L) | |
| .set("Role", Role.CLUB) | |
| .set("deletedAt", null) | |
| .sample(); | |
| return userRepository.save(user); | |
| } | |
| private Club createTestClub(User user) { | |
| Club club = fixtureMonkey.giveMeBuilder(Club.class) | |
| .set("id", 1L) | |
| .set("user", user) | |
| .set("score", null) | |
| .set("clubMembers", null) | |
| .set("deletedAt", null) | |
| .sample(); | |
| return clubRepository.save(club); | |
| } | |
| @DisplayName("폼지와 폼지 질문을 생성할 수 있다.") | |
| @Test | |
| void createForm() { | |
| // given | |
| User savedUser = createTestUser(); | |
| Club savedClub = createTestClub(savedUser); | |
| CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class) | |
| .set("user", savedUser) | |
| .sample(); | |
| // when | |
| facadeCentralFormService.createForm(createFormCommand); | |
| // then | |
| List<Form> forms = formRepository.findAll(); | |
| List<FormField> formFields = formFieldRepository.findAll(); | |
| assertThat(forms).hasSize(1); | |
| Form createdForm = forms.get(0); | |
| assertThat(createdForm.getClub()).isEqualTo(savedClub); | |
| assertThat(formFields) | |
| .isNotEmpty() | |
| .allSatisfy(field -> { | |
| assertThat(field.getForm()).isEqualTo(createdForm); | |
| assertThat(field.getQuestion()).isNotBlank(); | |
| }); | |
| } | |
| @DisplayName("권한이 없는 사용자는 폼지를 생성할 수 없다.") | |
| @Test | |
| void createForm_WithoutPermission() { | |
| // given | |
| User unauthorizedUser = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 1L) | |
| .set("Role", Role.USER) | |
| .set("deletedAt", null) | |
| .sample(); | |
| User savedUser = userRepository.save(unauthorizedUser); | |
| CreateFormCommand createFormCommand = fixtureMonkey.giveMeBuilder(CreateFormCommand.class) | |
| .set("user", savedUser) | |
| .sample(); | |
| // when & then | |
| assertThatThrownBy(() -> facadeCentralFormService.createForm(createFormCommand)) | |
| .isInstanceOf(UnauthorizedException.class); | |
| } |
|
Seooooo24
approved these changes
Jan 31, 2025
Comment on lines
39
to
55
| @Schema(description = "폼지 질문", example = "우리 동아리 들어올겁니까?") | ||
| String question, | ||
|
|
||
| @Schema(description = "질문 종류", example = "CHECK_BOX") | ||
| FieldType type, | ||
|
|
||
| @Schema(description = "질문의 선택리스트", example = "[지문1이다., 지문2이다., 지문3이다.]") | ||
| List<String> options, | ||
|
|
||
| @Schema(description = "필수여부", example = "true") | ||
| boolean required, | ||
|
|
||
| @Schema(description = "질문 순서", example = "1") | ||
| int order, | ||
|
|
||
| @Schema(description = "질문 섹션 종류", example = "공통") | ||
| String section |
There was a problem hiding this comment.
options 빼고는 NotNull이 필요할 것 같은데 혹시 추가하지 않으신 이유가 있을까요??
This was referenced Jan 31, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🚀 작업 내용
🤔 고민했던 내용
폼지의 질문이 몇십개 정도 될텐데 단건 삽입이 괜찮을까? 그래서 Spring JDBC에서 지원하는 bulk 삽입을 사용해볼까? 라는 고민을 하였습니다.
대규모 데이터가 아닌점, jdbcTemplate을 활용하여 직접 쿼리를 작성할 경우 런타입 환경에서 에러가 발생할 확률이 큰 점(타입 안정성 등), id생성 전략이 identity와 잘맞는 mysql인 점 때문에 그냥!!!
saveAll()로 결정했습니다
Summary by CodeRabbit
새로운 기능
개선 사항