Skip to content
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-102] 폼지 섹션 조회 API 구현 #245

Merged
merged 7 commits into from
Feb 8, 2025
Merged

Conversation

Seooooo24
Copy link
Collaborator

@Seooooo24 Seooooo24 commented Feb 7, 2025

🚀 작업 내용

폼지 섹션 조회 API 구현하였습니다.

🤔 고민했던 내용

💬 리뷰 중점사항

Summary by CodeRabbit

  • 새로운 기능
    • 폼의 제목, 설명 및 섹션 정보를 확인할 수 있는 새로운 API 엔드포인트가 추가되었습니다.
  • 리팩터링
    • 폼 신청 처리 관련 서비스 명칭을 업데이트하여 내부 일관성을 개선하였습니다.
  • 테스트
    • 폼 섹션 조회와 폼 신청 기능에 대한 신규 테스트 케이스가 추가되어 기능의 안정성을 검증하였습니다.

Copy link

coderabbitai bot commented Feb 7, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

이번 PR은 폼 섹션 조회 기능을 위해 새로운 API 인터페이스(UserFormApi)와 이를 구현하는 컨트롤러(UserFormController)가 추가되었습니다. 또한 폼 섹션 데이터를 표현하기 위한 DTO인 FormSectionResponse와 관련 변환 로직, 그리고 서비스 인터페이스(FacadeUserFormService)와 그 구현체(FacadeUserFormServiceImpl), 그리고 데이터를 캡슐화하는 FormSectionQuery가 도입되었습니다. 한편, 폼 응용 서비스의 네이밍 변경에 따른 호출 수정 및 테스트 코드 업데이트가 이루어졌습니다.

Changes

파일 경로 변경 요약
src/main/java/ddingdong/ddingdongBE/domain/form/api/UserFormApi.java
src/main/java/ddingdong/ddingdongBE/domain/form/controller/UserFormController.java
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormSectionResponse.java
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormService.java
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImpl.java
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormSectionQuery.java
폼 섹션 조회 기능 추가 및 관련 API, 컨트롤러, 서비스 인터페이스/구현, DTO/Query 정의
src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/UserFormApplicationController.java
src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.java
src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java
폼 응용 서비스 인터페이스 및 구현체의 네이밍 변경에 따른 호출 수정
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormApplicationServiceImplTest.java
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java
폼 섹션 및 응용 관련 테스트 업데이트; 기존 폼 응용 테스트 제거 및 getFormSection 테스트 추가

Sequence Diagram(s)

sequenceDiagram
    participant C as Client
    participant Ctrl as UserFormController
    participant Facade as FacadeUserFormServiceImpl
    participant Srv as FormService

    C->>Ctrl: GET /server/forms/{formId}/sections
    Ctrl->>Facade: getFormSection(formId)
    Facade->>Srv: findFormById(formId)
    Srv-->>Facade: Form 데이터 반환
    Facade->>Facade: FormSectionQuery 생성
    Facade-->>Ctrl: FormSectionQuery 반환
    Ctrl->>Ctrl: FormSectionResponse.from(query) 호출
    Ctrl-->>C: JSON 형식의 폼 섹션 응답 전달
Loading

Possibly related PRs

Suggested labels

D-1

Suggested reviewers

  • wonjunYou
  • 5uhwann
  • KoSeonJe

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7cabbf and 87d3c10.

📒 Files selected for processing (3)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/UserFormApplicationController.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (1 hunks)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (6)
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormSectionQuery.java (1)

12-18: 정적 팩토리 메서드에 유효성 검사 추가 필요

of 메서드에 다음과 같은 유효성 검사를 추가하는 것이 좋겠습니다:

  • title이 null이거나 비어있는 경우
  • sections가 null인 경우

다음과 같이 수정하는 것을 제안드립니다:

  public static FormSectionQuery of(String title, String description, List<String> sections) {
+   if (title == null || title.trim().isEmpty()) {
+     throw new IllegalArgumentException("제목은 필수 입력값입니다.");
+   }
+   if (sections == null) {
+     throw new IllegalArgumentException("섹션 목록은 null일 수 없습니다.");
+   }
    return FormSectionQuery.builder()
        .title(title)
        .description(description)
        .sections(sections)
        .build();
  }
