-
Notifications
You must be signed in to change notification settings - Fork 93
Description
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
}
}