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

feat: 수강생 명단 페이지 대시보드 조회 API 추가 #796

Merged
merged 17 commits into from
Oct 8, 2024

Conversation

Sangwook02
Copy link
Member

@Sangwook02 Sangwook02 commented Oct 5, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

image

  • 다양한 테이블을 조회해야 해서 서비스 로직이 좀 복잡합니다.
  • 각 레포지토리에 멤버마다 쿼리를 날리면 쿼리 수가 너무 많아져서 쿼리 나가는 수를 줄이기 위해, 조회하려는 멤버의 pk를 담은 List를 repository에 넘기는 방식을 사용했습니다.
  • 이렇게 가져온 것들을 stream.filter로 찾아서 활용합니다.
  • 기존 StudyTodoResponse가 가진 필드들이 이번 api에서 필요한 것과 겹쳐서 재활용했습니다. rename은 필요할 것 같아서 todo 남겨뒀고 이슈도(♻️ StudyTodoResponse 이름 변경 #795) 생성해뒀습니다.

📝 참고사항

📚 기타

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 수강생의 과제 제출 비율 및 출석률을 계산하는 기능이 추가되었습니다.
    • 출석 및 과제 상태에 '취소됨' 상태가 추가되었습니다.
  • 변경 사항

    • StudyStudentResponse 구조가 개선되어, 학생의 성취도 및 과제 제출 상태를 보다 효과적으로 관리할 수 있게 되었습니다.
    • StudyTodoResponse에서 출석 및 과제 유형의 취소 상태를 처리하는 로직이 개선되었습니다.
    • 수강생 응답 구조에 새로운 필드가 추가되어, 우수 학생 여부 및 과제와 출석률을 포함한 정보를 제공할 수 있게 되었습니다.
    • StudyStudentResponse의 메서드가 페이지네이션을 지원하도록 업데이트되었습니다.
    • StudyAchievement 엔티티를 특정 기준으로 조회하는 기능이 추가되었습니다.

@Sangwook02 Sangwook02 added the ✨ feature 새로운 기능 추가 및 수정 label Oct 5, 2024
@Sangwook02 Sangwook02 self-assigned this Oct 5, 2024
@Sangwook02 Sangwook02 requested a review from a team as a code owner October 5, 2024 03:34
Copy link

coderabbitai bot commented Oct 5, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

MentorStudyService 클래스의 getStudyStudents 메서드가 수정되어 List<StudyStudentResponse> 대신 Page<StudyStudentResponse>를 반환하도록 변경되었습니다. 이 메서드는 StudyHistory 항목을 페이지 방식으로 검색하고, 학생 관련 정보를 종합하기 위해 새로운 리포지토리에서 데이터를 가져옵니다. StudyStudentResponse 레코드의 생성자가 새로운 매개변수를 포함하도록 업데이트되었으며, 관련 로직이 추가되었습니다. AssignmentSubmissionStatusResponse 열거형에 CANCELLED 상수가 추가되었고, StudyTodoResponse 클래스는 취소 상태를 처리하는 로직이 강화되었습니다.

Changes

파일 경로 변경 요약
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java getStudyStudents 메서드 변경: 반환 타입을 Page<StudyStudentResponse>로 수정, 새로운 리포지토리에서 데이터 검색.
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java 생성자에 새로운 매개변수 추가, from 메서드를 of로 변경, 과제 및 출석 계산 로직 추가.
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmissionStatusResponse.java CANCELLED 상수 추가, from 메서드를 of로 변경, 로직 수정.
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentCurriculumResponse.java AssignmentSubmissionStatusResponse 인스턴스 생성 메서드 수정.
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java 출석 및 과제 유형의 취소 상태 처리 로직 추가.
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java StudyAchievementCustomRepository 구현: findByStudyIdAndMemberIds 메서드 추가, 데이터베이스 쿼리 처리.

Assessment against linked issues

Objective Addressed Explanation
수강생 조회 API 추가 (#789)

Possibly related PRs

Suggested reviewers

  • AlmondBreez3
  • seulgi99
  • uwoobeat

🐇 변화의 길을 뛰어넘어,
수강생 관리가 더욱 빛나네.
페이지로 나누어, 모두를 담고,
학습의 여정, 함께 걸어가자!
🌼✨


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

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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

github-actions bot commented Oct 5, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

github-actions bot commented Oct 5, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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: 10

🧹 Outside diff range and nitpick comments (10)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java (3)

6-6: 인터페이스 선언이 적절합니다. 문서화를 고려해 보세요.

StudyAchievementCustomRepository 인터페이스 이름이 Java 명명 규칙을 잘 따르고 있습니다. 'Custom' 접미사는 이것이 Spring Data JPA 커스텀 리포지토리 패턴의 일부임을 나타냅니다.

인터페이스의 목적과 사용 방법을 설명하는 Javadoc 주석을 추가하는 것이 좋습니다. 예를 들어:

/**
 * 커스텀 StudyAchievement 쿼리를 위한 리포지토리 인터페이스입니다.
 * 이 인터페이스는 Spring Data JPA에서 제공하지 않는 복잡한 쿼리를 구현하는 데 사용됩니다.
 */
public interface StudyAchievementCustomRepository {
    // 메서드 선언...
}

7-7: 메서드 선언이 적절합니다. 문서화를 추가해 주세요.

findByStudyIdAndMemberIds 메서드의 이름, 매개변수, 반환 타입이 모두 적절하게 선언되었습니다. 이 메서드는 PR의 목표인 쿼리 최적화를 위해 기본 키 목록을 전달하는 방식과 일치합니다.

메서드에 대한 Javadoc 주석을 추가하여 사용 방법과 목적을 명확히 설명하는 것이 좋습니다. 예를 들어:

/**
 * 주어진 스터디 ID와 회원 ID 목록에 해당하는 StudyAchievement 목록을 조회합니다.
 *
 * @param studyId 조회할 스터디의 ID
 * @param memberIds 조회할 회원들의 ID 목록
 * @return 조건에 맞는 StudyAchievement 객체들의 목록
 */
List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds);

1-8: 전반적인 구현이 PR 목표와 잘 일치합니다.

이 커스텀 리포지토리 인터페이스는 PR의 주요 목표인 쿼리 최적화와 데이터베이스 호출 횟수 감소를 잘 반영하고 있습니다. findByStudyIdAndMemberIds 메서드는 여러 ID를 한 번에 전달함으로써 효율적인 쿼리를 가능하게 합니다.

구현은 Spring Data JPA의 커스텀 리포지토리에 대한 모범 사례를 따르고 있어 좋습니다. 이는 복잡한 서비스 로직을 처리하는 데 도움이 될 것입니다.

이 인터페이스의 구현 클래스에서는 성능 최적화를 위해 다음 사항들을 고려해 보시기 바랍니다:

  1. 대량의 ID를 처리할 때 배치 처리 기법 사용
  2. 필요한 경우 인덱스 활용
  3. N+1 문제를 피하기 위한 적절한 조인 전략 사용
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementRepository.java (1)

1-7: 코드가 잘 작성되었습니다.

이 인터페이스는 Spring Data JPA의 모범 사례를 따르고 있으며, 기본 CRUD 작업과 함께 사용자 정의 기능을 제공할 수 있도록 잘 설계되었습니다.

인터페이스의 목적과 사용자 정의 메서드(있는 경우)에 대한 간단한 JavaDoc 주석을 추가하는 것이 좋습니다. 이는 코드의 가독성과 유지 보수성을 향상시킬 수 있습니다.

예를 들어:

/**
 * StudyAchievement 엔티티에 대한 리포지토리 인터페이스입니다.
 * 기본 CRUD 작업과 함께 StudyAchievementCustomRepository에 정의된 사용자 정의 쿼리 메서드를 제공합니다.
 */
public interface StudyAchievementRepository
        extends JpaRepository<StudyAchievement, Long>, StudyAchievementCustomRepository {
    // 여기에 추가 메서드 선언 (필요한 경우)
}
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AttendanceStatusResponse.java (2)

14-15: 열거형 상수 변경 승인 및 명명 규칙 제안

변경사항이 적절하게 구현되었습니다. CANCELLED 상수의 추가는 PR 목표와 일치하며, 휴강 상태를 나타내는 데 유용할 것입니다.

다만, 한 가지 제안사항이 있습니다:

Java의 일반적인 명명 규칙에 따라, 열거형 상수의 이름을 더 명확하게 만들 수 있습니다. 예를 들어:

-    CANCELLED("휴강");
+    CLASS_CANCELLED("휴강");

이렇게 하면 상수의 의미가 더 명확해지고, 코드의 가독성이 향상될 수 있습니다.


20-23: of 메서드 변경 승인 및 최적화 제안

of 메서드의 변경사항이 PR 목표와 잘 일치합니다. 휴강 상태를 먼저 확인하는 것은 논리적이며, 모든 가능한 수업 상태를 적절히 처리하고 있습니다.

다음과 같은 최적화를 고려해 보시기 바랍니다:

 public static AttendanceStatusResponse of(StudyDetail studyDetail, LocalDate now, boolean isAttended) {
     if (studyDetail.getCurriculum().isCancelled()) {
         return CANCELLED;
     }

-    if (studyDetail.getAttendanceDay().isAfter(now)) {
-        return BEFORE_ATTENDANCE;
-    }
-
-    if (isAttended) {
-        return ATTENDED;
-    } else {
-        return NOT_ATTENDED;
-    }
+    return studyDetail.getAttendanceDay().isAfter(now) ? BEFORE_ATTENDANCE
+           : isAttended ? ATTENDED : NOT_ATTENDED;
 }

이 변경은 코드를 더 간결하게 만들고 가독성을 향상시킬 수 있습니다. 삼항 연산자를 사용하여 여러 조건을 한 줄로 처리함으로써 코드의 복잡성을 줄일 수 있습니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Curriculum.java (1)

64-66: 새로운 isCancelled() 메서드가 잘 구현되었습니다.

isCancelled() 메서드의 구현이 적절하며 기존 코드 스타일과 일관성이 있습니다. 이 메서드는 커리큘럼의 취소 상태를 확인하는 데 유용할 것입니다.

일관성을 위해 isOpen() 메서드와 isCancelled() 메서드 사이에 빈 줄을 추가하는 것이 좋겠습니다. 다음과 같이 수정해 보세요:

     public boolean isOpen() {
         return status == StudyStatus.OPEN;
     }
+
     public boolean isCancelled() {
         return status == StudyStatus.CANCELLED;
     }
src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceCustomRepositoryImpl.java (1)

28-36: 새로운 메서드 구현이 전반적으로 잘 되었습니다.

새로운 findByStudyIdAndMemberIds 메서드의 구현이 PR의 목적에 잘 부합하며, 대시보드 API의 성능 향상에 도움이 될 것 같습니다. 다만, 몇 가지 제안사항이 있습니다:

  1. innerJoin을 사용한 이유가 있나요? 기존 메서드에서는 leftJoin을 사용했는데, 이 변경으로 인해 일부 데이터가 제외될 수 있습니다. 의도된 변경이라면 그 이유를 주석으로 설명해주시면 좋겠습니다.

  2. 메서드의 목적과 사용 사례를 설명하는 주석을 추가하는 것이 좋겠습니다. 이는 코드의 가독성과 유지보수성을 향상시킬 것입니다.

다음과 같이 주석을 추가하는 것은 어떨까요?

+    /**
+     * 주어진 스터디 ID와 회원 ID 목록에 해당하는 출석 기록을 조회합니다.
+     * 이 메서드는 대시보드 API에서 사용되며, 다수의 회원에 대한 출석 정보를 효율적으로 조회합니다.
+     *
+     * @param studyId 조회할 스터디의 ID
+     * @param memberIds 조회할 회원들의 ID 목록
+     * @return 조회된 출석 기록 목록
+     */
     @Override
     public List<Attendance> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
         // 메서드 구현...
     }
