Skip to content

Bug][SubscriptionBilling]: "Prices Including VAT" option does not correctly update Subscription Line amounts#8066

Open
miljance wants to merge 2 commits into
microsoft:mainfrom
miljance:SBPricesIncludingVAT
Open

Bug][SubscriptionBilling]: "Prices Including VAT" option does not correctly update Subscription Line amounts#8066
miljance wants to merge 2 commits into
microsoft:mainfrom
miljance:SBPricesIncludingVAT

Conversation

@miljance
Copy link
Copy Markdown
Contributor

@miljance miljance commented May 8, 2026

What & why

  • Add EnsureCalculationBaseAmountExcludesVAT on Subscription Line table to strip VAT from Calculation Base Amount when the Sales Header has Prices Including VAT
  • Add event subscriber in SalesSubscriptionLineMgmt to recalculate Sales Subscription Line amounts when Prices Including VAT is toggled on the Sales Header
  • Add four tests covering: CBA equals unit price with/without PIV, recalculation on PIV toggle, and net CBA on posted Subscription Line
  • Refactor test helpers: extract FindNonZeroVATPostingSetup, FindCustomerDocumentPrice- SalesServiceCommitment, SetupItemCustomerAndSalesHeaderWithVAT; replace magic VAT sentinel with explicit ReassignSalesLineToDifferentVATGroup/ReassignSalesLineToZeroVATGroup
  • Add GIVEN/WHEN/THEN comments to CheckVatCalculationForServiceCommitmentRhythmInReports

Linked work

Fixes #7399

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
Followed the repro steps from #7399:

  1. Created a new Sales Order for a subscription item with a non-zero VAT rate. Enabled Prices Including VAT on the header - confirmed that the Calculation Base Amount on all Sales Subscription Lines was updated to the VAT-exclusive (net) amount.
  2. Toggled Prices Including VAT off again - confirmed that Calculation Base Amount reverted to the original VAT-inclusive unit price (i.e. the sales line unit price as stored).
  3. Manually edited Calculation Base Amount on a Sales Subscription Line, then toggled Prices Including VAT - confirmed that the manually entered value was preserved but correctly adjusted (VAT stripped or added), not reset to the unit price.
  4. Posted the Sales Order - confirmed that the resulting Subscription Line Calculation Base Amount is the net (VAT-exclusive) amount, matching the formula UnitPrice / (1 + VAT% / 100).

Risk & compatibility

None. The change adds a new procedure called only from a new event subscriber and from CreateSubscriptionLineFromSalesLine. It does not touch any existing posting logic, table schema, permissions, or upgrade code. No breaking changes.

Fixes AB#634780

…AT is enabled

- Add EnsureCalculationBaseAmountExcludesVAT on Subscription Line table to strip
  VAT from Calculation Base Amount when the Sales Header has Prices Including VAT
- Add event subscriber in SalesSubscriptionLineMgmt to recalculate Sales Subscription
  Line amounts when Prices Including VAT is toggled on the Sales Header
- Add four tests covering: CBA equals unit price with/without PIV, recalculation on
  PIV toggle, and net CBA on posted Subscription Line
- Refactor test helpers: extract FindNonZeroVATPostingSetup, FindCustomerDocumentPrice-
  SalesServiceCommitment, SetupItemCustomerAndSalesHeaderWithVAT; replace magic VAT
  sentinel with explicit ReassignSalesLineToDifferentVATGroup/ReassignSalesLineToZeroVATGroup
- Add GIVEN/WHEN/THEN comments to CheckVatCalculationForServiceCommitmentRhythmInReports
@miljance miljance requested a review from a team as a code owner May 8, 2026 12:33
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork labels May 8, 2026
@JesperSchulz JesperSchulz added the Finance GitHub request for Finance area label May 11, 2026
@djukicmilica djukicmilica added the Linked Issue is linked to a Azure Boards work item label May 11, 2026
@djukicmilica djukicmilica enabled auto-merge (squash) May 11, 2026 19:39
@github-actions github-actions Bot added this to the Version 29.0 milestone May 11, 2026
auto-merge was automatically disabled May 12, 2026 09:41

Pull request was closed

@djukicmilica djukicmilica reopened this May 12, 2026
@djukicmilica djukicmilica enabled auto-merge (squash) May 12, 2026 09:42
auto-merge was automatically disabled May 13, 2026 07:31

Pull request was closed

@djukicmilica djukicmilica reopened this May 13, 2026
@djukicmilica djukicmilica enabled auto-merge (squash) May 13, 2026 07:31
@github-actions
Copy link
Copy Markdown

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

SalesSubscriptionLine.Modify called without transaction guard

In the new RecalculateSalesSubscriptionLineAmountsOnBeforeSalesLineModify event subscriber, SalesSubscriptionLine.Modify(false) is called inside a loop while processing a OnValidatePricesIncludingVATOnBeforeSalesLineModify event. If the calling transaction fails or rolls back after partial execution (e.g. on the second line), some subscription lines will have been recalculated and others will not, leaving the set in an inconsistent state.

Recommendation:

  • Ensure that the loop is written to be idempotent, or verify that the calling framework guarantees transactional rollback across all record modifications in this event. Consider using a collect-then-apply pattern rather than modifying inside an event fired mid-validation.
// Collect all changes first, then apply in a single pass after validation completes
// or rely on the caller's explicit Modify after the event returns

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Finance GitHub request for Finance area From Fork Pull request is coming from a fork Linked Issue is linked to a Azure Boards work item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][SubscriptionBilling]: "Prices Including VAT" option does not correctly update Subscription Line amounts

4 participants