Skip to content

chore: 지역·인기매장·매장찜·픽업슬롯 기능 릴리즈#148

Merged
chanwoo7 merged 15 commits into
mainfrom
develop
Jun 18, 2026
Merged

chore: 지역·인기매장·매장찜·픽업슬롯 기능 릴리즈#148
chanwoo7 merged 15 commits into
mainfrom
develop

Conversation

@chanwoo7

@chanwoo7 chanwoo7 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

develop → main 릴리즈입니다. figma 명세 "메인 인기 매장 리스트 + 픽업 필터" 기능 4건을 포함합니다.

포함 PR

주요 변경

  • 신규 feature: region / store / pickup
  • 신규 public query: regionGroups, regions, searchRegions, popularStores, pickupCalendar, pickupTimeSlots
  • 신규 mutation: addStoreToWishlist, removeStoreFromWishlist
  • 신규 글로벌: OptionalJwtAuthGuard, common/utils/kst-time
  • DB: Region 마스터 테이블 + Store.region_id FK

⚠️ 배포 시 주의

  • 신규 마이그레이션 1건 (20260617194530_add_region_and_store_region): region 테이블 생성 + store.region_id 추가. 배포 시 prisma migrate deploy 적용 필요.
  • 지역 시드: 수도권 Region 마스터는 영구 데이터입니다. 운영 반영 방법(시드 실행 vs 수동 주입) 확인 필요. 원본 CSV는 gitignore이며 파싱 산출 JSON(prisma/seed/data/regions.generated.json)으로 재현됩니다.

Test plan

  • develop 기준 159 test suites / 1342 tests 통과
  • 각 PR CI green(check·pr-title·coverage-report·CodeQL) + Codex 승인 후 머지 완료

후속(이월 — 기획 보강 필요)

  • 구매자용 매장 상세·상품 조회 API(현재 인기 리스트·판매자 관리만 존재)
  • 지역 2차 생활권 큐레이션, 인기순 KPI 확정·랭킹 캐시/배치, 위치 표기 규칙, nearbyRegions(현재위치 좌표)
  • 픽업 전역 슬롯의 매장별 capacity 반영(주문 단계 검증)

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 광역/시군구 2단계 지역 체계 도입으로 지역별 매장 관리 기능 추가
    • 지역명 기반 자동 검색 기능 제공
    • 월별 픽업 가능 날짜 및 시간 슬롯 조회 기능 추가
    • 지역별 인기 매장 조회 기능 추가 (비로그인 접근 가능)
    • 로그인 사용자의 매장 찜 및 해제 기능 추가
  • 테스트

    • 지역, 픽업, 매장 조회 기능 통합 테스트 추가

chanwoo7 added 13 commits June 18, 2026 04:56
feat(region): 지역 마스터(1·2차 시군구) 모델 + 조회/검색 API + 수도권 시드
feat(store): 인기 매장 리스트 API (popularStores·실시간 랭킹 집계)
feat(store): 매장 찜 토글 + 인기 매장 isWishlisted (옵셔널 JWT 가드)
feat(pickup): 홈 전역 픽업 날짜/시간 슬롯 (pickupCalendar/pickupTimeSlots)
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chanwoo7, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 27 minutes and 14 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ee95d496-0969-430c-8eb0-5f95edc14ae5

📥 Commits

Reviewing files that changed from the base of the PR and between 7d896ef and f1128cc.

📒 Files selected for processing (5)
  • prisma/seed/regions.ts
  • src/features/pickup/constants/pickup-error-messages.ts
  • src/features/pickup/services/pickup-slot.service.spec.ts
  • src/features/pickup/services/pickup-slot.service.ts
  • src/features/region/dto/inputs/search-regions.input.spec.ts
📝 Walkthrough

Walkthrough

region 테이블과 store.region_id FK를 추가하고, 수도권 법정동 CSV → JSON 시드 파이프라인을 구축했다. 지역 계층 조회 GraphQL API, 베이지안 인기도 점수 기반 popularStores 쿼리, 매장 찜 뮤테이션, KST 기반 픽업 캘린더/타임슬롯 API, 그리고 선택적 JWT 인증 가드를 신규 추가했다.

Changes

지역 마스터 + 인기 매장 랭킹 & 찜

Layer / File(s) Summary
지역 DB 스키마 & 시드 파이프라인
prisma/migrations/.../migration.sql, prisma/schema.prisma, scripts/generate-region-seed.ts, prisma/seed/data/regions.generated.json, prisma/seed/regions.ts, prisma/seed/stores.ts, prisma/seed.ts, .gitignore
migration으로 region 테이블과 store.region_id FK를 생성하고, Prisma 스키마에 Region 모델을 추가했다. CSV 파싱 스크립트로 생성된 regions.generated.json을 slug 기준 upsert로 적재하며, stores 시드도 region_id를 직접 조회해 주입한다.
선택적 JWT 인증 가드
src/global/auth/guards/optional-jwt-auth.guard.ts, src/global/auth/guards/optional-jwt-auth.guard.spec.ts, src/global/auth/auth-global.module.ts, src/global/auth/index.ts
OptionalJwtAuthGuardJwtAuthGuard를 상속해 user가 없을 때 undefined를 반환하도록 오버라이드하고, AuthGlobalModule에 provider/export로 등록했다.
지역 GraphQL API
src/features/region/region.types.graphql, src/features/region/types/region-output.type.ts, src/features/region/constants/region-error-messages.ts, src/features/region/dto/inputs/search-regions.input.ts, src/features/region/repositories/region.repository.ts, src/features/region/services/region-mappers.helper.ts, src/features/region/services/region.service.ts, src/features/region/resolvers/region-query.resolver.ts, src/features/region/region.module.ts, src/features/region/index.ts, src/features/region/...spec.ts
regionGroups/regions/searchRegions 쿼리와 관련 타입을 GraphQL 스키마에 정의하고, 리포지토리·서비스·매퍼·리졸버를 구현했다. NotFoundException, 빈 키워드 조기 반환, default limit 처리가 포함된다.
인기 매장 랭킹 — 상수·헬퍼·리포지토리
src/features/store/constants/store-ranking.constants.ts, src/features/store/services/store-ranking.helper.ts, src/features/store/repositories/store.repository.ts, src/features/store/services/store-mappers.helper.ts, src/features/store/...spec.ts
bayesianRating(베이지안 수축)과 popularityScore(log1p 가중합)를 구현하고, StoreRepository에 wishlist/review/order 집계 메서드와 케이크 이미지 조회를 추가했다.
매장 찜 추가/해제
src/features/store/store-wishlist.graphql, src/features/store/constants/store-wishlist-error-messages.ts, src/features/store/repositories/store-wishlist.repository.ts, src/features/store/services/store-wishlist.service.ts, src/features/store/resolvers/store-wishlist-mutation.resolver.ts, src/features/store/...spec.ts
upsertStoreWishlist(soft-delete 복원 포함)·softDeleteStoreWishlist 리포지토리 메서드와, USER 검증·ForbiddenException 처리를 포함한 StoreWishlistService, JwtAuthGuard 보호 뮤테이션 리졸버를 구현했다.
인기 매장 조회 — 서비스·리졸버·모듈 배선
src/features/store/store.types.graphql, src/features/store/types/store-output.type.ts, src/features/store/dto/inputs/popular-stores.input.ts, src/features/store/services/store-listing.service.ts, src/features/store/resolvers/store-query.resolver.ts, src/features/store/store.module.ts, src/features/store/index.ts, src/features/store/...spec.ts
StoreListingService가 집계→점수 산출→정렬→페이지네이션→이미지·찜 병렬 조회→PopularStoreConnection 반환 흐름을 처리하고, OptionalJwtAuthGuard로 보호된 popularStores 쿼리 리졸버를 추가했다.
AppModule 통합 & 테스트 팩토리 확장
src/app.module.ts, src/test/factories/index.ts, src/test/factories/region.factory.ts, src/test/factories/store-wishlist.factory.ts, src/test/factories/store.factory.ts
AppModule에 PickupModule, RegionModule, StoreModule을 추가하고, region/store-wishlist 테스트 팩토리 및 StoreOverrides.region_id 필드를 추가했다.