src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (1)

40-43: 페이지네이션 구현 및 메서드 설명 개선 제안

페이지네이션 구현은 PR의 목표와 일치하며 성능 최적화에 도움이 될 것입니다. 다만, 메서드 설명을 더 구체적으로 개선할 것을 제안합니다.

메서드 설명을 다음과 같이 변경하는 것이 어떨까요?

- @Operation(summary = "스터디 수강생 관리", description = "해당 스터디의 수강생을 관리합니다")
+ @Operation(summary = "스터디 수강생 목록 조회", description = "해당 스터디의 수강생 목록을 페이지네이션하여 조회합니다")

이렇게 하면 메서드의 실제 기능을 더 정확하게 설명할 수 있습니다.

src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (1)

14-14: 'StudyTodoResponse' 클래스명 변경에 대한 도움 제공

주석에서 '활용이 다양해졌으므로 rename 필요'라고 표시되어 있습니다. 클래스의 역할이 확대된 만큼 더 적합한 이름으로 변경하는 것이 좋을 것 같습니다. 리네이밍에 대한 제안을 드리거나, 새로운 GitHub 이슈를 생성해드릴까요?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 76d2a19 and 5fa5e05.

📒 Files selected for processing (17)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (4 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryQueryMethod.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceCustomRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceCustomRepositoryImpl.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Curriculum.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmissionStatusResponse.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AttendanceStatusResponse.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryRepository.java
🔇 Additional comments (20)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java (1)

1-5: 패키지 선언과 임포트가 적절합니다.

패키지 이름과 임포트 문이 Java 명명 규칙을 따르고 있으며, 인터페이스의 요구사항에 맞게 적절히 선택되었습니다.

src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceCustomRepository.java (1)

10-11: LGTM! PR 목표와 잘 일치합니다.

새로운 메서드 findByStudyIdAndMemberIds는 PR의 목표와 잘 일치합니다. 이 메서드는 다음과 같은 이점을 제공합니다:

  1. 여러 회원의 출석 기록을 한 번에 조회할 수 있어 성능 최적화에 도움이 됩니다.
  2. 복잡한 서비스 로직을 지원하는 유연한 쿼리를 가능하게 합니다.
  3. 메서드 시그니처가 명확하고 Java 명명 규칙을 잘 따르고 있습니다.

이 변경사항은 학생 명단 대시보드 조회 API 구현에 필요한 기능을 제공하며, 성능 최적화 목표도 달성할 수 있을 것으로 보입니다.

src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java (1)

14-15: LGTM! 새로운 메서드가 PR 목표와 잘 일치합니다.

새로 추가된 findByStudyIdAndMemberIds 메서드는 수강생 명단 페이지 대시보드 조회 API를 위한 좋은 추가사항으로 보입니다. 이 메서드는 여러 회원의 과제 이력을 한 번에 조회할 수 있게 해주어, PR 요약에서 언급된 대로 쿼리 수를 줄이고 성능을 최적화하는 데 도움이 될 것 같습니다.

src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (2)

8-9: 페이지네이션 지원을 위한 적절한 임포트 추가

PagePageable 클래스의 임포트는 새로운 페이지네이션 기능을 지원하기 위해 필요합니다. 이는 PR의 목표인 쿼리 최적화와 일치합니다.


22-22: 페이지네이션을 지원하는 새로운 메서드 추가

findByStudyId 메서드에 페이지네이션 기능을 추가한 것은 좋은 개선입니다. 이는 대량의 데이터를 효율적으로 처리할 수 있게 해주며, PR의 목표인 쿼리 최적화와 일치합니다.

다음 스크립트를 실행하여 이 메서드의 사용을 확인해 주세요:

src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AttendanceStatusResponse.java (1)

Line range hint 1-35: 전체 리뷰 요약

이 PR의 변경사항은 요구사항을 잘 충족하고 있습니다. CANCELLED 상태의 추가와 of 메서드의 수정은 휴강 상황을 적절히 처리할 수 있게 해줍니다.

주요 개선 사항:

  1. 열거형 상수의 명명 규칙 개선 (CANCELLEDCLASS_CANCELLED)
  2. of 메서드의 로직 최적화

이러한 minor한 개선사항들을 적용하면 코드의 품질과 가독성이 더욱 향상될 것입니다.

전반적으로 잘 구현된 변경사항입니다. 수고하셨습니다! 👍

src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java (3)

1-9: 패키지 선언과 임포트가 적절합니다.

패키지 선언과 임포트가 잘 구성되어 있습니다. QueryDSL을 위한 정적 임포트와 필요한 클래스들이 올바르게 임포트되어 있습니다.


24-26: 헬퍼 메서드가 잘 설계되었습니다.

eqStudyId 메서드는 잘 설계되었습니다. studyId가 null인 경우를 적절히 처리하여 where 절에서 이 조건을 선택적으로 사용할 수 있게 합니다. 이는 QueryDSL에서 권장되는 좋은 방식입니다.

이 접근 방식은 코드의 재사용성을 높이고, 동적 쿼리 생성을 용이하게 합니다.


11-14: 클래스 선언과 의존성 주입이 적절합니다.

클래스 구조가 잘 설계되어 있습니다. @RequiredArgsConstructor를 사용한 생성자 주입은 좋은 방식입니다.

StudyAchievementCustomRepository 인터페이스가 실제로 존재하는지 확인해 주세요. 다음 스크립트를 실행하여 인터페이스의 존재를 확인할 수 있습니다:

✅ Verification successful

인터페이스 존재 확인 완료

StudyAchievementCustomRepository 인터페이스가 존재함을 확인했습니다. 클래스 선언 및 의존성 주입은 적절합니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of StudyAchievementCustomRepository interface

# Test: Search for the StudyAchievementCustomRepository interface definition
rg --type java "interface StudyAchievementCustomRepository"

Length of output: 202

src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceCustomRepositoryImpl.java (1)

Line range hint 1-54: 전체적인 변경사항에 대한 의견

이 PR에서 추가된 findByStudyIdAndMemberIds 메서드는 수강생 명단 페이지 대시보드 조회 API의 요구사항을 잘 충족시키고 있습니다. 다음과 같은 장점이 있습니다:

  1. 여러 회원의 출석 정보를 한 번의 쿼리로 조회할 수 있어 성능이 향상될 것으로 예상됩니다.
  2. in 절을 사용하여 여러 회원 ID를 효율적으로 필터링하고 있습니다.
  3. fetchJoin을 사용하여 연관 엔티티를 함께 조회함으로써 N+1 문제를 방지하고 있습니다.

다만, innerJoin의 사용과 관련하여 추가 설명이 필요해 보입니다. 또한, 메서드의 목적을 설명하는 주석을 추가하면 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.

전반적으로, 이 변경사항은 PR의 목표를 잘 달성하고 있으며, 제안된 개선사항을 반영하면 더욱 완성도 높은 코드가 될 것 같습니다.

src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2)

