Skip to content

Honour a deliberate 0% discount on Amount Type = Discount price list lines - #11070

Draft
attilatoury wants to merge 1 commit into
mainfrom
private/attilatoury/fix-649151-variant-zero-discount
Draft

Honour a deliberate 0% discount on Amount Type = Discount price list lines#11070
attilatoury wants to merge 1 commit into
mainfrom
private/attilatoury/fix-649151-variant-zero-discount

Conversation

@attilatoury

@attilatoury attilatoury commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

What & why

A variant- or currency-specific price list line with Line Discount % = 0 could never win over a less specific line, so there was no way to configure "no discount for this variant".

In PickBestLine, the specificity reset was gated on (AmountType <> AmountType::Discount) or (PriceListLine."Line Discount %" > 0). During discount calculation AmountType is always ::Discount (passed as a literal from CalcBestAmount via ApplyDiscount), so the effective gate was just "Line Discount %" > 0 — a 0% line could never displace the generic one.

The gate now asks whether the line declares a discount rather than whether the value is positive: an explicit Amount Type = Discount line means the 0 was deliberate, while on an Amount Type = Any line a 0 is indistinguishable from an unset field and is still ignored. That keeps #582311 working (an Any line carrying a price with an untouched 0% discount must not wipe out a real discount).

A second change lets a declaring line take over a cleared best line, so selection is order-independent. Without it the fix only worked when the generic line happened to be iterated first.

Blast radius is provably narrow. DiscountIsDeclared differs from the old > 0 test only when Amount Type = Discount and the discount is 0, and the new take-over clause can only fire when IsBetterLine is false and the best line isn't real — which for discounts also means 0. For AmountType = Price the first clause of the gate is always true and the new clause always false, so the price path is unchanged.

Old pricing (V15) already behaves correctly here — PurchPriceCalcMgt.CalcBestLineDisc gives a specific line an unconditional win — so this also closes a V15 → V16 parity gap.

Linked work

Fixes #

AB#649151

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

  • Built Base Application locally with alc against a v30 dev instance: 0 errors, 635 warnings — all pre-existing AL0432/AL0684, none referencing PriceCalculationV16.
  • Built Tests-ERM with the new tests: 0 errors, 1 pre-existing warning in an unrelated file.
  • Published the rebuilt Base Application to a local BC instance (30.0.54275.1), 257/257 apps installed cleanly.
  • Manual verification in the client is still outstanding — this is why the PR is a draft. The scenario to walk through: purchase price list with a blank-variant Discount line at 30% and a variant-specific Discount line at 0%; a purchase line with that variant should show 0%, and switching the variant line to Price & Discount should return it to 30%.
  • The new automated tests have not been executed yet, only compiled.

Four tests added to TestPriceCalculationV16.Codeunit.al:

Test Covers
PurchVariantZeroPctDiscountLineWinsOverGenericDiscount Purchase, generic line stored first
PurchVariantZeroPctDiscountLineWinsWhenAddedBeforeGenericDiscount Purchase, variant line stored first — guards the order-dependence bug
SalesVariantZeroPctDiscountLineWinsOverGenericDiscount Sales side
PurchCurrencyZeroPctDiscountLineWinsOverGenericDiscount Currency Code specificity, which shares the code path

The three existing tests that pin the current behaviour were traced by hand against both iteration orders and remain satisfied, because all three use "Price Amount Type"::Any for their spurious 0% line:
DiscountPreservedWhenSpuriousZeroPctVariantLineExists, VariantSpecificDiscountOverridesHigherGenericDiscount, VariantDiscountSelectedWhenSpuriousZeroPctAndRealVariantDiscountExist.

Risk & compatibility

  • Behaviour change on upgrade. A tenant that already has an Amount Type = Discount line sitting at 0% will see it start taking effect; today such lines are inert. This is not measurable from telemetry. Mitigating factor: the V15 → V16 data conversion in CopyFromToPriceListLine writes migrated discount records as Amount Type = Discount with the variant preserved, so most affected lines are migrated configurations this change restores rather than breaks.
  • Does not auto-fix the reporting customer. Their lines are Amount Type = Price & Discount; they must change the variant line to Discount. Support needs to communicate this.
  • The same gap exists for Currency Code and is fixed by the same code path.
  • No schema, API or permission changes. Price calculation path untouched.
  • Written with agent assistance; the diff has been reviewed line by line and built locally, but not yet exercised in the client.

…lines

A variant- or currency-specific line with Line Discount % = 0 could never displace a less specific line, because the specificity reset was gated on the value being > 0. Gate it on the line declaring a discount instead, and let a declaring line take over a cleared best line so selection is order-independent. Fixes AB#649151
@github-actions github-actions Bot added the Team: SCM GitHub request for SCM area label Sep 4, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant