Skip to content

Improve performance when resetting errors #975

@TroyPatteson

Description

@TroyPatteson

The code to reset errors loops through a collection saving each record clearing the mailchimp_sync_delta and mailchimp_sync_error fields when it should use a single UPDATE query.

This can be remedied by replacing the following code in resetErrors() in app/code/community/Ebizmarts/MailChimp/Helper/Data.php

       $collection = Mage::getResourceModel('newsletter/subscriber_collection')
            ->addFieldToFilter('mailchimp_sync_error', array('neq' => ''));
        if ($scopeId != 0) {
            $collection = $this->addStoresToFilter($collection, $scopeId, $scope);
        }
        foreach ($collection as $subscriber) {
            $subscriber->setData("mailchimp_sync_delta", '0000-00-00 00:00:00');
            $subscriber->setData("mailchimp_sync_error", '');
            $subscriber->save();
        }

with

       $storesFilter = '';
        if ($scopeId != 0) {
            $storeIds = $this->getMagentoStoresForMCStoreIdByScope($scopeId, $scope);
            if (count($storeIds) > 0) {
                $storesFilter = array('store_id IN ('.implode(',', $storeIds).')');
            }
        }

        $resource = $this->getCoreResource();
        $conn = $resource->getConnection('core_write');
        $conn->update(
            $resource->getTableName('newsletter/subscriber'),
            array(
                'mailchimp_sync_delta' => '0000-00-00 00:00:00',
                'mailchimp_sync_error' => ''
            ),
            $storesFilter
        );

And while we are on the topic of performance there are a couple of instances in the helper where a foreach loop continues even after finding the element being search for.

In getMagentoStoresForMCStoreIdByScope()

    public function getMagentoStoresForMCStoreIdByScope($scopeId, $scope)
    {
        $ret = array();
        $storeRelation = $this->getStoreRelation();
        $mailchimpStoreIdForScope = $this->getMCStoreId($scopeId, $scope);
        if ($mailchimpStoreIdForScope) {
            foreach ($storeRelation as $mailchimpStoreId => $magentoStoreIds) {
                if ($mailchimpStoreIdForScope == $mailchimpStoreId) {
                    $ret = $magentoStoreIds;
                }
            }
        }
        return $ret;
    }

there is no need to even loop for the mailchimp store ID as the array is indexed by mailchimp store ID! That code should be replaced with:

    public function getMagentoStoresForMCStoreIdByScope($scopeId, $scope)
    {
        $ret = array();
        $storeRelation = $this->getStoreRelation();
        $mailchimpStoreIdForScope = $this->getMCStoreId($scopeId, $scope);
        if ($mailchimpStoreIdForScope && isset($storeRelation[$mailchimpStoreIdForScope])) {
            $ret = $storeRelation[$mailchimpStoreIdForScope];
        }
        return $ret;
    }

Note that isset() is better than array_key_exists() because it is significantly faster and we know that the values can't be NULL.

In createMergeFields($scopeId, $scope) there should be a break after alreadyExists is set to true:

                    $alreadyExists = false;
                    $created = false;
                    foreach ($mailchimpFields['merge_fields'] as $mailchimpField) {
                        if ($mailchimpField['tag'] == $chimpTag || strtoupper($chimpTag) == 'EMAIL') {
                            $alreadyExists = true;
                            break; // add this to prevent looping when we have found what we are looking for
                        }
                    }

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions