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
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package ddingdong.ddingdongBE.domain.form.api;

import ddingdong.ddingdongBE.domain.form.controller.dto.response.FormSectionResponse;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.tags.Tag;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;

@Tag(name = "Form - User", description = "User Form API")
@RequestMapping("/server")
public interface UserFormApi {

@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.

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

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package ddingdong.ddingdongBE.domain.form.controller;

import ddingdong.ddingdongBE.domain.form.api.UserFormApi;
import ddingdong.ddingdongBE.domain.form.controller.dto.response.FormSectionResponse;
import ddingdong.ddingdongBE.domain.form.service.FacadeUserFormService;
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormSectionQuery;
import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequiredArgsConstructor
public class UserFormController implements UserFormApi {

private final FacadeUserFormService facadeUserFormService;

@Override
public FormSectionResponse getFormSections(Long formId) {
FormSectionQuery query = facadeUserFormService.getFormSection(formId);
return FormSectionResponse.from(query);
}
Comment on lines +16 to +20
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);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package ddingdong.ddingdongBE.domain.form.controller.dto.response;

import ddingdong.ddingdongBE.domain.form.service.dto.query.FormSectionQuery;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;
import lombok.Builder;

@Builder
public record FormSectionResponse (
@Schema(description = "폼지 제목", example = "카우 1기 폼지")
String title,
@Schema(description = "폼지 설명", example = "폼지 설명입니다")
String description,
@Schema(description = "섹션 리스트", example = "[서버, 웹]")
List<String> sections
Comment on lines +10 to +15
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.

아하 넵 알겠습니다!

) {
public static FormSectionResponse from(FormSectionQuery formSectionQuery) {
return FormSectionResponse.builder()
.title(formSectionQuery.title())
.description(formSectionQuery.description())
.sections(formSectionQuery.sections())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package ddingdong.ddingdongBE.domain.form.service;

import ddingdong.ddingdongBE.domain.form.service.dto.query.FormSectionQuery;

public interface FacadeUserFormService {

FormSectionQuery getFormSection(Long formId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package ddingdong.ddingdongBE.domain.form.service;

import ddingdong.ddingdongBE.domain.form.entity.Form;
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormSectionQuery;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class FacadeUserFormServiceImpl implements FacadeUserFormService {

private final FormService formService;

@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.

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

}
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)을 적용하는 것을 고려해보세요.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package ddingdong.ddingdongBE.domain.form.service.dto.query;

import java.util.List;
import lombok.Builder;

@Builder
public record FormSectionQuery (
String title,
String description,
List<String> sections
){
public static FormSectionQuery of(String title, String description, List<String> sections) {
return FormSectionQuery.builder()
.title(title)
.description(description)
.sections(sections)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@

import ddingdong.ddingdongBE.domain.formapplication.api.UserFormApplicationApi;
import ddingdong.ddingdongBE.domain.formapplication.controller.dto.request.CreateFormApplicationRequest;
import ddingdong.ddingdongBE.domain.formapplication.service.FacadeUserFormService;
import ddingdong.ddingdongBE.domain.formapplication.service.FacadeUserFormApplicationService;
import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequiredArgsConstructor
public class UserFormApplicationController implements UserFormApplicationApi {

private final FacadeUserFormService facadeUserFormService;
private final FacadeUserFormApplicationService facadeUserFormApplicationService;

@Override
public void createFormApplication(Long formId, CreateFormApplicationRequest createFormApplicationRequest) {
facadeUserFormService.createFormApplication(createFormApplicationRequest.toCommand(formId));
facadeUserFormApplicationService.createFormApplication(createFormApplicationRequest.toCommand(formId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import ddingdong.ddingdongBE.domain.formapplication.service.dto.command.CreateFormApplicationCommand;

public interface FacadeUserFormService {
public interface FacadeUserFormApplicationService {

void createFormApplication(CreateFormApplicationCommand createFormApplicationCommand);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class FacadeUserFormServiceImpl implements FacadeUserFormService {
public class FacadeUserFormApplicationServiceImpl implements FacadeUserFormApplicationService {

private final FormApplicationService formApplicationService;
private final FormAnswerService formAnswerService;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package ddingdong.ddingdongBE.domain.form.service;

import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;

import com.navercorp.fixturemonkey.FixtureMonkey;
import ddingdong.ddingdongBE.common.support.FixtureMonkeyFactory;
import ddingdong.ddingdongBE.common.support.TestContainerSupport;
import ddingdong.ddingdongBE.domain.club.entity.Club;
import ddingdong.ddingdongBE.domain.club.repository.ClubRepository;
import ddingdong.ddingdongBE.domain.form.entity.FieldType;
import ddingdong.ddingdongBE.domain.form.entity.Form;
import ddingdong.ddingdongBE.domain.form.entity.FormField;
import ddingdong.ddingdongBE.domain.form.repository.FormFieldRepository;
import ddingdong.ddingdongBE.domain.form.repository.FormRepository;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;
import ddingdong.ddingdongBE.domain.formapplication.repository.FormAnswerRepository;
import ddingdong.ddingdongBE.domain.formapplication.repository.FormApplicationRepository;
import ddingdong.ddingdongBE.domain.formapplication.service.FacadeUserFormApplicationService;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.command.CreateFormApplicationCommand;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.command.CreateFormApplicationCommand.CreateFormAnswerCommand;
import ddingdong.ddingdongBE.domain.user.entity.Role;
import ddingdong.ddingdongBE.domain.user.entity.User;
import ddingdong.ddingdongBE.domain.user.repository.UserRepository;
import jakarta.persistence.EntityManager;
import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest
class FacadeUserFormApplicationServiceImplTest extends TestContainerSupport {

@Autowired
private FacadeUserFormApplicationService facadeUserFormApplicationService;

@Autowired
private FormApplicationRepository formApplicationRepository;

@Autowired
private FormAnswerRepository formAnswerRepository;

@Autowired
private FormRepository formRepository;

@Autowired
private ClubRepository clubRepository;

@Autowired
private UserRepository userRepository;

@Autowired
private FormFieldRepository formFieldRepository;

@Autowired
private EntityManager entityManager;

private static final FixtureMonkey fixtureMonkey = FixtureMonkeyFactory.getNotNullBuilderIntrospectorMonkey();

@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("답변"));
}
Comment on lines +61 to +111
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
    }
}

}
Loading
Loading