From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 29 Dec 2020 02:10:48 +0000 (+1300)
Subject: Move financial acl warning from FinancialType BAO to extension.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=d646594f36c18bcd0cf465f6ecab947f94c47699;p=civicrm-core.git

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 841952fba7..427d8bad90 100644
--- 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 4bcb999030..93a7755a56 100644
--- 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
index 0000000000..08fabb1408
--- /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 402dec1d38..0a96e4247a 100644
--- 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'],