Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup FinancialAccount BAO to use writeRecord and deprecate add #23079

Merged
merged 2 commits into from
Apr 4, 2022
Merged

Cleanup FinancialAccount BAO to use writeRecord and deprecate add #23079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efc59a3
Cleanup FinancialAccount BAO to use `writeRecord` and deprecate `add`
colemanw Apr 1, 2022
c633638
Fix unit test - don't use empty string as name
colemanw Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CRM/Core/DAO.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3314,8 +3314,8 @@ private function makeNameFromLabel(bool $isRequired): void {
return;
}
$label = $this->label ?? $this->title ?? NULL;
if (!$label && $label !== '0' && !$isRequired) {
// No label supplied and name not required, do nothing
if (!$label && $label !== '0') {
// No label supplied, do nothing
return;
}
$maxLen = static::getSupportedFields()['name']['maxlength'] ?? 255;
Expand Down
88 changes: 33 additions & 55 deletions CRM/Financial/BAO/FinancialAccount.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,72 +51,22 @@ public static function setIsActive($id, $is_active) {
/**
* Add the financial types.
*
* @deprecated
* @param array $params
* Reference array contains the values submitted by the form.
*
* @return CRM_Financial_DAO_FinancialAccount
*/
public static function add(&$params) {
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);
$params['is_tax'] = CRM_Utils_Array::value('is_tax', $params, FALSE);
$params['is_header_account'] = CRM_Utils_Array::value('is_header_account', $params, FALSE);
$params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE);
}
if (!empty($params['id'])
&& !empty($params['financial_account_type_id'])
&& CRM_Financial_BAO_FinancialAccount::validateFinancialAccount(
$params['id'],
$params['financial_account_type_id']
)
) {
throw new CRM_Core_Exception(ts('You cannot change the account type since this financial account refers to a financial item having an account type of Revenue/Liability.'));
}
if (!empty($params['is_default'])) {
$query = 'UPDATE civicrm_financial_account SET is_default = 0 WHERE financial_account_type_id = %1';
$queryParams = [1 => [$params['financial_account_type_id'], 'Integer']];
CRM_Core_DAO::executeQuery($query, $queryParams);
}

// action is taken depending upon the mode
$financialAccount = new CRM_Financial_DAO_FinancialAccount();

// invoke pre hook
$op = 'create';
if (!empty($params['id'])) {
$op = 'edit';
}
CRM_Utils_Hook::pre($op, 'FinancialAccount', CRM_Utils_Array::value('id', $params), $params);

if (!empty($params['id'])) {
$financialAccount->id = $params['id'];
$financialAccount->find(TRUE);
}

$financialAccount->copyValues($params);
$financialAccount->save();

if (!empty($params['custom']) && is_array($params['custom'])) {
CRM_Core_BAO_CustomValueTable::store($params['custom'], static::$_tableName, $financialAccount->id, $op);
}

// invoke post hook
$op = 'create';
if (!empty($params['id'])) {
$op = 'edit';
}
CRM_Utils_Hook::post($op, 'FinancialAccount', $financialAccount->id, $financialAccount);
CRM_Core_PseudoConstant::flush();

return $financialAccount;
public static function add($params) {
return self::writeRecord($params);
}

/**
* Delete financial Types.
*
* @deprecated
* @param int $financialAccountId
*
* @return bool
*/
public static function del($financialAccountId) {
try {
Expand Down Expand Up @@ -152,6 +102,34 @@ public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) {
}
}
}
if ($event->action === 'create' || $event->action === 'edit') {
$params = $event->params;
if (!empty($params['id'])
&& !empty($params['financial_account_type_id'])
&& CRM_Financial_BAO_FinancialAccount::validateFinancialAccount(
$params['id'],
$params['financial_account_type_id']
)
) {
throw new CRM_Core_Exception(ts('You cannot change the account type since this financial account refers to a financial item having an account type of Revenue/Liability.'));
}
if (!empty($params['is_default'])) {
if (empty($params['financial_account_type_id'])) {
$params['financial_account_type_id'] = CRM_Core_DAO::getFieldValue(__CLASS__, $params['id'], 'financial_account_type_id');
}
$query = 'UPDATE civicrm_financial_account SET is_default = 0 WHERE financial_account_type_id = %1';
$queryParams = [1 => [$params['financial_account_type_id'], 'Integer']];
CRM_Core_DAO::executeQuery($query, $queryParams);
}
}
}

/**
* Callback for hook_civicrm_post().
* @param \Civi\Core\Event\PostEvent $event
*/
public static function self_hook_civicrm_post(\Civi\Core\Event\PostEvent $event) {
CRM_Core_PseudoConstant::flush();
}

/**
Expand Down
25 changes: 17 additions & 8 deletions CRM/Financial/DAO/FinancialAccount.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
* Generated from xml/schema/CRM/Financial/FinancialAccount.xml
* DO NOT EDIT. Generated by CRM_Core_CodeGen
* (GenCodeChecksum:ae32a35de5bc66a8e230b5595b77810f)
* (GenCodeChecksum:bbb54fc9abdb9f5187f84b0b4eadb53e)
*/