src/main/java/ddingdong/ddingdongBE/domain/form/api/UserFormApi.java (2)

11-13: API 문서화 개선이 필요합니다.

@Tag 어노테이션의 description이 API의 구체적인 목적과 사용 사례를 설명하지 않습니다.

-@Tag(name = "Form - User", description = "User Form API")
+@Tag(name = "Form - User", description = "사용자의 폼 관련 작업을 위한 API - 폼 섹션 조회 등")

15-20: API 문서화에 응답 스키마 정보가 누락되었습니다.

@Operation@ApiResponse 어노테이션에 응답 스키마와 예시 데이터가 포함되어 있지 않습니다.

 @Operation(summary = "폼지 섹션 조회 API")
-@ApiResponse(responseCode = "200", description = "폼지 섹션 조회 성공")
+@Operation(
+    summary = "폼지 섹션 조회 API",
+    description = "주어진 폼 ID에 해당하는 폼의 섹션 정보를 조회합니다."
+)
+@ApiResponse(
+    responseCode = "200",
+    description = "폼지 섹션 조회 성공",
+    content = @Content(
+        mediaType = "application/json",
+        schema = @Schema(implementation = FormSectionResponse.class)
+    )
+)
+@ApiResponse(
+    responseCode = "404",
+    description = "폼을 찾을 수 없음"
+)
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormSectionResponse.java (1)

10-15: 스키마 예시 데이터를 더 현실적으로 개선해야 합니다.

현재 예시 데이터가 너무 단순하며, 실제 사용 사례를 잘 반영하지 않습니다.

-    @Schema(description = "폼지 제목", example = "카우 1기 폼지")
+    @Schema(description = "폼지 제목", example = "2024년도 1학기 COW 동아리 지원서")
-    @Schema(description = "폼지 설명", example = "폼지 설명입니다")
+    @Schema(description = "폼지 설명", example = "COW 동아리는 웹/서버 개발 동아리로, 프로젝트 중심의 학습을 지향합니다.")
-    @Schema(description = "섹션 리스트", example = "[서버, 웹]")
+    @Schema(description = "섹션 리스트", example = "[\"기본 정보\", \"개발 경험\", \"지원 동기\", \"프로젝트 경험\"]")
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java (1)

23-24: 테스트 클래스 이름이 구현체를 참조하고 있습니다.

테스트 클래스 이름을 인터페이스 기반으로 변경하는 것이 좋습니다.

