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: 스터디 수강생 명단 조회 #623

Merged
merged 24 commits into from
Aug 18, 2024

Conversation

AlmondBreez3
Copy link
Member

@AlmondBreez3 AlmondBreez3 commented Aug 14, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

https://www.notion.so/33fe9881f27a4131a6264a17bb2a95d5

📝 참고사항

  • 한 학생(멘티)가 하나의 study에 대해 study history는 여러개 존재하지 않으므로 단순하게 구현가능했습니다!

📚 기타

  • 멘토인지 아닌지만 구별하면 될 것같아서 정회원 준회원 게스트에 대한 검증은 멘토 지정할 때 그 부분에서 설정하면 될것같습니다

Summary by CodeRabbit

  • 신규 기능

    • 사용자 역할 확인을 위한 isAdmin()isMentor() 메서드 추가.
    • 멘토 유효성 검사를 위한 StudyValidator 클래스 도입.
    • 멘토가 특정 스터디에 등록된 학생 목록을 가져오는 기능 추가.
    • 특정 스터디의 히스토리를 조회할 수 있는 기능 추가.
    • 세션 시작 날짜 이전에 신청 시작 날짜가 오는지 검증하는 기능 추가.
    • 세부적인 오류 처리를 위한 새로운 오류 코드 추가.
  • 버그 수정

    • 멘토가 아닌 사용자가 멘토로 등록되지 않도록 유효성 검사 기능 개선.
  • 테스트

    • StudyValidator 클래스의 유효성 검사 로직에 대한 단위 테스트 추가.
    • 특정 역할을 가진 회원 생성을 위한 테스트 헬퍼 메서드 추가.

@AlmondBreez3 AlmondBreez3 self-assigned this Aug 14, 2024
@AlmondBreez3 AlmondBreez3 requested a review from a team as a code owner August 14, 2024 13:49
Copy link

coderabbitai bot commented Aug 14, 2024

## Walkthrough

이번 변경 사항은 멘토가 특정 스터디에 등록된 학생 목록을 조회할 수 있는 기능을 추가하고, 멤버 역할을 확인하는 메서드를 도입하여 역할 관리의 유연성을 높였습니다. `StudyValidator` 클래스를 통해 멘토 검증 로직을 구현하고, 새로운 테스트 클래스를 추가하여 관련 검증을 강화했습니다. 또한, `FixtureHelper`에서 다양한 멤버 생성 메서드를 통해 테스트 환경을 개선했습니다.

## Changes

| 파일                                                         | 변경 요약                                                                                                          |
|------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| `src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java` <br> `src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java` | 멘토가 스터디 ID로 학생 목록을 조회할 수 있는 기능 추가 및 멤버 역할 확인 메서드 추가 (`isAdmin`, `isMentor`) |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java`                                 | 멘토 검증 로직 추가 (`validateStudyMentor` 메서드)                                                               |
| `src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java`                            | `StudyValidator`에 대한 유닛 테스트 추가로 멘토 검증 로직 확인                                                     |
| `src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java`                                            | 멘토 및 관리자를 생성하는 메서드 추가 (`createAdmin`, `createMentor`)                                           |
| `src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java`                          | 스터디 ID로 스터디 히스토리를 조회하는 메서드 추가 (`findByStudyId`)                                           |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java`                                       | 세션 시작일 이전에 신청 시작일을 확인하는 검증 메서드 추가 (`validateApplicationStartDateBeforeSessionStartDate`) |
| `src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java`                                      | 새로운 오류 코드 추가 (`STUDY_ACCESS_NOT_ALLOWED`, `STUDY_MENTOR_INVALID`)                                      |

## Assessment against linked issues

| Objective                     | Addressed | Explanation                                            |
|-------------------------------|-----------|-------------------------------------------------------|
| 스터디 수강생 명단 조회 (#619) ||                                                       |


> 🐰 새로운 기능이 추가되어,  
> 멘토가 학생을 쉽게 찾을 수 있어요.  
> 스터디 ID로 조회하면,  
> 목록이 쏙쏙, 모두 나타나죠!  
> 코드가 깔끔해져서,  
> 유지보수도 문제 없어요! ✨

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f5587bc and 59eeb9a.

Files selected for processing (4)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (1 hunks)
Additional context used
Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
Learnt from: Sangwook02
PR: GDSC-Hongik/gdsc-server#431
File: src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java:50-57
Timestamp: 2024-07-07T15:32:34.451Z
Learning: Consider using Stream API for creating lists in a more concise and potentially performant manner compared to traditional for-loops.
Additional comments not posted (6)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1)

17-17: 새로운 메서드 추가 확인 완료.

findByStudyId(Long studyId) 메서드의 추가는 스터디 ID를 기반으로 스터디 기록을 조회할 수 있게 하여 데이터 접근성을 향상시킵니다. 이 변경은 리포지토리의 목적에 부합합니다.

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

