umbraco / Umbraco.Commerce.Issues

18 stars 2 forks source link

Confirmation and other emails are not send when the customer email contains whitespaces #528

Closed rpteasolutions closed 8 hours ago

rpteasolutions commented 1 week ago

Describe the bug Confirmation and other emails are not send when the customer email contains whitespaces.

Steps To Reproduce Steps to reproduce the behavior:

  1. Place an order with customer's email containing a whitespace at the end, eg "test@test.com "
  2. See error

Same happens if you try resend from backoffice UI.

Expected behavior Expected that the email was trimmed to not contain whitespaces.

Umbraco Commerce version: 13.1.4

mattbrailsford commented 1 week ago

I've just pushed an updated 13.1.5-preveiew.8 to our nightly server if you want to give that a try. It should now trim any value when being added to an orders property collection (this won't fix any already wrong data however).

You can find details here for how to use the nightly feed https://docs.umbraco.com/umbraco-cms/fundamentals/setup/install/installing-nightly-builds

rpteasolutions commented 5 days ago

Hi Matt! Thanks for quick execution. I tried to upgrade to 13.1.5-preview.8, but getting a migration error when trying to load backoffice and frontend afterwards.

Not sure, if I did something wrong. image

mattbrailsford commented 5 days ago

Ahh, no, the 13.1.5 update also includes and update to increase the size of price fields, but looks like we might have missed a default constraint. It's a shame that stack trace doesn't tell you which column it was trying to alter 🤦‍♂️

rpteasolutions commented 5 days ago

The best I can see from the log, is this file: IncreasePrecisionOfMonetaryColumns.cs

Full path: Umbraco.Commerce.Persistence.SqlServer.Migrations.Implement.v13_01_05.M20240516111500_IncreasePrecisionOfMonetaryColumns.cs

mattbrailsford commented 5 days ago

Could you do me a favor and check what your default constraints are named for the following tables / columns?

umbracoCommerceOrder > paymentTotalPrice
umbracoCommerceOrder > paymentTotalPriceTax
umbracoCommerceOrder > shippingTotalPrice
umbracoCommerceOrder > shippingTotalPriceTax
umbracoCommerceOrderLine > unitPriceBase
umbracoCommerceOrderLine > unitPriceBaseTax
umbracoCommerceOrderLine > unitPrice
umbracoCommerceOrderLine > unitPriceTax
umbracoCommerceGiftCard > remainingAmount
rpteasolutions commented 2 days ago

They all have ((0)) as 'Default Value or Binding' using SSMS19. If thats what you were looking for.

mattbrailsford commented 2 days ago

Sorry, no I was looking for the names of the default constraints. They should be in the form DF_{tableName}_{columnName} but if they were created without an explicit name, they would have been given a default one by SQL. We have code in the migration that drops those constraints and re-adds them, but if you have any that have different names, they won't have gotten dropped.

rpteasolutions commented 2 days ago

There seems to be a lot, but I have marked those that fits your template as bold. I used the script from here: https://stackoverflow.com/a/66566174 as I didn't know how else to find the default contraints.

umbracoCommerceOrder > paymentTotalPrice

umbracoCommerceOrder > paymentTotalPriceTax

umbracoCommerceOrder > shippingTotalPrice

umbracoCommerceOrder > shippingTotalPriceTax

umbracoCommerceOrderLine > unitPriceBase

umbracoCommerceOrderLine > unitPriceBaseTax

umbracoCommerceOrderLine > unitPrice

umbracoCommerceOrderLine > unitPriceTax

umbracoCommerceGiftCard > remainingAmount

mattbrailsford commented 2 days ago

Ahhh, there we go, I guess because this is a vendr migration all the default constraints still contain vendr rather than umbracoCommerce. Shouldn't be too hard for me to drop the vendr named defaults too. Thanks for confirming, that's been really helpful.

mattbrailsford commented 2 days ago

I've pushed a preview.10 nightly if you'd like to give that a try

rpteasolutions commented 2 days ago

No problem. We got there in the end 😅 And yeah, this solution was shortly running Vendr on Umbraco v11.

I will have a go at preview.10.

rpteasolutions commented 1 day ago

@mattbrailsford I have now successfully installed 13.1.5.preview10. Tested with mail containing a space in the user input field. Code is now trimming so it can send the confirmation mail etc.

Thanks!

mattbrailsford commented 1 day ago

Excellent. Thank you for confirming the fix, and your help with the migrations script. I'll get this pushed out this week.

rpteasolutions commented 1 day ago

Always happy to help 😊

mattbrailsford commented 8 hours ago

Fixed in 10.0.12 and 13.1.5 both released today. Thanks again for reporting.