13-14: 페이지네이션 구현을 위한 적절한 임포트 추가

PagePageable 클래스의 임포트는 getStudyStudents 메서드에 페이지네이션 기능을 추가하기 위해 필요합니다. 이는 PR의 목표와 일치하며 대규모 데이터 처리에 도움이 될 것입니다.


40-43: 서비스 및 리포지토리 계층 변경 확인 제안

컨트롤러의 변경사항이 서비스 및 리포지토리 계층에도 적절히 반영되었는지 확인이 필요합니다. 특히 PR 목표에서 언급된 복잡한 쿼리 최적화와 관련하여 중요합니다.

다음 스크립트를 실행하여 관련 변경사항을 확인할 수 있습니다:

서비스 또는 리포지토리 계층에 추가적인 변경이 필요한 경우, 구현을 도와드릴 수 있습니다. 도움이 필요하신가요?

src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryQueryMethod.java (4)

12-14: eqMember 메서드 구현이 적절합니다

membernull인지 확인하고 적절한 BooleanExpression을 반환하고 있습니다.


16-18: eqStudy 메서드 구현이 적절합니다

studynull인지 확인하고 적절한 BooleanExpression을 반환하고 있습니다.


20-22: isSubmitted 메서드 구현이 적절합니다

submissionStatusFAILURE 또는 SUCCESS 상태인지를 확인하고 있습니다.


