Skip to content

Conversation

@sweatbuckets
Copy link
Contributor

@sweatbuckets sweatbuckets commented Jan 25, 2026

  1. #⃣ 연관된 이슈
    • 관련 이슈를 명시해주세요.
    • 예: #이슈번호#이슈번호
  2. 📝 작업 내용
    • 이번 PR에서 작업한 내용을 간략히 설명해주세요.
    • 필요한 경우 이미지 첨부 가능.
  3. 📸 스크린샷 (선택)
    • 작업 내용을 시각적으로 표현할 스크린샷을 포함하세요.
  4. 💬 리뷰 요구사항 (선택)
    • 리뷰어가 특히 검토해주었으면 하는 부분이 있다면 작성해주세요.
    • 예: "메서드 XXX의 이름을 더 명확히 하고 싶은데, 좋은 아이디어가 있으신가요?"

Summary by CodeRabbit

  • New Features
    • Improved cursor-based pagination for photo albums: API now accepts an optional timestamp cursor to more reliably fetch recent items.
    • Pagination responses include both a cursor ID and a cursor timestamp (nextCursorId and nextCursorUpdatedAt) for precise sequential loading.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 25, 2026

📝 Walkthrough

Walkthrough

Introduces timestamp-aware cursor pagination for photo album retrieval by adding a cursorUpdatedAt (LocalDateTime) parameter through controller, service, repository, and response DTO layers, and updating query and DTO cursor fields to include both id and updatedAt.

Changes

Cohort / File(s) Summary
Controller Layer
src/main/java/cc/backend/photoAlbum/controller/PhotoAlbumController.java
Added optional cursorUpdatedAt (LocalDateTime) parameter to getAllPhotoAlbum endpoint and passed it to the service call.
Response DTO
src/main/java/cc/backend/photoAlbum/dto/PhotoAlbumResponseDTO.java
Replaced nextCursor with nextCursorId and added nextCursorUpdatedAt (LocalDateTime) in ScrollMemberPhotoAlbumDTO.
Repository
src/main/java/cc/backend/photoAlbum/repository/PhotoAlbumRepository.java
Added cursorUpdatedAt parameter to findNextAlbums signature and JPQL WHERE clause; bound :cursorUpdatedAt to compare p.updatedAt directly.
Service Interface
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumService.java
Updated getAllRecentPhotoAlbumList signature to accept updatedAt (LocalDateTime) parameter.
Service Implementation
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
Updated method signature to accept cursorUpdatedAt, passed it to repository, and populated nextCursorId + nextCursorUpdatedAt in the returned DTO.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller as PhotoAlbumController
    participant Service as PhotoAlbumService
    participant Repository as PhotoAlbumRepository
    participant DTO as ScrollMemberPhotoAlbumDTO

    Client->>Controller: GET /api/albums?cursorId=X&cursorUpdatedAt=T&size=Y
    Controller->>Service: getAllRecentPhotoAlbumList(cursorId, cursorUpdatedAt, size)
    Service->>Repository: findNextAlbums(cursorId, cursorUpdatedAt, pageable)
    Repository-->>Service: List<PhotoAlbum> (paginated results)
    Service->>Service: extract last item's id & updatedAt
    Service->>DTO: build ScrollMemberPhotoAlbumDTO with nextCursorId + nextCursorUpdatedAt
    Service-->>Controller: ScrollMemberPhotoAlbumDTO
    Controller-->>Client: JSON response with nextCursorId & nextCursorUpdatedAt
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰✨ I hopped through code to track time and id,

