From 578e4db3773f70f13e0bc76c02f2768c37293528 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 2 Jun 2022 21:13:27 +1200 Subject: [PATCH] Fix flow for groups & tags The new way is - create the tags & groups at the form layer & only the ids go to the parser - which adds them This patch should be a nullop in terms of the efficiency of these --- CRM/Contact/Import/Form/Preview.php | 135 +++++++++------ CRM/Contact/Import/Form/Summary.php | 7 +- CRM/Contact/Import/ImportJob.php | 156 ------------------ CRM/Contact/Import/Parser/Contact.php | 26 +-- CRM/Import/Forms.php | 2 +- CRM/Import/Parser.php | 95 ++++++++++- templates/CRM/Contact/Import/Form/Preview.tpl | 4 +- 7 files changed, 187 insertions(+), 238 deletions(-) diff --git a/CRM/Contact/Import/Form/Preview.php b/CRM/Contact/Import/Form/Preview.php index 8dfe8c6509..37ad7552ec 100644 --- a/CRM/Contact/Import/Form/Preview.php +++ b/CRM/Contact/Import/Form/Preview.php @@ -15,6 +15,9 @@ * @copyright CiviCRM LLC https://civicrm.org/licensing */ +use Civi\Api4\Group; +use Civi\Api4\Tag; + /** * This class previews the uploaded file and returns summary statistics. */ @@ -36,15 +39,6 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview { public function preProcess() { parent::preProcess(); $this->_disableUSPS = $this->getSubmittedValue('disableUSPS'); - - $groups = CRM_Core_PseudoConstant::nestedGroup(); - $this->set('groups', $groups); - - $tag = CRM_Core_PseudoConstant::get('CRM_Core_DAO_EntityTag', 'tag_id', array('onlyActive' => FALSE)); - if ($tag) { - $this->set('tag', $tag); - } - $this->setStatusUrl(); } @@ -63,24 +57,25 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview { ); } - $groups = $this->get('groups'); + $groups = CRM_Core_PseudoConstant::nestedGroup();; if (!empty($groups)) { - $this->addElement('select', 'groups', ts('Add imported records to existing group(s)'), $groups, array( + $this->addElement('select', 'groups', ts('Add imported records to existing group(s)'), $groups, [ 'multiple' => "multiple", 'class' => 'crm-select2', - )); + ]); } //display new tag $this->addElement('text', 'newTagName', ts('Tag'), CRM_Core_DAO::getAttribute('CRM_Core_DAO_Tag', 'name')); $this->addElement('text', 'newTagDesc', ts('Description'), CRM_Core_DAO::getAttribute('CRM_Core_DAO_Tag', 'description')); - $tag = $this->get('tag'); + $tag = CRM_Core_PseudoConstant::get('CRM_Core_DAO_EntityTag', 'tag_id', ['onlyActive' => FALSE]); if (!empty($tag)) { - foreach ($tag as $tagID => $tagName) { - $this->addElement('checkbox', "tag[$tagID]", NULL, $tagName); - } + $this->addElement('select', 'tag', ts(' Tag imported records'), $tag, [ + 'multiple' => 'multiple', + 'class' => 'crm-select2', + ]); } $this->addFormRule(array('CRM_Contact_Import_Form_Preview', 'formRule'), $this); @@ -142,37 +137,71 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview { /** * Process the mapped fields and map it into the uploaded file. * - * @throws \API_Exception + * @throws \API_Exception|\CRM_Core_Exception */ - public function postProcess() { - - $importJobParams = array( - 'doGeocodeAddress' => $this->getSubmittedValue('doGeocodeAddress'), - 'invalidRowCount' => $this->getRowCount(CRM_Import_Parser::ERROR), - 'onDuplicate' => $this->getSubmittedValue('onDuplicate'), - 'dedupe' => $this->getSubmittedValue('dedupe_rule_id'), - 'newGroupName' => $this->controller->exportValue($this->_name, 'newGroupName'), - 'newGroupDesc' => $this->controller->exportValue($this->_name, 'newGroupDesc'), - 'newGroupType' => $this->controller->exportValue($this->_name, 'newGroupType'), - 'groups' => $this->controller->exportValue($this->_name, 'groups'), - 'allGroups' => $this->get('groups'), - 'newTagName' => $this->controller->exportValue($this->_name, 'newTagName'), - 'newTagDesc' => $this->controller->exportValue($this->_name, 'newTagDesc'), - 'tag' => $this->controller->exportValue($this->_name, 'tag'), - 'allTags' => $this->get('tag'), - 'mapper' => $this->controller->exportValue('MapField', 'mapper'), - 'mapFields' => $this->getAvailableFields(), - 'contactType' => $this->getContactType(), - 'contactSubType' => $this->getSubmittedValue('contactSubType'), - 'primaryKeyName' => '_id', - 'statusFieldName' => '_status', - 'statusID' => $this->get('statusID'), - 'totalRowCount' => $this->getRowCount([]), - 'userJobID' => $this->getUserJobID(), - ); + public function postProcess(): void { + $groupsToAddTo = (array) $this->getSubmittedValue('groups'); + $summaryInfo = ['groups' => [], 'tags' => []]; + foreach ($groupsToAddTo as $groupID) { + // This is a convenience for now - really url & name should be determined at + // presentation stage - ie the summary screen. The only info we are really + // preserving is which groups were created vs already existed. + $summaryInfo['groups'][$groupID] = [ + 'url' => CRM_Utils_System::url('civicrm/group/search', 'reset=1&force=1&context=smog&gid=' . $groupID), + 'name' => Group::get(FALSE)->addWhere('id', '=', $groupID)->addSelect('name')->execute()->first()['name'], + 'new' => FALSE, + 'added' => 0, + 'notAdded' => 0, + ]; + } - $importJob = new CRM_Contact_Import_ImportJob(); - $importJob->setJobParams($importJobParams); + if ($this->getSubmittedValue('newGroupName')) { + /* Create a new group */ + $groupsToAddTo[] = $groupID = Group::create(FALSE)->setValues([ + 'title' => $this->getSubmittedValue('newGroupName'), + 'description' => $this->getSubmittedValue('newGroupDesc'), + 'group_type' => $this->getSubmittedValue('newGroupType') ?? [], + 'is_active' => TRUE, + ])->execute()->first()['id']; + $summaryInfo['groups'][$groupID] = [ + 'url' => CRM_Utils_System::url('civicrm/group/search', 'reset=1&force=1&context=smog&gid=' . $groupID), + 'name' => $this->getSubmittedValue('newGroupName'), + 'new' => TRUE, + 'added' => 0, + 'notAdded' => 0, + ]; + } + $tagsToAdd = (array) $this->getSubmittedValue('tag'); + foreach ($tagsToAdd as $tagID) { + // This is a convenience for now - really url & name should be determined at + // presentation stage - ie the summary screen. The only info we are really + // preserving is which tags were created vs already existed. + $summaryInfo['tags'][$tagID] = [ + 'url' => CRM_Utils_System::url('civicrm/contact/search', 'reset=1&force=1&context=smog&id=' . $tagID), + 'name' => Tag::get(FALSE)->addWhere('id', '=', $tagID)->addSelect('name')->execute()->first()['name'], + 'new' => TRUE, + 'added' => 0, + 'notAdded' => 0, + ]; + } + if ($this->getSubmittedValue('newTagName')) { + $tagsToAdd[] = $tagID = Tag::create(FALSE)->setValues([ + 'name' => $this->getSubmittedValue('newTagName'), + 'description' => $this->getSubmittedValue('newTagDesc'), + 'is_selectable' => TRUE, + 'used_for' => 'civicrm_contact', + ])->execute()->first()['id']; + $summaryInfo['tags'][$tagID] = [ + 'url' => CRM_Utils_System::url('civicrm/contact/search', 'reset=1&force=1&context=smog&id=' . $tagID), + 'name' => $this->getSubmittedValue('newTagName'), + 'new' => FALSE, + 'added' => 0, + 'notAdded' => 0, + ]; + } + // Store the actions to take on each row & the data to present at the end to the userJob. + $this->updateUserJobMetadata('post_actions', ['group' => $groupsToAddTo, 'tag' => $tagsToAdd]); + $this->updateUserJobMetadata('summary_info', $summaryInfo); // If ACL applies to the current user, update cache before running the import. if (!CRM_Core_Permission::check('view all contacts')) { @@ -184,16 +213,20 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview { CRM_Utils_Address_USPS::disable($this->_disableUSPS); // run the import - $importJob->runImport($this); + + $this->_parser = $this->getParser(); + $this->_parser->run( + [], + CRM_Import_Parser::MODE_IMPORT, + $this->get('statusID') + ); // Clear all caches, forcing any searches to recheck the ACLs or group membership as the import // may have changed it. CRM_Contact_BAO_Contact_Utils::clearContactCaches(TRUE); - // add all the necessary variables to the form - $importJob->setFormVariables($this); - // check if there is any error occurred + // @todo - it's really unclear that this error code should still exist... $errorStack = CRM_Core_Error::singleton(); $errors = $errorStack->getErrors(); $errorMessage = []; @@ -214,10 +247,6 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview { $this->set('errorFile', $errorFile); } - - //hack to clean db - //if job complete drop table. - $importJob->isComplete(); } /** diff --git a/CRM/Contact/Import/Form/Summary.php b/CRM/Contact/Import/Form/Summary.php index d4a4655af7..33d02af4c6 100644 --- a/CRM/Contact/Import/Form/Summary.php +++ b/CRM/Contact/Import/Form/Summary.php @@ -27,7 +27,8 @@ class CRM_Contact_Import_Form_Summary extends CRM_Import_Form_Summary { * @throws \CRM_Core_Exception */ public function preProcess() { - // set the error message path to display + // @todo - totally unclear that this errorFile could ever be set / render. + // Probably it can go. $this->assign('errorFile', $this->get('errorFile')); $onDuplicate = $this->getSubmittedValue('onDuplicate'); $this->assign('dupeError', FALSE); @@ -44,8 +45,8 @@ class CRM_Contact_Import_Form_Summary extends CRM_Import_Form_Summary { $this->assign('dupeError', TRUE); } - $this->assign('groupAdditions', $this->get('groupAdditions')); - $this->assign('tagAdditions', $this->get('tagAdditions')); + $this->assign('groupAdditions', $this->getUserJob()['metadata']['summary_info']['groups']); + $this->assign('tagAdditions', $this->getUserJob()['metadata']['summary_info']['tags']); $this->assign('totalRowCount', $this->getRowCount()); $this->assign('validRowCount', $this->getRowCount(CRM_Import_Parser::VALID) + $this->getRowCount(CRM_Import_Parser::UNPARSED_ADDRESS_WARNING)); $this->assign('invalidRowCount', $this->getRowCount(CRM_Import_Parser::ERROR)); diff --git a/CRM/Contact/Import/ImportJob.php b/CRM/Contact/Import/ImportJob.php index 035f6abe4d..a61ec18990 100644 --- a/CRM/Contact/Import/ImportJob.php +++ b/CRM/Contact/Import/ImportJob.php @@ -23,14 +23,10 @@ class CRM_Contact_Import_ImportJob { protected $_onDuplicate; protected $_dedupe; protected $_newGroupName; - protected $_newGroupDesc; - protected $_newGroupType; protected $_groups; protected $_allGroups; protected $_newTagName; - protected $_newTagDesc; protected $_tag; - protected $_allTags; protected $_mapper; protected $_mapperKeys = []; @@ -72,44 +68,6 @@ class CRM_Contact_Import_ImportJob { $this->_mapperKeys[$key] = $mapper[$key][0] ?? NULL; } - $this->_parser = new CRM_Contact_Import_Parser_Contact( - $this->_mapperKeys - ); - $this->_parser->setUserJobID($this->_userJobID); - $this->_parser->run( - [], - CRM_Import_Parser::MODE_IMPORT, - $this->_statusID - ); - - $contactIds = $this->_parser->getImportedContacts(); - - //get the related contactIds. CRM-2926 - $relatedContactIds = $this->_parser->getRelatedImportedContacts(); - if ($relatedContactIds) { - $contactIds = array_merge($contactIds, $relatedContactIds); - } - - if ($this->_newGroupName || count($this->_groups)) { - $groupAdditions = $this->_addImportedContactsToNewGroup($contactIds, - $this->_newGroupName, - $this->_newGroupDesc, - $this->_newGroupType - ); - if ($form) { - $form->set('groupAdditions', $groupAdditions); - } - } - - if ($this->_newTagName || !empty($this->_tag)) { - $tagAdditions = $this->_tagImportedContactsWithNewTag($contactIds, - $this->_newTagName, - $this->_newTagDesc - ); - if ($form) { - $form->set('tagAdditions', $tagAdditions); - } - } } /** @@ -119,118 +77,4 @@ class CRM_Contact_Import_ImportJob { $this->_parser->set($form, CRM_Import_Parser::MODE_IMPORT); } - /** - * Add imported contacts. - * - * @param array $contactIds - * @param string $newGroupName - * @param string $newGroupDesc - * @param string $newGroupType - * - * @return array|bool - */ - private function _addImportedContactsToNewGroup( - $contactIds, - $newGroupName, $newGroupDesc, $newGroupType - ) { - - $newGroupId = NULL; - - if ($newGroupName) { - /* Create a new group */ - $newGroupType = $newGroupType ?? []; - $gParams = array( - 'title' => $newGroupName, - 'description' => $newGroupDesc, - 'group_type' => $newGroupType, - 'is_active' => TRUE, - ); - $group = CRM_Contact_BAO_Group::create($gParams); - $this->_groups[] = $newGroupId = $group->id; - } - - if (is_array($this->_groups)) { - $groupAdditions = []; - foreach ($this->_groups as $groupId) { - $addCount = CRM_Contact_BAO_GroupContact::addContactsToGroup($contactIds, $groupId); - $totalCount = $addCount[1]; - if ($groupId == $newGroupId) { - $name = $newGroupName; - $new = TRUE; - } - else { - $name = $this->_allGroups[$groupId]; - $new = FALSE; - } - $groupAdditions[] = array( - 'url' => CRM_Utils_System::url('civicrm/group/search', - 'reset=1&force=1&context=smog&gid=' . $groupId - ), - 'name' => $name, - 'added' => $totalCount, - 'notAdded' => $addCount[2], - 'new' => $new, - ); - } - return $groupAdditions; - } - return FALSE; - } - - /** - * @param $contactIds - * @param string $newTagName - * @param $newTagDesc - * - * @return array|bool - * @throws \CRM_Core_Exception - */ - private function _tagImportedContactsWithNewTag( - $contactIds, - $newTagName, $newTagDesc - ) { - - $newTagId = NULL; - if ($newTagName) { - /* Create a new Tag */ - - $tagParams = array( - 'name' => $newTagName, - 'description' => $newTagDesc, - 'is_selectable' => TRUE, - 'used_for' => 'civicrm_contact', - ); - $addedTag = CRM_Core_BAO_Tag::add($tagParams); - $this->_tag[$addedTag->id] = 1; - } - //add Tag to Import - - if (is_array($this->_tag)) { - $tagAdditions = []; - foreach ($this->_tag as $tagId => $val) { - $addTagCount = CRM_Core_BAO_EntityTag::addEntitiesToTag($contactIds, $tagId, 'civicrm_contact', FALSE); - $totalTagCount = $addTagCount[1]; - if (isset($addedTag) && $tagId == $addedTag->id) { - $tagName = $newTagName; - $new = TRUE; - } - else { - $tagName = $this->_allTags[$tagId]; - $new = FALSE; - } - $tagAdditions[] = array( - 'url' => CRM_Utils_System::url('civicrm/contact/search', - 'reset=1&force=1&context=smog&id=' . $tagId - ), - 'name' => $tagName, - 'added' => $totalTagCount, - 'notAdded' => $addTagCount[2], - 'new' => $new, - ); - } - return $tagAdditions; - } - return FALSE; - } - } diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 5092ac1da6..8bfdd3fe1d 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -209,6 +209,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { */ public function import($onDuplicate, &$values) { $rowNumber = (int) $values[array_key_last($values)]; + $this->_unparsedStreetAddressContacts = []; if (!$this->getSubmittedValue('doGeocodeAddress')) { // CRM-5854, reset the geocode method to null to prevent geocoding @@ -251,11 +252,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { //fixed CRM-4148 //now we create new contact in update/fill mode also. $newContact = $this->createContact($formatted, $contactFields, $onDuplicate, $params['id'] ?? NULL, TRUE, $this->_dedupeRuleGroupID); - - if (!is_array($newContact)) { - $contactID = $newContact->id; - $this->_newContacts[] = $contactID; - } + $this->createdContacts[$newContact->id] = $contactID = $newContact->id; if ($contactID) { // call import hook @@ -296,18 +293,17 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if (empty($formatting['id']) || $this->isUpdateExistingContacts()) { try { $relatedNewContact = $this->createContact($formatting, $contactFields, $onDuplicate, $formatting['id']); + $relContactId = $relatedNewContact->id; + $this->createdContacts[$relContactId] = $relContactId; } catch (CiviCRM_API3_Exception $e) { $this->setImportStatus($rowNumber, 'ERROR', $e->getMessage()); return FALSE; } - $relContactId = $relatedNewContact->id; - $this->_newRelatedContacts[$relContactId] = $relContactId; } $this->createRelationship($key, $relContactId, $primaryContactId); } } - $this->setImportStatus($rowNumber, $this->getStatus(CRM_Import_Parser::VALID), $this->getSuccessMessage(), $contactID); return CRM_Import_Parser::VALID; } @@ -569,16 +565,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { * @return array */ public function getImportedContacts() { - return $this->_newContacts; - } - - /** - * Get the array of successfully imported related contact id's - * - * @return array - */ - public function &getRelatedImportedContacts() { - return $this->_newRelatedContacts; + return $this->createdContacts; } /** @@ -1256,6 +1243,8 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { // here to make sure it is instantiated. $this->getContactType(); $this->getContactSubType(); + // Reset user job in case to null in case it was loaded prior to the job being complete. + $this->userJob = NULL; $this->init(); @@ -1290,6 +1279,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { $prevTimestamp = $this->progressImport($statusID, FALSE, $startTimestamp, $prevTimestamp, $totalRowCount); } } + $this->doPostImportActions(); } /** diff --git a/CRM/Import/Forms.php b/CRM/Import/Forms.php index 1512bfc6f1..c3f3646002 100644 --- a/CRM/Import/Forms.php +++ b/CRM/Import/Forms.php @@ -599,7 +599,7 @@ class CRM_Import_Forms extends CRM_Core_Form { $this->_dataValues = array_values($this->getDataRows([], 2)); $this->assign('columnNames', $this->getColumnHeaders()); $this->assign('highlightedFields', $this->getHighlightedFields()); - $this->assign('columnCount', $this->_columnCount ); + $this->assign('columnCount', $this->_columnCount); $this->assign('dataValues', $this->_dataValues); } diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php index 7636bd1e93..1c2fe91fb0 100644 --- a/CRM/Import/Parser.php +++ b/CRM/Import/Parser.php @@ -53,6 +53,13 @@ abstract class CRM_Import_Parser { */ protected $userJobID; + /** + * The user job in use. + * + * @var array + */ + protected $userJob; + /** * Potentially ambiguous options. * @@ -76,6 +83,13 @@ abstract class CRM_Import_Parser { return $this->userJobID; } + /** + * Ids of contacts created this iteration. + * + * @var array + */ + protected $createdContacts = []; + /** * Set user job ID. * @@ -105,10 +119,13 @@ abstract class CRM_Import_Parser { * @throws \API_Exception */ protected function getUserJob(): array { - return UserJob::get() - ->addWhere('id', '=', $this->getUserJobID()) - ->execute() - ->first(); + if (empty($this->userJob)) { + $this->userJob = UserJob::get() + ->addWhere('id', '=', $this->getUserJobID()) + ->execute() + ->first(); + } + return $this->userJob; } /** @@ -606,6 +623,76 @@ abstract class CRM_Import_Parser { $this->validateRequiredFields($requiredFields, $params, $prefixString); } + protected function doPostImportActions() { + $userJob = $this->getUserJob(); + $summaryInfo = $userJob['metadata']['summary_info']; + $actions = $userJob['metadata']['post_actions']; + if (!empty($actions['group'])) { + $groupAdditions = $this->addImportedContactsToNewGroup($this->createdContacts, $actions['group']); + foreach ($actions['group'] as $groupID) { + $summaryInfo['groups'][$groupID]['added'] += $groupAdditions[$groupID]['added']; + $summaryInfo['groups'][$groupID]['notAdded'] += $groupAdditions[$groupID]['notAdded']; + } + } + if (!empty($actions['tag'])) { + $tagAdditions = $this->tagImportedContactsWithNewTag($this->createdContacts, $actions['tag']); + foreach ($actions['tag'] as $tagID) { + $summaryInfo['tags'][$tagID]['added'] += $tagAdditions[$tagID]['added']; + $summaryInfo['tags'][$tagID]['notAdded'] += $tagAdditions[$tagID]['notAdded']; + } + } + + $this->userJob['metadata']['summary_info'] = $summaryInfo; + UserJob::update(FALSE)->addWhere('id', '=', $userJob['id'])->setValues(['metadata' => $this->userJob['metadata']])->execute(); + } + + /** + * Add imported contacts to groups. + * + * @param array $contactIDs + * @param array $groups + * + * @return array + */ + private function addImportedContactsToNewGroup(array $contactIDs, array $groups): array { + $groupAdditions = []; + foreach ($groups as $groupID) { + // @todo - this function has been in use historically but it does not seem + // to add much efficiency of get + create api calls + // and it doesn't give enough control over cache flushing for smaller batches. + // Note that the import updates a lot of enities & checking & updating the group + // shouldn't add much performance wise. However, cache flushing will + $addCount = CRM_Contact_BAO_GroupContact::addContactsToGroup($contactIDs, $groupID); + $groupAdditions[$groupID] = [ + 'added' => (int) $addCount[1], + 'notAdded' => (int) $addCount[2], + ]; + } + return $groupAdditions; + } + + /** + * Tag imported contacts. + * + * @param array $contactIDs + * @param array $tags + * + * @return array + */ + private function tagImportedContactsWithNewTag(array $contactIDs, array $tags) { + $tagAdditions = []; + foreach ($tags as $tagID) { + // @todo - this function has been in use historically but it does not seem + // to add much efficiency of get + create api calls + // and it doesn't give enough control over cache flushing for smaller batches. + // Note that the import updates a lot of enities & checking & updating the group + // shouldn't add much performance wise. However, cache flushing will + $outcome = CRM_Core_BAO_EntityTag::addEntitiesToTag($contactIDs, $tagID, 'civicrm_contact', FALSE); + $tagAdditions[$tagID] = ['added' => $outcome[1], 'notAdded' => $outcome[2]]; + } + return $tagAdditions; + } + /** * Determines the file extension based on error code. * diff --git a/templates/CRM/Contact/Import/Form/Preview.tpl b/templates/CRM/Contact/Import/Form/Preview.tpl index 503d561a5f..77b28e9b6f 100644 --- a/templates/CRM/Contact/Import/Form/Preview.tpl +++ b/templates/CRM/Contact/Import/Form/Preview.tpl @@ -128,9 +128,7 @@
- {foreach from=$form.tag item="tag_val"} -
{$tag_val.html}
- {/foreach} + {$form.tag.html}
-- 2.25.1