CiviCampaign - Move serialized contents of civicrm_survey.recontact_i…

…nterval into civicrm_option_value.filter Refactors out a thoroughly strange data structure. Each option value associated with a survey has an associated recontact_interval, which is an integer storing the number of days before the respondent should be recontacted. Before: A serialized array was being stored in the survey with keys matching the option LABEL! After: There was an unused integer column, civicrm_option_value.filter which works fine for storing the number directly for each option_value with no messy mapping needed.
civicrm · Sep 3, 2023 · 3a92882 · 3a92882
1 parent 09b8113
commit 3a92882
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 80 deletions.
diff --git a/CRM/Campaign/BAO/Query.php b/CRM/Campaign/BAO/Query.php
@@ -468,13 +468,21 @@ public static function voterClause($params) {
       if ($searchVoterFor == 'reserve') {
         $operator = 'NOT IN';
         //filter out recontact survey contacts.
-        $recontactInterval = CRM_Core_DAO::getFieldValue('CRM_Campaign_DAO_Survey',
-          $surveyId, 'recontact_interval'
+        $optionGroupId = CRM_Core_DAO::getFieldValue('CRM_Campaign_DAO_Survey',
+          $surveyId, 'result_id'
         );
-        $recontactInterval = CRM_Utils_String::unserialize($recontactInterval);
+        if ($optionGroupId) {
+          // Lookup intervals from option group
+          // FIXME: Keyed by label because civicrm_activity.result unfortunately stores the option_value.label!
+          $recontactInterval = \Civi\Api4\OptionValue::get(FALSE)
+            ->addSelect('label', 'filter')
+            ->addWhere('option_group_id', '=', $optionGroupId)
+            ->execute()
+            ->indexBy('label')->column('filter');
+        }
         if ($surveyId &&
-          is_array($recontactInterval) &&
-          !empty($recontactInterval)
+          !empty($recontactInterval) &&
+          is_array($recontactInterval)
         ) {
           $voterIds = [];
           foreach ($voterActValues as $values) {

diff --git a/CRM/Campaign/DAO/Survey.php b/CRM/Campaign/DAO/Survey.php
@@ -6,7 +6,7 @@
  *
  * Generated from xml/schema/CRM/Campaign/Survey.xml
  * DO NOT EDIT.  Generated by CRM_Core_CodeGen
- * (GenCodeChecksum:189e49f95be8623d4a98ab881ab8f5c6)
+ * (GenCodeChecksum:754f9a2e62e86ff340afea563b561dec)
  */
 
 /**
@@ -81,15 +81,6 @@ class CRM_Campaign_DAO_Survey extends CRM_Core_DAO {
    */
   public $activity_type_id;
 
-  /**
-   * Recontact intervals for each status.
-   *
-   * @var string|null
-   *   (SQL type: text)
-   *   Note that values will be retrieved from the database as a string.
-   */
-  public $recontact_interval;
-
   /**
    * Script instructions for volunteers to use for the survey.
    *
@@ -375,29 +366,6 @@ public static function &fields() {
           ],
           'add' => '3.3',
         ],
-        'recontact_interval' => [
-          'name' => 'recontact_interval',
-          'type' => CRM_Utils_Type::T_TEXT,
-          'title' => ts('Follow up Interval'),
-          'description' => ts('Recontact intervals for each status.'),
-          'rows' => 20,
-          'cols' => 80,
-          'usage' => [
-            'import' => FALSE,
-            'export' => FALSE,
-            'duplicate_matching' => FALSE,
-            'token' => FALSE,
-          ],
-          'where' => 'civicrm_survey.recontact_interval',
-          'table_name' => 'civicrm_survey',
-          'entity' => 'Survey',
-          'bao' => 'CRM_Campaign_BAO_Survey',
-          'localizable' => 0,
-          'html' => [
-            'type' => 'TextArea',
-          ],
-          'add' => '3.3',
-        ],
         'instructions' => [
           'name' => 'instructions',
           'type' => CRM_Utils_Type::T_TEXT,

diff --git a/CRM/Campaign/Form/Survey/Main.php b/CRM/Campaign/Form/Survey/Main.php
@@ -83,18 +83,6 @@ public function setDefaultValues() {
 
     $defaults = $this->_values;
 
-    if ($this->_surveyId) {
-
-      if (!empty($defaults['result_id']) && !empty($defaults['recontact_interval'])) {
-
-        $resultId = $defaults['result_id'];
-        $recontactInterval = CRM_Utils_String::unserialize($defaults['recontact_interval']);
-
-        unset($defaults['recontact_interval']);
-        $defaults['option_group_id'] = $resultId;
-      }
-    }
-
     if (!isset($defaults['is_active'])) {
       $defaults['is_active'] = 1;
     }

diff --git a/CRM/Campaign/Form/Survey/Results.php b/CRM/Campaign/Form/Survey/Results.php
@@ -343,7 +343,6 @@ public function postProcess() {
       $resultSetOptGrpId = $params['option_group_id'];
     }
 
-    $recontactInterval = [];
     if ($updateResultSet) {
       $optionValue = new CRM_Core_DAO_OptionValue();
       $optionValue->option_group_id = $resultSetOptGrpId;
@@ -372,6 +371,7 @@ public function postProcess() {
         $optionValue->value = trim($v);
         $optionValue->weight = $params['option_weight'][$k];
         $optionValue->is_active = 1;
+        $optionValue->filter = $params['option_interval'][$k];
 
         if (!empty($params['default_option']) &&
           $params['default_option'] == $k
@@ -381,14 +381,9 @@ public function postProcess() {
 
         $optionValue->save();
 
-        // using is_numeric since 0 is a valid value for option_interval
-        if (is_numeric($params['option_interval'][$k])) {
-          $recontactInterval[$optionValue->label] = $params['option_interval'][$k];
-        }
       }
     }
 
-    $params['recontact_interval'] = serialize($recontactInterval);
     $survey = CRM_Campaign_BAO_Survey::create($params);
 
     // create report if required.

diff --git a/CRM/Campaign/Page/AJAX.php b/CRM/Campaign/Page/AJAX.php
@@ -91,27 +91,14 @@ public static function loadOptionGroupDetails() {
 
     $id = CRM_Utils_Request::retrieve('option_group_id', 'Integer', CRM_Core_DAO::$_nullObject, FALSE, NULL, 'POST');
     $status = 'fail';
-    $opValues = [];
 
     if ($id) {
-      $groupParams['id'] = $id;
-      CRM_Core_OptionValue::getValues($groupParams, $opValues);
-    }
-
-    $surveyId = CRM_Utils_Request::retrieve('survey_id', 'Integer', CRM_Core_DAO::$_nullObject, FALSE, NULL, 'POST');
-    if ($surveyId) {
-      $survey = new CRM_Campaign_DAO_Survey();
-      $survey->id = $surveyId;
-      $survey->result_id = $id;
-      if ($survey->find(TRUE)) {
-        if ($survey->recontact_interval) {
-          $recontactInterval = CRM_Utils_String::unserialize($survey->recontact_interval);
-          foreach ($opValues as $opValId => $opVal) {
-            if (is_numeric($recontactInterval[$opVal['label']])) {
-              $opValues[$opValId]['interval'] = $recontactInterval[$opVal['label']];
-            }
-          }
-        }
+      $opValues = \Civi\Api4\OptionValue::get(FALSE)
+        ->addWhere('option_group_id', '=', $id)
+        ->addOrderBy('weight')
+        ->execute()->indexBy('id');
+      foreach ($opValues as $id => $value) {
+        $opValues[$id]['interval'] = $value['filter'];
       }
     }
 

diff --git a/CRM/Upgrade/Incremental/php/FiveSixtySix.php b/CRM/Upgrade/Incremental/php/FiveSixtySix.php
@@ -36,6 +36,8 @@ public function upgrade_5_66_alpha1($rev): void {
     $this->addTask('Make ActionSchedule.name required', 'alterColumn', 'civicrm_action_schedule', 'name', "varchar(64) NOT NULL COMMENT 'physical tablename for entity being joined to discount, e.g. civicrm_event'");
     $this->addTask(ts('Create index %1', [1 => 'civicrm_action_schedule.UI_name']), 'addIndex', 'civicrm_action_schedule', 'name', 'UI');
     $this->addTask('Add fields to civicrm_mail_settings to allow more flexibility for email to activity', 'addMailSettingsFields');
+    $this->addTask('Move serialized contents of civicrm_survey.recontact_interval into civicrm_option_value.filter', 'migrateRecontactInterval');
+    $this->addTask('Drop column civicrm_survey.recontact_interval', 'dropColumn', 'civicrm_survey', 'recontact_interval');
     $this->addTask('Update afform tab names', 'updateAfformTabs');
     $this->addTask('Add in Client Removed Activity Type', 'addCaseClientRemovedActivity');
     $this->addTask('Update quicksearch options to v4 format', 'updateQuicksearchOptions');
@@ -146,6 +148,33 @@ public static function addCaseClientRemovedActivity() {
     return TRUE;
   }
 
+  /**
+   * Move serialized contents of Survey.recontact_interval into OptionValue.filter
+   *
+   * @param \CRM_Queue_TaskContext $ctx
+   * @return bool
+   */
+  public static function migrateRecontactInterval($ctx): bool {
+    if (!CRM_Core_BAO_SchemaHandler::checkIfFieldExists('civicrm_survey', 'recontact_interval')) {
+      // Upgrade has already run, nothing to do.
+      return TRUE;
+    }
+    $surveys = CRM_Core_DAO::executeQuery('SELECT result_id, recontact_interval FROM civicrm_survey')->fetchAll();
+    foreach ($surveys as $survey) {
+      if (empty($survey['recontact_interval']) || empty($survey['result_id'])) {
+        continue;
+      }
+      foreach (unserialize($survey['recontact_interval']) as $label => $interval) {
+        CRM_Core_DAO::executeQuery('UPDATE civicrm_option_value SET filter = %1 WHERE option_group_id = %2 AND label = %3', [
+          1 => [$interval, 'Integer'],
+          2 => [$survey['result_id'], 'Positive'],
+          3 => [$label, 'String'],
+        ]);
+      }
+    }
+    return TRUE;
+  }
+
   /**
    * Convert `quicksearch_options` setting to use new APIv4 field names
    *

diff --git a/xml/schema/Campaign/Survey.xml b/xml/schema/Campaign/Survey.xml
@@ -91,12 +91,8 @@
     <title>Follow up Interval</title>
     <type>text</type>
     <comment>Recontact intervals for each status.</comment>
-    <html>
-      <type>TextArea</type>
-      <rows>20</rows>
-      <cols>80</cols>
-    </html>
     <add>3.3</add>
+    <drop>5.66</drop>
   </field>
 
   <field>