1-22: 새로운 DTO 클래스 구조 확인 완료.

StudyStudentResponse 클래스는 스터디 학생 정보를 잘 구조화하여 제공하며, StudyHistory 객체를 StudyStudentResponse로 변환하는 from 메서드를 포함하고 있습니다. 이는 변환 로직을 위한 좋은 관행입니다.

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

31-36: 새로운 메서드 추가 확인 완료.

getStudyStudents(Long studyId) 메서드는 스터디 참가자 목록을 효율적으로 조회하고 스트림을 사용하여 DTO로 매핑합니다. 이는 효율적이며 모범 사례에 부합합니다.

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

5-5: 새로운 import 추가 확인

StudyStudentResponse를 위한 import가 추가되었습니다. 문제 없습니다.


12-12: 새로운 import 추가 확인

PathVariable을 위한 import가 추가되었습니다. 문제 없습니다.


31-36: 새로운 메서드 추가 확인

getStudyStudents 메서드가 추가되었습니다. 이 메서드는 특정 스터디의 수강생 목록을 조회합니다. 구현이 잘 되어 있으며, mentorStudyService를 사용하여 데이터를 가져옵니다. 문제 없습니다.

Copy link

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

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

@@ -25,4 +27,11 @@ public ResponseEntity<List<MentorStudyResponse>> getStudiesInCharge() {
List<MentorStudyResponse> response = mentorStudyService.getStudiesInCharge();
return ResponseEntity.ok(response);
}

@Operation(summary = "내 스터디 수강생 명단 조회", description = "내가 멘토로 있는 스터디 수강생 명단을 조회합니다")
@GetMapping("/me/{studyId}")
Copy link
Member

Choose a reason for hiding this comment

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

/me/{studyId}는 스터디의 정보를 조회하는 엔드포인트 아닌가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 다른 controller랑 겹치지 않아서 단순히 이렇게 해놨는데
/me/students/{studyId}
로 바꾸겠습니다

Copy link
Member

Choose a reason for hiding this comment

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

그럼 내 학생들의 스터디를 조회한다는 의미가 되는데, 더 적절해지지 않아질 것 같습니다

@@ -163,6 +165,12 @@ private static void validateAssignmentLineStudyTime(LocalTime studyStartTime, Lo
}
}

