[DDING-86] 동아리 지원 폼지 수정 API 구현 #231
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) 업데이트 기능을 추가하는 변경 사항을 포함하고 있습니다. 새로운 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant Controller
participant FacadeService
participant FormService
participant FormFieldService
participant Repository
Client->>Controller: updateForm 요청
Controller->>FacadeService: updateForm(command)
FacadeService->>FormService: getById(formId)
FormService-->>FacadeService: 기존 Form 반환
FacadeService->>FormFieldService: deleteAll(기존 FormFields)
FacadeService->>FormService: Form 업데이트
FacadeService->>FormFieldService: 새 FormFields 생성
FacadeService-->>Controller: 처리 완료
Controller-->>Client: 204 No Content
관련 가능성 있는 PR
제안된 레이블
제안된 리뷰어
✨ 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: 2
🧹 Nitpick comments (4)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (1)
26-26: 에러 메시지 생성 방식 개선 제안문자열 연결 대신 String.format이나 MessageFormat을 사용하면 더 깔끔할 것 같습니다.
예시:
- .orElseThrow(() -> new ResourceNotFound("Form(formId=" + formId + ")를 찾을 수 없습니다.")); + .orElseThrow(() -> new ResourceNotFound(String.format("Form(formId=%d)를 찾을 수 없습니다.", formId)));src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)
39-48: 전체 필드를 삭제 후 재생성하는 로직에 대한 성능 고려 안내
기존 필드를 전부 삭제하고 새로 생성하는 로직은 현재 규모에서는 무리가 없지만, 향후 질문 수가 급격히 늘어날 경우 성능상의 부담이 있을 수 있습니다. 부분 업데이트 로직도 고려해볼 수 있습니다.
50-54: 변수명 카멜 케이스 사용 권장
originform대신originForm처럼 카멜 케이스로 표기하면 가독성이 개선됩니다.- private List<FormField> toUpdateFormFields(Form originform, List<UpdateFormFieldCommand> updateFormFieldCommands) { + private List<FormField> toUpdateFormFields(Form originForm, List<UpdateFormFieldCommand> updateFormFieldCommands) {src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java (1)
11-13: 메서드 문서화를 추가하면 좋을 것 같습니다.인터페이스 메서드의 목적과 동작이 명확하지만, JavaDoc을 추가하면 더 좋을 것 같습니다.
다음과 같이 문서화를 추가하는 것을 제안합니다:
+ /** + * 특정 Form에 속한 모든 FormField를 조회합니다. + * @param form 조회할 Form 엔티티 + * @return Form에 속한 FormField 목록 + */ List<FormField> findAllByForm(Form form); + /** + * 주어진 FormField 목록을 일괄 삭제합니다. + * @param originFormFields 삭제할 FormField 목록 + */ void deleteAll(List<FormField> originFormFields);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
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/club/service/GeneralClubService.java
- src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (14)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (1)
9-9: 메서드 시그니처가 명확하고 적절합니다!Form 엔티티를 ID로 조회하는 메서드가 잘 정의되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (1)
23-27: 구현이 깔끔하고 예외 처리가 적절합니다!트랜잭션 처리와 예외 처리가 잘 구현되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)
9-10: 필요한 import 정상 반영입니다.
해당 클래스에서UpdateFormCommand관련 로직을 사용하는 데 필요한 import가 잘 추가되었습니다.
36-38: 트랜잭션 범위 설정이 적절합니다.
클래스 전체에readOnly = true가 적용되어 있지만, 이 메서드에서는 쓰기 작업을 수행하므로 메서드 레벨의@Transactional을 사용해 읽기 전용 설정을 무효화한 점이 적절합니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (1)
10-10: 업데이트 기능 인터페이스 추가에 대한 승인
CreateFormCommand와 대칭적으로UpdateFormCommand를 활용하여 양방향 기능을 확장한 점이 좋습니다.src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java (1)
27-34: 유저 정보 확인 로직 누락 가능성 점검 요청
createForm시에는principalDetails에서 유저를 가져와 커맨드에 전달하지만,updateForm은 유저 정보를 사용하지 않고 있습니다. 특정 권한 검증 또는 동아리 소유자 확인이 필요한 경우 추가 검토가 필요합니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/command/UpdateFormCommand.java (3)
10-19: 명령 객체의 구조가 잘 설계되어 있습니다.
@Builder패턴을 사용하여 객체 생성을 용이하게 하고, 필요한 모든 필드들이 포함되어 있습니다.
21-42: 내부 레코드의 구조도 적절합니다.
UpdateFormFieldCommand는 폼 필드 업데이트에 필요한 모든 정보를 포함하고 있으며, 빌더 패턴을 통해 객체 생성이 용이합니다.
31-41: 엔티티 변환 메서드의 구현이 깔끔합니다.
toEntity()메서드들이 명확하게 구현되어 있어 command 객체에서 엔티티로의 변환이 원활합니다.Also applies to: 44-52
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (1)
33-42: API 설계가 RESTful 원칙을 잘 따르고 있습니다.
- PUT 메서드 사용이 적절합니다
- 응답 상태 코드(204)가 적절합니다
- Swagger 문서화가 잘 되어있습니다
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/request/UpdateFormRequest.java (1)
73-86: toCommand 메서드의 구현이 깔끔합니다.스트림 API를 활용한 변환 로직이 효율적으로 구현되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1)
10-10: 메서드 구현이 적절합니다!Spring Data JPA 명명 규칙을 잘 따르고 있으며, Form 엔티티를 기반으로 관련된 FormField들을 조회하는 기능이 명확합니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormFieldService.java (2)
24-27: 구현이 깔끔합니다!Form 엔티티를 기반으로 FormField를 조회하는 구현이 명확하고 간단합니다.
29-33: 성능 고려사항에 대해 논의해보면 좋을 것 같습니다.PR 설명에서 언급된 대로, 전체 삭제 후 재생성 방식을 선택하셨네요. 현재는 문제가 없을 것으로 보이나, 다음 사항들을 고려해보시면 좋을 것 같습니다:
- 대량의 레코드 삭제 시 DB 부하
- 트랜잭션 롤백 시나리오
- 동시성 처리 방안
현재 구현이 적절한지 검증하기 위해 다음 스크립트로 FormField 테이블의 인덱스를 확인해보시면 좋을 것 같습니다:
Comment on lines
+61
to
+67
| public void update(Form updateForm) { | ||
| this.title = updateForm.getTitle(); | ||
| this.description = updateForm.getDescription(); | ||
| this.startDate = updateForm.getStartDate(); | ||
| this.endDate = updateForm.getEndDate(); | ||
| this.hasInterview = updateForm.isHasInterview(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
업데이트 메서드에 유효성 검증이 필요합니다.
update 메서드에서 날짜 유효성 검증이 필요합니다. startDate가 endDate보다 이후일 수 없습니다.
다음과 같이 유효성 검증을 추가하는 것을 제안합니다:
public void update(Form updateForm) {
+ if (updateForm.getStartDate().isAfter(updateForm.getEndDate())) {
+ throw new IllegalArgumentException("시작일은 종료일 이후일 수 없습니다.");
+ }
this.title = updateForm.getTitle();
this.description = updateForm.getDescription();
this.startDate = updateForm.getStartDate();
this.endDate = updateForm.getEndDate();
this.hasInterview = updateForm.isHasInterview();
}📝 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 void update(Form updateForm) { | |
| this.title = updateForm.getTitle(); | |
| this.description = updateForm.getDescription(); | |
| this.startDate = updateForm.getStartDate(); | |
| this.endDate = updateForm.getEndDate(); | |
| this.hasInterview = updateForm.isHasInterview(); | |
| } | |
| public void update(Form updateForm) { | |
| if (updateForm.getStartDate().isAfter(updateForm.getEndDate())) { | |
| throw new IllegalArgumentException("시작일은 종료일 이후일 수 없습니다."); | |
| } | |
| this.title = updateForm.getTitle(); | |
| this.description = updateForm.getDescription(); | |
| this.startDate = updateForm.getStartDate(); | |
| this.endDate = updateForm.getEndDate(); | |
| this.hasInterview = updateForm.isHasInterview(); | |
| } |
Comment on lines
+20
to
+26
| @Schema(description = "폼지 시작일자", example = "2001-01-01") | ||
| @NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.") | ||
| LocalDate startDate, | ||
|
|
||
| @Schema(description = "폼지 종료일자", example = "2001-01-02") | ||
| @NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.") | ||
| LocalDate endDate, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
날짜 필드에 대한 추가 유효성 검증이 필요합니다.
시작일과 종료일에 대한 상호 검증이 필요합니다. @AssertTrue를 사용하여 구현할 수 있습니다.
다음과 같이 검증 메서드를 추가하는 것을 제안합니다:
+ @AssertTrue(message = "시작일은 종료일 이후일 수 없습니다.")
+ private boolean isDateValid() {
+ return startDate == null || endDate == null || !startDate.isAfter(endDate);
+ }📝 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
| @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 = "2001-01-01") | |
| @NotNull(message = "폼지 시작일자는 null이 될 수 없습니다.") | |
| LocalDate startDate, | |
| @Schema(description = "폼지 종료일자", example = "2001-01-02") | |
| @NotNull(message = "폼지 종료일자는 null이 될 수 없습니다.") | |
| LocalDate endDate, | |
| @AssertTrue(message = "시작일은 종료일 이후일 수 없습니다.") | |
| private boolean isDateValid() { | |
| return startDate == null || endDate == null || !startDate.isAfter(endDate); | |
| } |
Seooooo24
approved these changes
Feb 2, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java(2 hunks)
⏰ 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)
15-16: 필요한 import 문이 잘 추가되었습니다.폼 수정 기능에 필요한 UpdateFormCommand와 UpdateFormFieldCommand가 적절히 import 되었습니다.
Comment on lines
+80
to
+123
| @DisplayName("폼지와 폼지 질문을 수정할 수 있다.") | ||
| @Test | ||
| void updateForm() { | ||
| // 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); | ||
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | ||
| .set("club", club) | ||
| .sample(); | ||
| Form savedForm = formService.create(form); | ||
| UpdateFormCommand updateFormCommand = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class) | ||
| .set("title", "수정된 제목") | ||
| .set("description", "수정된 설명") | ||
| .set("formId", savedForm.getId()) | ||
| .set("formFieldCommands", List.of( | ||
| fixtureMonkey.giveMeBuilder(UpdateFormFieldCommand.class) | ||
| .set("question", "수정된 질문") | ||
| .sample()) | ||
| ) | ||
| .sample(); | ||
| // when | ||
| facadeCentralFormService.updateForm(updateFormCommand); | ||
| // then | ||
| Form found = formRepository.findById(savedForm.getId()).orElse(null); | ||
| List<FormField> formFields = formFieldRepository.findAllByForm(found); | ||
| assertThat(found).isNotNull(); | ||
| assertThat(found.getTitle()).isEqualTo("수정된 제목"); | ||
| assertThat(found.getDescription()).isEqualTo("수정된 설명"); | ||
| assertThat(formFields).isNotEmpty(); | ||
| assertThat(formFields.get(0).getQuestion()).isEqualTo("수정된 질문"); | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
폼 수정 테스트 케이스에 대한 개선 제안
테스트 케이스가 기본적인 수정 기능을 잘 검증하고 있습니다만, 다음 사항들을 고려해보시기 바랍니다:
-
엣지 케이스 검증이 누락되어 있습니다:
- 존재하지 않는 폼 ID로 수정 시도
- 빈 formFieldCommands로 수정 시도
- 최대 허용 질문 수(30개) 초과 시도
-
삭제 후 재생성 방식을 선택하셨으므로, 기존 필드들이 실제로 삭제되었는지 확인하는 검증이 필요합니다.
다음과 같이 테스트를 보완하시는 것을 추천드립니다:
@Test
void updateForm() {
// given
// ... existing setup code ...
+ // 기존 폼 필드 생성
+ FormField existingField = fixtureMonkey.giveMeBuilder(FormField.class)
+ .set("form", savedForm)
+ .set("question", "기존 질문")
+ .sample();
+ formFieldRepository.save(existingField);
+
UpdateFormCommand updateFormCommand = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class)
.set("title", "수정된 제목")
.set("description", "수정된 설명")
.set("formId", savedForm.getId())
.set("formFieldCommands", List.of(
fixtureMonkey.giveMeBuilder(UpdateFormFieldCommand.class)
.set("question", "수정된 질문")
.sample())
)
.sample();
// when
facadeCentralFormService.updateForm(updateFormCommand);
// then
Form found = formRepository.findById(savedForm.getId()).orElse(null);
List<FormField> formFields = formFieldRepository.findAllByForm(found);
assertThat(found).isNotNull();
assertThat(found.getTitle()).isEqualTo("수정된 제목");
assertThat(found.getDescription()).isEqualTo("수정된 설명");
assertThat(formFields).isNotEmpty();
assertThat(formFields.get(0).getQuestion()).isEqualTo("수정된 질문");
+ // 기존 필드가 삭제되었는지 확인
+ assertThat(formFieldRepository.findById(existingField.getId())).isEmpty();
}
+
+@Test
+@DisplayName("존재하지 않는 폼 ID로 수정 시 예외가 발생한다")
+void updateForm_WithNonExistentId() {
+ // given
+ UpdateFormCommand command = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class)
+ .set("formId", 999L)
+ .sample();
+
+ // when & then
+ assertThatThrownBy(() -> facadeCentralFormService.updateForm(command))
+ .isInstanceOf(IllegalArgumentException.class);
+}
+
+@Test
+@DisplayName("최대 허용 질문 수를 초과하면 예외가 발생한다")
+void updateForm_ExceedMaxQuestions() {
+ // given
+ List<UpdateFormFieldCommand> commands =
+ fixtureMonkey.giveMe(UpdateFormFieldCommand.class, 31);
+
+ UpdateFormCommand command = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class)
+ .set("formFieldCommands", commands)
+ .sample();
+
+ // when & then
+ assertThatThrownBy(() -> facadeCentralFormService.updateForm(command))
+ .isInstanceOf(IllegalArgumentException.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 updateForm() { | |
| // 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); | |
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("club", club) | |
| .sample(); | |
| Form savedForm = formService.create(form); | |
| UpdateFormCommand updateFormCommand = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class) | |
| .set("title", "수정된 제목") | |
| .set("description", "수정된 설명") | |
| .set("formId", savedForm.getId()) | |
| .set("formFieldCommands", List.of( | |
| fixtureMonkey.giveMeBuilder(UpdateFormFieldCommand.class) | |
| .set("question", "수정된 질문") | |
| .sample()) | |
| ) | |
| .sample(); | |
| // when | |
| facadeCentralFormService.updateForm(updateFormCommand); | |
| // then | |
| Form found = formRepository.findById(savedForm.getId()).orElse(null); | |
| List<FormField> formFields = formFieldRepository.findAllByForm(found); | |
| assertThat(found).isNotNull(); | |
| assertThat(found.getTitle()).isEqualTo("수정된 제목"); | |
| assertThat(found.getDescription()).isEqualTo("수정된 설명"); | |
| assertThat(formFields).isNotEmpty(); | |
| assertThat(formFields.get(0).getQuestion()).isEqualTo("수정된 질문"); | |
| } | |
| @DisplayName("폼지와 폼지 질문을 수정할 수 있다.") | |
| @Test | |
| void updateForm() { | |
| // 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); | |
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("club", club) | |
| .sample(); | |
| Form savedForm = formService.create(form); | |
| + // 기존 폼 필드 생성 | |
| + FormField existingField = fixtureMonkey.giveMeBuilder(FormField.class) | |
| + .set("form", savedForm) | |
| + .set("question", "기존 질문") | |
| + .sample(); | |
| + formFieldRepository.save(existingField); | |
| + | |
| UpdateFormCommand updateFormCommand = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class) | |
| .set("title", "수정된 제목") | |
| .set("description", "수정된 설명") | |
| .set("formId", savedForm.getId()) | |
| .set("formFieldCommands", List.of( | |
| fixtureMonkey.giveMeBuilder(UpdateFormFieldCommand.class) | |
| .set("question", "수정된 질문") | |
| .sample()) | |
| ) | |
| .sample(); | |
| // when | |
| facadeCentralFormService.updateForm(updateFormCommand); | |
| // then | |
| Form found = formRepository.findById(savedForm.getId()).orElse(null); | |
| List<FormField> formFields = formFieldRepository.findAllByForm(found); | |
| assertThat(found).isNotNull(); | |
| assertThat(found.getTitle()).isEqualTo("수정된 제목"); | |
| assertThat(found.getDescription()).isEqualTo("수정된 설명"); | |
| assertThat(formFields).isNotEmpty(); | |
| assertThat(formFields.get(0).getQuestion()).isEqualTo("수정된 질문"); | |
| + // 기존 필드가 삭제되었는지 확인 | |
| + assertThat(formFieldRepository.findById(existingField.getId())).isEmpty(); | |
| } | |
| + | |
| +@Test | |
| +@DisplayName("존재하지 않는 폼 ID로 수정 시 예외가 발생한다") | |
| +void updateForm_WithNonExistentId() { | |
| + // given | |
| + UpdateFormCommand command = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class) | |
| + .set("formId", 999L) | |
| + .sample(); | |
| + | |
| + // when & then | |
| + assertThatThrownBy(() -> facadeCentralFormService.updateForm(command)) | |
| + .isInstanceOf(IllegalArgumentException.class); | |
| +} | |
| + | |
| +@Test | |
| +@DisplayName("최대 허용 질문 수를 초과하면 예외가 발생한다") | |
| +void updateForm_ExceedMaxQuestions() { | |
| + // given | |
| + List<UpdateFormFieldCommand> commands = | |
| + fixtureMonkey.giveMe(UpdateFormFieldCommand.class, 31); | |
| + | |
| + UpdateFormCommand command = fixtureMonkey.giveMeBuilder(UpdateFormCommand.class) | |
| + .set("formFieldCommands", commands) | |
| + .sample(); | |
| + | |
| + // when & then | |
| + assertThatThrownBy(() -> facadeCentralFormService.updateForm(command)) | |
| + .isInstanceOf(IllegalArgumentException.class); | |
| +} |
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.
🚀 작업 내용
🤔 고민했던 내용
formField가 추가, 삭제, 수정되는 경우의 수에 따라 기존의 것과 비교하여 수정하는 로직과,
그냥 기존의 것을 모두 삭제하고 새로 수정된 것을 모두 삽입하는 로직, 둘 중에 고민하였습니다.
두 번째 경우에 대규모 데이터일 경우 성능에 영향이 있겠지만, 지원 폼지 질문과 같은 소규모는 괜찮을 것 같아서 두 번째 방법을 선택하였습니다.
확장성을 고려한다 해도, 질문이 50개를 넘어갈 것 같지도 않고, 많아야 30개정도 될 것이라 생각했습니다.
어떻게 생각하시는 지 리뷰부탁드립니다.
Summary by CodeRabbit
새로운 기능
API 개선
보안 및 유효성 검사