nextCursorId and time now stride side-by-side.
Albums scroll neat, no more ambiguous tracks,
I nibbled bugs, then bounced back to relax. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only the template structure without any concrete information filled in—no issue references, work summary, screenshots, or review requests are provided. Fill in the description template with: related issue number(s), a summary of the updatedAt parameter changes across controller/service/repository layers, and any specific review concerns.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding an updatedAt parameter to the photo-album infinite scroll endpoint for recent photo queries.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/main/java/cc/backend/photoAlbum/repository/PhotoAlbumRepository.java`:
- Around line 42-47: The WHERE clause in PhotoAlbumRepository's findNextAlbums
uses only "cursorId IS NULL" to detect initial requests which can mis-handle
cases where cursorId and cursorUpdatedAt are not provided together; update the
predicate to treat the cursor as absent only when both parameters are null
(e.g., "(:cursorId IS NULL AND :cursorUpdatedAt IS NULL) OR ...") and ensure the
subsequent comparison branch uses both cursorUpdatedAt and cursorId (p.updatedAt
< :cursorUpdatedAt OR (p.updatedAt = :cursorUpdatedAt AND p.id < :cursorId)) so
the composite cursor is applied atomically.
🧹 Nitpick comments (1)
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (1)

303-304: Consider extracting the repeated condition and last-element access.

The condition hasNext && !limitedAlbums.isEmpty() and the last-element access are duplicated. A minor refactor could improve readability.

♻️ Suggested refactor
         // 다음 커서 설정
         boolean hasNext = albums.size() > size;
-        Long nextCursorId = (hasNext && !limitedAlbums.isEmpty()) ? limitedAlbums.get(limitedAlbums.size() - 1).getId() : null;
-        LocalDateTime nextCursorUpdatedAt = (hasNext && !limitedAlbums.isEmpty()) ? limitedAlbums.get(limitedAlbums.size() - 1).getUpdatedAt() : null;
+        Long nextCursorId = null;
+        LocalDateTime nextCursorUpdatedAt = null;
+        if (hasNext && !limitedAlbums.isEmpty()) {
+            PhotoAlbum lastAlbum = limitedAlbums.get(limitedAlbums.size() - 1);
+            nextCursorId = lastAlbum.getId();
+            nextCursorUpdatedAt = lastAlbum.getUpdatedAt();
+        }

Comment on lines 42 to 47
WHERE (:cursorId IS NULL
OR p.updatedAt < :cursorUpdatedAt
OR (p.updatedAt = :cursorUpdatedAt AND p.id < :cursorId))
ORDER BY p.updatedAt DESC, p.id DESC
""")
List<PhotoAlbum> findNextAlbums(@Param("cursorId") Long cursorId, Pageable pageable);
""")
List<PhotoAlbum> findNextAlbums(@Param("cursorId") Long cursorId, @Param("cursorUpdatedAt") LocalDateTime cursorUpdatedAt, Pageable pageable);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Cursor null-check logic may cause incorrect filtering on first page.

The query only checks cursorId IS NULL to determine if this is the initial request. However, both cursorId and cursorUpdatedAt form a composite cursor. If only one is null while the other has a value (due to client error or API misuse), the query may produce unexpected results.

Consider checking both cursor parameters together:

🔧 Proposed fix
     `@Query`("""
     SELECT p
     FROM PhotoAlbum p
     LEFT JOIN p.amateurShow a
-    WHERE (:cursorId IS NULL
+    WHERE ((:cursorId IS NULL OR :cursorUpdatedAt IS NULL)
            OR p.updatedAt < :cursorUpdatedAt
            OR (p.updatedAt = :cursorUpdatedAt AND p.id < :cursorId))
     ORDER BY p.updatedAt DESC, p.id DESC
     """)
🤖 Prompt for AI Agents
In `@src/main/java/cc/backend/photoAlbum/repository/PhotoAlbumRepository.java`
around lines 42 - 47, The WHERE clause in PhotoAlbumRepository's findNextAlbums
uses only "cursorId IS NULL" to detect initial requests which can mis-handle
cases where cursorId and cursorUpdatedAt are not provided together; update the
predicate to treat the cursor as absent only when both parameters are null
(e.g., "(:cursorId IS NULL AND :cursorUpdatedAt IS NULL) OR ...") and ensure the
subsequent comparison branch uses both cursorUpdatedAt and cursorId (p.updatedAt
< :cursorUpdatedAt OR (p.updatedAt = :cursorUpdatedAt AND p.id < :cursorId)) so
the composite cursor is applied atomically.

@sweatbuckets sweatbuckets merged commit 1b7042a into develop Jan 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants