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-97] 동아리 지원 폼지 통계 전체조회 API 구현 #242

Merged
merged 18 commits into from
Feb 9, 2025

Conversation

KoSeonJe
Copy link
Collaborator

@KoSeonJe KoSeonJe commented Feb 5, 2025

🚀 작업 내용

  • 지원자수 상위 5개 학과 및 관련 정보 통계 로직 구현
  • 전 반기 대비 지원비율 정보 통계 로직 구현
  • 아코디언에 포함될 폼지 질문 정보 반환
  • 통계 로직 테스트 작성

🤔 고민했던 내용

  • 기존에 없던 repository에서 직접 dto에 담아 반환하도록 하였습니다. 담을 dto는 새로 생성했고, 해당 repsitory 패키지에 생성하였습니다
  • 통계 구현 로직이 복잡해져 관련 vo를 생성해 책임을 덜었습니다

💬 리뷰 중점사항

FormStatisticServiceImpl 클래스에 구현된 로직 봐주시면 좋을 것 같아요.
HalfYear로 된 VO도 봐주세요..!
전 반기 대비 지원 비율 구하는게 조금 어렵습니다..ㅠ

Summary by CodeRabbit

  • 신규 기능

    • 양식 제출에 대한 종합 통계 기능이 추가되어 전체 신청 수, 부서별 분석, 지원자 및 항목별 성과 데이터를 제공합니다.
    • 최근 양식 제출 내역과 부서 순위 정보를 확인할 수 있어 양식 성과를 보다 명확하게 파악할 수 있습니다.
  • 개선 사항

    • 날짜 및 시간 처리 형식이 개선되어 일관된 정보 표시가 가능합니다.

@KoSeonJe KoSeonJe changed the title [DDING-97] 동아리 지원 폼지 통계 전체조회 [DDING-97] 동아리 지원 폼지 통계 전체조회 API 구현 Feb 5, 2025
Copy link

coderabbitai bot commented Feb 5, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

이번 PR은 여러 유틸리티 클래스와 API, 서비스, 저장소, DTO 및 테스트 코드에 기능 추가 및 리팩토링을 진행하였습니다. CalculationUtils와 TimeUtils에 새로운 정적 메소드 및 포맷팅 개선이 이루어졌으며, 중앙폼 API 및 통계 관련 기능(getFormStatistics)을 위한 메소드와 DTO, 서비스, 레코드가 추가되었습니다. 또한, 관련 저장소에 쿼리 메소드가 도입되고 테스트 케이스가 보강되어 통계 데이터 및 필드 조회 기능이 강화되었습니다.

Changes

파일 변경 요약
src/main/java/ddingdong/ddingdongBE/common/utils/CalculationUtils.java CalculationUtils 클래스 추가 (calculateRatio, calculateDifference, calculateDifferenceRatio 메소드 도입)
src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java 코드 포맷 정리, processDate 개선 및 getYearAndMonth 메소드 추가
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java
src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java
CentralFormApi 인터페이스와 컨트롤러에 getFormStatistics 메소드 추가, 기존 메소드 서명 정리
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormStatisticsResponse.java FormStatisticsResponse 및 관련 nested record (DepartmentStatisticResponse, ApplicantStatisticResponse, FieldStatisticsResponse, FieldStatisticsListResponse) 추가
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java
FacadeCentralFormService에 getStatisticsByForm 메소드 추가 및 구현, formStatisticService 필드 추가
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java
새로운 인터페이스 및 구현 클래스 도입: FormStatisticService와 통계 관련 메소드(getTotalApplicationCountByForm, createDepartmentStatistics, createApplicationStatistics, createFieldStatisticsByForm) 추가
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java 새로운 record FormStatisticsQuery 및 nested record 추가, from 메소드 도입
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java countByFormField 메소드 추가 (FormField 기반 FormAnswer 개수 조회)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java countByForm, findTopDepartmentsByFormId, findRecentFormByDateWithApplicationCount 메소드 추가
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/DepartmentInfo.java DepartmentInfo 인터페이스 추가 (getDepartment, getCount 메소드)
src/main/java/ddingdong/ddingdongBE/common/support/FixtureMonkeyFactory.java JqwikPlugin 및 JavaArbitraryResolver를 포함한 플러그인 구성 추가로 메소드 수정
src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java 최근 폼 및 부서 통계 관련 테스트 메소드 2건 추가
src/test/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImplTest.java FormStatisticService의 기능 테스트를 위한 신규 테스트 클래스 추가
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java findFieldWithAnswerCountByFormId 메소드 추가 및 기존 findAllByFormAndSection 제거
src/main/java/ddingdong/ddingdongBE/domain/form/repository/dto/FieldListInfo.java FieldListInfo 인터페이스 추가 (getId, getQuestion, getCount, getType, getSection 메소드)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/RecentFormInfo.java RecentFormInfo 인터페이스 추가 (getDate, getCount 메소드)
src/test/java/ddingdong/ddingdongBE/common/utils/CalculationUtilsTest.java CalculationUtils.calculateDifferenceRatio 기능 테스트를 위한 신규 테스트 클래스 추가
src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepositoryTest.java findFieldWithAnswerCountByFormId 테스트를 위한 신규 테스트 클래스 및 메소드 추가

Sequence Diagram(s)

sequenceDiagram
  participant C as Client
  participant CT as CentralFormController
  participant FCS as FacadeCentralFormService
  participant FS as FormStatisticServiceImpl

  C->>CT: getFormStatistics(formId, principalDetails)
  CT->>FCS: getStatisticsByForm(user, formId)
  FCS->>FS: getStatisticsByForm(user, formId)
  FS-->>FCS: FormStatisticsQuery 반환
  FCS-->>CT: FormStatisticsQuery 반환
  CT->>C: FormStatisticsResponse 반환 (from query)
Loading

Suggested labels

D-3, 📖문서

Suggested reviewers

  • wonjunYou
  • 5uhwann
  • Seooooo24

📜 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 d1d5cac and a21b31e.

📒 Files selected for processing (3)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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 (15)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (2)