픽업 캘린더 & 타임슬롯

Layer / File(s) Summary
KST 시간 유틸리티
src/common/utils/kst-time.ts, src/common/utils/kst-time.spec.ts
UTC+9 오프셋 기반으로 날짜 분해, 자정 UTC 변환, YYYY-MM/YYYY-MM-DD 파싱(존재하지 않는 날짜 거부), 일수 차이, HH:MM 포맷 함수를 추가하고 단위 테스트로 UTC 경계 케이스를 검증한다.
픽업 슬롯 서비스 & GraphQL API
src/features/pickup/pickup.types.graphql, src/features/pickup/types/pickup-output.type.ts, src/features/pickup/constants/*, src/features/pickup/services/pickup-slot.service.ts, src/features/pickup/resolvers/pickup-query.resolver.ts, src/features/pickup/pickup.module.ts, src/features/pickup/index.ts, src/features/pickup/...spec.ts
픽업 정책 상수(개점·마감·리드타임)와 오류 상수를 정의하고, pickupCalendar(PAST/OUT_OF_RANGE 판정)와 pickupTimeSlots(컷오프 기반 오전/오후 슬롯 분리)를 구현했다.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant StoreQueryResolver
  participant StoreListingService
  participant StoreRepository
  participant StoreWishlistRepository

  Client->>StoreQueryResolver: popularStores(input?)
  Note over StoreQueryResolver: OptionalJwtAuthGuard로 accountId 선택 추출
  StoreQueryResolver->>StoreListingService: popularStores(input, accountId?)
  StoreListingService->>StoreRepository: findActiveStoresForRanking(regionIds?)
  StoreListingService->>StoreRepository: aggregateWishlistCounts + aggregateReviewStats + aggregateRecentOrderCounts + globalReviewAverage
  Note over StoreListingService: bayesianRating → popularityScore 계산 후 정렬
  StoreListingService->>StoreRepository: findStoreCakeImages(pageStoreIds)
  StoreListingService->>StoreWishlistRepository: findWishlistedStoreIds(accountId, pageStoreIds)
  StoreListingService-->>StoreQueryResolver: PopularStoreConnection
  StoreQueryResolver-->>Client: items, totalCount, hasMore, rankedAt
Loading
sequenceDiagram
  participant Client
  participant PickupQueryResolver
  participant PickupSlotService

  Client->>PickupQueryResolver: pickupCalendar(yearMonth)
  PickupQueryResolver->>PickupSlotService: pickupCalendar(yearMonth, now)
  PickupSlotService->>PickupSlotService: parseKstYearMonth → 각 일자 PAST/OUT_OF_RANGE/selectable 판정
  PickupSlotService-->>Client: PickupCalendar { yearMonth, days[] }

  Client->>PickupQueryResolver: pickupTimeSlots(date)
  PickupQueryResolver->>PickupSlotService: pickupTimeSlots(date, now)
  PickupSlotService->>PickupSlotService: parseKstDate → kstDayDiff → 리드타임 컷오프 → 오전/오후 슬롯 생성
  PickupSlotService-->>Client: PickupTimeSlots { date, morning[], afternoon[] }
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목이 변경 내용의 주요 4가지 기능(지역, 인기매장, 매장찜, 픽업슬롯)을 간결하게 요약하고 있어 전체 변경 범위를 명확히 전달합니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

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.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

🧹 knip — dead-code 리포트

요약 항목 없음
전체 리포트
(knip 출력 없음 — 이슈 0이거나 실행 실패)

청소 후보(오탐 가능) · 기준 docs/guide/architecture-conventions.md

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

🩺 NestJS Doctor — 88/100 (Good)

진단 254건 (error 0).

Category error warning info
architecture 0 0 13
correctness 0 105 0
performance 0 24 16
schema 0 0 83
security 0 13 0
architecture / security 상위 항목
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal type 'IAuditLogRepository'.
  • warning security/security/no-exposed-env-vars: Direct 'process.env.NODE_ENV' access in 'AuthController'. Use ConfigService instead.
  • warning security/security/require-guards-on-endpoints: Endpoint 'start' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'callback' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'refresh' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'logout' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'sellerLogin' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'sellerRefresh' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'sellerLogout' has no @UseGuards() at class or method level.
  • warning security/security/require-guards-on-endpoints: Endpoint 'devIssueToken' has no @UseGuards() at class or method level.
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal module '@/features/conversation/repositories/conversation.repository'.
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal type 'ConversationRepository'.
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal module '@/features/order/repositories/order.repository'.
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal type 'OrderRepository'.
  • info architecture/architecture/no-barrel-export-internals: Barrel file re-exports internal module '@/features/product/repositories/product.repository'.

오탐 포함 가능 · 기준 docs/guide/architecture-conventions.md

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 97.42% 3886/3989
🟢 Branches 93.21% 1208/1296
🟢 Functions 94.64% 724/765
🟢 Lines 97.82% 3551/3630

Test suite run success

1344 tests passing in 159 suites.

Report generated by 🧪jest coverage report action from f1128cc

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d896ef6ab

ℹ️ 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".

is_active: true,
deleted_at: null,
...(regionIds && regionIds.length > 0
? { region_id: { in: regionIds } }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Backfill store.region_id before using it as the only filter

When this is deployed onto an existing database, every current store keeps region_id = NULL because the migration only adds the nullable column and the seller store update path still writes only the free-text address fields. In that state, any popularStores request with a region filter excludes all pre-existing stores in that region even if their address_city/address_district match, so the new regional popular-store list will look empty until an out-of-band backfill happens.

Useful? React with 👍 / 👎.

reason: PICKUP_DAY_REASON.OUT_OF_RANGE,
};
}
return { date: formatKstDate(date), selectable: true, reason: null };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Mark today unavailable once all time slots are closed

For today's date after the last viable slot cutoff (e.g. KST 18:31 with a 60-minute lead time and the last slot at 19:30), pickupTimeSlots returns every slot as unavailable, but pickupCalendar still marks today as selectable: true because it only checks the day offset. Calendar clients can therefore offer a date that cannot produce any selectable time, forcing users into a dead-end selection.

Useful? React with 👍 / 👎.

@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.

Actionable comments posted: 13

🧹 Nitpick comments (1)
src/features/pickup/resolvers/pickup-query.resolver.spec.ts (1)

5-19: ⚡ Quick win

Resolver 단위 테스트를 서비스 mock 기반으로 분리하세요.

현재 테스트는 실제 PickupSlotService를 생성해 사용해서 resolver 위임 계약(호출 인자, 예외 전파)을 독립적으로 검증하지 못합니다. 서비스는 mock/stub으로 대체하고, 정상 위임 + 예외 전파 케이스를 각각 추가하는 쪽이 더 안정적입니다.

예시 리팩터링
 describe('PickupQueryResolver', () => {
-  const service = new PickupSlotService();
+  const service = {
+    pickupCalendar: jest.fn(),
+    pickupTimeSlots: jest.fn(),
+  } as unknown as PickupSlotService;
   const resolver = new PickupQueryResolver(service);

   it('pickupCalendar: 서비스에 위임해 월 달력을 반환한다', () => {
+    (service.pickupCalendar as jest.Mock).mockReturnValue({
+      yearMonth: '2026-06',
+      days: [],
+    });
     const result = resolver.pickupCalendar('2026-06');
+    expect(service.pickupCalendar).toHaveBeenCalledWith('2026-06');
     expect(result.yearMonth).toBe('2026-06');
-    expect(result.days).toHaveLength(30);
+    expect(result.days).toHaveLength(0);
   });
+
+  it('pickupCalendar: 서비스 예외를 그대로 전파한다', () => {
+    (service.pickupCalendar as jest.Mock).mockImplementation(() => {
+      throw new Error('bad request');
+    });
+    expect(() => resolver.pickupCalendar('invalid')).toThrow('bad request');
+  });
 });

As per coding guidelines src/**/*.spec.ts: 테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하고, 정상 흐름뿐 아니라 주요 예외/분기 케이스를 포함해야 합니다.

