[Bug][SubscriptionBilling] Enforce Subscription Line Start Date change rules on all edit paths

[Miljan Milosavljević (miljance)]

What & why

The Subscription Line start date could still be changed after the line had been billed. The next billing date silently followed the new start date, so an already invoiced period could be invoiced again, or a period could be skipped and never invoiced at all — usually noticed only after the invoices had gone out.

Two separate gaps caused this:

1. The guard only covered two pages. It lived in SubscriptionLine.UpdateServiceCommitment, which is called from the customer and vendor contract line subpages only. The same edit on the Subscription Lines page — or from code, the import, or the API — bypassed it entirely.
2. Where it did run, it asked the wrong question. ErrorIfBillingLineArchiveForServiceCommitmentExist summed the archived billing amounts instead of asking whether billing had happened. A line billed at zero value (free or 100% discounted), or one whose invoices and credit memos netted to zero, was treated as never billed.

The check now lives in the OnValidate trigger of "Subscription Line Start Date", ahead of the next-billing-date recalculation, so every edit path is covered by construction. It evaluates the two conditions from the issue: the change is allowed when no Billing Line and no Billing Line Archive exist for the "Entry No.", or when "Next Billing Date" is still on "Subscription Line Start Date" — the state a cancellation or credit memo leaves behind, which is exactly the correction case that has to stay open.

Two implementation points a reviewer should know:

• The allowance is evaluated against the persisted record, not Rec. Inside OnValidate the field already carries the new value, and xRec is not dependable on the contract line subpages either, because the page assigns the source expression before UpdateServiceCommitment re-validates it. Reading the stored row is the only reliable source for "where was the next billing date before this edit".
• Temporary records are exempt. PostSubContractRenewal.InsertPlannedServiceCommitmentFromSalesServiceCommitment buffers an existing, already billed Subscription Line into a temporary record that carries its real "Entry No.", then validates the start date on it. Without the exemption, renewing any billed subscription would fail.

OnAfterCheckSubscriptionLineStartDateChangeAllowed passes the record by value, so a localization can tighten the rule by raising an error of its own. The guards themselves always run — an IsHandled bypass around an integrity check would let a subscriber switch off the very rule this change adds.

The existence check on Billing Line Archive now runs on every start-date validation rather than only from the two subpages, so the PR adds the missing key(SK2; "Subscription Line Entry No."). The same filter on Billing Line already rides SK7; the archive was the odd one out.

Linked work

Fixes #9976

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 with alc.exe (CodeCop + UICop + firstparty.ruleset.json) and published to a BC29 container over the dev endpoint. Both scenarios from the issue reproduce as bugs before the change and are blocked after it.

TDD, red first. The three "must be rejected" tests were run against the unchanged app and failed for the right reasons — for scenario A, PreventStartDateChangeOnSubscriptionLinesPageAfterBilling reported "An error was expected inside an ASSERTERROR statement", i.e. the page accepted the edit silently, which is the bug. All six pass after the change.

New tests in ServiceCommitmentTest.Codeunit.al (codeunit 148156):

Test	Covers
PreventStartDateChangeOnSubscriptionLinesPageAfterBilling	Scenario A end to end — bill and post a contract invoice, then edit the date on the real Subscription Lines page via TestPage
UT_PreventStartDateChangeWhenBilledSubscriptionLineHasZeroAmount	Scenario B — archived billing lines summing to zero
UT_PreventStartDateChangeWhenSubscriptionLineIsInCurrentBilling	Open billing proposal still blocks the change
UT_AllowStartDateChangeWhenNextBillingDateIsOnStartDate	The post-credit-memo correction stays open, and the next billing date follows
UT_AllowStartDateChangeWhenSubscriptionLineHasNotBeenBilled	Never-billed lines remain editable
UT_AllowStartDateChangeOnTemporarySubscriptionLine	The contract renewal buffer path is not blocked

Mutation testing, executed rather than reasoned. Each mutant was compiled, published and re-run: removing either existence check, comparing the in-memory start date instead of the persisted one, restoring CalcSums(Amount) <> 0, and dropping the IsTemporary() exemption. All are killed. The temporary-record mutant survived the first pass, which is why UT_AllowStartDateChangeOnTemporarySubscriptionLine exists — it was added specifically to kill it.

Regression. Roughly 600 tests across the Subscription Billing suite pass, including the pre-existing guard tests ExpectErrorOnModifyServiceStartDateWhenBillingLineArchiveExist (customer and vendor), UT_ExpectErrorOnModifyServiceStartDateWhenBillingLineExist, and both TestChangeServiceStartDateAfterCorrection* tests that pin the correction case. Re-run against the final build after the added key: 148156 (36), 139686 (13), 139687 (88), 139688 (46), 148153 (74), 148154 (33), 148155 (69), 139916 (5).

Five failures in Service Object Test and Contract Renewal Test are pre-existing: republishing the unchanged app reproduces all five identically. One is a missing localization table (12170) in the container.

All 14 production Validate("Subscription Line Start Date", …) call sites were traced. Every creation path zeroes "Entry No." before validating, so the guard exits; the import validates after Insert(true) on a row where start date and next billing date are both 0D, so it takes the allowance; renewal uses a temporary record. No path regresses.

Risk & compatibility

• Behavioural change, intended: editing the start date of a billed Subscription Line now fails everywhere, not just on the two contract line subpages. Anything that relied on the Subscription Lines page, the API, or a data import to move the start date of a billed line will now get an error. That is the point of the issue, but it is the change most likely to surface in existing automation.
• A line billed at zero value is no longer exempt. With the amount test replaced by an existence check, a fully credited line — including a usage-based rebilling line, whose next billing date is repointed at Original Invoiced to Date rather than the start date — stays blocked. This is the documented intent; OnAfterCheckSubscriptionLineStartDateChangeAllowed is not a way back in, since it can only tighten. A localization needing the opposite would have to be discussed separately.
• Schema: one new secondary key on Billing Line Archive. Index addition only — no field, no data change, no upgrade code.
• New integration event OnAfterCheckSubscriptionLineStartDateChangeAllowed. Its by-value parameter is deliberate and is a compatibility contract from here on: passing the record by value is what keeps subscribers from loosening the rule, and switching it to var later would be a breaking change.

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Accept

What this PR does

This PR moves the Subscription Line start-date guard into the table field validation, so all edit paths use the same rule. It also changes the billed check from archived amount totals to record existence, so zero-value billing is still treated as billing.

The fix matches the issue. The persisted Subscription Line is used to decide whether the old Next Billing Date was still on the old start date, which avoids using the new in-memory value too early. The checks run before the next billing date is recalculated, and the temporary-record escape keeps the contract renewal buffer path working. The added tests cover the missing page path, zero-value archive case, open billing line case, correction allowance, never-billed allowance, and temporary record path.

Suggestions

No suggestions.

Risk assessment and necessity

Risk: This changes validation for a financial data-integrity path. Existing imports, API calls, or page edits that moved the start date after billing will now get an error. That is intended. The new event is additive, the new archive key is index-only, and no public procedure signature is changed.

Necessity: The change is needed because the old behavior could skip a billing period or bill the same period twice. The scope is right: it fixes the table-level rule, removes the duplicated page-only guard, and keeps the documented correction case open.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10235 round=1 by=alexei-dobriansky at=2026-08-14T10:08:58.5672528Z lastSha=a0998b3885768abd2c8d3af42fd10a90c670f4cd reviewKey=na suggestions=none