/**
Expand Down Expand Up @@ -106,7 +106,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this a header account which does not allow transactions to be posted against it directly, but only to its sub-accounts?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand All @@ -115,7 +115,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this account tax-deductible?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand All @@ -124,7 +124,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this account for taxes?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand All @@ -142,7 +142,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this a predefined system object?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand All @@ -151,7 +151,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this property active?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand All @@ -160,7 +160,7 @@ class CRM_Financial_DAO_FinancialAccount extends CRM_Core_DAO {
/**
* Is this account the default one (or default tax one) for its financial_account_type?
*
* @var bool|string|null
* @var bool|string
* (SQL type: tinyint)
* Note that values will be retrieved from the database as a string.
*/
Expand Down Expand Up @@ -342,6 +342,7 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Header Financial Account?'),
'description' => ts('Is this a header account which does not allow transactions to be posted against it directly, but only to its sub-accounts?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_header_account',
'default' => '0',
'table_name' => 'civicrm_financial_account',
Expand All @@ -355,8 +356,9 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Deductible Financial Account?'),
'description' => ts('Is this account tax-deductible?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_deductible',
'default' => '1',
'default' => '0',
'table_name' => 'civicrm_financial_account',
'entity' => 'FinancialAccount',
'bao' => 'CRM_Financial_BAO_FinancialAccount',
Expand All @@ -368,6 +370,7 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Tax Financial Account?'),
'description' => ts('Is this account for taxes?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_tax',
'default' => '0',
'table_name' => 'civicrm_financial_account',
Expand Down Expand Up @@ -397,7 +400,9 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Reserved Financial Account?'),
'description' => ts('Is this a predefined system object?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_reserved',
'default' => '0',
'table_name' => 'civicrm_financial_account',
'entity' => 'FinancialAccount',
'bao' => 'CRM_Financial_BAO_FinancialAccount',
Expand All @@ -409,7 +414,9 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Financial Account is Active'),
'description' => ts('Is this property active?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_active',
'default' => '1',
'table_name' => 'civicrm_financial_account',
'entity' => 'FinancialAccount',
'bao' => 'CRM_Financial_BAO_FinancialAccount',
Expand All @@ -421,7 +428,9 @@ public static function &fields() {
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Default Financial Account'),
'description' => ts('Is this account the default one (or default tax one) for its financial_account_type?'),
'required' => TRUE,
'where' => 'civicrm_financial_account.is_default',
'default' => '0',
'table_name' => 'civicrm_financial_account',
'entity' => 'FinancialAccount',
'bao' => 'CRM_Financial_BAO_FinancialAccount',
Expand Down
8 changes: 8 additions & 0 deletions CRM/Upgrade/Incremental/php/FiveFortyNine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ class CRM_Upgrade_Incremental_php_FiveFortyNine extends CRM_Upgrade_Incremental_
'is_pay_later' => "DEFAULT 0",
'is_template' => "DEFAULT 0 COMMENT 'Shows this is a template for recurring contributions.'",
],
'civicrm_financial_account' => [
'is_header_account' => "DEFAULT 0 COMMENT 'Is this a header account which does not allow transactions to be posted against it directly, but only to its sub-accounts?'",
'is_deductible' => "DEFAULT 0 COMMENT 'Is this account tax-deductible?'",
'is_tax' => "DEFAULT 0 COMMENT 'Is this account for taxes?'",
'is_reserved' => "DEFAULT 0 COMMENT 'Is this a predefined system object?'",
'is_active' => "DEFAULT 1 COMMENT 'Is this property active?'",
'is_default' => "DEFAULT 0 COMMENT 'Is this account the default one (or default tax one) for its financial_account_type?'",
],
];

/**
Expand Down
11 changes: 10 additions & 1 deletion xml/schema/Financial/FinancialAccount.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,16 @@
<type>boolean</type>
<title>Header Financial Account?</title>
<default>0</default>
<required>true</required>
<comment>Is this a header account which does not allow transactions to be posted against it directly, but only to its sub-accounts?</comment>
<add>4.3</add>
</field>
<field>
<name>is_deductible</name>
<type>boolean</type>
<title>Deductible Financial Account?</title>
<default>1</default>
<default>0</default>
<required>true</required>
<comment>Is this account tax-deductible?</comment>
<add>4.3</add>
</field>
Expand All @@ -127,6 +129,7 @@
<type>boolean</type>
<title>Tax Financial Account?</title>
<default>0</default>
<required>true</required>
<comment>Is this account for taxes?</comment>
<add>4.3</add>
</field>
Expand All @@ -143,20 +146,26 @@
<type>boolean</type>
<title>Reserved Financial Account?</title>
<comment>Is this a predefined system object?</comment>
<default>0</default>
<required>true</required>
<add>4.3</add>
</field>
<field>
<name>is_active</name>
<type>boolean</type>
<title>Financial Account is Active</title>
<comment>Is this property active?</comment>
<default>1</default>
<required>true</required>
<add>4.3</add>
</field>
<field>
<name>is_default</name>
<type>boolean</type>
<title>Default Financial Account</title>
<comment>Is this account the default one (or default tax one) for its financial_account_type?</comment>
<default>0</default>
<required>true</required>
<add>4.3</add>
</field>
<index>
Expand Down