Subscription Billing: respect Default Dimension Priorities on Subscription Lines

[Franco111000]

What/why

Fixes AB#635866
Fixes #8084.

When a Customer Subscription Contract spawns Sales Invoices, Default Dimension
Priorities (table 354) were not respected. With priorities set as Item = 1,
Customer = 2 the resulting Sales Line still carried the Customer's dimension
value instead of the Item's.

Per the discussion with @miljance on #8084, the simple swap of GetCombinedDimensionSetID
parameter order at the merge step would fix the reported case but would also let
Sales Line defaults beat genuine Subscription Line user overrides, which is a
regression against the original design intent ("Subscription Line dimensions
should overwrite any duplicates from the default dimensions"). This PR addresses
the root cause upstream so both behaviors hold: priorities are respected during
Subscription Line creation, and Subscription Line dimensions remain authoritative
at billing time.

The bug had two compounding causes inside the Subscription Billing module:

1. SubscriptionLine.SetDefaultDimensions called DimMgt.GetDefaultDimID(..., '', ...) with an empty Source Code, so Default Dimension Priority was never read. Only Item / G/L Account were added as dim sources, never Customer / Vendor.
2. GetCombinedDimensionSetID is a blind last-slot-wins merge. UpdateFromCustomerContract then merged the Customer Contract dim set into the Subscription Line as slot 2, so Customer values systematically overrode whatever was on the Subscription Line.

The fix makes SubscriptionLine.SetDefaultDimensions the single priority-aware
entry point for all dim computation:

• Passes SourceCodeSetup.Sales (Customer side) or SourceCodeSetup.Purchases (Vendor side) so Default Dimension Priority is consulted
• Adds Customer / Vendor as a dim source from the linked Customer / Vendor Subscription Contract
• Uses GetRecDefaultDimID with the contract's Dimension Set ID passed as InheritFromDimSetID and Database::Customer / Database::Vendor as InheritFromTableNo, mirroring the standard SalesLine.CreateDim pattern in Base App

UpdateFromCustomerContract and UpdateFromVendorContract now route through
SetDefaultDimensions instead of blind-merging the contract dim set. Same
change at the Pathway B call sites where an existing Subscription Line is
assigned to a contract (CustomerSubscriptionContract.CreateCustomerContractLineFromServiceCommitment,
VendorSubscriptionContract.CreateVendorContractLineFromServiceCommitment, and
the Import paths).

The blind merge at billing time (CreateBillingDocuments.Codeunit.al lines 264
/ 383) is intentionally left unchanged. After this fix the Subscription Line's
Dimension Set ID is already priority-resolved, so the existing "Service
Commitment wins over Sales Line" merge produces the correct result while
preserving the design intent that subscription line dimensions stay
authoritative for billing, which lines up with @miljance's clarification: "the
contract dimensions should win over any conflicting ones from the Subscription
Lines but still respecting the configured priority order."

Linked work

• Fixes [Bug][SubscriptionBilling]: Default Dimension Priorities not respected when creating Sales Invoice from Customer Subscription Contract #8084

How I validated

• Added a focused test RespectDefaultDimensionPrioritiesOnCustomerContractCreation in Service Comm. Dimensions. Configures Default Dim Priority Item = 1, Customer = 2 for source code Sales, gives Customer and Item conflicting values on the same dimension, asserts the resulting Subscription Line carries Item's value.
• CI pipeline tests (will react to results after submission).

Risk & compatibility

Existing tests in ServiceCommDimensions.Codeunit.al may need adjustments if
they rely on the previous "Customer Contract dim always wins" merge order in
scenarios with overlapping dim codes. Will iterate based on CI results.

The two cross-link call sites that propagate Customer Contract dim onto
vendor-paired Subscription Lines (CustSubContractLine.DeleteRelatedVendorServiceCommDimensions,
VendSubContractLine.UpdateServiceCommitmentDimensions) are intentionally not
touched in this PR. They involve a related but distinct concern about
cross-partner dim propagation. Happy to follow up once the direction here is
settled.

Opening as draft so @miljance and the consultant can weigh in before we finalize.

Fixes AB#635866

[miljance]

Looks clean and good. I think @Franco111000 is more than capable of finishing this one. From my side, this one can be converted to real PR. I would just like to run all the tests to see if any of those needs adjustment.

[Franco111000]

Thanks @miljance! Converting to ready for review. Looking forward to the test results, I'll iterate on anything that needs adjustment.

[miljance]

Hi @Franco111000,

Two tests are currently failing:

ExpectEqualItemAndServiceCommitmentDimensionSetIDOnDeleteCustomerContractLine
I looked into the background for this test and the intent seems to be that when deleting a Contract Line, the dimensions for Service Commitment should exclude any dimensions that came in with the Contract. I'm not sure this behavior is actually achievable as designed - so I'm weighing whether it makes more sense to remove this test or adapt it.

TestTransferDimensionsFromServiceCommPackageToVendorServiceComm
Something appears to have broken with the changes. In the best case, the test just needs to be updated to reflect the new behavior; in the worst case, some functionality may have been unintentionally affected. Worth investigating to confirm which scenario we're in.

Can you check out and give your thoughts?

[Franco111000]

@miljance, both failures traced to the same cause: my first commit made SetDefaultDimensions priority-aware, but that recomputed the Subscription Line dim from scratch and wiped any custom dim already on the line (auto-insert from cross-package propagation, etc.).

Pushed a restructure:

• SetDefaultDimensions reverted to its original Item-only behavior.
• New ApplyContractDimensions does a 3-slot merge via GetCombinedDimensionSetID: current SC dim, Contract dim, and a priority overlay built only from sources that outrank the Contract partner table per Default Dimension Priority. When the priority table is at the BC default (Customer/Vendor outrank Item), the overlay is empty and the call degrades to the original blind merge with Contract winning, so existing tests that don't configure priorities see no behavior change. The overlay only activates when the user has explicitly flipped priorities (the [Bug][SubscriptionBilling]: Default Dimension Priorities not respected when creating Sales Invoice from Customer Subscription Contract #8084 case).
• UpdateFromCustomerContract / UpdateFromVendorContract and the Pathway B call sites use ApplyContractDimensions.
• Test cleanup added so the new test doesn't leak priority rows to other tests.

Could you give it another run when you have a moment?

[miljance]

@Franco111000 PR looks good and all tests pass. The ApplyContractDimensions approach with GetTableIDsForHigherPriorities is exactly right.

Two small things I'd like addressed before merging:

1. Duplicated source-building logic — SetDefaultDimensions and ApplyContractDimensions share identical code for building the dim-source list (Invoicing Item + ServiceObject type switch). Could you extract that into a shared local procedure? - Not critical though - if you like it this way better - fine by me.

2. Inconsistent priority handling in contract line tables — CustSubContractLine.DeleteRelatedVendorServiceCommDimensions and the equivalent in VendSubContractLine recalculate dims via SetDefaultDimensions + GetCombinedDimensionSetID, bypassing the same priority logic this PR fixes. Those should call ApplyContractDimensions instead.

Both should be straightforward to include in this PR. Let me know if anything is unclear.

[Franco111000]

@miljance, both done in the latest commit:

• Extracted AddDefaultDimensionSources as a local helper on Subscription Line, used by both SetDefaultDimensions and ApplyContractDimensions. The helper uses a guarded if ServiceObject.Get then; the original SetDefaultDimensions had an unguarded Get. No caller relies on that error path, so this is a small defensive improvement.
• CustSubContractLine.DeleteRelatedVendorServiceCommDimensions and VendSubContractLine.UpdateServiceCommitmentDimensions now call ApplyContractDimensions instead of the blind GetCombinedDimensionSetID. Source codes: Purchases + Database::Vendor on the Customer-side delete path (Vendor Contract being applied), Sales + Database::Customer on the Vendor-side cross-link path (Customer Contract being applied). Under vanilla priorities the overlay stays empty and behavior is identical to before.

[miljance]

looks great

[JesperSchulz]

Compilation seem to fail with:
Error: AL0132 'Codeunit Microsoft.Manufacturing.Subcontracting.Test."Subc. Library Mfg. Management"' does not contain a definition for 'CreateLaborReqWkshTemplateAndNameAndUpdateSetup'

[Franco111000]

Thanks @JesperSchulz! That error came from Manufacturing.Subcontracting test library changes in main that our branch hadn't picked up yet. Merged main in, should be good on the next CI run.

[github-actions]

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

GetCombinedDimensionSetID is now orphaned dead code

The internal procedure GetCombinedDimensionSetID in SubscriptionLine is no longer called from any of the contract creation paths in this PR, while still being called from billing document codeunits. The dual existence of the old simple-merge procedure alongside the new priority-aware one makes the code harder to reason about and risks future callers picking the wrong procedure.

Recommendation:

• Either remove GetCombinedDimensionSetID and migrate the two remaining billing callers (SalesDocuments, CreateBillingDocuments) to use ApplyContractDimensions, or add a comment explicitly documenting when each procedure should be used.

// If the simple merge is intentionally kept for billing documents, add:
// /// <summary>Use only for billing document line merges. For contract assignment use ApplyContractDimensions.</summary>
internal procedure GetCombinedDimensionSetID(DimSetID1: Integer; DimSetID2: Integer)

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

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

[Franco111000]

GetCombinedDimensionSetID is now orphaned dead code

Re the Copilot review note above: this one isn't dead code. Subscription Line.GetCombinedDimensionSetID still has two live callers, SalesDocuments (line 629) and PostSubContractRenewal (line 114), both merging a Sales Line dimension into a Subscription Line at sales-order / renewal posting time. That is a different scenario from contract assignment, where a plain merge is the right behavior, so those are intentionally left as-is. The GetCombinedDimensionSetID calls in CreateBillingDocuments are the separate Sales Line and Purchase Line table-extension procedures, not this one.

[JesperSchulz]

Re-running the single failed task. Seems like an instability.

[github-actions]

Copilot PR Review

Iteration 5 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Orchestrator pre-filter (13 file(s) excluded)

• layer-disabled (knowledge) : 13 file(s)

_{Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.}

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR moves Subscription Line contract dimension merging to a priority-aware path. It uses Source Code Setup plus DimensionManagement.GetTableIDsForHigherPriorities, GetRecDefaultDimID, and GetCombinedDimensionSetID, which matches the BaseApp Sales Line and Purchase Line priority pattern.

The main dimension result looks correct for the reported customer and item priority case, and the added tests cover customer and vendor contract creation. The remaining problem is extension compatibility: several paths no longer call the old merge procedure, so they also stop raising its integration event.

Suggestions

S1 - Preserve the dimension merge event
The changed contract paths now call ApplyContractDimensions instead of GetCombinedDimensionSetID. That skips the existing OnAfterGetCombinedDimensionSetID integration event for extensions that used to see these dimension changes. Fire an equivalent event from ApplyContractDimensions, or keep the old event firing after the new merge.

S2 - Add an invoice-level priority test
The new tests stop at Subscription Line. The reported bug was visible on the Sales Invoice line, and the billing merge is intentionally left unchanged. Add one assertion that generates the sales invoice and verifies the same dimension value on the Sales Line.

Risk assessment and necessity

Risk: Dimension values affect invoice reporting, deferrals, and downstream finance processes. The changed code is targeted, but it changes contract assignment, import, customer/vendor cross-link, and public integration-event behavior. Current PR checks also show failing unit-test jobs, so the branch should not merge until the failures are understood or green.

Necessity: The bug is valid and important: configured Default Dimension Priorities must be respected for Subscription Billing. The upstream fix is the right place because billing should consume an already resolved Subscription Line dimension set, but the extension event contract must be preserved.

---

[AI-PR-REVIEW] version=1 system=github pr=8115 round=1 by=alexei-dobriansky at=2026-07-02 lastSha=911038da81608e75f7048bee8ddc02d40523a900 suggestions=S1,S2

[Franco111000]

Thanks for the review, both suggestions are addressed in f3ca935.

S1 (preserve the dimension merge event): ApplyContractDimensions now fires OnAfterGetCombinedDimensionSetID(Rec) after the merge. Since ApplyContractDimensions is called exclusively from the call sites that previously called GetCombinedDimensionSetID (and under the same conditions), subscribers see the event on every contract assignment path exactly as before, with no new firings introduced elsewhere.

S2 (invoice-level priority test): RespectDefaultDimensionPrioritiesOnCustomerContractCreation now goes one step further: it creates the billing proposal and billing documents for the contract and asserts that the resulting Sales Line carries the Item's dimension value. The purchase side is left at Subscription Line level intentionally: the billing merge is the same code path for both partners, and the existing ExpectEqualDimensionSetIDPurchaseLineAndVendContractLine already covers Purchase Line dimensions at billing time.

On the failing checks: if I read the logs correctly, none of the five failing unit test jobs are related to this change. CA (UncategorizedTests) and CH (Default) failed on infrastructure (exclusive database access during demo data restore, container script error during app unpublish). AU/IN (LegacyTestsBucket1) and FI (LegacyTestsBucket2) fail on legacy Service, cost accounting and finance charge memo tests (PullServiceLineNoShipmentUsingUseFilter, GLTransferWithClosingDate, SuggestFinanceChargeMemo, ...), none of which touch Subscription Billing. Codeunit 148160 "Service Comm. Dimensions" does not appear in any failure list, which also lines up with @JesperSchulz's earlier note about instability. The new push should trigger a fresh run.

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

This PR makes Subscription Billing respect configured Default Dimension Priorities by resolving Subscription Line dimensions through the priority-aware path (Source Code Setup + DimensionManagement priority helpers), matching the BaseApp Sales/Purchase line pattern.

The two commits pushed since round 1 address both prior suggestions correctly. Subscription Line now raises OnAfterGetCombinedDimensionSetID(Rec) right after it computes the combined dimension set, so the integration event fires again on the new path. The customer-contract test now creates the billing documents and asserts that the billed Sales Line carries the highest-priority Item dimension value, so the fix is proven end to end from contract to invoice.

Status of previous suggestions

ID	Title	Status	Note
S1	Preserve the dimension merge event	Addressed	SubscriptionLine.Table.al now calls OnAfterGetCombinedDimensionSetID(Rec) after GetCombinedDimensionSetID, so extensions keep seeing the dimension changes on the priority-aware path.
S2	Add an invoice-level priority test	Addressed	RespectDefaultDimensionPrioritiesOnCustomerContractCreation now runs Create Billing Documents and verifies the Item dimension value on the billed Sales Line, not only the Subscription Line.

New observations (commits since round 1)

None - the changes only addressed the prior suggestions.

Risk assessment and necessity

Risk: Low and well contained. The event is restored at the same logical point (after the combined dimension set is computed), so extension behavior is preserved and the new firing is not a duplicate on the old path. Dimension values still affect invoice reporting and finance flows, but the added end-to-end assertion now guards the exact scenario. One pre-merge item remains outside the code: the full Pull Request Build has not yet run on this head (f3ca9358); confirm the previously-failing unit-test jobs come back green before merge.

Necessity: The bug is valid and important - configured Default Dimension Priorities must be respected for Subscription Billing. The fix resolves dimensions at the Subscription Line so billing consumes an already-resolved set, which is the right layer, and it now keeps the public event contract intact.

---

[AI-PR-REVIEW] version=1 system=github pr=8115 round=2 by=alexei-dobriansky at=2026-07-02 lastSha=f3ca9358e1cb52e7e73543dd2952e93be439f2bc suggestions=S1:addressed,S2:addressed parentRound=1

[JesperSchulz]

Let's restart the pipeline.

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

In the new helper SetDefaultDimensionPriority, the parameter 'Priority: Integer' shares its name with the 'Priority' field on the local 'Default Dimension Priority' record.

The calls 'DefaultDimensionPriority.Validate(Priority, Priority)' (lines 1101 and 1107) therefore resolve the first argument to the local Integer parameter, not to 'DefaultDimensionPriority.Priority' as intended. Validate's first argument must be a genuine field-access expression on the target record; passing the shadowed local variable instead is not a valid field reference and will fail to compile, blocking the build for this new test codeunit. Qualify the field explicitly as 'DefaultDimensionPriority.Priority' at both call sites.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    local procedure SetDefaultDimensionPriority(SourceCode: Code[10]; TableID: Integer; Priority: Integer)
    var
        DefaultDimensionPriority: Record "Default Dimension Priority";
    begin
        if DefaultDimensionPriority.Get(SourceCode, TableID) then begin
            DefaultDimensionPriority.Validate(DefaultDimensionPriority.Priority, Priority);
            DefaultDimensionPriority.Modify(true);
        end else begin
            DefaultDimensionPriority.Init();
            DefaultDimensionPriority."Source Code" := SourceCode;
            DefaultDimensionPriority."Table ID" := TableID;
            DefaultDimensionPriority.Validate(DefaultDimensionPriority.Priority, Priority);
            DefaultDimensionPriority.Insert(true);
        end;
    end;

Knowledge:

• microsoft/knowledge/style/variable-name-must-not-shadow.md

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