28-30: eqMemberId 메서드 구현이 적절합니다

memberIdnull이 아닌 경우 적절한 BooleanExpression을 반환하고 있습니다.

src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java (1)

14-15: AssignmentHistoryQueryMethod 인터페이스 구현 추가 확인

클래스가 AssignmentHistoryQueryMethod 인터페이스를 추가로 구현하도록 수정되었습니다. 이는 새로운 메서드 구현을 위한 적절한 변경으로 보입니다.

src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (2)

25-34: 취소된 커리큘럼에 대한 처리 로직이 적절하게 추가되었습니다

createAttendanceType 메서드에서 커리큘럼이 취소된 경우를 확인하고, 해당 경우에 맞게 StudyTodoResponse를 반환하도록 수정하신 점이 정확합니다. 로직이 올바르게 구현되었습니다.


46-55: 취소된 과제에 대한 처리 로직이 적절하게 추가되었습니다

createAssignmentType 메서드에서 과제가 취소된 경우를 확인하고, AssignmentSubmissionStatusResponse.CANCELLED 상태로 응답하도록 수정하신 점이 정확합니다. 로직이 올바르게 구현되었습니다.

src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)

117-120: isAttended 메소드의 정확한 구현

isAttended 메소드는 참석 여부를 정확하게 판단하고 있습니다. 구현 방식에 문제가 없습니다.

Comment on lines 16 to 22
@Override
public List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
return queryFactory
.selectFrom(studyAchievement)
.where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId))
.fetch();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

쿼리 메서드가 효율적으로 구현되었습니다만, 개선의 여지가 있습니다.

findByStudyIdAndMemberIds 메서드가 QueryDSL을 사용하여 효율적으로 구현되었습니다. in 절을 사용하여 여러 회원 ID를 한 번에 처리하는 것이 좋습니다.

하지만 memberIds 파라미터에 대한 null 체크가 없습니다. 빈 리스트가 전달될 경우 문제가 발생할 수 있습니다.

다음과 같이 memberIds에 대한 null 체크를 추가하는 것이 좋겠습니다:

 @Override
 public List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
+    if (memberIds == null || memberIds.isEmpty()) {
+        return List.of();
+    }
     return queryFactory
             .selectFrom(studyAchievement)
             .where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId))
             .fetch();
 }
📝 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 List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
return queryFactory
.selectFrom(studyAchievement)
.where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId))
.fetch();
}
@Override
public List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
if (memberIds == null || memberIds.isEmpty()) {
return List.of();
}
return queryFactory
.selectFrom(studyAchievement)
.where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId))
.fetch();
}

Comment on lines +48 to +55
@Override
public List<AssignmentHistory> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
return queryFactory
.selectFrom(assignmentHistory)
.innerJoin(assignmentHistory.studyDetail, studyDetail)
.fetchJoin()
.where(assignmentHistory.member.id.in(memberIds), eqStudyId(studyId))
.fetch();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

memberIds에 대한 null 또는 빈 리스트 처리 필요