-class FacadeUserFormServiceImplTest extends TestContainerSupport {
+class FacadeUserFormServiceTest extends TestContainerSupport {
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormApplicationServiceImplTest.java (1)

32-60: 테스트 클래스 구조에 대한 개선 제안

테스트 클래스의 기본 구조는 잘 잡혀있으나, 다음과 같은 개선사항을 제안드립니다:

  1. TestContainerSupport의 역할과 설정에 대한 주석 추가
  2. FixtureMonkey 설정을 테스트 설정 클래스로 분리하여 재사용성 향상
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 198b7b7 and abf013d.

📒 Files selected for processing (11)
  • src/main/java/ddingdong/ddingdongBE/domain/form/api/UserFormApi.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/controller/UserFormController.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormSectionResponse.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormService.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImpl.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormSectionQuery.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/controller/UserFormApplicationController.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormApplicationServiceImplTest.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeUserFormServiceImplTest.java (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationService.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/FacadeUserFormService.java (1)

5-8: 인터페이스 설계가 깔끔합니다!

단일 책임 원칙을 잘 준수하고 있으며, 메서드 시그니처가 명확합니다.

src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormSectionResponse.java (1)

17-23: 정적 팩토리 메서드가 잘 구현되었습니다.

from 메서드가 명확하고 간결하게 구현되어 있으며, 빌더 패턴을 적절히 활용하고 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (1)

17-18: 트랜잭션 설정이 적절합니다.

클래스 레벨에서 @Transactional(readOnly = true)를 설정하고, 쓰기 작업이 필요한 메서드에서만 @Transactional을 명시적으로 설정한 것이 좋은 방식입니다.

Comment on lines 16 to 20
@Override
public FormSectionQuery getFormSection(Long formId) {
Form form = formService.getById(formId);
return FormSectionQuery.of(form.getTitle(), form.getDescription(), form.getSections());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

예외 처리 및 의존성 개선 필요

  1. formService.getById()에서 발생할 수 있는 예외 처리가 누락되었습니다.
  2. FormService에 대한 직접적인 의존성이 있습니다.

다음과 같이 수정하는 것을 제안드립니다:

  @Override
  public FormSectionQuery getFormSection(Long formId) {
+   try {
      Form form = formService.getById(formId);
+     if (form == null) {
+       throw new FormNotFoundException(formId);
+     }
      return FormSectionQuery.of(form.getTitle(), form.getDescription(), form.getSections());
+   } catch (Exception e) {
+     throw new FormServiceException("폼 섹션 조회 중 오류가 발생했습니다.", e);
+   }
  }

FormService를 인터페이스로 분리하여 의존성 역전 원칙(DIP)을 적용하는 것을 고려해보세요.

Comment on lines +16 to +20
@Override
public FormSectionResponse getFormSections(Long formId) {
FormSectionQuery query = facadeUserFormService.getFormSection(formId);
return FormSectionResponse.from(query);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

입력값 검증 및 오류 처리 보완 필요

  1. formId에 대한 유효성 검사가 누락되었습니다.
  2. 예외 발생 시 적절한 오류 응답 처리가 필요합니다.

다음과 같이 수정하는 것을 제안드립니다:

  @Override
  public FormSectionResponse getFormSections(Long formId) {
+   if (formId == null || formId <= 0) {
+     throw new InvalidParameterException("유효하지 않은 폼 ID입니다.");
+   }
    try {
      FormSectionQuery query = facadeUserFormService.getFormSection(formId);
      return FormSectionResponse.from(query);
+   } catch (FormNotFoundException e) {
+     throw new ResponseStatusException(HttpStatus.NOT_FOUND, "요청한 폼을 찾을 수 없습니다.", e);
+   } catch (Exception e) {
+     throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "폼 섹션 조회 중 오류가 발생했습니다.", e);
+   }
  }
📝 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
@Override
public FormSectionResponse getFormSections(Long formId) {
FormSectionQuery query = facadeUserFormService.getFormSection(formId);
return FormSectionResponse.from(query);
}
@Override
public FormSectionResponse getFormSections(Long formId) {
if (formId == null || formId <= 0) {
throw new InvalidParameterException("유효하지 않은 폼 ID입니다.");
}
try {
FormSectionQuery query = facadeUserFormService.getFormSection(formId);
return FormSectionResponse.from(query);
} catch (FormNotFoundException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "요청한 폼을 찾을 수 없습니다.", e);
} catch (Exception e) {
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "폼 섹션 조회 중 오류가 발생했습니다.", e);
}
}

Comment on lines +40 to +77
@DisplayName("유저는 섹션 목록을 조회할 수 있다.")
@Test
void getFormSection() {
// given
User user = fixtureMonkey.giveMeBuilder(User.class)
.set("id", 1L)
.set("Role", Role.CLUB)
.set("deletedAt", null)
.sample();
User savedUser = userRepository.save(user);

@Autowired
private FormFieldRepository formFieldRepository;
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);

@Autowired
private EntityManager entityManager;
List<String> savedSections = new ArrayList<>();
savedSections.add("section1");
savedSections.add("section2");

private static final FixtureMonkey fixtureMonkey = FixtureMonkeyFactory.getNotNullBuilderIntrospectorMonkey();
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", club)
.set("sections", savedSections)
.sample();
Form savedForm = formRepository.save(form);

@DisplayName("사용자는 동아리에 지원할 수 있다.")
@Test
void createFormApplication() {
// 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);
Club savedClub = clubRepository.save(club);
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", savedClub)
.sample();
Form savedForm = formRepository.save(form);
FormField formField = FormField.builder()
.form(savedForm)
.fieldType(FieldType.CHECK_BOX)
.fieldOrder(1)
.section("서버")
.required(true)
.question("질문")
.build();
FormField savedFormField = formFieldRepository.save(formField);
CreateFormApplicationCommand createFormApplicationCommand = fixtureMonkey.giveMeBuilder(CreateFormApplicationCommand.class)
.set("formId", savedForm.getId())
.set("formAnswerCommands", List.of(new CreateFormAnswerCommand(savedFormField.getId(), List.of("답변"))))
.sample();
// when
facadeUserFormService.createFormApplication(createFormApplicationCommand);
// then
List<FormApplication> formApplications = formApplicationRepository.findAll();
List<FormAnswer> formAnswers = formAnswerRepository.findAll();
FormSectionQuery sectionQuery = facadeUserFormService.getFormSection(savedForm.getId());

assertThat(formApplications).isNotEmpty();
assertThat(formApplications.get(0).getForm().getId()).isEqualTo(savedForm.getId());
assertThat(formAnswers).isNotEmpty();
assertThat(formAnswers.get(0).getValue()).isEqualTo(List.of("답변"));
}
}
assertThat(sectionQuery.sections()).isEqualTo(savedSections);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

테스트 코드 개선이 필요합니다.

  1. 테스트 데이터 설정이 너무 길고 반복적입니다.
  2. 검증(assertion)이 충분하지 않습니다.

다음과 같이 개선하는 것을 제안합니다:

+  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);
+  }
+
+  private Form createTestForm(Club club, List<String> sections) {
+    Form form = fixtureMonkey.giveMeBuilder(Form.class)
+        .set("id", 1L)
+        .set("title", "띵동 폼")
+        .set("description", "저희 동아리는 띵동입니다.")
+        .set("hasInterview", false)
+        .set("club", club)
+        .set("sections", sections)
+        .sample();
+    return formRepository.save(form);
+  }

  @DisplayName("유저는 섹션 목록을 조회할 수 있다.")
  @Test
  void getFormSection() {
    // 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);

    List<String> savedSections = new ArrayList<>();
    savedSections.add("section1");
    savedSections.add("section2");

-    Form form = fixtureMonkey.giveMeBuilder(Form.class)
-        .set("id", 1L)
-        .set("title", "띵동 폼")
-        .set("description", "저희 동아리는 띵동입니다.")
-        .set("hasInterview", false)
-        .set("club", club)
-        .set("sections", savedSections)
-        .sample();
-    Form savedForm = formRepository.save(form);
+    Form savedForm = createTestForm(savedClub, savedSections);

    // when
    FormSectionQuery sectionQuery = facadeUserFormService.getFormSection(savedForm.getId());

    // then
-    assertThat(sectionQuery.sections()).isEqualTo(savedSections);
+    assertThat(sectionQuery.title()).isEqualTo(savedForm.getTitle());
+    assertThat(sectionQuery.description()).isEqualTo(savedForm.getDescription());
+    assertThat(sectionQuery.sections())
+        .hasSize(savedSections.size())
+        .containsExactlyElementsOf(savedSections);
  }
