Skip to content

[FIX] nullable 조회 조건으로 인한 홈 API 오류 수정 - #118

Merged
ownue merged 1 commit into
developfrom
fix/#117-api-error
Aug 2, 2026
Merged

[FIX] nullable 조회 조건으로 인한 홈 API 오류 수정#118
ownue merged 1 commit into
developfrom
fix/#117-api-error

Conversation

@ownue

@ownue ownue commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

📍 개요

홈 및 히스토리 조회 시 발생하는 PostgreSQL nullable 파라미터 오류 수정

⛓️‍💥 관련 이슈


🛠️ 작업 내용

  • 기간 없는 연주 목록 조회에서 nullable cutoff 파라미터 제거
  • 기간 필터 유무에 따라 Repository 쿼리 분리
  • 홈 최근 연주 조회 시 발생하던 SQLState 42P18 오류 수정
  • 히스토리 RECENT 및 기간 필터 없는 조회의 동일 오류 방지
  • Home/History 서비스 테스트 수정 및 기간 조회 분기 테스트 추가

🔥 리뷰 요청 사항

리뷰어가 중점적으로 확인해주었으면 하는 내용을 작성해주세요.

  • 기간 유무에 따른 Repository 쿼리 분리 방식
  • 기존 정렬 및 필터 조건이 동일하게 유지되는지
  • Home과 History 조회에 미치는 영향

✅ 체크리스트

  • 코드 컨벤션을 준수했습니다.
  • 불필요한 코드 및 import를 제거했습니다.
  • 예외 처리를 적용했습니다.
  • 테스트를 완료했습니다.
  • 관련 Issue를 연결했습니다.

📎 참고 사항

  • 오류 로그: SQLState: 42P18, could not determine data type of parameter $3
  • API 응답 구조와 DB 스키마 변경은 없습니다.

Summary by CodeRabbit

  • 개선 사항

    • 재생 기록 조회에서 기간 필터가 없으면 전체 기록을, 기간이 지정되면 해당 기간의 기록만 정확히 조회하도록 개선했습니다.
    • 최근 재생 목록 조회가 변경된 기준에 맞게 안정적으로 동작하도록 수정했습니다.
  • 테스트

    • 기간 필터가 적용된 기록 조회와 최근 재생 목록 조회에 대한 검증을 보강했습니다.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

PlayingRepository의 전체 기간 조회와 cutoff 기반 조회를 분리했습니다. HistoryService는 기간 유무에 따라 메서드를 선택하고, HomeService와 관련 테스트는 변경된 시그니처를 사용합니다.

Changes

재생 기록 조회 흐름

Layer / File(s) Summary
조회 메서드 계약 분리
src/main/java/com/mr/domain/playing/repository/PlayingRepository.java
전체 기간 조회에서 cutoff를 제거했습니다. findPlayingsByUserAndStatusSinceendedAt >= cutoff 조건을 추가했습니다.
히스토리 기간별 조회 라우팅
src/main/java/com/mr/domain/history/service/HistoryService.java, src/test/java/com/mr/domain/history/service/HistoryServiceTest.java
기간이 없으면 전체 조회를 사용합니다. 기간이 있으면 cutoff 기반 조회를 사용합니다. 테스트는 새 호출과 HistoryPeriod.WEEKLY 동작을 검증합니다.
홈 최근 재생 조회 연동
src/main/java/com/mr/domain/home/service/HomeService.java, src/test/java/com/mr/domain/home/service/HomeServiceTest.java
최근 재생 조회에서 사용하지 않는 null 인자를 제거했습니다. 테스트 목킹을 새 시그니처에 맞게 수정했습니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • Musereview/BE#52: 동일한 HistoryServicePlayingRepository 조회 메서드의 후속 변경입니다.
  • Musereview/BE#62: HistoryServicePlaying 연동을 함께 변경한 PR입니다.
  • Musereview/BE#63: HistoryService, HomeService, PlayingRepository의 재생 기록 조회를 함께 수정한 PR입니다.

Poem

