Finish location block logic refactoring
authorJKingsnorth <john@johnkingsnorth.co.uk>
Tue, 24 Nov 2015 13:07:38 +0000 (13:07 +0000)
committerJKingsnorth <john@johnkingsnorth.co.uk>
Tue, 24 Nov 2015 13:07:38 +0000 (13:07 +0000)
CRM/Contact/Form/Merge.php
CRM/Dedupe/Merger.php
templates/CRM/Contact/Form/Merge.tpl

index 183225b1796ce5c19a7d482b6228c53ba149b5dd..059b79f29926eb3ea45690897b984c00626a0252 100644 (file)
@@ -44,9 +44,6 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form {
 
   var $_contactType = NULL;
 
-  // variable to keep all location block ids.
-  protected $_locBlockIds = array();
-
   // FIXME: QuickForm can't create advcheckboxes with value set to 0 or '0' :(
   // see HTML_QuickForm_advcheckbox::setValues() - but patching that doesn't
   // help, as QF doesn't put the 0-value elements in exportValues() anyway...
@@ -214,11 +211,6 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form {
     $this->assign('mainLocBlock', json_encode($rowsElementsAndInfo['main_loc_block']));
     $this->assign('rows', $rowsElementsAndInfo['rows']);
 
-    $this->_locBlockIds = array(
-      'main' => $rowsElementsAndInfo['main_details']['loc_block_ids'],
-      'other' => $rowsElementsAndInfo['other_details']['loc_block_ids'],
-    );
-
     // add elements
     foreach ($rowsElementsAndInfo['elements'] as $element) {
       $this->addElement($element[0],
index f76f245b188ef29ccf1b496a25e5c75834a4d870..f4ae9d851d92260f09aa04d199b48e1ebbf97945 100644 (file)
@@ -900,6 +900,53 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     return FALSE;
   }
 
+  /**
+   * A function to build an array of information about location blocks that is
+   * required when merging location fields
+   *
+   * @return array
+   */
+  public static function getLocationBlockInfo() {
+    $locationBlocks = array(
+      'address' => array(
+        'label' => 'Address',
+        'displayField' => 'display',
+        'sortString' => 'location_type_id',
+        'hasLocation' => TRUE,
+        'hasType' => FALSE,
+      ),
+      'email' => array(
+        'label' => 'Email',
+        'displayField' => 'email',
+        'sortString' => 'location_type_id',
+        'hasLocation' => TRUE,
+        'hasType' => FALSE,
+      ),
+      'im' => array(
+        'label' => 'IM',
+        'displayField' => 'name',
+        'sortString' => 'location_type_id,provider_id',
+        'hasLocation' => TRUE,
+        'hasType' => 'provider_id',
+      ),
+      'phone' => array(
+        'label' => 'Phone',
+        'displayField' => 'phone',
+        'sortString' => 'location_type_id,phone_type_id',
+        'hasLocation' => TRUE,
+        'hasType' => 'phone_type_id',
+      ),
+      'website' => array(
+        'label' => 'Website',
+        'displayField' => 'url',
+        'sortString' => 'website_type_id',
+        'hasLocation' => FALSE,
+        'hasType' => 'website_type_id',
+      ),
+    );
+    return $locationBlocks;
+  }
+
   /**
    * A function to build an array of information required by merge function and the merge UI.
    *
@@ -1056,43 +1103,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     // @todo OpenID not in API yet, so is not supported here.
 
     // Set up useful information about the location blocks
-    $locationBlocks = array(
-      'address' => array(
-        'label' => 'Address',
-        'displayField' => 'display',
-        'sortString' => 'location_type_id',
-        'hasLocation' => TRUE,
-        'hasType' => FALSE,
-      ),
-      'email' => array(
-        'label' => 'Email',
-        'displayField' => 'email',
-        'sortString' => 'location_type_id',
-        'hasLocation' => TRUE,
-        'hasType' => FALSE,
-      ),
-      'im' => array(
-        'label' => 'IM',
-        'displayField' => 'name',
-        'sortString' => 'location_type_id,provider_id',
-        'hasLocation' => TRUE,
-        'hasType' => 'provider_id',
-      ),
-      'phone' => array(
-        'label' => 'Phone',
-        'displayField' => 'phone',
-        'sortString' => 'location_type_id,phone_type_id',
-        'hasLocation' => TRUE,
-        'hasType' => 'phone_type_id',
-      ),
-      'website' => array(
-        'label' => 'Website',
-        'displayField' => 'url',
-        'sortString' => 'website_type_id',
-        'hasLocation' => FALSE,
-        'hasType' => 'website_type_id',
-      ),
-    );
+    $locationBlocks = self::getLocationBlockInfo();
 
     $locations = array(
       'main' => array(),
@@ -1128,6 +1139,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
           foreach ($values['values'] as $index => $value) {
             $locations[$moniker][$blockName][$cnt] = $value;
             // Fix address display
+            $display = '';
             if ($blockName == 'address') {
               CRM_Core_BAO_Address::fixAddress($value);
               $display = CRM_Utils_Address::format($value);
@@ -1135,12 +1147,28 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
             }
 
             // Add any 'main' contact block values to an array for the JS
+            // @todo The JS should just access the main_details to find this info?
             if ($moniker == 'main') {
               if ($blockInfo['hasType']) {
-                $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id'] . "_" . $value[$blockInfo['hasType']] = $value[$blockInfo['displayField']];
+                // Handle websites, no location type ID
+                // @todo Remove the need for this specific 'if'
+                if ($blockName == 'website') {
+                  $value['location_type_id'] = 0;
+                }
+                $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id'] . "_" . $value[$blockInfo['hasType']]]['display'] = $value[$blockInfo['displayField']];
+                $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id'] . "_" . $value[$blockInfo['hasType']]]['id'] = $value['id'];
               }
               else {
-                $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id'] = $value[$blockInfo['displayField']];
+                // Get the correct display value for addresses
+                // @todo Remove the need for this if...
+                if ($blockName == 'address') {
+                  $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id']]['display'] = $display;
+                  $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id']]['id'] = $value['id'];
+                }
+                else {
+                  $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id']]['display'] = $value[$blockInfo['displayField']];
+                  $mainLocBlock["main_" . $blockName . "_" . $value['location_type_id']]['id'] = $value['id'];
+                }
               }
             }
 
@@ -1151,8 +1179,8 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
 
       // Now, build the table rows appropriately, based off the information on
       // the 'other' contact
-      if (!empty($locations['other'])) {
-        foreach ($locations['other'] as $count => $value) {
+      if (!empty($locations['other']) && !empty($locations['other'][$blockName])) {
+        foreach ($locations['other'][$blockName] as $count => $value) {
 
           $displayValue = $value[$blockInfo['displayField']];
 
@@ -1172,6 +1200,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
             $lookupType = $value[$blockInfo['hasType']];
           }
 
+          // Hold ID of main contact's matching block
+          $mainContactBlockId = 0;
+
           if (!empty($locations['main'][$blockName])) {
             foreach ($locations['main'][$blockName] as $mainValueCheck) {
               // No location/type, or matching location and type
@@ -1179,8 +1210,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
                 (empty($lookupLocation) || $lookupLocation == $mainValueCheck['location_type_id'])
                 && (empty($lookupType) || $lookupType == $mainValueCheck[$blockInfo['hasType']])
               ) {
-                // Set this value against the 'other' contact value
+                // Set this value as the default against the 'other' contact value
                 $rows["move_location_{$blockName}_{$count}"]['main'] = $mainValueCheck[$blockInfo['displayField']];
+                $mainContactBlockId = $mainValueCheck['id'];
                 break;
               }
             }
@@ -1192,6 +1224,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
           // Flag up this field to skipMerge function (@todo: do we need to?)
           $migrationInfo["move_location_{$blockName}_{$count}"] = 1;
 
+          // Add a hidden field to store the ID of the target main contact block
+          $elements[] = array('hidden', "location[$blockName][$count][mainContactBlockId]", $mainContactBlockId);
+
           // Setup variables
           $thisTypeId = FALSE;
           $thisLocId = FALSE;
@@ -1266,30 +1301,25 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
             );
 
             // Add the information to the migrationInfo (@todo Why?)
-            $migrationInfo['location_blocks'][$blockName][$count]['typeTypeId'] = $typeTypeId;
+            $migrationInfo['location_blocks'][$blockName][$count]['typeTypeId'] = $thisTypeId;
 
           }
 
           // Set the label for this row
-          // @todo Reformat for website, empty brackets are lazy and ugly
-          $rows["move_location_{$blockName}_$count"]['title'] = ts('%1 %2 (%3) (%4)',
-            array(
-              1 => $blockInfo['label'],
-              2 => $count,
-              3 => $thisLocId ? $listOptions['location_type_id'][$thisLocId] : '';
-              4 => $thisTypeId ? $listOptions[$blockInfo['hasType']][$thisTypeId] : '';
-            ));
-
-          // End loop through 'other' locations of this type
-        }
+          $rowTitle = $blockInfo['label'] . ' ' . ($count + 1);
+          if (!empty($thisLocId)) {
+            $rowTitle .= ' (' . $listOptions['location_type_id'][$thisLocId] . ')';
+          }
+          if (!empty($thisTypeId)) {
+            $rowTitle .= ' (' . $listOptions[$blockInfo['hasType']][$thisTypeId] . ')';
+          }
+          $rows["move_location_{$blockName}_$count"]['title'] = $rowTitle;
 
-        // End if 'other' location for this type exists
-      }
+        } // End loop through 'other' locations of this type
 
-      // End loop through each location block entity
-    }
+      } // End if 'other' location for this type exists
 
-    // @todo NEW REFACOTRING DONE UP TO HERE
+    } // End loop through each location block entity
 
     // add the related tables and unset the ones that don't sport any of the duplicate contact's info
     $config = CRM_Core_Config::singleton();
@@ -1414,10 +1444,8 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
       'migration_info' => $migrationInfo,
     );
 
-    $result['main_details']['loc_block_ids'] = $locBlockIds['main'];
-    $result['main_details']['other_type_block_ids'] = $typeBlockIds['main'];
-    $result['other_details']['loc_block_ids'] = $locBlockIds['other'];
-    $result['other_details']['other_type_block_ids'] = $typeBlockIds['other'];
+    $result['main_details']['location_blocks'] = $locations['main'];
+    $result['other_details']['location_blocks'] = $locations['other'];
 
     return $result;
   }
@@ -1427,10 +1455,6 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    * other contact to the main one - be it Location / CustomFields or Contact .. related info.
    * A superset of moveContactBelongings() function.
    *
-   * @todo
-   *   Websites do not have a location type. This needs to be supported:
-   *   Add move_type_website_1, etc.
-   *
    * @param int $mainId
    *   Main contact with whom merge has to happen.
    * @param int $otherId
@@ -1458,14 +1482,19 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
       ) {
         $submitted[substr($key, 5)] = $value;
       }
+
+      // Set up initial information for handling migration of location blocks
       elseif (substr($key, 0, 14) == 'move_location_' and $value != NULL) {
         $locField = explode('_', $key);
         $fieldName = $locField[2];
         $fieldCount = $locField[3];
+
+        // Set up the operation type (add/overwrite)
         // Ignore operation for websites
+        // @todo Tidy this up
         $operation = 0;
         if ($fieldName != 'website') {
-          $operation = CRM_Utils_Array::value('operation', $migrationInfo['location_blocks'][$fieldName][$fieldCount]);
+          $operation = CRM_Utils_Array::value('operation', $migrationInfo['location'][$fieldName][$fieldCount]);
         }
         // default operation is overwrite.
         if (!$operation) {
@@ -1473,11 +1502,8 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
         }
         $locBlocks[$fieldName][$fieldCount]['operation'] = $operation;
 
-        // Ignore 'locTypeId' for websites
-        if ($fieldName != 'website') {
-          $locBlocks[$fieldName][$fieldCount]['locTypeId'] = CRM_Utils_Array::value('locTypeId', $migrationInfo['location_blocks'][$fieldName][$fieldCount]);
-        }
       }
+
       elseif (substr($key, 0, 15) == 'move_rel_table_' and $value == '1') {
         $moveTables = array_merge($moveTables, $relTables[substr($key, 5)]['tables']);
         if (array_key_exists('operation', $migrationInfo)) {
@@ -1494,13 +1520,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     // @todo Handle OpenID (not currently in API).
     if (!empty($locBlocks)) {
 
-      $locationBlocks = array(
-        'address' => 'Address',
-        'email' => 'Email',
-        'im' => 'IM',
-        'phone' => 'Phone',
-        'website' => 'Website',
-      );
+      $locationBlocks = self::getLocationBlockInfo();
 
       $primaryBlockIds = CRM_Contact_BAO_Contact::getLocBlockIds($mainId, array('is_primary' => 1));
       $billingBlockIds = CRM_Contact_BAO_Contact::getLocBlockIds($mainId, array('is_billing' => 1));
@@ -1509,55 +1529,36 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
         if (!is_array($block) || CRM_Utils_System::isNull($block)) {
           continue;
         }
-        $daoName = 'CRM_Core_DAO_' . $locationBlocks[$name];
+        $daoName = 'CRM_Core_DAO_' . $locationBlocks[$name]['label'];
         $primaryDAOId = (array_key_exists($name, $primaryBlockIds)) ? array_pop($primaryBlockIds[$name]) : NULL;
         $billingDAOId = (array_key_exists($name, $billingBlockIds)) ? array_pop($billingBlockIds[$name]) : NULL;
 
         foreach ($block as $blkCount => $values) {
-          $locTypeId = CRM_Utils_Array::value('locTypeId', $values, 1);
-          $operation = CRM_Utils_Array::value('operation', $values, 2);
-          $otherBlockId = CRM_Utils_Array::value($blkCount, $migrationInfo['other_details']['loc_block_ids'][$name]);
-
-          // keep 1-1 mapping for address - loc type.
-          $idKey = $blkCount;
-          if (
-            $name != 'website'
-            && array_key_exists($name, $locationBlocks)
-          ) {
-            $idKey = $locTypeId;
-          }
 
-          if (isset($migrationInfo['main_details']['loc_block_ids'][$name])) {
-            $mainBlockId = CRM_Utils_Array::value($idKey, $migrationInfo['main_details']['loc_block_ids'][$name]);
-          }
+          // For the block which belongs to other-contact, link the location block to main-contact
+          $otherBlockDAO = new $daoName();
+          $otherBlockDAO->contact_id = $mainId;
 
+          // Get the ID of this block on the 'other' contact, otherwise skip
+          $otherBlockId = CRM_Utils_Array::value('id', $migrationInfo['other_details']['location_blocks'][$name][$blkCount]);
           if (!$otherBlockId) {
             continue;
           }
-
-          // for the block which belongs to other-contact, link the location block to main-contact
-          $otherBlockDAO = new $daoName();
           $otherBlockDAO->id = $otherBlockId;
-          $otherBlockDAO->contact_id = $mainId;
-          $otherBlockDAO->location_type_id = $locTypeId;
 
-          // Get typeTypeIDs for fields that have them
-          if ($name == 'im') {
-            $typeTypeId = $migrationInfo['location_blocks'][$name][$idKey]['typeTypeId'];
-            $otherBlockDAO->provider_id = $typeTypeId;
+          // Add/update location and type information from the form, if applicable
+          if ($locationBlocks[$name]['hasLocation']) {
+            $locTypeId = CRM_Utils_Array::value('locTypeId', $migrationInfo['location'][$name][$blkCount]);
+            $otherBlockDAO->location_type_id = $locTypeId;
           }
-          elseif ($name == 'phone') {
-            $typeTypeId = $migrationInfo['location_blocks'][$name][$idKey]['typeTypeId'];
-            $otherBlockDAO->phone_type_id = $typeTypeId;
-          }
-          elseif ($name == 'website') {
-            $typeTypeId = $migrationInfo['location_blocks'][$name][$idKey]['typeTypeId'];
-            $otherBlockDAO->website_type_id = $typeTypeId;
-            // No location ID, more special handling
-            $otherBlockDAO->location_type_id = NULL;
-            $mainBlockId = CRM_Utils_Array::value($typeTypeId, $migrationInfo['main_details']['other_type_block_ids'][$name]);
+          if ($locationBlocks[$name]['hasType']) {
+            $typeTypeId = CRM_Utils_Array::value('typeTypeId', $migrationInfo['location'][$name][$blkCount]);
+            $otherBlockDAO->{$locationBlocks[$name]['hasType']} = $typeTypeId;
           }
 
+          // Get main block ID
+          $mainBlockId = CRM_Utils_Array::value('mainContactBlockId', $migrationInfo['location'][$name][$blkCount], 0);
+
           // if main contact already has primary & billing, set the flags to 0.
           if ($primaryDAOId) {
             $otherBlockDAO->is_primary = 0;
@@ -1566,8 +1567,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
             $otherBlockDAO->is_billing = 0;
           }
 
+          $operation = CRM_Utils_Array::value('operation', $values, 2);
           // overwrite - need to delete block which belongs to main-contact.
-          if (isset($mainBlockId) && $mainBlockId && ($operation == 2)) {
+          if (!empty($mainBlockId) && ($operation == 2)) {
             $deleteDAO = new $daoName();
             $deleteDAO->id = $mainBlockId;
             $deleteDAO->find(TRUE);
index 723cf88091dc6d7e587886214151b63a3273b8ba..8d80f2196842a501ab0e1a4fc7a70741d99505a6 100644 (file)
@@ -229,22 +229,29 @@ You will need to manually delete that user (click on the link to open Drupal Use
       typeTypeId = element.value;
     }
 
+    // @todo Fix this 'special handling' for websites (no location id)
+    if (!locTypeId) { locTypeId = 0; }
+
     // Get the matching block, based on location and type, from the main contact record
-    if (!typeTypeId) {
-      var block = eval( "allBlock.main_" + blockname + "_" + locTypeId);
-    }
-    else {
-      // @todo Fix this 'special handling' for websites (no location id)
-      if (!locTypeId) { locTypeId = 0; }
-      var block = eval( "allBlock.main_" + blockname + "_" + locTypeId + "_" + typeTypeId);
+    var blockQuery = "allBlock.main_" + blockname + "_" + locTypeId;
+    if (typeTypeId) {
+      blockQuery += "_" + typeTypeId;
     }
+    var block = eval( blockQuery );
+    var mainBlockId = 0;
+    var mainBlockDisplay = '';
 
     // Create appropriate label / add new link after changing the block
-    if (!block) {
-      block = '';
+    if (typeof block == 'undefined') {
       label = '<span class="action_label">(add)</span>';
     }
     else {
+
+      // Set display and ID
+      mainBlockDisplay = block['display'];
+      mainBlockId = block['id'];
+
+      // Set label
       var label = '<span class="action_label">(overwrite)</span> ';
       if (blockname == 'email' || blockname == 'phone') {
         var opLabel = 'location[' + blockname + '][' + blockId + '][operation]';
@@ -253,7 +260,9 @@ You will need to manually delete that user (click on the link to open Drupal Use
       label += '<br>';
     }
 
-    CRM.$( "#main_" + blockname + "_" + blockId ).html( block );
+    // Update DOM
+    CRM.$( "input[name='location[" + blockname + "][" + blockId + "][mainContactBlockId]']" ).val( mainBlockId );
+    CRM.$( "#main_" + blockname + "_" + blockId ).html( mainBlockDisplay );
     CRM.$( "#main_" + blockname + "_" + blockId + "_overwrite" ).html( label );
   }