Merge pull request #23619 from darrick/pull/23534

Fix swapOptions writing over selected options on mapField page load.
civicrm · May 30, 2022 · e09e156 · e09e156
2 parents ee35b88 + 5a0a579
commit e09e156
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 69 deletions.
diff --git a/CRM/Contact/Import/Form/MapField.php b/CRM/Contact/Import/Form/MapField.php
@@ -287,39 +287,49 @@ public function buildQuickForm() {
     $processor->setMetadata($this->getContactImportMetadata());
     $processor->setContactTypeByConstant($this->getSubmittedValue('contactType'));
     $processor->setContactSubType($this->getSubmittedValue('contactSubType'));
+    $mapper = $this->getSubmittedValue('mapper');
 
     for ($i = 0; $i < $this->_columnCount; $i++) {
       $sel = &$this->addElement('hierselect', "mapper[$i]", ts('Mapper for Field %1', [1 => $i]), NULL);
+      $last_key = 0;
 
-      if ($this->getSubmittedValue('savedMapping') && $processor->getFieldName($i)) {
+      // Don't set any defaults if we are going to the next page.
+      // ... or coming back.
+      // But do add the js.
+      if (!empty($mapper)) {
+        $last_key = array_key_last($mapper[$i]);
+      }
+      elseif ($this->getSubmittedValue('savedMapping') && $processor->getFieldName($i)) {
         $defaults["mapper[$i]"] = $processor->getSavedQuickformDefaultsForColumn($i);
-        $js .= $processor->getQuickFormJSForField($i);
+        $last_key = array_key_last($defaults["mapper[$i]"]) ?? 0;
       }
       else {
-        $js .= "swapOptions($formName, 'mapper[$i]', 0, 3, 'hs_mapper_0_');\n";
         if ($hasColumnNames) {
           // do array search first to see if has mapped key
           $columnKey = array_search($this->_columnNames[$i], $this->getFieldTitles());
           if (isset($this->_fieldUsed[$columnKey])) {
-            $defaults["mapper[$i]"] = $columnKey;
+            $defaults["mapper[$i]"] = [$columnKey];
             $this->_fieldUsed[$key] = TRUE;
           }
           else {
             // Infer the default from the column names if we have them
             $defaults["mapper[$i]"] = [
               $this->defaultFromColumnName($this->_columnNames[$i]),
-              0,
             ];
           }
         }
         else {
           // Otherwise guess the default from the form of the data
           $defaults["mapper[$i]"] = [
             $this->defaultFromData($this->getDataPatterns(), $i),
-            //                     $defaultLocationType->id
-            0,
           ];
         }
+        $last_key = array_key_last($defaults["mapper[$i]"]) ?? 0;
+      }
+      // Call swapOptions on the deepest select element to hide the empty select lists above it.
+      // But we don't need to hide anything above $sel4.
+      if ($last_key < 3) {
+        $js .= "swapOptions($formName, 'mapper[$i]', $last_key, 4, 'hs_mapper_0_');\n";
       }
       $sel->setOptions([$sel1, $sel2, $sel3, $sel4]);
     }

diff --git a/CRM/Import/ImportProcessor.php b/CRM/Import/ImportProcessor.php
@@ -546,47 +546,6 @@ protected function isValidRelationshipKey($key) {
     return !empty($this->getValidRelationships()[$key]);
   }
 
-  /**
-   * Get the relevant js for quickform.
-   *
-   * @param int $column
-   *
-   * @return string
-   * @throws \CiviCRM_API3_Exception
-   */
-  public function getQuickFormJSForField($column) {
-    $columnNumbersToHide = [];
-    if ($this->getFieldName($column) === 'do_not_import') {
-      $columnNumbersToHide = [1, 2, 3];
-    }
-    elseif ($this->getRelationshipKey($column)) {
-      if (!$this->getWebsiteTypeID($column) && !$this->getLocationTypeID($column)) {
-        $columnNumbersToHide[] = 2;
-      }
-      if (!$this->getFieldName($column)) {
-        $columnNumbersToHide[] = 1;
-      }
-      if (!$this->getPhoneOrIMTypeID($column)) {
-        $columnNumbersToHide[] = 3;
-      }
-    }
-    else {
-      if (!$this->getLocationTypeID($column) && !$this->getWebsiteTypeID($column)) {
-        $columnNumbersToHide[] = 1;
-      }
-      if (!$this->getPhoneOrIMTypeID($column)) {
-        $columnNumbersToHide[] = 2;
-      }
-      $columnNumbersToHide[] = 3;
-    }
-
-    $jsClauses = [];
-    foreach ($columnNumbersToHide as $columnNumber) {
-      $jsClauses[] = $this->getFormName() . "['mapper[$column][" . $columnNumber . "]'].style.display = 'none';";
-    }
-    return empty($jsClauses) ? '' : implode("\n", $jsClauses) . "\n";
-  }
-
   /**
    * Get the defaults for the column from the saved mapping.
    *
@@ -596,19 +555,33 @@ public function getQuickFormJSForField($column) {
    * @throws \CiviCRM_API3_Exception
    */
   public function getSavedQuickformDefaultsForColumn($column) {
+    $fieldMapping = [];
+
+    // $sel1 is either unmapped, a relationship or a target field.
     if ($this->getFieldName($column) === 'do_not_import') {
-      return [];
+      return $fieldMapping;
     }
+
     if ($this->getValidRelationshipKey($column)) {
-      if ($this->getWebsiteTypeID($column)) {
-        return [$this->getValidRelationshipKey($column), $this->getFieldName($column), $this->getWebsiteTypeID($column)];
-      }
-      return [$this->getValidRelationshipKey($column), $this->getFieldName($column), $this->getLocationTypeID($column), $this->getPhoneOrIMTypeID($column)];
+      $fieldMapping[] = $this->getValidRelationshipKey($column);
     }
+
+    // $sel1
+    $fieldMapping[] = $this->getFieldName($column);
+
+    // $sel2
     if ($this->getWebsiteTypeID($column)) {
-      return [$this->getFieldName($column), $this->getWebsiteTypeID($column)];
+      $fieldMapping[] = $this->getWebsiteTypeID($column);
+    }
+    elseif ($this->getLocationTypeID($column)) {
+      $fieldMapping[] = $this->getLocationTypeID($column);
+    }
+
+    // $sel3
+    if ($this->getPhoneOrIMTypeID($column)) {
+      $fieldMapping[] = $this->getPhoneOrIMTypeID($column);
     }
-    return [(string) $this->getFieldName($column), $this->getLocationTypeID($column), $this->getPhoneOrIMTypeID($column)];
+    return $fieldMapping;
   }
 
   /**

diff --git a/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php b/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
@@ -237,31 +237,28 @@ public function mapFieldDataProvider(): array {
     return [
       [
         ['name' => 'first_name', 'contact_type' => 'Individual', 'column_number' => 0],
-        "document.forms.MapField['mapper[0][1]'].style.display = 'none';
-document.forms.MapField['mapper[0][2]'].style.display = 'none';
-document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
-        ['mapper[0]' => ['first_name', 0, NULL]],
+        "swapOptions(document.forms.MapField, 'mapper[0]', 0, 4, 'hs_mapper_0_');\n",
+        ['mapper[0]' => ['first_name']],
       ],
       [
         ['name' => 'phone', 'contact_type' => 'Individual', 'column_number' => 0, 'phone_type_id' => 1, 'location_type_id' => 2],
-        "document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
+        "swapOptions(document.forms.MapField, 'mapper[0]', 2, 4, 'hs_mapper_0_');\n",
         ['mapper[0]' => ['phone', 2, 1]],
       ],
       [
         ['name' => 'im', 'contact_type' => 'Individual', 'column_number' => 0, 'im_provider_id' => 1, 'location_type_id' => 2],
-        "document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
+        "swapOptions(document.forms.MapField, 'mapper[0]', 2, 4, 'hs_mapper_0_');\n",
         ['mapper[0]' => ['im', 2, 1]],
       ],
       [
         ['name' => 'url', 'contact_type' => 'Individual', 'column_number' => 0, 'website_type_id' => 1],
-        "document.forms.MapField['mapper[0][2]'].style.display = 'none';
-document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
+        "swapOptions(document.forms.MapField, 'mapper[0]', 1, 4, 'hs_mapper_0_');\n",
         ['mapper[0]' => ['url', 1]],
       ],
       [
         // Yes, the relationship mapping really does use url whereas non relationship uses website because... legacy
         ['name' => 'url', 'contact_type' => 'Individual', 'column_number' => 0, 'website_type_id' => 1, 'relationship_type_id' => 1, 'relationship_direction' => 'a_b'],
-        "document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
+        "swapOptions(document.forms.MapField, 'mapper[0]', 2, 4, 'hs_mapper_0_');\n",
         ['mapper[0]' => ['1_a_b', 'url', 1]],
       ],
       [
@@ -271,9 +268,7 @@ public function mapFieldDataProvider(): array {
       ],
       [
         ['name' => 'do_not_import', 'contact_type' => 'Individual', 'column_number' => 0],
-        "document.forms.MapField['mapper[0][1]'].style.display = 'none';
-document.forms.MapField['mapper[0][2]'].style.display = 'none';
-document.forms.MapField['mapper[0][3]'].style.display = 'none';\n",
+        "swapOptions(document.forms.MapField, 'mapper[0]', 0, 4, 'hs_mapper_0_');\n",
         ['mapper[0]' => []],
       ],
     ];
@@ -354,9 +349,8 @@ protected function loadSavedMapping(int $mappingID, int $columnNumber): array {
 
     $defaults = [];
     $defaults["mapper[$columnNumber]"] = $processor->getSavedQuickformDefaultsForColumn($columnNumber);
-    $js = $processor->getQuickFormJSForField($columnNumber);
 
-    return ['defaults' => $defaults, 'js' => $js];
+    return ['defaults' => $defaults];
   }
 
   /**

diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -1106,6 +1106,9 @@ public function testImportCountryStateCounty(): void {
       ['do_not_import'],
       // [$childKey, $customField, 'state_province'],
       ['do_not_import'],
+      // mapField Form expects all fields to be mapped.
+      ['do_not_import'],
+      ['do_not_import'],
     ];
     $csv = 'individual_country_state_county_with_related.csv';
     $this->validateMultiRowCsv($csv, $mapper, 'error_value');
@@ -1182,6 +1185,10 @@ public function testImportContactSubTypes(): void {
       ['5_a_b', 'organization_name'],
       ['contact_sub_type'],
       ['5_a_b', 'contact_sub_type'],
+      // mapField Form expects all fields to be mapped.
+      ['do_not_import'],
+      ['do_not_import'],
+      ['do_not_import'],
     ];
     $csv = 'individual_contact_sub_types.csv';
     $field = 'contact_sub_type';