Merged
Conversation
Phase 1: 테스트 커버리지 확보 - seller/user 서비스 단위 테스트 8개 파일 추가 (170 tests) - SellerBaseService, SellerStoreService, SellerContentService, SellerConversationService, UserBaseService, UserEngagementService, UserSearchService 테스트 작성 Phase 2: SellerProductService(1,292줄) 3개 서비스로 분리 - SellerProductCrudService: 상품 CRUD, 이미지, 카테고리/태그 - SellerOptionService: 옵션 그룹/아이템 CRUD - SellerCustomTemplateService: 커스텀 템플릿/텍스트 토큰 - 리졸버/모듈/테스트 일괄 갱신, 원본 파일 삭제 Phase 3: 매직 넘버 상수화 - seller.constants.ts, user.constants.ts 추가 - 텍스트 길이, 가격 범위, 이미지 수 등 비즈니스 규칙 상수 추출
- toOrderDetailOutput: sub-mapper(toOrderItemOutput, toFreeEditOutput) 추출 - toProductOutput: sub-mapper(toOptionGroupOutput, toCustomTemplateOutput) 추출 + 인라인 타입을 인터페이스로 분리 - sellerUpdateProduct/CreateProduct: validateProductPrices 공유 + buildProductUpdateData 추출 - sellerUpdateBanner: resolveNextBannerLinkValues + buildBannerUpdateData 추출 - sellerUpdateStoreBasicInfo: buildStoreBasicInfoUpdateData 추출
- cleanRequiredText, cleanNullableText → src/common/utils/text-cleaner.ts 추출 - toDate, toDateRequired → src/common/utils/date-parser.ts 추출 - SellerBaseService에서 추출된 4개 메서드 제거 (126줄 → 92줄) - 7개 seller 서비스의 76개 호출 지점을 공통 유틸 직접 호출로 변경 - 추출된 유틸에 대한 단위 테스트 10개 추가
- upsertStoreSpecialClosure → update/create 단순 CRUD로 분리 - upsertStoreDailyCapacity → update/create 단순 CRUD로 분리 - update vs create 분기 로직을 Service 레이어로 이동
- seller-error-messages.ts에 모든 에러 메시지 상수를 정의하고, 8개 서비스 파일의 하드코딩 문자열을 상수 참조로 교체. - 동적 메시지를 위한 헬퍼 함수(fieldRangeError, idsMismatchError, invalidIdsError) 추가.
- tsconfig.json paths + Jest moduleNameMapper 설정 - src/ 전체 상대 경로 import를 @/ alias로 변환 - seller/index.ts, user/index.ts barrel file 생성
- tsconfig.json에서 deprecated된 baseUrl 제거, paths를 ./src/*로 변경 - 11개 파일의 src/ bare import를 @/ alias로 변환
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTypeScript 경로 별칭( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Resolver as GraphQLResolver
participant Service as SellerService
participant Repo as SellerRepository
participant DB as PrismaDB
participant Audit as AuditLog
Client->>Resolver: GraphQL mutation/query (seller action)
Resolver->>Service: sellerXxx(accountId, input)
Service->>Repo: requireSellerContext, parseId(s), validation
Repo->>DB: SELECT / INSERT / UPDATE (create or update)
DB-->>Repo: row(s)
Repo-->>Service: row(s)
Service->>Audit: createAuditLog(action, before?, after?)
Audit->>DB: INSERT audit entry
Service-->>Resolver: mapped DTO
Resolver-->>Client: GraphQL response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/seller/repositories/seller.repository.ts (1)
96-116:⚠️ Potential issue | 🟠 Majorsoft-delete 된 동일 날짜 레코드를 다시 만들어도 복구되지 않습니다.
두
create*()메서드는 같은(storeId, date)키가 있으면upsert의 update 브랜치로 들어가는데, 여기서deleted_at을 되돌리지 않습니다. 그래서 soft-delete 된 레코드를 같은 날짜로 재등록하면 값만 갱신되고 삭제 상태는 그대로 남습니다.패치 예시
async createStoreSpecialClosure(args: { storeId: bigint; closureDate: Date; reason: string | null; }) { return this.prisma.storeSpecialClosure.upsert({ where: { store_id_closure_date: { store_id: args.storeId, closure_date: args.closureDate, }, }, create: { store_id: args.storeId, closure_date: args.closureDate, reason: args.reason, }, update: { + deleted_at: null, reason: args.reason, }, }); } @@ async createStoreDailyCapacity(args: { storeId: bigint; capacityDate: Date; capacity: number; }) { return this.prisma.storeDailyCapacity.upsert({ where: { store_id_capacity_date: { store_id: args.storeId, capacity_date: args.capacityDate, }, }, create: { store_id: args.storeId, capacity_date: args.capacityDate, capacity: args.capacity, }, update: { + deleted_at: null, capacity: args.capacity, }, }); }Also applies to: 163-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/seller/repositories/seller.repository.ts` around lines 96 - 116, The upsert in createStoreSpecialClosure (prisma.storeSpecialClosure.upsert) updates reason but does not clear the soft-delete marker, so re-creating a soft-deleted (store_id, closure_date) record leaves deleted_at set; modify the upsert update branch to explicitly reset deleted_at to null (e.g., include deleted_at: null) so the record is revived, and apply the same change to the other create*() upsert methods that use the same (storeId, date) unique key.
🧹 Nitpick comments (4)
src/prisma/soft-delete.middleware.spec.ts (1)
13-56: 테스트 파라미터 타입 지정 완료모든 테스트 케이스의
params객체에SoftDeleteInput타입을 명시적으로 지정하여 암묵적any사용을 제거했습니다. 현재 테스트는 주요 시나리오(필터 자동 추가, deleted_at 조건 우회, 비적용 연산)를 커버하고 있으며, 순수 함수를 테스트하므로 별도의 mock이 필요하지 않습니다.💡 선택적 개선사항: 테스트 커버리지 확장
향후 다음과 같은 추가 테스트 케이스를 고려해볼 수 있습니다:
findMany연산에 대한 필터 추가 테스트args가 없는 경우 처리 테스트- 복잡한
where조건(중첩된 AND/OR)에 대한 테스트현재 리팩터링 PR 범위에서는 필수는 아니지만, 향후 미들웨어 안정성을 높이는 데 도움이 될 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/prisma/soft-delete.middleware.spec.ts` around lines 13 - 56, The tests should explicitly type the params objects to remove implicit any: import and use the SoftDeleteInput type and annotate each params declaration (used with applySoftDeleteArgs) as SoftDeleteInput; ensure the test file imports SoftDeleteInput from the module that exports it (where applySoftDeleteArgs is defined) and update the three test cases (filter-added, deleted_at bypass, findUnique no-filter) to use that type for their params.src/common/utils/text-cleaner.spec.ts (1)
36-38:cleanNullableText의 공백-only 입력 분기 테스트를 추가해 주세요.
''는 검증하지만' '케이스가 빠져 있어 trim 이후 null 처리 회귀를 놓칠 수 있습니다.테스트 보강 예시
describe('cleanNullableText', () => { it('null이면 null을 반환해야 한다', () => { expect(cleanNullableText(null, 100)).toBeNull(); }); it('undefined이면 null을 반환해야 한다', () => { expect(cleanNullableText(undefined, 100)).toBeNull(); }); it('빈 문자열이면 null을 반환해야 한다', () => { expect(cleanNullableText('', 100)).toBeNull(); }); + + it('공백만 있으면 null을 반환해야 한다', () => { + expect(cleanNullableText(' ', 100)).toBeNull(); + }); it('최대 길이를 초과하면 BadRequestException을 던져야 한다', () => { expect(() => cleanNullableText('abcdef', 5)).toThrow(BadRequestException); });As per coding guidelines,
src/**/*.spec.ts: 테스트는 정상 흐름뿐 아니라 주요 예외/분기 케이스가 포함되는지 확인하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/common/utils/text-cleaner.spec.ts` around lines 36 - 38, Add a test case to src/common/utils/text-cleaner.spec.ts that verifies cleanNullableText treats whitespace-only strings as null by calling cleanNullableText(' ', 100) (and consider also '\t\n' variants) and asserting the result is null; this ensures the trim-after validation branch in cleanNullableText is covered and prevents regressions.src/features/user/services/user-search.service.spec.ts (1)
66-74:clearSearchHistories의 예외 분기 테스트도 추가를 권장합니다.성공 케이스는 좋습니다. 계정 컨텍스트가 유효하지 않을 때의 실패 분기도 함께 검증하면 회귀 방지가 더 강해집니다.
As per coding guidelines "src/**/*.spec.ts: 테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지, 정상 흐름뿐 아니라 주요 예외/분기 케이스가 포함되는지 확인하세요."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/user/services/user-search.service.spec.ts` around lines 66 - 74, Add a negative test for clearSearchHistories: simulate an invalid account context by mocking repo.findAccountWithProfile to resolve to null/undefined, call service.clearSearchHistories(BigInt(1)), and assert the failure path (expect the method to return false or reject according to the implementation). Also assert repo.clearSearchHistories was not called when the account context is missing. Use the existing test helpers and the same describe block for clearSearchHistories to keep tests consistent.src/features/user/services/user-base.service.spec.ts (1)
155-158: 실시간 시계에 의존하는 테스트입니다.미래 날짜 케이스가 현재 시점을 직접 읽어서 만들어집니다. 고정된 미래 fixture나 stubbed time으로 묶어 두는 편이 구현의 날짜 절단/타임존 처리에 덜 흔들립니다.
As per coding guidelines,
src/**/*.spec.ts: 테스트는 시간/uuid/네트워크/DB 의존성을 mock 또는 stub으로 통제하는지, 정상 흐름뿐 아니라 주요 예외/분기 케이스가 포함되는지 확인하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/user/services/user-base.service.spec.ts` around lines 155 - 158, The test for testNormalizeBirthDate depends on the real clock; change it to use a deterministic time fixture or stub the system clock so the "future date" is stable: either set a fixed now via Jest fake timers (e.g., jest.useFakeTimers()/setSystemTime(fixedNow) and restore after) or construct a hardcoded future Date fixture (e.g., a specific YYYY-MM-DD that is always > fixedNow) and then call service.testNormalizeBirthDate(futureDate) to expect the BadRequestException; ensure timers are restored/reset after the spec so other tests aren’t affected.
🤖 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/features/seller/services/seller-content.service.ts`:
- Around line 262-266: buildBannerUpdateData currently only copies fields
present in input, so when linkType changes it may leave obsolete link columns
(e.g., link_product_id, link_url) in the DB; change buildBannerUpdateData to use
the normalized object returned by resolveNextBannerLinkValues(resolved) (call
resolveNextBannerLinkValues(input, current) earlier or accept resolved as arg)
and explicitly set inactive link fields to null when linkType changes (ensure
update payload includes link_product_id: null or link_url: null as appropriate),
keep validateBannerOwnership and repo.updateBanner usage the same; then add
regression tests for sellerUpdateBanner covering link-type transitions
(PRODUCT→NONE, URL→PRODUCT, PRODUCT→URL) asserting that orphaned link fields are
cleared in DB.
In `@src/features/seller/services/seller-conversation.service.spec.ts`:
- Around line 49-122: Add a "happy path" unit test for
sellerSendConversationMessage (and optionally sellerConversationMessages) that
stubs repo.findSellerAccountContext and
conversationRepo.findConversationByIdAndStore to return valid
context/conversation, stubs the message persistence/mapping methods (e.g.,
messageRepo.create or messageRepo.save and any mapper used) to return a created
message, then call service.sellerSendConversationMessage with valid input and
assert that the returned value matches the mapped message and that audit/loging
methods (e.g., auditLogService.log or whatever spy was injected) and any mapper
methods were called with expected args; ensure time/uuid are mocked if used by
the service and use existing mock objects (SELLER_CONTEXT, conversation id
BigInt(10)) to keep tests deterministic.
In `@src/features/seller/services/seller-custom-template.service.ts`:
- Around line 121-150: When a tokenId is provided you must verify the token
belongs to the same templateId as the requested template, not just the same
store: after calling findCustomTextTokenById(tokenId) check that the resolved
token's template identifier (e.g. token.template.id or token.templateId
depending on shape) strictly equals the provided templateId and if not throw
NotFoundException(CUSTOM_TEXT_TOKEN_NOT_FOUND); update the validation near
findCustomTextTokenById and before calling upsertCustomTextToken to prevent
cross-template edits and incorrect audit data.
In `@src/features/seller/services/seller-option.service.ts`:
- Around line 114-149: The current minSelect/maxSelect validation only runs when
both are provided and can allow invalid final state when only one side is
updated; before calling productRepository.updateOptionGroup, fetch the existing
option group by optionGroupId (e.g., via this.productRepository.findById /
getOptionGroup / similar) and compute the effective values: finalMin =
input.minSelect !== undefined ? input.minSelect : existing.min_select and
finalMax = input.maxSelect !== undefined ? input.maxSelect :
existing.max_select; then validate finalMax >= finalMin (and any other
invariants) and throw BadRequestException(MAX_SELECT_BELOW_MIN) if violated,
only after that proceed to call productRepository.updateOptionGroup with the
patched data.
In `@src/features/seller/services/seller-product-crud.service.ts`:
- Around line 179-208: The product creation, image addition and audit logging
are separate writes and must be executed atomically to avoid partial writes;
update the repository boundary by adding a transactional method on
productRepository (e.g., createProductWithInitialImageAndAudit or
createProductTransaction) that wraps createProduct, addProductImage and the
audit log insert in a single DB transaction and call that method from
SellerProductCrudService instead of calling createProduct and addProductImage
individually; also apply the same transactional consolidation for the flow
referenced at lines 210-220 so all related writes for a product are committed or
rolled back together.
- Around line 364-373: The count-then-write flow in
seller-product-crud.service.ts (calls to productRepository.countProductImages
and productRepository.addProductImage) is racy; wrap the check-and-insert in a
single DB transaction or use a row-level lock (e.g., SELECT ... FOR UPDATE on
the product or a dedicated images counter) so two concurrent requests cannot
both pass the count check. Update productRepository to provide an atomic method
(e.g., addProductImageIfUnderLimit or a transaction that locks the product row,
re-checks count, then inserts) and use that here; apply the same
transactional/locked pattern to the analogous delete/remove logic referenced at
lines 399-406.
- Around line 245-249: validateProductPrices is being called only with the
update payload (data) so price checks miss the current DB values; fetch the
existing product before validating and merge current prices with the update
payload so validation sees the final state. In seller-product-crud.service.ts
update the flow around buildProductUpdateData/data: load the existing product
(e.g., via this.productRepository.findById or this.getProductById using the
incoming product id), create a merged object where sale_price/regular_price
fallback to existing values when absent in data, then call validateProductPrices
with the merged regular_price and sale_price; keep using buildProductUpdateData
and validateProductPrices for locating the changes. Ensure null/undefined
handling preserves intent (explicit null should be treated appropriately per
domain rules).
In `@src/features/seller/services/seller-store.service.spec.ts`:
- Around line 37-52: Update the spec to use the current repository API by
replacing the old upsert mocks with
createStoreSpecialClosure/updateStoreSpecialClosure and
createStoreDailyCapacity/updateStoreDailyCapacity on the repo mock, and add
separate test cases asserting the create branch is called when
closureId/capacityId is missing and the update branch is called when they are
present; specifically modify tests that call the service methods exercising
special-closure and daily-capacity logic to expect
repo.createStoreSpecialClosure vs repo.updateStoreSpecialClosure and
repo.createStoreDailyCapacity vs repo.updateStoreDailyCapacity accordingly, and
add negative/exception branch tests for key error paths. Also ensure
deterministic tests by stubbing/time-freezing and UUID generation where the
service uses timestamps or UUIDs (mock the UUID/time helpers used by the
service) and mock any DB/network interactions so assertions only validate the
correct create vs update repo method calls.
In `@src/features/user/services/user-base.service.spec.ts`:
- Around line 58-101: Add a positive-path unit test for requireActiveUser: mock
repo.findAccountWithProfile to resolve an active account object (id: BigInt(1),
deleted_at: null, account_type: AccountType.USER, user_profile: { deleted_at:
null, ... }) and assert that service.testRequireActiveUser(BigInt(1))
resolves/returns the expected account or profile value (rather than throwing);
place the test alongside the existing cases and use the same service method name
(testRequireActiveUser) and repo mock to ensure the success path is covered and
controlled by test-only mocks.
In `@src/features/user/services/user-base.service.ts`:
- Around line 125-137: The pagination normalization allows non-integer numbers;
update the block that computes offset and limit (where offset, limit,
DEFAULT_PAGINATION_LIMIT and MAX_PAGINATION_LIMIT are set) to enforce integers
by using Number.isInteger on input?.offset and input?.limit before casting; if
either is not an integer, throw a BadRequestException with a clear message (e.g.
"Offset must be an integer >= 0." / "Limit must be an integer between 1 and
MAX_PAGINATION_LIMIT."). Keep existing non-negative and max-limit checks (offset
< 0, limit <= 0 || limit > MAX_PAGINATION_LIMIT) but move or combine them so
fractional inputs are rejected early; leave unreadOnly handling unchanged.
In `@src/features/user/services/user-engagement.service.spec.ts`:
- Around line 47-48: repo.findAccountWithProfile과 repo.likeReview 모킹에서 as never
캐스팅을 제거하고 실제 타입을 사용하세요: USER_CONTEXT를 UserAccountWithProfile 타입의 fixture로 정의하거나
해당 변수에 명시적 타입(UserAccountWithProfile)을 부여해
repo.findAccountWithProfile.mockResolvedValue에 전달하고,
repo.likeReview.mockResolvedValue에는 유효한 반환값('liked' 또는 'already-liked' 등)만
사용하세요(예: replace 'ok' with 'liked'). Ensure you update all occurrences
(repo.findAccountWithProfile, repo.likeReview, USER_CONTEXT) to match the
declared types so TypeScript catches mismatches.
---
Outside diff comments:
In `@src/features/seller/repositories/seller.repository.ts`:
- Around line 96-116: The upsert in createStoreSpecialClosure
(prisma.storeSpecialClosure.upsert) updates reason but does not clear the
soft-delete marker, so re-creating a soft-deleted (store_id, closure_date)
record leaves deleted_at set; modify the upsert update branch to explicitly
reset deleted_at to null (e.g., include deleted_at: null) so the record is
revived, and apply the same change to the other create*() upsert methods that
use the same (storeId, date) unique key.
---
Nitpick comments:
In `@src/common/utils/text-cleaner.spec.ts`:
- Around line 36-38: Add a test case to src/common/utils/text-cleaner.spec.ts
that verifies cleanNullableText treats whitespace-only strings as null by
calling cleanNullableText(' ', 100) (and consider also '\t\n' variants) and
asserting the result is null; this ensures the trim-after validation branch in
cleanNullableText is covered and prevents regressions.
In `@src/features/user/services/user-base.service.spec.ts`:
- Around line 155-158: The test for testNormalizeBirthDate depends on the real
clock; change it to use a deterministic time fixture or stub the system clock so
the "future date" is stable: either set a fixed now via Jest fake timers (e.g.,
jest.useFakeTimers()/setSystemTime(fixedNow) and restore after) or construct a
hardcoded future Date fixture (e.g., a specific YYYY-MM-DD that is always >
fixedNow) and then call service.testNormalizeBirthDate(futureDate) to expect the
BadRequestException; ensure timers are restored/reset after the spec so other
tests aren’t affected.
In `@src/features/user/services/user-search.service.spec.ts`:
- Around line 66-74: Add a negative test for clearSearchHistories: simulate an
invalid account context by mocking repo.findAccountWithProfile to resolve to
null/undefined, call service.clearSearchHistories(BigInt(1)), and assert the
failure path (expect the method to return false or reject according to the
implementation). Also assert repo.clearSearchHistories was not called when the
account context is missing. Use the existing test helpers and the same describe
block for clearSearchHistories to keep tests consistent.
In `@src/prisma/soft-delete.middleware.spec.ts`:
- Around line 13-56: The tests should explicitly type the params objects to
remove implicit any: import and use the SoftDeleteInput type and annotate each
params declaration (used with applySoftDeleteArgs) as SoftDeleteInput; ensure
the test file imports SoftDeleteInput from the module that exports it (where
applySoftDeleteArgs is defined) and update the three test cases (filter-added,
deleted_at bypass, findUnique no-filter) to use that type for their params.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87b219f0-5600-407a-9bec-a08524d9851f
📒 Files selected for processing (106)
.vscode/settings.jsonpackage.jsonsrc/app.module.tssrc/common/utils/date-parser.spec.tssrc/common/utils/date-parser.tssrc/common/utils/request-context.tssrc/common/utils/text-cleaner.spec.tssrc/common/utils/text-cleaner.tssrc/common/utils/validation.tssrc/features/auth/auth.module.tssrc/features/auth/auth.seller.service.spec.tssrc/features/auth/auth.service.spec.tssrc/features/auth/auth.service.tssrc/features/auth/controllers/auth.controller.spec.tssrc/features/auth/controllers/auth.controller.tssrc/features/auth/helpers/auth-cookie.helper.tssrc/features/auth/repositories/auth.repository.spec.tssrc/features/auth/repositories/auth.repository.tssrc/features/auth/services/oidc-client.service.spec.tssrc/features/auth/services/oidc-client.service.tssrc/features/auth/strategies/jwt-bearer.strategy.tssrc/features/conversation/conversation.module.tssrc/features/conversation/index.tssrc/features/conversation/repositories/conversation.repository.tssrc/features/order/index.tssrc/features/order/order.module.tssrc/features/order/repositories/order.repository.tssrc/features/order/services/order-domain.service.tssrc/features/product/index.tssrc/features/product/product.module.tssrc/features/product/repositories/product.repository.tssrc/features/seller/constants/seller-error-messages.tssrc/features/seller/constants/seller.constants.tssrc/features/seller/index.tssrc/features/seller/repositories/seller.repository.tssrc/features/seller/resolvers/seller-content-mutation.resolver.tssrc/features/seller/resolvers/seller-content-query.resolver.tssrc/features/seller/resolvers/seller-conversation-mutation.resolver.tssrc/features/seller/resolvers/seller-conversation-query.resolver.tssrc/features/seller/resolvers/seller-order-mutation.resolver.tssrc/features/seller/resolvers/seller-order-query.resolver.tssrc/features/seller/resolvers/seller-product-mutation.resolver.tssrc/features/seller/resolvers/seller-product-query.resolver.tssrc/features/seller/resolvers/seller-store-mutation.resolver.tssrc/features/seller/resolvers/seller-store-query.resolver.tssrc/features/seller/resolvers/seller.resolver.spec.tssrc/features/seller/seller.module.tssrc/features/seller/services/seller-base.service.spec.tssrc/features/seller/services/seller-base.service.tssrc/features/seller/services/seller-content.service.spec.tssrc/features/seller/services/seller-content.service.tssrc/features/seller/services/seller-conversation.service.spec.tssrc/features/seller/services/seller-conversation.service.tssrc/features/seller/services/seller-custom-template.service.spec.tssrc/features/seller/services/seller-custom-template.service.tssrc/features/seller/services/seller-option.service.spec.tssrc/features/seller/services/seller-option.service.tssrc/features/seller/services/seller-order.service.spec.tssrc/features/seller/services/seller-order.service.tssrc/features/seller/services/seller-product-crud.service.spec.tssrc/features/seller/services/seller-product-crud.service.tssrc/features/seller/services/seller-product.service.tssrc/features/seller/services/seller-store.service.spec.tssrc/features/seller/services/seller-store.service.tssrc/features/system/system.module.tssrc/features/user/constants/user.constants.tssrc/features/user/index.tssrc/features/user/repositories/user.repository.tssrc/features/user/resolvers/user-engagement-mutation.resolver.tssrc/features/user/resolvers/user-notification-mutation.resolver.tssrc/features/user/resolvers/user-notification-query.resolver.tssrc/features/user/resolvers/user-profile-mutation.resolver.tssrc/features/user/resolvers/user-profile-query.resolver.tssrc/features/user/resolvers/user-search-mutation.resolver.tssrc/features/user/resolvers/user-search-query.resolver.tssrc/features/user/resolvers/user.resolver.spec.tssrc/features/user/services/user-base.service.spec.tssrc/features/user/services/user-base.service.tssrc/features/user/services/user-engagement.service.spec.tssrc/features/user/services/user-engagement.service.tssrc/features/user/services/user-notification.service.spec.tssrc/features/user/services/user-notification.service.tssrc/features/user/services/user-profile.service.spec.tssrc/features/user/services/user-profile.service.tssrc/features/user/services/user-search.service.spec.tssrc/features/user/services/user-search.service.tssrc/features/user/user.module.tssrc/global/auth/auth-global.module.tssrc/global/auth/decorators/current-user.decorator.tssrc/global/auth/index.tssrc/global/auth/parse-account-id.tssrc/global/filters/global-exception.filter.tssrc/global/graphql/graphql.module.tssrc/global/interceptors/api-response.interceptor.tssrc/global/interceptors/gql-logging.interceptor.tssrc/global/interceptors/http-logging.interceptor.tssrc/global/logger/custom-logger.service.tssrc/global/logger/logger.module.tssrc/global/middlewares/docs-access.middleware.spec.tssrc/global/types/express.d.tssrc/main.tssrc/prisma/index.tssrc/prisma/prisma.module.tssrc/prisma/prisma.service.tssrc/prisma/soft-delete.middleware.spec.tstsconfig.json
💤 Files with no reviewable changes (1)
- src/features/seller/services/seller-product.service.ts
Comment on lines
+179
to
+208
| const created = await this.productRepository.createProduct({ | ||
| storeId: ctx.storeId, | ||
| data: { | ||
| name: cleanRequiredText(input.name, MAX_PRODUCT_NAME_LENGTH), | ||
| description: cleanNullableText( | ||
| input.description, | ||
| MAX_PRODUCT_DESCRIPTION_LENGTH, | ||
| ), | ||
| purchase_notice: cleanNullableText( | ||
| input.purchaseNotice, | ||
| MAX_PRODUCT_PURCHASE_NOTICE_LENGTH, | ||
| ), | ||
| regular_price: input.regularPrice, | ||
| sale_price: input.salePrice ?? null, | ||
| currency: this.cleanCurrency(input.currency), | ||
| base_design_image_url: cleanNullableText( | ||
| input.baseDesignImageUrl, | ||
| MAX_URL_LENGTH, | ||
| ), | ||
| preparation_time_minutes: | ||
| input.preparationTimeMinutes ?? DEFAULT_PREPARATION_TIME_MINUTES, | ||
| is_active: input.isActive ?? true, | ||
| }, | ||
| }); | ||
|
|
||
| await this.productRepository.addProductImage({ | ||
| productId: created.id, | ||
| imageUrl: cleanRequiredText(input.initialImageUrl, MAX_URL_LENGTH), | ||
| sortOrder: 0, | ||
| }); |
There was a problem hiding this comment.
상품 생성 플로우가 원자적이지 않습니다.
상품 row 생성, 초기 이미지 추가, 감사 로그 기록이 분리된 write라서 중간 단계 하나만 실패해도 이미지 없는 상품 또는 로그 없는 상품이 남을 수 있습니다. 최소 1장 이미지 규칙이 있는 도메인이라 이건 바로 partial write 문제로 이어집니다. 이 흐름은 Service에서 Prisma에 직접 접근하지 말라는 현재 구조를 유지하면서, Repository 경계의 트랜잭션으로 묶는 편이 안전합니다.
Also applies to: 210-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/seller/services/seller-product-crud.service.ts` around lines 179
- 208, The product creation, image addition and audit logging are separate
writes and must be executed atomically to avoid partial writes; update the
repository boundary by adding a transactional method on productRepository (e.g.,
createProductWithInitialImageAndAudit or createProductTransaction) that wraps
createProduct, addProductImage and the audit log insert in a single DB
transaction and call that method from SellerProductCrudService instead of
calling createProduct and addProductImage individually; also apply the same
transactional consolidation for the flow referenced at lines 210-220 so all
related writes for a product are committed or rolled back together.
Comment on lines
+364
to
+373
| const count = await this.productRepository.countProductImages(productId); | ||
| if (count >= MAX_PRODUCT_IMAGES) { | ||
| throw new BadRequestException(IMAGE_LIMIT_EXCEEDED); | ||
| } | ||
|
|
||
| const row = await this.productRepository.addProductImage({ | ||
| productId, | ||
| imageUrl: cleanRequiredText(input.imageUrl, MAX_URL_LENGTH), | ||
| sortOrder: input.sortOrder ?? count, | ||
| }); |
There was a problem hiding this comment.
이미지 개수 제한 검사가 경쟁 상태에 취약합니다.
추가/삭제 모두 count -> write 순서라 동시 요청 두 개가 같은 개수를 보고 함께 통과할 수 있습니다. 그 결과 최대 이미지 수를 초과하거나 최소 이미지 수 아래로 내려갈 수 있습니다. 개수 확인과 write를 같은 트랜잭션/락 경계로 묶어야 합니다.
Also applies to: 399-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/seller/services/seller-product-crud.service.ts` around lines 364
- 373, The count-then-write flow in seller-product-crud.service.ts (calls to
productRepository.countProductImages and productRepository.addProductImage) is
racy; wrap the check-and-insert in a single DB transaction or use a row-level
lock (e.g., SELECT ... FOR UPDATE on the product or a dedicated images counter)
so two concurrent requests cannot both pass the count check. Update
productRepository to provide an atomic method (e.g., addProductImageIfUnderLimit
or a transaction that locks the product row, re-checks count, then inserts) and
use that here; apply the same transactional/locked pattern to the analogous
delete/remove logic referenced at lines 399-406.
Comment on lines
125
to
+137
| const offset = Number.isFinite(input?.offset) ? Number(input?.offset) : 0; | ||
| const limit = Number.isFinite(input?.limit) ? Number(input?.limit) : 20; | ||
| const limit = Number.isFinite(input?.limit) | ||
| ? Number(input?.limit) | ||
| : DEFAULT_PAGINATION_LIMIT; | ||
| const unreadOnly = Boolean(input?.unreadOnly); | ||
|
|
||
| if (offset < 0) { | ||
| throw new BadRequestException('Offset must be >= 0.'); | ||
| } | ||
| if (limit <= 0 || limit > 50) { | ||
| throw new BadRequestException('Limit must be between 1 and 50.'); | ||
| if (limit <= 0 || limit > MAX_PAGINATION_LIMIT) { | ||
| throw new BadRequestException( | ||
| `Limit must be between 1 and ${MAX_PAGINATION_LIMIT}.`, | ||
| ); |
There was a problem hiding this comment.
페이지네이션 정규화에서 정수 검증이 빠져 있습니다.
현재 offset/limit가 유한 수이기만 하면 통과되어 소수 입력이 내려갈 수 있습니다. 저장소 쿼리 계층에서 런타임 오류/예상치 못한 동작을 유발할 수 있어 서비스 레벨에서 정수 검증을 추가하는 편이 안전합니다.
패치 제안
const offset = Number.isFinite(input?.offset) ? Number(input?.offset) : 0;
const limit = Number.isFinite(input?.limit)
? Number(input?.limit)
: DEFAULT_PAGINATION_LIMIT;
@@
- if (offset < 0) {
+ if (!Number.isInteger(offset) || offset < 0) {
throw new BadRequestException('Offset must be >= 0.');
}
- if (limit <= 0 || limit > MAX_PAGINATION_LIMIT) {
+ if (!Number.isInteger(limit) || limit <= 0 || limit > MAX_PAGINATION_LIMIT) {
throw new BadRequestException(
`Limit must be between 1 and ${MAX_PAGINATION_LIMIT}.`,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/user/services/user-base.service.ts` around lines 125 - 137, The
pagination normalization allows non-integer numbers; update the block that
computes offset and limit (where offset, limit, DEFAULT_PAGINATION_LIMIT and
MAX_PAGINATION_LIMIT are set) to enforce integers by using Number.isInteger on
input?.offset and input?.limit before casting; if either is not an integer,
throw a BadRequestException with a clear message (e.g. "Offset must be an
integer >= 0." / "Limit must be an integer between 1 and
MAX_PAGINATION_LIMIT."). Keep existing non-negative and max-limit checks (offset
< 0, limit <= 0 || limit > MAX_PAGINATION_LIMIT) but move or combine them so
fractional inputs are rejected early; leave unreadOnly handling unchanged.
Comment on lines
+47
to
+48
| repo.findAccountWithProfile.mockResolvedValue(USER_CONTEXT as never); | ||
| repo.likeReview.mockResolvedValue('not-found'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: `as never` 우회 사용 위치와 실제 리포지토리 메서드 시그니처를 대조 확인합니다.
set -euo pipefail
echo "== 1) as never 사용 위치 확인 =="
rg -n --type ts '\bas\s+never\b' src/features/user/services/user-engagement.service.spec.ts
echo
echo "== 2) UserRepository 메서드 시그니처 확인 =="
fd user.repository.ts src --exec sed -n '1,260p' {}
echo
echo "== 3) 대상 서비스 likeReview 시그니처/분기 확인 =="
fd user-engagement.service.ts src --exec sed -n '1,260p' {}Repository: CaQuick/caquick-be
Length of output: 7691
🏁 Script executed:
# UserRepository에서 likeReview 메서드 전체 코드 조회
rg -A 20 'async likeReview' src/features/user/repositories/user.repository.tsRepository: CaQuick/caquick-be
Length of output: 667
🏁 Script executed:
# 테스트 파일의 line 45-71 전체 보기
sed -n '45,71p' src/features/user/services/user-engagement.service.spec.tsRepository: CaQuick/caquick-be
Length of output: 1035
as never 캐스팅을 제거하고 실제 타입과 일치하는 mock 값을 사용하세요.
likeReview의 반환 타입은 Promise<'liked' | 'already-liked' | 'not-found' | 'self-like'>입니다.
- 66번 라인:
'ok'는 유효하지 않은 값입니다. 성공 경로를 테스트하려면'liked'또는'already-liked'를 사용하세요. - 47, 56, 65번 라인:
USER_CONTEXT as never는 타입 검증을 우회합니다.USER_CONTEXT를 실제UserAccountWithProfile타입으로 fixture를 정의하거나 명시적 타입 선언을 추가하세요.
타입 캐스팅으로 오류를 가리면, 리포지토리나 서비스 인터페이스 변경 시 테스트가 조용히 실패할 수 있습니다.
현재 코드 (66번 라인)
repo.likeReview.mockResolvedValue('ok' as never);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/user/services/user-engagement.service.spec.ts` around lines 47 -
48, repo.findAccountWithProfile과 repo.likeReview 모킹에서 as never 캐스팅을 제거하고 실제 타입을
사용하세요: USER_CONTEXT를 UserAccountWithProfile 타입의 fixture로 정의하거나 해당 변수에 명시적
타입(UserAccountWithProfile)을 부여해 repo.findAccountWithProfile.mockResolvedValue에
전달하고, repo.likeReview.mockResolvedValue에는 유효한 반환값('liked' 또는 'already-liked' 등)만
사용하세요(예: replace 'ok' with 'liked'). Ensure you update all occurrences
(repo.findAccountWithProfile, repo.likeReview, USER_CONTEXT) to match the
declared types so TypeScript catches mismatches.
- 배너 linkType 변경 시 비활성 링크 필드를 명시적으로 null 처리 - optionGroup minSelect/maxSelect 부분 업데이트 시 기존 값과 merge하여 불변식 검증 - 상품 가격 업데이트 시 현재 DB 값과 merge하여 salePrice > regularPrice 방지 - 특별휴무/일별용량 upsert 시 soft-delete된 레코드의 deleted_at 복구 - customTextToken 수정 시 요청 templateId 소속 여부 검증 추가
- sellerConversationMessages/sellerSendConversationMessage 정상 흐름 테스트 추가 - seller-store spec의 upsert mock을 create/update 분기 mock으로 교체 - requireActiveUser 활성 계정 happy path 테스트 추가 - likeReview mock 반환값을 유효한 'liked'로 수정
- cleanNullableText 공백-only 입력 시 null 반환 테스트 추가 - clearSearchHistories 계정 무효 시 예외 전파 테스트 추가 - normalizeBirthDate 미래 날짜 테스트를 고정 fixture(2099-01-01)로 변경
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/features/seller/services/seller-product-crud.service.ts (2)
179-220:⚠️ Potential issue | 🟠 Major상품 생성 플로우의 원자성 미보장 문제가 남아 있습니다.
createProduct,addProductImage,createAuditLog가 별도의 write로 실행되어 중간 단계 실패 시 부분 데이터가 남을 수 있습니다. 최소 1장 이미지 규칙이 있는 도메인에서 이미지 없는 상품이 생성될 수 있습니다.Repository 경계에서 트랜잭션으로 묶는 방안을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/seller/services/seller-product-crud.service.ts` around lines 179 - 220, The code calls productRepository.createProduct, productRepository.addProductImage and repo.createAuditLog as separate writes which can leave partial state on failure; wrap these operations in a single database transaction so they commit or roll back together. Locate the sequence using createProduct, addProductImage and createAuditLog and move them inside a transactional block (use your repository/ORM transaction API or a unit-of-work method on productRepository), perform validations (e.g., ensure initialImageUrl present) before starting the transaction, execute createProduct, addProductImage and createAuditLog inside the same transaction context, and ensure errors cause the transaction to rollback and rethrow.
364-406:⚠️ Potential issue | 🟠 Major이미지 개수 제한 검사가 경쟁 상태에 취약합니다.
countProductImages -> addProductImage/softDeleteProductImage순서로 인해 동시 요청이 같은 개수를 보고 함께 통과할 수 있습니다. 결과적으로 최대/최소 이미지 수 제약을 위반할 수 있습니다.개수 확인과 write를 같은 트랜잭션/락 경계로 묶거나, Repository에서 원자적 메서드(예:
addProductImageIfUnderLimit)를 제공하는 방안을 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/seller/services/seller-product-crud.service.ts` around lines 364 - 406, The image-count check is racy because sellerAdd/sellerDeleteProductImage call countProductImages then perform writes (addProductImage/softDeleteProductImage) separately; wrap the check+write in a single transaction or provide and call repository-level atomic methods (e.g. addProductImageIfUnderLimit, deleteProductImageIfAboveMin) that perform the count and insert/delete under the same DB lock/transaction (or use SELECT ... FOR UPDATE on the product row) so concurrent requests cannot both pass the limit check; keep audit log creation (repo.createAuditLog) inside the same transaction after the write succeeds or ensure it runs only when the atomic repo method returns success.
🤖 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/features/seller/services/seller-content.service.ts`:
- Around line 321-345: buildBannerUpdateData currently spreads
buildBannerLinkFields(resolved) when input.linkType is provided, which lets
resolved.linkUrl bypass sanitization (resolved.linkUrl is raw). Update the code
so link URL fields are sanitized before being merged: either make
buildBannerLinkFields accept a sanitized version of resolved (run
resolved.linkUrl through cleanNullableText(..., MAX_URL_LENGTH) and
resolved.linkUrl -> cleaned value) or sanitize inside buildBannerLinkFields
itself; ensure the cleaned value (using cleanNullableText and MAX_URL_LENGTH) is
what gets returned/spread when link_type is changed so trimming and max-length
validation always run for linkUrl regardless of using
resolveNextBannerLinkValues.
- Around line 218-229: The banner creation and partial-update paths save all
link columns regardless of linkType, breaking the link invariants; update
createBanner and the partial-update handler in SellerContentService to normalize
link fields based on the resolved linkType (use toBannerLinkType result) before
calling this.repo.createBanner or this.repo.updateBanner: keep only the fields
relevant to the resolved linkType (e.g., for NONE clear
linkUrl/linkProductId/linkStoreId/linkCategoryId), reuse a shared normalization
helper (e.g., normalizeBannerLinks) that applies
cleanNullableText/cleanRequiredText/parseId and nulls out inactive link fields,
and ensure the service response only exposes the resolved/normalized link fields
so inactive values are not persisted or returned.
---
Duplicate comments:
In `@src/features/seller/services/seller-product-crud.service.ts`:
- Around line 179-220: The code calls productRepository.createProduct,
productRepository.addProductImage and repo.createAuditLog as separate writes
which can leave partial state on failure; wrap these operations in a single
database transaction so they commit or roll back together. Locate the sequence
using createProduct, addProductImage and createAuditLog and move them inside a
transactional block (use your repository/ORM transaction API or a unit-of-work
method on productRepository), perform validations (e.g., ensure initialImageUrl
present) before starting the transaction, execute createProduct, addProductImage
and createAuditLog inside the same transaction context, and ensure errors cause
the transaction to rollback and rethrow.
- Around line 364-406: The image-count check is racy because
sellerAdd/sellerDeleteProductImage call countProductImages then perform writes
(addProductImage/softDeleteProductImage) separately; wrap the check+write in a
single transaction or provide and call repository-level atomic methods (e.g.
addProductImageIfUnderLimit, deleteProductImageIfAboveMin) that perform the
count and insert/delete under the same DB lock/transaction (or use SELECT ...
FOR UPDATE on the product row) so concurrent requests cannot both pass the limit
check; keep audit log creation (repo.createAuditLog) inside the same transaction
after the write succeeds or ensure it runs only when the atomic repo method
returns success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98adcb6b-a1af-4b18-83f2-cc99cc35a863
📒 Files selected for processing (11)
src/common/utils/text-cleaner.spec.tssrc/features/seller/repositories/seller.repository.tssrc/features/seller/services/seller-content.service.tssrc/features/seller/services/seller-conversation.service.spec.tssrc/features/seller/services/seller-custom-template.service.tssrc/features/seller/services/seller-option.service.tssrc/features/seller/services/seller-product-crud.service.tssrc/features/seller/services/seller-store.service.spec.tssrc/features/user/services/user-base.service.spec.tssrc/features/user/services/user-engagement.service.spec.tssrc/features/user/services/user-search.service.spec.ts
✅ Files skipped from review due to trivial changes (1)
- src/features/user/services/user-engagement.service.spec.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/common/utils/text-cleaner.spec.ts
- src/features/user/services/user-search.service.spec.ts
- src/features/user/services/user-base.service.spec.ts
- src/features/seller/services/seller-option.service.ts
- src/features/seller/services/seller-custom-template.service.ts
Comment on lines
218
to
+229
| const row = await this.repo.createBanner({ | ||
| placement: this.toBannerPlacement(input.placement), | ||
| title: this.cleanNullableText(input.title, 200), | ||
| imageUrl: this.cleanRequiredText(input.imageUrl, 2048), | ||
| title: cleanNullableText(input.title, MAX_BANNER_TITLE_LENGTH), | ||
| imageUrl: cleanRequiredText(input.imageUrl, MAX_URL_LENGTH), | ||
| linkType: this.toBannerLinkType(input.linkType ?? 'NONE'), | ||
| linkUrl: this.cleanNullableText(input.linkUrl, 2048), | ||
| linkUrl: cleanNullableText(input.linkUrl, MAX_URL_LENGTH), | ||
| linkProductId: input.linkProductId ? parseId(input.linkProductId) : null, | ||
| linkStoreId: input.linkStoreId ? parseId(input.linkStoreId) : null, | ||
| linkCategoryId: input.linkCategoryId | ||
| ? parseId(input.linkCategoryId) | ||
| : null, | ||
| startsAt: this.toDate(input.startsAt) ?? null, | ||
| endsAt: this.toDate(input.endsAt) ?? null, | ||
| startsAt: toDate(input.startsAt) ?? null, |
There was a problem hiding this comment.
링크 불변식이 생성/partial update 경로에서는 아직 깨집니다.
Line 223-228은 linkType와 무관하게 모든 링크 컬럼을 저장하고, Line 349-374도 linkType 없이 들어온 링크 필드를 그대로 반영합니다. 그래서 NONE/PRODUCT 배너에 비활성 링크 값이 다시 남을 수 있고, 현재 output도 모든 링크 필드를 그대로 노출해서 모순 상태가 API 응답으로 보이게 됩니다. 생성과 링크 관련 partial update도 resolved 기반 정규화를 공통 적용하는 편이 안전합니다.
🛠️ 제안 diff
async sellerCreateBanner(
accountId: bigint,
input: SellerCreateBannerInput,
): Promise<SellerBannerOutput> {
const ctx = await this.requireSellerContext(accountId);
+ const normalizedLinkFields = this.buildBannerLinkFields({
+ linkType: input.linkType ?? 'NONE',
+ linkUrl: cleanNullableText(input.linkUrl, MAX_URL_LENGTH),
+ linkProductId: input.linkProductId ? parseId(input.linkProductId) : null,
+ linkStoreId: input.linkStoreId ? parseId(input.linkStoreId) : null,
+ linkCategoryId: input.linkCategoryId
+ ? parseId(input.linkCategoryId)
+ : null,
+ });
+
await this.validateBannerOwnership(ctx, {
linkType: input.linkType ?? 'NONE',
linkProductId: input.linkProductId ? parseId(input.linkProductId) : null,
linkStoreId: input.linkStoreId ? parseId(input.linkStoreId) : null,
linkCategoryId: input.linkCategoryId
@@
const row = await this.repo.createBanner({
placement: this.toBannerPlacement(input.placement),
title: cleanNullableText(input.title, MAX_BANNER_TITLE_LENGTH),
imageUrl: cleanRequiredText(input.imageUrl, MAX_URL_LENGTH),
linkType: this.toBannerLinkType(input.linkType ?? 'NONE'),
- linkUrl: cleanNullableText(input.linkUrl, MAX_URL_LENGTH),
- linkProductId: input.linkProductId ? parseId(input.linkProductId) : null,
- linkStoreId: input.linkStoreId ? parseId(input.linkStoreId) : null,
- linkCategoryId: input.linkCategoryId
- ? parseId(input.linkCategoryId)
- : null,
+ linkUrl: normalizedLinkFields.link_url,
+ linkProductId: normalizedLinkFields.link_product_id,
+ linkStoreId: normalizedLinkFields.link_store_id,
+ linkCategoryId: normalizedLinkFields.link_category_id,
@@
- ...(input.linkType !== undefined
+ ...(input.linkType !== undefined ||
+ input.linkUrl !== undefined ||
+ input.linkProductId !== undefined ||
+ input.linkStoreId !== undefined ||
+ input.linkCategoryId !== undefined
? {
- link_type: this.toBannerLinkType(input.linkType),
+ ...(input.linkType !== undefined
+ ? { link_type: this.toBannerLinkType(resolved.linkType) }
+ : {}),
...linkFieldsForType,
}Also applies to: 347-375
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/seller/services/seller-content.service.ts` around lines 218 -
229, The banner creation and partial-update paths save all link columns
regardless of linkType, breaking the link invariants; update createBanner and
the partial-update handler in SellerContentService to normalize link fields
based on the resolved linkType (use toBannerLinkType result) before calling
this.repo.createBanner or this.repo.updateBanner: keep only the fields relevant
to the resolved linkType (e.g., for NONE clear
linkUrl/linkProductId/linkStoreId/linkCategoryId), reuse a shared normalization
helper (e.g., normalizeBannerLinks) that applies
cleanNullableText/cleanRequiredText/parseId and nulls out inactive link fields,
and ensure the service response only exposes the resolved/normalized link fields
so inactive values are not persisted or returned.
- buildBannerLinkFields()의 URL case에서 resolved.linkUrl을 cleanNullableText()로 sanitize하여 trim/최대길이 검증 적용
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
이번 PR에서는 클린 코드 원칙에 따라 테스트, 서비스 분리, 상수화 등을 적용하여 코드 품질을 향상시켰습니다.
판매자 기능 관련 서비스, 리졸버, 레포지토리 등 다수의 파일에서 리팩토링을 진행하였고, Path Alias(@/) 적용 및 Barrel 파일 추가로 import 경로를 정리하였습니다.
또한, 에러 메시지를 상수화하여 일관성을 높였으며, 판매자 도메인 서비스에 대한 단위 테스트를 보강하였습니다.
Impact
Checklist
Summary by CodeRabbit
New Features
Improvements
Tests