[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #274

Profile-exe · 2024-09-23T05:58:14Z

개요

작업사항

정렬을 위한 쿼리 파라미터 이름을 order로 통일
지원 현황 조회, 면접 기록 조회 API에 searchKeyword 쿼리 파라미터를 추가해 별도의 검색 API 엔드포인트를 사용하지 않음
전체 결과에 대한 정렬 이후 페이징을 수행해 페이지별 일관성 확보

변경로직

AnswerService

@Transactional(readOnly = true)
public AnswersResponseDto execute(Integer year, Integer page, String sortType, String searchKeyword) {
    PageInfo pageInfo = getPageInfo(year, page, searchKeyword);
    List<MongoAnswer> sortedResult = answerAdaptor.findByYearAndSearchKeyword(year, page, sortType, searchKeyword);

    List<Map<String, Object>> qnaMapList = getQnaMapListWithIdAndPassState(sortedResult);

    if (qnaMapList.isEmpty()) {
        return AnswersResponseDto.of(Collections.emptyList(), pageInfo);
    }
    return AnswersResponseDto.of(qnaMapList, pageInfo);
}

RecordService

@Override
public RecordsViewResponseDto execute(Integer page, Integer year, String sortType, String searchKeyword) {
    List<Record> result = recordLoadPort.findAll();
    List<String> applicantIds = result.stream().map(Record::getApplicantId).toList();

    List<MongoAnswer> applicants;
    if (sortType.equals("score")) {
        applicants = applicantQueryUseCase.execute(year, sortType, searchKeyword, applicantIds);
    } else {
        applicants = applicantQueryUseCase.execute(page, year, sortType, searchKeyword, applicantIds);
    }

    if (result.isEmpty() || applicants.isEmpty()) {
        return createEmptyResponse(new PageInfo(0, page));
    }

    Map<String, Integer> yearByAnswerIdMap = applicants.stream().collect(Collectors.toMap(MongoAnswer::getId, MongoAnswer::getYear));

    applicantIds = applicants.stream().map(MongoAnswer::getId).toList();    // 검색 결과에 따라 applicantIds 재할당
    Map<String, Double> scoreMap = getScoreMap(year, applicantIds, yearByAnswerIdMap);

    result = result.stream().filter(record -> year == null ||
                    Optional.ofNullable(record.getApplicantId())
                            .map(yearByAnswerIdMap::get)
                            .map(y -> y.equals(year))
                            .orElse(false)
            )
            .toList();

    List<Record> records;
    if (sortType.equals("score")) {
        records = sortRecordsByScoresDesc(result, scoreMap, page);
    } else {
        records = sortRecordsByApplicantsAndSortType(result, applicants);
    }

    PageInfo pageInfo = applicantQueryUseCase.getPageInfo(year, page, searchKeyword);
    return RecordsViewResponseDto.of(pageInfo, records, scoreMap, applicants);
}

reference

데이터베이스 역순 정렬과 동일하게 정렬 수행

점수 정렬은 지원자를 모두 불러와 scoreMap과 매핑하고 정렬을 수행한 후 페이지별로 잘라야 함

- `@ParameterObject` 사용 시 swagger에서 필수 값으로 나타남 - `@RequestParam`의 `required` 속성을 `false`로 두어 swagger에 쿼리 파라미터 필수 여부가 올바르게 적용

- 불필요한 getPageInfo 메서드 제거

LJH098 · 2024-09-23T06:42:30Z

...ecruit-Api/src/main/java/com/econovation/recruit/api/applicant/service/ApplicantService.java

+    private List<Map<String, Object>> getQnaMapListWithIdAndPassState(List<MongoAnswer> sortedResult) {
+        return sortedResult.stream().map(answer -> {
+            Map<String, Object> qna = answer.getQna();
+            qna.put("id", answer.getId());
+            qna.put(PASS_STATE_KEY, answer.getApplicantStateOrDefault());
+            return qna;
+        }).toList();
+    }


getQnaMapListWithIdAndPassState 를 따로 나눈 이유가
id랑 pass_state_key만 따로 넣을 수 도 있어서 분리 한 것일까요? (정렬 없이)

넵 맞습니다. qna 맵을 추출하고 여기에 id와 passState를 넣기 때문에 메서드로 분리했습니다.

LJH098 · 2024-09-23T06:44:45Z

...t-Api/src/main/java/com/econovation/recruit/api/interviewer/service/InterviewersService.java

+        if (!interviewers.isEmpty()) {
+            interviewerSortHelper.sort(interviewers, sortType);
+        }


interviewers가 null 이라면 즉, 빈 배열이라면 아래 return return interviewers.stream().map(InterviewerResponseDto::from).toList(); 에서 NPE가 발생 할 것 같은데
이 부분 예외 처리를 따로 하면 좋을 것 같아요

실제로 지금 상황에서 interviewer 가 아무도 없을 때 500에러가 떠서 운영 환경에서 사용자는 아무 액션을 받지못함 (FE가 처리를 못하니까)

확인했습니다! 해당 부분은 9591722 커밋에 반영했습니다.

@Override public List<InterviewerResponseDto> findAll(String sortType) { List<Interviewer> interviewers = interviewerLoadPort.findAll(); if (interviewers != null && !interviewers.isEmpty()) { interviewerSortHelper.sort(interviewers, sortType); return interviewers.stream().map(InterviewerResponseDto::from).toList(); } return Collections.emptyList(); }

LJH098 · 2024-09-23T06:48:48Z

server/Recruit-Api/src/main/java/com/econovation/recruit/api/record/service/RecordService.java

+    private RecordsViewResponseDto createEmptyResponse(PageInfo pageInfo) {
+        return RecordsViewResponseDto.of(
+                pageInfo,
+                Collections.emptyList(),
+                Collections.emptyMap(),
+                Collections.emptyList());
+    }


저는 Dto 클래스 안에 static으로 Dto를 만드는 정적 팩토리 메서드를 사용하는데
service단에 private으로 Dto 만드는 메서드를 사용하시는 이유가 있을까요??

어떤 관점에서 service에 작성을 한건지 궁금합니다.

기존에 메서드 내부에 작성된 로직인데, DTO 클래스에 넣는게 좋다고 생각합니다! 반영하겠습니다.

b4dc8b6 커밋에 반영했습니다!

LJH098 · 2024-09-23T06:58:28Z

server/Recruit-Api/src/main/java/com/econovation/recruit/api/record/service/RecordService.java

+    private Map<String, Double> getScoreMap(Integer year, List<String> applicantIds, Map<String, Integer> yearByAnswerIdMap) {
+        List<Score> scores = scoreLoadPort.findByApplicantIds(applicantIds);
+        return scores.stream()
+                .filter(score -> year == null || yearByAnswerIdMap.get(score.getApplicantId()).equals(year))
+                .collect(
+                        Collectors.groupingBy(
+                                Score::getApplicantId,
+                                Collectors.averagingDouble(Score::getScore)));
    }


왜 메소드 명이 getScoreMap이죠? 이렇게 메소드 분리해서 책임을 명확히 분리할 거면 이 메소드를 사용하는 곳에서 이 메소드가 어떤
역할을 하는지 직관적이어야 합니다. (그래야 다른 사람이 코드를 읽을 때 파악하기 쉬움)

그런데 이 메소드 역할은 Score를 가져와서 평균을 내는 작업이네요

함수 내부 로직으로 있던 코드인데, 변수명이 scoreMap이라 그대로 사용했습니다.
4b0bf42 커밋에서 calculateAverageScoresByApplicant로 메서드명 변경했습니다!

LJH098 · 2024-09-23T07:05:29Z

server/Recruit-Api/src/main/java/com/econovation/recruit/api/record/service/RecordService.java

+        if (sortType.equals("score")) {
+            records = sortRecordsByScoresDesc(result, scoreMap, page);
+        } else {
+            records = sortRecordsByApplicantsAndSortType(result, applicants);
+        }
+
+        PageInfo pageInfo = applicantQueryUseCase.getPageInfo(year, page, searchKeyword);
+        return RecordsViewResponseDto.of(pageInfo, records, scoreMap, applicants);
+    }


