Move financial acl warning from FinancialType BAO to extension.

It turns out this is not currently triggered by the api as the api passes in the key FinancialType in
ids and it is looking for financialType. Regardless I think this makes sense in the extension
and setting in the session as currently sometimes works makes sense (at least enough to
move rather than remove)

diff --git a/CRM/Financial/BAO/FinancialType.php b/CRM/Financial/BAO/FinancialType.php
index 841952fba751fa8ce72ae95541677adf053dc195..427d8bad90de1f89f4eb1288a891a469dcf46f57 100644 (file)
--- a/CRM/Financial/BAO/FinancialType.php
+++ b/CRM/Financial/BAO/FinancialType.php
@@ -69,9 +69,30 @@ class CRM_Financial_BAO_FinancialType extends CRM_Financial_DAO_FinancialType {
     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
@@ -80,6 +101,7 @@ class CRM_Financial_BAO_FinancialType extends CRM_Financial_DAO_FinancialType {
    * @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);
@@ -89,18 +111,6 @@ class CRM_Financial_BAO_FinancialType extends CRM_Financial_DAO_FinancialType {
     // 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'])) {

diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php
index 4bcb999030f34cb435302cb998012e4d11289b86..93a7755a56c5bcafa98a06ce157c1cad26e0d4f2 100644 (file)
--- a/ext/financialacls/financialacls.php
+++ b/ext/financialacls/financialacls.php
@@ -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');
+    }
+  }
 }
 
 /**
@@ -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');
 }
 

diff --git a/ext/financialacls/tests/phpunit/Civi/Financialacls/FinancialTypeTest.php b/ext/financialacls/tests/phpunit/Civi/Financialacls/FinancialTypeTest.php
new file mode 100644 (file)
index 0000000..08fabb1
--- /dev/null
+++ b/ext/financialacls/tests/phpunit/Civi/Financialacls/FinancialTypeTest.php
@@ -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));
+  }
+
+}

diff --git a/tests/phpunit/api/v3/FinancialTypeTest.php b/tests/phpunit/api/v3/FinancialTypeTest.php
index 402dec1d3866b4091c84eadcb596b9e5a8b30faf..0a96e4247aed4de8c75da96a9e053633d3b38d80 100644 (file)
--- a/tests/phpunit/api/v3/FinancialTypeTest.php
+++ b/tests/phpunit/api/v3/FinancialTypeTest.php
@@ -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',
@@ -81,7 +83,7 @@ class api_v3_FinancialTypeTest extends CiviUnitTestCase {
 
       // 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'],