Stop overloading dataSource form
authorEileen McNaughton <emcnaughton@wikimedia.org>
Mon, 18 Apr 2022 01:54:32 +0000 (13:54 +1200)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Mon, 18 Apr 2022 19:38:24 +0000 (07:38 +1200)
CRM/Contact/Import/Form/DataSource.php
CRM/Core/xml/Menu/Import.xml
CRM/Import/Form/DataSourceConfig.php [new file with mode: 0644]
CRM/Import/Forms.php
templates/CRM/Contact/Import/Form/DataSource.tpl
templates/CRM/Import/Form/DataSourceConfig.tpl [new file with mode: 0644]
tests/phpunit/CiviTest/CiviUnitTestCase.php

index 4123c70d46b370bc14df025f57c2cc8b35842f14..474cd4ca632d9197e1fa6b69b5c02aa674bc733f 100644 (file)
  */
 class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
 
-  private $_dataSource;
-
-  private $_dataSourceIsValid = FALSE;
-
-  private $_dataSourceClass;
-
   /**
    * Get any smarty elements that may not be present in the form.
    *
@@ -70,59 +64,16 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
         2 => $config->uploadDir,
       ]));
     }
-
-    $this->_dataSourceIsValid = FALSE;
-    $this->_dataSource = CRM_Utils_Request::retrieveValue(
-      'dataSource',
-      'String',
-      NULL,
-      FALSE,
-      'GET'
-    );
-
-    $this->_params = $this->controller->exportValues($this->_name);
-    if (!$this->_dataSource) {
-      //considering dataSource as base criteria instead of hidden_dataSource.
-      $this->_dataSource = CRM_Utils_Array::value('dataSource',
-        $_POST,
-        CRM_Utils_Array::value('dataSource',
-          $this->_params
-        )
-      );
-      $this->assign('showOnlyDataSourceFormPane', FALSE);
-    }
-    else {
-      $this->assign('showOnlyDataSourceFormPane', TRUE);
-    }
-
-    $dataSources = $this->getDataSources();
-    if ($this->_dataSource && isset($dataSources[$this->_dataSource])) {
-      $this->_dataSourceIsValid = TRUE;
-      $this->assign('showDataSourceFormPane', TRUE);
-      $dataSourcePath = explode('_', $this->_dataSource);
-      $templateFile = 'CRM/Contact/Import/Form/' . $dataSourcePath[3] . '.tpl';
-    }
-    elseif ($this->_dataSource) {
-      $this->invalidConfig('Invalid data source');
-    }
-    $this->assign('dataSourceFormTemplateFile', $templateFile ?? NULL);
   }
 
   /**
    * Build the form object.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function buildQuickForm() {
 
-    // If there's a dataSource in the query string, we need to load
-    // the form from the chosen DataSource class
-    if ($this->_dataSourceIsValid) {
-      $this->_dataSourceClassFile = str_replace('_', '/', $this->_dataSource) . ".php";
-      require_once $this->_dataSourceClassFile;
-      $this->_dataSourceClass = new $this->_dataSource();
-      $this->_dataSourceClass->buildQuickForm($this);
-    }
-
-    $this->assign('urlPath', "civicrm/import");
+    $this->assign('urlPath', 'civicrm/import/datasource');
     $this->assign('urlPathVar', 'snippet=4');
 
     $this->add('select', 'dataSource', ts('Data Source'), $this->getDataSources(), TRUE,
@@ -176,6 +127,7 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
     if (Civi::settings()->get('address_standardization_provider') === 'USPS') {
       $this->addElement('checkbox', 'disableUSPS', ts('Disable USPS address validation during import?'));
     }
+    $this->buildDataSourceFields();
 
     $this->addButtons([
       [
@@ -199,7 +151,7 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
    */
   public function setDefaultValues() {
     $defaults = [
-      'dataSource' => 'CRM_Import_DataSource_CSV',
+      'dataSource' => $this->getDefaultDataSource(),
       'onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP,
       'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
       'fieldSeparator' => CRM_Core_Config::singleton()->fieldSeparator,
@@ -214,11 +166,13 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
 
   /**
    * Call the DataSource's postProcess method.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function postProcess() {
     $this->controller->resetPage('MapField');
 
-    if ($this->_dataSourceIsValid) {
+    if (1) {
       // Setup the params array
       $this->_params = $this->controller->exportValues($this->_name);
 
@@ -241,21 +195,16 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
 
       CRM_Core_Session::singleton()->set('dateTypes', $storeParams['dateFormats']);
 
-      // Get the PEAR::DB object
-      $dao = new CRM_Core_DAO();
-      $db = $dao->getDatabaseConnection();
-
       //hack to prevent multiple tables.
       $this->_params['import_table_name'] = $this->get('importTableName');
       if (!$this->_params['import_table_name']) {
         $this->_params['import_table_name'] = 'civicrm_import_job_' . md5(uniqid(rand(), TRUE));
       }
-
-      $this->_dataSourceClass->postProcess($this->_params, $db, $this);
+      $this->instantiateDataSource();
 
       // We should have the data in the DB now, parse it
       $importTableName = $this->get('importTableName');
-      $fieldNames = $this->_prepareImportTable($db, $importTableName);
+      $fieldNames = $this->_prepareImportTable($importTableName);
       $mapper = [];
 
       $parser = new CRM_Contact_Import_Parser_Contact($mapper);
@@ -276,9 +225,22 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
       // add all the necessary variables to the form
       $parser->set($this);
     }
-    else {
-      $this->invalidConfig("Invalid DataSource on form post. This shouldn't happen!");
-    }
+  }
+
+  /**
+   * Instantiate the datasource.
+   *
+   * This gives the datasource a chance to do any table creation etc.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  private function instantiateDataSource(): void {
+    $dataSourceName = $this->getDataSourceClassName();
+    $dataSource = new $dataSourceName();
+    // Get the PEAR::DB object
+    $dao = new CRM_Core_DAO();
+    $db = $dao->getDatabaseConnection();
+    $dataSource->postProcess($this->_params, $db, $this);
   }
 
   /**
@@ -290,7 +252,7 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
    *
    * @return array
    */
-  private function _prepareImportTable($db, $importTableName) {
+  private function _prepareImportTable($importTableName) {
     /* TODO: Add a check for an existing _status field;
      *  if it exists, create __status instead and return that
      */
@@ -315,7 +277,7 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Forms {
                        ADD COLUMN ${statusFieldName}Msg TEXT,
                        ADD COLUMN $primaryKeyName INT PRIMARY KEY NOT NULL
                                AUTO_INCREMENT";
-    $db->query($alterQuery);
+    CRM_Core_DAO::executeQuery($alterQuery);
 
     return ['status' => $statusFieldName, 'pk' => $primaryKeyName];
   }
index e874e2a5b03dd2e711900395a4c49dec79e8bc09..e1dec6916331f8baf06f7ae923acde56a1fd845c 100644 (file)
     <page_callback>CRM_Contact_Import_Page_AJAX::status</page_callback>
     <access_arguments>import contacts,access CiviCRM</access_arguments>
   </item>
+  <item>
+    <path>civicrm/import/datasource</path>
+    <title>Import</title>
+    <access_arguments>access CiviCRM</access_arguments>
+    <page_type>1</page_type>
+    <page_callback>CRM_Import_Form_DataSourceConfig</page_callback>
+    <weight>450</weight>
+  </item>
 </menu>
diff --git a/CRM/Import/Form/DataSourceConfig.php b/CRM/Import/Form/DataSourceConfig.php
new file mode 100644 (file)
index 0000000..93b34e7
--- /dev/null
@@ -0,0 +1,41 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+/**
+ * This class allows datasource specific fields to be added to the datasource form.
+ */
+class CRM_Import_Form_DataSourceConfig extends CRM_Import_Forms {
+
+  /**
+   * Set variables up before form is built.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function preProcess(): void {
+    $dataSourcePath = explode('_', $this->getDataSourceClassName());
+    $templateFile = 'CRM/Contact/Import/Form/' . $dataSourcePath[3] . '.tpl';
+    $this->assign('dataSourceFormTemplateFile', $templateFile ?? NULL);
+  }
+
+  /**
+   * Build the form object.
+   */
+  public function buildQuickForm(): void {
+    $this->buildDataSourceFields();
+  }
+
+}
index 237d2d7d1d9a639d18c437c296f5b3e0dfebb04b..808149483c3878f5614ab861ae74bcd57e2768e1 100644 (file)
@@ -34,6 +34,7 @@ class CRM_Import_Forms extends CRM_Core_Form {
       'contactType' => 'DataSource',
       'dateFormats' => 'DataSource',
       'savedMapping' => 'DataSource',
+      'dataSource' => 'DataSource',
     ];
     if (array_key_exists($fieldName, $mappedValues)) {
       return $this->controller->exportValue($mappedValues[$fieldName], $fieldName);
@@ -66,4 +67,76 @@ class CRM_Import_Forms extends CRM_Core_Form {
     return $dataSources;
   }
 
+  /**
+   * Get the name of the datasource class.
+   *
+   * This function prioritises retrieving from GET and POST over 'submitted'.
+   * The reason for this is the submitted array will hold the previous submissions
+   * data until after buildForm is called.
+   *
+   * This is problematic in the forward->back flow & option changing flow. As in....
+   *
+   * 1) Load DataSource form - initial default datasource is set to CSV and the
+   * form is via ajax (this calls DataSourceConfig to get the data).
+   * 2) User changes the source to SQL - the ajax updates the html but the
+   * form was built with the expectation that the csv-specific fields would be
+   * required.
+   * 3) When the user submits Quickform calls preProcess and buildForm and THEN
+   * retrieves the submitted values based on what has been added in buildForm.
+   * Only the submitted values for fields added in buildForm are available - but
+   * these have to be added BEFORE the submitted values are determined. Hence
+   * we look in the POST or GET to get the updated value.
+   *
+   * Note that an imminent refactor will involve storing the values in the
+   * civicrm_user_job table - this will hopefully help with a known (not new)
+   * issue whereby the previously submitted values (eg. skipColumnHeader has
+   * been checked or sql has been filled in) are not loaded via the ajax request.
+   *
+   * @return string|null
+   *
+   * @throws \CRM_Core_Exception
+   */
+  protected function getDataSourceClassName(): ?string {
+    $className = CRM_Utils_Request::retrieveValue(
+      'dataSource',
+      'String'
+    );
+    if (!$className) {
+      $className = $this->getSubmittedValue('dataSource');
+    }
+    if (!$className) {
+      $className = $this->getDefaultDataSource();
+    }
+    if ($this->getDataSources()[$className]) {
+      return $className;
+    }
+    throw new CRM_Core_Exception('Invalid data source');
+  }
+
+  /**
+   * Allow the datasource class to add fields.
+   *
+   * This is called as a snippet in DataSourceConfig and
+   * also from DataSource::buildForm to add the fields such
+   * that quick form picks them up.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  protected function buildDataSourceFields(): void {
+    $className = $this->getDataSourceClassName();
+    if ($className) {
+      $dataSourceClass = new $className();
+      $dataSourceClass->buildQuickForm($this);
+    }
+  }
+
+  /**
+   * Get the default datasource.
+   *
+   * @return string
+   */
+  protected function getDefaultDataSource(): string {
+    return 'CRM_Import_DataSource_CSV';
+  }
+
 }
index 0c2f4591599c1b9cff4d32fbde89a95bcb3088aa..deaef9695478dfb52455fc770a07758eff94c33a 100644 (file)
@@ -9,9 +9,6 @@
 *}
 
 <div class="crm-block crm-form-block crm-import-datasource-form-block">
-{if $showOnlyDataSourceFormPane}
-  {include file=$dataSourceFormTemplateFile}
-{else}
   {* Import Wizard - Step 1 (choose data source) *}
   {* WizardHeader.tpl provides visual display of steps thru the wizard as well as title for current step *}
   {include file="CRM/common/WizardHeader.tpl"}
@@ -31,9 +28,6 @@
 
   {* Data source form pane is injected here when the data source is selected. *}
   <div id="data-source-form-block">
-    {if $dataSourceFormTemplateFile}
-      {include file=$dataSourceFormTemplateFile}
-    {/if}
   </div>
 
   <div id="common-form-controls" class="form-item">
 
     </script>
   {/literal}
-{/if}
+
 </div>
diff --git a/templates/CRM/Import/Form/DataSourceConfig.tpl b/templates/CRM/Import/Form/DataSourceConfig.tpl
new file mode 100644 (file)
index 0000000..0bf8fa2
--- /dev/null
@@ -0,0 +1,10 @@
+{*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+*}
+{include file=$dataSourceFormTemplateFile}
index 903dfb47bd6c61784ae8cc5901210e7d712f03a1..ac70a549f0ade7086ec04424620e3c5dea109331 100644 (file)
@@ -3218,6 +3218,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
       case 'CRM_Event_Form_Registration_Confirm':
         $form->controller = new CRM_Event_Controller_Registration();
         break;
+
       case 'CRM_Contact_Import_Form_DataSource':
         $form->controller = new CRM_Contact_Import_Controller();
         break;