Conversation
|
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구독한 동아리 목록 조회 API와 이를 지원하는 쿼리/포트/퍼시스턴스 메서드가 추가 및 리네이밍되었습니다. 저장소 레이어에서 클럽 일괄 조회와 사용자별 구독 ID 조회가 추가되고, 서비스·컨트롤러·테스트가 이를 사용하도록 확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "사용자"
participant Controller as "UserClubSubscriptionApiV2"
participant Auth as "인증(토큰 검사)"
participant QueryService as "ClubQueryService"
participant RootUserPort as "RootUserPort"
participant SubPort as "ClubSubscriptionQueryPort"
participant ClubPort as "ClubQueryPort"
participant DB as "데이터베이스"
User->>Controller: GET /api/v2/users/subscriptions/clubs (Bearer JWT)
Controller->>Auth: 토큰 검증 및 이메일 추출
Auth-->>Controller: 이메일
Controller->>QueryService: getSubscribedClubs(SubscribedClubListCommand)
QueryService->>RootUserPort: findByEmail(email)
RootUserPort->>DB: SELECT * FROM root_user WHERE email = ?
DB-->>RootUserPort: RootUser
RootUserPort-->>QueryService: RootUser
QueryService->>SubPort: findSubscribedClubIdsByRootUserId(rootUserId)
SubPort->>DB: SELECT club_id FROM club_subscribe WHERE root_user_id = ?
DB-->>SubPort: List<Long> clubIds
SubPort-->>QueryService: clubIds
QueryService->>ClubPort: findClubReadModelsByIds(clubIds)
ClubPort->>DB: SELECT ... FROM club WHERE id IN (...)
DB-->>ClubPort: List<ClubReadModel>
ClubPort-->>QueryService: ClubReadModel 리스트
QueryService->>SubPort: countSubscribersByClubIds(clubIds)
SubPort->>DB: SELECT club_id, COUNT(*) ... WHERE club_id IN (...)
DB-->>SubPort: 구독자수 맵
SubPort-->>QueryService: subscriber counts
QueryService-->>Controller: ClubListResult (isSubscribed=true)
Controller-->>User: 200 OK, BaseResponse<ClubListResponse>
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)
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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/test/java/com/kustacks/kuring/acceptance/UserStep.java (1)
146-152: 응답 검증이 너무 느슨합니다.지금은
containsAll(clubIds)라서 API가 요청한 구독 목록 외의 동아리를 더 내려줘도 테스트가 통과합니다. 이 케이스는 새 엔드포인트 계약을 고정하는 테스트이므로, ID 집합을 정확히 비교하고 이번 PR에서 추가한 성공 메시지도 함께 확인하는 편이 좋습니다.검증 강도 보강 예시
public static void 구독한_동아리_목록_조회_응답_확인(ExtractableResponse<Response> response, List<Long> clubIds) { assertAll( () -> assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()), () -> assertThat(response.jsonPath().getInt("code")).isEqualTo(200), - () -> assertThat(response.jsonPath().getList("data.clubs.id", Long.class)).containsAll(clubIds), - () -> assertThat(response.jsonPath().getList("data.clubs.isSubscribed")).containsOnly(true) + () -> assertThat(response.jsonPath().getString("message")).isEqualTo("구독한 동아리 목록 조회에 성공하였습니다"), + () -> assertThat(response.jsonPath().getList("data.clubs.id", Long.class)).containsExactlyInAnyOrderElementsOf(clubIds), + () -> assertThat(response.jsonPath().getList("data.clubs.isSubscribed", Boolean.class)) + .hasSize(clubIds.size()) + .containsOnly(true) ); }🤖 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/UserStep.java` around lines 146 - 152, The test method 구독한_동아리_목록_조회_응답_확인 uses containsAll which allows extra clubs; replace that with an exact ID comparison (e.g., assert that the response list of IDs is exactly the expected set/collection from clubIds, using a containsExactlyInAnyOrder / equals assertion) and also add an assertion that the response message equals the new success message introduced in this PR (check response.jsonPath().getString("message") against the expected success text).src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java (1)
43-46: null/empty 체크 일관성 검토기존
findSubscribedClubIds(lines 54-60)와countSubscribersByClubIds(lines 78-80)는 null/empty 리스트에 대한 방어 로직이 있지만, 새로 추가된findClubsByIds는 없습니다. 현재 caller가 empty 체크를 하지만, 일관성을 위해 동일한 패턴 적용을 고려해 볼 수 있습니다.🤖 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/ClubPersistenceAdapter.java` around lines 43 - 46, findClubsByIds lacks the same null/empty input guard used by findSubscribedClubIds and countSubscribersByClubIds; update ClubPersistenceAdapter.findClubsByIds to defensively handle a null or empty ids list (return Collections.emptyList() or equivalent) before calling clubRepository.findClubsByIds(ids) so behavior is consistent with the other methods.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
72-89: 빈 리스트 또는 null 입력에 대한 방어 로직 추가를 고려하세요.현재 caller(
ClubCommandService.getSubscribedClubs)가 빈 리스트를 먼저 체크하지만, repository 레벨에서도 방어적으로 처리하면 향후 다른 caller가 추가될 때 더 안전합니다.🛡️ 방어 로직 추가 제안
`@Override` `@Transactional`(readOnly = true) public List<ClubReadModel> findClubsByIds(List<Long> ids) { + if (ids == null || ids.isEmpty()) { + return List.of(); + } + return queryFactory .select(new QClubReadModel( club.id,🤖 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 72 - 89, The method findClubsByIds in ClubQueryRepositoryImpl should defensively handle null or empty ids to avoid unnecessary query execution and potential NPEs; at the start of findClubsByIds check if ids is null or ids.isEmpty() and return Collections.emptyList() (or List.of()) immediately, otherwise proceed with the existing queryFactory.select(...).from(club).where(club.id.in(ids)).fetch(); keep the rest of the method (QClubReadModel construction) unchanged.src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (1)
83-85:userToken파라미터 미사용 확인
userToken이 선언되었지만 메서드 내에서 사용되지 않습니다. 기존deleteSubscription(line 69)과 동일한 패턴이므로 의도된 설계로 보이지만, 향후 로깅이나 감사(audit) 용도로 활용할 계획이 있는지 확인하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` around lines 83 - 85, The getMySubscriptions method declares a userToken parameter that is not used; either remove the unused parameter from the method signature (and update any callers/API docs) or, if it's intentionally kept for future logging/audit like deleteSubscription, make its purpose explicit by using it (e.g., pass it into the service/audit logger) or annotate/safeguard it (add a comment and `@SuppressWarnings`("unused") on the parameter or method). Locate getMySubscriptions in UserClubSubscriptionApiV2 and apply one of these fixes consistently with deleteSubscription.src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java (1)
187-219:StoragePortmock 추가를 권장합니다.현재 테스트에서
iconImagePath가null이라storagePort가 호출되지 않지만, 향후 테스트 데이터에 아이콘 경로가 추가되면NullPointerException이 발생합니다.ClubCommandService가StoragePort를 의존하므로 mock을 추가하는 것이 안전합니다.🔧 StoragePort mock 추가
클래스 레벨에 mock 필드 추가:
`@Mock` private StoragePort storagePort;아이콘 이미지 경로가 있는 케이스를 테스트하려면:
ClubReadModel readModel = new ClubReadModel( 1L, "쿠링", "건국대 공지사항 앱 만드는 개발 동아리", - null, + "clubs/icon/1.png", com.kustacks.kuring.club.domain.ClubCategory.ACADEMIC, com.kustacks.kuring.club.domain.ClubDivision.CENTRAL, null, null ); +when(storagePort.getPresignedUrl("clubs/icon/1.png")).thenReturn("https://cdn.example.com/clubs/icon/1.png");🤖 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/ClubCommandServiceTest.java` around lines 187 - 219, Add a mock for StoragePort in ClubCommandServiceTest and stub the method(s) ClubCommandService calls when resolving icon image paths so future non-null iconImagePath values don't NPE: declare a field like "@Mock private StoragePort storagePort" in the test class, inject/initialize it into the service under test, and add a when(...).thenReturn(...) for the StoragePort method used by ClubCommandService (the method that converts iconImagePath to a URL) so the getSubscribedClubs call (service.getSubscribedClubs(new SubscribedClubListCommand(...))) returns safe values even when ClubReadModel.iconImagePath is non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 43-46: findClubsByIds lacks the same null/empty input guard used
by findSubscribedClubIds and countSubscribersByClubIds; update
ClubPersistenceAdapter.findClubsByIds to defensively handle a null or empty ids
list (return Collections.emptyList() or equivalent) before calling
clubRepository.findClubsByIds(ids) so behavior is consistent with the other
methods.
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 72-89: The method findClubsByIds in ClubQueryRepositoryImpl should
defensively handle null or empty ids to avoid unnecessary query execution and
potential NPEs; at the start of findClubsByIds check if ids is null or
ids.isEmpty() and return Collections.emptyList() (or List.of()) immediately,
otherwise proceed with the existing
queryFactory.select(...).from(club).where(club.id.in(ids)).fetch(); keep the
rest of the method (QClubReadModel construction) unchanged.
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Around line 83-85: The getMySubscriptions method declares a userToken
parameter that is not used; either remove the unused parameter from the method
signature (and update any callers/API docs) or, if it's intentionally kept for
future logging/audit like deleteSubscription, make its purpose explicit by using
it (e.g., pass it into the service/audit logger) or annotate/safeguard it (add a
comment and `@SuppressWarnings`("unused") on the parameter or method). Locate
getMySubscriptions in UserClubSubscriptionApiV2 and apply one of these fixes
consistently with deleteSubscription.
In `@src/test/java/com/kustacks/kuring/acceptance/UserStep.java`:
- Around line 146-152: The test method 구독한_동아리_목록_조회_응답_확인 uses containsAll
which allows extra clubs; replace that with an exact ID comparison (e.g., assert
that the response list of IDs is exactly the expected set/collection from
clubIds, using a containsExactlyInAnyOrder / equals assertion) and also add an
assertion that the response message equals the new success message introduced in
this PR (check response.jsonPath().getString("message") against the expected
success text).
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java`:
- Around line 187-219: Add a mock for StoragePort in ClubCommandServiceTest and
stub the method(s) ClubCommandService calls when resolving icon image paths so
future non-null iconImagePath values don't NPE: declare a field like "@Mock
private StoragePort storagePort" in the test class, inject/initialize it into
the service under test, and add a when(...).thenReturn(...) for the StoragePort
method used by ClubCommandService (the method that converts iconImagePath to a
URL) so the getSubscribedClubs call (service.getSubscribedClubs(new
SubscribedClubListCommand(...))) returns safe values even when
ClubReadModel.iconImagePath is non-null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb3ec977-8220-4ec2-835c-fe981db649a5
📒 Files selected for processing (15)
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/ClubSubscriptionUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/SubscribedClubListCommand.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javasrc/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.javasrc/test/java/com/kustacks/kuring/acceptance/UserStep.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java
하단 내용에 반영했을걸유..? |
rlagkswn00
left a comment
There was a problem hiding this comment.
고생하셨습니다~
몇가지 남겼는데 확인 부탁드립니다
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
리뷰 반영해서 수정했습니다!
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java (1)
28-50:⚠️ Potential issue | 🟠 Major
StoragePortmock 누락으로 테스트 실패 발생
ClubCommandService는StoragePort를 필수 의존성으로 선언하고 있지만(src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javaline 33), 테스트 클래스에서 해당 mock이 누락되어 있습니다.@InjectMocks가 서비스를 생성할 때NullPointerException이 발생할 수 있습니다.🔧 수정 제안
`@Mock` private UserEventPort userEventPort; + `@Mock` + private StoragePort storagePort; + private RootUser rootUser;🤖 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/ClubCommandServiceTest.java` around lines 28 - 50, The test is missing a mock for the StoragePort dependency required by ClubCommandService; add a `@Mock` field for StoragePort (e.g., "private StoragePort storagePort;") in ClubCommandServiceTest so that `@InjectMocks` can create the ClubCommandService instance without NPE; locate the test class fields where other `@Mock` annotations (clubQueryPort, serverProperties, etc.) are declared and add the StoragePort mock there, and ensure Mockito initialization is run (e.g., MockitoAnnotations.openMocks(this)) if not already present.
🧹 Nitpick comments (2)
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
109-121: 구독 목록 응답 순서는 여기서 명시적으로 고정해두는 편이 안전합니다.
findClubReadModelsByIds(...)가IN조회라면subscribedClubIds의 입력 순서를 보장하지 않습니다. 지금 구현은 저장소 반환 순서에 그대로 의존해서 응답 순서가 비결정적으로 바뀔 수 있으니, ID 목록 기준으로 한 번 재정렬해두는 편이 안전합니다.예시 수정안
List<ClubReadModel> clubReadModels = clubQueryPort.findClubReadModelsByIds(subscribedClubIds); Map<Long, Long> subscriberCountMap = clubSubscriptionQueryPort.countSubscribersByClubIds(subscribedClubIds); - List<ClubItemResult> clubItemResults = toSubscribedClubItemResults(clubReadModels, subscriberCountMap); + Map<Long, ClubReadModel> clubReadModelMap = clubReadModels.stream() + .collect(Collectors.toMap(ClubReadModel::getId, r -> r)); + + List<ClubItemResult> clubItemResults = subscribedClubIds.stream() + .map(clubReadModelMap::get) + .filter(Objects::nonNull) + .map(r -> convertClubItemResult( + r, + true, + subscriberCountMap.getOrDefault(r.getId(), 0L) + )) + .toList();🤖 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 109 - 121, The response list order can be non-deterministic because clubQueryPort.findClubReadModelsByIds(subscribedClubIds) may not preserve the input ID order; update ClubQueryService to re-order the retrieved ClubReadModel objects by the original subscribedClubIds before mapping to results: after calling findClubReadModelsByIds(...) build a map from ClubReadModel.getId() to ClubReadModel, then produce an ordered List<ClubReadModel> by iterating subscribedClubIds and picking entries from that map (and handle missing IDs if needed), then pass that ordered list into toSubscribedClubItemResults(...) so the returned ClubListResult preserves the subscription order.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (1)
336-374: 빈 구독 목록 분기도 테스트로 묶어두면 좋겠습니다.지금은 성공 케이스만 고정돼 있어서
subscribedClubIds.isEmpty()조기 반환이 깨져도 바로 드러나지 않습니다. 빈 목록일 때 하위 조회가 호출되지 않는지까지 확인해두면 회귀를 더 잘 막을 수 있습니다.예시 테스트
+ `@DisplayName`("구독한 동아리가 없으면 빈 목록을 반환한다") + `@Test` + void get_subscribed_clubs_empty() { + when(rootUserQueryPort.findRootUserByEmail("client@konkuk.ac.kr")).thenReturn(Optional.of(rootUser)); + when(clubSubscriptionQueryPort.findSubscribedClubIdsByRootUserId(1L)).thenReturn(List.of()); + + ClubListResult result = clubQueryService.getSubscribedClubs(new SubscribedClubListCommand("client@konkuk.ac.kr")); + + assertThat(result.clubs()).isEmpty(); + verify(clubQueryPort, never()).findClubReadModelsByIds(anyList()); + verify(clubSubscriptionQueryPort, never()).countSubscribersByClubIds(anyList()); + }🤖 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 336 - 374, Add a new unit test in ClubQueryServiceTest that covers the empty-subscribed-clubs branch: mock rootUserQueryPort.findRootUserByEmail(...) to return rootUser and mock clubSubscriptionQueryPort.findSubscribedClubIdsByRootUserId(...) to return an empty list, then call clubQueryService.getSubscribedClubs(new SubscribedClubListCommand(...)) and assert the returned ClubListResult has an empty clubs() list; additionally verify that clubQueryPort.findClubReadModelsByIds(...), clubSubscriptionQueryPort.countSubscribersByClubIds(...), and storagePort.getPresignedUrl(...) are never invoked (use Mockito.verify(..., never()) on those methods) to ensure the early-return path is exercised.
🤖 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/service/ClubCommandService.java`:
- Line 33: ClubCommandService에 주입된 storagePort 필드가 사용되지 않으므로 불필요한 의존성을 제거하세요:
ClubCommandService 클래스에서 private final StoragePort storagePort 필드와 생성자(또는
`@Autowired`)에서 해당 파라미터를 제거하고 관련 import(StoragePort)를 삭제하거나, 만약 storagePort를 실제로
사용하려는 목적이 있다면 사용될 메서드(예: 파일 저장/삭제 로직)에 storagePort를 참조하도록 구현해 주세요.
---
Outside diff comments:
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java`:
- Around line 28-50: The test is missing a mock for the StoragePort dependency
required by ClubCommandService; add a `@Mock` field for StoragePort (e.g.,
"private StoragePort storagePort;") in ClubCommandServiceTest so that
`@InjectMocks` can create the ClubCommandService instance without NPE; locate the
test class fields where other `@Mock` annotations (clubQueryPort,
serverProperties, etc.) are declared and add the StoragePort mock there, and
ensure Mockito initialization is run (e.g., MockitoAnnotations.openMocks(this))
if not already present.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 109-121: The response list order can be non-deterministic because
clubQueryPort.findClubReadModelsByIds(subscribedClubIds) may not preserve the
input ID order; update ClubQueryService to re-order the retrieved ClubReadModel
objects by the original subscribedClubIds before mapping to results: after
calling findClubReadModelsByIds(...) build a map from ClubReadModel.getId() to
ClubReadModel, then produce an ordered List<ClubReadModel> by iterating
subscribedClubIds and picking entries from that map (and handle missing IDs if
needed), then pass that ordered list into toSubscribedClubItemResults(...) so
the returned ClubListResult preserves the subscription order.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 336-374: Add a new unit test in ClubQueryServiceTest that covers
the empty-subscribed-clubs branch: mock
rootUserQueryPort.findRootUserByEmail(...) to return rootUser and mock
clubSubscriptionQueryPort.findSubscribedClubIdsByRootUserId(...) to return an
empty list, then call clubQueryService.getSubscribedClubs(new
SubscribedClubListCommand(...)) and assert the returned ClubListResult has an
empty clubs() list; additionally verify that
clubQueryPort.findClubReadModelsByIds(...),
clubSubscriptionQueryPort.countSubscribersByClubIds(...), and
storagePort.getPresignedUrl(...) are never invoked (use Mockito.verify(...,
never()) on those methods) to ensure the early-return path is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 610f23bd-5ceb-45ac-b432-f6065ff6e22e
📒 Files selected for processing (15)
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/application/port/in/ClubQueryUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/in/ClubSubscriptionUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.javasrc/test/java/com/kustacks/kuring/support/DatabaseConfigurator.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
- src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java
rlagkswn00
left a comment
There was a problem hiding this comment.
고생하셨슴니다! 저번 리뷰에 답글 달았는데 그거만 확인해주세여!
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (2)
86-86: 페이지네이션 고려 (선택사항)현재 구독한 동아리 목록을 전체 반환하고 있습니다. 사용자가 구독 가능한 동아리 수에 제한이 있다면 문제없지만, 구독 수가 많아질 수 있는 경우 페이지네이션 도입을 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` at line 86, The current controller call to clubQueryUseCase.getSubscribedClubs(command) in UserClubSubscriptionApiV2 returns the full list (ClubListResult) and should support pagination: add page/size (or cursor) fields to the existing command object passed to getSubscribedClubs, update UserClubSubscriptionApiV2 to accept query params (e.g., page and size) and populate the command, and modify clubQueryUseCase.getSubscribedClubs (or add an overloaded method) to return a paged ClubListResult containing only the requested slice plus metadata (total/count/page info); ensure the DTO/model ClubListResult is extended to carry pagination metadata and adapt any callers to handle paged responses.
74-91: 새 엔드포인트 구현이 기존 패턴과 일관성이 있습니다.
getMySubscriptions메서드가 기존addSubscription,deleteSubscription과 동일한 인증 패턴을 따르고 있으며, 서비스 레이어 호출 및 응답 변환 흐름이 적절합니다.한 가지 확인 사항:
userToken파라미터가 메서드 내에서 사용되지 않습니다. 기존 엔드포인트들도 동일한 패턴이므로 의도적인 것으로 보이나, 향후 FCM 토큰 검증이나 로깅 목적이 있다면 주석으로 명시하거나, 사용 계획이 없다면@SuppressWarnings("unused")또는_prefix를 고려해볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` around lines 74 - 91, getMySubscriptions declares a RequestHeader parameter userToken that is never used; either document its intentional unused status or mark it to avoid compiler/warning noise. Update the method getMySubscriptions to either (a) add a short comment above the parameter usage explaining it's intentionally unused for future FCM/logging, (b) annotate the parameter (or method) with `@SuppressWarnings`("unused"), or (c) rename the parameter to _userToken to indicate unused intent; mirror the same approach used in addSubscription and deleteSubscription for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Line 86: The current controller call to
clubQueryUseCase.getSubscribedClubs(command) in UserClubSubscriptionApiV2
returns the full list (ClubListResult) and should support pagination: add
page/size (or cursor) fields to the existing command object passed to
getSubscribedClubs, update UserClubSubscriptionApiV2 to accept query params
(e.g., page and size) and populate the command, and modify
clubQueryUseCase.getSubscribedClubs (or add an overloaded method) to return a
paged ClubListResult containing only the requested slice plus metadata
(total/count/page info); ensure the DTO/model ClubListResult is extended to
carry pagination metadata and adapt any callers to handle paged responses.
- Around line 74-91: getMySubscriptions declares a RequestHeader parameter
userToken that is never used; either document its intentional unused status or
mark it to avoid compiler/warning noise. Update the method getMySubscriptions to
either (a) add a short comment above the parameter usage explaining it's
intentionally unused for future FCM/logging, (b) annotate the parameter (or
method) with `@SuppressWarnings`("unused"), or (c) rename the parameter to
_userToken to indicate unused intent; mirror the same approach used in
addSubscription and deleteSubscription for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0b6a167-f607-432d-8773-5fdf5d5a17b5
📒 Files selected for processing (2)
src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Around line 82-88: The getMySubscriptions handler currently declares an unused
required header parameter userToken and a matching `@SecurityRequirement` using
FCM_TOKEN_HEADER_KEY; remove the unused parameter (userToken) from the
getMySubscriptions method signature and delete the redundant
`@SecurityRequirement`(name = FCM_TOKEN_HEADER_KEY) annotation so the endpoint no
longer requires the User-Token header (leave the bearerToken/Authorization
extraction logic and JWT security requirement intact), and clean up any
now-unused imports or constants references if the compiler flags them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02b3ea4c-5a9e-402a-bbc2-6b2ab8d0bb26
📒 Files selected for processing (13)
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/application/port/in/ClubQueryUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.javasrc/test/java/com/kustacks/kuring/support/DatabaseConfigurator.java
💤 Files with no reviewable changes (1)
- src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/test/java/com/kustacks/kuring/support/DatabaseConfigurator.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java
- src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
- src/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.java
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java
- src/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionQueryPort.java
- src/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.java
#️⃣ 이슈
#336
📌 요약
사용자가 구독한 동아리 목록 조회 API를 구현하고 관련 테스트를 추가했습니다.
🛠️ 상세
GET /api/v2/users/subscriptions/clubs
-> 명세서에는 GET /api/v2/users/bookmarks/clubs 이었는데 구독으로 명칭 확정하고 다른 구독 관련api랑 맞추려고 변경했습니다. 명세서에도 반영하겠습니다!
ClubCommandService - getSubscribedClubs 추가
ClubSubscriptionQueryPort - findAllSubscribedClubIds(rootUserId), ClubQueryPort - findClubsByIds(ids) 추가
ClubSubscribeRepository - findClubIdsByRootUserId(rootUserId) 추가
ClubQueryRepositoryImpl - findClubsByIds(List ids) 추가
ClubCommandServiceTest, ClubPersistenceAdapterTest, UserAcceptanceTest에 관련 테스트 추가
💬 기타
ClubCommandService는 동아리 구독 관련 유스케이스를 담당하고 있는데 이번에 조회 관련 내용도 추가돼서 네이밍을 ClubSubscriptionService로 바꾸는건 어떻게 생각하시나요??
Summary by CodeRabbit
새로운 기능
개선사항
테스트
메시지