58-80: 이전 기간 대비 지원율 계산 로직 확인

createApplicationRateByForm 메서드에서 APPLICATION_COMPARE_COUNT 횟수만큼 반복하며 지원율을 계산하고 있습니다. 반복문 내에서 break 문을 사용하여 최대 지원자 수가 없을 경우 반복을 중단하는 로직이 있으므로, 반복문 조건을 명확하게 하기 위해 while 문이나 조건문을 사용하는 것을 고려해보세요.

다음과 같이 변경할 수 있습니다:

int count = 0;
while (count < APPLICATION_COMPARE_COUNT) {
    // 기존 로직 유지
    count++;
}

96-98: Integer 값의 변환 메서드 확인

parseToInt 메서드는 Integer 값을 int로 변환하면서 null 체크를 수행하고 있습니다. 유틸리티 클래스로 분리하여 재사용성을 높일 수 있습니다.

src/main/java/ddingdong/ddingdongBE/common/utils/CalculationUtils.java (1)

5-10: 정수로 반환되는 비율의 정확도 개선

현재 calculateRate 메서드는 비율 계산 후 (int) 캐스팅을 통해 정수로 반환하고 있어 소수점 이하의 값이 손실될 수 있습니다. 비율의 정확도를 높이기 위해 반환 타입을 double로 변경하는 것을 고려해보세요.

다음과 같이 수정할 수 있습니다:

