Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions CRM/Admin/Form/Setting/BankingSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@
'positiveInteger'
);

// allow status reset on transactions
$this->add(
'checkbox',
'allow_trx_reset',
E::ts("Allow status reset on ignored transactions"),

Check failure on line 124 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer

String "Allow status reset on ignored transactions" does not require double quotes; use single quotes instead
'');

Check failure on line 125 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Ignored error pattern #^Parameter \#4 \$attributes of method CRM_Core_Form\:\:add\(\) expects array\|null, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 5 times, but occurred 6 times.

Check failure on line 125 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Ignored error pattern #^Parameter \#4 \$attributes of method CRM_Core_Form\:\:add\(\) expects array\|null, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 5 times, but occurred 6 times.

Check failure on line 125 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Ignored error pattern #^Parameter \#4 \$attributes of method CRM_Core_Form\:\:add\(\) expects array\|null, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 5 times, but occurred 6 times.

Check failure on line 125 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Ignored error pattern #^Parameter \#4 \$attributes of method CRM_Core_Form\:\:add\(\) expects array\|null, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 5 times, but occurred 6 times.

// store bank accounts
$this->add(
'checkbox',
Expand Down Expand Up @@ -150,7 +157,7 @@
'checkbox',
'lenient_dedupe',
E::ts('Lenient bank account dedupe'),
'');

Check failure on line 160 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Parameter #4 $attributes of method CRM_Core_Form::add() expects array|null, string given.

Check failure on line 160 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Parameter #4 $attributes of method CRM_Core_Form::add() expects array|null, string given.

Check failure on line 160 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Parameter #4 $attributes of method CRM_Core_Form::add() expects array|null, string given.

Check failure on line 160 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Parameter #4 $attributes of method CRM_Core_Form::add() expects array|null, string given.

// validate bank account references?
$this->addElement(
Expand Down Expand Up @@ -187,6 +194,7 @@
$defaults['banking_log_level'] = Civi::settings()->get('banking_log_level');
$defaults['banking_log_file'] = Civi::settings()->get('banking_log_file');
$defaults[CRM_Banking_Config::SETTING_MAX_CONTACTS_ON_LOOKUP] = CRM_Banking_Config::getMaxContactsOnLookup();
$defaults['allow_trx_reset'] = Civi::settings()->get('allow_trx_reset');
$defaults['reference_store_disabled'] = Civi::settings()->get('reference_store_disabled');
$defaults['reference_normalisation'] = Civi::settings()->get('reference_normalisation');
$defaults['recently_completed_cutoff'] = Civi::settings()->get('recently_completed_cutoff');
Expand Down Expand Up @@ -222,6 +230,9 @@
CRM_Core_BAO_Navigation::resetNavigation();
}

// allow trx status reset
Civi::settings()->set('allow_trx_reset', !empty($values['allow_trx_reset']));

Check failure on line 234 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Ignored error pattern #^Construct empty\(\) is not allowed\. Use more strict comparison\.$# (empty.notAllowed) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 4 times, but occurred 5 times.

Check failure on line 234 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Ignored error pattern #^Construct empty\(\) is not allowed\. Use more strict comparison\.$# (empty.notAllowed) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 4 times, but occurred 5 times.

Check failure on line 234 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Ignored error pattern #^Construct empty\(\) is not allowed\. Use more strict comparison\.$# (empty.notAllowed) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 4 times, but occurred 5 times.

Check failure on line 234 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Ignored error pattern #^Construct empty\(\) is not allowed\. Use more strict comparison\.$# (empty.notAllowed) in path /home/runner/work/org.project60.banking/org.project60.banking/CRM/Admin/Form/Setting/BankingSettings.php is expected to occur 4 times, but occurred 5 times.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of empty() you could use (bool) ($values['allow_trx_reset'] ?? FALSE).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for me

!empty($values['allow_trx_reset'])

is more readable than

(bool) ($values['allow_trx_reset'] ?? FALSE)


// process menu entry
Civi::settings()->set('json_editor_mode', $values['json_editor_mode']);

Expand All @@ -238,7 +249,7 @@
Civi::settings()->set('reference_store_disabled', !empty($values['reference_store_disabled']));
Civi::settings()->set('reference_normalisation', !empty($values['reference_normalisation']));
Civi::settings()->set('reference_validation', !empty($values['reference_validation']));
Civi::settings()->set('lenient_dedupe', !empty($values['lenient_dedupe']));

Check failure on line 252 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Construct empty() is not allowed. Use more strict comparison.

Check failure on line 252 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Construct empty() is not allowed. Use more strict comparison.

Check failure on line 252 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Construct empty() is not allowed. Use more strict comparison.

Check failure on line 252 in CRM/Admin/Form/Setting/BankingSettings.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Construct empty() is not allowed. Use more strict comparison.
Civi::settings()->set('reference_matching_probability', $values['reference_matching_probability']);
Civi::settings()->set('recently_completed_cutoff', $values['recently_completed_cutoff']);

Expand Down
3 changes: 3 additions & 0 deletions CRM/Banking/Page/Review.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@
}
}

