No. Series: Ability to extend filters on finding No. Series Lines when getting new numbers

[dominicstarkl]

Describe the issue

In the current implementation it is not possible to set additional filters when matching No. Series Lines are filtered on getting new numbers, see procedure GetNoSeriesLine(), codeunit 304 "No. Series - Impl.".

The only way to extend the filtering right now is by subscribing to IntegrationEvent OnAfterSetNoSeriesCurrentLineFilters(), called in procedure SetNoSeriesCurrentLineFilters(), codeunit 305 "No. Series - Setup Impl.".
Obviously the No. Series Lines are filtered there for UI purposes when setting up new No. Series and not for getting new numbers as in GetNoSeriesLine().

Before the movement of No. Series module to Business Foundation filters applied to No. Series Lines could be extended by subscribing to OnNoSeriesLineFilterOnBeforeFindLast() called in SetNoSeriesLineFilter() of codeunit 396 NoSeriesManagement, which is now obsoleted.

The filtering in SetNoSeriesLineFilter() served both for UI and getting new numbers purposes, whereas in the new solution the filtering is done in separate places (codeunits 304 and 305).
Therefore OnAfterSetNoSeriesCurrentLineFilters() does not completely replace former OnNoSeriesLineFilterOnBeforeFindLast() - there is no ability to extend filters on getting new numbers as it was in the old solution.

Expected behavior

There should be the possibility of extending filters applied to No. Series Lines in GetNoSeriesLine(), codeunit 304 "No. Series - Impl.", as a partial replacement of former OnNoSeriesLineFilterOnBeforeFindLast() in order to unbreak current subscribers.

The new IntegrationEvents could be raised in the same manner as the obsoleted OnNoSeriesLineFilterOnBeforeFindLast() is currently raised. Suggested signature: OnGetNoSeriesLineOnBeforeFindLast(var NoSeriesLine: Record "No. Series Line")

Steps to reproduce

Use case: No. Series Lines are filtered by department of current user.

Example:

[EventSubscriber(ObjectType::Codeunit, Codeunit::NoSeriesManagement, OnNoSeriesLineFilterOnBeforeFindLast, '', false, false)]
local procedure NoSeriesManagement_OnNoSeriesLineFilterOnBeforeFindLast(var NoSeriesLine: Record "No. Series Line")
begin
	NoSeriesLine.SetFilter("Department", '%1|%2', '', UserSetup.GetDepartmentCode());
end;

Additional context

No response

I will provide a fix for a bug

• I will provide a fix for a bug

[grobyns]

Hi @dominicstarkl

I'd like get your thoughts on some things before going ahead and adding the event. There's a few points of concern:

• since the event would pass the No. Series Line as var in order for additional/other filters to be added, there's a possibility of completely changing the series you're getting numbers from. Unfortunately I don't think we have a good way of capturing only added filters in a good way.
• Have you checked whether a single raising of the event in the new code would suffice for your scenario? We do few rounds of setting and unsetting certain filters and those might interfere with your scenario. Ideally you'd just set your filter at the start and everything else should just work.

Another option which I tend to favour is to add an interface for the GetNoSeriesLine method and let you define your own implementation. This would require a little more rework on your end but in exchange you'd gain full control. You could then add the implementation as a setting on the No. Series Line so it would be very explicit.

I look forward to your comments,
br/Gert

[dominicstarkl]

@grobyns
Hi Gert

Many thanks for your reply and time for our issues.

1. I agree, partners/PTEs could extend the behaviour of finding No. Series Lines and may completely break the originators/microsoft intention. IMHO this may always be the case when subscring to an event, it's the nature of the event/subscriber model. BTW this is currently also the case when subscribing to the existing event when filters are composed on setting up No. Series Lines.

2. I have analyzed the current implementation and found out that adding the event publishers on the same places as in the deprecated solution would give subscribers the same control as they had before.

3. Interfacing is always a good option in many places. But in this case we would appreciate the event/subscriber model to keep it simple as it was before.

What are the next steps?
Can I make a pull request with new events or would you do that for us?

Is it the correct way to ask for events/extensibility of BCApps/Business Foundation App by creating a bug issue on this GitHub repo in the future?
I saw that there is currently just one event in the whole Business Foundation App. Partners may probably ask for further events/extensibility in the future.
Or can we place such requests in the ALAppExtensions repo just as we do now for the Base Application?

Kindly regards
Dominic

[grobyns]

Hi @dominicstarkl,

For anything on BCApps the approach is to create an issue and if/when approved create the PR, that includes event requests. We will very carefully evaluate every event request to make sure it's the right event, with the right parameters and name and in the right place, and where possible, we'll promote alternatives.

For this particular issue, I'll approve and you can create a PR. I'd suggest OnGetNoSeriesLineFilters (or similar) as event name and the documentation of the event publisher should make it clear the purpose is only to add additional filters to the line. I'd also add a testfield or similar check on the Series Code field to ensure we get the same series before and after the event.

br/Gert

[grobyns]

@dominicstarkl I just noticed this issue is still open. Is anything pending or can it be closed?

[dominicstarkl]

@grobyns
Hi Gert

Thanks for asking. I'm still on holidays.
I will provide a PR asap, hopefully until end of this week.

Cheers
Dominic