📝 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 getFormSection() {
// given
User user = fixtureMonkey.giveMeBuilder(User.class)
.set("id", 1L)
.set("Role", Role.CLUB)
.set("deletedAt", null)
.sample();
User savedUser = userRepository.save(user);
@Autowired
private FormFieldRepository formFieldRepository;
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);
@Autowired
private EntityManager entityManager;
List<String> savedSections = new ArrayList<>();
savedSections.add("section1");
savedSections.add("section2");
private static final FixtureMonkey fixtureMonkey = FixtureMonkeyFactory.getNotNullBuilderIntrospectorMonkey();
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", club)
.set("sections", savedSections)
.sample();
Form savedForm = formRepository.save(form);
@DisplayName("사용자는 동아리에 지원할 수 있다.")
@Test
void createFormApplication() {
// 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);
Club savedClub = clubRepository.save(club);
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", savedClub)
.sample();
Form savedForm = formRepository.save(form);
FormField formField = FormField.builder()
.form(savedForm)
.fieldType(FieldType.CHECK_BOX)
.fieldOrder(1)
.section("서버")
.required(true)
.question("질문")
.build();
FormField savedFormField = formFieldRepository.save(formField);
CreateFormApplicationCommand createFormApplicationCommand = fixtureMonkey.giveMeBuilder(CreateFormApplicationCommand.class)
.set("formId", savedForm.getId())
.set("formAnswerCommands", List.of(new CreateFormAnswerCommand(savedFormField.getId(), List.of("답변"))))
.sample();
// when
facadeUserFormService.createFormApplication(createFormApplicationCommand);
// then
List<FormApplication> formApplications = formApplicationRepository.findAll();
List<FormAnswer> formAnswers = formAnswerRepository.findAll();
FormSectionQuery sectionQuery = facadeUserFormService.getFormSection(savedForm.getId());
assertThat(formApplications).isNotEmpty();
assertThat(formApplications.get(0).getForm().getId()).isEqualTo(savedForm.getId());
assertThat(formAnswers).isNotEmpty();
assertThat(formAnswers.get(0).getValue()).isEqualTo(List.of("답변"));
}
}
assertThat(sectionQuery.sections()).isEqualTo(savedSections);
}
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);
}
private Form createTestForm(Club club, List<String> sections) {
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", club)
.set("sections", sections)
.sample();
return formRepository.save(form);
}
@DisplayName("유저는 섹션 목록을 조회할 수 있다.")
@Test
void getFormSection() {
// given
User savedUser = createTestUser();
Club savedClub = createTestClub(savedUser);
List<String> savedSections = new ArrayList<>();
savedSections.add("section1");
savedSections.add("section2");
Form savedForm = createTestForm(savedClub, savedSections);
// when
FormSectionQuery sectionQuery = facadeUserFormService.getFormSection(savedForm.getId());
// then
assertThat(sectionQuery.title()).isEqualTo(savedForm.getTitle());
assertThat(sectionQuery.description()).isEqualTo(savedForm.getDescription());
assertThat(sectionQuery.sections())
.hasSize(savedSections.size())
.containsExactlyElementsOf(savedSections);
}

