New notification trigger for post order validation

[JonBendtsen]

NEW notification trigger ORDER_POST_VALIDATE for post order validation notifications

The still existing trigger ORDER_VALIDATE is triggered pretty early in the validation process. This caused issues in the emails in that some substitution variables would not have content or contain the wrong content like the __REF__ which would still have the old name. Also, the PDF was not available for attachment.

This pull request introduces a new notification trigger called ORDER_POST_VALIDATE which is executed after the order actually has both been validated and committed to the database. This means that the substitution variable __REF__ & others now contains the correct name and that the file is actually attached in the email.

Known issues:
The document still needs to be re-generated before sending.

[JonBendtsen]

This pull request is not a complete fix of #25738 because this pull request only applies to orders, not proposals, invoices, ... or anything else that can be validated.

Also, the document attached should be regnerated.

However, if accepted, I'll probably slowly introduce similar pull requests for proposals, invoices and possible members - because I use those modules.

[JonBendtsen]

Here is the new message in Dolibarr when validating an order

[JonBendtsen]

The emails looks like this:

ORDER_VALIDATE

ORDER_POST_VALIDATE

[JonBendtsen]

This works like any other notifications, I need to enable them before usage

And I need to configure some email templates

Plus I need to select those templates in the notification module configuration

[eldy]

The goal of a trigger is to be inside a transaction so it can be canceled by the rollback, so, no trigger must be after commit(). Never.
You can retrieve the new ref into property this->newref
Generation of links that are using the old ref should be instead enhanced to use ->newref instead of ->ref when it is set.
Can you work on a fix going in this direction ?

[JonBendtsen]

The goal of a trigger is to be inside a transaction so it can be canceled by the rollback, so, no trigger must be after commit(). Never.

ok. What if you want to be "notified", informed, messaged, ... about an after post transaction fact? Sort of like "heads up, this thing happened, deal with it"

Sometimes there is a value in getting a post happening "call to action". In this case it can be emailing the customer with new information about their purchase status. And it certainly before emailing it could be to regenerate the document.

You can retrieve the new ref into property this->newref Generation of links that are using the old ref should be instead enhanced to user ->newref instead of ->ref when it is set. Can you work on a fix going in this direction ?

Perhaps, I'll have to look at it. Initially I don't really like that situation of just using different variables, and I don't think it makes good sense to move the post trigger to JUST before the commit, because it makes little sense to email the customer "accepted" when some other thing that listens to this trigger can cancel and then everything has been rolled back.

So how would you bring about a post transaction "trigger"/information/... ? (It could be named something like: Alleviate, Assauge, Commence, Debrief, Decommision, Mitigate, Mollify, Palliate, Placate, ...)

Would you make a whole duplication of the trigger system? but only for things that happened after a transaction has been committed?

[JonBendtsen]

I am not testing in latest development commit, it is a tuxgasy v19 container, but here's my comments on that

You can retrieve the new ref into property this->newref Generation of links that are using the old ref should be instead enhanced to user ->newref instead of ->ref when it is set. Can you work on a fix going in this direction ?

I just looked at what substitution variables is available when manually sending an order email, and here __NEWREF__ is not available, only __REF__ so I'll have to need 2 different email templates? one for manual sending and one for automatic notifications?

I also looked at this order using the API, and here the json does not contain any newref, only ref - and "oldref": null,

So those links that you say have to be updated to use ->newref instead of ->ref - can I trust that ->newref is always available? What about substitution variables in email templates, will I have to use different variables here too depending if I'm sending manually or sending using the automatic notification system?

Sorry sir, but I don't really like this "solution" either. I wish that the only difference between manually send emails and automatic send emails would be the sender, I don't wish to have multiple templates or to use different variables in the json depending on it's status if it is draft or validated - or has been validated.

Also, there's the whole issue of potentially having 2 or more different subsystems that listens to the ORDER_VALIDATE trigger, does something based on that - like reserving a product/resource for something or emails the customer, ... and then the last thing that listens to the ORDER_VALIDATE trigger says no, because it is my understanding that they can say no and interrupt the action about to happen. Then what? automatic notifications already send the email.

[eldy]

What if you want to be "notified", informed, messaged, ... about an after post transaction fact? Sort of like "heads up, this thing happened, deal with it"

The triggers (like ORDER_VALIDATE) are already executed at the end of the transaction. They alareayd are post action. A trigger is, by design, the last business action that has effect into database (except for delete, where it is the first).
So you should be able to use it for post action like sending an email. A trigger can already know if everything before is ok or not. All actions after triggers, are technical changes that should not be modified by external code, like updating cache table, but no business information are checked or modified.

The ->newref exists on object during the life of the trigger. So you can get it only inside code executed by a triggers.
When using an api, you will get only final result after end of transaction, so the ref you will get will always be the new final ref.

You don't need several template. A template should use __REF__ (You should never have __NEWREF__ inside a template, this key has no sense as a template is a user data and old ref / new ref is something managed on developer side). If the __REF__ does not contains the new ref when making the substitution of this key, it is was i said when i say "Generation of links that are using the old ref should be instead enhanced to use ->newref instead of ->ref when it is set.". I mean this:
There is something missing in the existing code in the core that make the substitution of __REF__. The current code does not take into account the ->newref field when it should.

Yes, you can have several triggers listening ORDER_VALIDATE, like you can have the same with a new ORDER_VALIDATE_POST. So problem will be the same. For this reason, there is a priority on trigger with a number between 01 and 99 into the trigger name provided by the module.

So this is a fix to apply (we must find why the new ref is not injected into REF when it should be when there is both ->ref and ->newref defined) and not a feature to add a post trigger, because all triggers are already post action triggers (except DELETE trigger,they are first action trigger).

[JonBendtsen]

Hi

Do you have 2 code examples? One where the code uses ->newref and one where it does not use ->newref but it should?

The ->newref exists on object during the life of the trigger. So you can get it only inside code executed by a triggers.

How does the code know if it is being executed inside a trigger? Could you provide an example? then I can see if I can understand it.

You don't need several template. A template should use __REF__ (You should never have __NEWREF__ inside a template, this key has no sense as a template is a user data and old ref / new ref is something managed on developer side). If the __REF__ does not contains the new ref when making the substitution of this key, it is was i said when i say "Generation of links that are using the old ref should be instead enhanced to use ->newref instead of ->ref when it is set.". I mean this: There is something missing in the existing code in the core that make the substitution of __REF__. The current code does not take into account the ->newref field when it should.

If ->newref is supposed to be hidden, how come it showed up in the list of substitution variables in the emails I attached pictures of above?

[eldy]

Where the code make the substitution __REF__ with ->ref, the code must instead do __REF__ replaced with ->newref (if ->newref is set).

[JonBendtsen]

Where the code make the substitution __REF__ with ->ref, the code must instead do __REF__ replaced with ->newref (if ->newref is set).

yeah? but where? I'm leaning towards closing this pull request and letting someone else fix the ref/newref quest.

[RAF0846]

Hello,
First at all, sorry for my english!
I have the same issue with automatic email notification on 20.0.1 version.
The PDF order confirmation sent to customer is not the validated one.
It is the draft one.