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

dev/core#2231 - Fix failure to calculate next_scheduled_date #19119

Merged
merged 1 commit into from
Dec 5, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Dec 4, 2020

Overview

Fixes a bug where the completeOrder flow was not setting the next scheduled date on recurrings. This is important for processors who rely on that date for billing ie

  • IATS (confirmed affected)
  • Omnipay (unconfirmed, likely)
  • Stripe (unconfirmed, less likely)
  • Eway (unconfirmed, less likely)

This bug affects recurring contribution series initiated on civicrm 5.29 or later

Before

0 (1970-00-00) stored as the next scheduled contribution date - confirmed in back office recurring data entry + add Payment flow

After

correct date calculated.

Technical Details

Regression arose from a reviewer's commit where I tried to meld 2 prs together but git-fu-d a line in from 1 that should not have been included. It resulted in no date being calculated when receive_date is unset - this is in the completeOrder flow. I have since put a test against master that would have prevented this

Comments

@civibot
Copy link

civibot bot commented Dec 4, 2020

(Standard links)

@civibot civibot bot added the 5.33 label Dec 4, 2020
@KarinG
Copy link
Contributor

KarinG commented Dec 4, 2020

Checks haven't completed yet - but it has been tested!

Before:
image

After: this PR fixes the issue -> Next Scheduled Date is set correctly:
image

@mattwire
Copy link
Contributor

mattwire commented Dec 4, 2020

@KarinG @Eileen It's been NULL for at least 4 years... and it was only changes in 5.31 not 5.29 where the issue starts to occur - are we sure that's really the cause of the issue?

@seamuslee001 removing merge on pass pending investigation per comment above - feel free to add back if you are convinced this is the cause of the issue.

@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Dec 4, 2020

@mattwire the issue is completeOrder is not updating correctly when payment is received - this is the offending line

605665c#diff-c886eb445f1cc2f7c23d798c1d2357637a2e5349381057e6074c24a9c7dd1fbcL838

I believe it is a gitfu error from taking changes from https://github.com/civicrm/civicrm-core/pull/17028/files#diff-c886eb445f1cc2f7c23d798c1d2357637a2e5349381057e6074c24a9c7dd1fbcL830 & melding them with other changes

@mattwire
Copy link
Contributor

mattwire commented Dec 4, 2020

Error was introduced via #17744

@eileenmcnaughton
Copy link
Contributor Author

PR with test fixes #19122

@mattwire do you want to re-add MOP label?

@seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 merged commit 1f66fbf into civicrm:5.33 Dec 5, 2020
@seamuslee001 seamuslee001 deleted the 532b branch December 5, 2020 00:38
@totten totten changed the title dev/core#2231 fix failure to calculate next_scheduled_date dev/core#2231 - Fix failure to calculate next_scheduled_date Dec 5, 2020
@eileenmcnaughton
Copy link
Contributor Author

Since the fix is in 5.32 one quick thing we can do is tell people how to use search kit to bulk update them should they wish to (search kit is unhidden but not-installed-by-default in 5.32 I believe). I would note 5.33 should be a big jump on 5.32 so people should probably not play too much on the 5.32 version

Use search kit to fix the next scheduled date2020-12-05 16-06

@KarinG
Copy link
Contributor

KarinG commented Dec 5, 2020

If we get any reports of iATS clients affected by this - they could go to the iATS Payments Recurring Contributions report -> and Sort that Report by Next Scheduled Date
/civicrm/report/com.iatspayments.com/recur?reset=1
Any 1970s will float to the top / will be easily visible.

If there are just a few I would recommend people just Navigate to that Contact's Recurring Series and hit Edit on the Recurring Series to update Next Scheduled Date -> as it needs to be calculated per Series.

If there are lots I'd look writing a small drush script that will grab the data (last successful completed contribution in that series + interval information) to calculate the Next Scheduled Date and update the civicrm_contribution_recur table directly.

@eileenmcnaughton
Copy link
Contributor Author

Yeah - I suspect - but didn't check - extended reports might have something too. I thought I'd be able to share the url for the above search + criteria which would be easy - but then of course I had a wordpress url & that is not that shareable after all

@herbdool
Copy link
Contributor

herbdool commented Dec 8, 2020

I ended up just searching directly in the db. At first I threw caution to the wind and enabled SearchKit to try the above but ended up in a bit of pain. SearchKit enabled okay but the icons were all messed up. So I thought perhaps it is dependent on one of the other extensions? (I can't recall if extensions can be prevented from installing if the dependent isn't enabled first) So I enabled Afform Core and that broke the site with some Uncaught errors (couldn't find classes). Anyway, I don't want to make this a bug issue but just to flag my experience with SearchKit thus far.

@eileenmcnaughton
Copy link
Contributor Author

Hmm - it doesn't depend on afform - what theme do you have @colemanw

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

Successfully merging this pull request may close these issues.

5 participants