From 7621050bb932f0fde6e52f05290feafecc26ce47 Mon Sep 17 00:00:00 2001 From: JohnFF Date: Sat, 14 Jun 2014 00:25:55 +0100 Subject: [PATCH] Refactored ParticipantListing.php 1) Commuted assigns in if statements into assigning, then checking, as otherwise fails code validators and breaches CiviCRM coding conventions. 2) Refactored repeated code found while repairing those errors into a separate reusable function that accepts parameters. I only did this where the code was identical. Where there was any distinction I didn't do this to prevent introducing bugs. https://issues.civicrm.org/jira/browse/CRM-14434 --- CRM/Report/Form/Event/ParticipantListing.php | 129 ++++++++++--------- 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/CRM/Report/Form/Event/ParticipantListing.php b/CRM/Report/Form/Event/ParticipantListing.php index eb1840c446..11754022a6 100644 --- a/CRM/Report/Form/Event/ParticipantListing.php +++ b/CRM/Report/Form/Event/ParticipantListing.php @@ -459,7 +459,8 @@ GROUP BY cv.label } } //add blank column at the end - if ($blankcols = CRM_Utils_Array::value('blank_column_end', $this->_params)) { + $blankcols = CRM_Utils_Array::value('blank_column_end', $this->_params); + if ($blankcols) { for ($i = 1; $i <= $blankcols; $i++) { $select[] = " '' as blankColumnEnd_{$i}"; $this->_columnHeaders["blank_{$i}"]['title'] = "_ _ _ _"; @@ -601,6 +602,26 @@ GROUP BY cv.label $this->endPostProcess($rows); } + /** + * @param $rows + * @param $entryFound + * @param $row + * @param $rowId + * @param $rowNum + * @param $types + */ + private function _initBasicRow(&$rows, &$entryFound, $row, $rowId, $rowNum, $types){ + if (!array_key_exists($rowId, $row)) { + return FALSE; + } + + $value = $row[$rowId]; + if ($value) { + $rows[$rowNum][$rowId] = $types[$value]; + } + $entryFound = TRUE; + } + /** * @param $rows */ @@ -619,11 +640,12 @@ GROUP BY cv.label // make count columns point to detail report // convert display name to links if (array_key_exists('civicrm_participant_event_id', $row)) { - if ($value = $row['civicrm_participant_event_id']) { - $rows[$rowNum]['civicrm_participant_event_id'] = CRM_Event_PseudoConstant::event($value, FALSE); + $eventId = $row['civicrm_participant_event_id']; + if ($eventId]) { + $rows[$rowNum]['civicrm_participant_event_id'] = CRM_Event_PseudoConstant::event($eventId, FALSE); $url = CRM_Report_Utils_Report::getNextUrl('event/income', - 'reset=1&force=1&id_op=in&id_value=' . $value, + 'reset=1&force=1&id_op=in&id_value=' . $eventId, $this->_absoluteUrl, $this->_id, $this->_drilldownReport ); $rows[$rowNum]['civicrm_participant_event_id_link'] = $url; @@ -633,48 +655,47 @@ GROUP BY cv.label } // handle event type id - if (array_key_exists('civicrm_event_event_type_id', $row)) { - if ($value = $row['civicrm_event_event_type_id']) { - $rows[$rowNum]['civicrm_event_event_type_id'] = $eventType[$value]; - } - $entryFound = TRUE; - } + $this->_initBasicRow(&$rows, &$entryFound, $row, 'civicrm_event_event_type_id', $rowNum, $eventType); // handle participant status id if (array_key_exists('civicrm_participant_status_id', $row)) { - if ($value = $row['civicrm_participant_status_id']) { - $rows[$rowNum]['civicrm_participant_status_id'] = CRM_Event_PseudoConstant::participantStatus($value, FALSE, 'label'); + $statusId = $row['civicrm_participant_status_id']; + if ($statusId) { + $rows[$rowNum]['civicrm_participant_status_id'] = CRM_Event_PseudoConstant::participantStatus($statusId, FALSE, 'label'); } $entryFound = TRUE; } // handle participant role id if (array_key_exists('civicrm_participant_role_id', $row)) { - if ($value = $row['civicrm_participant_role_id']) { - $roles = explode(CRM_Core_DAO::VALUE_SEPARATOR, $value); - $value = array(); + $roleId = $row['civicrm_participant_role_id']; + if ($roleId) { + $roles = explode(CRM_Core_DAO::VALUE_SEPARATOR, $roleId); + $roleId = array(); foreach ($roles as $role) { - $value[$role] = CRM_Event_PseudoConstant::participantRole($role, FALSE); + $roleId[$role] = CRM_Event_PseudoConstant::participantRole($role, FALSE); } - $rows[$rowNum]['civicrm_participant_role_id'] = implode(', ', $value); + $rows[$rowNum]['civicrm_participant_role_id'] = implode(', ', $roleId); } $entryFound = TRUE; } // Handel value seperator in Fee Level if (array_key_exists('civicrm_participant_participant_fee_level', $row)) { - if ($value = $row['civicrm_participant_participant_fee_level']) { - CRM_Event_BAO_Participant::fixEventLevel($value); - $rows[$rowNum]['civicrm_participant_participant_fee_level'] = $value; + $feeLevel = $row['civicrm_participant_participant_fee_level']; + if ($feeLevel) { + CRM_Event_BAO_Participant::fixEventLevel($feeLevel); + $rows[$rowNum]['civicrm_participant_participant_fee_level'] = $feeLevel; } $entryFound = TRUE; } // Convert display name to link - if (($displayName = CRM_Utils_Array::value('civicrm_contact_sort_name_linked', $row)) && - ($cid = CRM_Utils_Array::value('civicrm_contact_id', $row)) && - ($id = CRM_Utils_Array::value('civicrm_participant_participant_record', $row)) - ) { + $displayName = CRM_Utils_Array::value('civicrm_contact_sort_name_linked', $row); + $cid = CRM_Utils_Array::value('civicrm_contact_id', $row); + $id = CRM_Utils_Array::value('civicrm_participant_participant_record', $row); + + if ($displayName && $cid && $id) { $url = CRM_Report_Utils_Report::getNextUrl('contact/detail', "reset=1&force=1&id_op=eq&id_value=$cid", $this->_absoluteUrl, $this->_id, $this->_drilldownReport @@ -696,26 +717,29 @@ GROUP BY cv.label // Handle country id if (array_key_exists('civicrm_address_country_id', $row)) { - if ($value = $row['civicrm_address_country_id']) { - $rows[$rowNum]['civicrm_address_country_id'] = CRM_Core_PseudoConstant::country($value, TRUE); + $countryId = $row['civicrm_address_country_id']; + if ($countryId) { + $rows[$rowNum]['civicrm_address_country_id'] = CRM_Core_PseudoConstant::country($countryId, TRUE); } $entryFound = TRUE; - } + } // Handle state/province id if (array_key_exists('civicrm_address_state_province_id', $row)) { - if ($value = $row['civicrm_address_state_province_id']) { - $rows[$rowNum]['civicrm_address_state_province_id'] = CRM_Core_PseudoConstant::stateProvince($value, TRUE); + $provinceId = $row['civicrm_address_state_province_id']; + if ($provinceId) { + $rows[$rowNum]['civicrm_address_state_province_id'] = CRM_Core_PseudoConstant::stateProvince($provinceId, TRUE); } $entryFound = TRUE; } // Handle employer id if (array_key_exists('civicrm_contact_employer_id', $row)) { - if ($value = $row['civicrm_contact_employer_id']) { - $rows[$rowNum]['civicrm_contact_employer_id'] = CRM_Contact_BAO_Contact::displayName($value); + $employerId = $row['civicrm_contact_employer_id']; + if ($employerId) { + $rows[$rowNum]['civicrm_contact_employer_id'] = CRM_Contact_BAO_Contact::displayName($employerId); $url = CRM_Utils_System::url('civicrm/contact/view', - 'reset=1&cid=' . $value, $this->_absoluteUrl + 'reset=1&cid=' . $employerId, $this->_absoluteUrl ); $rows[$rowNum]['civicrm_contact_employer_id_link'] = $url; $rows[$rowNum]['civicrm_contact_employer_id_hover'] = ts('View Contact Summary for this Contact.'); @@ -723,48 +747,25 @@ GROUP BY cv.label } // Convert campaign_id to campaign title - if (array_key_exists('civicrm_participant_campaign_id', $row)) { - if ($value = $row['civicrm_participant_campaign_id']) { - $rows[$rowNum]['civicrm_participant_campaign_id'] = $this->activeCampaigns[$value]; - $entryFound = TRUE; - } - } + $this->_initBasicRow($rows, $entryFound, 'civicrm_participant_campaign_id', $this->activeCampaigns, $row, $rowNum); // handle contribution status - if (array_key_exists('civicrm_contribution_contribution_status_id', $row)) { - if ($value = $row['civicrm_contribution_contribution_status_id']) { - $rows[$rowNum]['civicrm_contribution_contribution_status_id'] = $contributionStatus[$value]; - } - $entryFound = TRUE; - } + $this->_initBasicRow($rows, $entryFound, 'civicrm_contribution_contribution_status_id', $contributionStatus, $row, $rowNum); // handle payment instrument - if (array_key_exists('civicrm_contribution_payment_instrument_id', $row)) { - if ($value = $row['civicrm_contribution_payment_instrument_id']) { - $rows[$rowNum]['civicrm_contribution_payment_instrument_id'] = $paymentInstruments[$value]; - } - $entryFound = TRUE; - } + $this->_initBasicRow($rows, $entryFound, 'civicrm_contribution_payment_instrument_id', $paymentInstruments, $row, $rowNum); // handle financial type - if (array_key_exists('civicrm_contribution_financial_type_id', $row)) { - if ($value = $row['civicrm_contribution_financial_type_id']) { - $rows[$rowNum]['civicrm_contribution_financial_type_id'] = $financialTypes[$value]; - } - $entryFound = TRUE; - } + $this->_initBasicRow($rows, $entryFound, 'civicrm_contribution_financial_type_id', $financialTypes, $row, $rowNum); - if (array_key_exists('civicrm_contact_gender_id', $row)) { - if ($value = $row['civicrm_contact_gender_id']) { - $rows[$rowNum]['civicrm_contact_gender_id'] = $genders[$value]; - } - $entryFound = TRUE; - } + // handle gender id + $this->_initBasicRow($rows, $entryFound, 'civicrm_contact_gender_id', $genders, $row, $rowNum); // display birthday in the configured custom format if (array_key_exists('civicrm_contact_birth_date', $row)) { - if ($value = $row['civicrm_contact_birth_date']) { - $rows[$rowNum]['civicrm_contact_birth_date'] = CRM_Utils_Date::customFormat($row['civicrm_contact_birth_date'], '%Y%m%d'); + $birthDate = $row['civicrm_contact_birth_date']; + if ($birthDate) { + $rows[$rowNum]['civicrm_contact_birth_date'] = CRM_Utils_Date::customFormat($birthDate, '%Y%m%d'); } $entryFound = TRUE; } -- 2.25.1