$allow_trx_reset = CRM_Core_BAO_Setting::getItem('CiviBanking', 'allow_trx_reset');

Check failure on line 104 in CRM/Banking/Page/Review.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer

Line indented incorrectly; expected 4 spaces, found 6
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Civi::settings()->get('allow_trx_reset') would be preferable.

$this->assign('allow_trx_reset', $allow_trx_reset);

Check failure on line 105 in CRM/Banking/Page/Review.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer

Line indented incorrectly; expected 4 spaces, found 6

// parse structured data
$this->assign('btxstatus', $choices[$btx_bao->status_id]);
$this->assign('payment', $btx_bao);
Expand Down
34 changes: 34 additions & 0 deletions api/v3/BankingTransaction.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,40 @@
*
*/

/**
* Adjust Metadata for Reset action
*
* The metadata is used for setting defaults, documentation & validation
* @param array $params array or parameters determined by getfields
*/
function _civicrm_api3_banking_transaction_reset_spec(&$params) {

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Function _civicrm_api3_banking_transaction_reset_spec() has parameter $params with no value type specified in iterable type array.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Function _civicrm_api3_banking_transaction_reset_spec() has no return type specified.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Function _civicrm_api3_banking_transaction_reset_spec() has parameter $params with no value type specified in iterable type array.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Function _civicrm_api3_banking_transaction_reset_spec() has no return type specified.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Function _civicrm_api3_banking_transaction_reset_spec() has parameter $params with no value type specified in iterable type array.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Function _civicrm_api3_banking_transaction_reset_spec() has no return type specified.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Function _civicrm_api3_banking_transaction_reset_spec() has parameter $params with no value type specified in iterable type array.

Check failure on line 32 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Function _civicrm_api3_banking_transaction_reset_spec() has no return type specified.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP type hints?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure about type hints here.
Core has a lot of _civicrm_api3_XYZ_spec() function all without type hints.

$params['trx_id'] = array(

Check failure on line 33 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer

Short array syntax must be used to define arrays
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short array syntax []?

'title' => 'Transaction ID',
'description' => 'The transaction ID to perform the status reset',
'required' => TRUE,
'type' => CRM_Utils_Type::T_INT,
);
}

/**
* civicrm_api3_banking_transaction_reset
* Resets a transaction's status into the status NEW
* Will also remove any suggestions stored into this transaction
*
* @param mixed $params
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think $params should be an array, not mixed.

* @return void
*/
function civicrm_api3_banking_transaction_reset($params) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP type hints?

// update the transaction
if (is_numeric($params['trx_id'])) {

Check failure on line 51 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Cannot access offset 'trx_id' on mixed.

Check failure on line 51 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Cannot access offset 'trx_id' on mixed.

Check failure on line 51 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Cannot access offset 'trx_id' on mixed.

Check failure on line 51 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Cannot access offset 'trx_id' on mixed.
// Get the status ID for new (not-analysed transaction)
$status_id_new = (int) banking_helper_optionvalueid_by_groupname_and_name('civicrm_banking.bank_tx_status', 'new');

Check failure on line 53 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Casting to int something that's already int.

Check failure on line 53 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Casting to int something that's already int.

Check failure on line 53 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Casting to int something that's already int.

Check failure on line 53 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Casting to int something that's already int.
CRM_Core_DAO::executeQuery("UPDATE `civicrm_bank_tx` SET `status_id` = ({$status_id_new}), suggestions = NULL WHERE `id` = '{$params['trx_id']}'");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backticks should either be dropped or used consequently, i.e. also for for suggestions.

return civicrm_api3_create_success('Status reset complete');

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Ignored error pattern #^Parameter \#1 \$values of function civicrm_api3_create_success expects array\|int, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/api/v3/BankingTransaction.php is expected to occur 2 times, but occurred 3 times.

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-lowest

Function civicrm_api3_banking_transaction_reset() with return type void returns array but should not return anything.

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.1 prefer-stable

Function civicrm_api3_banking_transaction_reset() with return type void returns array but should not return anything.

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Ignored error pattern #^Parameter \#1 \$values of function civicrm_api3_create_success expects array\|int, string given\.$# (argument.type) in path /home/runner/work/org.project60.banking/org.project60.banking/api/v3/BankingTransaction.php is expected to occur 2 times, but occurred 3 times.

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-lowest

Function civicrm_api3_banking_transaction_reset() with return type void returns array but should not return anything.

Check failure on line 55 in api/v3/BankingTransaction.php

View workflow job for this annotation

GitHub Actions / PHPStan with PHP 8.4 prefer-stable

Function civicrm_api3_banking_transaction_reset() with return type void returns array but should not return anything.
}

}

