fix: 릴리즈 리뷰 반영 (판매자 대화 목록 커서를 정렬 키와 일치시킴) - #290
Conversation
정렬은 (updated_at, id) desc인데 커서는 id 단독이라, `id < cursor`가 정렬 순서와 무관한 행을 잘라냈다. id가 크지만 오래된 대화는 커서 페이지에서 영영 빠진다. 예: A(id=100, 최신) · B(id=5) · C(id=200, 가장 오래됨) 순으로 정렬되면 limit 2에서 nextCursor=5가 나가고, 다음 페이지는 id < 5라 C(200)가 나오지 않는다. hasMore=true와 겹치면 클라이언트가 있지도 않은 페이지를 좇는다. PR #286이 hasMore를 노출하면서 이 결함이 실제 피해로 바뀌었다. 정렬을 id 단독으로 낮추는 대신 커서를 정렬 키에 맞췄다 — schema.prisma에 [store_id, updated_at] 인덱스가 있어 updated_at 정렬이 의도적이고 최적화까지 돼 있다. 구매자 대화 목록이 이미 같은 문제를 (last_message_at, id) 복합 커서로 풀어 뒀고 공용 유틸(common/utils/keyset-cursor)도 있어 그대로 재사용했다. - listConversationsByStore: (updated_at, id) desc 키셋 where로 교체 - sellerConversations: parseTimestampIdCursor / buildTimestampIdCursor 사용 - SellerConversationListInput 신설. 커서 형식이 공용 SellerCursorInput(id 단독)과 달라 분리했고, SDL의 cursor·nextCursor 타입도 ID → String(불투명 토큰)으로 바꿨다 같은 유형이 더 있는지 seller 목록 8종의 정렬 키와 커서 키를 전수 대조했다. 어긋난 건 이 목록 하나뿐이고 나머지 7종은 id desc + id 커서로 일관된다. 테스트 +1: id가 더 큰 오래된 대화가 커서 페이지에서 빠지지 않는지. 옛 동작(id 단독 커서)으로 되돌리면 이 테스트만 실패하는 것까지 확인했다. 전체 1,874건 통과.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
🧹 knip — dead-code 리포트전체 리포트
|
🩺 NestJS Doctor — 90/100 (Excellent)진단 313건 (error 0).
architecture / security 상위 항목
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9b4797956
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| extend type Query { | ||
| """내 매장 대화방 목록을 조회한다. 판매자 로그인 필수. SELLER 계정이 아니면 FORBIDDEN, 매장을 보유하지 않으면 NOT_FOUND.""" | ||
| sellerConversations(input: SellerCursorInput): SellerConversationConnection! | ||
| sellerConversations(input: SellerConversationListInput): SellerConversationConnection! |
There was a problem hiding this comment.
Preserve the existing GraphQL input type
Existing generated or persisted operations declare $input: SellerCursorInput; because GraphQL input object types are nominal, changing this argument to SellerConversationListInput makes those operations fail validation even when they only supply limit or omit the cursor. The existing cursor: ID field can already accept an opaque string such as timestamp:id, so retain the existing input type or introduce a versioned query rather than requiring every deployed client to update before this endpoint works again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
미반영(근거 확인 후 현행 유지): 지적한 원리는 맞다. GraphQL input 타입은 nominal이라 $input: SellerCursorInput으로 선언된 오퍼레이션은 limit만 넘겨도 검증에서 실패한다.
다만 전제인 "deployed client"가 이 엔드포인트엔 없다. 다시 확인했다.
- caquick-fe-v2 전 원격 브랜치에서 sellerConversations · SellerCursorInput 사용 0건
- org 레포는 caquick-be · caquick-fe-v2 · caquick-fe 셋뿐이고 판매자 프론트가 없다
- caquick-fe는 2025-12 이후 방치
즉 지금 깨질 오퍼레이션이 존재하지 않는다.
제안한 대안(cursor: ID에 "timestamp:id" 토큰을 담아 기존 타입 유지)도 동작은 한다. 그렇게 하지 않은 이유는 SellerCursorInput이 판매자 목록 7종이 공유하는 타입이기 때문이다. 같은 필드가 한 쿼리에서만 다른 형식을 뜻하게 되면, 이번 버그를 만든 것과 같은 종류의 암묵적 가정이 하나 더 생긴다 — 원래 버그도 "커서는 id"라는 공유 가정이 이 목록에만 성립하지 않아서 났다.
타입을 분리하면 형식 차이가 스키마에 드러나고, cursor: String으로 두면 불투명 토큰이라는 성격도 맞다.
클라이언트가 생긴 뒤였다면 판단이 달랐을 것이다. 그 시점엔 versioned query 쪽이 맞다.
Coverage report
Test suite run success1874 tests passing in 225 suites. Report generated by 🧪jest coverage report action from e9b4797 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
릴리즈 PR #289 리뷰에서 나온 지적. main에 직접 커밋하지 않고 develop으로 먼저 반영한다.
문제
정렬은
(updated_at, id) desc인데 커서는id단독이라,id < cursor가 정렬 순서와 무관한 행을 잘라낸다.hasMore=true와 겹치면 클라이언트가 있지도 않은 페이지를 좇는다. PR #286이hasMore를 노출하면서 이 결함이 실제 피해로 바뀌었다.정렬을 낮추지 않고 커서를 맞춘 이유
schema.prisma에[store_id, updated_at]인덱스가 있다 —updated_at정렬은 의도적이고 최적화까지 돼 있어,id desc로 낮추면 그 인덱스와 의도를 버리게 된다.구매자 대화 목록이 이미 같은 문제를
(last_message_at, id)복합 커서로 풀어 뒀고 공용 유틸(common/utils/keyset-cursor)도 있어 그대로 재사용했다.변경
listConversationsByStore—(updated_at, id)desc 키셋 where로 교체sellerConversations—parseTimestampIdCursor/buildTimestampIdCursor사용SellerConversationListInput신설. 커서 형식이 공용SellerCursorInput(id 단독)과 달라 분리했고, SDL의cursor·nextCursor도ID→String(불투명 토큰)으로 바꿨다같은 유형 전수 확인
seller 목록 8종의 정렬 키와 커서 키를 대조했다. 어긋난 건 이 목록 하나뿐이고 나머지 7종은
id desc+id커서로 일관된다.테스트
+1건 — "id가 더 큰 오래된 대화가 커서 페이지에서 빠지지 않는다". 옛 동작(id 단독 커서)으로 되돌리면 이 테스트만 실패하는 것까지 확인했다.
전체 225 suites 1,874건 통과.
lint·tsc·dto:check·docs:check·arch:check통과.