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.

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

fix #409: add option to preserve original receive_date #413

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarcMichalsky
Copy link
Contributor

@MarcMichalsky MarcMichalsky commented Jan 29, 2024

I have added a configuration option called preserve_receive_date which when set to true prevents the original receive_date of the contribution from being overwritten.

The configuration can be set in the Default Options Matcher and in the Contribution Matcher.

Fix #409

@MarcMichalsky
Copy link
Contributor Author

This PR is still waiting to be merged.

Copy link
Collaborator

@jensschuppe jensschuppe left a comment

Choose a reason for hiding this comment

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

@bjendres anything to add? Haven't tested though.

@@ -44,6 +44,7 @@ function __construct($config_name) {
if (!isset($config->default_financial_type_id)) $config->default_financial_type_id = 1;
if (!isset($config->createnew_value_propagation)) $config->createnew_value_propagation = array();
if (!isset($config->manual_default_financial_type_id)) $config->manual_default_financial_type_id = NULL;
if (!isset($config->preserve_receive_date)) $config->preserve_receive_date = 0; // Set to 1 to preserve the receive_date (instead of overwriting it with the booking_date). See issue #409.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this follows existing formatting, but conditions (if) without brackets ({…}) should really be avoided. Also, the comment should be put above the condition.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!isset($config->preserve_receive_date)) $config->preserve_receive_date = 0; // Set to 1 to preserve the receive_date (instead of overwriting it with the booking_date). See issue #409.
// Set to 1/TRUE to preserve the receive_date (instead of overwriting it with the booking_date). See issue #409.
$config->preserve_receive_date = (bool) ($config->preserve_receive_date ?? FALSE);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I agree with the changes, even though I think it doesn't necessarily contribute to the readability of the code to suddenly deviate from the scheme in the nth repeated line.

@@ -53,6 +53,7 @@ function __construct($config_name) {
if (!isset($config->date_penalty)) $config->date_penalty = 1.0;
if (!isset($config->payment_instrument_penalty)) $config->payment_instrument_penalty = 0.0;
if (!isset($config->financial_type_penalty)) $config->financial_type_penalty = 0.0;
if (!isset($config->preserve_receive_date)) $config->preserve_receive_date = 0; // Set to 1 to preserve the receive_date (instead of overwriting it with the booking_date). See issue #409.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here as above.

@jensschuppe jensschuppe requested a review from bjendres November 19, 2024 13:00
@jensschuppe jensschuppe added enhancement status:needs review Code needs review and testing labels Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement status:needs review Code needs review and testing
Projects
None yet
2 participants