/**
* Add an BankingTransaction for a contact
*
Expand Down
6 changes: 6 additions & 0 deletions templates/CRM/Admin/Form/Setting/BankingSettings.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@
<div class="clear"></div>
</div>

<div class="crm-section">
<div class="label">{$form.allow_trx_reset.label}</div>
<div class="content">{$form.allow_trx_reset.html}</div>
<div class="clear"></div>
</div>

<br/>
<h3>{ts domain='org.project60.banking'}Bank Account Settings{/ts}</h3>

Expand Down
65 changes: 65 additions & 0 deletions templates/CRM/Banking/Page/Review.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@
+--------------------------------------------------------*}
{* This page is generated by CRM/Banking/Page/Review.php *}

{assign var="resetLink" value=""}

{if $btxstatus.label eq 'Ignored'}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is $btxstatus.label an untranslatable label?

{if $allow_trx_reset}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it makes sense to combine it with the if before.

<!-- Check the form configuration button -->
{capture name="resetLink" assign="resetLink"}
<a href="#" class="button" onclick="resetPayment()"><span title="{ts domain='org.project60.banking'}Reset status{/ts}"><div class="icon next-icon ui-icon-cancel"></div>{ts domain='org.project60.banking'}Reset status{/ts}</span></a>
{/capture}
{/if}
{/if}

<div id="btx">
{foreach from=$summary_blocks item=block}
{$block}
Expand Down Expand Up @@ -51,14 +62,17 @@
{/if}
{/if}
{if $new_ui_enabled && $back_to_statement_lines}
{$resetLink}
<a href="{$url_back}" class="button" style="float:right;">
<span title="{ts escape='htmlattribute' domain='org.project60.banking'}Back{/ts}"><div class="icon back-icon ui-icon-arrowreturnthick-1-w"></div>{ts domain='org.project60.banking'}Back to statement lines{/ts}</span>
</a>
{elseif $new_ui_enabled && !$back_to_statement_lines}
{$resetLink}
<a href="{$url_back}" class="button" style="float:right;">
<span title="{ts escape='htmlattribute' domain='org.project60.banking'}Back{/ts}"><div class="icon back-icon ui-icon-arrowreturnthick-1-w"></div>{ts domain='org.project60.banking'}Back to statements{/ts}</span>
</a>
{else}
{$resetLink}
<a href="{$url_back}" class="button" style="float:right;">
<span title="{ts escape='htmlattribute' domain='org.project60.banking'}Back{/ts}"><div class="icon back-icon ui-icon-arrowreturnthick-1-w"></div>{ts domain='org.project60.banking'}Back to transaction list{/ts}</span>
</a>
Expand Down Expand Up @@ -188,6 +202,57 @@ function analysePayment() {
);
}

/**
* Resets a transaction into the status "New"
*/
function resetPayment() {
const response = confirm('{/literal}{ts domain='org.project60.banking'}The analysis of this transaction will be reset into its initial state and all saved suggestions will be reset. Are you sure you want to do that?{/ts}{literal}');
if (!response) {
return;
}

var reload_regex = new RegExp("(execute=)[0-9]+", 'ig');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var should be avoided in favor if const and let.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we should use const (I think).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

camelCase for reload_regex?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I am unsure about the proper use of camelCase in CiviCRM.

Coming from Drupal I was told "stick to Drupal's style and you're fine".

https://docs.civicrm.org/dev/en/latest/standards/#civicrm-vs-drupal says:

In general, CiviCRM follows the Drupal Coding Standards

I did not found anything CiviCRM specific about naming variables.

The linked https://www.drupal.org/docs/develop/standards is labeled "[Obsolete]" and links to https://project.pages.drupalcode.org/coding_standards/

Both of those say:

Variables should be named using lowercase, and words should be separated either with uppercase characters (example: $lowerCamelCase) or with an underscore (example: $snake_case). Be consistent; do not mix camelCase and snake_case variable naming inside a file.

Since templates/CRM/Banking/Page/Review.tpl uses snake_case for variables, so I think we should stick to it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of double and single quotes isn't consistent. (Here and in the rest of the code.)


// disable ALL buttons
cj(".button").addClass('disabled');
cj(".button").attr("onclick","");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a space after ,.


// remove old suggestions
cj(".suggestions").remove();
cj("#generating_suggestions").show();

// show busy indicator

// AJAX call the analyser
var query = {'q': 'civicrm/ajax/rest', 'sequential': 1};
// set the list or s_list parameter depending on the page mode
query['trx_id'] = "{/literal}{$payment->id}{literal}";
console.log(query);
CRM.api('BankingTransaction', 'reset', query,
{success: function(data) {
if (!data['is_error']) {
// remove 'execute' bit from URL before reload
var newURL = window.location.href.replace(reload_regex, '');
if (window.location.href == newURL) {
window.location.reload(false);
} else {
window.location = newURL;
}
} else {
cj('<div title="{/literal}{ts domain='org.project60.banking'}Error{/ts}{literal}"><span class="ui-icon ui-icon-alert" style="float:left;"></span>' + data['error_message'] + '</div>').dialog({
modal: true,
buttons: {
Ok: function() {
window.location = window.location.href.replace(reload_regex, '');
}
}
});
}
}
}
);
}

/**
* common function to bring CiviCRM 4.5+ popups into the game
*/
Expand Down
Loading