위에 sortType이 score인걸 확인하고 있는데 왜 아래서 또 확인을하고 적용하는 거죠? 한꺼번에 하면 안되는 이유가 있나요?
왜 두번 체크하는지 모르겠습니다.

a613304 해당 커밋에서 공통로직을 메서드로 추출 후 분기를 하나로 처리했습니다!

dad5f5a 커밋에서 기수별 면접기록 필터링 로직도 메서드로 추출했습니다.

rlajm1203 · 2024-09-23T08:44:11Z

...-Api/src/main/java/com/econovation/recruit/api/applicant/controller/ApplicantController.java

-            @ParameterObject String order) {
-        AnswersResponseDto result = applicantQueryUseCase.execute(year, page, order);
+            @ParameterObject String order,
+            @RequestParam(required = false) String searchKeyword) {


이 부분은 searchKeyword가 null일 경우에는 전체 조회를 하기 위한 용도인 거죠?

그리고 혹시 @ParameterObject 대신 @RequestParam을 사용하신 이유가 따로 있을까요? ㅎㅎ

@ParameterObject를 사용하면 swagger에서 required 필드로 보여 필수로 입력해야 한다고 인지됩니다!
그래서 @RequestParam의 required 값을 false로 두어 필수 값이 아님을 명시했습니다.

rlajm1203 · 2024-09-23T08:47:40Z

server/Recruit-Api/src/main/java/com/econovation/recruit/api/record/service/RecordService.java

    /**
     * 1. Newest // 시간 순 오름차순 2. Name // 이름순 오름차순 3. Object // 지원 분야별 오름차순 4. Score // 점수 내림차순
     *
-     * @return List<RecordResponseDto> // 지원자의 면접기록을 페이지별로 조회합니다 ( 이 화면에서는 Applicants,Scores,
-     *     Records를 모두 조회합니다 )
+     * @return List<RecordResponseDto> // 지원자의 면접기록을 페이지별로 조회합니다 ( 이 화면에서는 Applicants,Scores, Records를 모두 조회합니다 )
     */
    @Override
    public RecordsViewResponseDto execute(Integer page, Integer year, String sortType) {
        List<Record> result = recordLoadPort.findAll(page);
        PageInfo pageInfo = getPageInfo(page);

        List<String> applicantIds = result.stream().map(Record::getApplicantId).toList();
-        List<Score> scores = scoreLoadPort.findByApplicantIds(applicantIds);
-        List<MongoAnswer> applicants = applicantQueryUseCase.execute(applicantIds).stream().filter(applicant ->year == null || applicant.getYear().equals(year)).toList();
+        List<MongoAnswer> applicants = applicantQueryUseCase.execute(applicantIds).stream()
+                .filter(applicant -> year == null || applicant.getYear().equals(year)).toList();

        if (result.isEmpty() || applicants.isEmpty()) {
-            return RecordsViewResponseDto.of(
-                    pageInfo,
-                    Collections.emptyList(),
-                    Collections.emptyMap(),
-                    Collections.emptyList());
+            return createEmptyResponse(pageInfo);
        }

        Map<String, Integer> yearByAnswerIdMap = applicants.stream().collect(Collectors.toMap(MongoAnswer::getId, MongoAnswer::getYear));
-        Map<String, Double> scoreMap =
-                scores.stream()
-                        .filter(score ->year == null || yearByAnswerIdMap.get(score.getApplicantId()).equals(year))
-                        .collect(
-                                Collectors.groupingBy(
-                                        Score::getApplicantId,
-                                        Collectors.averagingDouble(Score::getScore)));
+        Map<String, Double> scoreMap = getScoreMap(year, applicantIds, yearByAnswerIdMap);

        result = result.stream().filter(record -> year == null ||
-            Optional.ofNullable(record.getApplicantId())
-                    .map(yearByAnswerIdMap::get)
-                    .map(y -> y.equals(year))
-                    .orElse(false)
-        )
-        .toList();
+                        Optional.ofNullable(record.getApplicantId())
+                                .map(yearByAnswerIdMap::get)
+                                .map(y -> y.equals(year))
+                                .orElse(false)
+                )
+                .toList();

        applicants = new ArrayList<>(applicants); // Unmodifiable List일 경우 Sort 불가. stream().toList()의 결과는 Unmodifiable List

+        List<Record> records;
        if (sortType.equals("score")) {
-            List<Record> records = sortRecordsByScoresDesc(result, scoreMap);
-            return RecordsViewResponseDto.of(pageInfo, records, scoreMap, applicants);
+            records = sortRecordsByScoresDesc(result, scoreMap);
        } else {
-            List<Record> records = sortRecordsByApplicantsAndSortType(result, applicants, sortType);
-            return RecordsViewResponseDto.of(pageInfo, records, scoreMap, applicants);
+            records = sortRecordsByApplicantsAndSortType(result, applicants, sortType);
        }
+
+        return RecordsViewResponseDto.of(pageInfo, records, scoreMap, applicants);
+    }