Comment on lines +61 to +111
@DisplayName("사용자는 동아리에 지원할 수 있다.")
@Test
void createFormApplication() {
// 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);
Club savedClub = clubRepository.save(club);
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "띵동 폼")
.set("description", "저희 동아리는 띵동입니다.")
.set("hasInterview", false)
.set("club", savedClub)
.sample();
Form savedForm = formRepository.save(form);
FormField formField = FormField.builder()
.form(savedForm)
.fieldType(FieldType.CHECK_BOX)
.fieldOrder(1)
.section("서버")
.required(true)
.question("질문")
.build();
FormField savedFormField = formFieldRepository.save(formField);
CreateFormApplicationCommand createFormApplicationCommand = fixtureMonkey.giveMeBuilder(CreateFormApplicationCommand.class)
.set("formId", savedForm.getId())
.set("formAnswerCommands", List.of(new CreateFormAnswerCommand(savedFormField.getId(), List.of("답변"))))
.sample();
// when
facadeUserFormApplicationService.createFormApplication(createFormApplicationCommand);
// then
List<FormApplication> formApplications = formApplicationRepository.findAll();
List<FormAnswer> formAnswers = formAnswerRepository.findAll();

assertThat(formApplications).isNotEmpty();
assertThat(formApplications.get(0).getForm().getId()).isEqualTo(savedForm.getId());
assertThat(formAnswers).isNotEmpty();
assertThat(formAnswers.get(0).getValue()).isEqualTo(List.of("답변"));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

테스트 메소드 구현에 대한 개선 제안

테스트 구현에 대해 다음과 같은 개선사항을 제안드립니다:

  1. 테스트 데이터 설정 부분이 너무 길어 가독성이 떨어집니다. 테스트 데이터 생성을 별도의 헬퍼 메소드로 추출하는 것을 추천드립니다.
  2. 현재 검증(assertion)이 기본적인 부분만 커버하고 있습니다. 다음과 같은 추가 검증이 필요합니다:
    • 생성된 FormApplication의 상태값 검증
    • FormAnswer의 모든 필드 검증
    • 저장된 데이터의 개수 검증
  3. 다음과 같은 엣지 케이스에 대한 테스트가 필요합니다:
    • 필수 필드가 누락된 경우
    • 잘못된 형식의 답변이 입력된 경우
    • 존재하지 않는 폼 ID가 입력된 경우

