-
Notifications
You must be signed in to change notification settings - Fork 1
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-87] 동아리 지원 폼지 삭제 API 구현 #232
Conversation
Walkthrough이번 PR은 인증 예외 처리와 중앙 폼의 업데이트 및 삭제 기능을 확장하는 변경사항을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant C as CentralFormController
participant S as FacadeCentralFormServiceImpl
participant DB as 데이터베이스
%% 폼 업데이트 흐름
U->>C: HTTP PUT /forms/{id} (updateForm 요청)
C->>S: updateForm(UpdateFormCommand) 호출
S->>S: validateEqualsClub() 실행 및 업데이트 로직 처리
S->>DB: 폼 및 관련 필드 업데이트
DB-->>S: 업데이트 결과 반환
S-->>C: 204 응답 반환
C-->>U: 204 응답 전송
%% 폼 삭제 흐름
U->>C: HTTP DELETE /forms/{id} (deleteForm 요청)
C->>S: deleteForm(formId, user) 호출
S->>S: validateEqualsClub() 실행 및 삭제 로직 처리
S->>DB: 폼 및 관련 필드 삭제
DB-->>S: 삭제 결과 반환
S-->>C: 204 응답 반환
C-->>U: 204 응답 전송
Suggested labels
Suggested reviewers
✨ 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 (3)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java (1)
61-67
: update 메서드에 유효성 검사 추가 필요update 메서드가 추가되었지만, 다음 사항들에 대한 유효성 검사가 필요합니다:
- startDate가 endDate보다 이후인 경우
- title이 빈 문자열인 경우
다음과 같이 유효성 검사를 추가하는 것을 제안합니다:
public void update(Form updateForm) { + if (updateForm.getStartDate().isAfter(updateForm.getEndDate())) { + throw new IllegalArgumentException("시작일은 종료일 이후일 수 없습니다"); + } + if (updateForm.getTitle().trim().isEmpty()) { + throw new IllegalArgumentException("제목은 비어있을 수 없습니다"); + } this.title = updateForm.getTitle(); this.description = updateForm.getDescription(); this.startDate = updateForm.getStartDate(); this.endDate = updateForm.getEndDate(); this.hasInterview = updateForm.isHasInterview(); }src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/UpdateFormRequest.java (2)
20-26
: 날짜 유효성 검사 강화 필요시작일과 종료일에 대한 기본적인 @NotNull 검사는 있지만, 추가적인 검증이 필요합니다:
- 시작일이 현재 날짜 이후인지 확인
- 종료일이 시작일 이후인지 확인
커스텀 validator를 추가하는 것을 제안합니다:
@Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Constraint(validatedBy = FormDateValidator.class) public @interface ValidFormDates { String message() default "날짜가 유효하지 않습니다"; Class<?>[] groups() default {}; Class<? extends Payload>[] payload() default {}; } public class FormDateValidator implements ConstraintValidator<ValidFormDates, UpdateFormRequest> { @Override public boolean isValid(UpdateFormRequest request, ConstraintValidatorContext context) { if (request.startDate() == null || request.endDate() == null) { return true; // @NotNull에서 처리됨 } return !request.startDate().isBefore(LocalDate.now()) && !request.endDate().isBefore(request.startDate()); } }
73-86
: toCommand 메서드의 안전성 향상 필요formFields가 null인 경우에 대한 처리가 필요합니다.
다음과 같이 수정하는 것을 제안합니다:
public UpdateFormCommand toCommand(Long formId) { - List<UpdateFormFieldCommand> updateFormFieldCommands = formFields.stream() + List<UpdateFormFieldCommand> updateFormFieldCommands = formFields == null ? + List.of() : formFields.stream() .map(UpdateFormFieldRequest::toCommand) .toList(); return UpdateFormCommand.builder() .formId(formId) .title(title) .description(description) .startDate(startDate) .endDate(endDate) .hasInterview(hasInterview) .formFieldCommands(updateFormFieldCommands) .build(); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/ddingdong/ddingdongBE/common/exception/AuthenticationException.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/UpdateFormRequest.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.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
(2 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
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/command/UpdateFormCommand.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java
- src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (26)
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1)
10-10
: 구현이 잘 되었습니다!JPA 명명 규칙을 잘 따르고 있으며, Form 엔티티를 기반으로 FormField 목록을 조회하는 메서드가 적절하게 정의되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java (1)
11-14
: 인터페이스 설계가 명확합니다!Form 엔티티와 관련된 FormField 조회 및 삭제 기능이 잘 정의되어 있습니다. 메서드 시그니처가 일관성 있게 구성되어 있어 좋습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java (2)
24-27
: 조회 구현이 적절합니다!Form 엔티티를 기반으로 FormField 목록을 조회하는 로직이 repository를 통해 잘 구현되어 있습니다.
29-33
: 트랜잭션 처리가 잘 되어있습니다!FormField 목록 삭제 시 @transactional 어노테이션을 통해 트랜잭션 처리가 적절하게 되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (9)
3-3
: 권한 예외 임포트 사항 확인
새로운 NonHaveAuthority 예외 클래스를 임포트해 권한 문제가 발생할 때 예외 처리를 명확히 할 수 있어 좋습니다.
10-11
: UpdateFormCommand 임포트 추가
업데이트 로직을 구체화하기 위해UpdateFormCommand
를 가져오는 구조가 명확합니다.
14-14
: Objects 유틸 사용
Objects.equals
사용을 위한 임포트로, 비교 로직 간결화에 유용합니다.
35-35
: 폼 생성 필드 변환 로직 검토
toCreateFormFields
메서드로 FormField 엔티티 목록을 일괄 생성하는 방식이 일관적이고 이해하기 쉽습니다.
39-51
: 폼 업데이트 로직: 기존 필드 전면 교체
기존 폼 필드를 전부 삭제 후 새로운 필드를 생성하여 업데이트하는 구조입니다. 부분 갱신보다는 단순하지만, 모든 필드가 교체되는 점을 주의해야 합니다.
53-60
: 폼 삭제 로직 및 권한 검증
validateEqualsClub
메서드를 통해 본인 소속 클럽의 폼인지 확인 후 삭제하는 접근이 적절합니다. Cascade 설정으로 연결된 필드가 자동 삭제되므로 관리가 수월합니다.
62-66
: 클럽 일치 검증 메서드
클럽 식별자를 비교해 권한을 확인하는 방식이 단순하고 분명합니다. 적절한 예외 처리를 통해 안전성을 높였습니다.
68-72
: toUpdateFormFields 메서드 구조
UpdateFormFieldCommand
를 빠르게 엔티티로 변환하여 재사용성 및 가독성이 높습니다.
Line range hint
74-78
: toCreateFormFields 메서드 명칭 변경
toCreateFormFields
로 변경하여 생성 로직임을 명확히 드러내, 유지보수 측면에서 가독성이 좋아졌습니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (2)
9-9
: 새로운 폼 조회 메서드
formId
로 폼을 가져올 수 있어, 수정 및 삭제 기능 구현에 유용합니다.
11-11
: 삭제 메서드 추가
서비스 레벨에서 폼 엔티티를 직접 삭제할 수 있게 되어, 폼 관리 로직이 간결해집니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (3)
4-5
: UpdateFormCommand 및 User 엔티티 임포트
폼 업데이트와 삭제 시 필요한 명세 및 사용자 정보를 명확히 드러내는 좋은 구조입니다.
10-10
: 폼 업데이트 메서드 선언
UpdateFormCommand
를 통한 업데이트로 디커플링이 이루어져, 유지보수 시 장점이 큽니다.
13-13
: 폼 삭제 메서드 선언
User
로 권한을 검증하고,formId
로 대상 폼을 지정하여 보다 안전한 삭제 로직을 구현할 수 있습니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (2)
23-27
: 구현이 깔끔하고 명확합니다!예외 메시지가 명확하고 한글로 작성되어 있어 좋습니다.
29-33
: 트랜잭션 처리가 적절합니다.
@Transactional
어노테이션이 적절히 사용되었습니다. 삭제 작업은 트랜잭션 내에서 수행되어야 하므로 올바르게 구현되었습니다.src/main/java/ddingdong/ddingdongBE/common/exception/AuthenticationException.java (1)
10-10
: 권한 관련 예외 처리가 일관성 있게 구현되었습니다.상수와 예외 클래스가 기존 코드 스타일을 잘 따르고 있으며, 메시지가 명확합니다.
Also applies to: 37-42
src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java (1)
36-40
: 구현이 적절합니다.사용자 권한 검증을 위해
principalDetails
에서 사용자 정보를 추출하여 서비스 계층에 전달하고 있습니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/command/UpdateFormCommand.java (1)
11-19
: 🛠️ Refactor suggestion입력값 검증 로직 추가가 필요합니다.
다음 항목들에 대한 유효성 검증이 필요합니다:
- 제목과 설명의 길이 제한
- 시작일이 종료일보다 이후일 수 없음
- formFieldCommands가 null이 아닌지 확인
Bean Validation 어노테이션을 사용하여 다음과 같이 구현할 것을 제안합니다:
@Builder public record UpdateFormCommand( Long formId, + @NotBlank @Size(max = 100) String title, + @Size(max = 1000) String description, + @NotNull LocalDate startDate, + @NotNull LocalDate endDate, boolean hasInterview, + @NotEmpty List<UpdateFormFieldCommand> formFieldCommands ) { + public UpdateFormCommand { + if (startDate != null && endDate != null && startDate.isAfter(endDate)) { + throw new IllegalArgumentException("시작일은 종료일보다 이후일 수 없습니다."); + } + }src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java (1)
45-52
: 생성자 가독성이 개선되었습니다!각 매개변수를 새 줄에 배치하여 코드의 가독성이 향상되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (2)
34-43
: 폼 수정 API가 적절하게 설계되었습니다!RESTful 원칙을 잘 따르고 있으며, 보안 요구사항과 응답 상태 코드가 적절합니다.
45-53
: 폼 삭제 API 설계가 적절합니다!cascade 설정으로 인해 form_field가 자동으로 삭제되는 점이 잘 반영되어 있습니다.
@Override | ||
public void updateForm( | ||
UpdateFormRequest updateFormRequest, | ||
Long formId, | ||
PrincipalDetails principalDetails | ||
) { | ||
facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId)); | ||
} |
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.
보안 검증이 누락되었습니다.
updateForm
메서드에서 principalDetails
에서 가져온 사용자 정보를 활용하지 않고 있습니다. 폼 수정 권한이 있는 사용자인지 확인하는 로직이 필요합니다.
다음과 같이 수정을 제안합니다:
public void updateForm(
UpdateFormRequest updateFormRequest,
Long formId,
PrincipalDetails principalDetails
) {
+ User user = principalDetails.getUser();
- facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId));
+ facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId), user);
}
📝 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.
@Override | |
public void updateForm( | |
UpdateFormRequest updateFormRequest, | |
Long formId, | |
PrincipalDetails principalDetails | |
) { | |
facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId)); | |
} | |
@Override | |
public void updateForm( | |
UpdateFormRequest updateFormRequest, | |
Long formId, | |
PrincipalDetails principalDetails | |
) { | |
+ User user = principalDetails.getUser(); | |
- facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId)); | |
+ facadeCentralFormService.updateForm(updateFormRequest.toCommand(formId), user); | |
} |
@Builder | ||
public record UpdateFormFieldCommand( | ||
String question, | ||
FieldType type, | ||
List<String> options, | ||
boolean required, | ||
int order, | ||
String section | ||
) { | ||
|
||
public FormField toEntity(Form form) { | ||
return FormField.builder() | ||
.form(form) | ||
.question(question) | ||
.fieldType(type) | ||
.options(options) | ||
.required(required) | ||
.fieldOrder(order) | ||
.section(section) | ||
.build(); | ||
} | ||
} |
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
필드 유효성 검증이 필요합니다.
UpdateFormFieldCommand
에도 다음 항목들의 유효성 검증이 필요합니다:
- 질문이 비어있지 않은지
- 순서가 음수가 아닌지
- options가 필요한 FieldType인 경우 비어있지 않은지
다음과 같이 구현할 것을 제안합니다:
@Builder
public record UpdateFormFieldCommand(
+ @NotBlank
String question,
+ @NotNull
FieldType type,
List<String> options,
boolean required,
+ @Min(0)
int order,
String section
) {
+ public UpdateFormFieldCommand {
+ if (type.requiresOptions() && (options == null || options.isEmpty())) {
+ throw new IllegalArgumentException(
+ "필드 타입 " + type + "는 옵션이 필요합니다."
+ );
+ }
+ }
📝 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.
@Builder | |
public record UpdateFormFieldCommand( | |
String question, | |
FieldType type, | |
List<String> options, | |
boolean required, | |
int order, | |
String section | |
) { | |
public FormField toEntity(Form form) { | |
return FormField.builder() | |
.form(form) | |
.question(question) | |
.fieldType(type) | |
.options(options) | |
.required(required) | |
.fieldOrder(order) | |
.section(section) | |
.build(); | |
} | |
} | |
@Builder | |
public record UpdateFormFieldCommand( | |
@NotBlank String question, | |
@NotNull FieldType type, | |
List<String> options, | |
boolean required, | |
@Min(0) int order, | |
String section | |
) { | |
public UpdateFormFieldCommand { | |
if (type.requiresOptions() && (options == null || options.isEmpty())) { | |
throw new IllegalArgumentException( | |
"필드 타입 " + type + "는 옵션이 필요합니다." | |
); | |
} | |
} | |
public FormField toEntity(Form form) { | |
return FormField.builder() | |
.form(form) | |
.question(question) | |
.fieldType(type) | |
.options(options) | |
.required(required) | |
.fieldOrder(order) | |
.section(section) | |
.build(); | |
} | |
} |
|
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.
고생 많으셨습니다!!
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: 0
🧹 Nitpick comments (2)
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java (1)
137-166
: 폼 삭제 테스트 케이스에 대한 제안사항이 있습니다.다음과 같은 추가 테스트 케이스 구현을 고려해보세요:
- 존재하지 않는 폼 ID로 삭제 시도
- 이미 삭제된 폼 재삭제 시도
- 폼 필드가 없는 폼 삭제
src/main/resources/db/migration/V35_create_form_response_and_answer_table.sql (1)
16-17
: 스키마 설계에 대한 제안사항이 있습니다.
- value 컬럼의 TEXT 타입은 적절하나, 최대 길이 제한 검토가 필요할 수 있습니다.
- application_id와 field_id에 인덱스 추가를 고려해보세요.
Also applies to: 21-22
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/entity/FormAnswer.java
(2 hunks)src/main/resources/db/migration/V35_create_form_response_and_answer_table.sql
(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (6)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java (2)
31-34
: LGTM! Form 엔티티로 FormField 목록을 조회하는 메서드가 잘 구현되었습니다.메서드가 간단하고 명확하며, 리포지토리 메서드를 적절히 활용하고 있습니다.
36-40
: LGTM! FormField 목록 삭제 메서드가 잘 구현되었습니다.@transactional 어노테이션이 적절히 사용되었으며, 벌크 삭제 연산이 효율적으로 구현되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/entity/FormAnswer.java (1)
32-34
: LGTM! 엔티티 관계가 적절히 설정되었습니다.
- FetchType.LAZY를 사용한 성능 최적화
- 명확한 외래 키 컬럼명 지정
- 적절한 다대일(ManyToOne) 관계 설정
Also applies to: 36-38
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java (2)
56-59
: LGTM! EntityManager를 적절히 초기화하여 테스트 격리를 보장합니다.각 테스트 실행 전에 EntityManager를 초기화하여 테스트 간 독립성을 보장합니다.
168-209
: LGTM! 권한 검증 테스트가 잘 구현되었습니다.클럽 소유권 검증 로직이 적절히 테스트되었습니다. NonHaveAuthority 예외 검증이 명확합니다.
src/main/resources/db/migration/V35_create_form_response_and_answer_table.sql (1)
10-11
: LGTM! CASCADE 삭제 설정이 적절합니다.폼이 삭제될 때 관련된 지원서도 자동으로 삭제되도록 하는 CASCADE 설정이 적절합니다.
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: 0
🧹 Nitpick comments (4)
src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/controller/UserFormController.java (1)
1-20
: 아키텍처 관점의 제안사항컨트롤러의 구조와 의존성 주입이 잘 구현되어 있습니다. 다만 다음 사항들을 고려해보시면 좋을 것 같습니다:
- API 응답에 대한 표준화된 응답 객체 사용 검토
- 예외 처리를 위한 @ExceptionHandler 추가 고려
- API 문서화를 위한 Swagger/OpenAPI 어노테이션 추가 검토
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java (1)
97-111
: 테스트 검증을 보강하면 좋을 것 같습니다.현재 검증은 기본적인 부분만 확인하고 있습니다. 다음과 같은 추가 검증을 고려해보세요:
- FormField의 속성들(fieldType, required 등)이 올바르게 저장되었는지 확인
- FormAnswer와 FormField의 연관관계가 올바른지 확인
예시 코드:
assertThat(formAnswers.get(0).getValue()).isEqualTo(List.of("답변")); +assertThat(formAnswers.get(0).getFormField().getId()).isEqualTo(savedFormField.getId()); +assertThat(formAnswers.get(0).getFormField().getFieldType()).isEqualTo(FieldType.CHECK_BOX); +assertThat(formAnswers.get(0).getFormField().isRequired()).isTrue();src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/FacadeUserFormServiceImpl.java (1)
36-43
: 코드 가독성이 향상되었습니다!다음과 같은 개선사항이 있습니다:
- 메서드 파라미터 포맷팅이 개선되어 가독성이 향상됨
- 람다 표현식이 더 간결하고 명확해짐
- 중첩된 메서드 호출이 잘 정리됨
하나의 제안사항이 있습니다:
람다 표현식을 더욱 간결하게 만들 수 있습니다:
- .map(formAnswerCommand - -> formAnswerCommand.toEntity(savedFormApplication, formFieldService.getById(formAnswerCommand.fieldId()))) + .map(command -> command.toEntity( + savedFormApplication, + formFieldService.getById(command.fieldId()) + ))src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (1)
35-44
: 폼 수정 API가 잘 구현되었습니다!RESTful 규칙을 잘 준수하고 있으며, 보안 설정과 문서화가 적절히 되어있습니다.
추가 제안: 응답 상태 코드 204에 대한 설명을 좀 더 구체적으로 작성하면 좋을 것 같습니다. 예를 들어:
-@ApiResponse(responseCode = "204", description = "동아리 지원 폼지 수정 성공") +@ApiResponse(responseCode = "204", description = "동아리 지원 폼지 수정 성공 (수정된 데이터 없음)")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java
(3 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/controller/UserFormController.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/controller/dto/request/CreateFormApplicationRequest.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/FacadeUserFormService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/FacadeUserFormServiceImpl.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/dto/CreateFormApplicationCommand.java
(2 hunks)src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (13)
src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/controller/UserFormController.java (1)
16-17
: 코드 변경이 적절하게 이루어졌습니다.매개변수 처리 방식이 개선되어 더 깔끔해졌습니다. formId를 CreateFormApplicationRequest의 toCommand 메서드로 직접 전달하는 방식은 책임 분리와 캡슐화 측면에서 더 나은 접근 방식입니다.
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java (2)
3-4
: 새로운 의존성 주입이 적절히 추가되었습니다!FormField 관련 기능 테스트를 위한 필요한 의존성들이 잘 추가되었습니다.
Also applies to: 10-14, 25-26, 53-58
61-61
: 테스트 가독성이 개선되었습니다!
- 테스트 이름이 한글로 변경되어 테스트의 의도가 더 명확해졌습니다.
- FormField 생성 로직이 builder 패턴을 사용하여 명확하게 구현되었습니다.
Also applies to: 88-96
src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/FacadeUserFormService.java (1)
7-7
: 메서드 시그니처 개선이 잘 되었습니다!
formId
를CreateFormApplicationCommand
객체 내부로 이동시킴으로써 메서드 시그니처가 더 간결해졌고, 커맨드 패턴의 캡슐화가 개선되었습니다.src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/FacadeUserFormServiceImpl.java (2)
6-10
: import 문 정리가 잘 되었습니다!필요한 엔티티 클래스들의 import가 적절히 복원되었습니다.
27-34
: createFormApplication 메서드 구현이 개선되었습니다!
formId
를 커맨드 객체에서 직접 가져오도록 변경되어 메서드 호출이 더 간단해졌습니다. 트랜잭션 처리와 엔티티 생성 흐름이 명확합니다.src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/service/dto/CreateFormApplicationCommand.java (3)
5-5
: 임포트 변경이 적절합니다.FormAnswer 엔티티와 Builder 어노테이션의 임포트가 추가되었습니다. 이는 코드의 의존성을 명확하게 표현합니다.
Also applies to: 9-9
13-13
: Form 객체 대신 formId를 사용하는 것이 좋은 설계 결정입니다.Form 엔티티 대신 식별자를 사용함으로써:
- 계층간 결합도가 낮아집니다
- DTO의 책임이 명확해집니다
- 메모리 사용량이 개선됩니다
23-23
: CreateFormAnswerCommand에서 valueType 필드 제거가 적절한지 확인이 필요합니다.valueType 필드가 제거되었는데, 이로 인해 답변 유효성 검증에 영향이 없는지 확인해주세요.
다음 스크립트로 valueType 사용 여부를 확인할 수 있습니다:
✅ Verification successful
검증 결과: valueType 필드 제거는 적절하며, 답변 유효성 검증에 영향이 없습니다.
- 전체 코드베이스에서 "valueType" 관련 참조가 발견되지 않았습니다.
- 관련된 로직에서는 "FieldType"이 사용되고 있으며, 이는 기존 valueType의 기능과 연관되지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: valueType 사용 여부 확인 # FormAnswer 엔티티에서 valueType 관련 로직 검색 rg -A 5 "valueType|FieldType" src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/ # 테스트 코드에서 valueType 검증 로직 검색 rg -A 5 "valueType|FieldType" src/test/java/ddingdong/ddingdongBE/domain/formapplicaion/Length of output: 280
Script:
#!/bin/bash # 전체 코드베이스에서 "valueType" 또는 "FieldType" 관련 참조가 있는지 검색합니다. rg -A 3 "valueType|FieldType" .Length of output: 6909
src/main/java/ddingdong/ddingdongBE/domain/formapplicaion/controller/dto/request/CreateFormApplicationRequest.java (2)
49-50
: 구현이 잘 되었습니다.Form ID를 직접 Command에 전달하도록 변경한 것이 좋은 접근 방식입니다. 이는 폼 관리 기능을 더 명확하게 만들어줍니다.
45-45
: 메서드 시그니처 변경이 호환성에 영향을 미칠 수 있습니다.메서드 시그니처가 변경되어 기존 호출자들에게 영향을 미칠 수 있습니다. 모든 호출 지점이 업데이트되었는지 확인해주세요.
다음 스크립트를 실행하여 모든 호출 지점이 업데이트되었는지 확인하세요:
✅ Verification successful
호출 지점 업데이트가 올바르게 이루어졌습니다.
- CreateFormApplicationRequest 내 새 메서드인
toCommand(Long formId)
는 호출부(예:UserFormController.java
)에서 인자와 함께 호출되고 있음이 확인되었습니다.- 나머지
toCommand()
호출들은 다른 DTO에서 사용되는 것으로 보이며, 본 PR의 변경 사항과 직접적인 관련은 없는 것으로 판단됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all callers of toCommand() have been updated # Search for old method calls without formId parameter rg "toCommand\(\)" --type java # Search for new method calls with formId parameter rg "toCommand\([^)]+\)" --type javaLength of output: 9515
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (2)
10-10
: 요청 유효성 검사가 적절히 추가되었습니다!
@Valid
어노테이션을 통한 요청 객체 유효성 검사 추가는 적절한 방어 로직입니다.Also applies to: 31-31
46-54
: 폼 삭제 API가 PR 목적에 맞게 잘 구현되었습니다!RESTful 규칙을 잘 준수하고 있으며, 보안 설정과 문서화가 적절히 되어있습니다.
폼 삭제 시 연관된
form_field
가 cascade 설정에 의해 함께 삭제되는 것을 확인하기 위해 다음 스크립트를 실행해보시기 바랍니다:✅ Verification successful
검증 결과: 폼 삭제 API 및 연관 필드의 cascade 삭제 설정 확인
PR에서 구현된 폼 삭제 API는 정상적으로 작성되었으며, 연관된
form_field
의 cascade 삭제 구성은 아래와 같이 migration 스크립트에서 확인되었습니다:
src/main/resources/db/migration/V35_create_form_response_and_answer_table.sql
파일 내 외래키 제약조건에ON DELETE CASCADE
가 명시되어 있습니다.엔티티 내 cascade 설정 검증 시 사용한 검색 명령어에 regex 오류가 있었으나, 데이터베이스 레벨에서 cascade 설정이 보장되므로 PR 목적에 부합합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify cascade delete configuration in entity classes # Search for cascade configuration in Form entity rg -A 5 "class.*Form.*{" | rg -A 5 "form_field" # Search for foreign key configuration in migration scripts fd -e sql | xargs rg -i "foreign key.*form_field"Length of output: 368
🚀 작업 내용
Summary by CodeRabbit
새로운 기능
개선 사항