[Bug][Subscription Billing] Contract Renewal Quote total ignores the Billing Base Period and shows the base-period amount instead of the renewal-term amount

[Miljan Milosavljević (miljance)]

What & why

The subscription totals block on a printed Contract Renewal Quote showed the amount per Billing Base Period instead of the amount for the renewal term the customer is actually being asked to sign. With Billing Base Period 1M, Billing Rhythm 12M and a Renewal Term of 12M, a Subscription Line priced at 100 per month printed a total of 100 instead of 1,200.

"Sales Subscription Line".CalcVATAmountLines delegates to the local procedure CreateTempSalesServiceCommitmentBuffForSalesServiceCommitment, which converted Amount from the Billing Base Period to the Billing Rhythm only in the regular branch. The contract-renewal branch multiplied by CalculateRenewalTermRatioByBillingRhythm alone — that ratio expresses how many Billing Rhythms fit into the renewal term, not how the base period relates to the rhythm. When the two happen to coincide the ratio is exactly 1, which is why the defect stayed invisible in the common 1M/1M case where both factors are 1.

This change moves the two FindDateFormulaTypeForComparison lookups above the branch so both paths share one conversion, and applies / BasePeriodCount * RhythmPeriodCount in the contract-renewal branch as well:

if SalesLineVAT.IsContractRenewal() then
    TempSalesServiceCommitmentBuff."Line Amount" += SalesServiceCommitment.Amount / BasePeriodCount * RhythmPeriodCount * ContractRenewalPriceCalculationRatio
else
    TempSalesServiceCommitmentBuff."Line Amount" += SalesServiceCommitment.Amount / BasePeriodCount * RhythmPeriodCount;

It also turns BasePeriodCount and RhythmPeriodCount from var parameters threaded in from CalcVATAmountLines into locals of the procedure, re-initialised to 1 per line. As var parameters they were initialised once per document, and FindDateFormulaType leaves PeriodCount untouched for empty, complex and current-period formulas — so one line's period count leaked into the next.

Linked work

Fixes #10356

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 (required — be specific: scenarios, commands, screenshots for UI changes)

Container sb10356. The Subscription Billing App, Test and Demo Data apps were built with alc.exe and published to that container, where the tests below were executed and the renewal quote total was also checked manually.

Build — clean, with CodeCop, UICop and PerTenantExtensionCop plus base.ruleset.json. No analyzer warnings from the changed files.

TDD (red first) — with the fix reverted, the new test CheckVatCalculationForContractRenewalWithBillingBasePeriodDifferentFromBillingRhythm failed with:

Expected:<1200> Actual:<100>

which is exactly the number reported in the issue.

Green — codeunit 139692 "Contract Renewal Test", run with al_run_tests against sb10356. The four relevant tests: 5 passed, 0 failed.

• CheckVatCalculationForContractRenewalWithBillingBasePeriodDifferentFromBillingRhythm (new) — the reported 1M base period / 12M rhythm / 12M renewal term scenario.
• CheckVatCalculationForContractRenewalWithoutBillingBasePeriod (new) — empty Billing Base Period falls back to a count of 1, the same way the regular branch does.
• CheckVatCalculationForContractRenewalWithComplexBillingRhythm (new) — a <1M+1D> rhythm has no period count, so the amount is taken as it is rather than multiplied by zero.
• CheckVatCalculationForContractRenewalServiceCommitmentRhythmInReports (existing) — its oracle in the helper TestContractRenewalPeriodCalculation was asserting the defect, because the base data uses Billing Base Period 1Y with Billing Rhythm 1M. The expectation now states the conversion factor as a literal guarded by TestField("Billing Base Period", ...) / TestField("Billing Rhythm", ...) instead of deriving it through Date Formula Management, so the assertion can no longer follow the implementation into a wrong conversion.

Codeunit 139915 "Sales Service Commitment Test" — covers the non-renewal branch of the same procedure, which this change also touches: 66 passed, 0 failed.

Mutation testing — executed, not reasoned: for each mutant the source was edited, rebuilt, republished and the tests re-run, then reverted. 7 mutants, all killed.

