dev/core#1973 Fix Email & Phone storage issues in event location

CRM/Event/Form/ManageEvent/Location.php

@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Event;
+
 /**
  *
  *
@@ -22,6 +24,11 @@
  */
 class CRM_Event_Form_ManageEvent_Location extends CRM_Event_Form_ManageEvent {
 
+  /**
+   * @var \Civi\Api4\Generic\Result
+   */
+  protected $locationBlock;
+
   /**
    * How many locationBlocks should we display?
    *
@@ -143,9 +150,11 @@ public function buildQuickForm() {
     $this->assign('action', $this->_action);
 
     if ($this->_id) {
-      $this->_oldLocBlockId = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_Event',
-        $this->_id, 'loc_block_id'
-      );
+      $this->locationBlock = Event::get()
+        ->addWhere('id', '=', $this->_id)
+        ->setSelect(['loc_block.*', 'loc_block_id'])
+        ->execute()->first();
+      $this->_oldLocBlockId = $this->locationBlock['loc_block_id'];
     }
 
     // get the list of location blocks being used by other events
@@ -211,7 +220,6 @@ public function postProcess() {
       );
     }
 
-    $this->_values['address'] = $this->_values['phone'] = $this->_values['email'] = [];
     // if 'create new loc' option is selected OR selected new loc is different
     // from old one, go ahead and delete the old loc provided thats not being
     // used by any other event
@@ -223,22 +231,20 @@ public function postProcess() {
     $params['entity_table'] = 'civicrm_event';
     $params['entity_id'] = $this->_id;
 
-    $defaultLocationType = CRM_Core_BAO_LocationType::getDefault();
+    $isUpdateToExistingLocationBlock = !empty($params['loc_event_id']) && (int) $params['loc_event_id'] === $this->locationBlock['loc_block_id'];
+    // It should be impossible for there to be no default location type. Consider removing this handling
+    $defaultLocationTypeID = CRM_Core_BAO_LocationType::getDefault()->id ?? 1;
     foreach ([
-      'address',
-      'phone',
-      'email',
-    ] as $block) {
-      if (empty($params[$block]) || !is_array($params[$block])) {
-        continue;
-      }
-      foreach ($params[$block] as $count => & $values) {
-        if ($count == 1) {
-          $values['is_primary'] = 1;
-        }
-        $values['location_type_id'] = ($defaultLocationType->id) ? $defaultLocationType->id : 1;
-        if (isset($this->_values[$block][$count])) {
-          $values['id'] = $this->_values[$block][$count]['id'];
+      'address' => $params['address'],
+      'phone' => $params['phone'],
+      'email' => $params['email'],
+    ] as $block => $locationEntities) {
+      $params[$block][1]['is_primary'] = 1;
+      foreach ($locationEntities as $index => $locationEntity) {
+        $params[$block][$index]['location_type_id'] = $defaultLocationTypeID;
+        $fieldKey = (int) $index === 1 ? '_id' : '_2_id';
+        if ($isUpdateToExistingLocationBlock && !empty($this->locationBlock['loc_block.' . $block . $fieldKey])) {
+          $params[$block][$index]['id'] = $this->locationBlock['loc_block.' . $block . $fieldKey];
         }
       }
     }

tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php

@@ -40,7 +40,7 @@ public function testBuildForm() {
   public function testSQLSource() {
     $this->callAPISuccess('Mapping', 'create', ['name' => 'Well dressed ducks', 'mapping_type_id' => 'Import Contact']);
     /** @var CRM_Import_DataSource_SQL $form */
-    $form = $this->getFormObject('CRM_Import_DataSource_SQL');
+    $form = $this->getFormObject('CRM_Import_DataSource_SQL', [], 'SQL');
     $coreForm = $this->getFormObject('CRM_Core_Form');
     $db = NULL;
     $params = ['sqlQuery' => 'SELECT 1 as id'];

tests/phpunit/CRM/Event/Form/ManageEvent/LocationTest.php

@@ -0,0 +1,123 @@
+<?php
+
+use Civi\Api4\Event;
+use Civi\Api4\Email;
+
+/**
+ *  Test CRM_Event_Form_Registration functions.
+ *
+ * @package   CiviCRM
+ * @group headless
+ */
+class CRM_Event_Form_ManageEvent_LocationTest extends CiviUnitTestCase {
+
+  /**
+   * Test the right emails exist after submitting the location form twice.
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  public function testSubmit() {
+    $eventID = (int) $this->eventCreate()['id'];
+    $form = $this->getFormObject('CRM_Event_Form_ManageEvent_Location', $this->getFormValues());
+    $form->set('id', $eventID);
+    $form->preProcess();
+    $form->buildQuickForm();
+    $form->postProcess();
+    $this->assertCorrectEmails($eventID);
+
+    // Now do it again to see if it gets messed with.
+    $form = $this->getFormObject('CRM_Event_Form_ManageEvent_Location', array_merge($this->getFormValues(), ['loc_event_id' => $this->ids['LocBlock'][0]]));
+    $form->set('id', $eventID);
+    $form->preProcess();
+    $form->buildQuickForm();
+    $form->postProcess();
+    $this->assertCorrectEmails($eventID);
+  }
+
+  /**
+   * Get the values to submit for the form.
+   *
+   * @return array
+   */
+  protected function getFormValues() {
+    return [
+      'address' =>
+        [
+          1 =>
+            [
+              'master_id' => '',
+              'street_address' => '581O Lincoln Dr SW',
+              'supplemental_address_1' => '',
+              'supplemental_address_2' => '',
+              'supplemental_address_3' => '',
+              'city' => 'Santa Fe',
+              'postal_code' => '87594',
+              'country_id' => '1228',
+              'state_province_id' => '1030',
+              'county_id' => '',
+              'geo_code_1' => '35.5212',
+              'geo_code_2' => '-105.982',
+            ],
+        ],
+      'email' =>
+        [
+          1 =>
+            [
+              'email' => 'celebration@example.org',
+            ],
+          2 =>
+            [
+              'email' => 'bigger_party@example.org',
+            ],
+        ],
+      'phone' =>
+        [
+          1 =>
+            [
+              'phone_type_id' => '1',
+              'phone' => '303 323-1000',
+              'phone_ext' => '',
+            ],
+          2 =>
+            [
+              'phone_type_id' => '1',
+              'phone' => '44',
+              'phone_ext' => '',
+            ],
+        ],
+      'location_option' => '2',
+      'loc_event_id' => '3',
+      'is_show_location' => '1',
+      'is_template' => '0',
+    ];
+  }
+
+  /**
+   * @param int $eventID
+   *
+   * @return \Civi\Api4\Generic\Result
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  protected function assertCorrectEmails($eventID) {
+    $emails = Email::get()
+      ->addWhere('email', 'IN', ['bigger_party@example.org', 'celebration@example.org'])
+      ->addOrderBy('email', 'DESC')
+      ->execute();
+
+    $this->assertCount(2, $emails);
+    $firstEmail = $emails->first();
+    $locationBlock = Event::get()
+      ->addWhere('id', '=', $eventID)
+      ->setSelect(['loc_block.*', 'loc_block_id'])
+      ->execute()->first();
+    $this->ids['LocBlock'][0] = $locationBlock['loc_block_id'];
+    $this->assertEquals($firstEmail['id'], $locationBlock['loc_block.email_id']);
+    $secondEmail = $emails->last();
+    $this->assertEquals($secondEmail['id'], $locationBlock['loc_block.email_2_id']);
+    return $emails;
+  }
+
+}

tests/phpunit/CiviTest/CiviUnitTestCase.php

@@ -3268,6 +3268,7 @@ public function onPost($op, $objectName, $objectId, &$objectRef) {
    */
   public function getFormObject($class, $formValues = [], $pageName = '') {
     $_POST = $formValues;
+    /* @var CRM_Core_Form $form */
     $form = new $class();
     $_SERVER['REQUEST_METHOD'] = 'GET';
     switch ($class) {
@@ -3280,8 +3281,7 @@ public function getFormObject($class, $formValues = [], $pageName = '') {
         $form->controller = new CRM_Core_Controller();
     }
     if (!$pageName) {
-      $formParts = explode('_', $class);
-      $pageName = array_pop($formParts);
+      $pageName = $form->getName();
     }
     $form->controller->setStateMachine(new CRM_Core_StateMachine($form->controller));
     $_SESSION['_' . $form->controller->_name . '_container']['values'][$pageName] = $formValues;