🤖 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/features/pickup/resolvers/pickup-query.resolver.spec.ts` around lines 5 -
19, Replace the real PickupSlotService instance in the PickupQueryResolver test
setup with a mock or jest.spyOn instance. Configure the mock to return the
expected values for the pickupCalendar and pickupTimeSlots method calls used in
the existing tests. Additionally, add new test cases that verify exception
propagation from the service to the resolver (when the service throws errors,
ensure the resolver propagates those errors correctly). This ensures the tests
validate the resolver's delegation contract independently without relying on
actual service implementation.

Source: Coding guidelines

🤖 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.

Inline comments:
In `@prisma/migrations/20260617194530_add_region_and_store_region/migration.sql`:
- Around line 2-33: The migration combines adding the region_id column to the
store table with creating its index and foreign key constraint in a single
operation, which risks locking issues on production with large datasets. Split
this into two separate migrations: the first migration should include adding the
region_id column to store and creating the index idx_store_region_id, plus the
self-referencing foreign key constraint region_parent_id_fkey on the region
table. Create a second migration in a separate file to add the
store_region_id_fkey foreign key constraint that links store.region_id to
region.id, scheduled to run during low-traffic periods after data validation.

In `@prisma/seed/regions.ts`:
- Around line 41-46: The upsert update sections in the regions seed file are
missing hierarchical normalization fields that should be set to maintain data
consistency. For the level1 update block, add level: 1 and parent_id: null to
the update object to ensure proper hierarchy normalization. For the level2
update block (which also applies to the similar section mentioned), add level: 2
to the update object. This ensures that existing contaminated data is corrected
during the upsert operation rather than being retained with stale hierarchy
values, preventing data drift as specified in the coding guidelines for Prisma
relationship changes.

In `@src/features/region/dto/inputs/search-regions.input.spec.ts`:
- Around line 12-48: The SearchRegionsInput class has a `@MaxLength`(80)
constraint on the keyword field, but the test suite in
SearchRegionsInput.spec.ts is missing boundary validation test cases for this
constraint. Add two new test cases to the describe block: one that validates a
keyword with exactly 80 characters passes validation, and another that validates
a keyword with 81 characters fails validation with the appropriate maxLength
constraint error. Use the existing validate function and build helper pattern to
implement these tests.

In `@src/features/region/resolvers/region-query.resolver.spec.ts`:
- Around line 20-23: The region-query.resolver.spec.ts file violates spec.ts
guidelines by using createTestingModuleWithRealDb which introduces real database
dependencies. Split this test file into two: move the existing real database
tests with createTestingModuleWithRealDb and the truncate cleanup logic (lines
20-23 and 33-35) to a new integration test file
(region-query.resolver.integration.ts), and refactor the spec.ts file to use
Test.createTestingModule instead, replacing the real RegionService and
RegionRepository with mocked/stubbed versions. Update all test cases in the
spec.ts file to work with the mocked RegionService instead of querying the real
database.

In `@src/features/region/services/region.service.spec.ts`:
- Around line 19-34: The region.service.spec.ts file violates test isolation by
using createTestingModuleWithRealDb with real database connections and
truncateAll for cleanup, which couples the tests to actual DB state. Convert
this to a unit test by replacing the RegionRepository provider with a mock or
stub implementation in the testing module setup, remove the real database
lifecycle methods (closeTruncateConnection and disconnectTestPrismaClient from
afterAll, and truncateAll from beforeEach), and relocate the real database
validation logic to a separate integration or e2e test suite. This ensures the
RegionService unit tests are fully isolated and do not depend on database state
or environment.

In `@src/features/store/resolvers/store-query.resolver.spec.ts`:
- Around line 20-40: The test for StoreQueryResolver is using a real database
connection via createTestingModuleWithRealDb, which violates the guideline that
resolver specs should mock external dependencies. Replace the real database
setup in the beforeAll hook by using a standard testing module with mock/stub
providers for StoreListingService, StoreRepository, and StoreWishlistRepository
instead of the actual implementations. Remove the afterAll hook that contains
closeTruncateConnection and disconnectTestPrismaClient, and remove the
beforeEach hook that calls truncateAll. This allows the resolver spec to test
only the resolver's branching logic without depending on actual database
connections, improving test speed and reproducibility.
- Around line 42-63: The test cases for the popularStores method in the
store-query.resolver.spec.ts file currently only cover the anonymous user path
by passing undefined. Add a new test case for the authenticated user scenario
that passes an actual CurrentUser object to the popularStores method, ensuring
the parseAccountId branching logic and authenticated path behavior are verified.
This test should validate that the resolver correctly handles and processes the
user context when a logged-in user calls the method, covering the branch that is
skipped when the user parameter is undefined.

In `@src/features/store/resolvers/store-wishlist-mutation.resolver.spec.ts`:
- Around line 22-32: The test setup in the beforeEach hook is using
createTestingModuleWithRealDb which violates test isolation principles by
creating direct database dependencies. Replace createTestingModuleWithRealDb
with the standard createTestingModule function and mock the StoreWishlistService
dependency instead of using the real implementation. Provide a mock provider for
StoreWishlistService while keeping StoreWishlistMutationResolver as the resolver
under test, allowing the resolver unit tests to focus on resolver logic without
database dependency. Apply this same refactoring pattern to all other test
blocks mentioned (lines 39-41, 53-61, and 88-91) to ensure consistent test
isolation across the entire spec file.

In `@src/features/store/services/store-listing.service.spec.ts`:
- Around line 26-45: The test file violates guidelines by directly depending on
a real database through createTestingModuleWithRealDb and truncateAll, and using
real time via Date.now(). Replace the real database dependencies by mocking
StoreRepository and StoreWishlistRepository providers instead of using the real
implementations. Mock or stub the time source used in the tests (replace direct
Date.now() calls with controllable fake timers or a mocked time service). Update
the beforeAll hook to create a testing module with mocked providers, and
simplify the afterAll and beforeEach hooks to only clean up mock state rather
than calling real database operations like closeTruncateConnection and
disconnectTestPrismaClient. This ensures tests are deterministic and do not
depend on external systems.

In `@src/features/store/services/store-wishlist.service.spec.ts`:
- Around line 20-43: The StoreWishlistService test currently depends on a real
database using createTestingModuleWithRealDb, truncateAll,
closeTruncateConnection, and disconnectTestPrismaClient, which violates the unit
test isolation requirement. Refactor this test to mock or stub the
StoreWishlistRepository and StoreRepository dependencies instead of using real
database operations, remove all the database lifecycle hooks (beforeAll,
afterAll with real DB cleanup, beforeEach with truncateAll), and create a
separate integration test file for actual database validation scenarios. Update
the test setup to use standard NestJS testing module creation with mocked
repository providers that return controlled test data.
- Around line 123-152: The test suite for the removeStoreFromWishlist describe
block is missing validation of error handling when an invalid storeId is
provided. Add a new test case after the existing tests that calls the
removeStoreFromWishlist method with an invalid or malformed storeId parameter
(for example, a non-numeric string instead of store.id.toString()) and verify
that it properly throws an error or rejects the promise with appropriate error
handling, ensuring the input parsing failure branch is covered.

In `@src/features/store/services/store-wishlist.service.ts`:
- Around line 43-53: The removeStoreFromWishlist method is missing the USER
permission validation that exists in the other method mentioned at lines 25-28.
Add the same USER authorization check at the beginning of the
removeStoreFromWishlist method before performing the soft-delete operation to
ensure consistent permission enforcement across both wishlist operations. This
will align with the policy that only regular users can manage store wishlists
and maintain consistent authorization rules at the service level.

In `@src/global/auth/guards/optional-jwt-auth.guard.ts`:
- Around line 15-20: The handleRequest method in OptionalJwtAuthGuard ignores
all errors by not checking the _err parameter, which prevents distinguishing
between acceptable missing tokens (UnauthorizedException) and security issues
like disabled accounts (ForbiddenException). Modify the handleRequest method to
check if _err is a ForbiddenException and throw it to reject such requests,
while allowing UnauthorizedException or undefined errors to proceed normally.
Additionally, add test cases to verify that ForbiddenException is properly
thrown and that UnauthorizedException or missing tokens are properly allowed.

---

Nitpick comments:
In `@src/features/pickup/resolvers/pickup-query.resolver.spec.ts`:
- Around line 5-19: Replace the real PickupSlotService instance in the
PickupQueryResolver test setup with a mock or jest.spyOn instance. Configure the
mock to return the expected values for the pickupCalendar and pickupTimeSlots
method calls used in the existing tests. Additionally, add new test cases that
verify exception propagation from the service to the resolver (when the service
throws errors, ensure the resolver propagates those errors correctly). This
ensures the tests validate the resolver's delegation contract independently
without relying on actual service implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 95050d63-6aa4-4bb2-9a64-efd0af5509ae

📥 Commits

Reviewing files that changed from the base of the PR and between b4d5f2c and 7d896ef.

📒 Files selected for processing (65)
  • .gitignore
  • prisma/migrations/20260617194530_add_region_and_store_region/migration.sql
  • prisma/schema.prisma
  • prisma/seed.ts
  • prisma/seed/data/regions.generated.json
  • prisma/seed/regions.ts
  • prisma/seed/stores.ts
  • scripts/generate-region-seed.ts
  • src/app.module.ts
  • src/common/utils/kst-time.spec.ts
  • src/common/utils/kst-time.ts
  • src/features/pickup/constants/pickup-error-messages.ts
  • src/features/pickup/constants/pickup.constants.ts
  • src/features/pickup/index.ts
  • src/features/pickup/pickup.module.ts
  • src/features/pickup/pickup.types.graphql
  • src/features/pickup/resolvers/pickup-query.resolver.spec.ts
  • src/features/pickup/resolvers/pickup-query.resolver.ts
  • src/features/pickup/services/pickup-slot.service.spec.ts
  • src/features/pickup/services/pickup-slot.service.ts
  • src/features/pickup/types/pickup-output.type.ts
  • src/features/region/constants/region-error-messages.ts
  • src/features/region/dto/inputs/search-regions.input.spec.ts
  • src/features/region/dto/inputs/search-regions.input.ts
  • src/features/region/index.ts
  • src/features/region/region.module.ts
  • src/features/region/region.types.graphql
  • src/features/region/repositories/region.repository.ts
  • src/features/region/resolvers/region-query.resolver.spec.ts
  • src/features/region/resolvers/region-query.resolver.ts
  • src/features/region/services/region-mappers.helper.ts
  • src/features/region/services/region.service.spec.ts
  • src/features/region/services/region.service.ts
  • src/features/region/types/region-output.type.ts
  • src/features/store/constants/store-ranking.constants.ts
  • src/features/store/constants/store-wishlist-error-messages.ts
  • src/features/store/dto/inputs/popular-stores.input.spec.ts
  • src/features/store/dto/inputs/popular-stores.input.ts
  • src/features/store/index.ts
  • src/features/store/repositories/store-wishlist.repository.ts
  • src/features/store/repositories/store.repository.ts
  • src/features/store/resolvers/store-query.resolver.spec.ts
  • src/features/store/resolvers/store-query.resolver.ts
  • src/features/store/resolvers/store-wishlist-mutation.resolver.spec.ts
  • src/features/store/resolvers/store-wishlist-mutation.resolver.ts
  • src/features/store/services/store-listing.service.spec.ts
  • src/features/store/services/store-listing.service.ts
  • src/features/store/services/store-mappers.helper.spec.ts
  • src/features/store/services/store-mappers.helper.ts
  • src/features/store/services/store-ranking.helper.spec.ts
  • src/features/store/services/store-ranking.helper.ts
  • src/features/store/services/store-wishlist.service.spec.ts
  • src/features/store/services/store-wishlist.service.ts
  • src/features/store/store-wishlist.graphql
  • src/features/store/store.module.ts
  • src/features/store/store.types.graphql
  • src/features/store/types/store-output.type.ts
  • src/global/auth/auth-global.module.ts
  • src/global/auth/guards/optional-jwt-auth.guard.spec.ts
  • src/global/auth/guards/optional-jwt-auth.guard.ts
  • src/global/auth/index.ts
  • src/test/factories/index.ts
  • src/test/factories/region.factory.ts
  • src/test/factories/store-wishlist.factory.ts
  • src/test/factories/store.factory.ts

Comment on lines +2 to +33
ALTER TABLE `store` ADD COLUMN `region_id` BIGINT UNSIGNED NULL;

-- CreateTable
CREATE TABLE `region` (
`id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
`parent_id` BIGINT UNSIGNED NULL,
`level` TINYINT UNSIGNED NOT NULL,
`name` VARCHAR(80) NOT NULL,
`slug` VARCHAR(120) NOT NULL,
`sort_order` INTEGER NOT NULL DEFAULT 0,
`is_active` BOOLEAN NOT NULL DEFAULT true,
`center_lat` DECIMAL(10, 7) NULL,
`center_lng` DECIMAL(10, 7) NULL,
`created_at` DATETIME(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3),
`updated_at` DATETIME(3) NOT NULL,
`deleted_at` DATETIME(3) NULL,

