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

Discussion: Should the Existing Contribution Matcher actually overwrite the receive_date of an existing contribution? #409

Open
MarcMichalsky opened this issue Jan 14, 2024 · 6 comments · May be fixed by #413

Comments

@MarcMichalsky
Copy link
Contributor

Hi @bjendres ,

I haven't found another issue that has covered this topic before.

I wonder if it actually makes sense for the Existing Contribution Matcher to overwrite the receive_date of the existing contribution. In our case, this doesn't make much sense, because we only use the matcher to finalize contributions that were announced to us via our payment service provider and then created in CiviCRM with the status "pending". As soon as the money arrives in our account, the matcher finalizes the contribution. Overwriting the original receive_date does not make sense, as it is more relevant for us when a contact has donated and not when the payment service provider has transferred the money to us.

Is there another use case in which overwriting makes sense?
Of course, it would also be possible to make overwriting optional.

I will probably fix it in the meantime by saving the original receive_date in a separate btx field and then add this to the value_propagation in the matcher.

I look forward to your opinions!

The responsible line in the code:

$query['receive_date'] = date('YmdHis', strtotime($btx->booking_date));

@jensschuppe
Copy link
Collaborator

I'd suggest making it optionally configurable to not overwrite the date for keeping backwards compatibility. In general, the receive_date semantically represents the "physical" booking date, as the money is not in your account until the actual transfer is completed, so the date which is being overwritten in your case seems to be more of an announcement date …?

@MarcMichalsky
Copy link
Contributor Author

Well, it is the date on which the donation is made. From a fundraising perspective, this information is more valuable. The actual booking date is important for our accounting department to reconcile with the bank accounts. However, this date is already available in the transaction.

Unfortunately, it is a pity that the transactions are stored in serialized form in the database and cannot be retrieved via APIv4, as otherwise it would be easy to create SearchKits that could display the donations combined with the data of the assigned transaction(s).

@jensschuppe
Copy link
Collaborator

Serializing CiviBanking transactions was necessary because there are not many distinct fields and there might be a lot of custom data per transaction. You should not rely on bank transactions for fundraising statistics anyway.

Would you be able to give my suggestion a try implementing? I guess that's what @bjendres would have been suggested as well. Your workaround with value_propagation also sounds practicable.

@MarcMichalsky
Copy link
Contributor Author

Serializing CiviBanking transactions was necessary because there are not many distinct fields and there might be a lot of custom data per transaction.

I understand that, but perhaps APIv4 support would still be useful?

You should not rely on bank transactions for fundraising statistics anyway.

The transaction date is not of interest for our fundraising but for accounting purposes.

Would you be able to give my suggestion a try implementing?

Of course! I would suggest implementing it as a field in the configuration of the Existing Contribution Matcher. How about preserve_receive_date?

@bjendres
Copy link
Member

Sounds good, I think both interpretations make sense, depending on the setup. Therefore an additional configuration parameter for that matcher seems the right way forward. Keep in mind this matcher can be configured to be in cancellation mode as well.

However, the priority should always be not to break existing setups.

@MarcMichalsky
Copy link
Contributor Author

Keep in mind this matcher can be configured to be in cancellation mode as well.

Yes, but I think it's fine to use the booking_date as the cancel_date in cancellation mode, as it shouldn't overwrite any existing information.

MarcMichalsky added a commit to MarcMichalsky/org.project60.banking that referenced this issue Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants