[Bug]: "Subscription & Recurring Billing": Codeunit 8060 "Create Billing Documents" - CreateSalesHeaderFromContract

[PierreGlicinski]

Describe the issue

Preliminary: In my test system, the Base, the Subscription & Recurring Billing, as well as a product solution from my company, were installed.

While testing the app, we noticed the following error: (see screenshot). Creating billing documents is not possible because the sales header has changed.

After some manual searching (since the app was set to "allowDebugging": false), I stumbled upon the following code -> Codeunit "Create Billing Documents" - procedure CreateSalesHeaderFromContract and Codeunit "Document Change Management" - "PreventChangeSalesHeader"

My actual issue is quite simple to explain: Due to the validations between the Insert and the Modify, it is possible for external apps (by events) to influence this SalesHeader. If a Modify is also executed in this event, the "Document Change Management" comes into play.

We now have the following situation:

1.) The record has already been written via Insert, so the primary keys are already set, but all other fields are still empty.

2.) Due to the Modify by my own app, the check is now triggered. However, the Rec now has data. In my case, the validate on the "Sell-to Customer No." has already run, and the corresponding fields have been populated.

3.) The check function calls the xRec via Get, which in this case is empty.

So during the check, an empty record is compared to the current Rec, and the corresponding IF leads to an error.

Expected behavior

Since the code was published in a separate app, I would not have expected anyone to check whether data has changed during a Modify. In my opinion, there are several solutions for this:

1.) Write code that is resistant to extensions. To prevent other apps from accidentally modifying the SalesHeader in this function and to ensure that the appropriate event is used, validations should be avoided in this function.

2.) Move SkipContractSalesHeaderModifyCheck. The procedure would itself run into the above-mentioned error if it didn’t disable the corresponding check. Skipping the check should happen at the beginning of the function, so other apps also have the chance to populate their fields correctly.

3.) Make the SessionStore available in the Base. When reading the code, I initially hoped this had already been implemented. If the SessionStore Codeunit were available in the Base App, I could easily set the corresponding SkipFlag without any issues.

Steps to reproduce

The corresponding app "Subscription & Recurring Billing" must be active.

1.) Create a new app. 2.) Create a new event subscriber "OnAfterValidatePostingDateInSalesHeader" and perform a Modify on the Rec in the Code. 3.) In the Customer Contract, execute the action Create Contract Invoice and attempt to create a new invoice.

Additional context

I am aware that there is already a theoretical way to solve my issue. I will create a bridge app between my product app and the "Subscription & Recurring Billing" app and check whether the "Recurring Billing" flag has been set.

I will repeat this for every app used in my company...

But this cannot be the right approach, can it? This error will affect not only my app but also those from other teams in my company, as well as customer projects that activate this app.

I find it hard to believe that this error has only affected us.

If modules should be outsourced from their own app, they should not have such an impact on other extensions.

I will provide a fix for a bug

• [X] I will provide a fix for a bug

[PierreGlicinski]

As a small addendum, I noticed my mistake at the previously mentioned point. Of course, this needs to be considered globally for the entire app (purchases, etc.).

[JesperSchulz]

@AndersLarsenMicrosoft, could you triage?

[AndersLarsenMicrosoft]

Approved (i remember to add the label this time :-))

[dannoe]

Off topic:

@AndersLarsenMicrosoft @JesperSchulz Isn't this SessionStore logic rather dangerous? If errors occur between the SetBooleanKey and RemoveBooleanKey, the SessionStore would have inconsistent data and could affect other code paths. Or does the dictionary inside Codeunit "Session Store" get rolled back on error?