[Subscription Billing] Remove internal modifiers and make enums extensible for partner extension support

[Copilot]

Multiple procedures in Subscription Billing are declared internal and several enums have Extensible = false, blocking partner extensions with AL0161 compiler errors or preventing custom enum values.

Part 1 — Procedures: removed internal modifier

• ExtendContract (Page 8002): SetParameters(), SetUsageBasedParameters()
• Subscription Header (Table 8057): SetUnitPriceAndUnitCostFromExtendContract(), ResetCalledFromExtendContract()
• Subscription Package (Table 8055): FilterCodeOnPackageFilter(), ServCommPackageLineExists()
• Item Subscription Package (Table 8058): GetAllStandardPackageFilterForItem(), three overloads of GetPackageFilterForItem()
• Usage Data Supplier Reference (Table 8015): FindSupplierReference()
• Usage Data Import (Table 8013): CollectVendorContractsAndCreateInvoices()
• Service Comm. Package Lines (Page 8058): SetItemNo(), SetShowAllPackageLines(), SetPackageCode()
• Sales Line (Tableextension 8054): IsContractRenewal()
• Sales Subscription Line Mgmt. (Codeunit 8069): AddSalesServiceCommitmentsForSalesLine()

Part 2 — Codeunits: removed Access = Internal

• Usage Based Contr. Subscribers (Codeunit 8028): removed codeunit-level access restriction; promoted CreateContractInvoicesFromUsageDataImport() to public
• Personalization Data Mgmt. (Codeunit 8020): removed codeunit-level access restriction; promoted SetDataPagePersonalization() and GetDataPagePersonalization() to public

Part 3 — Enums: set Extensible = true

Enum	ID
Customer Rec. Billing Grouping	8057
Vendor Rec. Billing Grouping	8058
Contract Invoice Text Type	8001
Rec. Billing Document Type	8054
Usage Based Billing Doc. Type	8008

[github-actions]

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

[github-actions]

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

[github-actions]

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

User personalization read API exposed to partners

Making GetDataPagePersonalization public allows partner extensions to read arbitrary personalization values stored for the current user (UserSecurityId()). Values stored via SetDataPagePersonalization are free-form text blobs with no DataClassification tag and no audit trail, so partner code could silently read or infer user preferences, saved state, or other user-associated data.

Recommendation:

• If this API must remain public, add a DataClassification annotation to the Page Data Personalization value field (or document that callers must only write DataClassification::SystemMetadata data), and expose read access through a narrower, purpose-specific method rather than a generic key-value getter.

Suggested change

	procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean
	[InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'R')]
	procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Public pricing override bypasses price calculation

Making SetUnitPriceAndUnitCostFromExtendContract public allows any partner extension to set CalledFromExtendContract := true with arbitrary prices. Once set, all subscription lines created from this table instance use the caller-supplied prices instead of the normal price calculation logic (lines 1947–1958 in the same file), with no guard ensuring ResetCalledFromExtendContract is called afterward.

Recommendation:

• Add an [InherentPermissions] attribute or expose only through a dedicated integration event. If the public API is intentional, document that callers must always call ResetCalledFromExtendContract() in a try-finally block to avoid pricing state leakage.

Suggested change

	internal procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)
	[InherentPermissions(PermissionObjectType::TableData, Database::"Subscription Header", 'M')]
	procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)
	begin
	CalledFromExtendContract := true;
	UnitPrice := NewUnitPrice;
	UnitCost := NewUnitCost;
	end;

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

[github-actions]

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

Personalization write API exposed without InherentPermissions

Removing Access = Internal makes SetDataPagePersonalization callable by any partner extension. The method directly inserts or modifies rows in the Page Data Personalization system table. Without an [InherentPermissions] annotation, there is no explicit permission contract for callers, and the ObjectID text is parsed via CopyStr+Evaluate without error handling—a non-numeric suffix causes a runtime error.

Recommendation:

• Add [InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'IMD')] and wrap the Evaluate(ObjectNo, ObjectID) call in an if not Evaluate(...) guard to prevent unhandled runtime errors on malformed input.

Suggested change

	procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)
	[InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'IMD')]
	procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)
	var
	...
	ObjectNo: Integer;
	begin
	ObjectID := CopyStr(ObjectID, StrPos(ObjectID, ' ') + 1);
	if not Evaluate(ObjectNo, ObjectID) then
	exit;
	...

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

[github-actions]

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

Subscription line attachment bypasses sales-line triggers

Making AddSalesServiceCommitmentsForSalesLine public allows partner extensions to attach subscription package commitments to any Sales Line record outside the normal OnAfterInsert/OnAfterModify trigger flow. This can attach commitments to lines that should not carry subscriptions, or double-add commitments if called together with the event-driven path.

Recommendation:

• Add an [InherentPermissions] attribute and consider exposing an OnBeforeAddSalesServiceCommitmentsForSalesLine integration event so that partners can influence the existing trigger-driven call rather than bypassing it entirely.

Suggested change

	internal procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)
	[InherentPermissions(PermissionObjectType::TableData, Database::"Sales Subscription Line", 'IM')]
	procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)

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

[github-actions]

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

ResetCalledFromExtendContract lacks pairing contract

