Skip to content

[Bug][SubscriptionBilling]: Multiple Issues with Usage-Based Billing in Subscription Billing#7299

Open
miljance wants to merge 4 commits intomicrosoft:mainfrom
miljance:SBUsageBasedBillingFixes
Open

[Bug][SubscriptionBilling]: Multiple Issues with Usage-Based Billing in Subscription Billing#7299
miljance wants to merge 4 commits intomicrosoft:mainfrom
miljance:SBUsageBasedBillingFixes

Conversation

@miljance
Copy link
Copy Markdown
Contributor

@miljance miljance commented Mar 20, 2026

Summary

Overview

This PR addresses four bugs in the Usage-Based Billing (UBB) feature within Subscription Billing, all of which could result in incorrect invoicing, duplicate billing, or data loss during document archival.

Changes

Bug 1 — Unit Price Calculation Failure
When import files supply unit prices instead of total amounts, the system previously failed to derive the missing values, producing zero-amount invoices. This fix adds logic to calculate quantity, unit price, or total amount from whichever values are available in the import data.

Bug 2 — Foreign Currency Handling
When usage data is denominated in a currency different from the local currency, amounts were calculated incorrectly with no validation or conversion applied. This fix introduces currency mismatch detection and applies proper currency conversion when processing usage data in non-local currencies.

Bug 3 — Credit Memo Duplication Loop
Posting a credit memo generated from negative usage amounts caused a duplicate billing line to be created, which would then trigger another identical credit memo on the next billing run — repeating indefinitely. This fix prevents duplicate billing lines from being created during credit memo posting.

Bug 4 — Archive Field Loss
Three UBB-related fields (Usage Based Billing, Usage Based Pricing, Pricing Unit Cost Surcharge %) were not being transferred to the Sales Subscription Line Archive table, breaking document restoration. This fix ensures all UBB fields are correctly included in the archive.

Impact

  • Billing accuracy for usage-based import files with unit prices
  • Correct handling of multi-currency usage data
  • Prevention of infinite credit memo loops
  • Full fidelity of archived subscription line documents

Code Review

Before being approved from Microsoft side I do expect the code review from @sit-zm and/or @samra-singhammer.
Code Review finished

Work Item(s)

Fixes #6972
AB#629332

@miljance miljance requested a review from a team as a code owner March 20, 2026 15:08
@github-actions github-actions bot added AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork labels Mar 20, 2026
@JesperSchulz JesperSchulz added the Finance GitHub request for Finance area label Mar 23, 2026
- Remove redundant DataClassification from archive table fields
  (SubscriptionLineArchive, SalesSubLineArchive) since table-level
  DataClassification already covers them
- Merge AlignVendorContractCurrency/AlignCustomerContractCurrency into
  a single AlignContractCurrency procedure with GetVendorContractCurrencyInfo
  and GetCustomerContractCurrencyInfo helpers
- Extract repeated derive+convert pattern in CalculateAmounts into a
  reusable DeriveAndConvertAmounts procedure
- Replace TestField with Assert.AreEqual in ArchivePreservesUsageBasedBillingFields test
@miljance
Copy link
Copy Markdown
Contributor Author

Thanks for the review @samra-singhammer! All feedback has been addressed:

  • Removed redundant DataClassification from the archive table fields
  • Merged AlignVendorContractCurrency/AlignCustomerContractCurrency into a single AlignContractCurrency procedure with GetVendorContractCurrencyInfo/GetCustomerContractCurrencyInfo helpers
  • Extracted the repeated derive+convert pattern in CalculateAmounts into a reusable DeriveAndConvertAmounts procedure
  • Replaced TestField with Assert.AreEqual in the archive test

@miljance
Copy link
Copy Markdown
Contributor Author

@sit-zm Can you please check the interface "Usage Data Processing"? I would like to make sure as much as possible, how the customer connector should be built.

…erface

Move 'No. of Imported Lines' and 'No. of Imported Line Errors' calculation
from hardcoded FlowFields (referencing Usage Data Generic Import) to the
Usage Data Processing interface, enabling custom connectors with different
staging tables.

- Add GetImportedLineCount and ShowImportedLines to the interface
- Implement both in Generic Connector Processing codeunit
- Update Usage Data Imports page to calculate counts and drill down via interface
- Mark fields 11 and 12 as obsolete (Pending 29.0, Removed 32.0)
@sit-zm
Copy link
Copy Markdown
Contributor

sit-zm commented Mar 25, 2026

