Conversation
훑어보기동아리 관리 도메인을 새로 추가합니다: Club 엔티티와 ClubSns·ClubSubscribe 연관, ClubCategory·ClubDivision·ClubSnsType 열거형, 관련 에러 코드, DB 마이그레이션, 그리고 fromName 검증 유닛 테스트가 포함됩니다. 변경 사항
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jiyun921
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/kustacks/kuring/club/domain/Club.java`:
- Around line 37-41: The Club entity's enum fields category and division lack
JPA non-null constraints; update the Club class by adding `@Column`(nullable =
false) to the category and division fields (the fields named category and
division in class Club) so the JPA mapping matches the migration schema and
enforces NOT NULL at the entity level.
🧹 Nitpick comments (8)
src/main/resources/db/migration/V260206__Create_club_table.sql (2)
1-22:category,division컬럼에 인덱스 추가를 고려해 주세요.동아리 목록을 카테고리나 소속별로 필터링하는 API가 추가될 가능성이 높습니다. 해당 컬럼에 인덱스가 없으면 테이블 풀 스캔이 발생할 수 있습니다.
CREATE INDEX idx_club_category ON club (category); CREATE INDEX idx_club_division ON club (division);
13-14:x,y컬럼명이 의미를 전달하지 못합니다.좌표를 나타내는 컬럼이라면
longitude/latitude같은 명시적인 이름이 스키마 가독성에 도움이 됩니다. 이미 초기 마이그레이션 단계이므로 지금 변경하는 것이 가장 비용이 적습니다. (JPA 엔티티의@Column(name = ...)도 함께 수정 필요)src/main/java/com/kustacks/kuring/club/domain/ClubCategory.java (2)
22-42:NAME_MAP을Map<String, ClubCategory>로 단순화하세요.현재
Map<String, String>으로 enum 상수 이름(문자열)을 저장한 후 다시valueOf()를 호출하고 있습니다. 중간 단계 없이 enum 인스턴스를 직접 매핑하면 더 간결하고 타입 안전합니다.♻️ 제안하는 수정 사항
- private static final Map<String, String> NAME_MAP; + private static final Map<String, ClubCategory> NAME_MAP; static { NAME_MAP = Collections.unmodifiableMap(Arrays.stream(ClubCategory.values()) - .collect(Collectors.toMap(ClubCategory::getName, ClubCategory::name)) + .collect(Collectors.toMap(ClubCategory::getName, cc -> cc)) ); } ... public static ClubCategory fromName(String name) { - String findName = Optional.ofNullable(NAME_MAP.get(name)) + return Optional.ofNullable(NAME_MAP.get(name)) .orElseThrow(() -> new NotFoundException(CAT_NOT_EXIST_CLUB_CATEGORY)); - return ClubCategory.valueOf(findName); }
30-30:name필드가Enum.name()을 섀도잉합니다.
java.lang.Enum의name필드와 이름이 겹쳐서name()vsgetName()호출 결과가 달라집니다 (ACADEMICvsacademic). 기능적으로 문제는 없지만, 유지보수 시 혼동의 여지가 있으므로apiName등으로 필드명을 변경하는 것을 고려해 주세요.src/test/java/com/kustacks/kuring/club/domain/ClubCategoryTest.java (1)
10-11:Assertions대신AssertionsForClassTypes를 직접 import 하고 있습니다.일반적으로
org.assertj.core.api.Assertions를 사용하는 것이 더 관용적이며, Iterable/Collection 관련 assertion도 함께 사용할 수 있습니다. 기능상 문제는 없으나 참고 사항입니다.src/main/java/com/kustacks/kuring/club/domain/Club.java (1)
18-73: public 생성자 또는 팩토리 메서드가 없습니다.현재
@NoArgsConstructor(access = AccessLevel.PROTECTED)만 존재하여, 애플리케이션 코드에서Club인스턴스를 생성할 방법이 없습니다. 초기 구현이므로 이후 기능 개발 시 생성자/빌더 추가가 필요할 것입니다.src/main/java/com/kustacks/kuring/club/domain/ClubDivision.java (2)
36-56:NAME_MAP을Map<String, ClubDivision>으로 변경하여 이중 조회를 제거할 수 있습니다.현재
NAME_MAP이String → String(lowercase name → enum constant name)으로 매핑한 뒤,fromName에서 다시valueOf로 변환하고 있어 불필요한 이중 조회가 발생합니다. 직접ClubDivision을 값으로 저장하면 코드가 더 간결해집니다.♻️ 제안하는 리팩터링
- private static final Map<String, String> NAME_MAP; + private static final Map<String, ClubDivision> NAME_MAP; static { NAME_MAP = Collections.unmodifiableMap(Arrays.stream(values()) - .collect(Collectors.toMap(ClubDivision::getName, ClubDivision::name)) + .collect(Collectors.toMap(ClubDivision::getName, v -> v)) ); } public static ClubDivision fromName(String name) { - String findName = Optional.ofNullable(NAME_MAP.get(name)) + return Optional.ofNullable(NAME_MAP.get(name)) .orElseThrow(() -> new NotFoundException(CAT_NOT_EXIST_CLUB_DIVISION)); - return ClubDivision.valueOf(findName); }
44-44:name필드가Enum.name()과 혼동될 수 있습니다.Java의
Enum클래스에는 이미 상수 이름을 반환하는name()메서드가 있습니다. 필드명으로name을 사용하면getName()(Lombok 생성, "central" 반환)과name()("CENTRAL" 반환)이 공존하게 되어 유지보수 시 혼동을 유발할 수 있습니다.externalName등 보다 구별되는 이름을 고려해 보세요.
| @Column(length = 30, nullable = false) | ||
| private String summary; | ||
|
|
||
| @Lob |
There was a problem hiding this comment.
이거 Lob 먹일정도로 content가 길어질까요?
내부적으로 CLOB or BLOB의 대용량 오브젝트를 사용하는데 이정도로 긴 내용을 작성하나 해서요~
추가로 Column 에노테이션도 필요해 보입니다~!
| @Column(length = 30) | ||
| private String room; | ||
|
|
||
| private Double x; |
There was a problem hiding this comment.
몰라서 그러는데 x랑 y는 뭔가요?? 위경도 인가요?
지나가는 동료 입장에서 x, y 변수명은 한눈에 이해하기 어려운것 같습니다!
There was a problem hiding this comment.
위도, 경도인데 x,y는 명확하지 않은것 같아서 lat, lon으로 수정했습니다!
| ) | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class ClubBookmark { |
There was a problem hiding this comment.
아마 Club 과 Book 간의 중간 테이블을 활용한 N:M 구조 인것 같은데,
북마크에 많은 기능이 있지 않다면, 그냥 Club 에서 userId 를 리스트로 즉, 간접참조를 적용해봐도 좋을것 같은데,
판단은 지윤님이 원하는 방향으로 하는거로!~~
다만 "간접참조"와 "직접참조"에 대하여는 한번 공부해보시고 넘어갔으면 좋겠습니다!
There was a problem hiding this comment.
아하 넵!! 생각해봤는데 북마크한 동아리 목록 조회나 모집 일정 푸시 알림처럼 특정 사용자 기준으로 조회하는 기능 때문에 직접참조로 하면 특정 user_id에 해당하는 row를 바로 찾을 수 있는데 간접참조로는 모든 동아리를 순회하면서 userId가 포함되어 있는지 확인하는 구조가 될 것 같아서 직접참조가 좋지 않을까 생각했습니다! 괜찮을까유??
| assertThat(result).isEqualTo(clubCategory); | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 name으로 ClubCategory를 찾으려 하면 예외가 발생한다") |
rlagkswn00
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/kustacks/kuring/club/domain/Club.java`:
- Around line 20-77: The Club entity only has a protected no-args constructor so
callers cannot create instances; add a creation API such as Lombok's `@Builder`
(or a public static factory) to the Club class so callers can construct objects
with its many fields. Specifically, annotate the Club class with `@Builder` (and
optionally `@AllArgsConstructor`) or implement a public static factory method
(e.g., Club.create(...)) that accepts the necessary fields (name, summary,
category, division, etc.) and sets defaults like homepageUrls = new
ArrayList<>() and isAlways = false; update places that construct Club (similar
to ClubSubscribe) to use the new builder/factory.
In `@src/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.java`:
- Around line 24-38: ClubSubscribe lacks a public constructor to create and
persist a new subscription: add a public constructor (or static factory) that
accepts Club and User and assigns them to the club and user fields (ensuring
JPA-compatible behavior), e.g. a public ClubSubscribe(Club club, User user) that
sets this.club and this.user; apply the same fix to the ClubSns entity (add a
public constructor or static factory accepting the appropriate associated
entities) so new instances can be created and persisted.
🧹 Nitpick comments (2)
src/main/java/com/kustacks/kuring/club/domain/Club.java (1)
47-48:homepageUrls필드명이 실제 의미와 맞지 않습니다.이 필드는
ClubSns(Instagram, YouTube 등) 목록인데,homepageUrls라는 이름은 홈페이지 URL을 의미합니다.snsLinks또는snsAccounts같은 이름이 더 적절합니다.src/main/resources/db/migration/V260206__Create_club_table.sql (1)
36-39:idx_club_subscribe_club(club_id)인덱스가 중복됩니다.
uk_club_user(club_id, user_id)유니크 제약조건이 이미club_id를 선두 컬럼으로 포함하고 있어, MySQL은club_id단독 조회 시 해당 인덱스를 사용할 수 있습니다.idx_club_subscribe_club은 불필요한 중복 인덱스입니다.♻️ 중복 인덱스 제거 제안
CONSTRAINT uk_club_user UNIQUE (club_id, user_id), - INDEX idx_club_subscribe_user (user_id), - INDEX idx_club_subscribe_club (club_id) + INDEX idx_club_subscribe_user (user_id)
|
리뷰랑 기획 변경 사항 반영했습니다!
|
rlagkswn00
left a comment
rlagkswn00
left a comment
There was a problem hiding this comment.
고생하셨습니다! 한가지 네이밍 관련해서 이야기 드려요!
추가로 코드래빗이 AI툴이긴 하지만,,리뷰 달리면 꼭 건바건 해결해주세요! 나름 답글 달면 답변도 해준답니다? 딱히 대응 안해도 되는거면 그냥 resolve 누르셔도 되구!
그리구 맨날 보고있는게 아니기 때무네,,,수정하시면 디코나 카톡으로 이야기 해주세요!!
기왕이면 소통은 과하게!! 투머치하게!!
| CAT_NOT_EXIST_CLUB_CATEGORY(HttpStatus.BAD_REQUEST, "서버에서 지원하지 않는 동아리 카테고리입니다."), | ||
| CAT_NOT_EXIST_CLUB_DIVISION(HttpStatus.BAD_REQUEST, "서버에서 지원하지 않는 동아리 소속입니다."), |
There was a problem hiding this comment.
아마도 여기 있는 CAT은 공지사항 카테고리(학사, 국제, 장학 등)이런거 같은데 네이밍이 적절치는 않은거 같습니다!
There was a problem hiding this comment.
넵 !! Club 관련 이름으로 수정했습니다!
3 participants
#️⃣ 이슈
#336
📌 요약
🛠️ 상세
💬 기타
Summary by CodeRabbit
새로운 기능
테스트
Chores