Exposing ResetCalledFromExtendContract publicly alongside SetUnitPriceAndUnitCostFromExtendContract without documenting a call-pair contract means partners could call Reset at arbitrary points, unexpectedly zeroing UnitPrice/UnitCost mid-flow when another caller has legitimately set them. There is no re-entrancy guard or usage counter protecting the state.

Recommendation:

• Document or enforce that these methods must be used as a matched set. Consider returning a disposable context object or using a try-finally guard pattern to ensure the reset is always paired with the set.

Suggested change

	internal procedure ResetCalledFromExtendContract()
	/// <summary>Must always be called in a try-finally block after SetUnitPriceAndUnitCostFromExtendContract.</summary>
	procedure ResetCalledFromExtendContract()
	begin
	CalledFromExtendContract := false;
	UnitPrice := 0;
	UnitCost := 0;
	end;

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

[github-actions]

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

Financial billing API exposed without permission guard

Removing Access = Internal and the internal modifier from CreateContractInvoicesFromUsageDataImport allows any partner extension to invoke invoice creation for arbitrary contract/line filters. The method has no [InherentPermissions] attribute, so there is no explicit declaration of what permissions callers require; a misconfigured extension could trigger billing runs it should not be allowed to start.

Recommendation:

• Add an [InherentPermissions] attribute that declares the minimum tabledata permissions needed, or keep the codeunit Access = Internal and instead expose the functionality via a dedicated integration event.

Suggested change

	procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)
	[InherentPermissions(PermissionObjectType::TableData, Database::"Billing Line", 'R')]
	procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)

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

[github-actions]

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

Vendor invoice creation publicly callable without guards

Making CollectVendorContractsAndCreateInvoices public allows partner code to initiate vendor contract invoice creation for any UsageDataImport record set. No [InherentPermissions] attribute documents the required access level, and there are no pre-conditions validating the caller's intent or the state of the import records.

Recommendation:

• Annotate with [InherentPermissions] declaring the minimum permissions needed (e.g., indirect Modify on billing tables), or restrict to an integration event pattern so that partner logic can hook in without taking full control of the invoicing flow.

Suggested change

	internal procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")
	[InherentPermissions(PermissionObjectType::TableData, Database::"Usage Data Import", 'RM')]
	procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Extensible enum breaks billing document creation

CreateBillingDocuments.Codeunit.al contains two case CustomerRecurringBillingGrouping of … end; blocks (lines 58–64 and 737–742) with no else clause. When a partner adds a new grouping value, the outer case at line 58 silently creates no billing documents, and the inner case at line 737 leaves PartnerNo empty, which would create billing documents with a blank customer.

Recommendation:

• Before merging, add else Error(...) or an OnBeforeProcessBillingLines integration event handler in both case blocks in CreateBillingDocuments.Codeunit.al so that unrecognised grouping values fail fast rather than silently misbehaving.

Suggested change

	Extensible = false;
	case CustomerRecurringBillingGrouping of
	CustomerRecurringBillingGrouping::Contract:
	CreateSalesDocumentsPerContract();
	CustomerRecurringBillingGrouping::"Sell-to Customer No.",
	CustomerRecurringBillingGrouping::"Bill-to Customer No.":
	CreateSalesDocumentsPerCustomer();
	else
	Error(UnhandledBillingGroupingErr, CustomerRecurringBillingGrouping);
	end;

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Extensible enum breaks vendor billing creation

CreateBillingDocuments.Codeunit.al has two case VendorRecurringBillingGrouping of … end; blocks (lines 66–72 and 748–752) with no else clause. A partner-added grouping value will cause the first block to create no purchase documents and the second block to leave PartnerNo empty, silently producing billing documents with a blank vendor.

Recommendation:

• Add else Error(...) to both case blocks in CreateBillingDocuments.Codeunit.al for VendorRecurringBillingGrouping, mirroring the existing OnGetAdditionalLineTextElseCase pattern used for ContractInvoiceTextType.

Suggested change

	Extensible = false;
	case VendorRecurringBillingGrouping of
	VendorRecurringBillingGrouping::Contract:
	CreatePurchaseDocumentsPerContract();
	VendorRecurringBillingGrouping::"Pay-to Vendor No.",
	VendorRecurringBillingGrouping::"Buy-from Vendor No.":
	CreatePurchaseDocumentsPerVendor();
	else
	Error(UnhandledBillingGroupingErr, VendorRecurringBillingGrouping);
	end;

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

[github-actions]

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

Extensible enum leaves cleanup logic with silent gaps

Document cleanup logic in UsageBasedContrSubscribers.Codeunit.al (lines 158–164 and 186–192) uses if … = ::Invoice … else if … = ::"Credit Memo" chains. When a partner adds a new UsageBasedBillingDocType value, deleted sales/purchase header records will not trigger any cleanup of UsageDataBilling rows, leaving stale billing references that corrupt usage reconciliation.

Recommendation:

• Expose an OnAfterCleanupUsageDataBillingForDocumentType integration event in the document-cleanup local procedures so that partners extending the enum can implement the corresponding cleanup logic.

Suggested change

	Extensible = false;
	[IntegrationEvent(false, false)]
	local procedure OnAfterCleanupUsageDataBillingForDocumentType(ServicePartner: Enum "Service Partner"; UsageBasedBillingDocType: Enum "Usage Based Billing Doc. Type"; DocumentNo: Code[20]; var IsHandled: Boolean)
	begin
	end;

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