cutoff는 갈라지고,
PageRequest는 함께 걷네.
홈은 더 짧게 부르고,
히스토리는 기간을 고르네.
테스트가 길을 비추네.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 nullable 조회 조건으로 발생한 홈 및 히스토리 API 오류 수정이라는 주요 변경 내용을 직접 설명합니다.
Linked Issues check ✅ Passed 변경 사항은 이슈 #117의 홈 화면 조회 오류와 히스토리 조회 오류를 모두 수정하고 관련 서비스 테스트를 갱신합니다.
Out of Scope Changes check ✅ Passed Repository 쿼리 분리, 서비스 호출 수정, 관련 테스트 변경은 연결된 이슈의 오류 해결 범위에 포함됩니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/#117-api-error

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/test/java/com/mr/domain/history/service/HistoryServiceTest.java (1)

135-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

HistoryPeriod.RECENT의 무필터 경로도 직접 검증하세요.

현재 테스트는 HistoryPeriod.WEEKLYfindPlayingsByUserAndStatusSince를 선택하는지만 확인합니다. HistoryService.resolveCutoffRECENT에서 null을 반환하므로, 이 경로는 findPlayingsByUserAndStatus를 사용해야 합니다. 해당 라우팅이 회귀해도 현재 테스트는 이를 직접 검출하지 못합니다.

RECENT 요청에서 status-only 메서드가 호출되고 cutoff 전용 메서드는 호출되지 않는지 검증하세요. 호출 메서드와 인자를 함께 검증하면 분리된 Repository 계약의 회귀를 조기에 찾을 수 있습니다.

테스트 보강 예시
+import static org.mockito.Mockito.never;
+
+    `@Test`
+    `@DisplayName`("getHistories - RECENT는 기간 조건 없는 쿼리를 사용한다")
+    void getHistories_recent_usesUnfilteredQuery() {
+        given(playingRepository.findPlayingsByUserAndStatus(
+                eq(1L), eq(PlayingStatus.COMPLETED), eq(PageRequest.of(0, 10))))
+                .willReturn(new SliceImpl<>(List.of(), PageRequest.of(0, 10), false));
+
+        historyService.getHistories(1L, 0, 10, HistoryPeriod.RECENT);
+
+        verify(playingRepository).findPlayingsByUserAndStatus(
+                eq(1L), eq(PlayingStatus.COMPLETED), eq(PageRequest.of(0, 10)));
+        verify(playingRepository, never()).findPlayingsByUserAndStatusSince(
+                eq(1L), eq(PlayingStatus.COMPLETED), any(LocalDateTime.class), any());
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test/java/com/mr/domain/history/service/HistoryServiceTest.java` around
lines 135 - 148, Extend getHistories_withPeriod_usesSinceQuery or add a focused
test for HistoryPeriod.RECENT that stubs and verifies
playingRepository.findPlayingsByUserAndStatus with the expected user, COMPLETED
status, and PageRequest, while explicitly verifying
findPlayingsByUserAndStatusSince is never called. Keep the assertion that the
returned response is empty and validate the repository routing and arguments
together.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/test/java/com/mr/domain/history/service/HistoryServiceTest.java`:
- Around line 135-148: Extend getHistories_withPeriod_usesSinceQuery or add a
focused test for HistoryPeriod.RECENT that stubs and verifies
playingRepository.findPlayingsByUserAndStatus with the expected user, COMPLETED
status, and PageRequest, while explicitly verifying
findPlayingsByUserAndStatusSince is never called. Keep the assertion that the
returned response is empty and validate the repository routing and arguments
together.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bea5527a-b784-419c-b447-00be74011049

📥 Commits

Reviewing files that changed from the base of the PR and between 4fe7edf and 0417431.

📒 Files selected for processing (5)
  • src/main/java/com/mr/domain/history/service/HistoryService.java
  • src/main/java/com/mr/domain/home/service/HomeService.java
  • src/main/java/com/mr/domain/playing/repository/PlayingRepository.java
  • src/test/java/com/mr/domain/history/service/HistoryServiceTest.java
  • src/test/java/com/mr/domain/home/service/HomeServiceTest.java

@kimyw1018 kimyw1018 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

버그 원인 쿼리로 깔끔하게 잘 처리 되어있네요! 수고하셨습니다

@rkdehdrbs7885-oss rkdehdrbs7885-oss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

같은 조회 조건에서도 방식에 따라서도 문제가 생길 수 있군요. 승인하겠습니다!

@ownue
ownue merged commit 5c3e68b into develop Aug 2, 2026
2 checks passed
@kimyw1018
kimyw1018 deleted the fix/#117-api-error branch August 4, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 BugFix - 담당 도메인 오류 해결

3 participants