엇 이 부분은 제가 이슈로 작성한 BE-103 내용이긴 한데, 명덕님께서 해결해주셨네요!

BE-103 이슈는 제가 당장 해결하지 못할 것 같아서 Assignee를 저로 하지는 않았지만 명덕님께서 해결하셨다면

해결하신 이슈에 Assignee 설정 부탁드리겠습니다!

넵 Assignee 설정 완료했습니다!

Profile-exe added 13 commits September 23, 2024 14:47

[feat]: 지원 현황 검색과 조회로직 통합 및 DB레벨 정렬 적용

1eedccd

[refactor]: 재사용되는 로직 메서드로 추출

d168a5f

[fix]: 데이터가 없는 경우 sortHelper의 IndexOutOfBoundsException 방지를 위한 분기 추가

3f06ec4

[refactor]: 알파벳 역순으로 순서 수정

e27fc6f

데이터베이스 역순 정렬과 동일하게 정렬 수행

[feat]: AnswerAdaptor 검색과 정렬이 적용된 면접 기록 지원자 조회 구현

9a05ba9

[feat]: AnswerService 면접 기록 지원자 조회 구현

7500caa

[feat]: AnswerService 면접 기록 지원자 페이지 정보 구현

e21cb07

[feat]: 면접 기록 조회 API 검색 적용 및 정렬 개선

