From a13c171d740f1db886724d6ed8c224476f210214 Mon Sep 17 00:00:00 2001 From: Camilo Rodriguez Date: Fri, 25 Aug 2017 15:16:28 +0000 Subject: [PATCH] CRM-12167: Finish Implementing Visibility per Price Option Added validations so that: - A public price field has at least one public option - An admin price field has all admin options Implemented removal for Select and Checkbox field types. Also added condition to remove Admin values only if logged user doen't have appropriate permissions. Implemented different permissions check to see if admin options should be deleted or not if the price set is being used for a contribution page or an event page. Implemented functionality so that if a user chooses Admin visibility for the price field, all options are forced to have Admin visibility as well. Added default visibility as public on creation, in case it's not set, and protected access to unexisting visibility_id array key. Moved SQL to add new visibility field to PHP upgrader for v4.7.26. --- CRM/Event/Form/Registration/Register.php | 14 +++++++ CRM/Price/BAO/PriceField.php | 42 +++++++++++++++++-- CRM/Price/BAO/PriceSet.php | 15 +++++++ CRM/Price/Form/Field.php | 32 +++++++++++++- CRM/Price/Form/Option.php | 21 ++++++++++ CRM/Upgrade/Incremental/php/FourSeven.php | 2 + CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl | 4 -- templates/CRM/Price/Form/Field.tpl | 27 ++++++++++++ templates/CRM/Price/Form/Option.tpl | 2 +- templates/CRM/Price/Form/OptionFields.tpl | 2 +- templates/CRM/Price/Form/PriceSet.tpl | 9 ---- templates/CRM/Price/Page/Field.hlp | 9 +++- .../CRM/Price/BAO/PriceFieldValueTest.php | 1 + tests/phpunit/CRM/Price/Form/FieldTest.php | 13 +++--- tests/phpunit/CRM/Price/Form/OptionTest.php | 5 ++- 15 files changed, 170 insertions(+), 28 deletions(-) mode change 100755 => 100644 CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl diff --git a/CRM/Event/Form/Registration/Register.php b/CRM/Event/Form/Registration/Register.php index c9114de5df..f2418bfc02 100644 --- a/CRM/Event/Form/Registration/Register.php +++ b/CRM/Event/Form/Registration/Register.php @@ -566,6 +566,11 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration { $adminFieldVisible = TRUE; } + $hideAdminValues = TRUE; + if (CRM_Core_Permission::check('edit event participants')) { + $hideAdminValues = FALSE; + } + foreach ($form->_feeBlock as $field) { // public AND admin visibility fields are included for back-office registration and back-office change selections if (CRM_Utils_Array::value('visibility', $field) == 'public' || @@ -583,9 +588,18 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration { //user might modified w/ hook. $options = CRM_Utils_Array::value('options', $field); + $formClasses = array('CRM_Event_Form_Participant', 'CRM_Event_Form_ParticipantFeeSelection'); + if (!is_array($options)) { continue; } + elseif ($hideAdminValues && !in_array($className, $formClasses)) { + foreach ($options as $key => $currentOption) { + if ($currentOption['visibility_id'] == CRM_Price_BAO_PriceField::getVisibilityOptionID('admin')) { + unset($options[$key]); + } + } + } $optionFullIds = CRM_Utils_Array::value('option_full_ids', $field, array()); diff --git a/CRM/Price/BAO/PriceField.php b/CRM/Price/BAO/PriceField.php index 4a5c4649b5..025f62e3ee 100644 --- a/CRM/Price/BAO/PriceField.php +++ b/CRM/Price/BAO/PriceField.php @@ -41,6 +41,13 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { protected $_options; + /** + * List of visibility option ID's, of the form name => ID + * + * @var array + */ + private static $visibilityOptionsKeys; + /** * Takes an associative array and creates a price field object. * @@ -75,6 +82,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { * (reference) an assoc array of name/value pairs. * * @return CRM_Price_DAO_PriceField + * @throws \CRM_Core_Exception */ public static function create(&$params) { if (empty($params['id']) && empty($params['name'])) { @@ -147,7 +155,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { 'is_default' => CRM_Utils_Array::value($params['option_weight'][$index], $defaultArray) ? $defaultArray[$params['option_weight'][$index]] : 0, 'membership_num_terms' => NULL, 'non_deductible_amount' => CRM_Utils_Array::value('non_deductible_amount', $params), - 'visibility_id' => $params['option_visibility_id'][$index], + 'visibility_id' => CRM_Utils_Array::value($index, CRM_Utils_Array::value('option_visibility_id', $params), self::getVisibilityOptionID('public')), ); if ($options['membership_type_id']) { @@ -436,7 +444,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { $visibility_id = $opt['visibility_id']; } else { - $visibility_id = 1; + $visibility_id = self::getVisibilityOptionID('public'); } $extra = array( 'price' => json_encode(array($elementName, $priceVal)), @@ -548,7 +556,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { $visibility_id = $opt['visibility_id']; } else { - $visibility_id = 1; + $visibility_id = self::getVisibilityOptionID('public'); } $element = &$qf->add('select', $elementName, $label, array( @@ -596,6 +604,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { 'price' => json_encode(array($opt['id'], $priceVal)), 'data-amount' => $opt[$valueFieldName], 'data-currency' => $currencyName, + 'visibility' => $opt['visibility_id'], ) ); if ($is_pay_later) { @@ -873,4 +882,31 @@ WHERE id IN (" . implode(',', array_keys($priceFields)) . ')'; return $label; } + /** + * Given the name of a visibility option, returns its ID. + * + * @param string $visibilityName + * + * @return int + */ + public static function getVisibilityOptionID($visibilityName) { + + if (!isset(self::$visibilityOptionsKeys)) { + self::$visibilityOptionsKeys = CRM_Price_BAO_PriceField::buildOptions( + 'visibility_id', + NULL, + array( + 'labelColumn' => 'name', + 'flip' => TRUE, + ) + ); + } + + if (isset(self::$visibilityOptionsKeys[$visibilityName])) { + return self::$visibilityOptionsKeys[$visibilityName]; + } + + return 0; + } + } diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index 7d92106f67..e1dabf5202 100644 --- a/CRM/Price/BAO/PriceSet.php +++ b/CRM/Price/BAO/PriceSet.php @@ -1033,6 +1033,11 @@ WHERE id = %1"; $adminFieldVisible = TRUE; } + $hideAdminValues = TRUE; + if (CRM_Core_Permission::check('edit contributions')) { + $hideAdminValues = FALSE; + } + foreach ($feeBlock as $id => $field) { if (CRM_Utils_Array::value('visibility', $field) == 'public' || (CRM_Utils_Array::value('visibility', $field) == 'admin' && $adminFieldVisible == TRUE) || @@ -1046,9 +1051,19 @@ WHERE id = %1"; $form->assign('ispricelifetime', TRUE); } } + + $formClasses = array('CRM_Contribute_Form_Contribution', 'CRM_Member_Form_Membership'); + if (!is_array($options) || !in_array($id, $validPriceFieldIds)) { continue; } + elseif ($hideAdminValues && !in_array($className, $formClasses)) { + foreach ($options as $key => $currentOption) { + if ($currentOption['visibility_id'] == CRM_Price_BAO_PriceField::getVisibilityOptionID('admin')) { + unset($options[$key]); + } + } + } if (!empty($options)) { CRM_Price_BAO_PriceField::addQuickFormElement($form, 'price_' . $field['id'], diff --git a/CRM/Price/Form/Field.php b/CRM/Price/Form/Field.php index ac32ccff26..75ce1a44b1 100644 --- a/CRM/Price/Form/Field.php +++ b/CRM/Price/Form/Field.php @@ -442,9 +442,10 @@ class CRM_Price_Form_Field extends CRM_Core_Form { if ($form->_action & CRM_Core_Action::ADD) { if ($fields['html_type'] != 'Text') { $countemptyrows = 0; - $_flagOption = $_rowError = 0; + $publicOptionCount = $_flagOption = $_rowError = 0; $_showHide = new CRM_Core_ShowHideBlocks('', ''); + $visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, array('labelColumn' => 'name')); for ($index = 1; $index <= self::NUM_OPTION; $index++) { @@ -530,6 +531,12 @@ class CRM_Price_Form_Field extends CRM_Core_Form { $_showHide->addHide($hideBlock); } + if (!empty($fields['option_visibility_id'][$index]) && (!$noLabel || !$noAmount)) { + if ($visibilityOptions[$fields['option_visibility_id'][$index]] == 'public') { + $publicOptionCount++; + } + } + $_flagOption = $_emptyRow = 0; } @@ -577,6 +584,29 @@ class CRM_Price_Form_Field extends CRM_Core_Form { $errors['option_label[1]'] = $errors['option_amount[1]'] = ts('Label and value cannot be empty.'); $_flagOption = 1; } + + if ($visibilityOptions[$fields['visibility_id']] == 'public' && $publicOptionCount == 0) { + $errors['visibility_id'] = ts('You have selected to make this field public but have not enabled any public price options. Please update your selections to include a public price option, or make this field admin visibility only.'); + for ($index = 1; $index <= self::NUM_OPTION; $index++) { + if (!empty($fields['option_label'][$index]) || !empty($fields['option_amount'][$index])) { + $errors["option_visibility_id[{$index}]"] = ts('Public field should at least have one public option.'); + } + } + } + + if ($visibilityOptions[$fields['visibility_id']] == 'admin' && $publicOptionCount > 0) { + $errors['visibility_id'] = ts('Field with \'Admin\' visibility should only contain \'Admin\' options.'); + + for ($index = 1; $index <= self::NUM_OPTION; $index++) { + + $isOptionSet = !empty($fields['option_label'][$index]) || !empty($fields['option_amount'][$index]); + $currentOptionVisibility = CRM_Utils_Array::value($fields['option_visibility_id'][$index], $visibilityOptions); + + if ($isOptionSet && $currentOptionVisibility == 'public') { + $errors["option_visibility_id[{$index}]"] = ts('\'Admin\' field should only have \'Admin\' visibility options.'); + } + } + } } elseif (!empty($fields['max_value']) && !empty($fields['count']) && diff --git a/CRM/Price/Form/Option.php b/CRM/Price/Form/Option.php index fbb2048714..022697a744 100644 --- a/CRM/Price/Form/Option.php +++ b/CRM/Price/Form/Option.php @@ -291,6 +291,27 @@ class CRM_Price_Form_Option extends CRM_Core_Form { ) { $errors['count'] = ts('Participant count can not be greater than max participants.'); } + + $priceField = CRM_Price_BAO_PriceField::findById($fields['fieldId']); + $visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, array('labelColumn' => 'name')); + + $publicCount = 0; + $options = CRM_Price_BAO_PriceField::getOptions($priceField->id); + foreach ($options as $currentOption) { + if ($fields['optionId'] == $currentOption['id'] && $visibilityOptions[$fields['visibility_id']] == 'public') { + $publicCount++; + } + elseif ($fields['optionId'] != $currentOption['id'] && $visibilityOptions[$currentOption['visibility_id']] == 'public') { + $publicCount++; + } + } + if ($visibilityOptions[$priceField->visibility_id] == 'public' && $publicCount == 0) { + $errors['visibility_id'] = ts('All other options for this \'Public\' field have \'Admin\' visibility. There should at least be one \'Public\' option, or make the field \'Admin\' only.'); + } + elseif ($visibilityOptions[$priceField->visibility_id] == 'admin' && $publicCount > 0) { + $errors['visibility_id'] = ts('You must choose \'Admin\' visibility for this price option, as it belongs to a field with \'Admin\' visibility.'); + } + return empty($errors) ? TRUE : $errors; } diff --git a/CRM/Upgrade/Incremental/php/FourSeven.php b/CRM/Upgrade/Incremental/php/FourSeven.php index a8a23363c4..50c28a9949 100644 --- a/CRM/Upgrade/Incremental/php/FourSeven.php +++ b/CRM/Upgrade/Incremental/php/FourSeven.php @@ -451,6 +451,8 @@ class CRM_Upgrade_Incremental_php_FourSeven extends CRM_Upgrade_Incremental_Base $this->addTask('CRM-21195 - Add icon field to civicrm_navigation', 'addColumn', 'civicrm_navigation', 'icon', "varchar(255) NULL DEFAULT NULL COMMENT 'CSS class name for an icon'"); + $this->addTask('CRM-12167 - Add visibility column to civicrm_price_field_value', 'addColumn', + 'civicrm_price_field_value', 'visibility_id', 'int(10) unsigned DEFAULT 1 COMMENT "Implicit FK to civicrm_option_group with name = \'visibility\'"'); } /* diff --git a/CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl b/CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl old mode 100755 new mode 100644 index 55dede9e34..dbedca1b2f --- a/CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/4.7.23.mysql.tpl @@ -41,7 +41,3 @@ ON price_field.price_field_id = cpf.id LEFT JOIN civicrm_price_set ps ON ps.id = cpf.price_set_id SET cpf.is_active = 1 WHERE ps.is_quick_config = 1 AND cpf.is_active = 0; - --- CRM-12167 -ALTER TABLE `civicrm_price_field_value` -ADD COLUMN `visibility_id` int(10); diff --git a/templates/CRM/Price/Form/Field.tpl b/templates/CRM/Price/Form/Field.tpl index cddc9e0a35..8e18ab3e55 100644 --- a/templates/CRM/Price/Form/Field.tpl +++ b/templates/CRM/Price/Form/Field.tpl @@ -65,6 +65,33 @@ } } + + var adminVisibilityID = 0; + cj('#visibility_id').on('change', function () { + if (adminVisibilityID == 0) { + CRM.api3('OptionValue', 'getvalue', { + 'sequential': 1, + 'return': 'value', + 'option_group_id': 'visibility', + 'name': 'admin' + }).done(function(result) { + adminVisibilityID = result.result; + if (cj('#visibility_id').val() == adminVisibilityID) { + updateVisibilitySelects(adminVisibilityID); + } + }); + } else { + if (cj('#visibility_id').val() == adminVisibilityID) { + updateVisibilitySelects(adminVisibilityID); + } + } + }); + + function updateVisibilitySelects(value) { + for (var i=1; i<=15; i++) { + cj('#option_visibility_id_' + i).val(value); + } + } {/literal}
diff --git a/templates/CRM/Price/Form/Option.tpl b/templates/CRM/Price/Form/Option.tpl index addfdc2df7..767c02c6dd 100644 --- a/templates/CRM/Price/Form/Option.tpl +++ b/templates/CRM/Price/Form/Option.tpl @@ -108,7 +108,7 @@ {/if} {$form.visibility_id.label} -  {$form.visibility_id.html} {help id="id-visibility"} +  {$form.visibility_id.html} {help id="id-visibility-options" file="CRM/Price/Page/Field.hlp"} diff --git a/templates/CRM/Price/Form/OptionFields.tpl b/templates/CRM/Price/Form/OptionFields.tpl index 0b11d28bbc..2cc2b7e006 100644 --- a/templates/CRM/Price/Form/OptionFields.tpl +++ b/templates/CRM/Price/Form/OptionFields.tpl @@ -48,7 +48,7 @@ {ts}Max Participant{/ts} {help id="id-participant-max"} {/if} {ts}Order{/ts} - {ts}Visibility{/ts} + {ts}Visibility{/ts} {help id="id-visibility-options"} {ts}Active?{/ts} diff --git a/templates/CRM/Price/Form/PriceSet.tpl b/templates/CRM/Price/Form/PriceSet.tpl index 87210e0a22..4979744ab1 100644 --- a/templates/CRM/Price/Form/PriceSet.tpl +++ b/templates/CRM/Price/Form/PriceSet.tpl @@ -61,15 +61,6 @@ {/if} {/if} {/foreach} - {literal} - - {/literal} {if $element.help_post}
{$element.help_post}
{/if} diff --git a/templates/CRM/Price/Page/Field.hlp b/templates/CRM/Price/Page/Field.hlp index d07006d441..ec2097e4c2 100644 --- a/templates/CRM/Price/Page/Field.hlp +++ b/templates/CRM/Price/Page/Field.hlp @@ -37,6 +37,13 @@ {ts}You may enter a negative amount value if the field or option is being used to discount the total price.{/ts} {/htxt} +{htxt id="id-visibility-options-title"} + {ts}Visibility per Option{/ts} +{/htxt} +{htxt id="id-visibility-options"} + {ts}Select between Admin/Public for each option. 'Public' will show the option to all users accessing the price set. Use 'Admin' to limit the option to users with the 'CiviEvent: edit event participants’ permission (when viewing the field via the event registration pages) or the ‘CiviContribute: edit contributions’ permission (when accessing the field through a contribution page).{/ts} +{/htxt} + {htxt id="id-participant-count-title"} {ts}Participant Count{/ts} {/htxt} @@ -56,7 +63,7 @@ {ts}Visibility{/ts} {/htxt} {htxt id="id-visibility"} - {ts}Fields with 'Public' visibility will be displayed on your Event Information page AND will be available in the online registration (self-service) form. For some events you may want to allow staff to select special options with special pricing and / or discounts (using negative price set field values). Select 'Admin' visibility for these Price Fields. They will only be included when staff or volunteers are registering participants from the back-office 'Register Event Participants' screen.{/ts} + {ts}Fields with 'Public' visibility will be displayed on your Event Information page AND will be available in the online registration (self-service) form. For some events you may want to allow staff to select special options with special pricing and / or discounts (using negative price set field values). Select 'Admin' visibility for these Price Fields. They will only be included when staff or volunteers are registering participants from the back-office 'Register Event Participants' screen. If the parent price field visibility is ‘Public’ then it should be possible to set individual price options as 'Admin', but it should have at least one 'Public' option. If the parent price field visibility is 'Admin', then all the price field options should be set to 'Admin', including any new options added to the form.{/ts} {/htxt} {htxt id="id-member-price-options-title"} diff --git a/tests/phpunit/CRM/Price/BAO/PriceFieldValueTest.php b/tests/phpunit/CRM/Price/BAO/PriceFieldValueTest.php index c8d3252b8a..ad083040b2 100755 --- a/tests/phpunit/CRM/Price/BAO/PriceFieldValueTest.php +++ b/tests/phpunit/CRM/Price/BAO/PriceFieldValueTest.php @@ -54,4 +54,5 @@ class CRM_Price_BAO_PriceFieldValueTest extends CiviUnitTestCase { $this->assertArrayKeyExists('visibility_id', $fields); $this->assertEquals('visibility', $fields['visibility_id']['pseudoconstant']['optionGroupName']); } + } diff --git a/tests/phpunit/CRM/Price/Form/FieldTest.php b/tests/phpunit/CRM/Price/Form/FieldTest.php index f87528f1a2..d1c13513bc 100755 --- a/tests/phpunit/CRM/Price/Form/FieldTest.php +++ b/tests/phpunit/CRM/Price/Form/FieldTest.php @@ -71,14 +71,15 @@ class CRM_Price_Form_FieldTest extends CiviUnitTestCase { ); for ($index = 1; $index <= CRM_Price_Form_Field::NUM_OPTION; $index++) { - $defaultParams['option_label'][$index] = null; - $defaultParams['option_value'][$index] = null; - $defaultParams['option_name'][$index] = null; - $defaultParams['option_weight'][$index] = null; - $defaultParams['option_amount'][$index] = null; - $defaultParams['option_visibility_id'][$index] = null; + $defaultParams['option_label'][$index] = NULL; + $defaultParams['option_value'][$index] = NULL; + $defaultParams['option_name'][$index] = NULL; + $defaultParams['option_weight'][$index] = NULL; + $defaultParams['option_amount'][$index] = NULL; + $defaultParams['option_visibility_id'][$index] = NULL; } return array_merge($defaultParams, $params); } + } diff --git a/tests/phpunit/CRM/Price/Form/OptionTest.php b/tests/phpunit/CRM/Price/Form/OptionTest.php index 8b31935759..4d297b86c4 100755 --- a/tests/phpunit/CRM/Price/Form/OptionTest.php +++ b/tests/phpunit/CRM/Price/Form/OptionTest.php @@ -82,10 +82,11 @@ class CRM_Price_Form_OptionTest extends CiviUnitTestCase { foreach ($this->priceFieldValues as $currentField) { if ($this->visibilityOptions[$currentField['visibility_id']] == 'public') { $this->publicValue = $currentField; - } else { + } + else { $this->adminValue = $currentField; } } - } + } -- 2.25.1