CRM-21065 Replace some CRM_Core_OptionGroup::values with CRM_Activity_BAO_Activ… #10860

mattwire · 2017-08-15T10:25:46Z

…ityContact::buildOptions

Overview

Replace CRM_Core_OptionGroup::values with CRM_Activity_BAO_ActivityContact::buildOptions where it is safe to do so (ie. parameters map directly). There are a few which have not been changed as they require a little more analysis of the passed parameters to ensure they are still returning the correct options.

Before

Old non-caching methods in use

After

New caching methods in use. Cleaner code.

Technical Details

Non functional change. Use a the newer buildOptions methods to return the same data as the older CRM_Core_OptionGroup::values methods.

Comments

All tests should pass, functionally there is no change.

…ityContact::buildOptions

eileenmcnaughton · 2017-08-23T05:33:52Z

CRM/Activity/ActionMapping.php

@@ -108,7 +108,7 @@ public function createQuery($schedule, $phase, $defaultParams) {
    $query['casDateField'] = 'e.activity_date_time';

    if (!is_null($schedule->limit_to)) {
-      $activityContacts = \CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = \CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


Putting this here for easy reference

public static function &values( $name, $flip = FALSE, $grouping = FALSE, $localize = FALSE, $condition = NULL, $labelColumnName = 'label', $onlyActive = TRUE, $fresh = FALSE, $keyColumnName = 'value', $orderBy = 'weight')

Here is the definition of the contexts for buildOptions

$contexts = array( 'get' => "get: all options are returned, even if they are disabled; labels are translated.", 'create' => "create: options are filtered appropriately for the object being created/updated; labels are translated.", 'search' => "search: searchable options are returned; labels are translated.", 'validate' => "validate: all options are returned, even if they are disabled; machine names are used in place of labels.", 'abbreviate' => "abbreviate: enabled options are returned; labels are replaced with abbreviations.", 'match' => "match: enabled options are returned using machine names as keys; labels are translated.", );

In the case of this change the $onlyActive is TRUE in the existing code & FALSE in the changed version, but I don't find it valid that we should be disabling ActivityContact record types.

Definition of record_type_id DOES refer to activity_contacts

<field> <name>record_type_id</name> <type>int unsigned</type> <title>Record Type ID</title> <comment>Nature of this contact's role in the activity: 1 assignee, 2 creator, 3 focus or target.</comment> <pseudoconstant> <optionGroupName>activity_contacts</optionGroupName> </pseudoconstant> <html> <type>Select</type> </html> <add>4.4</add> </field>

Summary of above - change looks good :-)

eileenmcnaughton · 2017-08-23T05:34:41Z

CRM/Activity/BAO/Activity.php

@@ -92,7 +92,7 @@ public static function retrieve(&$params, &$defaults) {
    $activity->copyValues($params);

    if ($activity->find(TRUE)) {
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


same as above = checked

eileenmcnaughton · 2017-08-23T05:34:50Z

CRM/Activity/BAO/Activity.php

@@ -209,7 +209,7 @@ public static function deleteActivity(&$params, $moveToTrash = FALSE) {
      $logMsg = 'Case Activity deleted for';
      $msgs = array();

-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


same as above = checked

eileenmcnaughton · 2017-08-23T05:34:54Z

CRM/Activity/BAO/Activity.php

@@ -633,7 +633,7 @@ public static function create(&$params) {
  public static function logActivityAction($activity, $logMessage = NULL) {
    $id = CRM_Core_Session::getLoggedInContactID();
    if (!$id) {
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


same as above = checked

eileenmcnaughton · 2017-08-23T05:47:25Z

CRM/Activity/BAO/Activity.php

@@ -1888,7 +1888,7 @@ public static function sendMessage(
      $toDisplayName = $toEmail;
    }

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:47:32Z

CRM/Activity/BAO/Activity.php

@@ -886,7 +886,7 @@ public static function deprecatedGetActivities($input) {
      'Bulk Email'
    );

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:55:12Z

CRM/Activity/BAO/Activity.php

@@ -1824,7 +1824,7 @@ public static function sendSMSMessage(
      return $sendResult;
    }

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:55:16Z

CRM/Activity/BAO/Activity.php

@@ -1307,7 +1307,7 @@ public static function deprecatedGetActivitySQLClause($input) {
    // build main activity table select clause
    $sourceSelect = '';

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:55:20Z

CRM/Activity/BAO/Activity.php

@@ -672,7 +672,7 @@ public static function getActivities($params, $getCount = FALSE) {
    $activities = array();

    // fetch all active activity types
-    $activityTypes = CRM_Core_OptionGroup::values('activity_type');
+    $activityTypes = CRM_Activity_BAO_Activity::buildOptions('activity_type_id', 'get');


Implicit in the first line is $labelColumnName = 'label'
The use of get as context has the same effect

I'm finding the way this parameter is used in the function kinda odd - ie

elseif (!empty($activityTypes) && count($activityTypes)) { $activityParams['activity_type_id'] = array('IN' => array_keys($activityTypes)); }

seems to imply we want to exclude active ones here. I would be inclined to drop this from the PR to get the others through without this holding them up

@eileenmcnaughton I've removed from this PR and raised a more complex change for this line in https://issues.civicrm.org/jira/browse/CRM-21111 - PR #10909 which should also improve performance of that function

eileenmcnaughton · 2017-08-23T05:59:12Z

CRM/Activity/BAO/Activity.php

@@ -1990,7 +1990,7 @@ public static function &importableFields($status = FALSE) {
  public static function getContactActivity($contactId) {
    // @todo remove this function entirely.
    $activities = array();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:59:21Z

CRM/Activity/BAO/Activity.php

@@ -2610,7 +2610,7 @@ public static function cleanupActivity($contactId) {
    if (!$contactId) {
      return $result;
    }
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T05:59:30Z

CRM/Activity/BAO/Activity.php

@@ -2745,7 +2745,7 @@ public static function checkPermission($activityId, $action) {
      $permission = CRM_Core_Permission::EDIT;
    }

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:02:09Z

CRM/Activity/BAO/Activity.php

@@ -3083,7 +3083,7 @@ public static function getActivityContact($activityId, $recordTypeID = NULL, $co
  public static function getSourceContactID($activityId) {
    static $sourceID = NULL;
    if (!$sourceID) {
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:02:14Z

CRM/Activity/BAO/Activity.php

@@ -3100,7 +3100,7 @@ public static function getSourceContactID($activityId) {
  public function setApiFilter(&$params) {
    if (!empty($params['target_contact_id'])) {
      $this->selectAdd();
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:02:41Z

CRM/Activity/BAO/Activity.php

-    $activityTypes = CRM_Core_OptionGroup::values('activity_type');
-    $activityStatuses = CRM_Core_OptionGroup::values('activity_status');
+    $activityTypes = CRM_Activity_BAO_Activity::buildOptions('activity_type_id', 'get');
+    $activityStatuses = CRM_Activity_BAO_Activity::buildOptions('activity_status_id', 'get');


yes, this looks correct, being used for rendering a label within the results. CRM_Core_Pseudoconstant::getLabel would make even more sense in the function context

@eileenmcnaughton As it's trivial and neater to use getLabel here I've updated the PR to do so!

eileenmcnaughton · 2017-08-23T06:02:51Z

CRM/Activity/BAO/ActivityAssignment.php

@@ -54,7 +54,7 @@ public function __construct() {
   */
  public static function create(&$params) {
    $assignment = new CRM_Activity_BAO_ActivityContact();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:02:58Z

CRM/Activity/BAO/ActivityTarget.php

@@ -53,7 +53,7 @@ public function __construct() {
   */
  public static function create(&$params) {
    $target = new CRM_Activity_BAO_ActivityContact();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:10Z

CRM/Activity/BAO/ActivityTarget.php

@@ -74,7 +74,7 @@ public static function retrieveTargetIdsByActivityId($activity_id) {
      return $targetArray;
    }

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:15Z

CRM/Activity/BAO/ActivityTarget.php

@@ -105,7 +105,7 @@ public static function getTargetNames($activityID) {
    if (empty($activityID)) {
      return $targetNames;
    }
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:23Z

CRM/Activity/BAO/Query.php

@@ -247,7 +247,7 @@ public static function whereClauseSingle(&$values, &$query) {

      case 'activity_role':
        CRM_Contact_BAO_Query::$_activityRole = $values[2];
-        $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+        $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:28Z

CRM/Activity/BAO/Query.php

@@ -399,7 +399,7 @@ public static function from($name, $mode, $side) {
        break;

      case 'source_contact':
-        $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+        $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:37Z

CRM/Activity/Form/Activity.php

@@ -986,7 +986,7 @@ public function postProcess($params = NULL) {
   */
  protected function processActivity(&$params) {
    $activityAssigned = array();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:44Z

CRM/Activity/Form/Task.php

@@ -164,7 +164,7 @@ public static function preProcessCommon(&$form, $useTable = FALSE) {
  public function setContactIDs() {
    $IDs = implode(',', $this->_activityHolderIds);

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:49Z

CRM/Activity/Form/Task/PickOption.php

@@ -131,7 +131,7 @@ public function postProcess() {
    $params = $this->exportValues();
    $this->_contacts = array();

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:03:56Z

CRM/Activity/Form/Task/SearchTaskHookSample.php

@@ -46,7 +46,7 @@ public function preProcess() {
    // display name and activity details of all selected contacts
    $activityIDs = implode(',', $this->_activityHolderIds);

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:04:03Z

CRM/Activity/Page/AJAX.php

@@ -339,7 +339,7 @@ public static function _convertToCaseActivity($params) {
      $targetContacts = array_unique(explode(',', $params['targetContactIds']));
    }

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:04:09Z

CRM/Activity/Selector/Search.php

@@ -269,7 +269,7 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
    $allCampaigns = CRM_Campaign_BAO_Campaign::getCampaigns(NULL, NULL, FALSE, FALSE, FALSE, TRUE);

    $engagementLevels = CRM_Campaign_PseudoConstant::engagementLevel();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:04:18Z

CRM/Campaign/BAO/Petition.php

@@ -240,7 +240,7 @@ public function confirmSignature($activity_id, $contact_id, $petition_id) {
    // change activity status to completed (status_id = 2)
    // I wonder why do we need contact_id when we have activity_id anyway? [chastell]
    $sql = 'UPDATE civicrm_activity SET status_id = 2 WHERE id = %1';
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:04:24Z

CRM/Campaign/BAO/Petition.php

@@ -307,7 +307,7 @@ public static function getPetitionSignatureTotalbyCountry($surveyId) {
                civicrm_survey.id =  %1 AND
                a.source_record_id =  %1  ";

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:09Z

CRM/Case/XMLProcessor/Report.php

@@ -425,7 +425,7 @@ public function &getActivity($clientID, $activityDAO, &$activityTypeInfo) {
      'value' => $this->redact($creator),
      'type' => 'String',
    );
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:15Z

CRM/Contact/BAO/Contact/Utils.php

@@ -702,7 +702,7 @@ public static function contactDetails($componentIds, $componentName, $returnProp
    }
    elseif ($componentName == 'Activity') {
      $compTable = 'civicrm_activity';
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:21Z

CRM/Contact/BAO/Query.php

@@ -3168,7 +3168,7 @@ public function tagSearch(&$values) {
      // search tag in activities
      $etActTable = "`civicrm_entity_act_tag-" . $value . "`";
      $tActTable = "`civicrm_act_tag-" . $value . "`";
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:28Z

CRM/Contact/BAO/Query.php

@@ -3227,7 +3227,7 @@ public function tag(&$values) {

      // search tag in cases
      $etCaseTable = "`civicrm_entity_case_tag-" . $value . "`";
-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:33Z

CRM/Contact/Form/Search/Custom/ActivitySearch.php

@@ -259,7 +259,7 @@ public function alterRow(&$row) {
   */
  public function from() {
    $this->buildACLClause('contact_a');
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:38Z

CRM/Contact/Form/Task/SMSCommon.php

@@ -140,7 +140,7 @@ public static function buildQuickForm(&$form) {
          continue;
        }

-        $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+        $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:45Z

CRM/Contribute/BAO/Contribution.php

@@ -2276,7 +2276,7 @@ public static function getOnbehalfIds($contributionId, $contributorId = NULL) {
   AND     civicrm_activity_contact.record_type_id = %3
 ";

-      $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+      $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:52Z

CRM/Friend/BAO/Friend.php

@@ -134,7 +134,7 @@ public static function create(&$params) {

    //activity creation
    $activity = CRM_Activity_BAO_Activity::create($activityParams);
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:06:59Z

CRM/Logging/Differ.php

@@ -106,7 +106,7 @@ public function diffsInTable($table, $contactID = NULL) {
          break;

        case 'civicrm_activity':
-          $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+          $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:04Z

CRM/Mailing/BAO/MailingJob.php

@@ -962,7 +962,7 @@ public function writeToDB(
        if (CRM_Core_BAO_Email::isMultipleBulkMail()) {
          static $targetRecordID = NULL;
          if (!$targetRecordID) {
-            $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+            $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:10Z

CRM/Report/Form/Activity.php

@@ -492,7 +492,7 @@ public function select($recordType = NULL) {
   * @param string $recordType
   */
  public function from($recordType) {
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:18Z

CRM/Report/Form/Activity.php

@@ -779,7 +779,7 @@ public function postProcess() {
      $this->_formValues["activity_date_time_relative"] = NULL;
    }
    $this->beginPostProcess();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:28Z

CRM/Report/Form/ActivitySummary.php

@@ -301,7 +301,7 @@ public function select() {
   * @param bool|FALSE $durationMode
   */
  public function from($durationMode = FALSE) {
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:41Z

CRM/Report/Form/Campaign/SurveyDetails.php

@@ -262,7 +262,7 @@ public function select() {

  public function from() {
    $this->_from = " FROM civicrm_contact {$this->_aliases['civicrm_contact']} {$this->_aclFrom} ";
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:49Z

CRM/Report/Form/Contact/Detail.php

@@ -514,7 +514,7 @@ public function from() {
      }

      if (!empty($this->_selectComponent['activity_civireport'])) {
-        $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+        $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:07:56Z

CRM/Report/Form/Contact/Log.php

@@ -175,7 +175,7 @@ public static function formRule($fields, $files, $self) {
  }

  public function from() {
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:05Z

CRM/Upgrade/Incremental/php/FourFour.php

@@ -543,7 +543,7 @@ public static function setContactImageUrl($cid, $newImageUrl) {
  public static function activityContacts(CRM_Queue_TaskContext $ctx) {
    $upgrade = new CRM_Upgrade_Form();

-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:11Z

CRM/Utils/Migrate/ExportJSON.php

@@ -449,7 +449,7 @@ public function relationship(&$contactIDs, &$additionalContacts) {
   */
  public function activity(&$contactIDs, &$additionalContacts) {
    static $_activitiesHandled = array();
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:17Z

sql/GenerateData.php

@@ -1199,7 +1199,7 @@ private function addActivity() {
    $contactDAO->find();

    $count = 0;
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:24Z

sql/GenerateData.php

@@ -1318,7 +1318,7 @@ private function addMembershipType() {
  private function addMembership() {
    $contact = new CRM_Contact_DAO_Contact();
    $contact->query("SELECT id FROM civicrm_contact where contact_type = 'Individual'");
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:29Z

sql/GenerateData.php

@@ -1693,7 +1693,7 @@ private function addParticipant() {
  (contact_id, activity_id, record_type_id)
 VALUES
 ";
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:34Z

sql/GenerateReportData.php

@@ -1188,7 +1188,7 @@ public function addActivity() {
    $contactDAO->find();

    $count = 0;
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:40Z

tests/phpunit/api/v3/ActionScheduleTest.php

@@ -50,7 +50,7 @@ public function setUp() {
   */
  public function testSimpleActionScheduleCreate() {
    $oldCount = CRM_Core_DAO::singleValueQuery('select count(*) from civicrm_action_schedule');
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:08:47Z

tests/phpunit/api/v3/ActionScheduleTest.php

@@ -87,7 +87,7 @@ public function testActionScheduleCreateWithoutRequired() {
   */
  public function testActionScheduleWithScheduledDatesCreate() {
    $oldCount = CRM_Core_DAO::singleValueQuery('select count(*) from civicrm_action_schedule');
-    $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+    $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');


eileenmcnaughton · 2017-08-23T06:11:25Z

@mattwire this is a good cleanup PR & replaces calls to a function that feels a bit nasty to one that is recommended & predictable.

There is one change where I'm not sure about the implicit change fro $onlyActive to not filtering by active. In the case of the others I feel I was able to feel safe that the changes were functionally equivalent. My suggestion is to remove that one from this PR & then we can merge this & work through that one separately (or leave it for now)

mattwire · 2017-08-28T17:25:24Z

@eileenmcnaughton Thanks so much for reviewing this PR! I've made two changes as requested/suggested. Assuming the checks pass I think it's good to merge.

eileenmcnaughton · 2017-08-28T21:09:11Z

done!

Replace some CRM_Core_OptionGroup::values with CRM_Activity_BAO_Activ…

44f817d

…ityContact::buildOptions

eileenmcnaughton reviewed Aug 23, 2017

View reviewed changes

Update based on comments from @eileenmcnaughton

f9aa1e8

eileenmcnaughton merged commit 443c556 into civicrm:master Aug 28, 2017

mattwire deleted the CRM-21065_optiongroup_to_pseudoconstant branch September 5, 2017 14:13

CRM-21065 Replace some CRM_Core_OptionGroup::values with CRM_Activity_BAO_Activ… #10860

CRM-21065 Replace some CRM_Core_OptionGroup::values with CRM_Activity_BAO_Activ… #10860

Conversation

mattwire commented Aug 15, 2017

Overview

Before

After

Technical Details

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Aug 23, 2017

mattwire commented Aug 28, 2017

eileenmcnaughton commented Aug 28, 2017