Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NoSeriesManagement - add var to handle obsolete No Series #1267

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bussik
Copy link

@bussik bussik commented Jun 6, 2024

Fixes #1268 https://github.com/microsoft/BCApps/issues/1268

On the previous versions of BC there was an event:
image

But now there is new event:
image
And var is missing despite of that the function in body has a var.

@bussik bussik requested a review from a team as a code owner June 6, 2024 09:13
@github-actions github-actions bot added AL: Business Foundation From Fork Pull request is coming from a fork labels Jun 6, 2024
@bussik bussik changed the title NoSeriesManagement - add var to handle obsolete No Series [24.x] NoSeriesManagement - add var to handle obsolete No Series Jun 6, 2024
@bussik bussik changed the title [24.x] NoSeriesManagement - add var to handle obsolete No Series #1268 [24.x] NoSeriesManagement - add var to handle obsolete No Series Jun 6, 2024
@bussik bussik changed the title #1268 [24.x] NoSeriesManagement - add var to handle obsolete No Series [24.x] NoSeriesManagement - add var to handle obsolete No Series Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Issue #1268 is not valid. Please make sure you link an issue that exists, is open and is approved.

@JesperSchulz JesperSchulz changed the title [24.x] NoSeriesManagement - add var to handle obsolete No Series NoSeriesManagement - add var to handle obsolete No Series Jun 6, 2024
@AndreasMoth
Copy link
Contributor

@bussik can you explain a bit more of what problem you are trying to solve and why you need this event? We have the new interface as well that you can implement, please also let us know why that interface does not work.

@bussik
Copy link
Author

bussik commented Jun 6, 2024

@bussik can you explain a bit more of what problem you are trying to solve and why you need this event? We have the new interface as well that you can implement, please also let us know why that interface does not work.

@AndreasMoth There are some partners solutions that are still on the old No. Series, and there is a need to have possibility for change No. Series number. And it is strange that before there is possibility to do it, but in new version not.

@StefanSosic
Copy link
Contributor

@bussik can you explain a bit more of what problem you are trying to solve and why you need this event? We have the new interface as well that you can implement, please also let us know why that interface does not work.

@AndreasMoth There are some partners solutions that are still on the old No. Series, and there is a need to have possibility for change No. Series number. And it is strange that before there is possibility to do it, but in new version not.

Hi @bussik,
make sure first that Issue #1268 get approved. Then move to implementing the changes and submitting PR for review, so that we don't review something which might not be Approved.

@JesperSchulz JesperSchulz added the Integration GitHub request for Integration area label Jun 20, 2024
@JesperSchulz
Copy link
Contributor

@AndreasMoth, please review.

@bussik
Copy link
Author

bussik commented Jun 24, 2024

@AndreasMoth, please review.

Hello, when can we expect the bugfix? It will crash No. Series after upgrade to BC24.

@grobyns
Copy link
Contributor

grobyns commented Jun 24, 2024

Hi @bussik,
I'm don't think this fix will accomplish what you want. The code where this method is called looks like this:

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

So var or no var, the NoSeriesLine passed to the event can't affect the result at this point. This was done on purpose as you should not be able to ask for a number from one no series only to have an event subscriber give you a number from another.

I'd be happy to understand the scenario you're facing better so we can come up with a solution together. Unfortunately I don't see how this PR fixes the issue you describe...
br/Gert

@bussik
Copy link
Author

bussik commented Jun 28, 2024

Hi @bussik, I'm don't think this fix will accomplish what you want. The code where this method is called looks like this:

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

So var or no var, the NoSeriesLine passed to the event can't affect the result at this point. This was done on purpose as you should not be able to ask for a number from one no series only to have an event subscriber give you a number from another.

I'd be happy to understand the scenario you're facing better so we can come up with a solution together. Unfortunately I don't see how this PR fixes the issue you describe... br/Gert

Here it is more explained:
microsoft/ALAppExtensions#26721 (comment)

@grobyns
Copy link
Contributor

grobyns commented Jul 1, 2024

I replied again here: microsoft/ALAppExtensions#26721 (comment)

in addition to adding a var to the parameter, the code where it is called would also need a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AL: Business Foundation From Fork Pull request is coming from a fork Integration GitHub request for Integration area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NoSeriesManagement - add var to handle obsolete No Series[Bug]:
5 participants