From 43b7eed54611d1b55e2c04ab4bb119178341196b Mon Sep 17 00:00:00 2001 From: colemanw Date: Sat, 2 Sep 2023 22:24:56 -0400 Subject: [PATCH] CiviCampaign - Move serialized contents of civicrm_survey.recontact_interval 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. --- CRM/Campaign/BAO/Query.php | 18 ++++++++--- CRM/Campaign/DAO/Survey.php | 34 +------------------- CRM/Campaign/Form/Survey/Main.php | 12 ------- CRM/Campaign/Form/Survey/Results.php | 7 +--- CRM/Campaign/Page/AJAX.php | 25 ++++---------- CRM/Upgrade/Incremental/php/FiveSixtySix.php | 29 +++++++++++++++++ xml/schema/Campaign/Survey.xml | 6 +--- 7 files changed, 51 insertions(+), 80 deletions(-) diff --git a/CRM/Campaign/BAO/Query.php b/CRM/Campaign/BAO/Query.php index 9431c0b95b61..cde52e31722c 100644 --- 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 which are stored in option_value.filter column. + // 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 index aa39ac1fe072..cbf590c94370 100644 --- 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 index 72fc38c7c161..2aca9a250b68 100644 --- 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 index a12e609326e3..27121645e8b6 100644 --- 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 index fc667ded8b8c..f5e857c2d358 100644 --- 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 index 54df02904a05..84b32bf5704f 100644 --- 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 index 3d18daf05984..8a2fe3627ca5 100644 --- a/xml/schema/Campaign/Survey.xml +++ b/xml/schema/Campaign/Survey.xml @@ -91,12 +91,8 @@ Follow up Interval text Recontact intervals for each status. - - TextArea - 20 - 80 - 3.3 + 5.66