microsoft / ALAppExtensions

Repository for collaboration on Microsoft AL application add-on and localization extensions for Microsoft Dynamics 365 Business Central.
MIT License
749 stars 600 forks source link

Parameter NoSeriesLine in procedure RaiseObsoleteOnAfterNextNo3 needs to be called via reference #26721

Closed thorjons closed 1 week ago

thorjons commented 3 weeks ago

Describe the request

Require Var for parameter NoSeriesLine in procedure RaiseObsoleteOnAfterNextNo3 in codeunit NoSeriesManagement

Before internal procedure RaiseObsoleteOnAfterGetNextNo3(NoSeriesLine: Record "No. Series Line"; ModifySeries: Boolean)

After internal procedure RaiseObsoleteOnAfterGetNextNo3(var NoSeriesLine: Record "No. Series Line"; ModifySeries: Boolean)

Refers to TrackingID#2406210050000089

Additional context

LS Retail has an extension adding prefixes to assigned numbers from number series. After MS moved number series functionality to a Foundation app this functionality broke. Internal work item: AB#539238

pri-kise commented 3 weeks ago

Duplicate of https://github.com/microsoft/BCApps/issues/1268

thorjons commented 2 weeks ago

LSRetail extension adds a prefix to an issued number from number series through publisher event OnAfterGetNextNo3 https://github.com/microsoft/ALAppExtensions/assets/57487293/c18d19f5-60ff-491f-8327-fcffaa554beb This has been working for many versions back, but now because of broken var chain through procedures the added value is not returned to the record. In the case of the attached video, to the Purchase Header.

grobyns commented 2 weeks ago

@thorjons I understand the issue but this is by design in the new implementation and adding a var to parameter alone wouldn't help because we return the result we got from before the raising of the event. The issue is that the number you use after adding a prefix doesn't fit the series anymore (the number you get doesn't fit between start number and end number / last number used.

You can still achieve this behaviour but to do so you need to implement your own version of the "No. Series - Single" interface.

thorjons commented 2 weeks ago

We have already implemented this new approach into our solution for our next release, but we are currently facing issues with customers running the current versions with MS codebase where you introduced the RaiseObsoleteOnAfterGetNextNo3 event. This is a breaking change on Microsoft behalf, since the existing, now obsoleted Event "OnAfterGetNextNo2" did have the var image

grobyns commented 2 weeks ago

Again, yes, the var is not there but adding it back is not sufficient. The code where RaiseObsoleteOnAfterGetNextNo3 is called would also need to be refactored:

        Result := NoSeriesSingle.GetNextNo(NoSeriesLine, UsageDate, HideErrorsAndWarnings);
        NoSeriesManagement.RaiseObsoleteOnAfterGetNextNo3(NoSeriesLine, true);
        exit(Result);

instead of exit(Result), this code would have to return NoSeriesLine."Last No. Used". the same for PeekNextNo.

grobyns commented 2 weeks ago

@thorjons please review the PR https://github.com/microsoft/BCApps/pull/1446

bussik commented 1 week ago

@thorjons please review the PR microsoft/BCApps#1446

Done

JesperSchulz commented 1 week ago

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.

Build ID: .

JesperSchulz commented 3 days ago

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.

Build ID: 21513.