UNIQUE INDEX `region_slug_key`(`slug`),
INDEX `idx_region_parent_sort`(`parent_id`, `sort_order`),
INDEX `idx_region_level_active`(`level`, `is_active`),
INDEX `idx_region_deleted_at`(`deleted_at`),
PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;

-- CreateIndex
CREATE INDEX `idx_store_region_id` ON `store`(`region_id`);

-- AddForeignKey
ALTER TABLE `region` ADD CONSTRAINT `region_parent_id_fkey` FOREIGN KEY (`parent_id`) REFERENCES `region`(`id`) ON DELETE SET NULL ON UPDATE CASCADE;

-- AddForeignKey
ALTER TABLE `store` ADD CONSTRAINT `store_region_id_fkey` FOREIGN KEY (`region_id`) REFERENCES `region`(`id`) ON DELETE SET NULL ON UPDATE CASCADE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

운영 반영 시 store FK 추가 구간의 잠금 리스크를 분리 배포로 완화해 주세요.

store에 컬럼 추가(Line 2) 후 인덱스(Line 27)와 FK(Line 33)를 같은 변경 세트로 적용하면, 실제 운영 데이터량에서 메타데이터 락/검증 스캔으로 쓰기 지연이 발생할 수 있습니다. region_id 컬럼·인덱스 선배포 → 데이터 검증 → 저부하 시간 FK 적용의 단계적 적용 계획이 필요합니다.
As per coding guidelines, prisma/**: "파괴적 변경이나 운영 반영 시 위험한 변경은 명확히 지적하세요."

🤖 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 `@prisma/migrations/20260617194530_add_region_and_store_region/migration.sql`
around lines 2 - 33, The migration combines adding the region_id column to the
store table with creating its index and foreign key constraint in a single
operation, which risks locking issues on production with large datasets. Split
this into two separate migrations: the first migration should include adding the
region_id column to store and creating the index idx_store_region_id, plus the
self-referencing foreign key constraint region_parent_id_fkey on the region
table. Create a second migration in a separate file to add the
store_region_id_fkey foreign key constraint that links store.region_id to
region.id, scheduled to run during low-traffic periods after data validation.

Source: Coding guidelines

Comment thread prisma/seed/regions.ts
Comment thread src/features/region/dto/inputs/search-regions.input.spec.ts
Comment on lines +20 to +23
beforeAll(async () => {
const { module, prisma: p } = await createTestingModuleWithRealDb({
providers: [RegionQueryResolver, RegionService, RegionRepository],
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

실DB 의존 테스트 구성이 spec 규칙과 충돌합니다.

Line [20]-Line [23]에서 실제 DB 모듈을 구성하고 Line [33]-Line [35]에서 truncate로 상태를 맞추는 방식은 src/**/*.spec.ts 기준(DB 의존성 mock/stub 통제)에 어긋납니다. 이 스위트는 integration 테스트로 분리하고, resolver spec은 RegionService stub으로 대체해 단위 테스트로 분리해 주세요.

