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)

CRM/Financial/BAO/FinancialType.php

@@ -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
@@ -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);
@@ -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'])) {

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');
 }
 

ext/financialacls/tests/phpunit/Civi/Financialacls/FinancialTypeTest.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Civi\Financialacls;
+
+use Civi\Api4\MembershipType;
+
+// 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));
+  }
+
+}

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 @@ 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'],