-
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 추가 #644
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1)
34-34
: 테스트 클래스 이름 변경기존의 테스트 클래스 이름이
스터디_멘토역할_검증시
로 변경되었습니다. 이 변경은 테스트 클래스의 목적을 더 명확하게 합니다.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/api/CommonStudyController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/CommonStudyService.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAnnouncementRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyAnnouncementResponse.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (3 hunks)
Additional comments not posted (8)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyAnnouncementResponse.java (1)
1-20
: DTO 정의가 적절합니다
StudyAnnouncementResponse
레코드는 DTO로서 적절하게 정의되었습니다. API 응답을 위한 변환 메서드도 잘 구현되어 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyValidator.java (1)
29-43
: 역할 검증 로직이 적절합니다
validateStudyMentorOrStudent
메서드는 학생과 멘토 역할을 모두 고려하여 검증 로직을 강화합니다. 이로 인해 접근 제어가 개선되었습니다.src/main/java/com/gdschongik/gdsc/domain/study/api/CommonStudyController.java (2)
5-5
: DTO 클래스StudyAnnouncementResponse
의 사용
StudyAnnouncementResponse
클래스가 새로 추가된 엔드포인트의 응답으로 사용됩니다. 이 DTO가 올바르게 정의되고 필요한 필드를 포함하고 있는지 확인하세요.
31-36
: 스터디 공지 목록 조회 엔드포인트 추가새로운
getStudyAnnouncements
메서드는 스터디 공지 목록을 조회하는 기능을 제공합니다. 이 메서드는commonStudyService
를 통해 데이터를 가져오고ResponseEntity
로 반환합니다. 이 변경 사항은 API의 기능을 확장하여 클라이언트가 스터디 공지를 액세스할 수 있도록 합니다. 메서드 설명과 주석이 명확하게 작성되어 있어 이해하기 쉽습니다.src/main/java/com/gdschongik/gdsc/domain/study/application/CommonStudyService.java (2)
Line range hint
5-33
: 새로운 의존성 및 유틸 클래스 추가
StudyAnnouncementRepository
,StudyHistoryRepository
,MemberUtil
,StudyValidator
등의 새로운 의존성이 추가되었습니다. 이들 클래스와 인터페이스가 적절히 정의되고 필요한 기능을 제공하는지 확인하세요.
41-53
: 스터디 공지 목록 조회 로직 추가
getStudyAnnouncements
메서드는 현재 사용자를 가져와 스터디의 유효성을 검사한 후, 공지 목록을 가져와StudyAnnouncementResponse
로 변환합니다. 이 과정에서 멘토 또는 학생 역할을 확인하여 적절한 권한이 있는지 검증합니다. 메서드의 트랜잭션 처리가 적절하게 설정되어 있어 데이터 일관성을 유지합니다.src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyValidatorTest.java (1)
93-132
: 새로운 테스트 시나리오 추가
스터디_멘토_또는_학생역할_검증시
라는 새로운 테스트 클래스가 추가되었습니다. 이 클래스는 학생과 멘토의 스터디 접근 권한을 검증하는 두 가지 새로운 테스트 메서드를 포함하고 있습니다. 이러한 테스트는 애플리케이션의 검증 로직을 강화하여 다양한 조건에서의 역할 검증을 보장합니다.src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java (1)
357-359
: LGTM!
isStudent()
메서드는studyRole
이STUDENT
인지 확인하는 간단한 역할 확인 메서드입니다. 기존의 역할 확인 메서드와 일관성을 유지하고 있으며, 문제 없이 잘 구현되었습니다.
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface StudyAnnouncementRepository extends JpaRepository<StudyAnnouncement, Long> { | ||
|
||
default StudyAnnouncement getById(Long id) { | ||
return findById(id).orElseThrow(() -> new CustomException(STUDY_ANNOUNCEMENT_NOT_FOUND)); | ||
} | ||
|
||
List<StudyAnnouncement> findAllByStudyIdOOrderByCreatedAtDesc(Long studyId); |
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.
메서드 이름의 오타 수정 필요
findAllByStudyIdOOrderByCreatedAtDesc
메서드 이름에 오타가 있습니다. findAllByStudyIdOrderByCreatedAtDesc
로 수정해야 합니다.
- List<StudyAnnouncement> findAllByStudyIdOOrderByCreatedAtDesc(Long studyId);
+ List<StudyAnnouncement> findAllByStudyIdOrderByCreatedAtDesc(Long studyId);
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.
List<StudyAnnouncement> findAllByStudyIdOOrderByCreatedAtDesc(Long studyId); | |
List<StudyAnnouncement> findAllByStudyIdOrderByCreatedAtDesc(Long studyId); |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/CommonStudyService.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAnnouncementRepository.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/CommonStudyService.java
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAnnouncementRepository.java
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.
lgtm
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.
마이너 제안이라 선승인할게요
final Study study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
Optional<StudyHistory> studyHistory = studyHistoryRepository.findByMenteeAndStudyId(currentMember, studyId); | ||
|
||
studyValidator.validateStudyMentorOrStudent(currentMember, study, studyHistory); |
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.
음 공지목록 조회는 굳이 권한 검증이 필요한가... 싶은데
이미 구현을 해두셔서 제거하기엔 좀 그렇고... 한번 보시고 제거 검토해보시면 좋을듯 합니다
|
||
public void validateStudyMentorOrStudent(Member currentMember, Study study, Optional<StudyHistory> studyHistory) { | ||
// 어드민인 경우 검증 통과 | ||
if (currentMember.isAdmin()) { |
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.
중복 로직인데 위에 있는 메서드 그대로 호출해서 쓰면 될 것 같습니다
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.
음 근데 아예 호출하면 에러터지니 따로 private 메소드로 빼서 사용하도록할게여
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.
lgtm
…into feature/643-get-study-announcements
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
신규 기능
isStudent()
메소드 추가.버그 수정
테스트