Service Line CalcVATAmountLines: count VAT Difference once and clear filters before OnAfterCalcVATAmountLines - #11026
Open
Franco111000 wants to merge 1 commit into
Conversation
…tern CalcVATAmountLines on table 5902 Service Line added the per-line VAT Difference to TotalVATAmount even though Amount Including VAT minus Amount already contains it, so the duplicated amount was settled on the invoice rounding VAT line. Sales Line and Purchase Line accumulate Amount Including VAT minus Amount only; Service Line now does the same. It also left the FindVATAmountLine filters on the var VATAmountLine when OnAfterCalcVATAmountLines fired, which made the Service Statistics VAT specification show a single line. The record is now reset after the rounding block, before the event, on every path. The NA layer copy of the table carries the same code and receives the same changes. Two tests cover each defect in codeunit 134028 and are mirrored into its APAC, ES and IT copies. Fixes microsoft#9102. Re-submission of microsoft/BusinessCentralApps#1913.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
CalcVATAmountLineson table 5902 "Service Line" diverges from the Sales Line and Purchase Line implementation in two places, and both show up on Service Statistics as soon as invoice rounding and a manual VAT difference meet:"Amount Including VAT" - Amount + "VAT Difference"per line."Amount Including VAT" - Amountalready contains the VAT difference, so it was counted twice and the excess was settled on the invoice rounding VAT line, which then showed a VAT amount equal to the VAT difference. Sales Line and Purchase Line accumulate"Amount Including VAT" - Amountonly.FindVATAmountLinestayed on thevar VATAmountLinewhenOnAfterCalcVATAmountLinesfired and control returned to the caller. Page 6030 "Service Statistics" then ranTempVATAmountLine.ModifyAll(Modified, false)on one row and handed a filtered set to the VAT Specification subform, so only one VAT line was visible.The fix drops the extra term, which makes the accumulation identical to Sales Line and Purchase Line, and adds
VATAmountLine.Reset()after the rounding block, before the event, so the record is unfiltered on every path. For the record, Sales Line is not fully clean here either:GetVATAmountLineOfMaxAmtresets its filters only on its fall-through branch and leaves them set on the earlyexit. That narrower gap is out of scope for this change. Both layers that carry the procedure are changed:src/Layers/W1and the full table copy insrc/Layers/NA. The two edited regions are byte-identical between the layers before and after the change.Linked work
Fixes #9102
Re-submission of microsoft/BusinessCentralApps#1913 (pilot issues #1562 and #1563), which received a code-owner approval and was closed unmerged when that repository was retired, with a note about internal test failures. Two things differ from that version and are the likely cause of those failures:
"VAT Identifier"on a new VAT Posting Setup directly, which fails in the IT localization, where that field has a table relation to the VAT Identifier table. The tests here go through the codeunit's existingCreateVATPostingSetuphelper, which the IT copy already overrides for exactly that reason.How I validated this
What I tested and the outcome
Two tests were added to codeunit 134028 "ERM Change VAT On VAT Amt Line", and mirrored into its full-replacement copies in the APAC, ES and IT test layers (those copies differ from W1 only in their pre-existing localized lines, verified by diffing each copy against W1 before and after the change).
ServiceCalcVATAmountLinesCountsVATDifferenceOnce: a Service Invoice with an item line (10% VAT) and an invoice rounding G/L line (0% VAT, own VAT Identifier). The VAT amount of the item line is raised by a VAT difference the way the VAT Specification subform and Service Statistics do it (Validate("VAT Amount"),Modified := true,UpdateVATOnLinesfor General and Invoicing), thenCalcVATAmountLinesruns again. It asserts that the total VAT on the VAT Amount Lines equals the VAT on the Service Lines, and that the rounding VAT line stays at 0. Against the unpatched table the residual afterUpdateLinesis exactly the VAT difference, the rounding block adds it to the rounding VAT line, and both assertions fail.ServiceCalcVATAmountLinesReturnsAllLinesAfterRoundingResidual: same document, no VAT difference. A rounding residual is injected throughOnCalcVATAmountLinesOnBeforeUpdateVATAmountLinefrom a manually bound instance of the test codeunit, so the rounding block runs without depending on the rounding arithmetic of a particular localization. It asserts that both VAT Amount Lines are visible on the record handed back to the caller, and that the residual was settled on a VAT Amount Line. With the first fix alone the block still leaves theFindVATAmountLinefilters in place andCount()returns 1, so this test isolates theReset().Neither test has executed yet:
.alpackagesis empty in this repository, so there is no local build, and the pull request build runs only after a maintainer approves the workflow. The pass and fail behaviour above comes from tracing the code paths by hand againstupstream/main, not from a test run.Risk & compatibility
OnCalcVATAmountLinesOnAfterCalcLineTotalson Service Line:TotalVATAmountno longer carries the duplicated VAT difference. That value was wrong; it now matches what the Sales and Purchase publishers pass.OnAfterCalcVATAmountLineson Service Line:VATAmountLinenow arrives unfiltered on every path. Nothing in this repository subscribes to that event.GetVATAmountLineOfMaxAmt.