#	Mutant	Killed by	Observed
1	Drop * RhythmPeriodCount	new ...WithBillingBasePeriodDifferentFromBillingRhythm	100 instead of 1200
2	Drop / BasePeriodCount	...ServiceCommitmentRhythmInReports (multi-term)	181.28 instead of 15.11
3	Drop * ContractRenewalPriceCalculationRatio	...ServiceCommitmentRhythmInReports (multi-term)	46.83
4	Swap the base and rhythm lookups	all three new tests	8.33
5	Drop BasePeriodCount := 1	...WithoutBillingBasePeriod	divide by zero
6	Drop RhythmPeriodCount := 1	...WithComplexBillingRhythm	total silently 0
7	Remove DateFormulaType::Year: PeriodCountForComparison * 12 from DateFormulaManagement.FindDateFormulaTypeForComparison	corrected oracle in TestContractRenewalPeriodCalculation	Expected:<15.11> Actual:<181.28>

Mutants 5 and 6 survived the first round; ...WithoutBillingBasePeriod and ...WithComplexBillingRhythm were written specifically to kill them. Mutant 7 reproduces the original defect through the date-formula helper — the old oracle stayed green on it, which is what demonstrates that the oracle correction has teeth.

This change adds no new Label, Caption or ToolTip, so it introduces no translatable strings and there is nothing to add to the XLIFF files.

Risk & compatibility

• Behavioural change, by design. Contract Renewal Quotes whose Subscription Lines have a Billing Base Period that differs from the Billing Rhythm now show a different (correct) subscription total. Quotes where base period and rhythm match are unaffected — both factors are 1 there.
• No stored data changes. The totals are computed into a temporary "Sales Service Commitment Buff." when the document is printed or statistics are opened, so existing open Contract Renewal Quotes will display the corrected total on their next print without any upgrade step. No table schema change, no permission, telemetry or upgrade impact.
• The regular (non-renewal) branch is unchanged in arithmetic; only the two FindDateFormulaTypeForComparison lookups moved above the branch and the two period counts became locals. The fixed leak of BasePeriodCount / RhythmPeriodCount between lines can also change a regular multi-line quote whose lines use different rhythms — previously the second line could inherit the first line's counts when its own formula is empty, complex or current-period.
• The signature change is on the local procedure CreateTempSalesServiceCommitmentBuffForSalesServiceCommitment; the public CalcVATAmountLines signature is untouched, so nothing external can be broken by it.
• Deliberately out of scope: the issue's secondary observation that the Subscriptions* detail rows print SalesSubscriptionLine.Price into "Unit Price" with no period indication. Fixing that means adding a column to three report layout .docx files, and the issue's "Expected behavior" section covers only the totals block. Left for a follow-up.

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Accept

What this PR does

This PR fixes the Contract Renewal Quote subscription totals when the Billing Base Period and Billing Rhythm differ. The linked issue describes a clear bug: a line priced at 100 per month, billed yearly, printed 100 instead of the 1,200 renewal-term total.

The diff targets the right place: CalcVATAmountLines builds the temporary totals used by the report, and the contract-renewal branch now applies the same base-period-to-rhythm conversion as the regular branch before applying the renewal-term ratio. Moving BasePeriodCount and RhythmPeriodCount into the helper also fixes the prior cross-line leak when a later line has an empty or complex formula. I did not find a BaseApp event dependency in this PR.

Suggestions

None.

Risk assessment and necessity

Risk: This affects computed totals for Subscription Billing sales documents, including VAT amount-line buffers used by reports/statistics. The change is narrow and keeps the existing non-renewal arithmetic, but it can change totals for renewal quotes and for regular multi-line quotes that previously reused period counts from an earlier line. The added tests cover the reported 1M/12M renewal case, empty base period fallback, complex rhythm fallback, and the existing non-renewal branch.

Necessity: The issue evidence is strong because the printed quote can show the customer the wrong renewal amount. The fix is in the calculation path that produces that total, has no schema or public signature change, and is scoped to the documented defect.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10445 round=1 by=alexei-dobriansky at=2026-08-20T13:13:25.3484895Z lastSha=f8596754841312d6facda1b7982539a47d406d4e reviewKey=f145a89e9f3e9e39445cccdf3de9e1c34338ef2dfc3130c4b9e8a55400c5a11c suggestions=