-    public static int calculateRate(int count, int totalCount) {
+    public static double calculateRate(int count, int totalCount) {

...

-        return (int) ((double) count / totalCount * 100);
+        return ((double) count / totalCount) * 100;
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java (1)

9-18: 각 메서드에 Javadoc 문서화가 필요합니다.

인터페이스의 메서드들이 명확한 목적을 가지고 있지만, 각 메서드의 구체적인 동작과 반환값에 대한 문서화가 필요합니다.

다음과 같이 Javadoc을 추가하는 것을 제안합니다:

 public interface FormStatisticService {
+    /**
+     * 특정 폼의 총 지원자 수를 반환합니다.
+     * @param form 통계를 조회할 폼
+     * @return 총 지원자 수
+     */
     int getTotalApplicationCountByForm(Form form);

+    /**
+     * 학과별 지원자 순위를 생성합니다.
+     * @param form 통계를 조회할 폼
+     * @return 학과별 순위 목록
+     */
     List<DepartmentRankQuery> createDepartmentRankByForm(Form form);

+    /**
+     * 지원율 통계를 생성합니다.
+     * @param form 통계를 조회할 폼
+     * @return 지원율 통계 목록
+     */
     List<ApplicantRateQuery> createApplicationRateByForm(Form form);

+    /**
+     * 필드별 통계 목록을 생성합니다.
+     * @param form 통계를 조회할 폼
+     * @return 필드별 통계 목록
+     */
     List<FieldStatisticsListQuery> createFieldStatisticsListByForm(Form form);
 }
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java (1)

23-30: 불필요한 빈 줄을 제거해주세요.

ApplicantRateQuery 레코드 내부에 불필요한 빈 줄이 있습니다.

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

     public record ApplicantRateQuery(
             String label,
             int count,
             int comparedToLastSemester
-    ) {
-
-
-    }
+    ) {}
src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java (1)

19-28: processDate 메서드의 중복된 null 체크 제거 필요

현재 구현에서 null과 blank 체크가 중복되어 있으며, parseToLocalDateTime을 호출하기 전에 불필요한 검사를 수행하고 있습니다.

다음과 같이 리팩토링하는 것을 추천드립니다:

    public static LocalDateTime processDate(String dateString, LocalDateTime currentDate) {
-        if (dateString == null) {
-            return null;
-        }
-
-        if (dateString.isBlank()) {
-            return null;
-        }
-
         return parseToLocalDateTime(dateString);
    }

참고: currentDate 파라미터가 사용되지 않고 있습니다. 필요하지 않다면 제거를 고려해보세요.

src/main/java/ddingdong/ddingdongBE/domain/form/service/vo/HalfYear.java (1)

20-27: minusHalves 메서드 명칭 개선 및 문서화 필요

메서드의 목적과 동작이 명확하게 전달되지 않습니다.

다음과 같이 JavaDoc을 추가하고 메서드 이름을 더 명확하게 변경하는 것을 추천드립니다:

-    public void minusHalves() {
+    /**
+     * 이전 반기로 이동합니다.
+     * 현재가 상반기인 경우 전년도 하반기로,
+     * 하반기인 경우 같은 년도의 상반기로 이동합니다.
+     */
+    public void moveToPreviousHalf() {
src/main/java/ddingdong/ddingdongBE/domain/form/service/vo/ApplicationRates.java (2)

30-31: 상수값에 대한 설명 필요

DEFAULT_APPLICATION_RATE 상수의 의미와 값이 100인 이유가 명확하지 않습니다.

다음과 같이 상수에 대한 설명을 추가하는 것을 추천드립니다:

+    /**
+     * 새로운 학기의 기본 비교율(100%)을 나타냅니다.
+     * 이전 학기와의 비교 기준점으로 사용됩니다.
+     */
     private static final int DEFAULT_APPLICATION_RATE = 100;

34-42: add 메서드의 가독성 개선 필요

메서드의 로직이 다소 복잡하고, 매개변수의 의미가 명확하지 않습니다.

다음과 같이 메서드를 개선하는 것을 추천드립니다:

+    /**
+     * 새로운 지원율 데이터를 추가합니다.
+     * @param label 학기 레이블 (예: "2024-1")
+     * @param count 지원자 수
+     * @param comparedToLastSemester 이전 학기 대비 비교율
+     */
     public void add(String label, int count, int comparedToLastSemester) {
+        if (count < 0) {
+            throw new IllegalArgumentException("지원자 수는 음수일 수 없습니다.");
+        }
         if (applicationRates.isEmpty()) {
             applicationRates.add(new ApplicationRate(label, count, comparedToLastSemester));
             return;
         }
-        ApplicationRate lastSemester = applicationRates.get(0);
-        lastSemester.updateComparedToLastSemester(comparedToLastSemester);
+        ApplicationRate currentSemester = applicationRates.get(0);
+        currentSemester.updateComparedToLastSemester(comparedToLastSemester);
         applicationRates.add(0, new ApplicationRate(label, count, DEFAULT_APPLICATION_RATE));
     }
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2)

32-40: 인덱스 추가를 고려해 보세요.

department 컬럼에 대한 GROUP BY와 ORDER BY가 수행되므로, 성능 최적화를 위해 department 컬럼에 인덱스 추가를 고려해 보세요.


42-54: 쿼리 최적화 제안

현재 쿼리는 모든 form_application 레코드를 스캔한 후 그룹화하여 최대값을 찾고 있습니다. 다음과 같이 서브쿼리를 사용하여 최적화할 수 있습니다:

-    @Query(value = """
-            SELECT COUNT(fa.id) AS count
-            FROM form_application fa
-            JOIN form f ON fa.form_id = f.id
-            WHERE f.end_date BETWEEN :startDate AND :endDate
-            GROUP BY f.id
-            ORDER BY count DESC
-            LIMIT 1
-            """, nativeQuery = true)
+    @Query(value = """
+            WITH form_counts AS (
+                SELECT f.id, COUNT(fa.id) AS count
+                FROM form f
+                LEFT JOIN form_application fa ON fa.form_id = f.id
+                WHERE f.end_date BETWEEN :startDate AND :endDate
+                GROUP BY f.id
+            )
+            SELECT count FROM form_counts
+            WHERE count = (SELECT MAX(count) FROM form_counts)
+            """, nativeQuery = true)
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormStatisticsResponse.java (2)

13-23: 빌더 패턴 사용 시 필수 필드 검증 추가 필요

빌더 패턴을 사용할 때 필수 필드에 대한 유효성 검증이 없습니다. 다음과 같이 필수 필드를 검증하는 정적 팩토리 메서드를 추가하는 것이 좋습니다:

public static class FormStatisticsResponseBuilder {
    public FormStatisticsResponse build() {
        if (departmentRanks == null || applicantRates == null || fields == null) {
            throw new IllegalStateException("Required fields must not be null");
        }
        return new FormStatisticsResponse(totalCount, departmentRanks, applicantRates, fields);
    }
}

88-104: 스트림 체이닝 리팩토링 제안

현재 코드는 여러 개의 스트림 연산을 별도로 수행하고 있습니다. 다음과 같이 메서드 참조를 활용하여 더 간결하게 작성할 수 있습니다:

    public static FormStatisticsResponse from(FormStatisticsQuery query) {
-        List<DepartmentRankResponse> departmentRankResponses = query.departmentRanks().stream()
-                .map(DepartmentRankResponse::from)
-                .toList();
-        List<ApplicantRateResponse> applicantRateResponses = query.applicantRates().stream()
-                .map(ApplicantRateResponse::from)
-                .toList();
-        List<FieldStatisticsListResponse> fieldStatisticsListResponses = query.fields().stream()
-                .map(FieldStatisticsListResponse::from)
-                .toList();
        return FormStatisticsResponse.builder()
                .totalCount(query.totalCount())
-                .departmentRanks(departmentRankResponses)
-                .applicantRates(applicantRateResponses)
-                .fields(fieldStatisticsListResponses)
+                .departmentRanks(query.departmentRanks().stream().map(DepartmentRankResponse::from).toList())
+                .applicantRates(query.applicantRates().stream().map(ApplicantRateResponse::from).toList())
+                .fields(query.fields().stream().map(FieldStatisticsListResponse::from).toList())
                .build();
    }
src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java (2)

154-221: 테스트 메서드 이름을 한국어 프로젝트 컨벤션에 맞게 수정하면 좋겠습니다.

테스트 로직은 잘 구현되어 있으나, 메서드 이름이 영문으로 되어 있어 한국어 프로젝트의 일관성에 맞지 않습니다.

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

-void findTopFiveDepartmentsByForm_ShouldReturnTopFiveDepartments()
+void 상위_5개_학과_조회_성공()

223-275: 테스트 메서드 이름을 한국어 프로젝트 컨벤션에 맞게 수정하면 좋겠습니다.

테스트 로직은 잘 구현되어 있으나, 메서드 이름이 영문으로 되어 있어 한국어 프로젝트의 일관성에 맞지 않습니다.

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

-void findMaxApplicationCountByDateRange_ShouldReturnHighestCount()
+void 기간별_최대_지원자수_조회_성공()
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 185c0bb and f8645c4.

📒 Files selected for processing (20)
  • src/main/java/ddingdong/ddingdongBE/common/utils/CalculationUtils.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.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/response/FormStatisticsResponse.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (3 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/vo/ApplicationRates.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/vo/HalfYear.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/DepartmentInfo.java (1 hunks)
  • src/main/resources/db/migration/V36__form_add_column_sections.sql (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/common/support/FixtureMonkeyFactory.java (2 hunks)
  • src/test/java/ddingdong/ddingdongBE/common/support/TestContainerSupport.java (0 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java (2 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImplTest.java (1 hunks)
💤 Files with no reviewable changes (1)
  • src/test/java/ddingdong/ddingdongBE/common/support/TestContainerSupport.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/FormStatisticServiceImpl.java (2)

36-39: 전체 지원자 수 계산 로직이 정확합니다

getTotalApplicationCountByForm 메서드는 해당 폼의 전체 지원자 수를 정확하게 반환하고 있습니다.


42-56: 학과별 지원자 순위 계산 로직이 효율적입니다

createDepartmentRankByForm 메서드는 스트림과 람다식을 활용하여 상위 5개의 학과에 대한 순위를 효과적으로 계산하고 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/DepartmentInfo.java (1)

3-8: 인터페이스 정의가 명확하고 적절합니다

DepartmentInfo 인터페이스는 학과명과 카운트를 반환하는 메서드를 정의하여 필요한 데이터를 효과적으로 제공하도록 설계되었습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (1)

3-3: Spring Data JPA 네이밍 컨벤션을 잘 따르고 있습니다!

메서드 이름이 명확하고 Spring Data JPA의 네이밍 규칙을 잘 준수하고 있습니다. 반환 타입도 count 연산에 적절한 int를 사용하였습니다.

Also applies to: 8-9

src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (1)

7-7: 파사드 패턴을 일관성 있게 잘 적용하였습니다!

새로 추가된 통계 조회 메서드가 기존 인터페이스의 스타일을 잘 따르고 있으며, 파사드 패턴의 목적에 맞게 구현되었습니다.

Also applies to: 22-23

src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java (1)

7-13: 통계 데이터 구조가 잘 설계되었습니다!

레코드를 사용하여 불변성을 보장하고, Builder 패턴을 적용하여 객체 생성의 유연성을 확보한 점이 좋습니다. 각 필드의 이름도 명확하게 지어졌습니다.

src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java (1)

38-40: isFirstHalf 메서드 구현이 명확하고 적절합니다.

월 범위 체크가 정확하게 구현되어 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (1)

30-30: LGTM!

Spring Data JPA의 명명 규칙을 잘 따르고 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java (1)

64-68: LGTM!

기존 컨트롤러의 패턴을 잘 따르고 있으며, 책임 분리가 잘 되어 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (1)

82-88: LGTM!

API 문서화가 잘 되어 있으며, RESTful 규칙을 잘 준수하고 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (1)

87-95: 통계 데이터를 효과적으로 수집하고 있습니다.

통계 데이터를 수집하는 로직이 잘 구조화되어 있으며, 각 통계 항목이 명확하게 분리되어 있습니다.

src/test/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImplTest.java (3)

48-83: 테스트 케이스가 잘 구성되어 있습니다.

총 지원자 수를 정확하게 검증하는 테스트가 잘 작성되어 있습니다.


85-155: 학과별 순위 테스트가 상세하게 구현되어 있습니다.

각 학과의 순위와 지원자 수를 정확하게 검증하는 테스트가 잘 작성되어 있습니다.


157-215: 지원율 비교 테스트가 체계적으로 구성되어 있습니다.

학기별 지원자 수와 비율을 정확하게 검증하는 테스트가 잘 작성되어 있습니다.

Comment on lines 16 to 18
public static HalfYear from(LocalDate date) {
return new HalfYear(date.getYear(), TimeUtils.isFirstHalf(date.getMonthValue()));
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

from 메서드에 입력값 검증 추가 필요

date가 null인 경우에 대한 처리가 누락되어 있습니다.

다음과 같이 null 체크를 추가하는 것을 추천드립니다:

    public static HalfYear from(LocalDate date) {
+        if (date == null) {
+            throw new IllegalArgumentException("날짜는 null일 수 없습니다.");
+        }
        return new HalfYear(date.getYear(), TimeUtils.isFirstHalf(date.getMonthValue()));
    }
📝 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 static HalfYear from(LocalDate date) {
return new HalfYear(date.getYear(), TimeUtils.isFirstHalf(date.getMonthValue()));
}
public static HalfYear from(LocalDate date) {
if (date == null) {
throw new IllegalArgumentException("날짜는 null일 수 없습니다.");
}
return new HalfYear(date.getYear(), TimeUtils.isFirstHalf(date.getMonthValue()));
}

Comment on lines +28 to +31
// if (context.findAnnotation(MaxOfLength.class).isPresent()) {
// return stringArbitrary.ofMaxLength(10);
// }
return stringArbitrary;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

주석 처리된 코드 정리 필요

구현되지 않은 기능에 대한 주석 처리된 코드가 있습니다.

주석 처리된 코드는 제거하거나 구현을 완료해야 합니다. 만약 향후 구현 예정이라면 TODO 주석으로 변경하는 것이 좋습니다:

-//                                        if (context.findAnnotation(MaxOfLength.class).isPresent()) {
-//                                            return stringArbitrary.ofMaxLength(10);
-//                                        }
+                                        // TODO: MaxOfLength 어노테이션 지원 구현
📝 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
// if (context.findAnnotation(MaxOfLength.class).isPresent()) {
// return stringArbitrary.ofMaxLength(10);
// }
return stringArbitrary;
// TODO: MaxOfLength 어노테이션 지원 구현
return stringArbitrary;

Comment on lines 217 to 263
@DisplayName("폼지의 질문 정보")
@Test
void createFieldStatisticsListByForm() {
// given
Form formFirst = fixture.giveMeBuilder(Form.class)
.set("id", 1L)
.set("club", null)
.set("startDate", LocalDate.of(2020, 2, 1))
.set("endDate", LocalDate.of(2020, 3, 1))
.set("sections", List.of("공통"))
.sample();
Form savedForm = formRepository.save(formFirst);
FormField formField = FormField.builder()
.question("설문 질문")
.required(true)
.fieldOrder(1)
.section("기본 정보")
.options(List.of("옵션1", "옵션2", "옵션3"))
.fieldType(FieldType.TEXT)
.form(savedForm)
.build();
FormField savedField = formFieldRepository.save(formField);

FormAnswer answer = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer2 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer3 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
formAnswerRepository.saveAll(List.of(answer, answer2, answer3));
// when
List<FieldStatisticsListQuery> fieldStatisticsList = formStatisticService.createFieldStatisticsListByForm(
savedForm);
// then
assertThat(fieldStatisticsList.size()).isEqualTo(1);
assertThat(fieldStatisticsList.get(0).count()).isEqualTo(3);
assertThat(fieldStatisticsList.get(0).question()).isEqualTo("설문 질문");
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

필드 통계 테스트의 개선이 필요합니다.

테스트에서 formApplicationvalue 필드에 null 값을 사용하고 있습니다. 더 현실적인 테스트 데이터를 사용하는 것이 좋겠습니다.

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

-                .formApplication(null)
-                .value(null)
+                .formApplication(formApplication)
+                .value("답변 내용")

Committable suggestion skipped: line range outside the PR's diff.

@@ -1 +1 @@
ALTER TABLE form ADD COLUMN sections TEXT NOT NULL DEFAULT '[]';
ALTER TABLE form ADD COLUMN sections VARCHAR(255) NOT NULL DEFAULT '[]';
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

sections 컬럼의 데이터 타입 선택을 재고해 보세요.

JSON 배열을 저장하는 용도라면 VARCHAR(255) 대신 TEXT나 JSONB 타입을 사용하는 것이 더 적절할 수 있습니다.

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

-ALTER TABLE form ADD COLUMN sections VARCHAR(255) NOT NULL DEFAULT '[]';
+ALTER TABLE form ADD COLUMN sections JSONB NOT NULL DEFAULT '[]';
📝 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
ALTER TABLE form ADD COLUMN sections VARCHAR(255) NOT NULL DEFAULT '[]';
ALTER TABLE form ADD COLUMN sections JSONB NOT NULL DEFAULT '[]';

@github-actions github-actions bot added the D-3 label Feb 6, 2025
KoSeonJe

This comment was marked as abuse.

@github-actions github-actions bot added D-2 and removed D-3 labels Feb 7, 2025
Copy link
Collaborator

@Seooooo24 Seooooo24 left a comment

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 D-1 and removed D-2 labels Feb 8, 2025
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: 1

🧹 Nitpick comments (12)
src/test/java/ddingdong/ddingdongBE/common/utils/CalculationUtilsTest.java (1)

10-25: 테스트 케이스 보완이 필요합니다.

현재 테스트는 기본적인 증가와 감소 시나리오만 다루고 있습니다. 다음과 같은 엣지 케이스들도 추가하면 좋을 것 같습니다:

  • 0값 테스트
  • 동일한 값 테스트 (변화율 0%)
  • 매우 큰 숫자 테스트
  • 음수 값 테스트 (허용되는 경우)

@ParameterizedTest를 사용하여 다음과 같이 리팩토링하는 것을 추천드립니다:

-    @DisplayName("beforeCount기준으로 beforeCount와 count의 증감율을 구한다.")
-    @Test
-    void calculateDifferenceRatio() {
-        // given
-        int beforeCount = 30;
-        int count = 45;
-
-        int beforeCount2 = 45;
-        int count2 = 30;
-        // when
-        int result = CalculationUtils.calculateDifferenceRatio(beforeCount, count);
-        int result2 = CalculationUtils.calculateDifferenceRatio(beforeCount2, count2);
-        // then
-        assertThat(result).isEqualTo(50);
-        assertThat(result2).isEqualTo(-33);
-    }
+    @DisplayName("beforeCount를 기준으로 증감율을 계산한다")
+    @ParameterizedTest
+    @CsvSource({
+        "30, 45, 50",    // 50% 증가
+        "45, 30, -33",   // 33% 감소
+        "100, 100, 0",   // 변화 없음
+        "0, 10, 0",      // 이전 값이 0인 경우
+        "10, 0, -100",   // 현재 값이 0인 경우
+        "1000000, 2000000, 100"  // 큰 숫자
+    })
+    void calculateDifferenceRatio(int beforeCount, int count, int expectedRatio) {
+        // when
+        int result = CalculationUtils.calculateDifferenceRatio(beforeCount, count);
+        
+        // then
+        assertThat(result).isEqualTo(expectedRatio);
+    }
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2)

33-44: 학과별 통계 쿼리에 인덱스 추가를 고려해보세요.

form_iddepartment 컬럼에 대한 인덱스가 있다면 쿼리 성능이 향상될 수 있습니다.


46-64: 서브쿼리 최적화를 검토해주세요.

현재 구현은 정확하지만, 다음과 같은 개선을 고려해볼 수 있습니다:

  1. 서브쿼리를 JOIN으로 변경하여 성능 향상
  2. start_date에 인덱스 추가
-    @Query(value = """
-            SELECT recent_forms.start_date AS date, COUNT(fa.id) AS count
-                        FROM (
-                            SELECT *
-                            FROM form
-                            WHERE club_id = :clubId
-                            AND start_date <= :date
-                            ORDER BY start_date
-                            LIMIT :size
-                        ) AS recent_forms
-                        LEFT JOIN form_application fa
-                        ON recent_forms.id = fa.form_id
-                        GROUP BY recent_forms.id
-            """, nativeQuery = true)
+    @Query(value = """
+            WITH recent_forms AS (
+                SELECT id, start_date
+                FROM form
+                WHERE club_id = :clubId
+                AND start_date <= :date
+                ORDER BY start_date
+                LIMIT :size
+            )
+            SELECT rf.start_date AS date, COUNT(fa.id) AS count
+            FROM recent_forms rf
+            LEFT JOIN form_application fa ON rf.id = fa.form_id
+            GROUP BY rf.id, rf.start_date
+            ORDER BY rf.start_date
+            """, nativeQuery = true)
src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java (2)

157-222: 테스트 케이스를 보완해주세요.

다음 테스트 케이스를 추가하면 좋을 것 같습니다:

  1. 학과가 없는 경우
  2. 동일한 지원자 수를 가진 학과들의 정렬 순서
  3. size 파라미터의 경계값 테스트

226-299: 테스트 데이터 설정을 개선해주세요.

  1. 테스트의 가독성을 위해 테스트 데이터 생성을 별도의 메서드로 분리
  2. 경계 조건 테스트 추가 (날짜가 같은 경우, size가 0인 경우 등)
+    private Form createFormWithDate(Club club, LocalDate startDate, LocalDate endDate) {
+        return fixture.giveMeBuilder(Form.class)
+                .set("club", club)
+                .set("startDate", startDate)
+                .set("endDate", endDate)
+                .set("sections", List.of("공통"))
+                .sample();
+    }
+
+    private FormApplication createFormApplication(Form form, String department) {
+        return FormApplication.builder()
+                .name("이름1")
+                .studentNumber("학번1")
+                .department(department)
+                .status(FormApplicationStatus.SUBMITTED)
+                .form(form)
+                .build();
+    }
src/main/java/ddingdong/ddingdongBE/common/utils/CalculationUtils.java (2)

5-10: 정밀도 향상을 위한 계산 방식 개선이 필요합니다.

현재 구현은 정수 나눗셈을 사용하여 정밀도가 떨어질 수 있습니다. 더 정확한 계산을 위해 BigDecimal 사용을 고려해보세요.

-    public static int calculateRatio(int numerator, int denominator) {
+    public static int calculateRatio(int numerator, int denominator) {
+        if (denominator == 0) {
+            return 0;
+        }
+        return BigDecimal.valueOf(numerator)
+            .multiply(BigDecimal.valueOf(100))
+            .divide(BigDecimal.valueOf(denominator), RoundingMode.HALF_UP)
+            .intValue();
+    }

5-19: 메서드 문서화가 필요합니다.

각 메서드의 목적, 매개변수, 반환값에 대한 JavaDoc 문서화가 필요합니다.

+    /**
+     * 비율을 계산합니다.
+     * @param numerator 분자
+     * @param denominator 분모
+     * @return 백분율로 표현된 비율 (0-100)
+     */
     public static int calculateRatio(int numerator, int denominator) {
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java (1)

10-19: 인터페이스 메서드 문서화가 필요합니다.

각 메서드의 목적과 반환값에 대한 자세한 설명이 필요합니다.

+    /**
+     * 특정 폼의 총 지원자 수를 반환합니다.
+     * @param form 조회할 폼
+     * @return 총 지원자 수
+     */
     int getTotalApplicationCountByForm(Form form);

+    /**
+     * 학과별 통계를 생성합니다.
+     * @param totalCount 총 지원자 수
+     * @param form 조회할 폼
+     * @return 학과별 통계 목록
+     */
     List<DepartmentStatisticQuery> createDepartmentStatistics(int totalCount, Form form);
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java (1)

44-52: 정적 팩토리 메서드의 유효성 검사가 필요합니다.

from 메서드에서 fieldListInfo가 null인 경우에 대한 처리가 없습니다.

     public static FieldStatisticsListQuery from(FieldListInfo fieldListInfo) {
+        if (fieldListInfo == null) {
+            throw new IllegalArgumentException("fieldListInfo must not be null");
+        }
         return new FieldStatisticsListQuery(
             fieldListInfo.getId(),
             fieldListInfo.getQuestion(),
             fieldListInfo.getCount(),
             fieldListInfo.getType(),
             fieldListInfo.getSection()
         );
     }
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1)

62-85: 첫 번째 데이터 처리 로직을 분리하는 것이 좋겠습니다.

현재 구현은 잘 작동하지만, 첫 번째 데이터 처리 로직을 별도의 private 메서드로 분리하면 가독성이 향상될 것 같습니다.

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

 private List<ApplicantStatisticQuery> createApplicationStatistics(Club club, Form form) {
     return IntStream.range(0, recentForms.size())
             .mapToObj(index -> {
                 RecentFormInfo recentFormInfo = recentForms.get(index);
                 String label = TimeUtils.getYearAndMonth(recentFormInfo.getDate());
                 int count = parseToInt(recentFormInfo.getCount());
-                if (index == 0) {
-                    return new ApplicantStatisticQuery(label, count, 0, 0);
-                }
-                int beforeCount = parseToInt(recentForms.get(index - 1).getCount());
-                int compareRatio = CalculationUtils.calculateDifferenceRatio(beforeCount, count);
-                int compareValue = CalculationUtils.calculateDifference(beforeCount, count);
-                return new ApplicantStatisticQuery(label, count, compareRatio, compareValue);
+                return index == 0
+                    ? createInitialStatistic(label, count)
+                    : createComparisonStatistic(label, count, recentForms.get(index - 1));
             }).toList();
 }

+private ApplicantStatisticQuery createInitialStatistic(String label, int count) {
+    return new ApplicantStatisticQuery(label, count, 0, 0);
+}
+
+private ApplicantStatisticQuery createComparisonStatistic(String label, int count, RecentFormInfo previousForm) {
+    int beforeCount = parseToInt(previousForm.getCount());
+    int compareRatio = CalculationUtils.calculateDifferenceRatio(beforeCount, count);
+    int compareValue = CalculationUtils.calculateDifference(beforeCount, count);
+    return new ApplicantStatisticQuery(label, count, compareRatio, compareValue);
+}
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1)

15-27: 쿼리 최적화를 고려해보세요.

현재 쿼리는 서브쿼리를 사용하고 있는데, 이는 불필요할 수 있습니다. 다음과 같이 단순화할 수 있습니다:

-    @Query(value = """
-            SELECT f.id AS id, f.question AS question, f.field_type AS type, f.section AS section, COUNT(fa.id) AS count
-            FROM (
-                    SELECT *
-                    FROM form_field field
-                    WHERE field.form_id = :formId
-            ) AS f
-            LEFT JOIN form_answer AS fa
-            ON fa.field_id = f.id
-            GROUP BY f.id
-            ORDER BY f.id
-            """, nativeQuery = true)
+    @Query(value = """
+            SELECT ff.id AS id, ff.question AS question, ff.field_type AS type, ff.section AS section, COUNT(fa.id) AS count
+            FROM form_field ff
+            LEFT JOIN form_answer fa ON fa.field_id = ff.id
+            WHERE ff.form_id = :formId
+            GROUP BY ff.id
+            ORDER BY ff.id
+            """, nativeQuery = true)
src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepositoryTest.java (1)

56-71: 중복 코드를 제거해주세요.

FormAnswer 객체 생성 코드가 반복되고 있습니다. 테스트 코드의 가독성을 위해 테스트 데이터 생성을 별도의 메서드로 분리하는 것이 좋겠습니다.

+    private FormAnswer createFormAnswer(FormField formField) {
+        return FormAnswer.builder()
+                .formApplication(null)
+                .value("테스트 답변")
+                .formField(formField)
+                .build();
+    }
+
     @Test
     void findFieldWithAnswerCountByFormId() {
         // ...
-        FormAnswer answer = FormAnswer.builder()
-                .formApplication(null)
-                .value(null)
-                .formField(savedField)
-                .build();
-        FormAnswer answer2 = FormAnswer.builder()
-                .formApplication(null)
-                .value(null)
-                .formField(savedField)
-                .build();
-        FormAnswer answer3 = FormAnswer.builder()
-                .formApplication(null)
-                .value(null)
-                .formField(savedField)
-                .build();
+        List<FormAnswer> answers = List.of(
+            createFormAnswer(savedField),
+            createFormAnswer(savedField),
+            createFormAnswer(savedField)
+        );
-        formAnswerRepository.saveAll(List.of(answer, answer2, answer3));
+        formAnswerRepository.saveAll(answers);
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 34f4004 and d1d5cac.

📒 Files selected for processing (18)
  • src/main/java/ddingdong/ddingdongBE/common/utils/CalculationUtils.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.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/response/FormStatisticsResponse.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/repository/dto/FieldListInfo.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (3 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormStatisticsQuery.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/RecentFormInfo.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/common/utils/CalculationUtilsTest.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java (2 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepositoryTest.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImplTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java
  • src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java
  • src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java
🔇 Additional comments (22)
src/test/java/ddingdong/ddingdongBE/common/utils/CalculationUtilsTest.java (2)

1-8: 코드 구조가 잘 구성되어 있습니다!

패키지 구조와 import 문이 적절하게 구성되어 있으며, AssertJ와 JUnit Jupiter를 사용한 테스트 구조가 잘 갖춰져 있습니다.


8-11: 테스트 클래스 명명 규칙과 문서화가 잘 되어있습니다!

클래스 이름이 테스트 대상 클래스에 "Test"를 붙이는 규칙을 잘 따르고 있으며, @DisplayName을 통한 한글 설명이 명확합니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/RecentFormInfo.java (1)

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

날짜와 개수를 조회하는 메서드가 명확하게 정의되어 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (1)

31-31: LGTM!

간단한 카운트 쿼리로 적절하게 구현되었습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (1)

82-91: API가 잘 문서화되어 있습니다.

OpenAPI 어노테이션을 사용한 API 문서화가 잘 되어있습니다. 보안 요구사항과 응답 상태 코드가 일관되게 정의되어 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (5)

38-40: 구현이 적절합니다!

폼 별 총 지원자 수를 조회하는 로직이 간단하고 명확하게 구현되어 있습니다.


43-59: 구현이 적절합니다!

학과별 통계를 생성하는 로직이 다음과 같이 잘 구현되어 있습니다:

  • IntStream을 사용하여 순위를 효과적으로 관리
  • CalculationUtils를 활용한 비율 계산

88-93: 구현이 적절합니다!

폼 필드 통계를 생성하는 로직이 간단하고 명확하게 구현되어 있습니다.


95-99: 구현이 적절합니다!

필드 리스트 정보를 변환하는 로직이 간단하고 명확하게 구현되어 있습니다.


101-103: 구현이 적절합니다!

Integer를 int로 안전하게 변환하는 로직이 잘 구현되어 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)

35-35: 오타를 수정해야 합니다.

formStatisTicService 필드 이름에 오타가 있습니다. T가 대문자로 되어 있습니다.

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

-    private final FormStatisticService formStatisTicService;
+    private final FormStatisticService formStatisticService;

88-97: 구현이 적절합니다!

통계 데이터를 조회하는 로직이 다음과 같이 잘 구현되어 있습니다:

  • 사용자의 클럽과 폼을 조회
  • 총 지원자 수, 학과별 통계, 지원자 통계, 필드 통계를 순차적으로 조회
  • 최종 결과를 FormStatisticsQuery로 적절하게 변환
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormStatisticsResponse.java (5)

14-24: 구현이 적절합니다!

FormStatisticsResponse 레코드가 다음과 같이 잘 구현되어 있습니다:

  • Builder 패턴 사용
  • Swagger 어노테이션을 통한 명확한 API 문서화
  • 적절한 필드 구조화

26-46: 구현이 적절합니다!

DepartmentStatisticResponse 레코드가 다음과 같이 잘 구현되어 있습니다:

  • Builder 패턴 사용
  • Swagger 어노테이션을 통한 명확한 API 문서화
  • 적절한 매핑 메서드

48-72: 구현이 적절합니다!

ApplicantStatisticResponse 레코드가 다음과 같이 잘 구현되어 있습니다:

  • Builder 패턴 사용
  • Swagger 어노테이션을 통한 명확한 API 문서화
  • 중첩 레코드를 통한 데이터 그룹화
  • 적절한 매핑 메서드

74-108: 구현이 적절합니다!

FieldStatisticsResponse 레코드가 다음과 같이 잘 구현되어 있습니다:

  • Builder 패턴 사용
  • Swagger 어노테이션을 통한 명확한 API 문서화
  • 중첩 레코드를 통한 데이터 그룹화
  • 적절한 매핑 메서드

110-123: 구현이 적절합니다!

FormStatisticsResponse의 매핑 메서드가 다음과 같이 잘 구현되어 있습니다:

  • Stream API를 활용한 효율적인 데이터 변환
  • Builder 패턴을 통한 객체 생성
src/test/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImplTest.java (4)

55-90: 구현이 적절합니다!

총 지원자 수 조회 테스트가 다음과 같이 잘 구현되어 있습니다:

  • Given-When-Then 패턴 사용
  • 적절한 테스트 데이터 설정
  • 명확한 검증

92-164: 구현이 적절합니다!

학과별 순위 조회 테스트가 다음과 같이 잘 구현되어 있습니다:

  • Given-When-Then 패턴 사용
  • 다양한 케이스를 포함한 테스트 데이터
  • 상세한 검증

166-235: 구현이 적절합니다!

지원자 증감 비교 테스트가 다음과 같이 잘 구현되어 있습니다:

  • Given-When-Then 패턴 사용
  • 시간 기반 테스트 데이터
  • 증감율과 증감값 검증

260-275: 테스트 데이터를 개선해야 합니다.

FormAnswer 엔티티 생성 시 null 값을 사용하고 있습니다. 더 현실적인 테스트 데이터를 사용하는 것이 좋겠습니다.

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

-                .formApplication(null)
-                .value(null)
+                .formApplication(formApplication)
+                .value("답변 내용")
src/main/java/ddingdong/ddingdongBE/domain/form/repository/dto/FieldListInfo.java (1)

5-16: 인터페이스가 잘 설계되었습니다!

필드 정보와 답변 수를 효과적으로 조회할 수 있는 projection 인터페이스입니다. 메서드 명이 명확하고 타입이 적절하게 사용되었습니다.

Comment on lines +33 to +80
@DisplayName("원하는 필드 정보와 해당 필드의 답변 개수를 반환한다.")
@Test
void findFieldWithAnswerCountByFormId() {
// given
Form formFirst = fixture.giveMeBuilder(Form.class)
.set("id", 1L)
.set("club", null)
.set("startDate", LocalDate.of(2020, 2, 1))
.set("endDate", LocalDate.of(2020, 3, 1))
.set("sections", List.of("공통"))
.sample();
Form savedForm = formRepository.save(formFirst);
FormField formField = FormField.builder()
.question("설문 질문")
.required(true)
.fieldOrder(1)
.section("기본 정보")
.options(List.of("옵션1", "옵션2", "옵션3"))
.fieldType(FieldType.TEXT)
.form(savedForm)
.build();
FormField savedField = formFieldRepository.save(formField);

FormAnswer answer = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer2 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer3 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
formAnswerRepository.saveAll(List.of(answer, answer2, answer3));

// when
List<FieldListInfo> fieldListInfos = formFieldRepository.findFieldWithAnswerCountByFormId(
savedForm.getId());
// then
assertThat(fieldListInfos.size()).isEqualTo(1);
assertThat(fieldListInfos.get(0).getQuestion()).isEqualTo(formField.getQuestion());
assertThat(fieldListInfos.get(0).getCount()).isEqualTo(3);
}
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. 답변이 없는 필드의 경우 (count가 0인 경우)
  2. 여러 개의 필드가 있는 경우
  3. 존재하지 않는 formId를 조회하는 경우

또한 테스트 데이터의 개선이 필요합니다:

-                .value(null)
+                .value("테스트 답변")
📝 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 findFieldWithAnswerCountByFormId() {
// given
Form formFirst = fixture.giveMeBuilder(Form.class)
.set("id", 1L)
.set("club", null)
.set("startDate", LocalDate.of(2020, 2, 1))
.set("endDate", LocalDate.of(2020, 3, 1))
.set("sections", List.of("공통"))
.sample();
Form savedForm = formRepository.save(formFirst);
FormField formField = FormField.builder()
.question("설문 질문")
.required(true)
.fieldOrder(1)
.section("기본 정보")
.options(List.of("옵션1", "옵션2", "옵션3"))
.fieldType(FieldType.TEXT)
.form(savedForm)
.build();
FormField savedField = formFieldRepository.save(formField);
FormAnswer answer = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer2 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
FormAnswer answer3 = FormAnswer.builder()
.formApplication(null)
.value(null)
.formField(savedField)
.build();
formAnswerRepository.saveAll(List.of(answer, answer2, answer3));
// when
List<FieldListInfo> fieldListInfos = formFieldRepository.findFieldWithAnswerCountByFormId(
savedForm.getId());
// then
assertThat(fieldListInfos.size()).isEqualTo(1);
assertThat(fieldListInfos.get(0).getQuestion()).isEqualTo(formField.getQuestion());
assertThat(fieldListInfos.get(0).getCount()).isEqualTo(3);
}
@DisplayName("원하는 필드 정보와 해당 필드의 답변 개수를 반환한다.")
@Test
void findFieldWithAnswerCountByFormId() {
// given
Form formFirst = fixture.giveMeBuilder(Form.class)
.set("id", 1L)
.set("club", null)
.set("startDate", LocalDate.of(2020, 2, 1))
.set("endDate", LocalDate.of(2020, 3, 1))
.set("sections", List.of("공통"))
.sample();
Form savedForm = formRepository.save(formFirst);
FormField formField = FormField.builder()
.question("설문 질문")
.required(true)
.fieldOrder(1)
.section("기본 정보")
.options(List.of("옵션1", "옵션2", "옵션3"))
.fieldType(FieldType.TEXT)
.form(savedForm)
.build();
FormField savedField = formFieldRepository.save(formField);
FormAnswer answer = FormAnswer.builder()
.formApplication(null)
- .value(null)
+ .value("테스트 답변")
.formField(savedField)
.build();
FormAnswer answer2 = FormAnswer.builder()
.formApplication(null)
- .value(null)
+ .value("테스트 답변")
.formField(savedField)
.build();
FormAnswer answer3 = FormAnswer.builder()
.formApplication(null)
- .value(null)
+ .value("테스트 답변")
.formField(savedField)
.build();
formAnswerRepository.saveAll(List.of(answer, answer2, answer3));
// when
List<FieldListInfo> fieldListInfos = formFieldRepository.findFieldWithAnswerCountByFormId(
savedForm.getId());
// then
assertThat(fieldListInfos.size()).isEqualTo(1);
assertThat(fieldListInfos.get(0).getQuestion()).isEqualTo(formField.getQuestion());
assertThat(fieldListInfos.get(0).getCount()).isEqualTo(3);
}

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.

확인했습니다. 코멘트 확인 부탁드립니다~

@github-actions github-actions bot added D-0 and removed D-1 labels Feb 9, 2025
@KoSeonJe KoSeonJe merged commit cb52e2e into develop Feb 9, 2025
0 of 2 checks passed
@KoSeonJe KoSeonJe deleted the feature/DDING-97 branch February 9, 2025 12:56
@KoSeonJe KoSeonJe self-assigned this Feb 9, 2025
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