emartech / 3rd-party-integrations-magento-M2

Please note: this plugin is deprecated. we have a new version of the plugin- please contact emarsys support for more info
MIT License
0 stars 5 forks source link

infinite loop on subcriber save event #95

Open yohanespradono opened 6 years ago

yohanespradono commented 6 years ago

Hi,

We experienced an infinite loop on subcriber save event. This affected checkout place order. I debugged the code and this is what I found.

There is an event on etc/events.xml

    <event name="newsletter_subscriber_save_after">
        <observer name="emarsys_newsletter_subscribers" instance="Emarsys\Emarsys\Observer\RealTimeSubscriber" />
    </event>

on RealtimeSubscriber.php line 115-116

            $this->dataHelper->realtimeTimeBasedOptinSync($subscriber);
            $result = $this->subscriberModel->syncSubscriber(
                $subscriberId,
                $storeId,
                $frontendFlag,
                $pageHandle,
                $websiteId,
                0,
                $subscriberEmailChangeFlag
            );

it never goes to $result = $this->subscriberModel->syncSubscriber() and looping inside realtimeTimeBasedOptinSync() if newsletter_subscriber.change_status_at IS NULL. I am not sure why some of our customers have this field NULL and subcriber_status = 1.

from inside of realtimeTimeBasedOptinSync() in file Emarsys\Emarsys\Helper\Data.php:

            $response = $this->modelApi->get('contact/last_change', $payload);

returns a value inside $response['data']['time'] and has $response['data']['current_value'] value that is not the same as $subscriber->getSubscriberStatus(); causing this line:

 if ((($EmarsysOptinChangeTime == $magentoOptinChangeTime && self::OPTIN_PRIORITY =='Emarsys') || ($EmarsysOptinChangeTime >= $magentoOptinChangeTime)) && $emarsysOptinValue != $magentoOptinValue) {

to returns true and causes the loop (because of the observer explained above)

$subscriber->setSubscriberStatus($statusToBeChanged)
                        ->setEmarsysNoExport(true)
                        ->save();

For some reason, I am not sure why some our customer's magento values have change_status_at = null and subscriber_status =(different than API response) maybe some records went out of sync due to they were edited from other extension or edited directly. whatever the case, this should not cause infinite loop.

As of now, we are turning off real time synchronization .

Thanks.

eghanin commented 6 years ago

thank you @yohanespradono did you experience it with 1.0.12 ?

thanks Guy

yohanespradono commented 6 years ago

@eghanin yes, i also upgraded to 1.0.13 but still happens. I turned off realtime sync, and processed the particular customer (using like $customerRepo->save($customer)) and everything went through. then i turned on realtime sync, and the problem is still fixed for this customer because the API response $response['data'] is empty now.

I can't find the API response log. Maybe $response['data']['current_value'] returned 2 and the code set $statusToBeChanged = \Magento\Newsletter\Model\Subscriber::STATUS_UNSUBSCRIBED; to 3

That's why $emarsysOptinValue != $magentoOptinValue to return true?

                    if ($emarsysOptinValue == 1) {
                        $statusToBeChanged = \Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED;
                    } elseif ($emarsysOptinValue == 2) {
                        $statusToBeChanged = \Magento\Newsletter\Model\Subscriber::STATUS_UNSUBSCRIBED;
                    } else {
                        $statusToBeChanged = \Magento\Newsletter\Model\Subscriber::STATUS_UNSUBSCRIBED;
                    }
yohanespradono commented 6 years ago

OK i can reproduce this issue. I made a change for this customer subscription from emarsys dashboard, and now the API responses:

array(3) {
  ["replyCode"]=>
  int(0)
  ["replyText"]=>
  string(2) "OK"
  ["data"]=>
  array(3) {
    ["old_value"]=>
    string(1) "1"
    ["current_value"]=>
    string(1) "2"
    ["time"]=>
    string(19) "2017-10-30 04:17:13"
  }
}

As i have guessed, because time (2017-10-30 04:17:13) > magento $magentoOptinChangeTime (null) returns true and current_value (2) != magento value (3) returns true, this causes the loop

Maybe we need to refactor this part

if ((($EmarsysOptinChangeTime == $magentoOptinChangeTime && self::OPTIN_PRIORITY =='Emarsys') || ($EmarsysOptinChangeTime >= $magentoOptinChangeTime)) && $emarsysOptinValue != $magentoOptinValue) {
eghanin commented 6 years ago

Thanks so much, we're going to look into this as soon as we can. SO to summarise what you're experiencing.

If there is a change in emarsys side ( e.g. customer optin value changed, and is picked up by the optin sync back ). and in Magento the time of change is NULL, then the synchronisation is getting to a loop.

Is that a correct description ?

we will keep you posted here. and eventually suggest a patch for it.

yohanespradono commented 6 years ago

@eghanin exactly! i mean, from subscribe (value 1) to unsubscribe (value 2, from emarsys side) while subscribe status on magento is 3

romastepa commented 6 years ago

hello @yohanespradono, did you save mapping for customer? I've tried to reproduce this, but without luck.

yohanespradono commented 6 years ago

@romastepa i didn't. But i have Unique Field set to Email.

romastepa commented 6 years ago

@yohanespradono can you, plz, save mapping first (in admin Emarsys->Mappings->Customers for your store Update Schema -> Recommend Mapping) and try one more time.

yohanespradono commented 6 years ago

Hi @romastepa , sorry we already set this up. I just didn't remember it.

image

peterlembke commented 5 years ago

We suffer this problem too. System: Magento 2.2.6 , Emarsys 1.0.12 In Magento 2.1.10 with Emarsys 1.0.12 we do not have this problem.

Every hour three accounts are registered in table newsletter_subscriber. in Emarsys admin we updated schemas and saved mappings in each store. Then the issue was gone.

I registered a new account and now that account are registered every hour.

I tried to: update newsletter_subscriber set change_status_at = "2018-05-01 01:01:01" where change_status_at is NULL; I had 22000 items that was NULL. But that made no difference. The issue still happens for the new account.

We can not go live with Magento 2.2.6 until this issue is fixed. Any tips what I can do?