Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Switch to Payment.create to fix issues with payment_instrument_id on payments #70

Merged
merged 1 commit into from
Feb 4, 2020

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Feb 3, 2020

@artfulrobot This fixes #63. I started debugging and then thought what happens if I switch to the new preferred api Payment.create instead of completetransaction. And by magic it works perfectly.

Payment.create calls completetransaction internally if required. As this is now our preferred, tested route for completing a transaction it makes sense to switch rather than investigate a broken "legacy" code-path.

@artfulrobot
Copy link
Owner

It looks like this change ends up with a different algorithm for setting membership dates - specifically that the membership date seems to include a time element now as well as a date - and the date's slightly off, too.

Can you update the failing test to fix this?

PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

...........F...............                                       27 / 27 (100%)

Time: 20.34 seconds, Memory: 46.00MB

There was 1 failure:

1) GoCardlessTest::testWebhookPaymentConfirmationFirst
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'2020-02-01 00:00:00'
+'2020-02-03 17:59:25'

/buildkit/build/dmaster/sites/default/files/civicrm/ext/uk.artfulrobot.civicrm.gocardless/tests/phpunit/GoCardlessTest.php:785
/buildkit/extern/phpunit6/phpunit6.phar:570

FAILURES!
Tests: 27, Assertions: 120, Failures: 1.

@mattwire mattwire force-pushed the paymentinstrument_fix branch from 3b24602 to f83a6b9 Compare February 3, 2020 22:38
@mattwire
Copy link
Contributor Author

mattwire commented Feb 3, 2020

@artfulrobot This passes tests now - I've had to call contribution.create again to set some params - see comments in code

@artfulrobot artfulrobot merged commit adef75d into artfulrobot:master Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Payment Method is null for recurring GoCardless payment
2 participants