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

Update civicrm versions and copy over Transaction API into webform civicrm code base #434

Merged
merged 12 commits into from
Jan 17, 2021

Conversation

KarinG
Copy link
Collaborator

@KarinG KarinG commented Jan 13, 2021

Overview

Bumping CiviCRM versions for testing -> CiviCRM Transaction API resurfaced -> handled by copying the function into the webform civicrm module itself

Before

Testing CiviCRM 5.32 and 5.33-rc
No deprecation notice for CiviCRM Transact API

After

Testing CiviCRM 5.33 and 5.34-rc
Loud deprecation notice for CiviCRM Transaction API resurfaced b/c of a change in 5.34-rc.
Solved by copying over the function into webform civicrm module itself.

Technical Details

Just copied over the relevant function into the webform civicrm code base.

Comments

Also added a first live payment processor test! 🎉

@KarinG KarinG changed the title Update civicrm versions Update civicrm versions and copy over Transaction API into webform civicrm code base Jan 13, 2021
@mattwire
Copy link
Collaborator

I think you'd be better copying over the "guts" of this function: https://lab.civicrm.org/extensions/contributiontransactlegacy/-/blob/master/api/v3/Contribution/Transact.php - the core one does not interact properly with Civi and if you copy it over the above extension will no longer work because it can't override. As a minimum that means Stripe stops working.

@KarinG
Copy link
Collaborator Author

KarinG commented Jan 13, 2021

We don't want that!

a) what specifically would you need copied over from your Extension to ensure Stripe can process transactions with webform civicrm? And how specific are your edits to Stripe?
b) I'm working on getting live payment processor testing as part of our automated testing - once I have that going for iATS Payments would you be able to add one for Stripe?

I'm thinking when we have replaced Transaction API and have confirmation the replacement works in our automated tests for both iATS and Stripe -> then we should merge this.

Changes in 5.34 have made this Transaction API notice louder... so I can no longer test for notices/errors on webform submission without the test balking at the Transaction API one.

@jackrabbithanna
Copy link

This stripe payment processor sure has caused a lot of problems and necessary core changes, changes to webform civicrm, changes to civicrm entity. And at the end of the day, it still doesn't work worth a darn after all these years. We tried it again last year and ended up abandoning it for something that actually works. Maybe stripe should be sacrificed for the greater good and not the other way around. I'm sorry this comment doesn't help but pox on stripe

@KarinG
Copy link
Collaborator Author

KarinG commented Jan 14, 2021

Two ways to enable CiviCRM extensions:

Through the UI
https://{{ domain }}/civicrm/admin/extensions?action=enable&id=com.iatspayments.civicrm&key=com.iatspayments.civicrm
and press the Enable button

Or via cv (CiviCRM CLI tools)
cv ext:enable com.iatspayments.civicrm

@KarinG
Copy link
Collaborator Author

KarinG commented Jan 14, 2021

Doh - of course that dir does not exist yet until CiviCRM is actually installed!

Ok trying to download and install an extension via the API - that could/should work!

$result = civicrm_api3('Extension', 'download', [
    'key' => "com.iatspayments.civicrm",
]);

Yes! Seeing it now!

    18 => 
    array (
      'key' => 'com.iatspayments.civicrm',
      'type' => 'module',
      'name' => 'iATS Payments',
      'label' => 'iATS Payments',
      'file' => 'iats',

@KarinG
Copy link
Collaborator Author

KarinG commented Jan 14, 2021

Happy dance 🎉 ✅

This is the first live transaction with a real payment processor. I had to dump the $contribution array to see the trxn_id for myself:

image

@KarinG
Copy link
Collaborator Author

KarinG commented Jan 16, 2021

Matt says ok to merge - he'll PR Stripe specific bits separately!

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.

3 participants