As per coding guidelines src/**/*.spec.ts: "테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지, 정상 흐름뿐 아니라 주요 예외/분기 케이스가 포함되는지 확인하세요."

Also applies to: 33-35

🤖 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/features/region/resolvers/region-query.resolver.spec.ts` around lines 20
- 23, The region-query.resolver.spec.ts file violates spec.ts guidelines by
using createTestingModuleWithRealDb which introduces real database dependencies.
Split this test file into two: move the existing real database tests with
createTestingModuleWithRealDb and the truncate cleanup logic (lines 20-23 and
33-35) to a new integration test file (region-query.resolver.integration.ts),
and refactor the spec.ts file to use Test.createTestingModule instead, replacing
the real RegionService and RegionRepository with mocked/stubbed versions. Update
all test cases in the spec.ts file to work with the mocked RegionService instead
of querying the real database.

Source: Coding guidelines

Comment on lines +19 to +34
beforeAll(async () => {
const { module, prisma: p } = await createTestingModuleWithRealDb({
providers: [RegionService, RegionRepository],
});
service = module.get(RegionService);
prisma = p;
});

afterAll(async () => {
await closeTruncateConnection();
await disconnectTestPrismaClient();
});

beforeEach(async () => {
await truncateAll();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

.spec.ts에서 실DB 의존을 직접 사용해 테스트 격리가 깨집니다.

createTestingModuleWithRealDb + truncateAll 기반 구성은 이 스펙이 DB 상태/환경에 결합되게 만듭니다. RegionRepository를 mock/stub으로 대체한 단위 스펙으로 전환하고, 실DB 검증은 별도 integration/e2e 스위트로 분리해 주세요.
As per coding guidelines src/**/*.spec.ts: "테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지 ... 확인하세요."

🤖 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/features/region/services/region.service.spec.ts` around lines 19 - 34,
The region.service.spec.ts file violates test isolation by using
createTestingModuleWithRealDb with real database connections and truncateAll for
cleanup, which couples the tests to actual DB state. Convert this to a unit test
by replacing the RegionRepository provider with a mock or stub implementation in
the testing module setup, remove the real database lifecycle methods
(closeTruncateConnection and disconnectTestPrismaClient from afterAll, and
truncateAll from beforeEach), and relocate the real database validation logic to
a separate integration or e2e test suite. This ensures the RegionService unit
tests are fully isolated and do not depend on database state or environment.

