dotmailer / dotmailer-magento2-extension

The official Dotdigital for Magento2 extension
https://dotdigital.com/integrations/magento
MIT License
48 stars 63 forks source link

Trying to get property 'status' of non-object (running ddg_automation_abandonedcarts cron) #577

Closed NoDiskInDriveA closed 3 years ago

NoDiskInDriveA commented 3 years ago

As far as I'm aware, this still apllies to the current master

This happens in Model\AbandonedCart\PendingContactUpdater::checkStatusForPendingContactsInternal():

foreach ($collection as $item) {
    $websiteId = $this->helper->storeManager->getStore($item->getStoreId())->getWebsiteId();
    $contact   = $this->helper->getOrCreateContact($item->getEmail(), $websiteId);
    if (isset($contact->id) && $contact->status !== Automation::CONTACT_STATUS_PENDING) {
        $idsToUpdateStatus[] = $item->getId();
    } elseif (($item->getCreatedAt() < $expiryDate) &&
              $contact->status === Automation::CONTACT_STATUS_PENDING
    ) {
        $idsToExpire[] = $item->getId();
    } else {
        $idsToUpdateDate[] = $item->getId();
    }
}

Dotdigitalgroup\Email\Helper\Data::getOrCreateContact() will return false in case the response says the contact is not subscribed:

if (isset($response->status) && $response->status !== 'Subscribed') {
    $contact->setEmailImported(1);
    $contact->setSuppressed(1);
    $this->saveContact($contact);
    return false;
}

So $contact->status === Automation::CONTACT_STATUS_PENDING will trigger an error if the item is expired.

Removing the return false; fixes my specific use case, but I'm not sure about other implications this might have.

sta1r commented 3 years ago

@NoDiskInDriveA thanks for this, you're right of course. We'll patch it and report back.

In the meantime, removing return false; from Dotdigitalgroup\Email\Helper\Data::getOrCreateContact() may avert this specific issue, but will almost certainly cause other problems and may result in suppressed contacts being re-subscribed.

You can manually mark rows as status 'Expired' using a mysql query. It's likely that you have one row with a contact who was initially PendingOptIn, but who (soon afterwards, like inside an hour) became unsubscribed or suppressed in Engagement Cloud. The sync script will keep checking that one contact, and will keep breaking as per your bug. Find the oldest PendingOptIn row, mark that as Expired, let the sync run again, and keep going like that until it starts running.

NoDiskInDriveA commented 3 years ago

Thanks for your quick reply - yeah that was the decision we made, not to patch your code but remove the two offending contacts from the DB, since the method is used in multiple places and we're not very familiar with the API side of this 😬.

We'll be watching this issue and backport any patch that eventually comes up. Thanks again!

sta1r commented 3 years ago

@NoDiskInDriveA Our 4.13.5 release should resolve your pending contact issues. Thanks again for raising.

NoDiskInDriveA commented 3 years ago

Thanks!