Feat/tax inclusive promotions percentage#4
Conversation
|
CodeAnt AI is reviewing your PR. |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| ) | ||
| ).data.cart | ||
|
|
||
| console.log(JSON.stringify(cart, null, 2)) |
There was a problem hiding this comment.
Suggestion: Remove the debug console.log from this test. It dumps full cart payloads on every run, which clutters CI output and can leak internal pricing/adjustment details into logs, making failures harder to triage at scale. [code quality]
Severity Level: Major ⚠️
- ⚠️ Promotions admin integration test logs full cart JSON.
- ⚠️ CI logs become noisy, obscuring real test failures.
- ⚠️ Detailed cart pricing structure exposed in CI output.Steps of Reproduction ✅
1. Run the integration test suite that includes
`integration-tests/http/__tests__/promotions/admin/promotions.spec.ts` (e.g., via the
project's Jest-based integration test runner).
2. During execution of the `it("should add tax inclusive promotion to cart successfully
with percentage discount", ...)` test starting at line ~1670 in `promotions.spec.ts`, the
code at line 1791 executes `console.log(JSON.stringify(cart, null, 2))` after the cart is
created.
3. This `console.log` prints the full `cart` object, including all pricing, adjustments,
and tax details, to stdout for every run of this test.
4. When tests run in CI, these logs accumulate in the job output, cluttering logs with
large JSON payloads and exposing detailed internal cart/pricing structures that are not
needed for asserting correctness.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
**Line:** 1791:1791
**Comment:**
*Code Quality: Remove the debug `console.log` from this test. It dumps full cart payloads on every run, which clutters CI output and can leak internal pricing/adjustment details into logs, making failures harder to triage at scale.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| export class Migration20250708075529 extends Migration { | ||
|
|
||
| override async up(): Promise<void> { | ||
| this.addSql(`alter table if exists "cart_line_item_adjustment" add column if not exists "promotion_type" text check ("promotion_type" in ('fixed', 'percentage')) null;`); |
There was a problem hiding this comment.
Suggestion: This migration adds promotion_type as nullable but never backfills existing cart_line_item_adjustment rows, so historical percentage promotion adjustments will remain NULL. Downstream totals logic checks promotion_type === "percentage" and treats NULL as non-percentage, which changes tax-inclusive discount math and can produce incorrect subtotal/tax/total values when old carts are recalculated. Add a data migration step to populate existing promotion adjustments with the correct type (or a safe fallback strategy that preserves previous pricing behavior). [incomplete implementation]
Severity Level: Critical 🚨
- ❌ Existing carts with tax-inclusive percentage promotion miscalculate totals.
- ❌ Storefront checkout may display inconsistent discounts for old carts.
- ⚠️ Legacy cart adjustments behave as fixed discounts post-migration.Steps of Reproduction ✅
1. Apply the cart module migrations so that `cart_line_item_adjustment` has both
`is_tax_inclusive` (added in `Migration20250508081553` at
`packages/modules/cart/src/migrations/Migration20250508081553.ts:5-6`) and the new
nullable `promotion_type` column (added in `Migration20250708075529` at
`packages/modules/cart/src/migrations/Migration20250708075529.ts:5-7`).
2. Insert or simulate an existing line item adjustment row representing a tax-inclusive
percentage promotion that predates `promotion_type` by creating a cart and line item via
`CartModuleService.createCarts`
(`packages/modules/cart/src/services/cart-module.ts:261-325`), then directly persisting a
`cart_line_item_adjustment` row with `item_id` pointing to that line item, `amount` set to
a discount value, `is_tax_inclusive = true`, `promotion_id` referencing a promotion whose
`application_method.type = 'percentage'` (see promotion model at
`packages/modules/promotion/src/models/promotion.ts:6-13` and compute-actions at
`packages/modules/promotion/src/utils/compute-actions/line-items.ts:161-169`), and
`promotion_type` left `NULL` (the default after the migration).
3. Retrieve the affected cart through the cart module in a way that requests totals, for
example by calling `CartModuleService.retrieveCart` with a `select` including total fields
(`packages/modules/cart/src/services/cart-module.ts:199-213`), which in turn invokes
`decorateCartTotals(cart)` from `packages/core/utils/src/totals/cart/index.ts:5-8,27-35`.
This function calls `getLineItemsTotals`, which passes `item.adjustments` into
`calculateAdjustmentTotal`
(`packages/core/utils/src/totals/line-item/index.ts:58-69,130-138`).
4. Inside `calculateAdjustmentTotal`
(`packages/core/utils/src/totals/adjustment/index.ts:25-38,43-47`), each adjustment is
processed using `adj.is_tax_inclusive` and `adj.promotion_type`: for tax-inclusive
adjustments, the subtotal uses the special branch only when `adj.promotion_type ===
"percentage"`. Because the historical adjustment row has `promotion_type = NULL`, the
condition is false even though the underlying promotion is percentage, so the function
divides by `(1 + taxRate)` instead of treating the amount as a percentage-style inclusive
discount. This changes `discount_subtotal`, `discount_tax_total`, and the cart
`subtotal`/`tax_total` compared to equivalent new adjustments where `promotion_type` is
correctly populated (`prepareAdjustmentsFromPromotionActionsStep` sets `promotion_type`
for new rows at
`packages/core/core-flows/src/cart/steps/prepare-adjustments-from-promotion-actions.ts:130-139`).
Thus, old carts with tax-inclusive percentage promotions are recalculated with different
totals solely because their `promotion_type` remained `NULL` after the migration.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/modules/cart/src/migrations/Migration20250708075529.ts
**Line:** 6:6
**Comment:**
*Incomplete Implementation: This migration adds `promotion_type` as nullable but never backfills existing `cart_line_item_adjustment` rows, so historical percentage promotion adjustments will remain `NULL`. Downstream totals logic checks `promotion_type === "percentage"` and treats `NULL` as non-percentage, which changes tax-inclusive discount math and can produce incorrect subtotal/tax/total values when old carts are recalculated. Add a data migration step to populate existing promotion adjustments with the correct type (or a safe fallback strategy that preserves previous pricing behavior).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| item_id: item.id, | ||
| amount, | ||
| code: promotion.code!, | ||
| promotion_type: promotion.application_method?.type!, | ||
| }) |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
Buy-get computed actions now set promotion_type but still omit is_tax_inclusive, while calculateAdjustmentTotal only applies the tax-inclusive percentage path when both flags are set on adjustments. As a result, tax-inclusive buy-get percentage promotions produce line-item adjustments that are always treated as tax-exclusive in totals.
Suggestion: Propagate promotion.is_tax_inclusive into buy-get ADD_ITEM_ADJUSTMENT actions (mirroring standard item promotions) and add an integration test covering a buy-get + percentage + tax-inclusive cart to verify the totals align with calculateAdjustmentTotal's new condition.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** packages/modules/promotion/src/utils/compute-actions/buy-get.ts
**Line:** 272:276
**Comment:**
*HIGH: Buy-get computed actions now set `promotion_type` but still omit `is_tax_inclusive`, while `calculateAdjustmentTotal` only applies the tax-inclusive percentage path when both flags are set on adjustments. As a result, tax-inclusive buy-get percentage promotions produce line-item adjustments that are always treated as tax-exclusive in totals.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| adj.promotion_type === "percentage" | ||
| ? adjustmentAmount | ||
| : MathBN.div(adjustmentAmount, MathBN.add(1, taxRate)) |
There was a problem hiding this comment.
Suggestion: This new percentage check assumes promotion_type is always present, but the migration adds the column as nullable and does not backfill existing adjustments. Any pre-existing tax-inclusive percentage adjustments (or adjustments created without this field) will be treated as fixed discounts, so subtotal/tax will be under-discounted. Add a backward-compatible fallback (or backfill strategy) so legacy percentage adjustments are not misclassified. [logic error]
Severity Level: Critical 🚨
- ❌ Cart totals wrong for legacy tax-inclusive percentage adjustments.
- ❌ Checkout shows smaller discount and higher tax amounts.
- ⚠️ Historical carts with promotions report inconsistent discount subtotals.Steps of Reproduction ✅
1. Apply database migrations so that the `cart_line_item_adjustment` table exists and has
a nullable `promotion_type` column, as defined in
`packages/modules/cart/src/migrations/Migration20240222170223.ts:9-24` (base table) and
`Migration20250708075529.ts:5-7` (adds `promotion_type` as `text ... null` without
backfill).
2. Seed a cart that has at least one line item adjustment row in
`cart_line_item_adjustment` with `is_tax_inclusive = true` (column added with default
true/false in `Migration20250508081553.ts:5-7`) and `promotion_type IS NULL`, representing
a tax-inclusive percentage promotion created before the `promotion_type` column existed or
created via APIs that don't set `promotion_type` (see `CreateLineItemAdjustmentDTO` in
`packages/core/types/src/cart/mutations.ts:40-45`, which has no `promotion_type` field).
3. Retrieve that cart through the cart module with totals enabled—for example, by calling
`CartModuleService.retrieveCart` in
`packages/modules/cart/src/services/cart-module.ts:199-214`, which, when
`shouldIncludeTotals` returns true, invokes `decorateCartTotals(cart)` to compute totals
using the utilities in `packages/core/utils/src/totals`.
4. During totals computation, `getLineItemTotals` in
`packages/core/utils/src/totals/line-item/index.ts:130-138` calls
`calculateAdjustmentTotal({ adjustments: item.adjustments || [], includesTax:
isTaxInclusive, taxRate: sumTaxRate })`. Inside `calculateAdjustmentTotal`
(`packages/core/utils/src/totals/adjustment/index.ts:32-38`), the adjustment object for
this legacy row has `adj.is_tax_inclusive === true` and `adj.promotion_type ===
undefined`, so the check `adj.promotion_type === "percentage"` at line 35 is false. The
code therefore executes the `fixed` branch `MathBN.div(adjustmentAmount, MathBN.add(1,
taxRate))` instead of treating it as percentage, which underestimates the discount
subtotal and overstates tax for this adjustment compared to the intended percentage
behavior, demonstrating the misclassification of legacy percentage adjustments with null
`promotion_type`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/core/utils/src/totals/adjustment/index.ts
**Line:** 35:37
**Comment:**
*Logic Error: This new percentage check assumes `promotion_type` is always present, but the migration adds the column as nullable and does not backfill existing adjustments. Any pre-existing tax-inclusive percentage adjustments (or adjustments created without this field) will be treated as fixed discounts, so subtotal/tax will be under-discounted. Add a backward-compatible fallback (or backfill strategy) so legacy percentage adjustments are not misclassified.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
|
PR review started! Estimated time: 40-45 minutes. Learn More Ask Codity questions: Mention Trigger a manual review: Comment Generate unit tests: Comment Run security scan again: Comment |
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. AnalysisComplex complexity • Components: LineItemAdjustment model, prepareAdjustmentsFromPromotionActionsStep, promotion compute-actions utilities sequenceDiagram
title Tax-Inclusive Percentage Promotions Workflow
participant Client
participant AdminAPI
participant StoreAPI
participant PromotionService
participant CartService
participant TotalsCalculator
participant Database
Note over Client,Database: Creating Tax-Inclusive Percentage Promotion
Client->>AdminAPI: POST /admin/promotions
Note right: is_tax_inclusive: true<br>type: percentage<br>value: 10
AdminAPI->>Database: Save promotion with is_tax_inclusive flag
Database-->>AdminAPI: Promotion created
AdminAPI-->>Client: 200 OK with promotion details
Note over Client,Database: Applying Promotion to Cart
Client->>StoreAPI: POST /store/carts with promo_codes
StoreAPI->>PromotionService: computeActions(cartContext, promoCodes)
PromotionService->>PromotionService: Validate promotion rules
alt Promotion is percentage and tax-inclusive
PromotionService->>PromotionService: Calculate percentage discount
Note right: discount = subtotal * percentage
end
PromotionService-->>StoreAPI: Return computed actions with promotion_type
StoreAPI->>CartService: prepareAdjustmentsFromPromotionActions(actions)
CartService->>CartService: Transform actions to adjustments
Note right: Include promotion_type in adjustment data
CartService->>Database: Save LineItemAdjustment with promotion_type
Database-->>CartService: Adjustment saved
StoreAPI->>TotalsCalculator: decorateCartTotals(cart)
TotalsCalculator->>TotalsCalculator: Calculate cart totals
alt Adjustment has promotion_type and is tax-inclusive
TotalsCalculator->>TotalsCalculator: Apply special tax calculation
Note right: For percentage discounts on tax-inclusive prices<br>Calculate discount on tax-inclusive amount<br>Then compute tax on discounted base
else Standard calculation
TotalsCalculator->>TotalsCalculator: Apply normal tax calculation
end
TotalsCalculator-->>StoreAPI: Return calculated totals
StoreAPI-->>Client: 200 OK with cart and calculated totals
Note over Client,Database: Completing Cart to Order
Client->>StoreAPI: POST /store/carts/{id}/complete
StoreAPI->>CartService: Process cart completion
CartService->>Database: Create order from cart
Note right: Preserve adjustment amounts and promotion references
Database-->>CartService: Order created
StoreAPI-->>Client: 200 OK with order details
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
![codity-dm[bot]](https://avatars.githubusercontent.com/u/234452856?s=60&v=4){kind=small}
| // items: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // quantity: 1, | ||
| // unit_price: 1300, | ||
|
|
||
| // subtotal: 1040, | ||
| // tax_total: 240, | ||
| // total: 1200, | ||
|
|
||
| // original_total: 1300, | ||
| // original_tax_total: 260, | ||
|
|
||
| // discount_total: 100, | ||
| // discount_subtotal: 80, | ||
| // discount_tax_total: 20, | ||
|
|
||
| // adjustments: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // amount: 100, | ||
| // is_tax_inclusive: true, | ||
| // }), | ||
| // ]), | ||
| // }), | ||
| // ]), |
There was a problem hiding this comment.
A large block of item-level assertions (lines 1831: 1854) is commented out with internally inconsistent values. The test creates one line item with quantity 2 and unit_price 1300, but the commented assertions specify quantity 1 and unit_price 1300. The discount figures (discount_subtotal: 80, discount_tax_total: 20) do not match the expected values for a 10% discount on 2 × 1300 DKK. This leaves item-level behavior entirely untested while giving a false impression of coverage.
Suggested fix
items: expect.arrayContaining([
expect.objectContaining({
quantity: 2,
unit_price: 1300,
subtotal: 2080,
tax_total: 468,
total: 2340,
original_total: 2600,
original_tax_total: 520,
discount_total: 260,
discount_subtotal: 208,
discount_tax_total: 52,
adjustments: expect.arrayContaining([
expect.objectContaining({
amount: 260,
is_tax_inclusive: true,
}),
]),
}),
]),Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1831-1854
Issue Type: functional-medium
Severity: medium
Issue Description:
A large block of item-level assertions (lines 1831: 1854) is commented out with internally inconsistent values. The test creates one line item with quantity 2 and unit_price 1300, but the commented assertions specify quantity 1 and unit_price 1300. The discount figures (discount_subtotal: 80, discount_tax_total: 20) do not match the expected values for a 10% discount on 2 × 1300 DKK. This leaves item-level behavior entirely untested while giving a false impression of coverage.
Current Code:
// items: expect.arrayContaining([
// expect.objectContaining({
// quantity: 1,
// unit_price: 1300,
// subtotal: 1040,
// tax_total: 240,
// total: 1200,
// original_total: 1300,
// original_tax_total: 260,
// discount_total: 100,
// discount_subtotal: 80,
// discount_tax_total: 20,
// adjustments: expect.arrayContaining([
// expect.objectContaining({
// amount: 100,
// is_tax_inclusive: true,
// }),
// ]),
// }),
// ]),
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| ) | ||
| ).data.cart | ||
|
|
||
| console.log(JSON.stringify(cart, null, 2)) |
There was a problem hiding this comment.
A debug console.log(JSON.stringify(cart, null, 2)) statement was left in the integration test at line 1791. This will output verbose cart JSON data to CI logs on every test run, creating unnecessary noise and potentially exposing internal data structures in build artifacts.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1791-1791
Issue Type: maintainability-low
Severity: low
Issue Description:
A debug `console.log(JSON.stringify(cart, null, 2))` statement was left in the integration test at line 1791. This will output verbose cart JSON data to CI logs on every test run, creating unnecessary noise and potentially exposing internal data structures in build artifacts.
Current Code:
console.log(JSON.stringify(cart, null, 2))
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
The total: BigNumberInput field in the ComputeActionItemLine interface is unused by promotion calculation logic. The calculateAdjustmentAmountFromPromotion function derives totals from lineItem.subtotal / lineItem.quantity * quantity, and the allocation loop sums only item.subtotal, making the total field a dead field that creates a misleading API contract.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/types/src/promotion/common/compute-actions.ts
Lines: 3-6
Issue Type: functional-medium
Severity: medium
Issue Description:
The `total: BigNumberInput` field in the `ComputeActionItemLine` interface is unused by promotion calculation logic. The `calculateAdjustmentAmountFromPromotion` function derives totals from `lineItem.subtotal / lineItem.quantity * quantity`, and the allocation loop sums only `item.subtotal`, making the `total` field a dead field that creates a misleading API contract.
Current Code:
/**
* The total of the line item.
*/
total: BigNumberInput
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 35.4sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 1 packagesMPL-2.0 (1 packages):
Unknown Licenses - 56 packages
...and 36 more Powered by Codity.ai · Docs |
|
@codity review |
PR SummaryWhat Changed
Key Changes by AreaDatabase: Added Tax Calculation: Updated Promotion Module: Added Types: Extended Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The tax calculation fix intentionally branches by promotion type because percentage discounts are pre-calculated on tax-inclusive bases, while fixed discounts need tax extraction. This preserves backward compatibility for fixed promotions. Risks:
Merge StatusNOT MERGEABLE — PR Score 42/100, below threshold (50)
Gate reference: H1=Critical SAST · H2=Critical CVE/dep · H3=Copyleft license · H4=Score below floor · H5=Critical review finding · H6=Catastrophic code quality · H7=High SAST |
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Tax-Inclusive Promotion Calculation FlowComplex complexity • Components: LineItemAdjustment model, prepareAdjustmentsFromPromotionActionsStep, calculateAdjustmentTotal utility sequenceDiagram
title Tax-Inclusive Percentage Promotions Workflow
participant Client
participant AdminAPI as Admin API
participant StoreAPI as Store API
participant PromotionService as Promotion Service
participant CartWorkflow as Cart Workflow
participant TaxCalculator as Tax Calculator
participant Database
Client->>AdminAPI: POST /admin/promotions (is_tax_inclusive: true, type: percentage)
AdminAPI->>PromotionService: Create promotion with tax-inclusive flag
PromotionService->>Database: Save promotion configuration
PromotionService-->>AdminAPI: Return created promotion
AdminAPI-->>Client: Promotion created (PERCENTAGE_10)
Client->>StoreAPI: POST /store/carts (with promo_codes)
StoreAPI->>CartWorkflow: Process cart creation with promotions
CartWorkflow->>PromotionService: computeActions(cart context)
PromotionService->>PromotionService: Calculate percentage discount (10%)
PromotionService->>PromotionService: Apply tax-inclusive logic in line-items.ts
PromotionService-->>CartWorkflow: Return adjustment actions with is_tax_inclusive flag
CartWorkflow->>CartWorkflow: prepareAdjustmentsFromPromotionActionsStep
Note right of CartWorkflow: Maps promotion_type to LineItemAdjustment model
CartWorkflow->>Database: Create line item adjustments with promotion_type
CartWorkflow->>TaxCalculator: calculateAdjustmentTotal
Note right of TaxCalculator: Uses promotion_type for tax-inclusive percentage calculations
TaxCalculator->>TaxCalculator: Compute taxable base (subtotal - discount_subtotal)
TaxCalculator->>TaxCalculator: Apply tax rate to net amount
TaxCalculator-->>CartWorkflow: Return calculated totals
CartWorkflow->>Database: Update cart totals
CartWorkflow-->>StoreAPI: Return cart with adjustments and totals
StoreAPI-->>Client: Cart response with tax-inclusive discount applied
alt Promotion is tax-inclusive percentage
Note over TaxCalculator: discount_total = percentage of tax-inclusive pricebr/discount_subtotal = discount_total / 1 + tax_ratebr/discount_ta...
else Promotion is tax-exclusive
Note over TaxCalculator: discount_total = percentage of tax-exclusive pricebr/discount_subtotal = discount_totalbr/discount_tax_total = disco...
end
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
| // items: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // quantity: 1, | ||
| // unit_price: 1300, | ||
|
|
||
| // subtotal: 1040, | ||
| // tax_total: 240, | ||
| // total: 1200, | ||
|
|
||
| // original_total: 1300, | ||
| // original_tax_total: 260, | ||
|
|
||
| // discount_total: 100, | ||
| // discount_subtotal: 80, | ||
| // discount_tax_total: 20, | ||
|
|
||
| // adjustments: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // amount: 100, | ||
| // is_tax_inclusive: true, | ||
| // }), | ||
| // ]), | ||
| // }), | ||
| // ]), |
There was a problem hiding this comment.
Lines 1831: 1854 contain a large commented-out block of assertions that verify per-item discount breakdowns including unit_price, subtotal, tax_total, discount details, and tax-inclusive adjustments. These assertions are directly relevant to validating the tax-inclusive percentage discount fix at the item level, but the test only asserts cart-level totals. The commented-out code should be uncommented to activate these important assertions or removed entirely to avoid hiding incomplete test coverage.
Suggested fix
items: expect.arrayContaining([
expect.objectContaining({
quantity: 1,
unit_price: 1300,
subtotal: 1040,
tax_total: 240,
total: 1200,
original_total: 1300,
original_tax_total: 260,
discount_total: 100,
discount_subtotal: 80,
discount_tax_total: 20,
adjustments: expect.arrayContaining([
expect.objectContaining({
amount: 100,
is_tax_inclusive: true,
}),
]),
}),
]),Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1831-1854
Issue Type: robustness-low
Severity: low
Issue Description:
Lines 1831: 1854 contain a large commented-out block of assertions that verify per-item discount breakdowns including unit_price, subtotal, tax_total, discount details, and tax-inclusive adjustments. These assertions are directly relevant to validating the tax-inclusive percentage discount fix at the item level, but the test only asserts cart-level totals. The commented-out code should be uncommented to activate these important assertions or removed entirely to avoid hiding incomplete test coverage.
Current Code:
// items: expect.arrayContaining([
// expect.objectContaining({
// quantity: 1,
// unit_price: 1300,
//
// subtotal: 1040,
// tax_total: 240,
// total: 1200,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| item_id: item.id, | ||
| amount, | ||
| code: promotion.code!, | ||
| promotion_type: promotion.application_method?.type!, |
There was a problem hiding this comment.
The expression combines optional chaining (?.) with non-null assertion (!), which masks a potential undefined value. When application_method is absent, the chain short-circuits to undefined, and the ! only suppresses the TypeScript error without providing a runtime value. This causes promotion_type to be stored as NULL in the database, leading to incorrect discount calculations in calculateAdjustmentTotal where buy-get percentage promotions are treated as fixed-amount discounts and divided by the tax rate twice.
| promotion_type: promotion.application_method?.type!, | |
| promotion_type: promotion.application_method.type, |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/modules/promotion/src/utils/compute-actions/buy-get.ts
Lines: 275-275
Issue Type: functional-high
Severity: high
Issue Description:
The expression combines optional chaining (`?.`) with non-null assertion (`!`), which masks a potential `undefined` value. When `application_method` is absent, the chain short-circuits to `undefined`, and the `!` only suppresses the TypeScript error without providing a runtime value. This causes `promotion_type` to be stored as NULL in the database, leading to incorrect discount calculations in `calculateAdjustmentTotal` where buy-get percentage promotions are treated as fixed-amount discounts and divided by the tax rate twice.
Current Code:
promotion_type: promotion.application_method?.type!,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| amount, | ||
| code: promotion.code!, | ||
| is_tax_inclusive: promotion.is_tax_inclusive, | ||
| promotion_type: promotion.application_method?.type!, |
There was a problem hiding this comment.
At line 168, the code re-accesses promotion.application_method?.type! instead of using the already-destructured and validated applicationMethod.type. If the type property is unset on a partially-constructed promotion record, promotion_type silently becomes undefined, causing incorrect discount calculations for tax-inclusive percentage promotions.
| promotion_type: promotion.application_method?.type!, | |
| promotion_type: applicationMethod.type, |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/modules/promotion/src/utils/compute-actions/line-items.ts
Lines: 168-168
Issue Type: functional-high
Severity: high
Issue Description:
At line 168, the code re-accesses `promotion.application_method?.type!` instead of using the already-destructured and validated `applicationMethod.type`. If the `type` property is unset on a partially-constructed promotion record, `promotion_type` silently becomes `undefined`, causing incorrect discount calculations for tax-inclusive percentage promotions.
Current Code:
promotion_type: promotion.application_method?.type!,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| is_tax_inclusive: (action as AddItemAdjustmentAction).is_tax_inclusive, | ||
| item_id: (action as AddItemAdjustmentAction).item_id, | ||
| promotion_id: promotionsMap.get(action.code)?.id, | ||
| promotion_type: (action as AddItemAdjustmentAction).promotion_type, |
There was a problem hiding this comment.
The shippingMethodAdjustmentsToCreate mapping (lines 145: 155) is missing the promotion_type field that is correctly included in the lineItemAdjustmentsToCreate mapping (line 138). This causes shipping method adjustments to always have promotion_type === undefined, preventing the percentage-discount logic in calculateAdjustmentTotal from being applied to shipping adjustments, resulting in incorrect tax calculations for tax-inclusive percentage promotions on shipping.
Suggested fix
const shippingMethodAdjustmentsToCreate = actions
.filter(
(a) => a.action === ComputedActions.ADD_SHIPPING_METHOD_ADJUSTMENT
)
.map((action) => ({
code: action.code,
amount: (action as AddShippingMethodAdjustment).amount,
is_tax_inclusive: (action as AddShippingMethodAdjustment).is_tax_inclusive,
shipping_method_id: (action as AddShippingMethodAdjustment)
.shipping_method_id,
promotion_id: promotionsMap.get(action.code)?.id,
promotion_type: (action as AddShippingMethodAdjustment).promotion_type,
}))Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/core-flows/src/cart/steps/prepare-adjustments-from-promotion-actions.ts
Lines: 138-138
Issue Type: functional-high
Severity: high
Issue Description:
The `shippingMethodAdjustmentsToCreate` mapping (lines 145: 155) is missing the `promotion_type` field that is correctly included in the `lineItemAdjustmentsToCreate` mapping (line 138). This causes shipping method adjustments to always have `promotion_type === undefined`, preventing the percentage-discount logic in `calculateAdjustmentTotal` from being applied to shipping adjustments, resulting in incorrect tax calculations for tax-inclusive percentage promotions on shipping.
Current Code:
promotion_type: (action as AddItemAdjustmentAction).promotion_type,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| ) | ||
| ).data.cart | ||
|
|
||
| console.log(JSON.stringify(cart, null, 2)) |
There was a problem hiding this comment.
A debug console.log statement was left in the integration test at line 1791 that outputs the full cart JSON object. This pollutes CI output and makes test failures harder to read, and should be removed.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1791-1791
Issue Type: maintainability-low
Severity: low
Issue Description:
A debug console.log statement was left in the integration test at line 1791 that outputs the full cart JSON object. This pollutes CI output and makes test failures harder to read, and should be removed.
Current Code:
console.log(JSON.stringify(cart, null, 2))
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 34.6sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 1 packagesMPL-2.0 (1 packages):
Unknown Licenses - 56 packages
...and 36 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/medusa · PR #4Scanned: 2026-06-08 16:09 UTC | Score: 46/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
integration-tests/http/__tests__/promotions/admin/promotions.spec.ts |
0 | 1 | 3 | 24 | 28 |
packages/modules/cart/src/migrations/Migration20250708075529.ts |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
PR SummaryWhat Changed
Key Changes by AreaCart Module: Added Promotion Module: Updated Core Flows: Modified Types: Extended Tests: Added integration test verifying 10% tax-inclusive percentage promotion calculates correctly (subtotal 2080, discount 260, tax 468, total 2340 DKK). Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split calculation approach (percentage vs fixed) is intentional. Percentage discounts apply to tax-inclusive total directly. Fixed amount discounts are adjusted by tax rate to maintain consistent discount value. This matches how different promotion types interact with tax-inclusive pricing. Risks:
Merge StatusMERGEABLE — PR Score 59/100, above threshold (50). All gates passed. |
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Tax-Inclusive Promotion Calculation FlowComplex complexity • Components: LineItemAdjustment model, prepareAdjustmentsFromPromotionActionsStep, calculateAdjustmentTotal utility sequenceDiagram
title Tax-Inclusive Percentage Promotions Workflow
participant Client as Client
participant AdminAPI as Admin Promotions API
participant StoreAPI as Store Cart API
participant PromoService as Promotion Service
participant CartWorkflow as Cart Workflow
participant CartService as Cart Service
participant Database as Database
Note over Client,Database: Creating Tax-Inclusive Percentage Promotion
Client->>AdminAPI: POST /admin/promotions
Note right of Client: is_tax_inclusive: truebr/type: "percentage"br/value: 10
AdminAPI->>Database: Create promotion with is_tax_inclusive flag
Database-->>AdminAPI: Promotion created
AdminAPI-->>Client: Return promotion with is_tax_inclusive: true
Note over Client,Database: Applying Promotion to Cart
Client->>StoreAPI: POST /store/carts with promo_codes
StoreAPI->>CartWorkflow: Trigger cart update workflow
CartWorkflow->>PromoService: computeActions(cartContext)
Note right of CartWorkflow: Context includes items, currency,br/and existing adjustments
PromoService->>PromoService: Calculate percentage discount
Note right of PromoService: For percentage type with is_tax_inclusive,br/compute discount based on tax-inclusive amount
PromoService-->>CartWorkflow: Return promotion actions with<br/>is_tax_inclusive metadata
CartWorkflow->>CartWorkflow: prepareAdjustmentsFromPromotionActionsStep
Note right of CartWorkflow: Map promotion actions to line item adjustmentsbr/preserving is_tax_inclusive flag
CartWorkflow->>CartService: Create line item adjustments
CartService->>Database: Insert LineItemAdjustment with promotion_type column
Note right of Database: New migration adds promotion_type enumbr/to cart_line_item_adjustment table
CartWorkflow->>CartWorkflow: calculateAdjustmentTotal
Note right of CartWorkflow: For tax-inclusive percentage discounts,br/calculate tax portion of discount separately
CartWorkflow-->>StoreAPI: Return updated cart with calculated totals
StoreAPI-->>Client: Cart with tax-inclusive discount applied
Note over Client,Database: Verification Flow
Client->>StoreAPI: GET /store/carts/:id
StoreAPI->>CartService: Fetch cart with adjustments
CartService->>Database: Query line item adjustments with promotion_type
Database-->>CartService: Return adjustments
CartService-->>StoreAPI: Cart with is_tax_inclusive adjustments
StoreAPI-->>Client: Verify discount_total, discount_subtotal,<br/>and discount_tax_total are correctly calculated
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
![codity-chait[bot]](https://avatars.githubusercontent.com/u/228176587?s=60&v=4){kind=small}
There was a problem hiding this comment.
The lineItemAdjustmentsToCreate property in the PrepareAdjustmentsFromPromotionActionsStepOutput interface is missing two properties that are actually constructed at runtime: is_tax_inclusive and promotion_type. This causes a type mismatch where consumers cannot access these properties without TypeScript compilation errors, even though they are present in the actual runtime objects.
Suggested fix
lineItemAdjustmentsToCreate: {
/**
* The promotion code that computed the adjustment.
*/
code: string
/**
* The amount of the adjustment.
*/
amount: number
/**
* The ID of the line item to adjust.
*/
item_id: string
/**
* The ID of the applied promotion.
*/
promotion_id?: string
/**
* Whether the adjustment is tax inclusive.
*/
is_tax_inclusive: boolean
/**
* The type of the promotion.
*/
promotion_type: string
}[]Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/core-flows/src/cart/steps/prepare-adjustments-from-promotion-actions.ts
Lines: 30-47
Issue Type: robustness-high
Severity: high
Issue Description:
The `lineItemAdjustmentsToCreate` property in the `PrepareAdjustmentsFromPromotionActionsStepOutput` interface is missing two properties that are actually constructed at runtime: `is_tax_inclusive` and `promotion_type`. This causes a type mismatch where consumers cannot access these properties without TypeScript compilation errors, even though they are present in the actual runtime objects.
Current Code:
lineItemAdjustmentsToCreate: {
/**
* The promotion code that computed the adjustment.
*/
code: string
/**
* The amount of the adjustment.
*/
amount: number
/**
* The ID of the line item to adjust.
*/
item_id: string
/**
* The ID of the applied promotion.
*/
promotion_id?: string
}[]
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
The total: BigNumberInput field in ComputeActionItemLine is defined as a required property but is never accessed or used in the promotion module's computation code. Since only item.subtotal is utilized in the codebase, the required total field creates a breaking change that causes TypeScript strict mode to flag existing callers (including compute-actions.spec.ts) that construct items without providing this unused field.
Suggested fix
/**
* The total of the line item.
*/
total?: BigNumberInputPrompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/types/src/promotion/common/compute-actions.ts
Lines: 3-6
Issue Type: maintainability-medium
Severity: medium
Issue Description:
The `total: BigNumberInput` field in `ComputeActionItemLine` is defined as a required property but is never accessed or used in the promotion module's computation code. Since only `item.subtotal` is utilized in the codebase, the required `total` field creates a breaking change that causes TypeScript strict mode to flag existing callers (including `compute-actions.spec.ts`) that construct items without providing this unused field.
Current Code:
/**
* The total of the line item.
*/
total: BigNumberInput
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Nitpicks (Low Priority)Found 1 low-priority suggestions for code improvement Click to expand nitpicks
|
Security Scan Summary
No critical security issues detected Scan completed in 43.3sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 1 packagesMPL-2.0 (1 packages):
Unknown Licenses - 56 packages
...and 36 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/medusa · PR #4Scanned: 2026-06-08 18:19 UTC | Score: 46/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
integration-tests/http/__tests__/promotions/admin/promotions.spec.ts |
0 | 1 | 3 | 24 | 28 |
packages/modules/cart/src/migrations/Migration20250708075529.ts |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
PR SummaryWhat Changed
Key Changes by AreaCore Totals: Modified Data Model: Added Promotion Module: Updated compute-actions to include Cart Flows: Updated adjustment preparation to pass through Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split handling between fixed and percentage promotions in Risks: The unused Merge StatusMERGEABLE — PR Score 55/100, above threshold (50). All gates passed. |
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Tax-Inclusive Promotion Calculation FlowComplex complexity • Components: LineItemAdjustment model, prepareAdjustmentsFromPromotionActionsStep, promotion compute-actions utilities sequenceDiagram
title Tax-Inclusive Percentage Promotions Workflow
participant Client
participant AdminAPI
participant StoreAPI
participant PromotionService
participant CartService
participant TaxCalculator
participant Database
Note over Client,Database: Creating Tax-Inclusive Percentage Promotion
Client->>AdminAPI: POST /admin/promotions
Note right of Client: is_tax_inclusive: truebr/type: "percentage"br/value: 10
AdminAPI->>PromotionService: createPromotion()
PromotionService->>Database: INSERT promotion with is_tax_inclusive
Database-->>PromotionService: promotion created
PromotionService-->>AdminAPI: return promotion
AdminAPI-->>Client: 200 OK with promotion data
Note over Client,Database: Applying Promotion to Cart
Client->>StoreAPI: POST /store/carts with promo_codes
StoreAPI->>CartService: retrieveCart()
CartService->>Database: SELECT cart with items
Database-->>CartService: cart data
StoreAPI->>PromotionService: computeActions([promo_code], context)
Note right of StoreAPI: context includes items, currency, customer
PromotionService->>PromotionService: validate promotion rules
PromotionService->>PromotionService: calculate percentage discount
Note right of PromotionService: For 10% on 2600 DKK = 260 DKKbr/is_tax_inclusive affects calculation
PromotionService-->>StoreAPI: return adjustment actions with is_tax_inclusive flag
StoreAPI->>CartService: prepareAdjustmentsFromPromotionActions()
Note right of StoreAPI: Maps promotion_type to adjustmentbr/Stores is_tax_inclusive on adjustment
CartService->>Database: INSERT line_item_adjustments
Note right of CartService: amount: 260br/is_tax_inclusive: truebr/promotion_type: "percentage"
Database-->>CartService: adjustments created
StoreAPI->>TaxCalculator: calculateTotals(cart)
Note right of StoreAPI: Tax-inclusive calculation:br/subtotal: 2080br/discount_subtotal: 208br/taxable_base: 1872br/tax: 468br/total: 2340
TaxCalculator-->>StoreAPI: calculated totals
StoreAPI-->>Client: 200 OK with cart totals
Note over Client,Database: Order Completion Flow
Client->>StoreAPI: POST /store/carts/{id}/complete
StoreAPI->>CartService: completeCart()
CartService->>Database: SELECT cart with adjustments
Database-->>CartService: cart data with is_tax_inclusive adjustments
CartService->>TaxCalculator: recalculateTotals()
TaxCalculator-->>CartService: final totals
CartService->>Database: INSERT order with adjustments
Note right of CartService: Preserves is_tax_inclusivebr/and promotion_type on order items
Database-->>CartService: order created
CartService-->>StoreAPI: order completed
StoreAPI-->>Client: 200 OK with order
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
| // items: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // quantity: 1, | ||
| // unit_price: 1300, | ||
|
|
||
| // subtotal: 1040, | ||
| // tax_total: 240, | ||
| // total: 1200, | ||
|
|
||
| // original_total: 1300, | ||
| // original_tax_total: 260, | ||
|
|
||
| // discount_total: 100, | ||
| // discount_subtotal: 80, | ||
| // discount_tax_total: 20, | ||
|
|
||
| // adjustments: expect.arrayContaining([ | ||
| // expect.objectContaining({ | ||
| // amount: 100, | ||
| // is_tax_inclusive: true, | ||
| // }), | ||
| // ]), | ||
| // }), | ||
| // ]), |
There was a problem hiding this comment.
The entire items assertion block is commented out (lines 1831: 1854), preventing verification of per-item totals, tax breakdown, and adjustment amounts. The commented values also contain inconsistencies: they specify quantity: 1 and discount_total: 100, but the cart setup has 2 items with an expected per-item discount_total of 130 (260÷2). This missing test coverage leaves the fix for per-item tax-inclusive percentage discounts unverified and susceptible to regressions.
Suggested fix
items: expect.arrayContaining([
expect.objectContaining({
quantity: 1,
unit_price: 1300,
subtotal: 1040,
tax_total: 240,
total: 1200,
original_total: 1300,
original_tax_total: 260,
discount_total: 130,
discount_subtotal: 104,
discount_tax_total: 26,
adjustments: expect.arrayContaining([
expect.objectContaining({
amount: 130,
is_tax_inclusive: true,
}),
]),
}),
]),Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1831-1854
Issue Type: robustness-low
Severity: low
Issue Description:
The entire items assertion block is commented out (lines 1831: 1854), preventing verification of per-item totals, tax breakdown, and adjustment amounts. The commented values also contain inconsistencies: they specify `quantity: 1` and `discount_total: 100`, but the cart setup has 2 items with an expected per-item discount_total of 130 (260÷2). This missing test coverage leaves the fix for per-item tax-inclusive percentage discounts unverified and susceptible to regressions.
Current Code:
// items: expect.arrayContaining([
// expect.objectContaining({
// quantity: 1,
// unit_price: 1300,
// subtotal: 1040,
// tax_total: 240,
// total: 1200,
// original_total: 1300,
// original_tax_total: 260,
// discount_total: 100,
// discount_subtotal: 80,
// discount_tax_total: 20,
// adjustments: expect.arrayContaining([
// expect.objectContaining({
// amount: 100,
// is_tax_inclusive: true,
// }),
// ]),
// }),
// ]),
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| @@ -66,6 +66,10 @@ export interface AddItemAdjustmentAction { | |||
| is_tax_inclusive?: boolean | |||
There was a problem hiding this comment.
The total field is defined as a required non-optional property in ComputeActionItemLine, but it is never read or used anywhere in the promotion compute-actions logic. Since the field is unused and its addition as a required property breaks external callers who construct the interface directly, it should be marked as optional to maintain backwards compatibility.
Suggested fix
/**
* The total of the line item.
*/
total?: BigNumberInputPrompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/types/src/promotion/common/compute-actions.ts
Lines: 66-66
Issue Type: robustness-high
Severity: high
Issue Description:
The `total` field is defined as a required non-optional property in `ComputeActionItemLine`, but it is never read or used anywhere in the promotion compute-actions logic. Since the field is unused and its addition as a required property breaks external callers who construct the interface directly, it should be marked as optional to maintain backwards compatibility.
Current Code:
/**
* The total of the line item.
*/
total: BigNumberInput
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| ) | ||
| ).data.cart | ||
|
|
||
| console.log(JSON.stringify(cart, null, 2)) |
There was a problem hiding this comment.
A debug console.log statement logging the cart object as JSON was left in the integration test. This outputs verbose diagnostic information that pollutes test logs and provides no value to the test suite, and should be removed before merging.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1791-1791
Issue Type: style-low
Severity: low
Issue Description:
A debug console.log statement logging the cart object as JSON was left in the integration test. This outputs verbose diagnostic information that pollutes test logs and provides no value to the test suite, and should be removed before merging.
Current Code:
console.log(JSON.stringify(cart, null, 2))
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| ) | ||
| ).data.cart | ||
|
|
||
| console.log(JSON.stringify(cart, null, 2)) |
There was a problem hiding this comment.
A debug console.log() statement was left in the test code at line 1791, outputting the entire cart object as JSON. This will flood test output with several hundred lines of unnecessary logging on every test run, making logs harder to read and failures harder to diagnose.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: integration-tests/http/__tests__/promotions/admin/promotions.spec.ts
Lines: 1791-1791
Issue Type: style-low
Severity: low
Issue Description:
A debug `console.log()` statement was left in the test code at line 1791, outputting the entire cart object as JSON. This will flood test output with several hundred lines of unnecessary logging on every test run, making logs harder to read and failures harder to diagnose.
Current Code:
console.log(JSON.stringify(cart, null, 2))
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| /** | ||
| * The total of the line item. | ||
| */ | ||
| total: BigNumberInput |
There was a problem hiding this comment.
The total field on the ComputeActionItemLine interface is published but never consumed by any code in the promotion compute-actions pipeline. The lineItemsTotal accumulator and discount calculations use only subtotal, making total unused and creating confusion about whether callers should populate it and whether future discount logic should reference it.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/core/types/src/promotion/common/compute-actions.ts
Lines: 193-196
Issue Type: maintainability-medium
Severity: medium
Issue Description:
The `total` field on the `ComputeActionItemLine` interface is published but never consumed by any code in the promotion compute-actions pipeline. The `lineItemsTotal` accumulator and discount calculations use only `subtotal`, making `total` unused and creating confusion about whether callers should populate it and whether future discount logic should reference it.
Current Code:
/**
* The total of the line item.
*/
total: BigNumberInput
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
The buy-get computed action is missing the is_tax_inclusive field when creating an ADD_ITEM_ADJUSTMENT action. This causes the field to default to false in the database, preventing the tax-correction logic in calculateAdjustmentTotal from executing the percentage-specific path that requires both adj.is_tax_inclusive === true and adj.promotion_type === 'percentage'. As a result, tax-inclusive percentage buy-get promotions are double-divided by (1 + taxRate), producing incorrect discount amounts and wrong cart totals. The fix is incomplete compared to line-items.ts, which correctly sets both is_tax_inclusive and promotion_type.
Suggested fix
computedActions.push({
action: ComputedActions.ADD_ITEM_ADJUSTMENT,
item_id: item.id,
amount,
code: promotion.code!,
promotion_type: promotion.application_method?.type!,
is_tax_inclusive: promotion.is_tax_inclusive,
})Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/modules/promotion/src/utils/compute-actions/buy-get.ts
Lines: 270-276
Issue Type: robustness-high
Severity: high
Issue Description:
The buy-get computed action is missing the `is_tax_inclusive` field when creating an ADD_ITEM_ADJUSTMENT action. This causes the field to default to `false` in the database, preventing the tax-correction logic in `calculateAdjustmentTotal` from executing the percentage-specific path that requires both `adj.is_tax_inclusive === true` and `adj.promotion_type === 'percentage'`. As a result, tax-inclusive percentage buy-get promotions are double-divided by `(1 + taxRate)`, producing incorrect discount amounts and wrong cart totals. The fix is incomplete compared to `line-items.ts`, which correctly sets both `is_tax_inclusive` and `promotion_type`.
Current Code:
computedActions.push({
action: ComputedActions.ADD_ITEM_ADJUSTMENT,
item_id: item.id,
amount,
code: promotion.code!,
promotion_type: promotion.application_method?.type!,
})
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 32.9sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 1 packagesMPL-2.0 (1 packages):
Unknown Licenses - 56 packages
...and 36 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/medusa · PR #4Scanned: 2026-06-08 19:51 UTC | Score: 41/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
integration-tests/http/__tests__/promotions/admin/promotions.spec.ts |
0 | 2 | 2 | 23 | 27 |
packages/core/types/src/cart/common.ts |
0 | 0 | 0 | 1 | 1 |
packages/core/utils/src/totals/adjustment/index.ts |
0 | 0 | 1 | 0 | 1 |
packages/modules/cart/src/migrations/Migration20250708075529.ts |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
3 participants
User description
Summary
What — What changes are introduced in this PR?
Please provide answer here
Why — Why are these changes relevant or necessary?
Please provide answer here
How — How have these changes been implemented?
Please provide answer here
Testing — How have these changes been tested, or how can the reviewer test the feature?
Please provide answer here
Examples
Provide examples or code snippets that demonstrate how this feature works, or how it can be used in practice.
This helps with documentation and ensures maintainers can quickly understand and verify the change.
// Example usageChecklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
Add any additional context, related issues, or references that might help the reviewer understand this PR.
CodeAnt-AI Description
Handle tax-inclusive percentage promotions correctly and show admin-user setup progress
What Changed
Impact
✅ Correct totals for tax-inclusive percentage discounts✅ Fewer pricing mismatches at checkout✅ Clearer app setup progress💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.