public void validateMentor(Member currentMember) {
if (currentMember != mentor) {
Copy link
Member

Choose a reason for hiding this comment

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

엔티티는 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.

앗 넵 studyValidator만들어서 따로 구현하겠습니다

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 59eeb9a and 73c601d.

Files selected for processing (4)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java
Additional comments not posted (4)
src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (1)

31-36: 새로운 엔드포인트 추가 검토

새로운 엔드포인트 /me/students/{studyId}가 추가되었습니다. 이 엔드포인트는 멘토가 자신의 스터디에 등록된 학생 목록을 조회할 수 있도록 합니다. 엔드포인트의 경로와 메서드의 설명이 일치하며, mentorStudyService를 통해 데이터를 가져오는 로직도 적절합니다.

이 변경 사항이 다른 컨트롤러와 충돌하지 않는지 확인하는 것이 좋습니다. 기존의 /me/{studyId} 경로와의 충돌 가능성에 대해 주의가 필요합니다.

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

130-131: 검증 로직 추가

새로운 검증 로직이 추가되었습니다. validateApplicationStartDateBeforeSessionStartDate 메서드는 신청 시작일이 세션 시작일보다 빠른지 확인하여 데이터 무결성을 보장합니다. 이 검증은 스터디 생성 시 유효한 날짜 구성을 보장하는 데 유용합니다.


168-172: 멘토 검증 로직 추가

validateMentor 메서드는 현재 멤버가 지정된 멘토인지 확인합니다. 멘토가 아닌 경우 STUDY_MENTOR_INVALID 예외를 발생시킵니다. 이 검증은 멘토 할당에 대한 제어 흐름을 강화합니다.

src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)

103-103: 새로운 오류 코드 추가

STUDY_MENTOR_INVALID 오류 코드가 추가되었습니다. 이 코드는 사용자가 해당 스터디의 멘토가 아닐 때 발생하는 오류를 나타내며, HttpStatus.CONFLICT 상태와 함께 사용됩니다. 오류 메시지가 명확하여 사용자가 문제를 이해하는 데 도움이 됩니다.

Copy link

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

@DomainService
public class StudyValidator {
public void validateStudyMentor(Long currentMemberId, Long mentorId) {
if (!currentMemberId.equals(mentorId)) {
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

@Sangwook02 Sangwook02 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 10 to 11
@Schema(description = "디스코드 사용자명") String discordName,
@Schema(description = "디스코드 닉네임") String discordUsername,
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
@Schema(description = "디스코드 사용자명") String discordName,
@Schema(description = "디스코드 닉네임") String discordUsername,
@Schema(description = "디스코드 사용자명") String discordUsername,
@Schema(description = "디스코드 닉네임") String nickname,

Comment on lines 18 to 19
studyHistory.getMentee().getDiscordId(),
studyHistory.getMentee().getDiscordUsername(),
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
studyHistory.getMentee().getDiscordId(),
studyHistory.getMentee().getDiscordUsername(),
studyHistory.getMentee().getDiscordUsername(),
studyHistory.getMentee().getNickname(),

Copy link

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, codebase verification and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1)

11-11: 관리자 접근 문제 확인 필요

StudyValidator 클래스에서 관리자 접근에 대한 처리가 이루어지지 않는 것으로 보입니다. 현재 로직은 멘토와의 일치 여부만 확인하고 있어, 관리자 권한을 가진 사용자가 접근할 수 없는 문제가 있을 수 있습니다. 관리자 권한을 고려하여 로직을 수정하는 것이 좋습니다.

Analysis chain

관리자 접근 문제 확인 필요

현재 멘토와의 일치 여부만 확인하고 있어, 관리자가 접근할 수 없는 문제가 있을 수 있습니다. 관리자 권한을 고려하여 로직을 수정하는 것이 좋습니다.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if admin access is handled in the validation logic or elsewhere in the codebase.

# Test: Search for admin access handling related to study validation.
rg --type java 'admin' -A 5

Length of output: 23495


Script:

#!/bin/bash
# Description: Search for admin-related logic within the StudyValidator class and related study classes.

# Test: Search for admin-related keywords in the StudyValidator and related study classes.
fd -e java | xargs rg 'admin|ADMIN' -A 5

Length of output: 26981

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 73c601d and 90846e6.

Files selected for processing (4)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java
Additional context used
Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
Learnt from: Sangwook02
PR: GDSC-Hongik/gdsc-server#431
File: src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java:50-57
Timestamp: 2024-07-07T15:32:34.451Z
Learning: Consider using Stream API for creating lists in a more concise and potentially performant manner compared to traditional for-loops.
Additional comments not posted (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)

35-46: 스트림 API 사용 확인 및 오류 처리 검토

getStudyStudents 메서드는 스트림 API를 사용하여 StudyStudentResponse로 매핑하고 있습니다. 이는 간결하고 성능 면에서 유리합니다. 또한 STUDY_NOT_FOUND 오류 처리가 적절히 이루어지고 있습니다.

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 90846e6 and 55f4ce6.

Files selected for processing (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java

@DomainService
public class StudyValidator {
public void validateStudyMentor(Member currentMember, Long mentorId) {
if (currentMember.isGuest()) {
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.

멘토 할당 받는 회원이 준회원이나 정회원일 수 있는거 아닌가요!?!

Copy link
Member

Choose a reason for hiding this comment

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

해당 validate 메서드는 스터디 멘토인지의 여부를 확인하는 것 아닌가요?
만약 아니라면 메서드 이름이 명료하지 않은 것 같습니다.
말씀하신대로 멘토를 할당하는데 쓰는 validation 로직이라면 다른 네이밍을 사용하거나 별도의 메서드로 분리해야 합니다.

그리고 현재 로그인한 멤버가 멘토인지 항상 확인하게 된다면, 어드민은 멘토가 아니므로 접근이 불가능하게 됩니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

아아 아닙니다 스터디 멘토인지의 여부를 확인하는 건데 멘토 할당할떄 아직 validate가 없어서 정확한기준에 대해서 몰랐던 것 같습니다

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.

커밋 메세지 컨벤션 체크해주세요

@AlmondBreez3 AlmondBreez3 force-pushed the feature/619-get-all-study-students branch from 823eb0b to ee35677 Compare August 16, 2024 12:02
Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 823eb0b and ee35677.

Files selected for processing (3)
  • src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ee35677 and 9790339.

Files selected for processing (4)
  • src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java
Additional comments not posted (2)
src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (2)

50-54: 리플렉션 사용 주의

createAdmin 메서드는 테스트 목적으로 리플렉션을 사용하여 manageRoleADMIN으로 설정합니다. 리플렉션 사용은 테스트 환경에서 적절하지만, 실제 코드에서는 주의가 필요합니다.


56-61: 리플렉션 사용 주의

createMentor 메서드는 멘토 역할을 할당하고 리플렉션을 사용하여 studyRoleMENTOR로 설정합니다. 테스트 환경에서의 리플렉션 사용은 적절하지만, 실제 코드에서는 주의가 필요합니다.

@seulgi99 seulgi99 added the ✨ feature 새로운 기능 추가 및 수정 label Aug 16, 2024
Copy link
Contributor

@seulgi99 seulgi99 left a comment

Choose a reason for hiding this comment

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

LGTM
리뷰남긴거만 처리해주세요~~

@@ -25,4 +27,11 @@ public ResponseEntity<List<MentorStudyResponse>> getStudiesInCharge() {
List<MentorStudyResponse> response = mentorStudyService.getStudiesInCharge();
return ResponseEntity.ok(response);
}

@Operation(summary = "내 스터디 수강생 명단 조회", description = "내가 멘토로 있는 스터디 수강생 명단을 조회합니다")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Operation(summary = "스터디 수강생 명단 조회", description = "내가 멘토로 있는 스터디 수강생 명단을 조회합니다")
@Operation(summary = "스터디 수강생 명단 조회", description = "해당 스터디의 수강생 명단을 조회합니다")

}

// 어드민이 아니고 멘토 역할도 아니면 경우에 예외가 밸생합니다.
if (!currentMember.isAdmin() && !currentMember.isMentor()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!currentMember.isAdmin() && !currentMember.isMentor()) {
if (!currentMember.isMentor()) {

위의 조건에서 어드민이 아니다(!currentMember.isAdmin())는 반드시 참입니다

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9790339 and 7fc485d.

Files selected for processing (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java

Copy link
Member

@Sangwook02 Sangwook02 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 18 to 19

StudyValidator studyValidator = new StudyValidator();
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
StudyValidator studyValidator = new StudyValidator();
StudyValidator studyValidator = new StudyValidator();

class 스터디_수강원_명단_조회시 {

@Test
public void 멘토역할이_아니라면_실패한다() {
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 void 멘토역할이_아니라면_실패한다() {
void 멘토역할이_아니라면_실패한다() {

Copy link
Member

Choose a reason for hiding this comment

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

아래에 있는 다른 test들도 같이 반영해주세요

}

@Nested
class 스터디_수강원_명단_조회시 {
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
class 스터디_수강원_명단_조회시 {
class 스터디_수강자_명단_조회시 {

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7fc485d and aa9a407.

Files selected for processing (1)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java

Copy link
Member

@Sangwook02 Sangwook02 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 3 to 4
import static com.gdschongik.gdsc.global.exception.ErrorCode.STUDY_MENTOR_INVALID;
import static com.gdschongik.gdsc.global.exception.ErrorCode.STUDY_MENTOR_IS_UNAUTHORIZED;
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
import static com.gdschongik.gdsc.global.exception.ErrorCode.STUDY_MENTOR_INVALID;
import static com.gdschongik.gdsc.global.exception.ErrorCode.STUDY_MENTOR_IS_UNAUTHORIZED;
import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

return;
}

// 어드민이 아니고 멘토 역할도 아니면 경우에 예외가 밸생합니다.
Copy link
Member

Choose a reason for hiding this comment

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

오타입니다


// 어드민이 아니고 멘토 역할도 아니면 경우에 예외가 밸생합니다.
if (!currentMember.isMentor()) {
throw new CustomException(STUDY_MENTOR_IS_UNAUTHORIZED);
Copy link
Member

Choose a reason for hiding this comment

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

STUDY_MENTOR_IS_UNAUTHORIZED(HttpStatus.CONFLICT, "게스트인 회원은 멘토로 지정할 수 없습니다."),

이러면 멘토로 validate 메서드를 멘토로 지정하는 경우에만 사용할 수 없게 되지 않나요?

}

@Nested
class 스터디_수강자_명단_조회시 {
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

@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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between aa9a407 and 203449b.

Files selected for processing (3)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java
Additional comments not posted (2)
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2)

103-103: 추가된 에러 코드가 적절합니다.

STUDY_ACCESS_NOT_ALLOWED 에러 코드는 HttpStatus.FORBIDDEN 상태와 함께 적절하게 정의되어 있습니다. 접근 권한이 없는 경우를 명확히 설명합니다.


104-104: 추가된 에러 코드가 적절합니다.

STUDY_MENTOR_INVALID 에러 코드는 HttpStatus.CONFLICT 상태와 함께 적절하게 정의되어 있습니다. 멘토 역할 검증 실패를 명확히 설명합니다.

Copy link

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 203449b and b870af0.

Files selected for processing (6)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (2 hunks)
Files skipped from review as they are similar to previous changes (6)
  • src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java
  • src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java

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

}

@Nested
class 스터디_검증시 {
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
class 스터디_검증시 {
class 스터디_멘토_검증시 {

@AlmondBreez3 AlmondBreez3 merged commit 1dc3f2d into develop Aug 18, 2024
1 check passed
@AlmondBreez3 AlmondBreez3 deleted the feature/619-get-all-study-students branch August 22, 2024 15:49
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.

✨ 스터디 수강생 명단 조회
4 participants