[FEAT] 파트너 저장/삭제 및 검색 제외 처리#23
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 Walkthrough개요사용자가 선호하는 파트너를 저장하고 제거할 수 있는 기능을 구현합니다. 저장된 파트너는 향후 검색 결과에서 자동으로 제외되며, 선택적 사용자 인증을 위한 새로운 decorator를 추가합니다. 주요 변경 사항파트너 저장/제거 기능
검토 포인트🔍 주요 로직 검토 사항
🔐 보안 체크
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly describes the main features: partner save/delete and search exclusion handling for authenticated users. |
| Linked Issues check | ✅ Passed | All coding requirements from issue #18 are met: partner save API, persistence to Users.savedPartners, and exclusion from search results for authenticated users. |
| Out of Scope Changes check | ✅ Passed | All changes are within scope of the linked issue #18, including supporting infrastructure like OptionalCurrentUser decorator and custom exceptions for the my-business domain. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ 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
feat/save-partner
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/modules/partners/partners.service.spec.ts (1)
259-272: ⚡ Quick win$nin 배열의 첫 번째 요소만 검증하고 있습니다.
Line 271에서
filterArg._id.$nin[0]만 확인하는데, 현재 테스트는 savedPartners가 1개일 때만 동작합니다. 향후 여러 파트너가 저장된 경우를 대비해 배열 전체 또는 길이도 검증하는 것이 좋습니다.♻️ 제안: 배열 전체 검증 추가
const [filterArg] = sellerModel.find.mock.calls[0]; - expect(filterArg._id.$nin[0].toString()).toBe(savedId.toString()); + expect(filterArg._id.$nin).toHaveLength(1); + expect(filterArg._id.$nin[0].toString()).toBe(savedId.toString());🤖 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/modules/partners/partners.service.spec.ts` around lines 259 - 272, The test currently only asserts the first element of the $nin array (filterArg._id.$nin[0]) against savedId; update the assertion to validate the whole array returned to handle multiple saved partners: when mocking userModel.findById to return savedPartners (e.g., savedPartners array of partnerId values), assert filterArg._id.$nin has the expected length and that every element (or the set) matches the saved partnerIds (use the same savedPartners data you mock), referencing the variables savedId/savedPartners and the sellerModel.find call to locate the assertion.src/modules/partners/partners.service.ts (2)
345-346: 벡터 검색에서 $nin 필터 위치로 인한 결과 수 감소 가능성을 고려해야 합니다.MongoDB Atlas의
$vectorSearch는 먼저 100개의 후보를 찾은 후,$match단계에서$nin필터가 적용됩니다. 사용자의 savedPartners에 많은 ID가 포함된 경우, 최종 결과가 요청한 limit보다 훨씬 적을 수 있습니다.현재 구조는 Atlas vector search의 표준 패턴이지만, 사용자가 많은 파트너를 저장했을 때 결과 부족 현상이 발생할 수 있음을 인지하고 모니터링하는 것이 좋습니다.
🤖 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/modules/partners/partners.service.ts` around lines 345 - 346, The current use of matchStage._id = { $nin: excludeObjectIds } after $vectorSearch can shrink results because Atlas finds ~100 candidates before $match; to fix, detect when excludeObjectIds is large and either (A) increase the vector search candidate pool/limit parameter in the $vectorSearch stage so more candidates are returned before $match, or (B) remove the $nin from matchStage and perform exclusion client-side by running $vectorSearch (e.g., using the existing vector search stage) then filtering results by excludeObjectIds in the service code (in the function surrounding matchStage and the $vectorSearch invocation) and optionally repeat with a larger candidate limit until the requested limit is fulfilled; reference symbols: matchStage, excludeObjectIds, $vectorSearch.
196-205: 대량의 savedPartners가 있을 때 쿼리 성능 저하 가능성을 모니터링해야 합니다.MongoDB의
$nin연산자는 배열 크기가 커질수록 성능이 저하될 수 있습니다. 사용자가 수백 개 이상의 파트너를 저장한 경우, 인덱스 활용이 제한되고 전체 컬렉션 스캔이 발생할 수 있습니다.현재 구현은 초기 버전으로 적절하지만, 향후 다음을 고려할 수 있습니다:
- savedPartners 개수에 실용적인 제한 설정 (예: 최대 1000개)
- 대량 저장 시 다른 필터링 전략 사용 (예: 별도 쿼리로 결과 후처리)
- 성능 메트릭 수집 및 모니터링
🤖 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/modules/partners/partners.service.ts` around lines 196 - 205, The current exclusion logic builds excludeObjectIds from user.savedPartners which can lead to a large $nin array and degrade query performance; modify the block in partners.service.ts (referencing opts.userId, this.userModel, savedPartners and partnerId used to populate excludeObjectIds) to impose a practical cap (e.g. only take first N savedPartners like 1000) or switch to a safer strategy (e.g. fetch results without $nin and filter out saved partnerIds in-memory when savedPartners is huge), and add a metric/log when the savedPartners length exceeds the cap so we can monitor occurrences and adjust later.
🤖 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 `@src/modules/my-business/my-business.service.ts`:
- Around line 80-94: Replace the separate exists-check + push with an atomic
conditional update: first verify the partner exists in the chosen model (use
sellerModel or buyerModel as in the diff and throw PartnerNotFoundException if
not), then call this.userModel.updateOne({ _id: userId,
"savedPartners.partnerId": { $ne: partnerObjectId } }, { $push: { savedPartners:
{ partnerId: partnerObjectId, partnerType } } }) (or use $addToSet) so the
check-and-insert is atomic, and inspect the returned result
(matchedCount/modifiedCount) to decide outcome—if matchedCount is 0 throw
PartnerNotFoundException or user-missing, if modifiedCount is 0 throw
PartnerAlreadySavedException, otherwise return the success message; reference
userModel, savedPartners, partnerObjectId, partnerType, sellerModel, buyerModel,
PartnerNotFoundException, PartnerAlreadySavedException.
- Around line 73-79: Validate partnerId before constructing a new Types.ObjectId
in savePartner: perform the same pre-check used in removePartner to ensure
partnerId is a valid ObjectId string and return/throw a controlled error if
invalid instead of calling new Types.ObjectId(partnerId) blindly; after
performing the updateOne call in savePartner (the block that currently writes
partner info, lines ~90-94), inspect the result.modifiedCount (or matchedCount
as appropriate) and return/fail when no document was modified so the caller does
not receive a false success—mirror the error/response behavior used in
removePartner for both validation and update result handling.
In `@src/modules/partners/partners.controller.ts`:
- Around line 10-13: The absolute import of OptionalCurrentUser and SessionUser
("src/common/decorators/current-user.decorator") in partners.controller.ts
breaks Jest resolution; replace that absolute import with a correct relative
import that points to the same current-user.decorator module so the imports for
OptionalCurrentUser and SessionUser resolve during tests and CI.
---
Nitpick comments:
In `@src/modules/partners/partners.service.spec.ts`:
- Around line 259-272: The test currently only asserts the first element of the
$nin array (filterArg._id.$nin[0]) against savedId; update the assertion to
validate the whole array returned to handle multiple saved partners: when
mocking userModel.findById to return savedPartners (e.g., savedPartners array of
partnerId values), assert filterArg._id.$nin has the expected length and that
every element (or the set) matches the saved partnerIds (use the same
savedPartners data you mock), referencing the variables savedId/savedPartners
and the sellerModel.find call to locate the assertion.
In `@src/modules/partners/partners.service.ts`:
- Around line 345-346: The current use of matchStage._id = { $nin:
excludeObjectIds } after $vectorSearch can shrink results because Atlas finds
~100 candidates before $match; to fix, detect when excludeObjectIds is large and
either (A) increase the vector search candidate pool/limit parameter in the
$vectorSearch stage so more candidates are returned before $match, or (B) remove
the $nin from matchStage and perform exclusion client-side by running
$vectorSearch (e.g., using the existing vector search stage) then filtering
results by excludeObjectIds in the service code (in the function surrounding
matchStage and the $vectorSearch invocation) and optionally repeat with a larger
candidate limit until the requested limit is fulfilled; reference symbols:
matchStage, excludeObjectIds, $vectorSearch.
- Around line 196-205: The current exclusion logic builds excludeObjectIds from
user.savedPartners which can lead to a large $nin array and degrade query
performance; modify the block in partners.service.ts (referencing opts.userId,
this.userModel, savedPartners and partnerId used to populate excludeObjectIds)
to impose a practical cap (e.g. only take first N savedPartners like 1000) or
switch to a safer strategy (e.g. fetch results without $nin and filter out saved
partnerIds in-memory when savedPartners is huge), and add a metric/log when the
savedPartners length exceeds the cap so we can monitor occurrences and adjust
later.
🪄 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: a0da567f-7c31-46a1-bf9c-0361a457373c
📒 Files selected for processing (10)
src/common/decorators/current-user.decorator.tssrc/common/exceptions/index.tssrc/common/exceptions/my-business.exceptions.tssrc/modules/my-business/dto/save-partner.dto.tssrc/modules/my-business/my-business.controller.tssrc/modules/my-business/my-business.service.spec.tssrc/modules/my-business/my-business.service.tssrc/modules/partners/partners.controller.tssrc/modules/partners/partners.service.spec.tssrc/modules/partners/partners.service.ts
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.
Resolves #18
변경 사항
My Business
POST /my-business/partners— 파트너 저장 (seller/buyer 구분)DELETE /my-business/partners/:partnerId— 저장된 파트너 삭제Partners 검색
기타
OptionalCurrentUser데코레이터 추가 (인증 선택적 엔드포인트용)PartnerAlreadySavedException,PartnerNotFoundException,SavedPartnerNotFoundException)테스트
my-business.service.spec.ts— savePartner / removePartner 케이스 추가partners.service.spec.ts— savedPartners $nin 필터 적용 케이스 추가 (로그인/비로그인/빈 목록)Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트