다음과 같이 테스트 데이터 설정을 분리하는 것을 제안드립니다:

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class FacadeUserFormApplicationServiceImplTest extends TestContainerSupport {
    // ... existing fields ...

    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);
    }

    private Form createTestForm(Club club) {
        Form form = fixtureMonkey.giveMeBuilder(Form.class)
                .set("id", 1L)
                .set("title", "띵동 폼")
                .set("description", "저희 동아리는 띵동입니다.")
                .set("hasInterview", false)
                .set("club", club)
                .sample();
        return formRepository.save(form);
    }

    @Test
    @DisplayName("사용자는 동아리에 지원할 수 있다.")
    void createFormApplication() {
        // given
        User savedUser = createTestUser();
        Club savedClub = createTestClub(savedUser);
        Form savedForm = createTestForm(savedClub);
        // ... rest of the test
    }

    @Test
    @DisplayName("필수 필드가 누락된 경우 예외가 발생한다.")
    void createFormApplication_WithMissingRequiredField() {
        // ... implement edge case test
    }
}

Copy link
Collaborator

@KoSeonJe KoSeonJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다. 고생하셨어요!

Comment on lines 15 to 20
@Operation(summary = "폼지 섹션 조회 API")
@ApiResponse(responseCode = "200", description = "폼지 섹션 조회 성공")
@GetMapping("/forms/{formId}/sections")
FormSectionResponse getFormSections(
@PathVariable("formId") Long formId
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2)
ResponseStatus 지정해줘야 할 것 같습니다.
추가로 응답 값이 어떻게 반환되는 지 DTO 통해 보여주는 법 다른 api 참고해서 추가해주면 좋을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하였습니다! 감사합니다.

Comment on lines +10 to +15
@Schema(description = "폼지 제목", example = "카우 1기 폼지")
String title,
@Schema(description = "폼지 설명", example = "폼지 설명입니다")
String description,
@Schema(description = "섹션 리스트", example = "[서버, 웹]")
List<String> sections
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3)
이 api필요한 이유가 어떤 섹션을 선택할지 사용자에게 선택지를 보여주기 위함인 것으로 알고있는데, 관련 안내문구같은거는 프론트에서 저장해서 보여주나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

피그마 상에 따로 안내 문구가 없었어서 저는 동아리에서 description에 자체적으로 넣을 거라고 생각했습니다. 혹시 안내 문구를 엔터티에 따로 저장해야 할까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아! "지원 분야를 선택해주세요" 같은 부분을 말씀하시는 걸까요? 저는 모든 동아리에서 동일하게 보여지는 문구라 프론트에서 저장하여 보여준다고 생각했는데 선배님은 백엔드에서 응답에 포함해주는 것이 좋다고 생각하시는 걸까요? 선배님 생각이 어떤지 여쭤보고 싶습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아뇨 그냥 궁금했습니다. 프론트에서 고정된 문구 보여주는 것도 괜찮을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 넵 알겠습니다!

Copy link
Collaborator

@5uhwann 5uhwann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

@Override
public FormSectionQuery getFormSection(Long formId) {
Form form = formService.getById(formId);
return FormSectionQuery.of(form.getTitle(), form.getDescription(), form.getSections());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p4)
사소한거긴 한데 FormSectionQuery.of() 메서드의 파라미터로 넘기는 값이 모두 form 객체의 필드라면 form 객체를 넘겨 팩토리 메서드에서 필드에 접근하는 것이 더 나아보입니다!

Suggested change
return FormSectionQuery.of(form.getTitle(), form.getDescription(), form.getSections());
return FormSectionQuery.from(form);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확실히 그렇네요! 수정했습니다!

@github-actions github-actions bot added the D-3 label Feb 8, 2025
@Seooooo24 Seooooo24 merged commit 5084cc4 into develop Feb 8, 2025
0 of 2 checks passed
@Seooooo24 Seooooo24 deleted the feature/DDING-102 branch February 8, 2025 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants