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 test / possible live error on submitting credit card renewals #14316

Merged
merged 1 commit into from
May 25, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 24, 2019

Overview

Fix intermittent error on credit card renewal by credit card

Before

Intermittent ? fail

After

No fail

Technical Details

In the course of fixing #14315 I found that I was getting test failures due to the contribution having no receive_date - this is carried over as trxn_date to the financialTrxn date and fatals.

I was able to replicate in the UI renewing with a dummy processor - but I suspect there must be some factor that makes this intermittent despite it seeming reliable for me. I have a feeling I've seen it as an intermittent error on jenkins

We 'know' that the receive_date is 'now' if we are paying by card so this should be safe

Comments

@civibot
Copy link

civibot bot commented May 24, 2019

(Standard links)

@civibot civibot bot added the master label May 24, 2019
@eileenmcnaughton
Copy link
Contributor Author

test this please

@mattwire
Copy link
Contributor

This doesn't really seem like something we should be passing in on each form and instead should be getting a default somewhere. However, the MembershipRenewal form is pretty toxic so as this helps consistency I'll merge.

@mattwire mattwire merged commit 1af6842 into civicrm:master May 25, 2019
@seamuslee001 seamuslee001 deleted the membershipRenewal branch May 25, 2019 21:04
@eileenmcnaughton
Copy link
Contributor Author

@mattwire are you meaning the BAO_Contribution create method should default to 'now' if no receive_date is set? That makes sense I think

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.

2 participants