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 구현 #819

Merged
merged 7 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -4,7 +4,6 @@
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;
Expand All @@ -25,15 +24,15 @@ public class MentorStudyAchievementController {
@Operation(summary = "우수 스터디원 지정", description = "우수 스터디원으로 지정합니다.")
@PostMapping
public ResponseEntity<Void> designateOutstandingStudent(
@RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) {
@RequestParam(name = "studyId") Long studyId, @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) {
@RequestParam(name = "studyId") Long studyId, @RequestBody OutstandingStudentRequest request) {
mentorStudyAchievementService.withdrawOutstandingStudent(studyId, request);
return ResponseEntity.ok().build();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.gdschongik.gdsc.domain.study.api;

import com.gdschongik.gdsc.domain.study.application.MentorStudyHistoryService;
import com.gdschongik.gdsc.domain.study.dto.request.StudyCompleteRequest;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
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.RestController;

@Tag(name = "Mentor Study History", description = "멘토 스터디 수강 이력 API입니다.")
@RestController
@RequestMapping("/mentor/study-history")
@RequiredArgsConstructor
public class MentorStudyHistoryController {

private final MentorStudyHistoryService mentorStudyHistoryService;

Comment on lines +14 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

보안 및 권한 검증이 필요합니다.

컨트롤러 레벨에서 다음 보안 관련 개선사항이 필요합니다:

  1. 멘토 권한 검증을 위한 @PreAuthorize("hasRole('MENTOR')") 어노테이션 추가
  2. CSRF 보호를 위한 적절한 보안 설정 검토
 @Tag(name = "Mentor Study History", description = "멘토 스터디 수강 이력 API입니다.")
+@PreAuthorize("hasRole('MENTOR')")
 @RestController
 @RequestMapping("/mentor/study-history")
 @RequiredArgsConstructor
 public class MentorStudyHistoryController {
📝 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
@Tag(name = "Mentor Study History", description = "멘토 스터디 수강 이력 API입니다.")
@RestController
@RequestMapping("/mentor/study-history")
@RequiredArgsConstructor
public class MentorStudyHistoryController {
private final MentorStudyHistoryService mentorStudyHistoryService;
@Tag(name = "Mentor Study History", description = "멘토 스터디 수강 이력 API입니다.")
@PreAuthorize("hasRole('MENTOR')")
@RestController
@RequestMapping("/mentor/study-history")
@RequiredArgsConstructor
public class MentorStudyHistoryController {
private final MentorStudyHistoryService mentorStudyHistoryService;

@Operation(summary = "스터디 수료 처리", description = "스터디 수료 처리합니다.")
@PostMapping("/complete")
public ResponseEntity<Void> completeStudy(@RequestBody StudyCompleteRequest request) {
mentorStudyHistoryService.completeStudy(request);
return ResponseEntity.ok().build();
}
Comment on lines +22 to +27
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. @Valid 어노테이션으로 요청 데이터 검증
  2. 명확한 응답 메시지 제공
  3. 예외 처리 추가
 @Operation(summary = "스터디 수료 처리", description = "스터디 수료 처리합니다.")
 @PostMapping("/complete")
-public ResponseEntity<Void> completeStudy(@RequestBody StudyCompleteRequest request) {
+public ResponseEntity<ApiResponse> completeStudy(@Valid @RequestBody StudyCompleteRequest request) {
     mentorStudyHistoryService.completeStudy(request);
-    return ResponseEntity.ok().build();
+    return ResponseEntity.ok(new ApiResponse("스터디 수료 처리가 완료되었습니다."));
 }

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


@Operation(summary = "스터디 수료 철회", description = "스터디 수료 처리를 철회합니다.")
@PostMapping("/withdraw-completion")
public ResponseEntity<Void> withdrawStudyCompletion(@RequestBody StudyCompleteRequest request) {
mentorStudyHistoryService.withdrawStudyCompletion(request);
return ResponseEntity.ok().build();
}
Comment on lines +29 to +34
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

수료 철회 기능에 제한사항이 구현되지 않았습니다.

  • 수료 철회 가능 기간 제한
  • 철회 사유 필드 추가
  • 철회 이력 관리
🔗 Analysis chain

수료 철회 기능에 대한 제한사항 검토가 필요합니다.

다음 사항들을 고려해주세요:

  1. 수료 철회 가능 기간 제한
  2. 철회 사유 필드 추가
  3. 철회 이력 관리
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 수료 철회 관련 비즈니스 로직 확인
ast-grep --pattern 'class StudyHistory {
  $$$
  withdraw$_($$$) {
    $$$
  }
  $$$
}'

Length of output: 89


Script:

#!/bin/bash
# withdrawStudyCompletion 메소드 정의 검색
rg 'public .* withdrawStudyCompletion' --context 2

Length of output: 1433


Script:

#!/bin/bash
# withdrawStudyCompletion 메소드 구현 내용 검색
ast-grep --pattern 'class MentorStudyHistoryService {
  $$$
  public void withdrawStudyCompletion($_) {
    $$$
  }
  $$$
}'

# StudyCompleteRequest 클래스 정의 검색
fd 'StudyCompleteRequest.java' | xargs cat

# 철회 이력 관리 관련 코드 검색
rg 'withdrawalHistory' --context 2

Length of output: 358

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package com.gdschongik.gdsc.domain.study.application;

import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

import com.gdschongik.gdsc.domain.member.domain.Member;
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.StudyHistory;
import com.gdschongik.gdsc.domain.study.domain.StudyHistoryValidator;
import com.gdschongik.gdsc.domain.study.domain.StudyValidator;
import com.gdschongik.gdsc.domain.study.dto.request.StudyCompleteRequest;
import com.gdschongik.gdsc.global.exception.CustomException;
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 MentorStudyHistoryService {

private final MemberUtil memberUtil;
private final StudyValidator studyValidator;
private final StudyHistoryValidator studyHistoryValidator;
private final StudyRepository studyRepository;
private final StudyHistoryRepository studyHistoryRepository;

@Transactional
public void completeStudy(StudyCompleteRequest request) {
Member currentMember = memberUtil.getCurrentMember();
Study study =
studyRepository.findById(request.studyId()).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
List<StudyHistory> studyHistories =
studyHistoryRepository.findAllByStudyIdAndStudentIds(request.studyId(), request.studentIds());

studyValidator.validateStudyMentor(currentMember, study);
studyHistoryValidator.validateAppliedToStudy(
studyHistories.size(), request.studentIds().size());

studyHistories.forEach(StudyHistory::complete);

log.info(
"[MentorStudyHistoryService] 스터디 수료 처리: studyId={}, studentIds={}",
request.studyId(),
request.studentIds());
}

@Transactional
public void withdrawStudyCompletion(StudyCompleteRequest request) {
Member currentMember = memberUtil.getCurrentMember();
Study study =
studyRepository.findById(request.studyId()).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
List<StudyHistory> studyHistories =
studyHistoryRepository.findAllByStudyIdAndStudentIds(request.studyId(), request.studentIds());

studyValidator.validateStudyMentor(currentMember, study);
studyHistoryValidator.validateAppliedToStudy(
studyHistories.size(), request.studentIds().size());

studyHistories.forEach(StudyHistory::withdrawCompletion);

log.info(
"[MentorStudyHistoryService] 스터디 수료 철회: studyId={}, studentIds={}",
request.studyId(),
request.studentIds());
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package com.gdschongik.gdsc.domain.study.dao;

import com.gdschongik.gdsc.domain.study.domain.StudyHistory;
import java.util.List;

public interface StudyHistoryCustomRepository {

long countByStudyIdAndStudentIds(Long studyId, List<Long> studentIds);

List<StudyHistory> findAllByStudyIdAndStudentIds(Long studyId, List<Long> studentIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.gdschongik.gdsc.domain.study.domain.QStudyHistory.*;

import com.gdschongik.gdsc.domain.study.domain.StudyHistory;
import com.querydsl.core.types.dsl.BooleanExpression;
import com.querydsl.jpa.impl.JPAQueryFactory;
import java.util.List;
Expand All @@ -21,6 +22,14 @@ public long countByStudyIdAndStudentIds(Long studyId, List<Long> studentIds) {
.fetchOne();
}

@Override
public List<StudyHistory> findAllByStudyIdAndStudentIds(Long studyId, List<Long> studentIds) {
return queryFactory
.selectFrom(studyHistory)
.where(eqStudyId(studyId), studyHistory.student.id.in(studentIds))
.fetch();
}

private BooleanExpression eqStudyId(Long studyId) {
return studyHistory.study.id.eq(studyId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ public void complete() {
studyHistoryStatus = COMPLETED;
}

/**
* 스터디 수료 철회
*/
public void withdrawCompletion() {
studyHistoryStatus = NONE;
}
Comment on lines +81 to +86
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. JavaDoc에 메서드의 사용 조건, 제약 사항, 부작용 등을 상세히 기술해 주세요.
  2. 현재 상태가 COMPLETED인 경우에만 철회가 가능하도록 검증 로직을 추가하는 것이 좋겠습니다.
  3. 수료 철회와 같은 중요한 상태 변경 시 이벤트를 발행하여 다른 시스템에서 대응할 수 있도록 하는 것을 고려해 주세요.

다음과 같은 개선을 제안드립니다:

    /**
     * 스터디 수료 철회
+     * 
+     * @throws IllegalStateException 현재 상태가 COMPLETED가 아닌 경우
     */
    public void withdrawCompletion() {
+        if (studyHistoryStatus != COMPLETED) {
+            throw new IllegalStateException("수료 상태인 경우에만 철회가 가능합니다.");
+        }
        studyHistoryStatus = NONE;
+        registerEvent(new StudyCompletionWithdrawnEvent(this.study.getId(), this.student.getId()));
    }

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


// 데이터 전달 로직
public boolean isWithinApplicationAndCourse() {
return study.isWithinApplicationAndCourse();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.gdschongik.gdsc.domain.study.dto.request;

import java.util.List;

public record StudyCompleteRequest(Long studyId, List<Long> studentIds) {}
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,27 @@ class 스터디_수료시 {
assertThat(studyHistory.getStudyHistoryStatus()).isEqualTo(COMPLETED);
}
}

@Nested
class 스터디_수료_철회시 {

@Test
void 수료상태는_NONE이다() {
// given
Member student = fixtureHelper.createRegularMember(1L);
Member mentor = fixtureHelper.createRegularMember(2L);
LocalDateTime now = LocalDateTime.now();
Study study = fixtureHelper.createStudy(
mentor, Period.of(now.plusDays(5), now.plusDays(10)), Period.of(now.minusDays(5), now));

StudyHistory studyHistory = StudyHistory.create(student, study);
studyHistory.complete();

// when
studyHistory.withdrawCompletion();

// then
assertThat(studyHistory.getStudyHistoryStatus()).isEqualTo(NONE);
}
}
}