@miljance
Looking at this interface, I noticed a few more things that can be implemented in the interface:

  1. Open a table/page other than "Generic Import Settings" - what if the implementer wants to have its own custom Import Settings table/page for its Usage Data Supplier Type. I see that the generic Usage Data Supplier Type is used for the ProcessUsageData method of the interface. Look at the OpenSupplierSettings() procedure in the UsageDataSupplier table.
  2. Also, about the OnDelete trigger of the UsageDataSupplier table, the related rows for "Usage Data Supplier Reference" and "Generic Import Settings" are deleted. The question is whether it makes sense to somehow implement the interface of the OnDeleteUsageDataSupplier method, which, based on the "Usage Data Supplier Type" from the "Usage Data Supplier" table, would also make sure that the data from other tables that may have been used in the implementation (e.g. a different settings table from point 1) would be deleted.
  3. The implementation methods in the GenericConnectorProcessing codeunit are set to be internal. What if the implementer wants to use most of the implemented generic methods without having to copy the code from the existing implementation - should they be made public procedures?
  4. In the OnDelete trigger of the Usage Data Import table, the DeleteImportedData procedure call is implemented, but the UsageDataBlob is left outside the implementation procedure, although it is clear that the ImportUsageData method is used to write data there. E.g. UsageDataGenericImport is deleted in DeleteImportedData and UsageDataBlob is not - it is always explicitly deleted. Does this mean that ImportUsageData must always explicitly use UsageDataBlob?
  5. For "Process Usage Data Billing," no procedure is placed in the interface, while for "Process Usage Data Import" "Create Usage Data Billing" there is an interface. On purpose?
    That's it for now.

@miljance
Copy link
Copy Markdown
Contributor Author

@miljance Looking at this interface, I noticed a few more things that can be implemented in the interface:

  1. Open a table/page other than "Generic Import Settings" - what if the implementer wants to have its own custom Import Settings table/page for its Usage Data Supplier Type. I see that the generic Usage Data Supplier Type is used for the ProcessUsageData method of the interface. Look at the OpenSupplierSettings() procedure in the UsageDataSupplier table.
  2. Also, about the OnDelete trigger of the UsageDataSupplier table, the related rows for "Usage Data Supplier Reference" and "Generic Import Settings" are deleted. The question is whether it makes sense to somehow implement the interface of the OnDeleteUsageDataSupplier method, which, based on the "Usage Data Supplier Type" from the "Usage Data Supplier" table, would also make sure that the data from other tables that may have been used in the implementation (e.g. a different settings table from point 1) would be deleted.
  3. The implementation methods in the GenericConnectorProcessing codeunit are set to be internal. What if the implementer wants to use most of the implemented generic methods without having to copy the code from the existing implementation - should they be made public procedures?
  4. In the OnDelete trigger of the Usage Data Import table, the DeleteImportedData procedure call is implemented, but the UsageDataBlob is left outside the implementation procedure, although it is clear that the ImportUsageData method is used to write data there. E.g. UsageDataGenericImport is deleted in DeleteImportedData and UsageDataBlob is not - it is always explicitly deleted. Does this mean that ImportUsageData must always explicitly use UsageDataBlob?
  5. For "Process Usage Data Billing," no procedure is placed in the interface, while for "Process Usage Data Import" "Create Usage Data Billing" there is an interface. On purpose?
    That's it for now.

@sit-zm Very valuable feedback. Thank you so much for that!

  1. absolutely, great find.
  2. yes
  3. Not sure here, these are interface implementation methods. I am wondering which ones could be called externally. Definity a topic to think about. It feels like I might want to expose some of the method but not all of them.
  4. I generally think that UsageDataBlob should not be deleted in the implementation codeunit. It is however true that some implementation might use the text files and thus Blobs while others might use API calls. So ImportUsageData does not have to use the UsageDataBlob, but I am unsure if I would like to delete it connector specific. I will ask AI and let it decide :) .
  5. "Process Usage Data Billing" is at the moment independent from a connector. Once created regardless of where the data come from the processing should be the same. So at the moment I do not see a need for an interface for processing usage data billing.

@miljance
Copy link
Copy Markdown
Contributor Author

Feedback points 1.-3. addressed. Points 4-5 were not included, left for some other future time if the needs arise.

@miljance
Copy link
Copy Markdown
Contributor Author

Ready for Microsoft Code Review.

@github-actions github-actions bot added the Linked Issue is linked to a Azure Boards work item label Mar 27, 2026
@github-actions github-actions bot modified the milestone: Version 29.0 Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Finance GitHub request for Finance area From Fork Pull request is coming from a fork Linked Issue is linked to a Azure Boards work item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][SubscriptionBilling]: Multiple Issues with Usage-Based Billing in Subscription Billing

5 participants