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-95] 지원자 상세 조회 API 구현 #240

Merged
merged 7 commits into from
Feb 6, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public record FormQuery(

@Builder
public record FormFieldListQuery(
Long id,
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 이건 왜 추가된 것인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fieldId를 FormFieldAnswerListQuery에서 사용해야 해서 추가했었습니다. 현재는 삭제하고 선배님 말씀 참고해서 삭제했습니다!

String question,
FieldType type,
List<String> options,
Expand All @@ -29,6 +30,7 @@ public record FormFieldListQuery(
) {
public static FormFieldListQuery from(FormField formField) {
return FormFieldListQuery.builder()
.id(formField.getId())
.question(formField.getQuestion())
.type(formField.getFieldType())
.options(formField.getOptions())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ddingdong.ddingdongBE.domain.formapplication.api;

import ddingdong.ddingdongBE.auth.PrincipalDetails;
import ddingdong.ddingdongBE.domain.formapplication.controller.dto.response.FormApplicationResponse;
import ddingdong.ddingdongBE.domain.formapplication.controller.dto.response.MyFormApplicationPageResponse;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
Expand Down Expand Up @@ -31,4 +32,16 @@ MyFormApplicationPageResponse getMyFormApplicationPage(
@AuthenticationPrincipal PrincipalDetails principalDetails
);

@Operation(summary = "지원자 상세 조회 API")
@ApiResponse(responseCode = "200", description = "지원자 상세 조회 성공",
content = @Content(schema = @Schema(implementation = FormApplicationResponse.class)))
@ResponseStatus(HttpStatus.OK)
@SecurityRequirement(name = "AccessToken")
@GetMapping("/my/forms/{formId}/applications/{applicationId}")
FormApplicationResponse getFormApplication(
@PathVariable("formId") Long formId,
@PathVariable("applicationId") Long applicationId,
@AuthenticationPrincipal PrincipalDetails principalDetails
);

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package ddingdong.ddingdongBE.domain.formapplication.controller;

import ddingdong.ddingdongBE.auth.PrincipalDetails;
import ddingdong.ddingdongBE.domain.formapplication.controller.dto.response.FormApplicationResponse;
import ddingdong.ddingdongBE.domain.formapplication.service.FacadeCentralFormApplicationService;
import ddingdong.ddingdongBE.domain.formapplication.api.CentralFormApplicationApi;
import ddingdong.ddingdongBE.domain.formapplication.controller.dto.response.MyFormApplicationPageResponse;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.FormApplicationQuery;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.MyFormApplicationPageQuery;
import ddingdong.ddingdongBE.domain.user.entity.User;
import lombok.RequiredArgsConstructor;
Expand All @@ -21,4 +23,11 @@ public MyFormApplicationPageResponse getMyFormApplicationPage(Long formId, int s
MyFormApplicationPageQuery query = facadeCentralFormService.getMyFormApplicationPage(formId, user, size, currentCursorId);
return MyFormApplicationPageResponse.from(query);
}

@Override
public FormApplicationResponse getFormApplication(Long formId, Long applicationId, PrincipalDetails principalDetails) {
User user = principalDetails.getUser();
FormApplicationQuery query = facadeCentralFormService.getFormApplication(formId, applicationId, user);
return FormApplicationResponse.from(query);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package ddingdong.ddingdongBE.domain.formapplication.controller.dto.response;

import ddingdong.ddingdongBE.domain.form.entity.FieldType;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplicationStatus;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.FormApplicationQuery;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.FormApplicationQuery.FormFieldAnswerListQuery;

import io.swagger.v3.oas.annotations.media.ArraySchema;
import io.swagger.v3.oas.annotations.media.Schema;
import lombok.Builder;

import java.time.LocalDateTime;
import java.util.List;

@Builder
public record FormApplicationResponse (
@Schema(description = "제출일시", example = "2025-01-01T00:00")
LocalDateTime submittedAt,
@Schema(description = "지원자 이름", example = "김띵동")
String name,
@Schema(description = "지원자 학번", example = "60201111")
String studentNumber,
@Schema(description = "지원자 학과", example = "융합소프트웨어학부")
String department,
@Schema(description = "status", example = "SUBMITTED")
FormApplicationStatus status,
@ArraySchema(schema = @Schema(implementation = FormFieldAnswerListResponse.class))
List<FormFieldAnswerListResponse> formFieldAnswers
){
@Builder
record FormFieldAnswerListResponse (
@Schema(description = "폼지 질문 ID", example = "1")
Long fieldId,
@Schema(description = "폼지 질문", example = "성별이 무엇입니까??")
String question,
@Schema(description = "폼지 질문 유형", example = "RADIO", allowableValues = {"CHECK_BOX", "RADIO", "TEXT", "LONG_TEXT", "FILE"})
FieldType type,
@Schema(description = "폼지 지문", example = "[\"여성\", \"남성\"]")
List<String> options,
@Schema(description = "필수 여부", example = "true")
Boolean required,
@Schema(description = "질문 순서", example = "1")
Integer order,
@Schema(description = "섹션", example = "공통")
String section,
@Schema(description = "질문 답변 값", example = "[\"지문1\"]")
List<String> value
) {
public static FormFieldAnswerListResponse from(FormFieldAnswerListQuery formFieldAnswerListQuery) {
return FormFieldAnswerListResponse.builder()
.fieldId(formFieldAnswerListQuery.fieldId())
.question(formFieldAnswerListQuery.question())
.type(formFieldAnswerListQuery.type())
.options(formFieldAnswerListQuery.options())
.required(formFieldAnswerListQuery.required())
.order(formFieldAnswerListQuery.order())
.section(formFieldAnswerListQuery.section())
.value(formFieldAnswerListQuery.value())
.build();
}
}
public static FormApplicationResponse from(FormApplicationQuery formApplicationQuery) {
List<FormFieldAnswerListResponse> responses = formApplicationQuery.formFieldAnswers().stream()
.map(FormFieldAnswerListResponse::from)
.toList();

return FormApplicationResponse.builder()
.submittedAt(formApplicationQuery.createdAt())
.name(formApplicationQuery.name())
.studentNumber(formApplicationQuery.studentNumber())
.department(formApplicationQuery.department())
.status(formApplicationQuery.status())
.formFieldAnswers(responses)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package ddingdong.ddingdongBE.domain.formapplication.repository;

import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

public interface FormAnswerRepository extends JpaRepository<FormAnswer, Long> {
List<FormAnswer> findAllByFormApplication(FormApplication formApplication);
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package ddingdong.ddingdongBE.domain.formapplication.service;

import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.FormApplicationQuery;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.MyFormApplicationPageQuery;
import ddingdong.ddingdongBE.domain.user.entity.User;

public interface FacadeCentralFormApplicationService {

MyFormApplicationPageQuery getMyFormApplicationPage(Long formId, User user, int size, Long currentCursorId);

FormApplicationQuery getFormApplication(Long formId, Long applicationId, User user);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import ddingdong.ddingdongBE.domain.club.entity.Club;
import ddingdong.ddingdongBE.domain.club.service.ClubService;
import ddingdong.ddingdongBE.domain.form.entity.FormField;
import ddingdong.ddingdongBE.domain.form.service.FormFieldService;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.FormApplicationQuery;
import ddingdong.ddingdongBE.domain.formapplication.service.dto.query.PagingQuery;
import ddingdong.ddingdongBE.domain.form.entity.Form;
import ddingdong.ddingdongBE.domain.form.service.FormService;
Expand All @@ -25,6 +29,8 @@ public class FacadeCentralFormApplicationServiceImpl implements FacadeCentralFor
private final ClubService clubService;
private final FormService formService;
private final FormApplicationService formApplicationService;
private final FormAnswerService formAnswerService;
private final FormFieldService formFieldService;

@Override
public MyFormApplicationPageQuery getMyFormApplicationPage(Long formId, User user, int size, Long currentCursorId) {
Expand All @@ -44,6 +50,18 @@ public MyFormApplicationPageQuery getMyFormApplicationPage(Long formId, User use
PagingQuery pagingQuery = PagingQuery.of(currentCursorId, completeFormApplications, formApplicationPage.hasNext());

return MyFormApplicationPageQuery.of(formApplicationListQueries, pagingQuery);
}

@Override
public FormApplicationQuery getFormApplication(Long formId, Long applicationId, User user) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3)
formId, user 파라미터는 해당 메서드에서 사용을 안하는데 따로 받는 이유가 있을까요?
없다면 제거해도 괜찮을거 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

추후 해당 폼이 로그인한 사용자 소유의 폼인지 유효성 검사를 해야 할 것 같아서 받아왔는데 일단 삭제해두는 게 맞을까요?

Club club = clubService.getByUserId(user.getId());
Form form = formService.getById(formId);
if (!form.getClub().equals(club)) {
throw new AccessDeniedException("권한이 없습니다.");
}
FormApplication formApplication = formApplicationService.getById(applicationId);
List<FormField> formFields = formFieldService.findAllByForm(form);
List<FormAnswer> formAnswers = formAnswerService.getAllByApplication(formApplication);
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) form으로 fields를 찾는 것보다 formAnswer내부의 field를 사용하면 dto내 복잡한 로직을 줄일 수 있을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하...!!! 제가 엔터티가 객체랑 매핑되어 있는 걸 잊었네요... 테이블에 있는 것처럼 id만 있다고 생각 했더니 이렇게 됐습니다. 알려주셔서 감사합니다! dto 재사용이 조심스러워야 한다는 점도 처음 알았습니다. 감사합니다. 수정 완료했습니다!

return FormApplicationQuery.of(formApplication, formFields, formAnswers);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package ddingdong.ddingdongBE.domain.formapplication.service;

import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;

import java.util.List;

public interface FormAnswerService {

void createAll(List<FormAnswer> formAnswers);

List<FormAnswer> getAllByApplication(FormApplication formApplication);

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ public interface FormApplicationService {
FormApplication create(FormApplication formApplication);

Slice<FormApplication> getFormApplicationPageByFormId(Long formId, int size, Long currentCursorId);

FormApplication getById(Long applicationId);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ddingdong.ddingdongBE.domain.formapplication.service;

import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;
import ddingdong.ddingdongBE.domain.formapplication.repository.FormAnswerRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -21,4 +22,8 @@ public void createAll(List<FormAnswer> formAnswers) {
formAnswerRepository.saveAll(formAnswers);
}

@Override
public List<FormAnswer> getAllByApplication(FormApplication formApplication) {
return formAnswerRepository.findAllByFormApplication(formApplication);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public Slice<FormApplication> getFormApplicationPageByFormId(Long formId, int si
return buildSlice(formApplicationPages, size);
}

@Override
public FormApplication getById(Long applicationId) {
return formApplicationRepository.findById(applicationId).orElse(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1)
getById는 현재 동작상 반드시 FormApplication 객체 반환을 보장해야 하는 거 같은데 조회된 값이 없을 시 null을 return 하는 거 보단 ResourceNotFoundException(커스텀 exception 정의되어 있음)을 던지는게 좋을거 같습니다!

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.

⚠️ Potential issue

예외 처리 개선 필요

현재 구현에서 발견된 문제점들:

  1. null을 반환하는 대신 명시적인 예외 처리가 필요합니다
  2. applicationId 유효성 검증이 없습니다
  3. 오류 로깅이 누락되었습니다

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

     @Override
     public FormApplication getById(Long applicationId) {
-        return formApplicationRepository.findById(applicationId).orElse(null);
+        if (applicationId == null || applicationId <= 0) {
+            throw new IllegalArgumentException("유효하지 않은 applicationId입니다: " + applicationId);
+        }
+        return formApplicationRepository.findById(applicationId)
+            .orElseThrow(() -> new EntityNotFoundException("ID가 " + applicationId + "인 폼 어플리케이션을 찾을 수 없습니다"));
     }

추가로 로깅을 위해 다음 의존성을 추가하는 것을 추천드립니다:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

그리고 클래스 상단에 로거를 선언:

private static final Logger log = LoggerFactory.getLogger(GeneralFormApplicationService.class);


private Slice<FormApplication> buildSlice(Slice<FormApplication> originalSlice, int size) {
List<FormApplication> content = new ArrayList<>(originalSlice.getContent());
if (content.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package ddingdong.ddingdongBE.domain.formapplication.service.dto.query;

import ddingdong.ddingdongBE.domain.form.entity.FieldType;

import ddingdong.ddingdongBE.domain.form.entity.FormField;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormAnswer;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplicationStatus;
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormQuery.FormFieldListQuery;

import lombok.Builder;

import java.util.function.Function;
import java.util.stream.Collectors;
import java.time.LocalDateTime;
import java.util.List;
import java.util.Map;

@Builder
public record FormApplicationQuery (
LocalDateTime createdAt,
String name,
String studentNumber,
String department,
FormApplicationStatus status,
List<FormFieldAnswerListQuery> formFieldAnswers
) {
@Builder
public record FormAnswerListQuery (
Long fieldId,
List<String> value
) {
public static FormAnswerListQuery from(FormAnswer formAnswer) {
return FormAnswerListQuery.builder()
.fieldId(formAnswer.getFormField().getId())
.value(formAnswer.getValue())
.build();
}
}
@Builder
public record FormFieldAnswerListQuery (
Long fieldId,
String question,
FieldType type,
List<String> options,
Boolean required,
Integer order,
String section,
List<String> value
) {
public static FormFieldAnswerListQuery from(FormFieldListQuery formFieldListQuery, FormAnswerListQuery formAnswerListQuery) {
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에서 dto를 재사용하는 것은 거의 드뭅니다! 거의 공통된 도메인을 목적으로 사용하는 것이 아니라면 따로 만드는 것이 좋아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 명심하겠습니다! 그런데 여쭤보고 싶은 점이 있습니다. feed에 있는 내용 거의 그대로인 PagingQuery와 PagingResponse 같은 경우 이후에 같은 용도로 사용될 일이 많을 것 같은데, 일반화해서 common 패키지에 빼놓는 것은 어떻게 생각하시나요? 당장 우선순위가 높은 일은 아니지만 궁금해서 선배님 의견을 여쭤보고 싶습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

그래도 해당 도메인의 페이지 처리에 요구사항이 변경될 수 있다고 하는데, 그렇게 사용하는 건 위험이 너무 크다고 생각합니다.
하나 바꾸면 전체 바꿔야 하니까!.
사실 위 같은 이유때문에 common패키지란 것이 정말 사용을 많이 고민하고 사용해야 합니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그렇군요!! 설명해주셔서 감사합니다!

return FormFieldAnswerListQuery.builder()
.fieldId(formFieldListQuery.id())
.question(formFieldListQuery.question())
.type(formFieldListQuery.type())
.options(formFieldListQuery.options())
.required(formFieldListQuery.required())
.order(formFieldListQuery.order())
.section(formFieldListQuery.section())
.value(formAnswerListQuery.value())
.build();
}
}
public static FormApplicationQuery of(FormApplication formApplication, List<FormField> formFields, List<FormAnswer> formAnswers) {
List<FormFieldListQuery> formFieldListQueries = formFields.stream()
.map(FormFieldListQuery::from)
.toList();
List<FormAnswerListQuery> formAnswerListQueries = formAnswers.stream()
.map(FormAnswerListQuery::from)
.toList();
Map<Long, FormAnswerListQuery> answerMap = formAnswerListQueries.stream()
.collect(Collectors.toMap(FormAnswerListQuery::fieldId, Function.identity()));
List<FormFieldAnswerListQuery> formFieldAnswerListQueries = formFieldListQueries.stream()
.map(fieldQuery -> {
FormAnswerListQuery answerQuery = answerMap.get(fieldQuery.id());
if (answerQuery == null) {
answerQuery = FormAnswerListQuery.builder()
.fieldId(fieldQuery.id())
.value(null)
.build();
}
return FormFieldAnswerListQuery.from(fieldQuery, answerQuery);
})
.collect(Collectors.toList());
return FormApplicationQuery.builder()
.createdAt(formApplication.getCreatedAt())
.name(formApplication.getName())
.studentNumber(formApplication.getStudentNumber())
.department(formApplication.getDepartment())
.status(formApplication.getStatus())
.formFieldAnswers(formFieldAnswerListQueries)
.build();
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

결측 답변에 대한 처리 로직 보강 제안
of 메서드에서 필드에 대응하는 답변이 없을 때 valuenull로 설정하고 있습니다. 이 로직이 의도한 로직인지, 또는 빈 리스트(Collections.emptyList()) 등으로 처리하는 편이 더 안전한지 고려해 보시면 좋겠습니다. null 값이 뷰나 다른 계층에서 처리될 때 예외가 발생할 수 있으니, 처리 방식을 명확히 정의해 주세요.

Loading