-
Notifications
You must be signed in to change notification settings - Fork 1
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
Changes from 15 commits
8243f47
24e4dca
7f1be83
6747b93
526b60b
27c983e
1ee2e40
7c4f1a7
5fa5e05
223286d
b60e067
4dbd45f
7841254
991d43b
d9cab13
961dea1
3f40fb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,18 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import static com.gdschongik.gdsc.domain.study.domain.AssignmentSubmissionStatus.*; | ||
import static com.gdschongik.gdsc.domain.study.domain.QAssignmentHistory.*; | ||
import static com.gdschongik.gdsc.domain.study.domain.QStudyDetail.studyDetail; | ||
|
||
import com.gdschongik.gdsc.domain.member.domain.Member; | ||
import com.gdschongik.gdsc.domain.study.domain.AssignmentHistory; | ||
import com.gdschongik.gdsc.domain.study.domain.Study; | ||
import com.querydsl.core.types.dsl.BooleanExpression; | ||
import com.querydsl.jpa.impl.JPAQueryFactory; | ||
import java.util.List; | ||
import lombok.RequiredArgsConstructor; | ||
|
||
@RequiredArgsConstructor | ||
public class AssignmentHistoryCustomRepositoryImpl implements AssignmentHistoryCustomRepository { | ||
public class AssignmentHistoryCustomRepositoryImpl | ||
implements AssignmentHistoryCustomRepository, AssignmentHistoryQueryMethod { | ||
|
||
private final JPAQueryFactory queryFactory; | ||
|
||
|
@@ -28,18 +27,6 @@ public boolean existsSubmittedAssignmentByMemberAndStudy(Member member, Study st | |
return fetchOne != null; | ||
} | ||
|
||
private BooleanExpression eqMember(Member member) { | ||
return member == null ? null : assignmentHistory.member.eq(member); | ||
} | ||
|
||
private BooleanExpression eqStudy(Study study) { | ||
return study == null ? null : assignmentHistory.studyDetail.study.eq(study); | ||
} | ||
|
||
private BooleanExpression isSubmitted() { | ||
return assignmentHistory.submissionStatus.in(FAILURE, SUCCESS); | ||
} | ||
|
||
@Override | ||
public List<AssignmentHistory> findAssignmentHistoriesByStudentAndStudyId(Member currentMember, Long studyId) { | ||
return queryFactory | ||
|
@@ -50,10 +37,6 @@ public List<AssignmentHistory> findAssignmentHistoriesByStudentAndStudyId(Member | |
.fetch(); | ||
} | ||
|
||
private BooleanExpression eqStudyId(Long studyId) { | ||
return studyId != null ? studyDetail.study.id.eq(studyId) : null; | ||
} | ||
|
||
@Override | ||
public void deleteByStudyIdAndMemberId(Long studyId, Long memberId) { | ||
queryFactory | ||
|
@@ -62,7 +45,13 @@ public void deleteByStudyIdAndMemberId(Long studyId, Long memberId) { | |
.execute(); | ||
} | ||
|
||
private BooleanExpression eqMemberId(Long memberId) { | ||
return memberId != null ? assignmentHistory.member.id.eq(memberId) : null; | ||
@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(); | ||
Comment on lines
+48
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
현재 메서드 다음과 같이 수정할 수 있습니다: if (memberIds == null || memberIds.isEmpty()) {
return Collections.emptyList();
} |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import static com.gdschongik.gdsc.domain.study.domain.AssignmentSubmissionStatus.*; | ||
import static com.gdschongik.gdsc.domain.study.domain.QAssignmentHistory.*; | ||
import static com.gdschongik.gdsc.domain.study.domain.QStudyDetail.*; | ||
|
||
import com.gdschongik.gdsc.domain.member.domain.Member; | ||
import com.gdschongik.gdsc.domain.study.domain.Study; | ||
import com.querydsl.core.types.dsl.BooleanExpression; | ||
|
||
public interface AssignmentHistoryQueryMethod { | ||
default BooleanExpression eqMember(Member member) { | ||
return member == null ? null : assignmentHistory.member.eq(member); | ||
} | ||
|
||
default BooleanExpression eqStudy(Study study) { | ||
return study == null ? null : assignmentHistory.studyDetail.study.eq(study); | ||
} | ||
|
||
default BooleanExpression isSubmitted() { | ||
return assignmentHistory.submissionStatus.in(FAILURE, SUCCESS); | ||
} | ||
|
||
default BooleanExpression eqStudyId(Long studyId) { | ||
return studyId != null ? studyDetail.study.id.eq(studyId) : null; | ||
} | ||
Sangwook02 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
default BooleanExpression eqMemberId(Long memberId) { | ||
return memberId != null ? assignmentHistory.member.id.eq(memberId) : null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,5 +8,6 @@ | |
|
||
public interface AssignmentHistoryRepository | ||
extends JpaRepository<AssignmentHistory, Long>, AssignmentHistoryCustomRepository { | ||
// todo: public 제거 | ||
public Optional<AssignmentHistory> findByMemberAndStudyDetail(Member member, StudyDetail studyDetail); | ||
Comment on lines
+11
to
12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로직에 영향을 주는 변경점이 아니라면 개발하면서 고쳐도 괜찮을 것 같다고 생각했는데, 이런 부분은 보통 다른 이슈에서 처리하나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그냥 한 이슈에서 처리해도 되는데 public 일부러 붙여두신건가 싶어서 별도 이슈로 분리했어요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제거해야합니다 굿 |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import com.gdschongik.gdsc.domain.study.domain.StudyAchievement; | ||
import java.util.List; | ||
|
||
public interface StudyAchievementCustomRepository { | ||
List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds); | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,27 @@ | ||||||||||||||||||||||||||||||||||||
package com.gdschongik.gdsc.domain.study.dao; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
import static com.gdschongik.gdsc.domain.study.domain.QStudyAchievement.*; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
import com.gdschongik.gdsc.domain.study.domain.StudyAchievement; | ||||||||||||||||||||||||||||||||||||
import com.querydsl.core.types.dsl.BooleanExpression; | ||||||||||||||||||||||||||||||||||||
import com.querydsl.jpa.impl.JPAQueryFactory; | ||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||
import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
@RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||
public class StudyAchievementCustomRepositoryImpl implements StudyAchievementCustomRepository { | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
private final JPAQueryFactory queryFactory; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||
public List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds) { | ||||||||||||||||||||||||||||||||||||
return queryFactory | ||||||||||||||||||||||||||||||||||||
.selectFrom(studyAchievement) | ||||||||||||||||||||||||||||||||||||
.where(studyAchievement.student.id.in(memberIds), eqStudyId(studyId)) | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사소하긴 한데 studyId가 where절 앞에 오는 편이 더 효율적일듯 합니다 |
||||||||||||||||||||||||||||||||||||
.fetch(); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쿼리 메서드가 효율적으로 구현되었습니다만, 개선의 여지가 있습니다.
하지만 다음과 같이 @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
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
private BooleanExpression eqStudyId(Long studyId) { | ||||||||||||||||||||||||||||||||||||
return studyId != null ? studyAchievement.study.id.eq(studyId) : null; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import com.gdschongik.gdsc.domain.study.domain.StudyAchievement; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface StudyAchievementRepository | ||
extends JpaRepository<StudyAchievement, Long>, StudyAchievementCustomRepository {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getSubmittedAssignment
메서드 개선 제안getSubmittedAssignment
메서드의 구조는 좋지만, null 체크가 부족하여NullPointerException
이 발생할 수 있습니다.다음과 같이 null 체크를 추가하고 최적화하는 것을 제안합니다:
이렇게 하면
NullPointerException
을 방지하고 메서드의 안정성을 높일 수 있습니다.