artfulrobot / uk.artfulrobot.civicrm.gocardless

A CiviCRM extension providing GoCardless integration to handle UK Direct Debits.
GNU Affero General Public License v3.0
5 stars 18 forks source link

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

Closed mattwire closed 4 years ago

mattwire commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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