f28b145

[feat]: 페이징 없는 지원자 조회 구현

9a73ce8

점수 정렬은 지원자를 모두 불러와 scoreMap과 매핑하고 정렬을 수행한 후 페이지별로 잘라야 함

[feat]: 어플리케이션 레벨 페이징 유틸 함수 구현

005ddc3

[fix]: 점수 기준 정렬 오류 해결

1bbc0c0

[refactor]: 검색 쿼리 파라미터 required를 false로 변경

6a12907

- `@ParameterObject` 사용 시 swagger에서 필수 값으로 나타남 - `@RequestParam`의 `required` 속성을 `false`로 두어 swagger에 쿼리 파라미터 필수 여부가 올바르게 적용

[fix]: pageInfo가 올바른 totalCount를 가지도록 수정

2600682

- 불필요한 getPageInfo 메서드 제거

Profile-exe added bug-fix🚨 Bug Fix ( HotFix ) feature✨ This issue or pull request already exists labels Sep 23, 2024

Profile-exe requested review from rlajm1203 and LJH098 September 23, 2024 05:58

Profile-exe self-assigned this Sep 23, 2024

Profile-exe requested review from BlackBean99 and loopy-lim as code owners September 23, 2024 05:58

This was linked to issues Sep 23, 2024

[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #263

Closed

[BE-103] 면접 기록 조회 페이지네이션을 수정합니다. #272

Open

LJH098 requested changes Sep 23, 2024

View reviewed changes

[fix]: interviewer가 없는 경우 NPE 방지를 위한 분기 처리 추가

9591722

rlajm1203 reviewed Sep 23, 2024

View reviewed changes

Profile-exe added 4 commits September 23, 2024 20:49

[style]: spotless

b395075

[refactor]: RecordsViewResponseDto 내부 정적 팩토리 메서드 구현

b4dc8b6

[refactor]: getScoreMap 메서드 calculateAverageScoresByApplicant 로 이름 변경

4b0bf42

[refactor]: 면접 기록 조회 공통로직 메서드로 추출 및 if 분기 하나로 통합

a613304

Profile-exe added 2 commits September 23, 2024 23:50

[refactor]: 기수별 면접기록 필터링 로직 메서드로 추출

dad5f5a

[style]: spotless

09c1d32

LJH098 approved these changes Sep 24, 2024

View reviewed changes

LJH098 merged commit cefca5f into develop Sep 24, 2024

LJH098 mentioned this pull request Sep 24, 2024

release 4.0.1 #277

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #274

[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #274

Profile-exe commented Sep 23, 2024

LJH098 Sep 23, 2024

Profile-exe Sep 23, 2024

LJH098 Sep 23, 2024

LJH098 Sep 23, 2024

Profile-exe Sep 23, 2024

LJH098 Sep 23, 2024

Profile-exe Sep 23, 2024

Profile-exe Sep 23, 2024

LJH098 Sep 23, 2024

Profile-exe Sep 23, 2024

LJH098 Sep 23, 2024

Profile-exe Sep 23, 2024

Profile-exe Sep 23, 2024

rlajm1203 Sep 23, 2024

Profile-exe Sep 23, 2024

rlajm1203 Sep 23, 2024

Profile-exe Sep 23, 2024

[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #274

[BE-97] 지원 현황 및 면접 기록 조회 API 정렬 로직 개선, 검색 쿼리 추가 #274

Conversation

Profile-exe commented Sep 23, 2024

개요

작업사항

변경로직

reference

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment