Skip to content

Conversation

@twlite
Copy link
Contributor

@twlite twlite commented Dec 20, 2025

Description

This PR updates promotion-service to exclude seller orders when counting promotion usage. Fixes #3042

Breaking changes

N/A

Screenshots

N/A

Checklist

📌 Always:

  • I have set a clear title
  • My PR is small and contains a single feature
  • I have checked my own PR

👍 Most of the time:

  • I have added or updated test cases
  • I have updated the README if needed

@vercel
Copy link

vercel bot commented Dec 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
docs Ready Ready Preview Dec 29, 2025 11:53am
vendure-storybook Ready Ready Preview, Comment Dec 29, 2025 11:53am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 20, 2025

Walkthrough

Updated promotion-service queries to exclude Seller-type orders when counting promotion usages. Two methods—countPromotionUsagesForCustomer and countPromotionUsages—now filter out orders that are inactive and not of OrderType.Seller; an import of OrderType was added.

Changes

Cohort / File(s) Change Summary
Promotion service update
packages/core/src/service/services/promotion.service.ts
Added import of OrderType and updated countPromotionUsagesForCustomer and countPromotionUsages query builders to exclude orders where order.active = false and order.type != OrderType.Seller.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description includes a clear summary linked to issue #3042, notes no breaking changes, and the checklist reflects completion of core requirements.
Linked Issues check ✅ Passed The code changes directly address issue #3042 by excluding Seller-type orders from promotion usage counts in both required query builders.
Out of Scope Changes check ✅ Passed All changes are focused on the specific objective of excluding seller orders from promotion usage counting; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The PR title accurately summarizes the main change: excluding seller orders when counting promotion usage, which directly addresses the issue of incorrect promotion usage counting for split orders.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e235d85 and 61c9362.

