feat: 스터디 수료 필드와 우수 스터디원 테이블 추가 #782
Conversation
Walkthrough이 PR에서는 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
src/main/java/com/gdschongik/gdsc/domain/study/domain/AchievementType.java (1)
1-13: 문서화를 추가하여 코드 유지보수성을 향상시키는 것이 좋겠습니다.코드의 가독성과 유지보수성을 높이기 위해 클래스와 각 enum 상수에 대한 JavaDoc 주석을 추가하는 것이 좋겠습니다. 이는 다른 개발자들이 이 enum을 사용할 때 각 상수의 의미와 용도를 쉽게 이해할 수 있게 해줄 것입니다.
다음과 같이 JavaDoc을 추가할 수 있습니다:
package com.gdschongik.gdsc.domain.study.domain; import lombok.Getter; import lombok.RequiredArgsConstructor; +/** + * 스터디 업적 유형을 나타내는 열거형입니다. + * 각 상수는 특정 유형의 우수 스터디원을 나타냅니다. + */ @Getter @RequiredArgsConstructor public enum AchievementType { + /** 1차 우수 스터디원을 나타냅니다. */ FIRST_ROUND_OUTSTANDING_STUDENT("1차 우수 스터디원"), + /** 2차 우수 스터디원을 나타냅니다. */ SECOND_ROUND_OUTSTANDING_STUDENT("2차 우수 스터디원"); + /** 업적 유형의 한국어 설명입니다. */ private final String value; }src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyAchievement.java (2)
22-35: achievementType 필드에 대한 추가 정보가 필요합니다.필드들과 그에 대한 어노테이션들이 대체로 잘 정의되어 있습니다. 하지만 achievementType 필드에 대해 몇 가지 개선사항을 제안합니다:
- AchievementType이 어떤 타입인지 명확하지 않습니다. Enum인지 다른 엔티티인지 명시적으로 표현하면 좋겠습니다.
- 이 필드에 @Enumerated 어노테이션을 추가하는 것이 좋습니다 (만약 Enum이라면).
- 데이터베이스 컬럼 이름을 명시적으로 지정하는 것이 좋습니다.
다음과 같이 achievementType 필드를 수정하는 것을 고려해보세요:
- private AchievementType achievementType; + @Enumerated(EnumType.STRING) + @Column(name = "achievement_type") + private AchievementType achievementType;
44-50: 정적 팩토리 메서드가 잘 구현되었습니다.create 메서드가 StudyAchievement 인스턴스를 생성하는 명확한 방법을 제공하고 있습니다. 모든 필요한 매개변수를 받아 빌더를 통해 객체를 생성하고 있어 좋습니다.
한 가지 제안사항:
메서드의 가독성을 높이기 위해 다음과 같이 매개변수를 정렬하는 것을 고려해보세요:
public static StudyAchievement create(Member student, Study study, AchievementType achievementType) { return StudyAchievement.builder() - .student(student) - .study(study) - .achievementType(achievementType) - .build(); + .student(student) + .study(study) + .achievementType(achievementType) + .build(); }src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (2)
43-44: 새로운 필드가 적절히 추가되었습니다.
hasCompleted필드가 올바르게 추가되었습니다.@Comment어노테이션을 사용하여 데이터베이스 스키마에 설명을 추가한 것은 좋은 방식입니다.다음과 같이 필드 이름을 더 명확하게 변경하는 것을 고려해 보세요:
-private boolean hasCompleted; +private boolean isCompleted;이렇게 하면 "완료되었는가?"라는 의미가 더 명확해집니다.
69-74: 새로운complete()메서드가 적절히 추가되었습니다.
complete()메서드가 올바르게 구현되어hasCompleted필드를true로 설정합니다. 메서드 이름이 명확하고 간결합니다.다음과 같이 메서드 설명을 조금 더 상세하게 작성하는 것을 고려해 보세요:
/** - * 스터디 수료 + * 스터디를 수료 상태로 표시합니다. + * 이 메서드를 호출하면 hasCompleted 필드가 true로 설정됩니다. */이렇게 하면 메서드의 동작과 영향을 더 명확하게 설명할 수 있습니다.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AchievementType.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyAchievement.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (3 hunks)
Additional comments not posted (7)
src/main/java/com/gdschongik/gdsc/domain/study/domain/AchievementType.java (1)
1-13: 전반적인 구조와 구현이 잘 되어 있습니다.enum의 구조가 올바르고 Java 규칙을 잘 따르고 있습니다. Lombok 어노테이션을 적절히 사용하여 보일러플레이트 코드를 줄였습니다. enum 상수의 명명 규칙도 올바르게 대문자와 밑줄을 사용하고 있습니다.
value필드를 통해 각 업적 유형에 대한 한국어 설명을 제공하는 것도 좋습니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyAchievement.java (3)
1-20: 클래스 선언과 어노테이션이 적절합니다.클래스 선언과 사용된 어노테이션들이 JPA 엔티티에 대한 모범 사례를 잘 따르고 있습니다. @entity, @getter, @NoArgsConstructor 어노테이션의 사용이 적절합니다.
37-42: 생성자 구현이 적절합니다.private 생성자와 @builder 어노테이션의 사용이 객체 생성을 제어하는 좋은 방법입니다. 모든 필드가 올바르게 초기화되고 있습니다.
1-51: 전반적인 구현이 PR 목표와 잘 부합합니다.StudyAchievement 엔티티의 구현이 전반적으로 잘 되어 있으며, JPA와 Java의 모범 사례를 따르고 있습니다. 이 구현은 PR의 목표인 우수 스터디원 정보를 별도의 테이블에 저장하여 관리하는 것을 잘 달성하고 있습니다.
주요 장점:
- Study와 Member 엔티티와의 관계가 명확히 정의되어 있습니다.
- AchievementType을 통해 다양한 성취 유형을 구분할 수 있습니다.
- 객체 생성이 정적 팩토리 메서드를 통해 제어되고 있습니다.
개선을 위한 제안사항들을 반영하면, 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (3)
20-20: 새로운 import 문이 적절히 추가되었습니다.
@Comment어노테이션을 사용하기 위해 필요한 import 문이 올바르게 추가되었습니다.
50-50: 생성자가 적절히 업데이트되었습니다.새로운
hasCompleted필드가 생성자에서false로 초기화되어, 새로운StudyHistory객체가 생성될 때 완료되지 않은 상태로 시작하도록 올바르게 설정되었습니다.
Line range hint
1-82: 전체적인 변경 사항이 PR 목표와 잘 부합합니다.이 PR의 주요 목표인
StudyHistory에 수료 상태 필드를 추가하는 것이 성공적으로 구현되었습니다. 새로운hasCompleted필드와complete()메서드를 통해 스터디 완료 여부를 추적할 수 있게 되었습니다.변경 사항들은 잘 구조화되어 있고 좋은 코딩 관행을 따르고 있습니다. 특히
@Comment어노테이션을 사용하여 데이터베이스 스키마에 설명을 추가한 점이 좋습니다.다음 스크립트를 실행하여
StudyHistory클래스가 다른 곳에서 사용되는 방식을 확인해 주세요:이 스크립트를 통해
StudyHistory클래스와 새로 추가된complete()메서드가 다른 파일에서 어떻게 사용되는지 확인할 수 있습니다. 결과를 바탕으로 필요한 경우 관련 코드를 업데이트하세요.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
|
혹시 테스트는 어디까지 검증하도록 작성하시나요? |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
도메인 단위 테스트는 데이터 수정하는 메서드가 실행시 어떤 결과를 가져와야 하는지 정도로 작성하면 될 것 같아요. 그러고 보니 test 빠뜨렸네요,, |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryTest.java (3)
16-36: 테스트가 잘 작성되었습니다. 추가 테스트 케이스를 고려해보세요.
스터디_히스토리_생성시클래스의 테스트가 새로 생성된StudyHistory의 초기 상태를 올바르게 검증하고 있습니다. 다음과 같은 추가 테스트 케이스를 고려해보시는 것이 좋겠습니다:
- 다양한 기간의 스터디에 대한 테스트
- 잘못된 입력값으로
StudyHistory를 생성하려 할 때의 예외 처리 테스트getStudent()와getStudy()메서드의 반환값 검증이러한 추가 테스트를 통해
StudyHistory클래스의 동작을 더욱 철저히 검증할 수 있을 것입니다.
38-61: 테스트가 잘 작성되었습니다. 추가 테스트 케이스와 예외 처리를 고려해보세요.
스터디_수료시클래스의 테스트가complete()메서드의 동작을 올바르게 검증하고 있습니다. 다음과 같은 추가 테스트 케이스와 예외 처리를 고려해보시는 것이 좋겠습니다:
- 이미 수료된 스터디에 대해
complete()메서드를 다시 호출할 때의 동작 테스트- 스터디 기간이 끝나지 않았을 때
complete()메서드 호출 시의 예외 처리 테스트complete()메서드 호출 전후의 타임스탬프 검증 (만약 수료 시간을 기록하는 필드가 있다면)또한,
StudyHistory클래스에uncomplete()또는reset()같은 메서드가 있다면, 이에 대한 테스트도 추가하는 것이 좋겠습니다.이러한 추가 테스트를 통해
StudyHistory클래스의complete()메서드와 관련 기능을 더욱 철저히 검증할 수 있을 것입니다.
1-61: 전반적으로 잘 작성된 테스트입니다. 테스트 커버리지 개선을 고려해보세요.
StudyHistoryTest클래스는StudyHistory의 기본 기능을 잘 테스트하고 있습니다.FixtureHelper를 사용하여 테스트 데이터를 생성하는 방식이 코드의 재사용성과 가독성을 높이고 있습니다.다음과 같은 개선 사항을 고려해보시면 좋겠습니다:
- 경계값 테스트: 스터디 기간의 시작과 끝에 대한 테스트 추가
- 예외 처리 테스트: 잘못된 입력이나 상태에 대한 예외 처리 테스트 추가
- 메서드 커버리지 확대:
StudyHistory클래스의 다른 메서드들에 대한 테스트 추가- 파라미터화된 테스트: 다양한 시나리오를 커버하기 위한 파라미터화된 테스트 도입 고려
또한, 테스트 메서드 이름을 더 구체적으로 작성하여 각 테스트의 목적을 명확히 하는 것도 좋은 방법입니다.
이러한 개선을 통해
StudyHistory클래스의 동작을 더욱 철저히 검증하고, 향후 발생할 수 있는 버그를 사전에 방지할 수 있을 것입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (3 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java
🔇 Additional comments not posted (1)
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryTest.java (1)
1-15: LGTM: 임포트 및 설정이 적절합니다.필요한 클래스들이 올바르게 임포트되었으며,
FixtureHelper를 사용하여 테스트 데이터를 생성하는 방식이 효율적입니다.
| @@ -39,10 +40,14 @@ public class StudyHistory extends BaseEntity { | |||
|
|
|||
| private String repositoryLink; | |||
|
|
|||
| @Comment("수료 여부") | |||
| private Boolean hasCompleted; | |||
There was a problem hiding this comment.
boolean 대신 기존 컨벤션대로 enum을 사용하는 게 어떨까요?
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryStatus.java (2)
8-13: 열거형 선언이 명확하고 적절합니다.
StudyHistoryStatus열거형의 구조가 잘 정의되어 있습니다. 한국어 설명을 사용하여 가독성을 높인 점이 좋습니다.향후 국제화를 고려한다면, 문자열 값을 리소스 번들로 분리하는 것을 고려해 보세요. 예를 들어:
public enum StudyHistoryStatus { NONE("study.status.not_completed"), COMPLETED("study.status.completed"); private final String messageKey; // getter for messageKey }이렇게 하면 나중에 다국어 지원을 쉽게 추가할 수 있습니다.
1-13: 단위 테스트 추가 제안
StudyHistoryStatus열거형에 대한 단위 테스트를 추가하는 것이 좋겠습니다. 이는 PR 코멘트에서 언급된 테스트 커버리지 개선에 도움이 될 것입니다.테스트에서 다음 사항을 확인할 수 있습니다:
- 각 상수의
value필드가 올바른 값을 가지고 있는지valueOf()메서드가 예상대로 작동하는지- 열거형 상수의 개수가 예상과 일치하는지
예를 들어:
@Test void testStudyHistoryStatusValues() { assertEquals("미수료", StudyHistoryStatus.NONE.getValue()); assertEquals("수료", StudyHistoryStatus.COMPLETED.getValue()); } @Test void testStudyHistoryStatusValueOf() { assertEquals(StudyHistoryStatus.NONE, StudyHistoryStatus.valueOf("NONE")); assertEquals(StudyHistoryStatus.COMPLETED, StudyHistoryStatus.valueOf("COMPLETED")); } @Test void testStudyHistoryStatusCount() { assertEquals(2, StudyHistoryStatus.values().length); }이러한 테스트를 추가하면 코드의 신뢰성을 높이고 향후 변경 사항에 대한 안전성을 확보할 수 있습니다.
이러한 테스트 코드를 생성하거나 GitHub 이슈를 열어 이 작업을 추적하길 원하시나요?
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (1)
74-79:complete()메서드가 추가되었지만, 추가적인 검증이 필요할 수 있습니다.
complete()메서드가 추가되어 스터디를 완료 상태로 표시할 수 있게 된 것은 좋습니다. 하지만 현재 구현에는 몇 가지 개선의 여지가 있습니다:
- 이미 완료된 스터디를 다시 완료하는 것을 방지하기 위한 검증 로직이 없습니다.
- 스터디가 완료 가능한 상태인지 확인하는 로직이 없습니다 (예: 필요한 조건을 모두 충족했는지).
다음과 같이 메서드를 개선하는 것을 고려해보세요:
public void complete() { if (studyHistoryStatus == COMPLETED) { throw new IllegalStateException("이미 완료된 스터디입니다."); } // 추가적인 완료 조건 확인 로직 studyHistoryStatus = COMPLETED; }또한, 이 메서드에 대한 단위 테스트를 작성하여 다양한 시나리오를 검증하는 것이 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryStatus.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryTest.java
🔇 Additional comments (6)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryStatus.java (2)
1-5: 패키지 선언과 임포트가 적절합니다.패키지 구조가 도메인 구조를 잘 반영하고 있으며, Lombok 어노테이션의 사용도 적절합니다.
6-7: Lombok 어노테이션 사용이 적절합니다.
@Getter와@RequiredArgsConstructor어노테이션의 사용으로 불필요한 코드를 줄이고 열거형을 간결하게 만들었습니다. 이는 코드 유지보수성을 향상시킵니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (4)
3-3: import 문이 적절히 추가되었습니다.
StudyHistoryStatusenum,EnumType,Enumerated, 그리고Comment어노테이션에 대한 import 문이 올바르게 추가되었습니다. 이는 클래스의 변경 사항을 지원하는 데 필요합니다.Also applies to: 9-10, 24-24
47-49:studyHistoryStatus필드가 적절히 추가되었습니다.새로운
studyHistoryStatus필드가StudyHistoryStatusenum 타입으로 추가된 것은 좋은 선택입니다. 이는 이전 리뷰 코멘트에서 제안된 대로 boolean 대신 enum을 사용하여 구현되었습니다.
@Comment어노테이션을 사용하여 데이터베이스 스키마에 명확한 설명을 추가한 것도 좋습니다. 또한,@Enumerated(EnumType.STRING)을 사용하여 enum을 문자열로 저장하는 것은 데이터베이스에서의 가독성과 유지보수성을 향상시킵니다.
55-55: 생성자에서studyHistoryStatus가 적절히 초기화되었습니다.생성자에서
studyHistoryStatus를NONE으로 초기화한 것은 좋은 방식입니다. 이렇게 하면 새로운StudyHistory객체가 생성될 때 항상 일관된 초기 상태를 가지게 됩니다.
Line range hint
1-85: 전반적인 구현에 대한 의견
StudyHistory클래스의 변경사항은 전반적으로 잘 구현되었습니다. enum을 사용하여 스터디 완료 상태를 표현한 것은 좋은 선택입니다. 이는 원래 요구사항에서 언급된 boolean 필드 대신 사용되었지만, 더 유연하고 확장 가능한 솔루션을 제공합니다.하지만 다음 사항들을 고려해보시기 바랍니다:
- PR 목표와 실제 구현 사이의 차이점(boolean vs enum)에 대해 팀과 논의하고, 필요하다면 문서를 업데이트하세요.
complete()메서드에 대한 단위 테스트를 작성하여 다양한 시나리오를 검증하세요.StudyHistoryStatusenum의 정의를 제공하여 가능한 모든 상태를 명확히 하세요.전체적으로 이 변경사항은 스터디 완료 상태를 효과적으로 관리할 수 있는 좋은 기반을 제공합니다.
| @Builder(access = AccessLevel.PRIVATE) | ||
| private StudyHistory(Member student, Study study) { | ||
| this.student = student; | ||
| this.study = study; | ||
| this.studyHistoryStatus = NONE; |
There was a problem hiding this comment.
상용 배포할 때 NONE으로 초기화해주셔야 하는 거 리마드릴게용
There was a problem hiding this comment.
상용 배포할 때 NONE으로 초기화 해야한다는 게 무슨 뜻인지 잘 안 와닿아서 설명해주실 수 있나요?
상용 배포말고 다른 상황에서는 할 필요가 없다거나 하는 게 있는지 궁금해요
| @Getter | ||
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"member_id", "study_id", "achievement_type"})}) |
There was a problem hiding this comment.
achievement_type 까지 묶을 필요는 없지 않나요?
There was a problem hiding this comment.
우수 스터디원 설정할 때 따닥 발생할 수 있을 것 같아서요.
예를 들어 {1번 멤버, 1번 스터디에 1차 우수 스터디원} 지정한다고 할 때, {1번 멤버, 1번 스터디에 1차 우수 스터디원}은 유일해야 하니까요.
There was a problem hiding this comment.
네네 그러면 columnNames = {"member_id", "study_id",} 가 되어야 하지 않냐는 의미였습니다
There was a problem hiding this comment.
1차 우수 스터디원이 2차 우수 스터디원이 될수도 있는데요?
그럼 achievement_type도 있는게 맞지 않나요?
There was a problem hiding this comment.
아 both enum 같은 거 안넣기로 했었죠?
확인했습니다.
🌱 관련 이슈
📌 작업 내용 및 특이사항
StudyAchievement테이블을 추가했습니다.StudyAchievement이StudyHistory를 가질지, Study와 Member를 각각 가질지에 대한 포인트에서 고민이 있었습니다.우수 스터디원인지를 조회하는 일은 과제와 출석 테이블을 조회할 때 같이 발생하는데, 이 두가지가 모두 Study와 Member를 이용하므로
StudyAchievement도 Study와 Member를 각각 가지는것이 적합해보입니다.📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
AchievementType열거형 추가: 두 가지 상수FIRST_ROUND_OUTSTANDING_STUDENT및SECOND_ROUND_OUTSTANDING_STUDENT가 추가되어 성취 유형을 정의합니다.StudyAchievement클래스 추가: 학생, 연구 및 성취 유형을 포함하는 새로운 엔티티 클래스입니다.StudyHistory클래스 수정:studyHistoryStatus필드 추가로 연구 완료 여부를 추적할 수 있게 되었습니다.StudyHistoryStatus열거형 추가: 연구 상태를 나타내는 두 가지 상수NONE및COMPLETED가 정의되었습니다.버그 수정
complete()메서드 추가: 연구 완료 상태를 업데이트할 수 있는 기능이 추가되었습니다.