From 18befc6add38d6d55a063aac4d9e5c2b22431de7 Mon Sep 17 00:00:00 2001 From: Shaun Hogan Date: Thu, 7 Apr 2022 14:44:24 +0000 Subject: [PATCH 1/5] Merged PR 36716: Fix duplication of guest emails in order migration ## What's being changed We have updated the `InsertEmailOrderTable` migration by wrapping `array_column()` method in `array_unique()` to filter out duplicate guest emails and changed `array_diff()` to `array_udiff()` implementing `strcasecmp` as a comparator allowing a case-insensitive diff of the filtered list. ## Why it's being changed Running dotdigital:migrate would cause emails address duplicates in the email_contact table. This could happen with emails having a casing mismatch between Magento and Dotdigital, but we were also inserting multiple duplicate rows for guest order emails if they did not already exist in the table. This regression dates from the order sync refactor in 4.15.0. ## How to review / test this change - Register to a magento website with an email address that starts with capital letter (e,g) Chaz@emailsim.io - Place two orders as a guest with the same email that starts with a small letter (e,g) chaz@emailsim.io, Ignore the login request by magneto and process with guest checkout. - Run `bin/magento dotdigital:migrate` - Check` email_contact` table insure there are no duplicated email address. Related work items: #171251 --- Setup/Install/Type/InsertEmailOrderTable.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Setup/Install/Type/InsertEmailOrderTable.php b/Setup/Install/Type/InsertEmailOrderTable.php index ac70eb21..b6710e0a 100755 --- a/Setup/Install/Type/InsertEmailOrderTable.php +++ b/Setup/Install/Type/InsertEmailOrderTable.php @@ -140,20 +140,27 @@ private function insertGuestsFromArray(array $orderData) foreach ($storeGroups as $storeId => $storeOrders) { $guestsToInsert = []; $websiteId = $this->storeManager->getStore($storeId)->getWebsiteId(); - $guestOrderEmails = array_column($storeOrders, 'customer_email'); + $guestOrderEmails = array_unique( + array_column( + $storeOrders, + 'customer_email' + ) + ); $matchingContactEmails = $this->contactCollectionFactory->create() ->matchEmailsToContacts( $guestOrderEmails, $websiteId ); - foreach (array_diff($guestOrderEmails, $matchingContactEmails) as $email) { + foreach (array_udiff($guestOrderEmails, $matchingContactEmails, 'strcasecmp') as $email) { + $guestsToInsert[] = [ 'is_guest' => '1', 'email' => $email, 'store_id' => (string) $storeId, 'website_id' => $websiteId, ]; + } if (empty($guestsToInsert)) { From 3990230d437c7a316b31ab1517fdd59eba42ca20 Mon Sep 17 00:00:00 2001 From: Alastair Mucklow Date: Fri, 8 Apr 2022 11:53:03 +0000 Subject: [PATCH 2/5] Merged PR 36722: Fix regression in image types config ## What's being changed ImageRoleProcessor backend model. ## Why it's being changed In an effort to fix an [array to string conversion bug](https://dev.azure.com/dotdigital/ec/_workitems/edit/170230/) we modified the isValueChanged logic [here](https://dev.azure.com/dotdigital/ec/_git/ec-core-magento2-extension/commit/2919693263bf64cccbfb9233c823f6e6e87838b7?refName=refs/heads/master&path=/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php&_a=compare) with the result that once again we are resetting the catalog any time any image type value is changed. ## How to review / test this change - Change the Abandoned Cart image type (test from 0 to a value, and from value to value) - Catalog must not be reset ## Notes Both these checks must be specified on a test case for Image Types. i.e. - Confirm catalog is ONLY reset if the catalog image type changes - Confirm if an image type config value is `{"1":[1,2,3]}` we do not get array to string conversion error in PHP Related work items: #162933 --- Model/Config/Backend/ImageTypes/ImageRoleProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php b/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php index 7dddb87e..3011d103 100644 --- a/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php +++ b/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php @@ -147,6 +147,6 @@ private function _isValueChanged() $oldValueId = $this->getOldValue(); } - return !is_string($oldValueId) || $this->getValue() !== $oldValueId; + return is_array($oldValueId) || $this->getValue() !== (string) $oldValueId; } } From 8eb9933f895089f0958aa82af8f426b2f7c6f82e Mon Sep 17 00:00:00 2001 From: Fanis Strezos Date: Mon, 11 Apr 2022 10:59:39 +0000 Subject: [PATCH 3/5] Merged PR 36631: Resolve importer error if contact sync cron executes every 60' ## What's being changed In this PR we return instantly that the cron duration is 60 mins in `getDecodedCronValue()`. ## Why it's being changed This function is not getting called only on configuration save (where we initialize the cron patterns with the offset) but in importer cron too, to figure out the last contact sync cron time. https://github.com/dotmailer/dotmailer-magento2-extension/blob/master/Model/Sync/Importer/ImporterReportHandler.php#L289 In case that the cron schedule for contact sync is set to run every 60' (pattern is X * * * * *) the decoder function throws an exception instead of sending the correct value. ## How to review / test this change _Test report handler_ - set contact cron to run every 60' - make sure that you have in your email_importer bulk contact records - Run importer sync twice - Make sure that in second run you get the correct value [here](https://github.com/dotmailer/dotmailer-magento2-extension/blob/master/Model/Sync/Importer/ImporterReportHandler.php#L289) _Test config changes_ - save different values for cron timings - test saving both 15/30 minutes AND 60 minutes Related work items: #171487 --- Model/Adminhtml/Backend/CronOffset.php | 9 +++---- Model/Cron/CronOffsetter.php | 25 +++++++++++++------ Model/Sync/Importer/ImporterReportHandler.php | 3 ++- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/Model/Adminhtml/Backend/CronOffset.php b/Model/Adminhtml/Backend/CronOffset.php index a4c3a793..2080d6db 100644 --- a/Model/Adminhtml/Backend/CronOffset.php +++ b/Model/Adminhtml/Backend/CronOffset.php @@ -54,7 +54,7 @@ protected function _afterLoad() } /** - * @return CronOffset + * @return void */ public function beforeSave() { @@ -70,10 +70,7 @@ public function beforeSave() */ private function isCronValueChanged() { - if (strpos($this->getOldValue(), "/") !== false) { - return (int) $this->cronOffsetter->getDecodedCronValue($this->getOldValue()) != (int)$this->getValue(); - } else { - return $this->getValue() != '00'; - } + $oldValue = $this->cronOffsetter->getDecodedCronValue($this->getOldValue()); + return $this->getValue() != $oldValue; } } diff --git a/Model/Cron/CronOffsetter.php b/Model/Cron/CronOffsetter.php index 7f268bbe..096d4be0 100644 --- a/Model/Cron/CronOffsetter.php +++ b/Model/Cron/CronOffsetter.php @@ -5,26 +5,37 @@ class CronOffsetter { /** - * @param $cronValue + * Get cron pattern with offset. + * + * @param string $cronValue * @return string */ public function getCronPatternWithOffset($cronValue) { if ($cronValue !== '00') { - $valueWithOffset = rand(1, $cronValue -1) . '-59' . '/' . $cronValue; + $valueWithOffset = rand(1, (int) $cronValue - 1) . '-59' . '/' . $cronValue; return sprintf('%s * * * *', $valueWithOffset); } else { - return sprintf('%s * * * *', (string) rand(0, 59)); + return sprintf('%s * * * *', rand(0, 59)); } } /** - * @param $cronValue - * @return mixed|string + * Get decoded cron value. + * + * This function inspects the cron pattern. + * If "/" character is missing then the cron executed every 60'. + * + * @param string $cronValue + * @return string */ public function getDecodedCronValue($cronValue) { - $temp = explode("/", $cronValue); - return explode("*", $temp[1])[0]; + if (strpos($cronValue, "/") !== false) { + $temp = explode("/", $cronValue); + return trim(explode("*", $temp[1])[0]); + } + + return '00'; } } diff --git a/Model/Sync/Importer/ImporterReportHandler.php b/Model/Sync/Importer/ImporterReportHandler.php index 423beffc..2020c22b 100644 --- a/Model/Sync/Importer/ImporterReportHandler.php +++ b/Model/Sync/Importer/ImporterReportHandler.php @@ -284,12 +284,13 @@ private function filterContactsInReportCsv($data) private function getLastContactSyncTime() { // get a time period for the last contact sync - $cronMinutes = filter_var( + $decodedCronValue = filter_var( $this->cronOffsetter->getDecodedCronValue( $this->scopeConfig->getValue(Config::XML_PATH_CRON_SCHEDULE_CONTACT) ), FILTER_SANITIZE_NUMBER_INT ); + $cronMinutes = ($decodedCronValue === '00' ? '60' : $decodedCronValue); $time = new \DateTime($this->dateTime->formatDate(true), new \DateTimeZone('UTC')); return $time->sub(new \DateInterval("PT{$cronMinutes}M")); } From 66908386d9314b13420afa60d5a6a90f4a9b5bea Mon Sep 17 00:00:00 2001 From: Alastair Mucklow Date: Mon, 11 Apr 2022 16:05:01 +0000 Subject: [PATCH 4/5] Merged PR 36811: Protect against bad image types configuration ## What's being changed ImageRoleProcessor. ## Why it's being changed Some merchants have reported seeing a blank screen in our Dotdigital > Configuration layout. This has normally been resolved by manually purging image types configuration from `core_config_data`, but this PR should remove the need for this. ## How to review / test this change - Run this query: ``` update core_config_data set value = '{"1":[1,2,3]}' where path = 'connector_configuration/image_types/catalog_sync'; ``` - Refresh Dotdigital > Configuration (don't see a blank screen) - Run `bin/magento cache:flush config` - Refresh Dotdigital > Configuration - Try to save a new value for the Catalog Sync image type (should work as expected) Related work items: #172418 --- Model/Config/Backend/ImageTypes/ImageRoleProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php b/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php index 3011d103..a54799ba 100644 --- a/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php +++ b/Model/Config/Backend/ImageTypes/ImageRoleProcessor.php @@ -119,7 +119,7 @@ protected function _afterLoad() $this->setValue(false); return; } - $this->setValue(empty($unserialized) ? false : $unserialized['id']); + $this->setValue(empty($unserialized) || !isset($unserialized['id']) ? false : $unserialized['id']); } /** From b8be11aa267a872c5a9aebdc13d6a5384e9b37f7 Mon Sep 17 00:00:00 2001 From: Alastair Mucklow Date: Tue, 12 Apr 2022 07:41:27 +0000 Subject: [PATCH 5/5] Merged PR 36826: 4.16.1 pre-release Release notes for 4.16.1. Related work items: #172516 --- CHANGELOG.md | 8 ++++++++ composer.json | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e36e84..7794fb1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# 4.16.1 + +### Bug fixes +- One of our data migration scripts was inserting duplicate rows for guest purchasers in `email_contact`. This was a regression from 4.15.0. +- We fixed a fault in the importer sync that occurred if the cron schedule for contact sync was set to 'Every 60 Minutes'. +- We fixed a bug with the catalog reset that happens when image type values are changed, in **Dotdigital > Configuration > Image Types**. +- Some stored image types values could cause a blank screen in **Dotdigital > Configuration**; this has been fixed. + # 4.16.0 ### What's new diff --git a/composer.json b/composer.json index baeab481..74ea333b 100644 --- a/composer.json +++ b/composer.json @@ -2,7 +2,7 @@ "name": "dotdigital/dotdigital-magento2-extension", "description": "Dotdigital for Magento 2", "type": "magento2-module", - "version": "4.16.0", + "version": "4.16.1", "license": "MIT", "repositories": [ {