📒 Files selected for processing (1)
  • packages/core/src/service/services/promotion.service.ts (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: e2e tests (20.x, mariadb)
  • GitHub Check: e2e tests (20.x, postgres)
  • GitHub Check: e2e tests (24.x, postgres)
  • GitHub Check: e2e tests (20.x, sqljs)
  • GitHub Check: e2e tests (24.x, mariadb)
  • GitHub Check: e2e tests (24.x, sqljs)
  • GitHub Check: build (24.x)
  • GitHub Check: unit tests (24.x)
  • GitHub Check: unit tests (20.x)
  • GitHub Check: build (22.x)
  • GitHub Check: build (20.x)
  • GitHub Check: publish_install (macos-latest, 22.x)
  • GitHub Check: publish_install (ubuntu-latest, 22.x)
  • GitHub Check: publish_install (ubuntu-latest, 20.x)
  • GitHub Check: publish_install (windows-latest, 20.x)
  • GitHub Check: publish_install (macos-latest, 24.x)
  • GitHub Check: publish_install (macos-latest, 20.x)
  • GitHub Check: publish_install (windows-latest, 22.x)
  • GitHub Check: publish_install (ubuntu-latest, 24.x)
  • GitHub Check: publish_install (windows-latest, 24.x)

Comment on lines 365 to 366
.andWhere('order.active = :active', { active: false })
.andWhere('order.type != :type', { type: 'Seller' });
Copy link
Contributor

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

🏁 Script executed:

cat -n packages/core/src/service/services/promotion.service.ts | sed -n '350,375p'

Repository: vendure-ecommerce/vendure

Length of output: 1465


🏁 Script executed:

fd -e "spec.ts" -e "test.ts" | grep -i promotion

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

git diff --name-only | grep -E "(promotion|test)"

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

find . -type d -name "__tests__" -o -type d -name "test" -o -type d -name "tests" | head -20

Repository: vendure-ecommerce/vendure

Length of output: 144


🏁 Script executed:

rg -l "promotion" --type ts --type js | grep -i "test\|spec" | head -20

Repository: vendure-ecommerce/vendure

Length of output: 1016


🏁 Script executed:

git status | grep -E "(promotion|test)"

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "countPromotionUsages" packages/core/

Repository: vendure-ecommerce/vendure

Length of output: 628


🏁 Script executed:

wc -l packages/core/e2e/promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 112


🏁 Script executed:

wc -l packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 119


🏁 Script executed:

grep -n "countPromotionUsages\|order.type\|Seller" packages/core/e2e/promotion.e2e-spec.ts | head -30

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

grep -n "countPromotionUsages\|order.type\|Seller" packages/core/e2e/order-promotion.e2e-spec.ts | head -30

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

grep -n "Seller\|type.*:" packages/core/src/service/services/promotion.service.ts | head -20

Repository: vendure-ecommerce/vendure

Length of output: 208


🏁 Script executed:

sed -n '340,370p' packages/core/src/service/services/promotion.service.ts

Repository: vendure-ecommerce/vendure

Length of output: 1374


🏁 Script executed:

rg "countPromotionUsages" -B5 -A5 packages/core/src/service/services/promotion.service.ts

Repository: vendure-ecommerce/vendure

Length of output: 1717


🏁 Script executed:

grep -n "usageCount\|usageLimit" packages/core/src/service/services/promotion.service.ts | head -20

Repository: vendure-ecommerce/vendure

Length of output: 539


🏁 Script executed:

sed -n '260,285p' packages/core/src/service/services/promotion.service.ts

Repository: vendure-ecommerce/vendure

Length of output: 1246


🏁 Script executed:

rg "perCustomerUsageLimit\|usageLimit" packages/core/e2e/promotion.e2e-spec.ts -A3 -B3 | head -40

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "perCustomerUsageLimit\|usageLimit" packages/core/e2e/order-promotion.e2e-spec.ts -A3 -B3 | head -40

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

sed -n '1,50p' packages/core/e2e/promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 1889


🏁 Script executed:

rg "describe.*[Uu]sage|it.*[Uu]sage|test.*[Uu]sage" packages/core/e2e/promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "describe.*[Uu]sage|it.*[Uu]sage|test.*[Uu]sage" packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 1372


🏁 Script executed:

rg "cancelled orders do not count against usage limit" -A20 packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 2127


🏁 Script executed:

rg "per-customer usage limit|usage limit" -A30 packages/core/e2e/order-promotion.e2e-spec.ts | head -80

Repository: vendure-ecommerce/vendure

Length of output: 3310


🏁 Script executed:

grep -n "Seller\|order.type" packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "type.*Seller|Seller.*type" packages/core/e2e/ -l

Repository: vendure-ecommerce/vendure

Length of output: 177


🏁 Script executed:

rg "type.*:.*order\|order.*type" packages/core/src/entity/ | grep -i seller

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

grep -rn "enum.*OrderType\|type.*Seller" packages/core/src/entity/order/

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

fd "order.entity.ts" packages/core/src/entity/

Repository: vendure-ecommerce/vendure

Length of output: 115


🏁 Script executed:

rg "OrderType|type.*string" packages/core/src/entity/order/order.entity.ts -A3 -B3 | head -40

Repository: vendure-ecommerce/vendure

Length of output: 472


🏁 Script executed:

grep -rn "Seller" packages/core/src/entity/order/ | head -20

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "enum OrderType|OrderType.*=" packages/core/ | head -20

Repository: vendure-ecommerce/vendure

Length of output: 297


🏁 Script executed:

fd "generated-types.ts" packages/common/lib/ | head -5

Repository: vendure-ecommerce/vendure

Length of output: 176


🏁 Script executed:

grep -rn "OrderType.*Seller\|Seller.*OrderType" packages/

Repository: vendure-ecommerce/vendure

Length of output: 1243


🏁 Script executed:

cat packages/core/src/api/schema/common/order.type.graphql | grep -A5 "enum OrderType"

Repository: vendure-ecommerce/vendure

Length of output: 125


🏁 Script executed:

rg "it\(.*seller.*\)" -i packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

sed -n '1,100p' packages/core/e2e/order-promotion.e2e-spec.ts | grep -E "describe|import"

Repository: vendure-ecommerce/vendure

Length of output: 1377


🏁 Script executed:

rg "Seller.*order\|order.*Seller" packages/core/e2e/order-promotion.e2e-spec.ts

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

rg "multivendor\|seller" -i packages/core/e2e/order-promotion.e2e-spec.ts | head -10

Repository: vendure-ecommerce/vendure

Length of output: 51


🏁 Script executed:

grep -n "describe\|it(" packages/core/e2e/order-promotion.e2e-spec.ts | grep -i "usage\|seller\|vendor"

Repository: vendure-ecommerce/vendure

Length of output: 950


Test coverage is missing for the seller order exclusion filter.

The consistency between countPromotionUsagesForCustomer and countPromotionUsages is good—both now exclude seller orders from usage counts. However, existing tests cover cancelled orders and basic usage limit logic, but do not verify that seller orders are actually excluded from promotion usage calculations.

Add test cases for:

  1. Seller orders do not count toward per-customer usage limits
  2. Seller orders do not count toward global usage limits
  3. Regular and Aggregate order types are still counted correctly

@twlite twlite changed the title Fix(core): exclude seller orders when counting promotion usage fix(core): Exclude seller orders when counting promotion usage Dec 20, 2025
@sonarqubecloud
Copy link

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.

Incorrect promotion usage count for split orders

2 participants