Skip to content

Commit

Permalink
Merge pull request #19283 from eileenmcnaughton/facl
Browse files Browse the repository at this point in the history
Move financial acl warning from FinancialType BAO to extension.
  • Loading branch information
mattwire authored Dec 31, 2020
2 parents af5d7ae + d646594 commit 5e58aa7
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 15 deletions.
34 changes: 22 additions & 12 deletions CRM/Financial/BAO/FinancialType.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,30 @@ public static function setIsActive($id, $is_active) {
return CRM_Core_DAO::setFieldValue('CRM_Financial_DAO_FinancialType', $id, 'is_active', $is_active);
}

/**
* Create a financial type.
*
* @param array $params
*
* @return \CRM_Financial_DAO_FinancialType
*/
public static function create(array $params) {
$hook = empty($params['id']) ? 'create' : 'edit';
CRM_Utils_Hook::pre($hook, 'FinancialType', $params['id'] ?? NULL, $params);
$financialType = self::add($params);
CRM_Utils_Hook::post($hook, 'FinancialType', $financialType->id, $financialType);
return $financialType;
}

/**
* Add the financial types.
*
* Note that add functions are being deprecated in favour of create.
* The steps here are to remove direct calls to this function from
* core & then move the innids of the function to the create function.
* This function would remain for 6 months or so as a wrapper of create with
* a deprecation notice.
*
* @param array $params
* Reference array contains the values submitted by the form.
* @param array $ids
Expand All @@ -80,6 +101,7 @@ public static function setIsActive($id, $is_active) {
* @return object
*/
public static function add(&$params, &$ids = []) {
// @todo deprecate this, move the code to create & call create from add.
if (empty($params['id'])) {
$params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE);
$params['is_deductible'] = CRM_Utils_Array::value('is_deductible', $params, FALSE);
Expand All @@ -89,18 +111,6 @@ public static function add(&$params, &$ids = []) {
// action is taken depending upon the mode
$financialType = new CRM_Financial_DAO_FinancialType();
$financialType->copyValues($params);
if (!empty($ids['financialType'])) {
$financialType->id = $ids['financialType'] ?? NULL;
if (self::isACLFinancialTypeStatus()) {
$prevName = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType', $financialType->id, 'name');
if ($prevName != $params['name']) {
CRM_Core_Session::setStatus(ts("Changing the name of a Financial Type will result in losing the current permissions associated with that Financial Type.
Before making this change you should likely note the existing permissions at Administer > Users and Permissions > Permissions (Access Control),
then clicking the Access Control link for your Content Management System, then noting down the permissions for 'CiviCRM: {financial type name} view', etc.
Then after making the change of name, reset the permissions to the way they were."), ts('Warning'), 'warning');
}
}
}
$financialType->save();
// CRM-12470
if (empty($ids['financialType']) && empty($params['id'])) {
Expand Down
11 changes: 10 additions & 1 deletion ext/financialacls/financialacls.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,15 @@ function financialacls_civicrm_pre($op, $objectName, $id, &$params) {
throw new API_Exception('You do not have permission to ' . $op . ' this line item');
}
}
if ($objectName === 'FinancialType' && !empty($params['id']) && !empty($params['name'])) {
$prevName = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType', $params['id']);
if ($prevName !== $params['name']) {
CRM_Core_Session::setStatus(ts("Changing the name of a Financial Type will result in losing the current permissions associated with that Financial Type.
Before making this change you should likely note the existing permissions at Administer > Users and Permissions > Permissions (Access Control),
then clicking the Access Control link for your Content Management System, then noting down the permissions for 'CiviCRM: {financial type name} view', etc.
Then after making the change of name, reset the permissions to the way they were."), ts('Warning'), 'warning');
}
}
}

/**
Expand Down Expand Up @@ -288,7 +297,7 @@ function financialacls_civicrm_fieldOptions($entity, $field, &$options, $params)
*
* @return bool
*/
function financialacls_is_acl_limiting_enabled() {
function financialacls_is_acl_limiting_enabled(): bool {
return (bool) Civi::settings()->get('acl_financial_type');
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Civi\Financialacls;

use Civi;
use CRM_Core_Session;

// I fought the Autoloader and the autoloader won.
require_once 'BaseTestClass.php';

/**
* @group headless
*/
class FinancialTypeTest extends BaseTestClass {

/**
* Test that a message is put in session when changing the name of a
* financial type.
*
* @throws \CRM_Core_Exception
*/
public function testChangeFinancialTypeName(): void {
Civi::settings()->set('acl_financial_type', TRUE);
$type = $this->callAPISuccess('FinancialType', 'create', [
'name' => 'my test',
]);
$this->callAPISuccess('FinancialType', 'create', [
'name' => 'your test',
'id' => $type['id'],
]);
$status = CRM_Core_Session::singleton()->getStatus(TRUE);
$this->assertEquals('Changing the name', substr($status[0]['text'], 0, 17));
}

}
6 changes: 4 additions & 2 deletions tests/phpunit/api/v3/FinancialTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ class api_v3_FinancialTypeTest extends CiviUnitTestCase {

/**
* Test Create, Read, Update Financial type with custom field.
*
* @throws \CRM_Core_Exception
*/
public function testCreateUpdateFinancialTypeCustomField() {
public function testCreateUpdateFinancialTypeCustomField(): void {
$this->callAPISuccess('OptionValue', 'create', [
'label' => ts('Financial Type'),
'name' => 'civicrm_financial_type',
Expand Down Expand Up @@ -81,7 +83,7 @@ public function testCreateUpdateFinancialTypeCustomField() {

// get financial type to check custom field value
$expectedResult = array_filter(array_merge($params, $customFields), function($var) {
return (!is_null($var) && $var != '');
return (!is_null($var) && $var !== '');
});
$this->callAPISuccessGetSingle('FinancialType', [
'id' => $financialType['id'],
Expand Down

0 comments on commit 5e58aa7

Please sign in to comment.