Feat: 동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api 구현#347
Merged
Feat: 동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api 구현#347
Conversation
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough클럽 조회 기능을 위한 V2 REST API 엔드포인트, 애플리케이션 서비스 계층, 포트 인터페이스, 저장소 쿼리 메서드를 추가합니다. JWT 검증을 통한 선택적 사용자 인증, 클럽 분류 및 구분 필터링, 구독 상태 확인, 모집 상태 계산 기능을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as ClubQueryApiV2
participant Service as ClubQueryService
participant JwtProvider as JwtTokenProvider
participant QueryPort as ClubQueryPort
participant SubPort as ClubSubscriptionQueryPort
participant UserPort as RootUserQueryPort
participant StoragePort as StoragePort
participant Repository as 저장소
Client->>Controller: GET /api/v2/clubs/{id}<br/>Authorization: Bearer token
activate Controller
Controller->>JwtProvider: 토큰 검증 및 이메일 추출
activate JwtProvider
JwtProvider-->>Controller: 사용자 이메일 (또는 null)
deactivate JwtProvider
Controller->>Service: getClubDetail(clubId, email)
deactivate Controller
activate Service
Service->>UserPort: email로 사용자 조회
activate UserPort
UserPort-->>Service: RootUser (Optional)
deactivate UserPort
Service->>QueryPort: 클럽 상세 조회 (id)
activate QueryPort
QueryPort->>Repository: findClubDetailById(id)
Repository-->>QueryPort: ClubDetailReadModel
QueryPort-->>Service: ClubDetailReadModel
deactivate QueryPort
alt 클럽 미존재
Service-->>Controller: CLUB_NOT_FOUND 예외
else 클럽 존재
Service->>SubPort: 구독자 수 계산 (clubId)
activate SubPort
SubPort->>Repository: countByClubId(clubId)
Repository-->>SubPort: 구독자 수
SubPort-->>Service: 구독자 수
deactivate SubPort
alt 사용자 로그인됨
Service->>SubPort: 구독 여부 확인 (userId, clubId)
activate SubPort
SubPort->>Repository: existsSubscription(...)
Repository-->>SubPort: boolean
SubPort-->>Service: 구독 여부
deactivate SubPort
else 사용자 미로그인
Service->>Service: isSubscribed = false
end
Service->>StoragePort: 포스터 이미지 서명 URL 생성
activate StoragePort
StoragePort-->>Service: 서명 URL (또는 null)
deactivate StoragePort
Service->>Service: 모집 상태 계산<br/>(isAlways, startAt, endAt, now)
Service->>Service: ClubDetailResult 생성
Service-->>Controller: ClubDetailResult
end
deactivate Service
activate Controller
Controller->>Controller: ClubDetailResponse 매핑
Controller-->>Client: BaseResponse<ClubDetailResponse> (HTTP 200)
deactivate Controller
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.java (1)
10-11:ClubDetailResult와category/division타입 불일치
ClubDetailResult는 동일한 필드에ClubCategory,ClubDivision도메인 열거형을 사용하지만,ClubItemResult는String을 사용합니다. 두 DTO가 같은 개념의 데이터를 나타내면서 타입이 다르면, 매핑 레이어에서 변환 실수가 발생하거나 API 응답 일관성이 깨질 수 있습니다. 통일된 표현 방식을 사용하거나, 의도적인 차이라면 주석으로 명시하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.java` around lines 10 - 11, ClubItemResult uses String for the category and division fields while ClubDetailResult uses the domain enums ClubCategory and ClubDivision, causing inconsistency; change the types of the category and division fields in ClubItemResult to ClubCategory and ClubDivision respectively, then update any mappers (e.g., where ClubItemResult is populated) and unit tests to pass/format the enums consistently (or add clear javadoc if the String choice was intentional) so both DTOs represent the same domain types.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (2)
43-75:mockReadModels필드에final선언 추가를 권장합니다.해당 필드는 초기화 후 변경되지 않으므로
final로 선언하는 것이 적절합니다.♻️ 제안된 변경
- private List<ClubReadModel> mockReadModels = + private final List<ClubReadModel> mockReadModels =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java` around lines 43 - 75, The field mockReadModels in ClubQueryServiceTest is immutable after initialization but not declared final; change its declaration to add the final modifier (i.e., make the List<ClubReadModel> mockReadModels field final) so the intent is explicit and the compiler enforces immutability of the reference; locate the field by the symbol mockReadModels in the ClubQueryServiceTest class and update its declaration accordingly.
112-113:countClubs목(mock)이 반환하는 값(2)이searchClubs반환 목록 크기(3)와 불일치합니다.
totalCount가 2이지만 실제로 3개 항목이 반환되는 설정은 현실적이지 않아 테스트 독자에게 혼동을 줄 수 있습니다.totalCount는 반환 목록 크기 이상이어야 의미가 일관됩니다.♻️ 제안된 변경
when(clubQueryPort.countClubs(category, divisionList)) - .thenReturn(2); + .thenReturn(3);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java` around lines 112 - 113, The test sets clubQueryPort.countClubs(...) to return 2 while searchClubs returns 3 items, which is inconsistent; update the mock return of countClubs in the test (the when(clubQueryPort.countClubs(category, divisionList)).thenReturn(...)) so that totalCount is at least the size of the list returned by ClubQueryService.searchClubs (e.g., change to 3) or alternatively reduce the mocked searchClubs result to match the count, ensuring totalCount >= returned list size.src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java (1)
42-83:@QueryProjection이 실제로 사용되지 않습니다.
ClubQueryRepositoryImpl.findClubDetailById는QClubDetailDto를 이용한 QueryDSL 프로젝션 대신Tuple방식으로ClubDetailDto를 직접 생성합니다. 또한recruitmentStatus는 DB 컬럼이 아닌 계산된 값이므로 QueryDSL 프로젝션으로 처리하는 것 자체가 불가능합니다. 불필요한QClubDetailDto생성을 방지하려면@QueryProjection을 제거하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java` around lines 42 - 83, Remove the unused QueryDSL projection by deleting the `@QueryProjection` annotation from the ClubDetailDto constructor declaration in ClubDetailDto; ensure no other code relies on QClubDetailDto (the repository ClubQueryRepositoryImpl.findClubDetailById constructs ClubDetailDto from Tuple), and remove any unused import of com.querydsl.core.annotations.QueryProjection or unused generated QClubDetailDto artifacts if present.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
200-220:start,end모두null이고isAlways = false일 때RECRUITING을 반환하는 것이 의도된 동작인지 확인하세요.모집 기간 정보가 전혀 없는 경우 기본값으로
RECRUITING을 반환하면 의도치 않게 모집 중으로 표시될 수 있습니다.CLOSED혹은 별도의UNKNOWN상태가 더 적합할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java` around lines 200 - 220, The current calculateRecruitmentStatus method returns RECRUITING when start and end are both null and isAlways is false; change this to return a safer default (e.g., introduce ClubRecruitmentStatus.UNKNOWN and return UNKNOWN) when start==null && end==null && Boolean.FALSE.equals(isAlways), update the ClubRecruitmentStatus enum to include UNKNOWN, adjust any callers/tests that expect RECRUITING for missing period data, and ensure calculateRecruitmentStatus still handles the existing branches (ALWAYS, BEFORE, CLOSED, RECRUITING) unchanged otherwise.src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
97-97:userToken파라미터가 사용되지 않습니다.
getClubDetail메서드에서userToken을 인자로 받지만 구현 내부에서 전혀 사용하지 않습니다. 향후 사용 예정이면// TODO주석을 추가하고, 그렇지 않다면 인터페이스와 함께 제거하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java` at line 97, getClubDetail 메서드의 userToken 파라미터가 사용되지 않으므로 사용자 토큰이 향후 필요하다면 ClubQueryService.getClubDetail(Long id, String userToken, Long loginUserId) 선언과 메서드 구현에 "// TODO: userToken will be used for X" 주석을 추가하고 호출부에 전달되는 값을 유지하되 사용 시점을 명시하세요; 필요 없다면 인터페이스와 구현에서 userToken 파라미터를 제거(메서드 시그니처 변경)하고, 관련 호출부들(및 테스트)을 찾아 모두 수정하여 컴파일 오류가 없도록 하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/kustacks/kuring/club/adapter/in/web/ClubQueryApiV2.java`:
- Around line 78-114: The JWT handling between getClubs and getClubDetail is
inconsistent; update getClubDetail to match getClubs by removing the thrown
InvalidStateException and only setting loginUserId when
jwtTokenProvider.validateToken(jwt) returns true (i.e., if bearerToken != null
then extractAuthorizationValue(...), call validateToken(jwt) and if valid set
loginUserId = Long.parseLong(jwtTokenProvider.getPrincipal(jwt)), otherwise
leave loginUserId null), so getClubDetail uses the same silent fallback behavior
as getClubs (unless you intend to require authentication—if so, make getClubs
throw instead).
- Line 82: The code calls Long.parseLong(jwtTokenProvider.getPrincipal(jwt))
(used to set loginUserId at least in ClubQueryApiV2) which can throw
NumberFormatException if the JWT subject isn't a numeric string; add handling by
validating or parsing safely: either change JwtTokenProvider.getPrincipal() to
return a Long (or an Optional<Long>) if you can guarantee numeric subjects, or
wrap the parse in a try/catch that catches NumberFormatException and translates
it into a controlled response (e.g., throw a custom authentication/validation
exception or return a 401/400) so the service does not propagate 500; apply the
same fix for the other occurrence at the second parse around line 113.
In
`@src/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDetailResponse.java`:
- Around line 28-58: The null-check in ClubDetailResponse.from is ineffective
because ClubQueryService.getClubDetail always returns a
ClubDetailResult.Location instance even when all location fields are null; fix
this in ClubQueryService.getClubDetail by changing the logic that constructs
ClubDetailResult.Location so it returns null when all constituent fields
(building, room, lon, lat) are null/absent, i.e., only create new
ClubDetailResult.Location(...) when at least one of those fields is non-null;
keep ClubDetailResponse.from as-is so it will receive a true null when there is
no location.
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 31-58: The cursor check in searchClubs currently uses
idAfterCursor(cursor) which only applies club.id.gt(Long.parseLong(cursor)),
causing missing rows when sort order is not by id; update idAfterCursor (or
create a new method used by searchClubs) to decode a composite cursor that
includes the last sort key and lastId (e.g. "lastKey|lastId") and produce a
boolean expression matching keyset pagination: for sortBy "name" produce
(club.name.gt(lastName).or(club.name.eq(lastName).and(club.id.gt(lastId)))) and
for "recruitEndAt" produce
(club.recruitEndAt.gt(lastEndAt).or(club.recruitEndAt.eq(lastEndAt).and(club.id.gt(lastId)))),
falling back to club.id.gt(lastId) for id-only sort; ensure searchClubs uses
getOrderSpecifiers(sortBy) and the new composite cursor predicate so ordering
and cursor logic align.
- Around line 180-183: The idAfterCursor method currently calls
Long.parseLong(cursor) without handling NumberFormatException; update
idAfterCursor(String cursor) to validate or safely parse the cursor (e.g.,
try-catch NumberFormatException or use a numeric-checked parse) and if parsing
fails return null (or an appropriate empty predicate) instead of letting the
exception propagate; ensure you keep the existing behavior of returning null for
null/"0" and use the parsed long value with club.id.gt(...) when parsing
succeeds.
- Around line 39-58: The projection is mapping club.posterImagePath into the
ClubReadModel's 4th constructor parameter (iconImageUrl), which is semantically
incorrect; pick one resolution and implement it consistently: either add an
iconImagePath field to the Club entity and use club.iconImagePath in the
QClubReadModel projection inside ClubQueryRepositoryImpl, or rename/change
ClubReadModel's 4th parameter to posterImageUrl and update
QClubReadModel/constructor usages accordingly so the projected field names and
entity fields match (references: ClubQueryRepositoryImpl, QClubReadModel,
club.posterImagePath, ClubReadModel constructor).
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.java`:
- Line 15: getClubDetail 메서드의 파라미터 중 userToken과 loginUserId가 중복되어 책임 경계가 모호합니다:
ClubQueryUseCase 인터페이스의 getClubDetail(Long id, String userToken, Long
loginUserId)에서 userToken이 단순 식별용이라면 userToken을 제거하고 loginUserId만 사용하도록 시그니처를
변경하고, 컨트롤러/시큐리티 레이어에서 토큰을 파싱해 loginUserId를 전달하도록 수정하세요; 만약 userToken이 외부 API 호출
등 별도 목적이라면 getClubDetail 선언에 해당 의도를 주석으로 명확히 남기고 getClubs(Long id, Long
loginUserId)와의 일관성을 위해 다른 메서드들도 같은 패턴(userToken 포함)으로 맞추거나 인터페이스 설계 문서를 업데이트하세요.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 55-82: The current mapping in ClubQueryService iterates
cursorBasedList.getContents() and calls clubQueryPort.countSubscribers(...) and
clubQueryPort.existsSubscription(...) per club causing N+1 queries; instead add
port methods to fetch subscriber counts and subscription existence in bulk
(e.g., clubQueryPort.countSubscribersByClubIds(List<Long> clubIds) returning a
Map<Id,Integer> and clubQueryPort.findSubscribedClubIdsByUser(Long userId,
List<Long> clubIds) returning a Set<Id>), call those once before the stream,
then construct each ClubItemResult using the pre-fetched counts and subscription
set (replace per-item calls to countSubscribers and existsSubscription with
lookups into the returned collections).
- Around line 109-133: The code in ClubQueryService always constructs a new
ClubDetailResult.Location even when all location fields are null, causing
ClubDetailResponse.from()'s result.location() to be non-null with all-null
fields; change the return construction in ClubQueryService so you only create
and pass a new ClubDetailResult.Location when at least one of dto.getBuilding(),
dto.getRoom(), dto.getLon(), dto.getLat() is non-null (otherwise pass null),
ensuring Location is null for clubs without location data.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 200-220: The current calculateRecruitmentStatus method returns
RECRUITING when start and end are both null and isAlways is false; change this
to return a safer default (e.g., introduce ClubRecruitmentStatus.UNKNOWN and
return UNKNOWN) when start==null && end==null && Boolean.FALSE.equals(isAlways),
update the ClubRecruitmentStatus enum to include UNKNOWN, adjust any
callers/tests that expect RECRUITING for missing period data, and ensure
calculateRecruitmentStatus still handles the existing branches (ALWAYS, BEFORE,
CLOSED, RECRUITING) unchanged otherwise.
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.java`:
- Around line 10-11: ClubItemResult uses String for the category and division
fields while ClubDetailResult uses the domain enums ClubCategory and
ClubDivision, causing inconsistency; change the types of the category and
division fields in ClubItemResult to ClubCategory and ClubDivision respectively,
then update any mappers (e.g., where ClubItemResult is populated) and unit tests
to pass/format the enums consistently (or add clear javadoc if the String choice
was intentional) so both DTOs represent the same domain types.
In
`@src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java`:
- Around line 42-83: Remove the unused QueryDSL projection by deleting the
`@QueryProjection` annotation from the ClubDetailDto constructor declaration in
ClubDetailDto; ensure no other code relies on QClubDetailDto (the repository
ClubQueryRepositoryImpl.findClubDetailById constructs ClubDetailDto from Tuple),
and remove any unused import of com.querydsl.core.annotations.QueryProjection or
unused generated QClubDetailDto artifacts if present.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Line 97: getClubDetail 메서드의 userToken 파라미터가 사용되지 않으므로 사용자 토큰이 향후 필요하다면
ClubQueryService.getClubDetail(Long id, String userToken, Long loginUserId) 선언과
메서드 구현에 "// TODO: userToken will be used for X" 주석을 추가하고 호출부에 전달되는 값을 유지하되 사용
시점을 명시하세요; 필요 없다면 인터페이스와 구현에서 userToken 파라미터를 제거(메서드 시그니처 변경)하고, 관련 호출부들(및 테스트)을
찾아 모두 수정하여 컴파일 오류가 없도록 하세요.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 43-75: The field mockReadModels in ClubQueryServiceTest is
immutable after initialization but not declared final; change its declaration to
add the final modifier (i.e., make the List<ClubReadModel> mockReadModels field
final) so the intent is explicit and the compiler enforces immutability of the
reference; locate the field by the symbol mockReadModels in the
ClubQueryServiceTest class and update its declaration accordingly.
- Around line 112-113: The test sets clubQueryPort.countClubs(...) to return 2
while searchClubs returns 3 items, which is inconsistent; update the mock return
of countClubs in the test (the when(clubQueryPort.countClubs(category,
divisionList)).thenReturn(...)) so that totalCount is at least the size of the
list returned by ClubQueryService.searchClubs (e.g., change to 3) or
alternatively reduce the mocked searchClubs result to match the count, ensuring
totalCount >= returned list size.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
257-268:getOrderSpecifiers에서recruitmentGroup헬퍼 메서드를 재사용하지 않음Lines 259-262의 inline CASE 표현식이 lines 277-282의
recruitmentGroup(now)메서드와 완전히 동일합니다.cursorCondition은recruitmentGroup을 재사용하지만,getOrderSpecifiers는 복제합니다.♻️ 제안된 리팩터
case "recruitEndDate" -> { - - var statusOrder = new CaseBuilder() - .when(club.recruitEndAt.isNull()).then(2) - .when(club.recruitEndAt.lt(now)).then(1) - .otherwise(0); - + var statusOrder = recruitmentGroup(now); yield new OrderSpecifier[]{ statusOrder.asc(), club.recruitEndAt.asc().nullsLast(), club.id.asc() }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java` around lines 257 - 268, The order specifier for "recruitEndDate" duplicates the CASE logic already implemented in recruitmentGroup(now); modify getOrderSpecifiers to call recruitmentGroup(now) instead of inlining the CaseBuilder: obtain the CaseBuilder/Expression from recruitmentGroup(now) (as used by cursorCondition) and use it as statusOrder, then yield the same OrderSpecifier array (statusOrder.asc(), club.recruitEndAt.asc().nullsLast(), club.id.asc()); this removes duplication and ensures both cursorCondition and getOrderSpecifiers share the same recruitmentGroup logic.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (1)
51-52:mockReadModels필드를final로 선언재할당되지 않는 테스트 픽스처 필드는
final로 선언하는 것이 관례입니다.♻️ 제안된 수정
- private List<ClubReadModel> mockReadModels = + private final List<ClubReadModel> mockReadModels =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java` around lines 51 - 52, The field mockReadModels in ClubQueryServiceTest should be declared final since it’s a test fixture that is never reassigned; update the declaration of mockReadModels to be private final List<ClubReadModel> mockReadModels to reflect immutability and follow test convention, ensuring no other code tries to reassign it.src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
44-65:null일 때 불필요한rootUserQueryPort.findRootUserByEmail(null)DB 호출 발생
getClubs와getClubDetail모두email == null인 경우에도findRootUserByEmail(null)을 호출합니다. 비로그인 사용자의 요청마다 불필요한 DB 조회가 발생합니다.♻️ 제안된 수정 (getClubs 및 getClubDetail 모두 동일하게 적용)
- Optional<RootUser> optionalRootUser = rootUserQueryPort.findRootUserByEmail(email); - Long loginUserId = optionalRootUser.map(RootUser::getId).orElse(null); + Long loginUserId = email != null + ? rootUserQueryPort.findRootUserByEmail(email) + .map(RootUser::getId) + .orElse(null) + : null;Also applies to: 107-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java` around lines 44 - 65, Avoid calling rootUserQueryPort.findRootUserByEmail when email is null: in getClubs (and mirror the same change in getClubDetail) guard the DB call with an email null check so you only call rootUserQueryPort.findRootUserByEmail(email) when email != null, otherwise keep loginUserId as null; locate the usage around Optional<RootUser> optionalRootUser = rootUserQueryPort.findRootUserByEmail(email) and the mapping to loginUserId (RootUser::getId) and modify control flow so the optional lookup is skipped for null email and the rest of the method uses the existing loginUserId variable unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 64-99: countSubscribersByClubIds and findSubscribedClubIds
currently load full ClubSubscribe entities (risking lazy-load of club and OOM)
and aggregate in memory; change to repository-level aggregation/projection
queries that return clubId and count/exists directly. Add methods on
clubSubscribeRepository (e.g., findSubscriberCountsByClubIds(List<Long> clubIds)
with a `@Query` selecting s.club.id AS clubId, COUNT(s) AS cnt GROUP BY s.club.id
and findSubscribedClubIdsByUser(List<Long> clubIds, Long loginUserId) with a
`@Query` selecting s.club.id where s.user.loginUserId = :loginUserId) and adapt
countSubscribersByClubIds and findSubscribedClubIds to call those repo methods
and convert the projection results into Map<Long,Integer> / Map<Long,Boolean>
without loading full entities.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 161-181: The cursor parsing in cursorCondition breaks when club
names contain '|' because it uses split("\\|") and then
Long.parseLong(parts[1]); update the parsing logic (not generateCursor) to
locate the last '|' via lastIndexOf('|'), extract the id as the substring after
that last delimiter and parse it to Long, and treat the prefix (name) as
everything before that last delimiter so names containing '|' are preserved and
parsing never attempts to parse non-numeric parts; adjust any null/empty checks
and exception handling accordingly in the method that currently performs
split("\\|") (cursorCondition).
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 200-204: 테스트 getClubs_withoutLogin에서 잘못된 메서드를 검증하고 있으므로
verify(clubQueryPort, never()).existsSubscription(...) 대신 로그인 없을 때 구독 관련 조회가
호출되지 않음을 검증하도록 변경하세요; 구체적으로 ClubQueryServiceTest의 getClubs_withoutLogin에서
verify(clubQueryPort, never()).findSubscribedClubIds(anyLong()) 를 추가하거나
existsSubscription 검증을 삭제하고 findSubscribedClubIds 호출 부재를 검증하여 비로그인 경로에서
findSubscribedClubIds가 호출되지 않음을 확인하십시오.
---
Duplicate comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Line 47: The code maps Club.posterImagePath into the 4th parameter of
ClubReadModel (which is semantically iconImageUrl) causing a mismatch used later
by ClubQueryService (r.getIconImageUrl()); fix by making the mapping explicit
and correct: either pass club.getIconImageUrl() into the ClubReadModel
constructor (or add a getIconImageUrl() on Club that returns posterImagePath if
that is the intended source), or adjust the ClubReadModel constructor/parameter
order so the field names match; update ClubQueryRepositoryImpl to use the
correct symbol (iconImageUrl/getIconImageUrl) instead of posterImagePath so
ClubQueryService.r.getIconImageUrl() remains valid.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 257-268: The order specifier for "recruitEndDate" duplicates the
CASE logic already implemented in recruitmentGroup(now); modify
getOrderSpecifiers to call recruitmentGroup(now) instead of inlining the
CaseBuilder: obtain the CaseBuilder/Expression from recruitmentGroup(now) (as
used by cursorCondition) and use it as statusOrder, then yield the same
OrderSpecifier array (statusOrder.asc(), club.recruitEndAt.asc().nullsLast(),
club.id.asc()); this removes duplication and ensures both cursorCondition and
getOrderSpecifiers share the same recruitmentGroup logic.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 44-65: Avoid calling rootUserQueryPort.findRootUserByEmail when
email is null: in getClubs (and mirror the same change in getClubDetail) guard
the DB call with an email null check so you only call
rootUserQueryPort.findRootUserByEmail(email) when email != null, otherwise keep
loginUserId as null; locate the usage around Optional<RootUser> optionalRootUser
= rootUserQueryPort.findRootUserByEmail(email) and the mapping to loginUserId
(RootUser::getId) and modify control flow so the optional lookup is skipped for
null email and the rest of the method uses the existing loginUserId variable
unchanged.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 51-52: The field mockReadModels in ClubQueryServiceTest should be
declared final since it’s a test fixture that is never reassigned; update the
declaration of mockReadModels to be private final List<ClubReadModel>
mockReadModels to reflect immutability and follow test convention, ensuring no
other code tries to reassign it.
Collaborator
Author
|
코드래빗 리뷰 확인해서 수정했습니다! 변경 사항
질문상세 조회 API 명세상 userToken을 함께 받도록 되어 있는데 현재 코드상에서는 사용되지 않고 있어서 이 파라미터를 제거해도 되는지 확인 부탁드립니다!! |
rlagkswn00
requested changes
Feb 23, 2026
Member
There was a problem hiding this comment.
고생하셨습니다 리뷰가 늦어져 죄송합니다! 코멘트 확인 후 답글 꼭 부탁드립니다~
전체적인 이야기를 해보자면 로직의 흐름은 잘 만들어진거 같으나, AI가 과도하게 영향을 끼쳐서 팀의 이전까지의 방향과 너무 다른 결로 작성하셨다는 생각이 드네요!!
-
팀 아키텍처에 맞지 않게 레이어, 객체 간 로직이 혼재되어 있다는 생각이 듭니다! 특히 비지니스 로직, 서비스의 중요 정책과 연관된 내용이 서비스 코드에 없다는 점이 가장 크게 느껴져요!
-
이전의 프로젝트에서 충분히 활용할 수 있는 점들도 활용이 안된 느낌입니다! 커서 관련 내용도 그러하고 전반적인 메서드 네이밍도 그렇구요! 이 또한 아마도 컨텍스트를 다 이해하지 못한 AI의 영향이지 않을까 싶어요..!! 활용하시는건 매우 좋지만 충분히 기능 기획과 팀과 동기화가 되길..!!
-
마지막으로 시간 걸리시더라도 테스트는 항상 꼭 작성해주세요! 만들어주신 서비스레이어 테스트도 좋지만 커버리지 만큼은 코어 흐름에 대한 테스트는 꼭..!!
+) AI를 사용하시는건 좋지만 단순 기능 구현도만 보지 말고 그 외적인 부분도 검토를 많이 해주세요!!
++) 전체 프로젝트 아키텍처와 이전까지의 객체지향 방향성을 조금 더 고민해주셨으면 합니다!
rlagkswn00
changed the title
Collaborator
Author
|
잘못 포함된 vectorstore.json 파일 제거하고 SonarQube 이슈 반영하여 테스트 코드를 개선했습니다! |
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/test/java/com/kustacks/kuring/acceptance/ClubStep.java (1)
43-57: 목록 응답 검증이 첫 번째 동아리에만 한정되어 회귀를 놓칠 수 있습니다.
data.clubs[0]만 확인하지 말고 전체 목록 필드 품질을 검증하는 쪽이 안전합니다.🔧 제안 수정
public static void 동아리_목록_조회_응답_확인(ExtractableResponse<Response> response) { assertAll( () -> assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()), () -> assertThat(response.jsonPath().getInt("code")).isEqualTo(200), () -> assertThat(response.jsonPath().getString("message")).isEqualTo(CLUB_LIST_SEARCH_SUCCESS.getMessage()), () -> assertThat(response.jsonPath().getList("data.clubs")).isNotEmpty(), - () -> assertThat(response.jsonPath().getLong("data.clubs[0].id")).isPositive(), - () -> assertThat(response.jsonPath().getString("data.clubs[0].name")).isNotBlank(), - () -> assertThat(response.jsonPath().getString("data.clubs[0].summary")).isNotBlank(), - () -> assertThat(response.jsonPath().getString("data.clubs[0].category")).isNotBlank(), - () -> assertThat(response.jsonPath().getString("data.clubs[0].division")).isNotBlank(), - () -> assertThat(response.jsonPath().getBoolean("data.clubs[0].isSubscribed")).isInstanceOf(Boolean.class), - () -> assertThat(response.jsonPath().getLong("data.clubs[0].subscriberCount")).isGreaterThanOrEqualTo(0) + () -> assertThat(response.jsonPath().getList("data.clubs.id", Long.class)).allMatch(id -> id != null && id > 0), + () -> assertThat(response.jsonPath().getList("data.clubs.name", String.class)).allMatch(v -> v != null && !v.isBlank()), + () -> assertThat(response.jsonPath().getList("data.clubs.summary", String.class)).allMatch(v -> v != null && !v.isBlank()), + () -> assertThat(response.jsonPath().getList("data.clubs.category", String.class)).allMatch(v -> v != null && !v.isBlank()), + () -> assertThat(response.jsonPath().getList("data.clubs.division", String.class)).allMatch(v -> v != null && !v.isBlank()), + () -> assertThat(response.jsonPath().getList("data.clubs.isSubscribed", Boolean.class)).allMatch(v -> v != null), + () -> assertThat(response.jsonPath().getList("data.clubs.subscriberCount", Integer.class)).allMatch(v -> v != null && v >= 0) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/acceptance/ClubStep.java` around lines 43 - 57, The test method 동아리_목록_조회_응답_확인 currently validates only data.clubs[0], risking regressions; update it to still assert the HTTP status and top-level code/message, then extract the full list via response.jsonPath().getList("data.clubs") and iterate over every club entry to assert each club's fields (id is positive, name/summary/category/division are not blank, isSubscribed is a Boolean, subscriberCount >= 0). Locate the method 동아리_목록_조회_응답_확인 in ClubStep and replace the single-index assertions with a loop (or stream) that performs the same checks against each element in the clubs list to ensure whole-list field quality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 49-54: findSubscribedClubIds currently calls
clubSubscribeRepository.findByClubIdInAndRootUserId even when clubIds is null or
empty; add an early guard in the findSubscribedClubIds method to return
Collections.emptyList() (or List.of()) when clubIds is null or
clubIds.isEmpty(), then call
clubSubscribeRepository.findByClubIdInAndRootUserId(clubIds, rootUserId)
otherwise; this mirrors the guard used elsewhere in the class and prevents
unnecessary repository calls or potential errors.
In `@src/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.java`:
- Around line 29-41: The test look_up_club_list only checks success/schema but
not that filters are applied; update the test to assert that the response from
동아리_목록_조회_요청(category, division) contains only clubs matching category
"academic" and whose division is one of "central" or "engineering" (use the
response shape returned by 동아리_목록_조회_응답_확인 or inspect the payload from
동아리_목록_조회_요청), and add an assertion that at least one matching club exists;
ensure you use the existing helper methods (동아리_목록_조회_요청 and 동아리_목록_조회_응답_확인) to
parse the response before applying these filter-specific assertions.
- Around line 55-57: The test extracts clubId directly from 동아리_목록_조회_응답 using
동아리_목록_조회_요청 and risks an index error when the list is empty; before assigning
Long clubId = 동아리_목록_조회_응답.jsonPath().getLong("data.clubs[0].id"), add an
assertion that the response contains at least one club (e.g., assert that
data.clubs size > 0 or that data.clubs is not empty) and fail with a clear
message if empty so the test reports an intended assertion failure instead of an
IndexOutOfBoundsException.
---
Nitpick comments:
In `@src/test/java/com/kustacks/kuring/acceptance/ClubStep.java`:
- Around line 43-57: The test method 동아리_목록_조회_응답_확인 currently validates only
data.clubs[0], risking regressions; update it to still assert the HTTP status
and top-level code/message, then extract the full list via
response.jsonPath().getList("data.clubs") and iterate over every club entry to
assert each club's fields (id is positive, name/summary/category/division are
not blank, isSubscribed is a Boolean, subscriberCount >= 0). Locate the method
동아리_목록_조회_응답_확인 in ClubStep and replace the single-index assertions with a loop
(or stream) that performs the same checks against each element in the clubs list
to ensure whole-list field quality.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.javasrc/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.javasrc/test/java/com/kustacks/kuring/acceptance/ClubStep.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java
rlagkswn00
requested changes
Mar 2, 2026
Member
rlagkswn00
left a comment
rlagkswn00
left a comment
There was a problem hiding this comment.
현재 내용 중 URL들에 대해서 정확하게 어떤 값이 반환되는건가요?? 로컬에서 확인해보셨는지요! 아마 PresignedUrl을 사용하지 않고서는 사용이 불가할겁니다! 이 부분확인해주시죠!
그리고 작업량이 점점 길어지고 많아지는데, 다음부터는 최소한 1개 API로는 쪼개서 작업하시죠 소속 조회는 사실 진작에 끝난건데 계속 다른거때문에 배포 못되고 있는게 신경이쓰이네요!
Collaborator
Author
|
로컬 테스트할 때 DB에 값 직접 넣어서 응답 구조만 확인했어서 Presigned URL을 적용 못했던 것 같습니다ㅜㅜ db에는 s3 key만 저장되니까 조회할 때 Presigned URL로 변환해서 반환하도록 수정하겠습니다!! 그리고 다음부터는 api 별로 pr 나눠서 올리도록 하겠습니당 |
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.java (2)
57-58:⚠️ Potential issue | 🟡 Minor상세 조회 준비 단계에서 빈 목록 방어가 없어 실패 원인이 흐려집니다.
clubId추출 전에 목록 응답 검증을 먼저 호출해 의도된 assertion 실패로 유도하는 편이 좋습니다.🔧 제안 수정안
void look_up_club_detail() { // given var 동아리_목록_조회_응답 = 동아리_목록_조회_요청("academic", "central,engineering"); + 동아리_목록_조회_응답_확인(동아리_목록_조회_응답); Long clubId = 동아리_목록_조회_응답.jsonPath().getLong("data.clubs[0].id");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.java` around lines 57 - 58, The test extracts clubId from 동아리_목록_조회_응답 without asserting the list is non-empty, which hides the real failure; before calling Long clubId = 동아리_목록_조회_응답.jsonPath().getLong(...), add an explicit assertion on 동아리_목록_조회_응답 (from 동아리_목록_조회_요청) that the "data.clubs" array exists and has size > 0 (or that "data.clubs[0]" is present) so the test fails with a clear assertion if the list is empty rather than throwing a misleading extraction error.
41-43:⚠️ Potential issue | 🟠 Major목록 조회 테스트가 division 필터 계약을 아직 검증하지 않습니다.
현재는 category만 검증하므로 division 필터가 무시돼도 테스트가 통과할 수 있습니다.
🔧 제안 수정안
+import static org.assertj.core.api.Assertions.assertThat; @@ // then 동아리_목록_조회_응답_확인(동아리_목록_조회_응답); 동아리_카테고리_필터_검증(동아리_목록_조회_응답, "academic"); + assertThat(동아리_목록_조회_응답.jsonPath().getList("data.clubs.division", String.class)) + .isNotEmpty() + .allMatch(v -> "central".equals(v) || "engineering".equals(v));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.java` around lines 41 - 43, 현재 테스트 only verifies category via 동아리_카테고리_필터_검증 so the division filter can be ignored; add an assertion that validates the division filter on the response. Update the test to call the division-check helper (e.g., 동아리_분과_필터_검증 or add a new assertion) with the response object (동아리_목록_조회_응답) and the expected division value used in the request, ensuring each returned club's division matches the requested division filter.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
76-100:⚠️ Potential issue | 🟠 Major동일 SNS 타입이 여러 건일 때 URL 선택이 비결정적이며 덮어쓰기가 발생합니다.
현재 구현은 같은 타입이 중복되면 마지막 값으로 덮어써지고, 정렬이 없어 “마지막” 자체도 DB 결과 순서에 의존합니다.
🔧 제안 수정안 (결정성 확보)
List<Tuple> tuples = queryFactory .select( club.id, club.name, @@ .from(club) .leftJoin(club.homepageUrls, clubSns) .where(club.id.eq(id)) + .orderBy(clubSns.id.asc()) .fetch(); @@ for (Tuple t : tuples) { ClubSnsType type = t.get(clubSns.type); String url = t.get(clubSns.url); if (type == null) continue; switch (type) { - case INSTAGRAM -> instagram = url; - case YOUTUBE -> youtube = url; - case ETC -> etc = url; + case INSTAGRAM -> { + if (instagram == null) instagram = url; + } + case YOUTUBE -> { + if (youtube == null) youtube = url; + } + case ETC -> { + if (etc == null) etc = url; + } } }Also applies to: 112-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java` around lines 76 - 100, The query that joins club.homepageUrls (clubSns) can return multiple rows for the same clubSns.type and currently the code implicitly uses the “last seen” value which is non-deterministic; update the QueryDSL query in ClubQueryRepositoryImpl to enforce a deterministic ordering (e.g., add .orderBy(clubSns.type.asc(), clubSns.id.asc() or clubSns.createdAt.desc() if available) to the query that builds tuples) and adjust the mapping logic that reduces those tuples into the Club DTO to explicitly choose the intended record per SNS type (e.g., pick the first entry per type instead of letting later rows overwrite earlier ones); apply this change in both the select/fetch block (the code around queryFactory.select(...).from(club).leftJoin(club.homepageUrls, clubSns).where(...).fetch()) and the similar block later (the section around lines 112-123) so selection is deterministic and duplicates do not silently overwrite.
🧹 Nitpick comments (3)
src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java (1)
106-124:findSubscribedClubIds의 null/빈 입력 가드 케이스도 테스트로 고정해두면 좋겠습니다.현재는 성공 경로만 검증하고 있어 가드 회귀를 놓치기 쉽습니다.
🧪 제안 테스트 추가
`@Test` `@DisplayName`("findSubscribedClubIds는 구독된 clubId 목록을 반환한다") void findSubscribedClubIds_success() { @@ verify(clubSubscribeRepository).findByClubIdInAndRootUserId(clubIds, rootUserId); } + + `@Test` + `@DisplayName`("findSubscribedClubIds는 null 또는 빈 리스트면 빈 리스트를 반환한다") + void findSubscribedClubIds_empty_input() { + // when + List<Long> nullResult = adapter.findSubscribedClubIds(null, 100L); + List<Long> emptyResult = adapter.findSubscribedClubIds(List.of(), 100L); + + // then + assertThat(nullResult).isEmpty(); + assertThat(emptyResult).isEmpty(); + verify(clubSubscribeRepository, never()).findByClubIdInAndRootUserId(any(), any()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java` around lines 106 - 124, Add unit tests in ClubPersistenceAdapterTest to cover guard cases for findSubscribedClubIds: write one test where clubIds is null and another where clubIds is empty (and optionally a test where rootUserId is null if the method should guard that), call adapter.findSubscribedClubIds(...) and assert the result is empty (e.g., emptyList) and that clubSubscribeRepository.findByClubIdInAndRootUserId is never invoked; reference the existing test pattern (findSubscribedClubIds_success, clubSubscribeRepository) to mock/verify behavior and ensure these guard-case tests fail if the adapter regresses.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (1)
144-161: 아이콘 URL 변환 결과 검증 누락Line 144-145에서
storagePort.getPresignedUrl을 mock하고, Line 160에서 호출 횟수를 검증하고 있지만, 실제 결과 객체의iconImageUrl이 "presigned-icon-url"로 설정되었는지에 대한 assertion이 없습니다.💚 제안된 개선
// then assertThat(result.clubs()).hasSize(3); assertThat(result.clubs().get(0).subscriberCount()).isEqualTo(10); assertThat(result.clubs().get(0).isSubscribed()).isTrue(); + assertThat(result.clubs().get(0).iconImageUrl()).isEqualTo("presigned-icon-url"); verify(rootUserQueryPort).findRootUserByEmail(email);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java` around lines 144 - 161, The test is missing assertions that the returned ClubListResult contains the presigned icon URL; after calling clubQueryService.getClubs (in ClubQueryServiceTest) and mocking storagePort.getPresignedUrl to return "presigned-icon-url", add assertions on result.clubs() (e.g., for each club or at least the first club) to assert that their iconImageUrl (or the exact DTO field used for the icon URL) equals "presigned-icon-url" so the mapping from storagePort.getPresignedUrl(...) into the returned Club DTO is verified.src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
69-75: 빈 목록에 대한 조기 반환 고려
clubReadModels가 비어있을 경우, 불필요하게countSubscribersByClubIds와getSubscribedMap이 빈 리스트로 호출됩니다. 조기 반환을 추가하면 불필요한 쿼리를 방지할 수 있습니다.♻️ 제안된 개선
List<Long> clubIds = clubReadModels.stream() .map(ClubReadModel::getId) .toList(); + + if (clubIds.isEmpty()) { + return new ClubListResult(List.of()); + } Map<Long, Long> subscriberCountMap = clubSubscriptionQueryPort.countSubscribersByClubIds(clubIds);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java` around lines 69 - 75, When clubReadModels is empty you should return early to avoid calling clubSubscriptionQueryPort.countSubscribersByClubIds and getSubscribedMap with an empty list; add a guard after building clubIds (e.g., if clubIds.isEmpty()) that returns the appropriate empty result (or default response) instead of proceeding to call countSubscribersByClubIds and getSubscribedMap, so subscriberCountMap and subscribedMap are not populated with unnecessary queries for an empty input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailReadModel.java`:
- Around line 84-88: The hasLocation() method currently treats empty strings as
present; update it to consider building and room absent when they are null or
blank by checking e.g. building != null && !building.trim().isEmpty() and
similarly for room, while keeping the existing null checks for lon and lat (lon
!= null || lat != null); modify the hasLocation() boolean expression in the
ClubDetailReadModel.hasLocation method accordingly so only non-blank
building/room or non-null lon/lat return true.
---
Duplicate comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 76-100: The query that joins club.homepageUrls (clubSns) can
return multiple rows for the same clubSns.type and currently the code implicitly
uses the “last seen” value which is non-deterministic; update the QueryDSL query
in ClubQueryRepositoryImpl to enforce a deterministic ordering (e.g., add
.orderBy(clubSns.type.asc(), clubSns.id.asc() or clubSns.createdAt.desc() if
available) to the query that builds tuples) and adjust the mapping logic that
reduces those tuples into the Club DTO to explicitly choose the intended record
per SNS type (e.g., pick the first entry per type instead of letting later rows
overwrite earlier ones); apply this change in both the select/fetch block (the
code around queryFactory.select(...).from(club).leftJoin(club.homepageUrls,
clubSns).where(...).fetch()) and the similar block later (the section around
lines 112-123) so selection is deterministic and duplicates do not silently
overwrite.
In `@src/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.java`:
- Around line 57-58: The test extracts clubId from 동아리_목록_조회_응답 without
asserting the list is non-empty, which hides the real failure; before calling
Long clubId = 동아리_목록_조회_응답.jsonPath().getLong(...), add an explicit assertion on
동아리_목록_조회_응답 (from 동아리_목록_조회_요청) that the "data.clubs" array exists and has size
> 0 (or that "data.clubs[0]" is present) so the test fails with a clear
assertion if the list is empty rather than throwing a misleading extraction
error.
- Around line 41-43: 현재 테스트 only verifies category via 동아리_카테고리_필터_검증 so the
division filter can be ignored; add an assertion that validates the division
filter on the response. Update the test to call the division-check helper (e.g.,
동아리_분과_필터_검증 or add a new assertion) with the response object (동아리_목록_조회_응답) and
the expected division value used in the request, ensuring each returned club's
division matches the requested division filter.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 69-75: When clubReadModels is empty you should return early to
avoid calling clubSubscriptionQueryPort.countSubscribersByClubIds and
getSubscribedMap with an empty list; add a guard after building clubIds (e.g.,
if clubIds.isEmpty()) that returns the appropriate empty result (or default
response) instead of proceeding to call countSubscribersByClubIds and
getSubscribedMap, so subscriberCountMap and subscribedMap are not populated with
unnecessary queries for an empty input.
In
`@src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java`:
- Around line 106-124: Add unit tests in ClubPersistenceAdapterTest to cover
guard cases for findSubscribedClubIds: write one test where clubIds is null and
another where clubIds is empty (and optionally a test where rootUserId is null
if the method should guard that), call adapter.findSubscribedClubIds(...) and
assert the result is empty (e.g., emptyList) and that
clubSubscribeRepository.findByClubIdInAndRootUserId is never invoked; reference
the existing test pattern (findSubscribedClubIds_success,
clubSubscribeRepository) to mock/verify behavior and ensure these guard-case
tests fail if the adapter regresses.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 144-161: The test is missing assertions that the returned
ClubListResult contains the presigned icon URL; after calling
clubQueryService.getClubs (in ClubQueryServiceTest) and mocking
storagePort.getPresignedUrl to return "presigned-icon-url", add assertions on
result.clubs() (e.g., for each club or at least the first club) to assert that
their iconImageUrl (or the exact DTO field used for the icon URL) equals
"presigned-icon-url" so the mapping from storagePort.getPresignedUrl(...) into
the returned Club DTO is verified.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubListCommand.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailReadModel.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.javasrc/test/java/com/kustacks/kuring/acceptance/ClubAcceptanceTest.javasrc/test/java/com/kustacks/kuring/acceptance/ClubStep.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java
- src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java
rlagkswn00
approved these changes
Mar 4, 2026
This was referenced Mar 18, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 이슈
#336
📌 요약
동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api를 구현하고 관련 테스트도 추가하였습니다.
🛠️ 상세
GET /api/v2/clubs/divisions
GET /api/v2/clubs
GET /api/v2/clubs/{id}
💬 기타
Summary by CodeRabbit
릴리스 노트
신규 기능
버그 수정