Source: Coding guidelines

Comment on lines +26 to +45
beforeAll(async () => {
const { module, prisma: p } = await createTestingModuleWithRealDb({
providers: [
StoreListingService,
StoreRepository,
StoreWishlistRepository,
],
});
service = module.get(StoreListingService);
prisma = p;
});

afterAll(async () => {
await closeTruncateConnection();
await disconnectTestPrismaClient();
});

beforeEach(async () => {
await truncateAll();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

실DB·실시간 의존 테스트는 현재 가이드라인을 위반합니다.

이 스펙은 createTestingModuleWithRealDb/truncateAll로 DB에 직접 의존하고, 시간도 Date.now()를 직접 사용합니다. 저장소와 시간 소스를 mock/stub(또는 fake timer)으로 고정해 결정론적으로 검증해 주세요.
As per coding guidelines, src/**/*.spec.ts: 테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지 확인하세요.

Also applies to: 137-137

🤖 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/features/store/services/store-listing.service.spec.ts` around lines 26 -
45, The test file violates guidelines by directly depending on a real database
through createTestingModuleWithRealDb and truncateAll, and using real time via
Date.now(). Replace the real database dependencies by mocking StoreRepository
and StoreWishlistRepository providers instead of using the real implementations.
Mock or stub the time source used in the tests (replace direct Date.now() calls
with controllable fake timers or a mocked time service). Update the beforeAll
hook to create a testing module with mocked providers, and simplify the afterAll
and beforeEach hooks to only clean up mock state rather than calling real
database operations like closeTruncateConnection and disconnectTestPrismaClient.
This ensures tests are deterministic and do not depend on external systems.

Source: Coding guidelines

Comment on lines +20 to +43
describe('StoreWishlistService (real DB)', () => {
let service: StoreWishlistService;
let prisma: PrismaClient;

beforeAll(async () => {
const { module, prisma: p } = await createTestingModuleWithRealDb({
providers: [
StoreWishlistService,
StoreWishlistRepository,
StoreRepository,
],
});
service = module.get(StoreWishlistService);
prisma = p;
});

afterAll(async () => {
await closeTruncateConnection();
await disconnectTestPrismaClient();
});

beforeEach(async () => {
await truncateAll();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

실DB 의존 테스트로 인해 결정성/속도/격리성이 떨어집니다.

현재 스펙은 DB를 직접 붙이는 통합 형태라 테스트 격리가 약하고 실행 비용이 큽니다. 단위 스펙 레벨에서는 repository를 stub/mock으로 통제하고, 실DB 검증은 별도 integration 스위트로 분리하는 편이 안전합니다.
As per coding guidelines, "src/**/*.spec.ts: 테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지 ... 확인하세요."

🤖 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/features/store/services/store-wishlist.service.spec.ts` around lines 20 -
43, The StoreWishlistService test currently depends on a real database using
createTestingModuleWithRealDb, truncateAll, closeTruncateConnection, and
disconnectTestPrismaClient, which violates the unit test isolation requirement.
Refactor this test to mock or stub the StoreWishlistRepository and
StoreRepository dependencies instead of using real database operations, remove
all the database lifecycle hooks (beforeAll, afterAll with real DB cleanup,
beforeEach with truncateAll), and create a separate integration test file for
actual database validation scenarios. Update the test setup to use standard
NestJS testing module creation with mocked repository providers that return
controlled test data.

Source: Coding guidelines

Comment on lines +123 to +152
describe('removeStoreFromWishlist', () => {
it('찜을 해제한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);
await createStoreWishlist(prisma, {
account_id: account.id,
store_id: store.id,
});

const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);

expect(ok).toBe(true);
expect(await activeWishlistCount(account.id, store.id)).toBe(0);
});

it('찜이 없어도 멱등하게 true를 반환한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);

const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);

expect(ok).toBe(true);
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

removeStoreFromWishlist의 잘못된 storeId 예외 분기도 테스트에 추가해 주세요.

현재 제거 경로는 성공/멱등만 검증하고 있어, 입력 파싱 실패 분기 회귀를 놓칠 수 있습니다.

테스트 추가 예시
   describe('removeStoreFromWishlist', () => {
@@
     it('찜이 없어도 멱등하게 true를 반환한다', async () => {
@@
       expect(ok).toBe(true);
     });
+
+    it('유효하지 않은 storeId면 BadRequestException', async () => {
+      const account = await createAccount(prisma, { account_type: 'USER' });
+      await expect(
+        service.removeStoreFromWishlist(account.id, 'not-a-number'),
+      ).rejects.toThrow(BadRequestException);
+    });
   });

As per coding guidelines, "src/**/*.spec.ts: ... 정상 흐름뿐 아니라 주요 예외/분기 케이스가 포함되는지 확인하세요."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe('removeStoreFromWishlist', () => {
it('찜을 해제한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);
await createStoreWishlist(prisma, {
account_id: account.id,
store_id: store.id,
});
const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);
expect(ok).toBe(true);
expect(await activeWishlistCount(account.id, store.id)).toBe(0);
});
it('찜이 없어도 멱등하게 true를 반환한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);
const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);
expect(ok).toBe(true);
});
});
describe('removeStoreFromWishlist', () => {
it('찜을 해제한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);
await createStoreWishlist(prisma, {
account_id: account.id,
store_id: store.id,
});
const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);
expect(ok).toBe(true);
expect(await activeWishlistCount(account.id, store.id)).toBe(0);
});
it('찜이 없어도 멱등하게 true를 반환한다', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
const store = await createStore(prisma);
const ok = await service.removeStoreFromWishlist(
account.id,
store.id.toString(),
);
expect(ok).toBe(true);
});
it('유효하지 않은 storeId면 BadRequestException', async () => {
const account = await createAccount(prisma, { account_type: 'USER' });
await expect(
service.removeStoreFromWishlist(account.id, 'not-a-number'),
).rejects.toThrow(BadRequestException);
});
});
🤖 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/features/store/services/store-wishlist.service.spec.ts` around lines 123
- 152, The test suite for the removeStoreFromWishlist describe block is missing
validation of error handling when an invalid storeId is provided. Add a new test
case after the existing tests that calls the removeStoreFromWishlist method with
an invalid or malformed storeId parameter (for example, a non-numeric string
instead of store.id.toString()) and verify that it properly throws an error or
rejects the promise with appropriate error handling, ensuring the input parsing
failure branch is covered.

Source: Coding guidelines

Comment on lines +43 to +53
async removeStoreFromWishlist(
accountId: bigint,
storeIdStr: string,
): Promise<boolean> {
const storeId = parseId(storeIdStr);
await this.wishlistRepo.softDeleteStoreWishlist({
accountId,
storeId,
now: new Date(),
});
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

removeStoreFromWishlist에도 USER 권한 검증을 동일하게 적용하세요.

Line 43-53 경로는 Line 25-28과 달리 USER 검증 없이 soft-delete를 수행합니다. 현재 정책(“매장 찜은 일반 사용자만”)과 불일치해서 권한 규칙이 엔드포인트별로 달라집니다.

🔧 제안 수정안
   async removeStoreFromWishlist(
     accountId: bigint,
     storeIdStr: string,
   ): Promise<boolean> {
+    const isUser = await this.wishlistRepo.isActiveUserAccount(accountId);
+    if (!isUser) {
+      throw new ForbiddenException(STORE_WISHLIST_ERRORS.USER_ONLY);
+    }
     const storeId = parseId(storeIdStr);
     await this.wishlistRepo.softDeleteStoreWishlist({
       accountId,
       storeId,
       now: new Date(),
     });
     return true;
   }

As per coding guidelines, "인증/로깅/에러 처리에서 민감정보가 노출되지 않는지 확인하세요." 및 서비스 레벨에서 인증 규칙을 일관되게 적용해야 합니다.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async removeStoreFromWishlist(
accountId: bigint,
storeIdStr: string,
): Promise<boolean> {
const storeId = parseId(storeIdStr);
await this.wishlistRepo.softDeleteStoreWishlist({
accountId,
storeId,
now: new Date(),
});
return true;
async removeStoreFromWishlist(
accountId: bigint,
storeIdStr: string,
): Promise<boolean> {
const isUser = await this.wishlistRepo.isActiveUserAccount(accountId);
if (!isUser) {
throw new ForbiddenException(STORE_WISHLIST_ERRORS.USER_ONLY);
}
const storeId = parseId(storeIdStr);
await this.wishlistRepo.softDeleteStoreWishlist({
accountId,
storeId,
now: new Date(),
});
return true;
}
🤖 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/features/store/services/store-wishlist.service.ts` around lines 43 - 53,
The removeStoreFromWishlist method is missing the USER permission validation
that exists in the other method mentioned at lines 25-28. Add the same USER
authorization check at the beginning of the removeStoreFromWishlist method
before performing the soft-delete operation to ensure consistent permission
enforcement across both wishlist operations. This will align with the policy
that only regular users can manage store wishlists and maintain consistent
authorization rules at the service level.

