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

Cleanup date handling on Payment.create #15687

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fix a bug identified by @kcristiano whereby adding a payment with the
additional payment form changes the contribution receive_date.

Also, add a default to Payment.create of 'now' for the
trxn_date and require it to be set. Use that value later on.

Some string fixes

Before

Adding a payment on AdditionalPaymentForm changes Contribution.receive_date

After

Contribution.receive_date not altered

Technical Details

@kcristiano @magnolia61

Comments

Fix a bug identified by @kcristiano whereby adding a payment with the
additional payment form changes the contribution receive_date.

Also, add a default to Payment.create of 'now' for the
trxn_date and require it to be set. Use that value later on.

Some string fixes
@civibot
Copy link

civibot bot commented Nov 1, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

Unrelated fail

@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Nov 1, 2019

test this please

2 similar comments
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

test this please

@magnolia61
Copy link
Contributor

I tested this and can confirm it works.

@mattwire mattwire merged commit 2543ccb into civicrm:master Nov 1, 2019
@mattwire
Copy link
Contributor

mattwire commented Nov 1, 2019

Merging based on review + @magnolia61 test

@eileenmcnaughton eileenmcnaughton deleted the add_pay_date branch November 1, 2019 20:55
@kcristiano
Copy link
Member

@eileenmcnaughton I am still having an issue on Received Date in the bookkeeping report.

image

The dates on the contribution screen look correct:

image

This looks more like a reporting issue.

@seamuslee001
Copy link
Contributor

@kcristiano what is the date received on the contribution screen? like when you click the view link on the contribution its self?

@kcristiano
Copy link
Member

@seamuslee001 That is the second screen shot and those dates are correct.

@kcristiano
Copy link
Member

image

The Contrib report only picks up one date from civicrm_contribution

@seamuslee001
Copy link
Contributor

@kcristiano yeh the 2nd screenshot in your previous comment only had the payments showing

@eileenmcnaughton
Copy link
Contributor Author

@kcristiano I think that is not exactly a bug - ie. the trxn_date is shown on your report & it's correct. We should possibly not offer the receive date as a field or rename it or otherwise make it clear it's not the trxn_date

@kcristiano
Copy link
Member

The trx_date is when the transaction is created. The received date is used by many of our client using cash basis accounting.

I am going to look again and re-evaluate.

This is an improvement, but we need to not change prior behavior - and I need to test for that. Specifically a line item's receive date should not change if edited.

@eileenmcnaughton
Copy link
Contributor Author

@kcristiano yep - I'm keen to see a thorough set of testing done on 5.20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants