-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 누락된 스터디 역할 부여 케이스 처리를 위한 디스코드 커맨드 #1025
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: 누락된 스터디 역할 부여 케이스 처리를 위한 디스코드 커맨드 #1025
Conversation
📝 WalkthroughWalkthrough이 PR은 Discord 봇에 스터디 역할 할당 기능을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Discord User
participant Listener as AssignStudyRoleCommandListener
participant Handler as AssignStudyRoleCommandHandler
participant Service as CommonDiscordService
participant Repo as StudyV2Repository/StudyHistoryV2Repository
participant DiscordUtil as Discord Util
User->>Listener: 슬래시 커맨드 실행
Listener->>Handler: 이벤트 위임
Handler->>Service: assignDiscordStudyRole(사용자, studyTitle, academicYear, semester)
Service->>Repo: 스터디 및 기록 조회
Repo-->>Service: 조회 결과 반환
Service->>DiscordUtil: 각 멤버에 역할 할당
DiscordUtil-->>Service: 역할 할당 완료
Service->>Logger: 할당 로그 기록
Service-->>Handler: 작업 완료 응답
Handler->>User: 응답 메시지 전송 (일시적)
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 using PR comments)
Other keywords and placeholders
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: 3
🧹 Nitpick comments (6)
src/main/java/com/gdschongik/gdsc/domain/discord/application/listener/AssignStudyRoleCommandListener.java (1)
19-23:@Override애노테이션을 추가해주세요.부모 클래스의 메서드를 오버라이드할 때는
@Override애노테이션을 추가하는 것이 좋습니다. 이를 통해 컴파일러가 메서드 시그니처가 부모 클래스의 메서드와 일치하는지 확인할 수 있습니다.- public void onSlashCommandInteraction(@NotNull SlashCommandInteractionEvent event) { + @Override + public void onSlashCommandInteraction(@NotNull SlashCommandInteractionEvent event) {src/main/java/com/gdschongik/gdsc/domain/discord/application/CommonDiscordService.java (2)
62-68: 입력 매개변수 검증 추가 필요.현재 코드는
currentMemberDiscordUsername에 대한 검증만 있고studyTitle,academicYear,semester파라미터에 대한 유효성 검증이 없습니다. 이로 인해 잘못된 입력값이 전달될 경우 예상치 못한 오류가 발생할 수 있습니다.다음과 같이 입력 매개변수 검증을 추가하는 것이 좋습니다:
@Transactional public void assignDiscordStudyRole( String currentMemberDiscordUsername, String studyTitle, Integer academicYear, String semester) { + if (studyTitle == null || studyTitle.isBlank()) { + throw new CustomException(INVALID_INPUT_VALUE, "스터디 제목은 필수입니다."); + } + if (academicYear == null || academicYear <= 0) { + throw new CustomException(INVALID_INPUT_VALUE, "학년도는 유효한 값이어야 합니다."); + } + if (semester == null || semester.isBlank()) { + throw new CustomException(INVALID_INPUT_VALUE, "학기는 필수입니다."); + } + Member currentMember = memberRepository .findByDiscordUsername(currentMemberDiscordUsername) .orElseThrow(() -> new CustomException(MEMBER_NOT_FOUND));
79-79: 로그 메시지 개선.로그 메시지에 스터디 제목, 학년도, 학기와 같은 추가 정보를 포함하면 문제 해결 시 더 유용할 것입니다.
다음과 같이 로그 메시지를 개선할 수 있습니다:
- log.info("[CommonDiscordService] 스터디 디스코드 역할 부여 완료: studyId = {}", study.getId()); + log.info("[CommonDiscordService] 스터디 디스코드 역할 부여 완료: studyId = {}, 제목 = {}, 학년도 = {}, 학기 = {}", + study.getId(), studyTitle, academicYear, semester);src/main/java/com/gdschongik/gdsc/global/common/constant/DiscordConstant.java (3)
43-48: 옵션 용어 일관성 검토.옵션 이름 중
OPTION_NAME_STUDY_TITLE에 대응하는 한글 값이 "스터디-이름"으로 되어 있는데, 서비스 메서드의 매개변수명studyTitle과 일관성을 위해 "스터디-제목"으로 변경하는 것이 좋을 수 있습니다.다음과 같이 변경을 고려해보세요:
- public static final String OPTION_NAME_STUDY_TITLE = "스터디-이름"; - public static final String OPTION_DESCRIPTION_STUDY_TITLE = "스터디 이름을 입력해주세요."; + public static final String OPTION_NAME_STUDY_TITLE = "스터디-제목"; + public static final String OPTION_DESCRIPTION_STUDY_TITLE = "스터디 제목을 입력해주세요.";
38-38: 주석과 상수명 불일치.주석에는 "스터디 역할 부여 누락자 부여 커맨드"라고 되어 있지만, 실제 상수명과 설명에는 "누락자"에 대한 언급이 없습니다. 주석이 정확히 기능을 반영하도록 수정하는 것이 좋습니다.
다음과 같이 주석을 수정하는 것이 좋습니다:
- // 스터디 역할 부여 누락자 부여 커맨드 + // 스터디 역할 부여 커맨드
38-48: 에러 메시지 상수 추가 고려.현재 스터디 역할 부여 관련 상수들 중에 에러 메시지에 대한 상수가 없습니다. 역할 부여 실패 시 사용할 에러 메시지 상수를 추가하는 것이 좋습니다.
다음과 같은 에러 메시지 상수를 추가할 수 있습니다:
public static final String OPTION_DESCRIPTION_STUDY_YEAR = "연도를 입력해주세요."; public static final String OPTION_NAME_STUDY_SEMESTER = "스터디-학기"; public static final String OPTION_DESCRIPTION_STUDY_SEMESTER = "학기를 입력해주세요."; + public static final String ERROR_MESSAGE_ASSIGN_STUDY_ROLE = "스터디 역할 부여 작업 중 오류가 발생했습니다: %s";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/gdschongik/gdsc/domain/discord/application/CommonDiscordService.java(3 hunks)src/main/java/com/gdschongik/gdsc/domain/discord/application/handler/AssignStudyRoleCommandHandler.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/discord/application/listener/AssignStudyRoleCommandListener.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2Repository.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/common/constant/DiscordConstant.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/config/DiscordConfig.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2Repository.java (1)
Learnt from: uwoobeat
PR: GDSC-Hongik/gdsc-server#889
File: src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java:35-39
Timestamp: 2025-03-28T07:52:58.277Z
Learning: In the GDSC server project's V2 domain, following DDD principles, StudyV2 is an aggregate root and StudySession is its child entity. Therefore, StudySession should only be accessed through StudyV2 aggregate root using StudyV2Repository, not directly via a separate StudySessionRepository.
🧬 Code Definitions (2)
src/main/java/com/gdschongik/gdsc/domain/discord/application/listener/AssignStudyRoleCommandListener.java (2)
src/main/java/com/gdschongik/gdsc/global/common/constant/DiscordConstant.java (1)
DiscordConstant(3-49)src/main/java/com/gdschongik/gdsc/domain/discord/application/handler/AssignStudyRoleCommandHandler.java (1)
Component(12-38)
src/main/java/com/gdschongik/gdsc/domain/discord/application/handler/AssignStudyRoleCommandHandler.java (2)
src/main/java/com/gdschongik/gdsc/global/common/constant/DiscordConstant.java (1)
DiscordConstant(3-49)src/main/java/com/gdschongik/gdsc/domain/discord/application/listener/AssignStudyRoleCommandListener.java (1)
Component(12-24)
🔇 Additional comments (7)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2Repository.java (1)
13-14: 좋은 구현입니다!제목과 학기로 스터디를 찾는 메서드가 추가되었습니다. Optional 타입을 반환하여 null 처리도 잘 고려하셨습니다.
src/main/java/com/gdschongik/gdsc/domain/discord/application/listener/AssignStudyRoleCommandListener.java (1)
19-23:Details
✅ Verification successful
부모 메서드 호출 확인이 필요합니다.
현재 구현에서는
super.onSlashCommandInteraction(event)를 호출하지 않고 있습니다. 일반적으로ListenerAdapter의 메서드를 오버라이드할 때는 부모 메서드도 호출하는 것이 좋습니다. 다른 명령어 처리에 영향을 주지 않는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash # 다른 리스너 클래스들에서 super 메서드 호출 패턴 확인 rg -A 1 -B 1 "extends ListenerAdapter" --type java rg "public void onSlashCommandInteraction.*super\.onSlashCommandInteraction" --type javaLength of output: 3321
검증 결과 - 부모 메서드 호출 생략은 일관된 패턴으로 보입니다.
- 실행한
rg명령어 결과,onSlashCommandInteraction메서드에서super.onSlashCommandInteraction(event)를 호출하는 부분은 다른 Listener 클래스에서도 찾아볼 수 없었습니다.AssignStudyRoleCommandListener를 비롯한 여러 리스너(AdvanceFailedMemberCommandListener,DiscordIdBatchCommandListener등)에서 동일한 패턴이 적용되어 있는 점을 고려하면, 현재 구현 방식이 의도한 것으로 보입니다.- 현재 코드베이스에서는 부모 메서드 호출 없이도 다른 명령어 처리에 영향이 없는 것으로 판단되므로, 별도의 수정은 필요하지 않아 보입니다.
src/main/java/com/gdschongik/gdsc/global/config/DiscordConfig.java (1)
51-54: 좋은 구현입니다!스터디 역할 부여 명령어가 잘 추가되었습니다. 필요한 옵션들(스터디 이름, 연도, 학기)도 모두 포함되어 있습니다.
src/main/java/com/gdschongik/gdsc/domain/discord/application/CommonDiscordService.java (3)
6-14: 필요한 Import 추가 확인.새로운 스터디 역할 부여 기능에 필요한 Import들이 적절히 추가되었습니다.
29-30: 의존성 주입 확인.스터디 관련 Repository 의존성이 적절히 추가되었습니다.
75-77: 🛠️ Refactor suggestionDiscordId null 검사 추가 필요.
스터디 멤버들의 DiscordId가 null이거나 비어있을 경우 예외가 발생할 수 있습니다. 이에 대한 검증이 필요합니다.
다음과 같이 수정하는 것이 좋습니다:
- List<StudyHistoryV2> studyHistories = studyHistoryV2Repository.findAllByStudy(study); - studyHistories.forEach(studyHistoryV2 -> discordUtil.addRoleToMemberById( - study.getDiscordRoleId(), studyHistoryV2.getStudent().getDiscordId())); + List<StudyHistoryV2> studyHistories = studyHistoryV2Repository.findAllByStudy(study); + studyHistories.forEach(studyHistoryV2 -> { + String discordId = studyHistoryV2.getStudent().getDiscordId(); + if (discordId != null && !discordId.isBlank()) { + discordUtil.addRoleToMemberById(study.getDiscordRoleId(), discordId); + } else { + log.warn("[CommonDiscordService] 디스코드 ID가 없어 역할 부여를 건너뜁니다: 학생 ID = {}", + studyHistoryV2.getStudent().getId()); + } + });⛔ Skipped due to learnings
Learnt from: kckc0608 PR: GDSC-Hongik/gdsc-server#1012 File: src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryV2.java:71-74 Timestamp: 2025-03-28T07:52:58.277Z Learning: 이 프로젝트에서는 discordRoleId와 discordId의 null 처리가 StudyHistoryV2 클래스의 postPersist 메서드가 아닌 다른 곳에서 처리되고 있어, 중복 검사가 필요하지 않습니다.src/main/java/com/gdschongik/gdsc/global/common/constant/DiscordConstant.java (1)
38-42: Discord 명령어 상수 추가 확인.스터디 역할 부여를 위한 명령어와 관련 설명이 적절히 추가되었습니다.
| public void delegate(GenericEvent genericEvent) { | ||
| SlashCommandInteractionEvent event = (SlashCommandInteractionEvent) genericEvent; | ||
| event.deferReply(true).setContent(DEFER_MESSAGE_ASSIGN_STUDY_ROLE).queue(); |
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
이벤트 타입 검사가 필요합니다.
genericEvent를 SlashCommandInteractionEvent로 캐스팅하기 전에 instanceOf 검사를 수행하는 것이 안전합니다. 현재 구현은 다른 타입의 이벤트가 전달될 경우 ClassCastException이 발생할 수 있습니다.
- public void delegate(GenericEvent genericEvent) {
- SlashCommandInteractionEvent event = (SlashCommandInteractionEvent) genericEvent;
+ public void delegate(GenericEvent genericEvent) {
+ if (!(genericEvent instanceof SlashCommandInteractionEvent)) {
+ return;
+ }
+ SlashCommandInteractionEvent event = (SlashCommandInteractionEvent) genericEvent;📝 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.
| public void delegate(GenericEvent genericEvent) { | |
| SlashCommandInteractionEvent event = (SlashCommandInteractionEvent) genericEvent; | |
| event.deferReply(true).setContent(DEFER_MESSAGE_ASSIGN_STUDY_ROLE).queue(); | |
| public void delegate(GenericEvent genericEvent) { | |
| if (!(genericEvent instanceof SlashCommandInteractionEvent)) { | |
| return; | |
| } | |
| SlashCommandInteractionEvent event = (SlashCommandInteractionEvent) genericEvent; | |
| event.deferReply(true).setContent(DEFER_MESSAGE_ASSIGN_STUDY_ROLE).queue(); | |
| } |
| commonDiscordService.assignDiscordStudyRole(currentMemberDiscordUsername, studyTitle, academicYear, semester); | ||
|
|
||
| event.getHook() | ||
| .sendMessage(REPLY_MESSAGE_ASSIGN_STUDY_ROLE) | ||
| .setEphemeral(true) | ||
| .queue(); |
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
예외 처리 로직이 필요합니다.
commonDiscordService.assignDiscordStudyRole 메서드 호출 시 발생할 수 있는 예외에 대한 처리가 없습니다. 서비스에서 발생한 예외를 적절히 처리하고 사용자에게 의미 있는 오류 메시지를 제공하는 것이 좋습니다.
- commonDiscordService.assignDiscordStudyRole(currentMemberDiscordUsername, studyTitle, academicYear, semester);
-
- event.getHook()
- .sendMessage(REPLY_MESSAGE_ASSIGN_STUDY_ROLE)
- .setEphemeral(true)
- .queue();
+ try {
+ commonDiscordService.assignDiscordStudyRole(currentMemberDiscordUsername, studyTitle, academicYear, semester);
+
+ event.getHook()
+ .sendMessage(REPLY_MESSAGE_ASSIGN_STUDY_ROLE)
+ .setEphemeral(true)
+ .queue();
+ } catch (Exception e) {
+ event.getHook()
+ .sendMessage("스터디 역할 부여 중 오류가 발생했습니다: " + e.getMessage())
+ .setEphemeral(true)
+ .queue();
+ }📝 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.
| commonDiscordService.assignDiscordStudyRole(currentMemberDiscordUsername, studyTitle, academicYear, semester); | |
| event.getHook() | |
| .sendMessage(REPLY_MESSAGE_ASSIGN_STUDY_ROLE) | |
| .setEphemeral(true) | |
| .queue(); | |
| try { | |
| commonDiscordService.assignDiscordStudyRole(currentMemberDiscordUsername, studyTitle, academicYear, semester); | |
| event.getHook() | |
| .sendMessage(REPLY_MESSAGE_ASSIGN_STUDY_ROLE) | |
| .setEphemeral(true) | |
| .queue(); | |
| } catch (Exception e) { | |
| event.getHook() | |
| .sendMessage("스터디 역할 부여 중 오류가 발생했습니다: " + e.getMessage()) | |
| .setEphemeral(true) | |
| .queue(); | |
| } |
| StudyV2 study = studyV2Repository | ||
| .findByTitleAndSemester(studyTitle, Semester.of(academicYear, SemesterType.valueOf(semester))) | ||
| .orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); |
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
Enum 변환 시 예외 처리 필요.
SemesterType.valueOf(semester) 호출 시 잘못된 학기 값이 전달되면 IllegalArgumentException이 발생할 수 있습니다. 이에 대한 예외 처리가 필요합니다.
다음과 같이 예외 처리를 추가하는 것이 좋습니다:
- StudyV2 study = studyV2Repository
- .findByTitleAndSemester(studyTitle, Semester.of(academicYear, SemesterType.valueOf(semester)))
- .orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
+ StudyV2 study;
+ try {
+ SemesterType semesterType = SemesterType.valueOf(semester);
+ study = studyV2Repository
+ .findByTitleAndSemester(studyTitle, Semester.of(academicYear, semesterType))
+ .orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
+ } catch (IllegalArgumentException e) {
+ throw new CustomException(INVALID_INPUT_VALUE, "유효하지 않은 학기 값입니다: " + semester);
+ }📝 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.
| StudyV2 study = studyV2Repository | |
| .findByTitleAndSemester(studyTitle, Semester.of(academicYear, SemesterType.valueOf(semester))) | |
| .orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | |
| StudyV2 study; | |
| try { | |
| SemesterType semesterType = SemesterType.valueOf(semester); | |
| study = studyV2Repository | |
| .findByTitleAndSemester(studyTitle, Semester.of(academicYear, semesterType)) | |
| .orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | |
| } catch (IllegalArgumentException e) { | |
| throw new CustomException(INVALID_INPUT_VALUE, "유효하지 않은 학기 값입니다: " + semester); | |
| } |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
uwoobeat
left a comment
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
kimsh1017
left a comment
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
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit