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

OnAfterOnInsertButBeforeOnAfterInsert #699

Closed
vjekob opened this Issue Sep 20, 2017 · 4 comments

Comments

Projects
None yet
6 participants
@vjekob

vjekob commented Sep 20, 2017

Sorry for the provocative title, but we need some change in the eventing infrastructure.

Thinking of how we customized solutions before events, a typical scenario would be adding some fields to a table, and then perhaps assigning some values to those fields from within OnInsert or OnModify triggers. The only way to support that in event-based environment is to subscribe to OnAfterInsert or OnAfterModify events.

However! The only way to persist changes on Rec variable from within the OnAfterInsert or OnAfterModify events is to explicitly call Rec.Modify(). If you don't do this, a change in OnAfterInsert will not persist in the database (the OnAfter* events fire after the actual database operation was completed).

The problem with this is cascading modifications. If there was only one extension, the problem is not that big. However, if there are more extensions, both subscribing to the OnAfterModify event on a table, one extension would trigger another extension, and two extensions can end in an endless loop of cascading invocations of the OnModify trigger.

We should be able to do changes on Rec from within OnAfterInsert and OnAfterModify triggers to be persisted without having to explicitly call Modify() and risk cascading trigger invocations.

Yes, I know we can use OnBeforeInsert (or OnBeforeModify) trigger, but that trigger is far too early, because we usually put our own code at the end of triggers, rather than beginning (in the old days), because we often depend on, for example, a number from number series to be assigned or some other standard logic to have occurred.

In multi-tenant, multi-app world, this is a major consideration, and a major issue.

@ghost

This comment has been minimized.

ghost commented Sep 20, 2017

Hello,

I do not know if I am doing right but in my company, we usually write the OnAfterSomething event like that :


procedure OnAfterSomething(VAR Rec : ..., VAR xRec : ..., RunTrigger: boolean);
begin
  with Rec do begin
    if not RunTrigger or IsTemporary then
      exit;

    //DoSomething
  end;
end;

This way you do not enter in an infinite loop if the same trigger is subscribed multiple times because, obviously, you do not call modify(true).

For sure, it relies on the fact that everyone follow that kind of pattern...

@StanislawStempin

This comment has been minimized.

Member

StanislawStempin commented Sep 21, 2017

This is a good suggestion. We will add addressing this problem to our backlog.

@ajkauffmann

This comment has been minimized.

Contributor

ajkauffmann commented Nov 18, 2017

@StanislawStempin What is the status on this?

I tested with the OnAfter triggers on tableextensions, hoping that they are raised before the real database write, just because they are more closely related to the table than eventsubscribers in Codeunits.

However, also the OnAfter triggers in tableextensions appear to be after the database write. At least consistent behavior if you want to be positive. But since they are so closely related to the normal triggers it doesn't make sense to have to call MODIFY from within the OnAfterInsert or OnAfterModify trigger on a tableextension.

The only thing I found out is that triggers in the tableextension seem to be raised first, before any eventsubscriber in a Codeunit. Is that correct?

@Gallimathias

This comment has been minimized.

Contributor

Gallimathias commented Nov 20, 2017

I would like to take part in the discussion here. I think the event problem thaht described @ajkauffmann and @vjekob here is the same one we have with the "inheritance" generally. As already mentioned in #593 (#186, #120, #153) there will be massive problems with competing apps in the long run.

I am in favour of overwriting and clear inheritance. According to @StanislawStempin, the topic of inheritance should come one day. If this is the case, then the event problem also solves itself.

@esbenk esbenk added bug accepted and removed suggestion labels Feb 7, 2018

@thpeder thpeder added this to the March 2018 milestone Feb 21, 2018

@Microsoft Microsoft locked and limited conversation to collaborators May 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.