현재 메서드 findByStudyIdAndMemberIds에서 memberIdsnull이거나 빈 리스트일 경우, 쿼리에서 예기치 않은 결과가 발생할 수 있습니다. memberIds에 대한 null 및 빈 리스트 체크를 추가하여 예외를 방지하고 적절한 결과를 반환하도록 수정하는 것이 좋습니다.

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

if (memberIds == null || memberIds.isEmpty()) {
    return Collections.emptyList();
}

Copy link

github-actions bot commented Oct 5, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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: 0

🧹 Outside diff range and nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (3)

19-24: 새 필드 추가 승인 및 명명 규칙 개선 제안

새로운 필드들이 학생 대시보드 API의 요구사항을 잘 반영하고 있습니다. 학생의 성과와 상태에 대한 포괄적인 정보를 제공합니다.

다만, 불리언 필드의 이름에 대해 작은 개선을 제안합니다:

- @Schema(description = "1차 우수 스터디원") boolean isFirstRoundOutstandingStudent,
- @Schema(description = "2차 우수 스터디원") boolean isSecondRoundOutstandingStudent,
+ @Schema(description = "1차 우수 스터디원") boolean firstRoundOutstandingStudent,
+ @Schema(description = "2차 우수 스터디원") boolean secondRoundOutstandingStudent,

Java에서는 불리언 필드의 이름에 'is' 접두사를 사용하지 않는 것이 관례입니다. 게터 메서드가 자동으로 'is' 접두사를 추가하기 때문입니다.


25-53: 메서드 변경 승인 및 개선 제안

of 메서드로의 변경과 추가 매개변수는 PR의 목표를 잘 반영하고 있습니다. 하지만 몇 가지 개선 사항을 제안합니다:

  1. 과제 및 출석 비율 계산 로직을 별도의 메서드로 추출하여 가독성을 높이세요.
  2. 0으로 나누는 오류를 방지하기 위해 안전 장치를 추가하세요.

예시 리팩토링:

private static double calculateRate(long successCount, long totalCount, long cancelledCount) {
    long validCount = totalCount - cancelledCount;
    return validCount > 0 ? (double) successCount * 100 / validCount : 0.0;
}

// 메서드 내에서 사용
double assignmentRate = calculateRate(successAssignmentsCount, assignments.size(), cancelledAssignmentsCount);
double attendanceRate = calculateRate(attendedCount, attendances.size(), cancelledAttendanceCount);

이렇게 하면 코드의 가독성이 향상되고 0으로 나누는 오류를 방지할 수 있습니다.


68-71: 출석 상태별 카운트 메서드 승인 및 리팩토링 제안

countAttendanceByStatus 메서드는 출석 상태별 카운트를 효과적으로 수행합니다. 그러나 countAssignmentByStatus 메서드와 유사한 구조를 가지고 있어, 두 메서드를 하나로 통합할 수 있는 기회가 있습니다.

다음과 같은 리팩토링을 고려해 보세요:

private static <T> long countByStatus(List<StudyTodoResponse> todos, T status, Function<StudyTodoResponse, T> statusExtractor) {
    return todos.stream()
            .filter(todo -> statusExtractor.apply(todo) == status)
            .count();
}

// 사용 예시
long successAssignmentsCount = countByStatus(assignments, SUCCESS, StudyTodoResponse::assignmentSubmissionStatus);
long attendedCount = countByStatus(attendances, AttendanceStatusResponse.ATTENDED, StudyTodoResponse::attendanceStatus);

이렇게 하면 코드 중복을 줄이고 더 유연한 구조를 만들 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5fa5e05 and 223286d.

📒 Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (1 hunks)
🔇 Additional comments (4)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (4)

3-11: LGTM: 임포트 문 변경 승인

새로운 정적 임포트와 StudyAchievement 클래스의 임포트가 추가되었습니다. 이러한 변경은 새로운 기능을 지원하고 코드의 가독성을 향상시킵니다.


55-59: 우수 학생 확인 메서드 승인

isOutstandingStudent 메서드는 간결하고 효과적으로 우수 학생을 식별합니다. 스트림 API를 사용하여 가독성 있게 구현되었습니다.


61-66: 과제 상태별 카운트 메서드 승인

countAssignmentByStatus 메서드는 과제 상태별 카운트를 효율적으로 수행합니다. 스트림 API를 사용하여 간결하게 구현되었으며, 코드의 재사용성을 높입니다.


1-72: 전체 파일 리뷰 요약

이 PR은 학생 대시보드 API 추가라는 목표를 잘 달성하고 있습니다. 주요 변경 사항은 다음과 같습니다:

  1. 새로운 필드 추가로 학생 정보를 더 포괄적으로 표현
  2. of 메서드를 통한 데이터 처리 로직 구현
  3. 우수 학생, 과제 및 출석 상태 확인을 위한 헬퍼 메서드 추가

전반적으로 코드는 잘 구조화되어 있고 좋은 관행을 따르고 있습니다. 다만, 다음과 같은 개선 사항을 고려해 보시기 바랍니다:

  1. 불리언 필드 이름에서 'is' 접두사 제거
  2. 비율 계산 로직을 별도 메서드로 추출
  3. countAssignmentByStatuscountAttendanceByStatus 메서드 통합

이러한 개선을 통해 코드의 가독성과 유지보수성을 더욱 높일 수 있을 것입니다.

Copy link

github-actions bot commented Oct 6, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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

🧹 Outside diff range and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmissionStatusResponse.java (1)

18-21: 메서드 이름 변경 및 로직 업데이트 승인

from에서 of로의 메서드 이름 변경은 Java의 정적 팩토리 메서드 명명 규칙을 따르는 좋은 변경입니다. 새로운 로직은 취소된 과제를 적절히 처리하고 있습니다.

하나의 개선 제안이 있습니다:

studyDetail이 null일 경우를 처리하는 것이 좋겠습니다. 다음과 같이 수정을 고려해보세요:

