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 추가 #809

Merged
merged 20 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.gdschongik.gdsc.domain.study.dto.response.AssignmentResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyCurriculumResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyMentorAttendanceResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyStatisticsResponse;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
Expand Down Expand Up @@ -80,4 +81,11 @@ public ResponseEntity<List<StudyMentorAttendanceResponse>> getAttendanceNumbers(
List<StudyMentorAttendanceResponse> response = mentorStudyDetailService.getAttendanceNumbers(studyId);
return ResponseEntity.ok(response);
}

@Operation(summary = "스터디 통계 조회", description = "멘토가 자신의 스터디 출석률, 과제 제출률, 수료율에 대한 통계를 조회합니다. 휴강 주차는 계산에서 제외합니다.")
@GetMapping("/statistics")
public ResponseEntity<StudyStatisticsResponse> getStudyStatistics(@RequestParam(name = "studyId") Long studyId) {
StudyStatisticsResponse response = mentorStudyDetailService.getStudyStatistics(studyId);
return ResponseEntity.ok(response);
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
package com.gdschongik.gdsc.domain.study.application;

import static com.gdschongik.gdsc.domain.study.domain.AssignmentSubmissionStatus.SUCCESS;
import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

import com.gdschongik.gdsc.domain.member.domain.Member;
import com.gdschongik.gdsc.domain.study.dao.AssignmentHistoryRepository;
import com.gdschongik.gdsc.domain.study.dao.AttendanceRepository;
import com.gdschongik.gdsc.domain.study.dao.StudyDetailRepository;
import com.gdschongik.gdsc.domain.study.dao.StudyHistoryRepository;
import com.gdschongik.gdsc.domain.study.dao.StudyRepository;
import com.gdschongik.gdsc.domain.study.domain.Study;
import com.gdschongik.gdsc.domain.study.domain.StudyDetail;
import com.gdschongik.gdsc.domain.study.domain.StudyDetailValidator;
import com.gdschongik.gdsc.domain.study.domain.StudyHistory;
import com.gdschongik.gdsc.domain.study.domain.StudyValidator;
import com.gdschongik.gdsc.domain.study.dto.request.AssignmentCreateUpdateRequest;
import com.gdschongik.gdsc.domain.study.dto.response.AssignmentResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyCurriculumResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyMentorAttendanceResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyStatisticsResponse;
import com.gdschongik.gdsc.domain.study.dto.response.StudyWeekStatisticsResponse;
import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.global.util.MemberUtil;
import java.time.LocalDate;
Expand All @@ -27,6 +37,11 @@ public class MentorStudyDetailService {
private final MemberUtil memberUtil;
private final StudyDetailRepository studyDetailRepository;
private final StudyDetailValidator studyDetailValidator;
private final StudyHistoryRepository studyHistoryRepository;
private final AttendanceRepository attendanceRepository;
private final AssignmentHistoryRepository assignmentHistoryRepository;
private final StudyValidator studyValidator;
private final StudyRepository studyRepository;

@Transactional(readOnly = true)
public List<AssignmentResponse> getWeeklyAssignments(Long studyId) {
Expand Down Expand Up @@ -108,4 +123,80 @@ public List<StudyMentorAttendanceResponse> getAttendanceNumbers(Long studyId) {
.limit(2)
.toList();
}

@Transactional(readOnly = true)
public StudyStatisticsResponse getStudyStatistics(Long studyId) {
Member currentMember = memberUtil.getCurrentMember();
Study study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
List<StudyHistory> studyHistories = studyHistoryRepository.findAllByStudyId(studyId);
List<StudyDetail> studyDetails = studyDetailRepository.findAllByStudyIdOrderByWeekAsc(studyId);
studyValidator.validateStudyMentor(currentMember, study);

long totalStudentCount = studyHistories.size();
long studyCompleteStudentCount =
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
long studyCompleteStudentCount =
long studyCompletedStudentCount =

Copy link
Member

Choose a reason for hiding this comment

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

이거 아직 반영 안 된 것 같은데 resolve되어 있어서 unresolve 해둘게요!

Copy link
Member Author

Choose a reason for hiding this comment

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

제가 코드 상에 반영해두고 resolve 한 뒤 커밋을 안 했었네요. 앞으론 커밋하고 resolve 할게요! 감사합니당

studyHistories.stream().filter(StudyHistory::isComplete).count();

List<StudyWeekStatisticsResponse> studyWeekStatisticsResponses = studyDetails.stream()
.map((studyDetail -> calculateWeekStatistics(studyDetail, totalStudentCount)))
.toList();

long averageAttendanceRate = calculateAverageWeekAttendanceRate(studyWeekStatisticsResponses);
long averageAssignmentSubmitRate = calculateAverageWeekAssignmentSubmitRate(studyWeekStatisticsResponses);
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved

return StudyStatisticsResponse.of(
totalStudentCount,
studyCompleteStudentCount,
averageAttendanceRate,
averageAssignmentSubmitRate,
studyWeekStatisticsResponses);
}
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved

private StudyWeekStatisticsResponse calculateWeekStatistics(StudyDetail studyDetail, Long totalStudentCount) {
boolean isCanceledWeek = !studyDetail.getCurriculum().isOpen();
Copy link
Member

Choose a reason for hiding this comment

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

그리고 개인적인 생각인데 '스터다 주차' 라고 하면 수업과 과제 모두를 포괄하는 느낌이라서, 뭔가 좀 어색한 것 같네요. '해당 주차가 취소되었는지' 보다는 '해당 주차 수업 / 과제가 취소되었는지' 로 나타내는게 더 정확하지 않나 싶네요 (cc @Sangwook02 )

Copy link
Member Author

Choose a reason for hiding this comment

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

week 가 포괄적인 느낌이라는데 공감합니다. 변수명 고민을 좀 더 해봐야겠네요..

Copy link
Member

Choose a reason for hiding this comment

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

isCanceled 메서드가 있는데 isOpen을 쓰신 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 그걸로 고민을 했었는데, 지금 존재하는 상태가 open, cancel, none 이렇게 3가지가 있어서, isCanceled 를 사용하면 none 으로 설정된 studyDetail 도 열린 주차로 인식하는 문제가 발생할 것 같았어요. 그래서 cancel, none 타입인 경우 모두 canceled week 로 간주하도록 했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

아 open이 아닌지만 확인하면 되는거였군요
그럼 변수명을 수정하면 좋을 것 같아요!

boolean isCanceledAssignment = !studyDetail.getAssignment().isOpen() || isCanceledWeek;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

NullPointerException 방지를 위한 Null 체크 추가 필요

studyDetail.getCurriculum() 또는 studyDetail.getAssignment()null인 경우 NullPointerException이 발생할 수 있습니다. 이를 방지하기 위해 null 체크를 추가하는 것이 좋습니다.

적용할 코드 변경 사항:

- boolean isCanceledWeek = !studyDetail.getCurriculum().isOpen();
+ boolean isCanceledWeek = studyDetail.getCurriculum() == null || !studyDetail.getCurriculum().isOpen();

- boolean isCanceledAssignment = !studyDetail.getAssignment().isOpen() || isCanceledWeek;
+ boolean isCanceledAssignment = studyDetail.getAssignment() == null || !studyDetail.getAssignment().isOpen() || isCanceledWeek;
📝 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
boolean isCanceledWeek = !studyDetail.getCurriculum().isOpen();
boolean isCanceledAssignment = !studyDetail.getAssignment().isOpen() || isCanceledWeek;
boolean isCanceledWeek = studyDetail.getCurriculum() == null || !studyDetail.getCurriculum().isOpen();
boolean isCanceledAssignment = studyDetail.getAssignment() == null || !studyDetail.getAssignment().isOpen() || isCanceledWeek;


if (totalStudentCount == 0) {
return StudyWeekStatisticsResponse.emptyOf(studyDetail.getWeek(), isCanceledAssignment, isCanceledWeek);
}

if (isCanceledWeek) {
return StudyWeekStatisticsResponse.canceledWeekFrom(studyDetail.getWeek());
}

long attendanceCount = attendanceRepository.countByStudyDetailId(studyDetail.getId());
long attendanceRate = Math.round(attendanceCount / (double) totalStudentCount * 100);

if (isCanceledAssignment) {
return StudyWeekStatisticsResponse.canceledAssignmentOf(studyDetail.getWeek(), attendanceRate);
}

long assignmentCount =
Copy link
Member

Choose a reason for hiding this comment

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

제출 성공인 과제의 수를 count하는 것 같은데 변수명을 보고는 알기 어려운 것 같습니다!

assignmentHistoryRepository.countByStudyDetailIdAndSubmissionStatusEquals(studyDetail.getId(), SUCCESS);
long assignmentRate = Math.round(assignmentCount / (double) totalStudentCount * 100);

return StudyWeekStatisticsResponse.openedOf(studyDetail.getWeek(), attendanceRate, assignmentRate);
}
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved

private long calculateAverageWeekAttendanceRate(List<StudyWeekStatisticsResponse> studyWeekStatisticsResponses) {

double averageAttendanceRate = studyWeekStatisticsResponses.stream()
.filter(weekStatisticsResponse -> !weekStatisticsResponse.isCanceledWeek())
.mapToLong(StudyWeekStatisticsResponse::attendanceRate)
.average()
.orElse(0);

return Math.round(averageAttendanceRate);
}

private long calculateAverageWeekAssignmentSubmitRate(
List<StudyWeekStatisticsResponse> studyWeekStatisticsResponses) {

double averageAssignmentSubmitRate = studyWeekStatisticsResponses.stream()
.filter(studyWeekStatistics -> !studyWeekStatistics.isCanceledAssignment())
.mapToLong(StudyWeekStatisticsResponse::assignmentSubmitRate)
.average()
.orElse(0);

return Math.round(averageAssignmentSubmitRate);
}
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import com.gdschongik.gdsc.domain.member.domain.Member;
import com.gdschongik.gdsc.domain.study.domain.AssignmentHistory;
import com.gdschongik.gdsc.domain.study.domain.AssignmentSubmissionStatus;
import com.gdschongik.gdsc.domain.study.domain.StudyDetail;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface AssignmentHistoryRepository
extends JpaRepository<AssignmentHistory, Long>, AssignmentHistoryCustomRepository {
Optional<AssignmentHistory> findByMemberAndStudyDetail(Member member, StudyDetail studyDetail);

long countByStudyDetailIdAndSubmissionStatusEquals(Long studyDetailId, AssignmentSubmissionStatus status);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@

public interface AttendanceRepository extends JpaRepository<Attendance, Long>, AttendanceCustomRepository {
boolean existsByStudentIdAndStudyDetailId(Long studentId, Long studyDetailId);

long countByStudyDetailId(Long studyDetailId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,8 @@ public void complete() {
public boolean isWithinApplicationAndCourse() {
return study.isWithinApplicationAndCourse();
}

public boolean isComplete() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public boolean isComplete() {
public boolean isCompleted() {

return studyHistoryStatus == COMPLETED;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.gdschongik.gdsc.domain.study.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;

public record StudyStatisticsResponse(
@Schema(description = "스터디 전체 수강생 수") Long totalStudentCount,
@Schema(description = "스터디 수료 수강생 수") Long completeStudentCount,
@Schema(description = "평균 출석률") Long averageAttendanceRate,
@Schema(description = "평균 과제 제출률") Long averageAssignmentSubmitRate,
@Schema(description = "스터디 수료율") Long studyCompleteRate,
@Schema(description = "주차별 출석률 및 과제 제출률") List<StudyWeekStatisticsResponse> studyWeekStatisticsResponses) {

public static StudyStatisticsResponse of(
Long totalStudentCount,
Long completeStudentCount,
Long averageAttendanceRate,
Long averageAssignmentSubmitRate,
List<StudyWeekStatisticsResponse> studyWeekStatisticsResponses) {
return new StudyStatisticsResponse(
totalStudentCount,
completeStudentCount,
averageAttendanceRate,
averageAssignmentSubmitRate,
totalStudentCount == 0 ? 0 : Math.round(completeStudentCount / (double) totalStudentCount * 100),
studyWeekStatisticsResponses);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.gdschongik.gdsc.domain.study.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;

public record StudyWeekStatisticsResponse(
@Schema(description = "스터디 주차") Long week,
@Schema(description = "출석률") Long attendanceRate,
@Schema(description = "과제 제출률") Long assignmentSubmitRate,
@Schema(description = "과제 휴강 여부") boolean isCanceledAssignment,
@Schema(description = "스터디 휴강 여부") boolean isCanceledWeek) {

public static StudyWeekStatisticsResponse openedOf(Long week, Long attendanceRate, Long assignmentSubmitRate) {
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved
return new StudyWeekStatisticsResponse(week, attendanceRate, assignmentSubmitRate, false, false);
}

public static StudyWeekStatisticsResponse emptyOf(Long week, boolean isCanceledAssignment, boolean isCanceledWeek) {
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved
return new StudyWeekStatisticsResponse(week, 0L, 0L, isCanceledAssignment, isCanceledWeek);
}

public static StudyWeekStatisticsResponse canceledWeekFrom(Long week) {
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved
return StudyWeekStatisticsResponse.emptyOf(week, true, true);
}
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved

public static StudyWeekStatisticsResponse canceledAssignmentOf(Long week, Long attendanceRate) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static StudyWeekStatisticsResponse canceledAssignmentOf(Long week, Long attendanceRate) {
public static StudyWeekStatisticsResponse assignmentCanceled(Long week, Long attendanceRate) {

Copy link
Member Author

Choose a reason for hiding this comment

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

of, from 컨벤션을 지키면서 케이스가 나뉘는 경우 어떻게 처리해야 하나 궁금했는데, 자세하게 알려주셔서 감사합니다~!

Copy link
Member

@uwoobeat uwoobeat Oct 17, 2024

Choose a reason for hiding this comment

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

이전 컨벤션은 create 접두사를 붙이는 쪽이었는데, 이번에 DTO / VO 쪽에서는 접두사 없이 가면서 조금 어색한 감이 있을 수는 있어요.
일단 쭉 써보면서 계속 논의해봅시다~

return new StudyWeekStatisticsResponse(week, attendanceRate, 0L, true, false);
}
kckc0608 marked this conversation as resolved.
Show resolved Hide resolved
}