Source: Coding guidelines

Comment on lines +15 to +20
override handleRequest<TUser = JwtUser>(
_err: unknown,
user: TUser | false | null,
): TUser | undefined {
return user || undefined;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In NestJS Passport AuthGuard, what error types are passed to handleRequest(err, user, info, context, status), and is it recommended to swallow all errors for optional JWT auth or only Unauthorized/no-token cases?

💡 Result:

In NestJS Passport AuthGuard, the handleRequest(err, user, info, context, status) method is the designated hook for customizing how authentication results are processed [1][2]. The parameters passed to this method are: - err: Any error encountered during the authentication process (e.g., database connection issues, unexpected strategy failures). - user: The user object returned by the strategy upon successful authentication. - info: Additional information or details about the authentication result (e.g., failure reasons, challenge messages from Passport strategies). - context: The NestJS ExecutionContext. - status: A status code or additional status information (often provided by Passport strategies when they fail) [3][4][5]. Regarding optional JWT authentication, it is not recommended to blindly "swallow" all errors. If you ignore all errors, you may inadvertently suppress critical system issues or actual authentication failures that should be logged or handled (e.g., a token that is malformed vs. a token that is expired vs. a database error) [2][6]. For optional authentication, the recommended practice is to create a custom guard by extending AuthGuard and overriding handleRequest to return user || null only when appropriate [7][2]. A safe implementation typically checks if the error is a genuine system or strategy error before deciding to pass or fail [6][8]: typescript import { Injectable, UnauthorizedException } from '`@nestjs/common`'; import { AuthGuard } from '`@nestjs/passport`'; `@Injectable`() export class OptionalJwtAuthGuard extends AuthGuard('jwt') { handleRequest(err, user, info, context, status) { // If there is an actual error or authentication failure, // you might still want to throw or log it. if (err || !user) { // Return null to treat as "unauthenticated" rather than throwing return null; } return user; } } By returning null instead of throwing an UnauthorizedException, the request will proceed with request.user as null (or undefined), allowing your route handlers to differentiate between authenticated and guest users [7][9]. Always ensure you are explicitly handling or logging the err and info objects if you need to distinguish between a missing token and a technically invalid one [2][6].

Citations:


🏁 Script executed:

# First, locate and inspect the OptionalJwtAuthGuard file
find . -name "optional-jwt-auth.guard.ts" -type f

Repository: CaQuick/caquick-be

Length of output: 114


🏁 Script executed:

# Check the full implementation of the guard file
cat -n src/global/auth/guards/optional-jwt-auth.guard.ts

Repository: CaQuick/caquick-be

Length of output: 775


🏁 Script executed:

# Check if there's a parent JwtAuthGuard to understand inheritance
find . -name "*jwt-auth.guard.ts" -type f | head -20

Repository: CaQuick/caquick-be

Length of output: 157


🏁 Script executed:

# Check the imports and see what strategies/decorators are being used
rg "JwtAuthGuard\|JwtStrategy" --type ts -A 5 -B 2 | head -50

Repository: CaQuick/caquick-be

Length of output: 44


🏁 Script executed:

# Inspect the parent JwtAuthGuard implementation
cat -n src/global/auth/guards/jwt-auth.guard.ts

Repository: CaQuick/caquick-be

Length of output: 1010


🏁 Script executed:

# Check if there's a JwtStrategy and what it does
find . -name "*jwt.strategy.ts" -type f

Repository: CaQuick/caquick-be

Length of output: 44


🏁 Script executed:

# Look for any error handling or logging in the auth module
rg "UnauthorizedException\|throw\|log" src/global/auth/ --type ts | head -30

Repository: CaQuick/caquick-be

Length of output: 44


🏁 Script executed:

# Check if there are tests for OptionalJwtAuthGuard to understand expected behavior
find . -name "*.spec.ts" -type f | xargs grep -l "OptionalJwtAuthGuard" 2>/dev/null

Repository: CaQuick/caquick-be

Length of output: 119


🏁 Script executed:

# Check how OptionalJwtAuthGuard is used in the codebase
rg "OptionalJwtAuthGuard" --type ts -B 2 -A 2

Repository: CaQuick/caquick-be

Length of output: 2645


🏁 Script executed:

# Verify what the Passport JWT strategy actually throws for different scenarios
rg "JwtStrategy\|jwt\(" src/global/auth/ --type ts -A 10 -B 2

Repository: CaQuick/caquick-be

Length of output: 44


🏁 Script executed:

# Check the test file for OptionalJwtAuthGuard
cat -n src/global/auth/guards/optional-jwt-auth.guard.spec.ts

Repository: CaQuick/caquick-be

Length of output: 999


🏁 Script executed:

# Check if there's a JWT strategy implementation
find . -path "*/node_modules" -prune -o -name "*.strategy.ts" -type f -print | xargs grep -l "jwt" 2>/dev/null

Repository: CaQuick/caquick-be

Length of output: 116


🏁 Script executed:

# Look for how Passport JWT is configured in the auth module
cat -n src/global/auth/auth-global.module.ts

Repository: CaQuick/caquick-be

Length of output: 1312


🏁 Script executed:

# Check the JWT strategy implementation to understand what errors it throws
cat -n src/features/auth/strategies/jwt-bearer.strategy.ts

Repository: CaQuick/caquick-be

Length of output: 2637


🏁 Script executed:

# Search for UnauthorizedException usage in the auth module to see error patterns
rg "UnauthorizedException\|throw" src/global/auth/ src/features/auth/ --type ts -B 2 -A 2

Repository: CaQuick/caquick-be

Length of output: 44


인증 실패와 인증 불가를 구분하지 않으면 계정 상태 이상이 숨겨집니다.

Line 19 구현은 모든 에러를 무시하기 때문에 토큰 부재(UnauthorizedException, 정상)와 계정 비활성화(ForbiddenException, 이상 징후)를 구분하지 못합니다. Optional 인증은 "미인증 허용"이지만, 계정 상태 이상까지 무시하면 보안 문제를 감지할 수 없습니다.

현재 테스트(line 20-22)는 handleRequest(new Error('expired'), user) 시나리오를 검증하고 있으나, 실제 Passport 흐름에서는 계정 비활성화(ForbiddenException)와 토큰 부재(UnauthorizedException)를 구분하는 것이 중요합니다.

수정 예시
-import { Injectable } from '`@nestjs/common`';
+import { Injectable, ForbiddenException } from '`@nestjs/common`';

 export class OptionalJwtAuthGuard extends JwtAuthGuard {
   override handleRequest<TUser = JwtUser>(
-    _err: unknown,
+    err: unknown,
     user: TUser | false | null,
   ): TUser | undefined {
+    // ForbiddenException은 계정 상태 이상이므로 전파하고, 
+    // UnauthorizedException(토큰 부재/검증 실패)만 무시
+    if (err && err instanceof ForbiddenException) {
+      throw err;
+    }
     return user || undefined;
   }
 }

테스트도 ForbiddenException 케이스를 추가로 검증해야 합니다.

🤖 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/global/auth/guards/optional-jwt-auth.guard.ts` around lines 15 - 20, The
handleRequest method in OptionalJwtAuthGuard ignores all errors by not checking
the _err parameter, which prevents distinguishing between acceptable missing
tokens (UnauthorizedException) and security issues like disabled accounts
(ForbiddenException). Modify the handleRequest method to check if _err is a
ForbiddenException and throw it to reject such requests, while allowing
UnauthorizedException or undefined errors to proceed normally. Additionally, add
test cases to verify that ForbiddenException is properly thrown and that
UnauthorizedException or missing tokens are properly allowed.

fix: 릴리즈 리뷰 반영 (pickup CLOSED·region 시드 정규화·keyword 경계 테스트)
@chanwoo7 chanwoo7 merged commit 90faaa7 into main Jun 18, 2026
15 checks passed
@chanwoo7 chanwoo7 deleted the develop branch June 18, 2026 12:22
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.

1 participant