I have a webshop running on Umbraco 9.5.4 and Vendr 2.3.4.
I use the OrderFinalizedHandler to add invoice numbers to orders and orderlines and to be displayed in the order email.
Both works...But only sometimes.
I have added Auto Capture in our payment method (QuickPay).
When creating an Uow the invoice numbers are only saved to the orders for the most part. Some times the invoice number isn't added.
We have made sure, its only running once and only is the invoicenumber is null.
I've tried using OrderTransactionUpdatedHandler instead - but this fails when trying to send a mail with _vendrApi.SendEmail.
Any ideas what to do? I can see in the log files that the code is running, but nothing is saved to the order property.
The code is the following.
using (IUnitOfWork uow = _vendrApi.Uow.Create())
{
Order order = orderReadOnly
.AsWritable(uow)
.SetProperties(new Dictionary<string, string>
{
{ "invoiceNumber", _orderHelper.GenerateInvoiceNumber() }
});
_vendrApi.SaveOrder(order);
Serilog.Log.Information($"Added invoice number on order {order.Properties["invoiceNumber"]}");
int orderlineCounter = 1;
foreach (var orderline in order.OrderLines)
{
var orderlineNumbering = orderlineCounter.ToString().PadLeft(3, '0');
var orderLineID = order.Properties["invoiceNumber"] + "_" + orderlineNumbering;
Serilog.Log.Information($"OrderLineId {orderLineID}");
order.WithOrderLine(orderline.Id).SetProperty("OrderLineID", orderLineID);
_vendrApi.SaveOrder(order);
orderlineCounter++;
}
uow.Complete();
Serilog.Log.Information("Exiting CreateInvoice");
Generally speaking I'd suggest that you only have one SaveOrder per unit of work so really I'd say to add order properties and call save at the end. The save inside the for loop could be extra problematic as you could be calling it many times in quick succession.
Sure, and the example you reference has multiple saves because it is saving different types of entities, but if you are saving the same entity multiple times, this really should only need to be once.
Where is your orderReadOnly coming from? The event handler argument? If so, it may be worth you re-fetching the order before you make it writable as events broadcast with a "snapshot" of the order when the event took place, but it's possible it could have changed via some other events in the meantime, so it's always best to refetch the order before making it writable.
This said, I can't see that being your issue here 🤔
Generally speaking, a save would only rollback / not persist if uow.Complete() wasn't called, here, or wrapping code that also opens a unit of work. I assume this is all the code within your event handler?
Order ReadOnly is coming from the handlers evt.. I previously also used GetOrder on that, but have removed it to not call it more than needed.
public override void Handle(OrderFinalizedNotification evt)
{
CreateInvoice(evt.Order);
}
But I will just readd it!
Yes it is the only Uow open in this handler. I have a method for sending the email through _vendrApi.SendMail. But this doesn't use Uow.
I have an idea that the problem is the updating of the PaymentStatus from QuickPay - because it seems that OrderTransactionUpdatedHandler and OrderFinalizedHandler runs at the same time.
I just tried to put in a Thread.Sleep(30000); in the invoice method and then every test order have a invoicenumber.
Could that be the issue? And can I somehow handle it better than using a sleep?
I think what is likely happening is that the webhook from the payment gateway is landing at practically the same time as your event handler and so it's possible the order being updated by the webhook is overriding your changes in the finalized handler.
Unfortunately we can't control when a webhook lands, but I would have expected the OrderTransactionUpdatedHandler might be a better option, though I'd probably check with this whether the order is captured as you'll likely get one of those firing when the transaction is initialized, but then again when the transaction is captured via the webhook.
The problem with OrderTransactionUpdatedHandler is that we cannot use _vendrApi.SendEmail() in it, to have invoice on the order mail. It gives a work queue error when using it there. Maybe you have a work around to this issue?
I've tried it previously - as we can, as you say, ask if the payment status is captured and then run the invoice method. So it would be perfect, if we could send the order mail in OrderTransactionUpdatedHandler.
I didn't have the log - so I reimplemented and tested it and now it works!
I figured my problem wasn't SendEmail as such. I was using await _vendrApi.CaptureOrderPaymentAsync(order); to manual set payment status as "Captured" and made the OrderTransactionUpdatedHandler async aswell. I think that making OrderTransactionUpdatedHandler async was the problem with SendEmail.
The reason for this, is that the "status" in the orderlist doesn't alway show "Captured", but only "Authorized". Even though the orders are both "Authorized" and "Captured".
I think its a timing issue with the Auto Capture method?
But this is of course a minor issue than not having our invoice numbers (So I went back to using Auto Capture) :)
OrderFinalizedHandler issue
Hi,
I have a webshop running on Umbraco 9.5.4 and Vendr 2.3.4.
I use the OrderFinalizedHandler to add invoice numbers to orders and orderlines and to be displayed in the order email.
Both works...But only sometimes.
I have added Auto Capture in our payment method (QuickPay).
When creating an Uow the invoice numbers are only saved to the orders for the most part. Some times the invoice number isn't added.
We have made sure, its only running once and only is the invoicenumber is null. I've tried using OrderTransactionUpdatedHandler instead - but this fails when trying to send a mail with _vendrApi.SendEmail.
Any ideas what to do? I can see in the log files that the code is running, but nothing is saved to the order property.
The code is the following.
Hey Morten,
Generally speaking I'd suggest that you only have one
SaveOrder
per unit of work so really I'd say to add order properties and call save at the end. The save inside the for loop could be extra problematic as you could be calling it many times in quick succession.Hi Matt,
I actually had that before with the same problem.
I adjusted it because of the documentation here: https://vendr.net/docs/core/2.1.0/umbraco-v9/key-concepts/unit-of-work/
As I can see, it also have 2 Save's in a Unit Of Work?
However I just tried with only 1 save and it still doesn't save the invoice number neither on the orderline or the order property.
Can you advise?
Sure, and the example you reference has multiple saves because it is saving different types of entities, but if you are saving the same entity multiple times, this really should only need to be once.
Where is your
orderReadOnly
coming from? The event handler argument? If so, it may be worth you re-fetching the order before you make it writable as events broadcast with a "snapshot" of the order when the event took place, but it's possible it could have changed via some other events in the meantime, so it's always best to refetch the order before making it writable.This said, I can't see that being your issue here 🤔
Generally speaking, a save would only rollback / not persist if
uow.Complete()
wasn't called, here, or wrapping code that also opens a unit of work. I assume this is all the code within your event handler?Order ReadOnly is coming from the handlers evt.. I previously also used GetOrder on that, but have removed it to not call it more than needed.
But I will just readd it!
Yes it is the only Uow open in this handler. I have a method for sending the email through _vendrApi.SendMail. But this doesn't use Uow.
I have an idea that the problem is the updating of the PaymentStatus from QuickPay - because it seems that OrderTransactionUpdatedHandler and OrderFinalizedHandler runs at the same time.
I just tried to put in a Thread.Sleep(30000); in the invoice method and then every test order have a invoicenumber.
Could that be the issue? And can I somehow handle it better than using a sleep?
Ahhh, ok, I think you may be right.
I think what is likely happening is that the webhook from the payment gateway is landing at practically the same time as your event handler and so it's possible the order being updated by the webhook is overriding your changes in the finalized handler.
Unfortunately we can't control when a webhook lands, but I would have expected the
OrderTransactionUpdatedHandler
might be a better option, though I'd probably check with this whether the order is captured as you'll likely get one of those firing when the transaction is initialized, but then again when the transaction is captured via the webhook.The problem with
OrderTransactionUpdatedHandler
is that we cannot use_vendrApi.SendEmail()
in it, to have invoice on the order mail. It gives a work queue error when using it there. Maybe you have a work around to this issue?I've tried it previously - as we can, as you say, ask if the payment status is captured and then run the invoice method. So it would be perfect, if we could send the order mail in
OrderTransactionUpdatedHandler
.Can you share the error you receieve when you try to send email in the
OrderTransactionUpdatedHandler
?Hi again,
I didn't have the log - so I reimplemented and tested it and now it works!
I figured my problem wasn't
SendEmail
as such. I was usingawait _vendrApi.CaptureOrderPaymentAsync(order);
to manual set payment status as "Captured" and made theOrderTransactionUpdatedHandler
async aswell. I think that makingOrderTransactionUpdatedHandler
async was the problem withSendEmail
.The reason for this, is that the "status" in the orderlist doesn't alway show "Captured", but only "Authorized". Even though the orders are both "Authorized" and "Captured".
I think its a timing issue with the Auto Capture method?
But this is of course a minor issue than not having our invoice numbers (So I went back to using Auto Capture) :)
is working on a reply...