-
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 구현 #800
Changes from 6 commits
5ab0ac9
a691cb1
35b1e51
ec20137
d85696d
8d63cdd
00352c3
c40b53f
bcf85b2
30757ee
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 | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||
package com.gdschongik.gdsc.domain.study.api; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import com.gdschongik.gdsc.domain.study.application.MentorStudyAchievementService; | ||||||||||||||||||||||||||||||
import com.gdschongik.gdsc.domain.study.dto.request.OutstandingStudentRequest; | ||||||||||||||||||||||||||||||
import io.swagger.v3.oas.annotations.Operation; | ||||||||||||||||||||||||||||||
import io.swagger.v3.oas.annotations.tags.Tag; | ||||||||||||||||||||||||||||||
import jakarta.validation.Valid; | ||||||||||||||||||||||||||||||
import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||
import org.springframework.http.ResponseEntity; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.DeleteMapping; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.PostMapping; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.RequestBody; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.RequestMapping; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.RequestParam; | ||||||||||||||||||||||||||||||
import org.springframework.web.bind.annotation.RestController; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
@Tag(name = "Mentor StudyAchievement", description = "멘토 스터디 우수 스터디원 관리 API입니다.") | ||||||||||||||||||||||||||||||
@RestController | ||||||||||||||||||||||||||||||
@RequestMapping("/mentor/study-achievements") | ||||||||||||||||||||||||||||||
@RequiredArgsConstructor | ||||||||||||||||||||||||||||||
public class MentorStudyAchievementController { | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
private final MentorStudyAchievementService mentorStudyAchievementService; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
@Operation(summary = "우수 스터디원 지정", description = "우수 스터디원으로 지정합니다.") | ||||||||||||||||||||||||||||||
@PostMapping | ||||||||||||||||||||||||||||||
public ResponseEntity<Void> designateOutstandingStudent( | ||||||||||||||||||||||||||||||
@RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) { | ||||||||||||||||||||||||||||||
mentorStudyAchievementService.designateOutstandingStudent(studyId, request); | ||||||||||||||||||||||||||||||
return ResponseEntity.ok().build(); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
@Operation(summary = "우수 스터디원 철회", description = "우수 스터디원 지정을 철회합니다.") | ||||||||||||||||||||||||||||||
@DeleteMapping | ||||||||||||||||||||||||||||||
public ResponseEntity<Void> withdrawOutstandingStudent( | ||||||||||||||||||||||||||||||
@RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) { | ||||||||||||||||||||||||||||||
mentorStudyAchievementService.withdrawOutstandingStudent(studyId, request); | ||||||||||||||||||||||||||||||
return ResponseEntity.ok().build(); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+33
to
+39
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. 🛠️ Refactor suggestion 에러 처리 및 응답 코드 개선 제안 이 메서드도 이전 메서드와 유사한 개선이 필요합니다:
다음과 같이 코드를 수정해 보세요: @DeleteMapping
public ResponseEntity<Void> withdrawOutstandingStudent(
@RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) {
mentorStudyAchievementService.withdrawOutstandingStudent(studyId, request);
- return ResponseEntity.ok().build();
+ return ResponseEntity.noContent().build();
} 또한, 전역 예외 처리기를 구현하여 다양한 예외 상황(예: 존재하지 않는 studyId)에 대응하는 것을 고려해 보세요. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,69 @@ | ||||||||||
package com.gdschongik.gdsc.domain.study.application; | ||||||||||
|
||||||||||
import com.gdschongik.gdsc.domain.member.dao.MemberRepository; | ||||||||||
import com.gdschongik.gdsc.domain.member.domain.Member; | ||||||||||
import com.gdschongik.gdsc.domain.study.dao.StudyAchievementRepository; | ||||||||||
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.StudyAchievement; | ||||||||||
import com.gdschongik.gdsc.domain.study.domain.StudyHistoryValidator; | ||||||||||
import com.gdschongik.gdsc.domain.study.domain.StudyValidator; | ||||||||||
import com.gdschongik.gdsc.domain.study.dto.request.OutstandingStudentRequest; | ||||||||||
import com.gdschongik.gdsc.global.util.MemberUtil; | ||||||||||
import java.util.List; | ||||||||||
import lombok.RequiredArgsConstructor; | ||||||||||
import lombok.extern.slf4j.Slf4j; | ||||||||||
import org.springframework.stereotype.Service; | ||||||||||
import org.springframework.transaction.annotation.Transactional; | ||||||||||
|
||||||||||
@Slf4j | ||||||||||
@Service | ||||||||||
@RequiredArgsConstructor | ||||||||||
public class MentorStudyAchievementService { | ||||||||||
|
||||||||||
private final MemberUtil memberUtil; | ||||||||||
private final StudyValidator studyValidator; | ||||||||||
private final StudyHistoryValidator studyHistoryValidator; | ||||||||||
private final StudyRepository studyRepository; | ||||||||||
private final StudyHistoryRepository studyHistoryRepository; | ||||||||||
private final StudyAchievementRepository studyAchievementRepository; | ||||||||||
private final MemberRepository memberRepository; | ||||||||||
|
||||||||||
@Transactional | ||||||||||
public void designateOutstandingStudent(Long studyId, OutstandingStudentRequest request) { | ||||||||||
Member currentMember = memberUtil.getCurrentMember(); | ||||||||||
Study study = studyRepository.getById(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.
제안된 수정 코드: - Study study = studyRepository.getById(studyId);
+ Study study = studyRepository.findById(studyId)
+ .orElseThrow(() -> new EntityNotFoundException("해당 스터디를 찾을 수 없습니다. studyId: " + studyId)); 📝 Committable suggestion
Suggested change
|
||||||||||
boolean isAllAppliedToStudy = | ||||||||||
studyHistoryRepository.existsByStudyIdAndStudentIds(studyId, request.studentIds()); | ||||||||||
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. 모든 학생들이 스터디에 적용되었는지 정확한 검증이 필요합니다
제안된 수정: - boolean isAllAppliedToStudy =
- studyHistoryRepository.existsByStudyIdAndStudentIds(studyId, request.studentIds());
+ long appliedStudentCount = studyHistoryRepository.countByStudyIdAndStudentIds(studyId, request.studentIds());
+ boolean isAllAppliedToStudy = appliedStudentCount == request.studentIds().size(); 📝 Committable suggestion
Suggested change
|
||||||||||
|
||||||||||
studyValidator.validateStudyMentor(currentMember, study); | ||||||||||
studyHistoryValidator.validateAppliedToStudy(isAllAppliedToStudy); | ||||||||||
|
||||||||||
List<Member> outstandingStudents = memberRepository.findAllById(request.studentIds()); | ||||||||||
List<StudyAchievement> studyAchievements = outstandingStudents.stream() | ||||||||||
.map(member -> StudyAchievement.create(member, study, request.achievementType())) | ||||||||||
.toList(); | ||||||||||
studyAchievementRepository.saveAll(studyAchievements); | ||||||||||
|
||||||||||
log.info( | ||||||||||
"[MentorStudyAchievementService] 우수 스터디원 지정: studyId={}, studentIds={}", studyId, request.studentIds()); | ||||||||||
} | ||||||||||
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. 🛠️ Refactor suggestion 중복된 코드의 리팩토링을 고려하세요
예시로, 공통되는 검증 로직을 메서드로 추출할 수 있습니다: private Study validateStudyAndMentor(Long studyId) {
Member currentMember = memberUtil.getCurrentMember();
Study study = studyRepository.findById(studyId)
.orElseThrow(() -> new EntityNotFoundException("해당 스터디를 찾을 수 없습니다. studyId: " + studyId));
studyValidator.validateStudyMentor(currentMember, study);
return study;
}
private void validateAllStudentsApplied(Long studyId, List<Long> studentIds) {
long appliedStudentCount = studyHistoryRepository.countByStudyIdAndStudentIds(studyId, studentIds);
boolean isAllAppliedToStudy = appliedStudentCount == studentIds.size();
studyHistoryValidator.validateAppliedToStudy(isAllAppliedToStudy);
} 그리고 각 메서드에서 이를 활용합니다: public void designateOutstandingStudent(Long studyId, OutstandingStudentRequest request) {
- Member currentMember = memberUtil.getCurrentMember();
- Study study = studyRepository.getById(studyId);
+ Study study = validateStudyAndMentor(studyId);
- boolean isAllAppliedToStudy =
- studyHistoryRepository.existsByStudyIdAndStudentIds(studyId, request.studentIds());
- studyHistoryValidator.validateAppliedToStudy(isAllAppliedToStudy);
+ validateAllStudentsApplied(studyId, request.studentIds());
// 이하 생략
} Also applies to: 54-68 |
||||||||||
|
||||||||||
@Transactional | ||||||||||
public void withdrawOutstandingStudent(Long studyId, OutstandingStudentRequest request) { | ||||||||||
Member currentMember = memberUtil.getCurrentMember(); | ||||||||||
Study study = studyRepository.getById(studyId); | ||||||||||
Sangwook02 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
boolean isAllAppliedToStudy = | ||||||||||
studyHistoryRepository.existsByStudyIdAndStudentIds(studyId, request.studentIds()); | ||||||||||
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. 모든 학생들이 스터디에 적용되었는지 정확한 검증이 필요합니다 위와 동일하게, 제안된 수정: - boolean isAllAppliedToStudy =
- studyHistoryRepository.existsByStudyIdAndStudentIds(studyId, request.studentIds());
+ long appliedStudentCount = studyHistoryRepository.countByStudyIdAndStudentIds(studyId, request.studentIds());
+ boolean isAllAppliedToStudy = appliedStudentCount == request.studentIds().size(); 📝 Committable suggestion
Suggested change
|
||||||||||
|
||||||||||
studyValidator.validateStudyMentor(currentMember, study); | ||||||||||
studyHistoryValidator.validateAppliedToStudy(isAllAppliedToStudy); | ||||||||||
|
||||||||||
studyAchievementRepository.deleteByStudyAndAchievementTypeAndMemberIds( | ||||||||||
studyId, request.achievementType(), request.studentIds()); | ||||||||||
|
||||||||||
log.info( | ||||||||||
"[MentorStudyAchievementService] 우수 스터디원 철회: studyId={}, studentIds={}", studyId, request.studentIds()); | ||||||||||
} | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,12 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import com.gdschongik.gdsc.domain.study.domain.AchievementType; | ||
import com.gdschongik.gdsc.domain.study.domain.StudyAchievement; | ||
import java.util.List; | ||
|
||
public interface StudyAchievementCustomRepository { | ||
List<StudyAchievement> findByStudyIdAndMemberIds(Long studyId, List<Long> memberIds); | ||
|
||
void deleteByStudyAndAchievementTypeAndMemberIds( | ||
Long studyId, AchievementType achievementType, List<Long> memberIds); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import java.util.List; | ||
|
||
public interface StudyHistoryCustomRepository { | ||
|
||
boolean existsByStudyIdAndStudentIds(Long studyId, List<Long> studentIds); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package com.gdschongik.gdsc.domain.study.dao; | ||
|
||
import static com.gdschongik.gdsc.domain.study.domain.QStudyHistory.*; | ||
|
||
import com.querydsl.core.types.dsl.BooleanExpression; | ||
import com.querydsl.jpa.impl.JPAQueryFactory; | ||
import java.util.List; | ||
import lombok.RequiredArgsConstructor; | ||
|
||
@RequiredArgsConstructor | ||
public class StudyHistoryCustomRepositoryImpl implements StudyHistoryCustomRepository { | ||
|
||
private final JPAQueryFactory queryFactory; | ||
|
||
@Override | ||
public boolean existsByStudyIdAndStudentIds(Long studyId, List<Long> studentIds) { | ||
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. 인자로 넘긴 student id 리스트와 count가 일치하는지 비교하는 메서드로 보입니다. |
||
Long count = queryFactory | ||
.select(studyHistory.count()) | ||
.from(studyHistory) | ||
.where(eqStudyId(studyId), studyHistory.student.id.in(studentIds)) | ||
.fetchOne(); | ||
return count != null && count == studentIds.size(); | ||
} | ||
|
||
private BooleanExpression eqStudyId(Long studyId) { | ||
return studyHistory.study.id.eq(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. 🛠️ Refactor suggestion 전반적으로 우수한 구현이지만, 몇 가지 고려사항이 있습니다. 이 구현은 전체적으로 잘 작성되었습니다. 그러나 다음 사항들을 고려해 보시기 바랍니다:
이러한 개선사항들을 구현하는 데 도움이 필요하시면 말씀해 주세요. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package com.gdschongik.gdsc.domain.study.dto.request; | ||
|
||
import com.gdschongik.gdsc.domain.study.domain.AchievementType; | ||
import java.util.List; | ||
|
||
public record OutstandingStudentRequest(List<Long> studentIds, AchievementType achievementType) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
package com.gdschongik.gdsc.domain.study.application; | ||
|
||
import static com.gdschongik.gdsc.domain.study.domain.AchievementType.*; | ||
import static org.assertj.core.api.Assertions.*; | ||
|
||
import com.gdschongik.gdsc.domain.member.domain.Member; | ||
import com.gdschongik.gdsc.domain.recruitment.domain.vo.Period; | ||
import com.gdschongik.gdsc.domain.study.domain.Study; | ||
import com.gdschongik.gdsc.domain.study.domain.StudyAchievement; | ||
import com.gdschongik.gdsc.domain.study.dto.request.OutstandingStudentRequest; | ||
import com.gdschongik.gdsc.helper.IntegrationTest; | ||
import java.time.LocalDateTime; | ||
import java.util.List; | ||
import org.junit.jupiter.api.Nested; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
|
||
public class MentorStudyAchievementServiceTest extends IntegrationTest { | ||
|
||
@Autowired | ||
private MentorStudyAchievementService mentorStudyAchievementService; | ||
|
||
@Nested | ||
class 우수_스터디원_지정시 { | ||
|
||
@Test | ||
void 성공한다() { | ||
// given | ||
LocalDateTime now = LocalDateTime.now(); | ||
Member mentor = createMentor(); | ||
Study study = createStudy( | ||
mentor, | ||
Period.createPeriod(now.plusDays(5), now.plusDays(10)), | ||
Period.createPeriod(now.minusDays(5), now)); | ||
|
||
Member student = createRegularMember(); | ||
createStudyHistory(student, study); | ||
|
||
logoutAndReloginAs(mentor.getId(), mentor.getRole()); | ||
OutstandingStudentRequest request = | ||
new OutstandingStudentRequest(List.of(student.getId()), FIRST_ROUND_OUTSTANDING_STUDENT); | ||
|
||
// when | ||
mentorStudyAchievementService.designateOutstandingStudent(study.getId(), request); | ||
|
||
// then | ||
List<StudyAchievement> studyAchievements = | ||
studyAchievementRepository.findByStudyIdAndMemberIds(study.getId(), request.studentIds()); | ||
assertThat(studyAchievements).hasSize(request.studentIds().size()); | ||
} | ||
} | ||
|
||
@Nested | ||
class 우수_스터디원_철회시 { | ||
|
||
@Test | ||
void 성공한다() { | ||
// given | ||
Member student = createRegularMember(); | ||
LocalDateTime now = LocalDateTime.now(); | ||
Member mentor = createMentor(); | ||
Study study = createStudy( | ||
mentor, | ||
Period.createPeriod(now.plusDays(5), now.plusDays(10)), | ||
Period.createPeriod(now.minusDays(5), now)); | ||
createStudyHistory(student, study); | ||
createStudyAchievement(student, study, FIRST_ROUND_OUTSTANDING_STUDENT); | ||
|
||
logoutAndReloginAs(mentor.getId(), mentor.getRole()); | ||
OutstandingStudentRequest request = | ||
new OutstandingStudentRequest(List.of(student.getId()), FIRST_ROUND_OUTSTANDING_STUDENT); | ||
|
||
// when | ||
mentorStudyAchievementService.withdrawOutstandingStudent(study.getId(), request); | ||
|
||
// then | ||
List<StudyAchievement> studyAchievements = | ||
studyAchievementRepository.findByStudyIdAndMemberIds(study.getId(), request.studentIds()); | ||
assertThat(studyAchievements).isEmpty(); | ||
} | ||
} | ||
} |
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.
🛠️ Refactor suggestion
에러 처리 및 응답 코드 개선 제안
메서드 구조는 적절하지만, 다음과 같은 개선사항을 고려해 보시기 바랍니다:
다음과 같이 코드를 수정해 보세요:
또한, 전역 예외 처리기를 구현하여 다양한 예외 상황에 대응하는 것을 고려해 보세요.
📝 Committable suggestion