public static AssignmentSubmissionStatusResponse of(AssignmentHistory assignmentHistory, StudyDetail studyDetail) {
+   if (studyDetail == null || studyDetail.getAssignment() == null) {
+       throw new IllegalArgumentException("StudyDetail or its assignment cannot be null");
+   }
    if (studyDetail.getAssignment().isCancelled()) {
        return CANCELLED;
    }
    // ... 나머지 코드는 그대로
}

이렇게 하면 메서드의 견고성이 향상되고 잠재적인 NullPointerException을 방지할 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 223286d and b60e067.

📒 Files selected for processing (3)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmissionStatusResponse.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentCurriculumResponse.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java
🧰 Additional context used
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmissionStatusResponse.java (2)

4-4: 새로운 import 추가 승인

StudyDetail import 추가는 메서드 시그니처 변경에 따른 적절한 수정입니다.


13-14: 새로운 CANCELLED 상태 추가 및 SUCCESS 수정 승인

CANCELLED 상태 추가는 PR 목표에 부합하며, 시스템에 더 많은 유연성을 제공합니다. SUCCESS 뒤의 쉼표 추가는 일관성을 위한 적절한 스타일 변경입니다.

Copy link

github-actions bot commented Oct 6, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

github-actions bot commented Oct 6, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

github-actions bot commented Oct 6, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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

🧹 Outside diff range and nitpick comments (6)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (3)

19-24: LGTM: 레코드 선언이 적절히 확장되었습니다.

새로운 필드들이 학생의 성과와 상태에 대한 추가 정보를 제공합니다. Swagger 어노테이션을 사용하여 API 문서화가 개선되었습니다.

개선 제안:

  • assignmentRateattendanceRate의 설명에 백분율임을 명시하는 것이 좋겠습니다. 예: "과제 수행률 (%)"

25-57: LGTM: of 메서드가 적절히 구현되었습니다. 몇 가지 개선 사항을 제안합니다.

새로운 of 메서드는 학생에 대한 더 포괄적인 정보를 제공합니다. 과제 및 출석률 계산 로직이 올바르게 구현되었습니다.

개선 제안:

  1. 과제 및 출석 필터링 로직을 별도의 메서드로 추출하여 코드 가독성을 높일 수 있습니다.
  2. 비율 계산 로직을 별도의 메서드로 추출하여 재사용성을 높일 수 있습니다.
  3. 매직 넘버 100을 상수로 추출하는 것이 좋습니다 (예: private static final double PERCENTAGE_MULTIPLIER = 100.0;).

72-75: LGTM: countAttendanceByStatus 메서드가 잘 구현되었습니다. 리팩토링 제안이 있습니다.

이 메서드는 출석 상태별 카운팅 로직을 잘 추상화했습니다. Stream API를 효과적으로 사용하여 구현되었습니다.

리팩토링 제안:
countAssignmentByStatuscountAttendanceByStatus 메서드는 매우 유사한 구조를 가지고 있습니다. 제네릭을 사용하여 이 두 메서드를 하나의 메서드로 통합할 수 있을 것 같습니다. 예를 들어:

private static <T, S> long countByStatus(List<T> items, S status, Function<T, S> statusExtractor) {
    return items.stream()
            .filter(item -> statusExtractor.apply(item) == status)
            .count();
}

이렇게 하면 코드 중복을 줄이고 더 유연한 구조를 만들 수 있습니다.

src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3)

45-46: 필드 선언 개선 승인 및 제안

중복 선언된 StudyValidatorStudyDetailValidator가 제거되어 코드 가독성이 향상되었습니다. 새로운 리포지토리 필드 추가는 PR 목적에 부합합니다.

코드의 일관성을 위해 새로 추가된 리포지토리 필드들을 알파벳 순으로 정렬하는 것이 좋겠습니다.

Also applies to: 51-53


68-79: 데이터 조회 로직 개선 승인 및 제안

새로운 데이터 조회 로직이 PR 목적에 맞게 잘 구현되었습니다. 다양한 테이블을 쿼리하고 성능을 최적화하려는 노력이 보입니다.

다음 개선 사항을 고려해 보세요:

  1. studyDetailRepository.findAllByStudyId(studyId)의 결과가 비어있을 경우에 대한 처리를 추가하세요.
  2. studentIds가 비어있을 경우, 불필요한 데이터베이스 쿼리를 방지하기 위한 early return을 고려해 보세요.
  3. 각 리포지토리 메서드 호출에 대한 예외 처리를 추가하여 더 강건한 코드를 만들어 보세요.

80-112: 데이터 처리 로직 구현 승인 및 성능 개선 제안

다양한 소스에서 데이터를 처리하고 상호 연관시키는 복잡한 로직이 PR 목적에 맞게 잘 구현되었습니다.

성능 개선을 위해 다음 사항을 고려해 보세요:

  1. 반복적인 stream().filter() 연산 대신 Map을 사용하여 데이터를 그룹화하고 조회하는 방식으로 변경하세요. 예를 들어:
    Map<Long, List<StudyAchievement>> achievementMap = studyAchievements.stream()
        .collect(Collectors.groupingBy(sa -> sa.getStudent().getId()));
    이렇게 하면 각 학생 ID에 대한 데이터 조회가 O(1) 시간에 가능해집니다.
  2. studyDetails를 Map으로 변환하여 반복적인 조회를 최적화하세요.
  3. StudyTodoResponse 생성 로직을 별도의 메서드로 추출하여 코드 가독성을 높이세요.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b60e067 and 991d43b.

