Conversation
An alert row keeps both id_customer and customer_email, but the mail was always addressed from the customer record. Once that customer is deleted the id no longer resolves, Customer::__construct() hands back an unloaded object and the recipient ends up empty, so Mail::Send() reports 'Error: parameter "to" is corrupted' - fatally when the shop runs in debug mode, which takes the whole stock update down with it. Use the stored address when the customer record no longer resolves, and skip a row that has no usable address at all rather than letting one bad row break the alerts for every other customer waiting on the product.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ps_mailalert_customer_oosstores bothid_customerandcustomer_email, butMailAlert::sendCustomerAlert()always took the recipient from the customer record. Once that customer is deleted the id no longer resolves,Customer::__construct()hands back an unloaded object, the recipient is empty andMail::Send()reportsError: parameter "to" is corrupted- fatally when the shop is in debug mode, which aborts the stock update. The stored address is used when the customer record no longer resolves, and a row with no usable address at all is skipped so one bad row cannot break the alerts for every other customer waiting on that product.ps_mailalert_customer_oossurvives with the now danglingid_customer. Put the shop in debug mode and set the product's quantity above zero. Before: the save dies withError: parameter "to" is corrupted. After: the alert is sent to the address stored on the row and the stock update completes. Alerts for existing customers are unchanged.Measured RED -> GREEN through the real caller
Seeded
ps_mailalert_customer_oos(id_customer = 999999, customer_email = 'probe27489@example.test')for anactive product, then called
MailAlert::sendCustomerAlert($idProduct, 0):Supporting measurement:
new Customer(999999)givesid=NULL,email=NULL, so(string) $c->emailis''and
Mail::Send()hitsif (!is_array($to) && !Validate::isEmail($to))atclasses/Mail.php:261.Recipient resolution after the fix returns the stored
probe27489@example.test; a row with an emptycustomer_emailyieldsValidate::isEmail('') === falseand is skipped.Verified against upstream, not just the installed copy
The installed module is 3.0.1; the fix targets
PrestaShop/ps_emailalerts@dev, whoseMailAlert.phpcarriesthe identical block (the only difference between the two files is an unrelated
id_product_attributeline).Gates
php -lclean;php-cs-fixer(module's own.php-cs-fixer.dist.php)Found 0 of 1 files that can be fixed.tests/contains only PHPStan configs - so there is no unit testto add; the deterministic repro above stands in, per the same convention used for blockwishlist.
phpstan.neonlistsMailAlert.phpunderscanFiles, notpaths,so it is never analysed there anyway.
Not done
maildevis not running in this environment, so delivery of the alert was not observed end to end - onlythat the resolved recipient is a valid address and that the call no longer dies.