From d66c61b6853a6df997581c9c6a6b2f390671cc3b Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 21 Apr 2017 15:00:53 +1200 Subject: [PATCH] CRM-20459 replace all instances where CRM_Core_OptionGroup::getValue is called on activities. I limited this to activities as there are only 2 separate values & I thought it would keep it readable. --- CRM/Activity/BAO/Activity.php | 22 ++++++------- CRM/Campaign/BAO/Survey.php | 2 +- CRM/Case/BAO/Case.php | 2 +- CRM/Case/XMLProcessor/Process.php | 11 ++----- CRM/Contribute/BAO/Contribution.php | 33 ++++++++----------- CRM/Contribute/BAO/ContributionRecur.php | 12 +++---- CRM/Core/BAO/ActionSchedule.php | 11 +++++-- CRM/Event/BAO/Participant.php | 11 ++----- CRM/Report/Form/Case/Demographics.php | 2 +- .../CRM/Contribute/BAO/ContributionTest.php | 7 ++-- tests/phpunit/api/v3/ActionScheduleTest.php | 12 +++---- 11 files changed, 52 insertions(+), 73 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index df28122fb2..d335f09bc7 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -66,6 +66,8 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } /** + * @deprecated + * * Fetch object based on array of properties. * * @param array $params @@ -76,6 +78,8 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { * @return CRM_Activity_DAO_Activity */ public static function retrieve(&$params, &$defaults) { + // this will bypass acls - use the api instead. + // @todo add deprecation logging to this function. $activity = new CRM_Activity_DAO_Activity(); $activity->copyValues($params); @@ -92,7 +96,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $assignee_contact_names = CRM_Activity_BAO_ActivityContact::getNames($activity->id, $assigneeID); $defaults['assignee_contact_value'] = implode('; ', $assignee_contact_names); $sourceContactId = self::getActivityContact($activity->id, $sourceID); - if ($activity->activity_type_id != CRM_Core_OptionGroup::getValue('activity_type', 'Bulk Email', 'name')) { + if ($activity->activity_type_id != CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Bulk Email')) { $defaults['target_contact'] = CRM_Activity_BAO_ActivityContact::retrieveContactIdsByActivityId($activity->id, $targetID); $target_contact_names = CRM_Activity_BAO_ActivityContact::getNames($activity->id, $targetID); $defaults['target_contact_value'] = implode('; ', $target_contact_names); @@ -528,9 +532,9 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } else { $q = "action=view&reset=1&id={$activity->id}&atype={$activity->activity_type_id}&cid=" . CRM_Utils_Array::value('source_contact_id', $params) . "&context=home"; - if ($activity->activity_type_id != CRM_Core_OptionGroup::getValue('activity_type', 'Email', 'name')) { + if ($activity->activity_type_id != CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Email')) { $url = CRM_Utils_System::url('civicrm/activity', $q); - if ($activity->activity_type_id == CRM_Core_OptionGroup::getValue('activity_type', 'Print PDF Letter', 'name')) { + if ($activity->activity_type_id == CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Print PDF Letter')) { $recentOther['editUrl'] = CRM_Utils_System::url('civicrm/activity/pdf/add', "action=update&reset=1&id={$activity->id}&atype={$activity->activity_type_id}&cid={$params['source_contact_id']}&context=home" ); @@ -1635,17 +1639,11 @@ SELECT display_name $activityParams = array( 'source_contact_id' => $activity->contact_id, 'source_record_id' => $activity->id, - 'activity_type_id' => CRM_Core_OptionGroup::getValue('activity_type', - $activityType, - 'name' - ), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', $activityType), 'subject' => $subject, 'activity_date_time' => $date, 'is_test' => $activity->is_test, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - 'Completed', - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'), 'skipRecentView' => TRUE, 'campaign_id' => $activity->campaign_id, ); @@ -1668,6 +1666,8 @@ SELECT display_name if ($targetContactID) { $activityParams['target_contact_id'][] = $targetContactID; } + // @todo - use api - remove lots of wrangling above. Remove deprecated fatal & let form layer + // deal with any exceptions. if (is_a(self::create($activityParams), 'CRM_Core_Error')) { CRM_Core_Error::fatal("Failed creating Activity for $component of id {$activity->id}"); return FALSE; diff --git a/CRM/Campaign/BAO/Survey.php b/CRM/Campaign/BAO/Survey.php index 5b09731f98..dfc855e621 100644 --- a/CRM/Campaign/BAO/Survey.php +++ b/CRM/Campaign/BAO/Survey.php @@ -269,7 +269,7 @@ SELECT survey.id as id, if (!$includePetition) { //we only have activity type as a //difference between survey and petition. - $petitionTypeID = CRM_Core_OptionGroup::getValue('activity_type', 'petition', 'name'); + $petitionTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'petition'); $where = array(); if ($petitionTypeID) { diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index 991ccb9e33..3d099d0094 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -2991,7 +2991,7 @@ WHERE id IN (' . implode(',', $copiedActivityIds) . ')'; */ public static function createCaseViewsQuery($section = 'upcoming') { $sql = ""; - $scheduled_id = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name'); + $scheduled_id = CRM_Core_Pseudoconstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Scheduled'); switch ($section) { case 'upcoming': $sql = "CREATE OR REPLACE VIEW `civicrm_view_case_activity_upcoming` diff --git a/CRM/Case/XMLProcessor/Process.php b/CRM/Case/XMLProcessor/Process.php index 7222827578..fe1469bfac 100644 --- a/CRM/Case/XMLProcessor/Process.php +++ b/CRM/Case/XMLProcessor/Process.php @@ -452,10 +452,7 @@ AND a.is_deleted = 0 'is_auto' => FALSE, 'is_current_revision' => 1, 'subject' => CRM_Utils_Array::value('subject', $params) ? $params['subject'] : $activityTypeName, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - $statusName, - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', $statusName), 'target_contact_id' => $client, 'medium_id' => CRM_Utils_Array::value('medium_id', $params), 'location' => CRM_Utils_Array::value('location', $params), @@ -470,10 +467,7 @@ AND a.is_deleted = 0 'source_contact_id' => $params['creatorID'], 'is_auto' => TRUE, 'is_current_revision' => 1, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - $statusName, - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', $statusName), 'target_contact_id' => $client, 'weight' => $orderVal, ); @@ -556,6 +550,7 @@ AND a.is_deleted = 0 $activityParams['skipRecentView'] = TRUE; } + // @todo - switch to using api & remove the parameter pre-wrangling above. $activity = CRM_Activity_BAO_Activity::create($activityParams); if (!$activity) { diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index a6d6b67f5d..3c21ba3163 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -560,20 +560,20 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { $transaction->commit(); - // check if activity record exist for this contribution, if - // not add activity - $activity = new CRM_Activity_DAO_Activity(); - $activity->source_record_id = $contribution->id; - $activity->activity_type_id = CRM_Core_OptionGroup::getValue('activity_type', - 'Contribution', - 'name' - ); + $activity = civicrm_api3('Activity', 'get', array( + 'source_record_id' => $contribution->id, + 'options' => array('limit' => 1), + 'sequential' => 1, + 'activity_type_id' => 'Contribution', + 'return' => array('id', 'campaign'), + )); //CRM-18406: Update activity when edit contribution. - if ($activity->find(TRUE)) { + if ($activity['count']) { // CRM-13237 : if activity record found, update it with campaign id of contribution - CRM_Core_DAO::setFieldValue('CRM_Activity_BAO_Activity', $activity->id, 'campaign_id', $contribution->campaign_id); - $contribution->activity_id = $activity->id; + // @todo compare campaign ids first. + CRM_Core_DAO::setFieldValue('CRM_Activity_BAO_Activity', $activity['id'], 'campaign_id', $contribution->campaign_id); + $contribution->activity_id = $activity['id']; } if (empty($contribution->contact_id)) { $contribution->find(TRUE); @@ -3987,16 +3987,10 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']}) $activityParams = array( 'source_contact_id' => $targetCid, 'source_record_id' => $srcRecId, - 'activity_type_id' => CRM_Core_OptionGroup::getValue('activity_type', - $activityType, - 'name' - ), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', $activityType), 'subject' => $subject, 'activity_date_time' => $date, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - 'Completed', - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'), 'skipRecentView' => TRUE, ); @@ -4007,6 +4001,7 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']}) $activityParams['source_contact_id'] = $id; $activityParams['target_contact_id'][] = $targetCid; } + // @todo use api. CRM_Activity_BAO_Activity::create($activityParams); } diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php index 97dfc720fa..7678ae43ca 100644 --- a/CRM/Contribute/BAO/ContributionRecur.php +++ b/CRM/Contribute/BAO/ContributionRecur.php @@ -283,17 +283,11 @@ SELECT r.payment_processor_id $activityParams = array( 'source_contact_id' => $dao->contact_id, 'source_record_id' => CRM_Utils_Array::value('source_record_id', $activityParams), - 'activity_type_id' => CRM_Core_OptionGroup::getValue('activity_type', - 'Cancel Recurring Contribution', - 'name' - ), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Cancel Recurring Contribution'), 'subject' => CRM_Utils_Array::value('subject', $activityParams, ts('Recurring contribution cancelled')), 'details' => $details, 'activity_date_time' => date('YmdHis'), - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - 'Completed', - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'), ); $session = CRM_Core_Session::singleton(); $cid = $session->get('userID'); @@ -301,6 +295,7 @@ SELECT r.payment_processor_id $activityParams['target_contact_id'][] = $activityParams['source_contact_id']; $activityParams['source_contact_id'] = $cid; } + // @todo use the api & do less wrangling above CRM_Activity_BAO_Activity::create($activityParams); } @@ -310,6 +305,7 @@ SELECT r.payment_processor_id return TRUE; } else { + // @todo - this is bad! Get the function out of the ipn. $baseIPN = new CRM_Core_Payment_BaseIPN(); return $baseIPN->cancelled($objects, $transaction); } diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index b3194882e3..35a41f1aae 100644 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -465,12 +465,14 @@ FROM civicrm_action_schedule cas $session = CRM_Core_Session::singleton(); if ($mapping->getEntity() == 'civicrm_membership') { + // @todo - not required with api $activityTypeID - = CRM_Core_OptionGroup::getValue('activity_type', 'Membership Renewal Reminder', 'name'); + = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Membership Renewal Reminder'); } else { + // @todo - not required with api $activityTypeID - = CRM_Core_OptionGroup::getValue('activity_type', 'Reminder Sent', 'name'); + = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Reminder Sent'); } $activityParams = array( @@ -478,11 +480,14 @@ FROM civicrm_action_schedule cas 'details' => $tokenRow->render('body_html'), 'source_contact_id' => $session->get('userID') ? $session->get('userID') : $contactID, 'target_contact_id' => $contactID, + // @todo - not required with api 'activity_date_time' => CRM_Utils_Time::getTime('YmdHis'), - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', 'Completed', 'name'), + // @todo - not required with api + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'status_id', 'Completed'), 'activity_type_id' => $activityTypeID, 'source_record_id' => $entityID, ); + // @todo use api, remove all the above wrangling $activity = CRM_Activity_BAO_Activity::create($activityParams); //file reminder on case if source activity is a case activity diff --git a/CRM/Event/BAO/Participant.php b/CRM/Event/BAO/Participant.php index 50dc253ee2..ed3e91be08 100644 --- a/CRM/Event/BAO/Participant.php +++ b/CRM/Event/BAO/Participant.php @@ -2209,16 +2209,10 @@ WHERE (entity_table = 'civicrm_participant' AND entity_id = {$participantId} AND $activityParams = array( 'source_contact_id' => $targetCid, 'source_record_id' => $srcRecId, - 'activity_type_id' => CRM_Core_OptionGroup::getValue('activity_type', - $activityType, - 'name' - ), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', $activityType), 'subject' => $subject, 'activity_date_time' => $date, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', - 'Completed', - 'name' - ), + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'), 'skipRecentView' => TRUE, ); @@ -2229,6 +2223,7 @@ WHERE (entity_table = 'civicrm_participant' AND entity_id = {$participantId} AND $activityParams['source_contact_id'] = $id; $activityParams['target_contact_id'][] = $targetCid; } + // @todo use api & also look at duplication of similar methods. CRM_Activity_BAO_Activity::create($activityParams); } diff --git a/CRM/Report/Form/Case/Demographics.php b/CRM/Report/Form/Case/Demographics.php index 22c0d33b2c..67a4aa5492 100644 --- a/CRM/Report/Form/Case/Demographics.php +++ b/CRM/Report/Form/Case/Demographics.php @@ -208,7 +208,7 @@ class CRM_Report_Form_Case_Demographics extends CRM_Report_Form { $this->_groupFilter = TRUE; $this->_tagFilter = TRUE; - $open_case_val = CRM_Core_OptionGroup::getValue('activity_type', 'Open Case', 'name'); + $open_case_val = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Open Case'); $crmDAO = &CRM_Core_DAO::executeQuery("SELECT cg.table_name, cg.extends AS ext, cf.label, cf.column_name FROM civicrm_custom_group cg INNER JOIN civicrm_custom_field cf ON cg.id = cf.custom_group_id where (cg.extends='Contact' OR cg.extends='Individual' OR cg.extends_entity_column_value='$open_case_val') AND cg.is_active=1 AND cf.is_active=1 ORDER BY cg.table_name"); $curTable = ''; diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php index 6307c3de42..77acd3d2cd 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php @@ -36,7 +36,6 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase { */ public function tearDown() { $this->quickCleanUpFinancialEntities(); - $this->quickCleanUpFinancialEntities(array('civicrm_event')); parent::tearDown(); } @@ -800,11 +799,9 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; // Check amount in activity. $activityParams = array( 'source_record_id' => $contribution->id, - 'activity_type_id' => CRM_Core_OptionGroup::getValue('activity_type', - 'Contribution', - 'name' - ), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Contribution'), ); + // @todo use api instead. $activity = CRM_Activity_BAO_Activity::retrieve($activityParams, $defaults); $this->assertEquals($contribution->id, $activity->source_record_id, 'Check for activity associated with contribution.'); diff --git a/tests/phpunit/api/v3/ActionScheduleTest.php b/tests/phpunit/api/v3/ActionScheduleTest.php index a96ecbf5a7..c455aa971e 100644 --- a/tests/phpunit/api/v3/ActionScheduleTest.php +++ b/tests/phpunit/api/v3/ActionScheduleTest.php @@ -52,15 +52,13 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { $oldCount = CRM_Core_DAO::singleValueQuery('select count(*) from civicrm_action_schedule'); $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts); - $scheduledStatus = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name'); - $activityTypeId = CRM_Core_OptionGroup::getValue('activity_type', "Meeting", 'name'); $title = "simpleActionSchedule" . substr(sha1(rand()), 0, 7); $params = array( 'title' => $title, 'recipient' => $assigneeID, 'limit_to' => 1, - 'entity_value' => $activityTypeId, - 'entity_status' => $scheduledStatus, + 'entity_value' => 'Meeting', + 'entity_status' => 'Scheduled', 'is_active' => 1, 'record_activity' => 1, 'start_action_date' => 'activity_date_time', @@ -91,15 +89,13 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { $oldCount = CRM_Core_DAO::singleValueQuery('select count(*) from civicrm_action_schedule'); $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts); - $scheduledStatus = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name'); - $activityTypeId = CRM_Core_OptionGroup::getValue('activity_type', "Meeting", 'name'); $title = "simpleActionSchedule" . substr(sha1(rand()), 0, 7); $params = array( 'title' => $title, 'recipient' => $assigneeID, 'limit_to' => 1, - 'entity_value' => $activityTypeId, - 'entity_status' => $scheduledStatus, + 'entity_value' => 'Meeting', + 'entity_status' => 'Scheduled', 'is_active' => 1, 'record_activity' => 1, 'mapping_id' => CRM_Activity_ActionMapping::ACTIVITY_MAPPING_ID, -- 2.25.1