📒 Files selected for processing (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (4 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (1 hunks)
🧰 Additional context used
🔇 Additional comments (7)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (4)

3-11: LGTM: 임포트 문이 적절히 업데이트되었습니다.

새로운 기능을 위해 필요한 클래스들이 올바르게 임포트되었습니다. 정적 임포트를 사용하여 코드 가독성이 향상되었습니다.


59-63: LGTM: isOutstandingStudent 메서드가 잘 구현되었습니다.

이 메서드는 학생의 우수성을 판단하는 로직을 잘 추상화했습니다. Stream API를 효과적으로 사용하여 간결하게 구현되었습니다.


65-70: LGTM: countAssignmentByStatus 메서드가 잘 구현되었습니다.

이 메서드는 과제 상태별 카운팅 로직을 잘 추상화했습니다. Stream API를 효과적으로 사용하여 과제를 필터링하고 카운트하고 있어, 코드의 가독성이 향상되었습니다.


1-75: 전반적인 변경 사항이 PR 목표와 잘 부합합니다.

이 파일의 변경 사항은 학생 목록 대시보드 조회 API 추가라는 PR의 주요 목적을 잘 달성하고 있습니다. 주요 개선 사항은 다음과 같습니다:

  1. 복잡한 서비스 로직을 효과적으로 구현하여 다양한 테이블의 데이터를 종합하고 있습니다.
  2. StudyTodoResponse 클래스의 필드를 재사용하여 일관성을 유지하고 있습니다.
  3. 성능 최적화를 위해 스트림 API를 효과적으로 활용하고 있습니다.

전체적으로 코드는 잘 구조화되어 있으며 현대적인 Java 기능을 효과적으로 사용하고 있습니다. 제안된 리팩토링을 적용하면 코드의 품질과 유지보수성이 더욱 향상될 것입니다.

src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3)

6-8: 새로운 의존성 추가 승인

새로운 리포지토리와 페이지네이션 관련 클래스의 import 문이 적절히 추가되었습니다. 이는 PR의 목적인 학생 목록 대시보드 조회 API 구현과 일치합니다.

Also applies to: 23-23, 26-27, 33-35


63-63: 메서드 시그니처 변경 승인

getStudyStudents 메서드의 반환 타입이 List<StudyStudentResponse>에서 Page<StudyStudentResponse>로 변경되고 Pageable 매개변수가 추가된 것은 페이지네이션 구현을 위한 적절한 변경입니다. 이는 PR의 목적인 학생 목록 대시보드 조회 API 구현과 일치합니다.


115-118: isAttended 헬퍼 메서드 승인

isAttended 메서드가 간결하고 효과적으로 구현되었습니다. 스트림 연산을 적절히 사용하여 출석 여부를 확인하고 있습니다.

Copy link
Member

@kckc0608 kckc0608 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 진짜 로직이 복잡하네요 ㅋㅋ

Comment on lines 70 to 78
List<Long> studentIds = studyHistories.getContent().stream()
.map(studyHistory -> studyHistory.getStudent().getId())
.toList();

List<StudyAchievement> studyAchievements =
studyAchievementRepository.findByStudyIdAndMemberIds(studyId, studentIds);
List<Attendance> attendances = attendanceRepository.findByStudyIdAndMemberIds(studyId, studentIds);
List<AssignmentHistory> assignmentHistories =
assignmentHistoryRepository.findByStudyIdAndMemberIds(studyId, studentIds);
Copy link
Member

@kckc0608 kckc0608 Oct 6, 2024

Choose a reason for hiding this comment

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

스터디를 듣는 모든 수강생의 정보를 봐야 한다면 학생의 id 를 쿼리 조건에 걸지 않고, 스터디 id 기준으로 모든 학생의 정보를 조회해도 괜찮을 것 같은데, 학생 아이디 조건을 함께 걸으신 이유가 궁금해요!

Copy link
Member Author

Choose a reason for hiding this comment

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

Page<StudyHistory> studyHistories = studyHistoryRepository.findByStudyId(studyId, pageable);로 가져온 수강생들의 정보만 dto에 넣을거라서 이렇게 했어요

Comment on lines +11 to 12
// todo: public 제거
public Optional<AssignmentHistory> findByMemberAndStudyDetail(Member member, StudyDetail studyDetail);
Copy link
Member

@kckc0608 kckc0608 Oct 6, 2024

Choose a reason for hiding this comment

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

로직에 영향을 주는 변경점이 아니라면 개발하면서 고쳐도 괜찮을 것 같다고 생각했는데, 이런 부분은 보통 다른 이슈에서 처리하나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

그냥 한 이슈에서 처리해도 되는데 public 일부러 붙여두신건가 싶어서 별도 이슈로 분리했어요.
한 이슈에서 하면 pr에서 묻힐수도 있을 것 같아서

Copy link
Member

Choose a reason for hiding this comment

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

제거해야합니다 굿

Copy link

github-actions bot commented Oct 7, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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

🧹 Outside diff range and nitpick comments (4)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (4)

64-64: 메서드 시그니처 변경 승인 및 제안

getStudyStudents 메서드의 반환 타입을 Page<StudyStudentResponse>로 변경하고 Pageable 파라미터를 추가한 것은 대규모 데이터셋 처리와 성능 향상을 위한 좋은 접근입니다.

메서드에 대한 문서화(JavaDoc)가 있다면 이 변경사항을 반영하여 업데이트하는 것이 좋겠습니다.


69-88: 데이터 검색 및 처리 로직 개선 승인

스터디 세부 정보, 이력 및 관련 데이터를 검색하는 새로운 로직과 효율적인 데이터 접근을 위한 맵 생성은 성능 향상에 도움이 됩니다.

성능을 더욱 최적화하기 위해 다음과 같은 방법을 고려해 보세요:

  1. studyAchievements, attendances, assignmentHistories를 병렬로 조회하여 전체 조회 시간을 줄일 수 있습니다.
  2. 페이지네이션된 studyHistories에 대해서만 관련 데이터를 조회하여 불필요한 데이터 로딩을 방지할 수 있습니다.

90-110: 응답 생성 로직 개선 승인 및 제안

StudyStudentResponse 객체를 생성하는 새로운 로직과 isAttended, getSubmittedAssignment 헬퍼 메서드의 사용은 코드의 가독성을 향상시킵니다.

Java 스트림을 사용하여 로직을 더욱 간소화할 수 있습니다. 예를 들어:

List<StudyStudentResponse> response = studyHistories.getContent().stream()
    .map(studyHistory -> {
        List<StudyTodoResponse> studyTodos = studyDetails.stream()
            .flatMap(studyDetail -> Stream.of(
                StudyTodoResponse.createAttendanceType(
                    studyDetail, LocalDate.now(), 
                    isAttended(attendanceMap.getOrDefault(studyHistory.getStudent().getId(), List.of()), studyDetail)),
                StudyTodoResponse.createAssignmentType(
                    studyDetail, 
                    getSubmittedAssignment(assignmentHistoryMap.getOrDefault(studyHistory.getStudent().getId(), List.of()), studyDetail))
            ))
            .collect(toList());
        
        return StudyStudentResponse.of(
            studyHistory, 
            studyAchievementMap.getOrDefault(studyHistory.getStudent().getId(), List.of()), 
            studyTodos);
    })
    .collect(toList());

이 방식은 중첩된 루프를 제거하고 코드를 더 선언적으로 만들어 가독성을 높일 수 있습니다.


113-116: isAttended 헬퍼 메서드 승인 및 개선 제안

isAttended 메서드는 간결하고 스트림을 효과적으로 사용하고 있습니다.

메서드의 견고성을 높이기 위해 null 체크를 추가하는 것이 좋겠습니다:

private boolean isAttended(List<Attendance> attendances, StudyDetail studyDetail) {
    return attendances != null && studyDetail != null && attendances.stream()
            .anyMatch(attendance -> 
                attendance.getStudyDetail() != null && 
                attendance.getStudyDetail().getId().equals(studyDetail.getId()));
}

이렇게 하면 NullPointerException을 방지할 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 991d43b and d9cab13.

📒 Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (5 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2)

3-4: 의존성 및 임포트 변경 승인

새로운 기능을 위해 추가된 의존성과 임포트 문이 적절해 보입니다. groupingBy 컬렉터의 정적 임포트와 새로운 리포지토리 의존성들이 getStudyStudents 메서드의 변경사항과 일치합니다.

Also applies to: 7-9, 24-24, 27-28, 30-30, 34-36, 46-47, 52-54


176-181: updateStudy 메서드 개선 승인

toSet() 컬렉터를 사용하여 수동 Set 생성을 대체한 것은 코드를 더 간결하고 관용적인 Java 스타일로 만들어 좋은 개선입니다.

이러한 작은 개선사항들이 전체적인 코드의 가독성과 유지보수성을 향상시킵니다.

Comment on lines +118 to +124
private AssignmentHistory getSubmittedAssignment(
List<AssignmentHistory> assignmentHistories, StudyDetail studyDetail) {
return assignmentHistories.stream()
.filter(assignmentHistory ->
assignmentHistory.getStudyDetail().getId().equals(studyDetail.getId()))
.findFirst()
.orElse(null);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

getSubmittedAssignment 메서드 개선 제안

getSubmittedAssignment 메서드의 구조는 좋지만, null 체크가 부족하여 NullPointerException이 발생할 수 있습니다.

다음과 같이 null 체크를 추가하고 최적화하는 것을 제안합니다:

private AssignmentHistory getSubmittedAssignment(
        List<AssignmentHistory> assignmentHistories, StudyDetail studyDetail) {
    return assignmentHistories != null && studyDetail != null ? assignmentHistories.stream()
            .filter(assignmentHistory ->
                    assignmentHistory.getStudyDetail() != null &&
                    assignmentHistory.getStudyDetail().getId().equals(studyDetail.getId()))
            .findFirst()
            .orElse(null)
            : null;
}

이렇게 하면 NullPointerException을 방지하고 메서드의 안정성을 높일 수 있습니다.

Copy link
Member

@kckc0608 kckc0608 left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

lgtm 코멘트 몇 개만 확인해주세요

Comment on lines +11 to 12
// todo: public 제거
public Optional<AssignmentHistory> findByMemberAndStudyDetail(Member member, StudyDetail studyDetail);
Copy link
Member

Choose a reason for hiding this comment

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

제거해야합니다 굿

@@ -9,11 +10,15 @@
public enum AssignmentSubmissionStatusResponse {
NOT_SUBMITTED("미제출"),
FAILURE("제출 실패"),
SUCCESS("제출 성공");
SUCCESS("제출 성공"),
CANCELLED("휴강");
Copy link
Member

Choose a reason for hiding this comment

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

별도 이슈긴 한데 CANCELLED, CANCELED 혼용하고 있어서 추후 CANCELED로 통일해야 할듯 합니다

Copy link
Member

Choose a reason for hiding this comment

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

근데 SUCCESS와 CANCELED가 기존에 없었다면 해당 enum 사용하는 API는 기존에 어떻게 동작했나요?

isOutstandingStudent(FIRST_ROUND_OUTSTANDING_STUDENT, studyAchievements),
isOutstandingStudent(SECOND_ROUND_OUTSTANDING_STUDENT, studyAchievements),
studyTodos,
assignments.size() != cancelledAssignmentsCount
Copy link
Member

Choose a reason for hiding this comment

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

로직에 네이밍 붙여주면 좋을듯 합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

좋습니다~

public List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) {
return queryFactory
.selectFrom(studyAchievement)
.where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId))
Copy link
Member

Choose a reason for hiding this comment

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

사소하긴 한데 studyId가 where절 앞에 오는 편이 더 효율적일듯 합니다

Copy link

github-actions bot commented Oct 8, 2024

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

@Sangwook02 Sangwook02 merged commit 98d4681 into develop Oct 8, 2024
1 check passed
@Sangwook02 Sangwook02 deleted the feature/789-get-students branch October 8, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature 새로운 기능 추가 및 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 수강생 조회 API 추가
3 participants