Migrate Eway(Single Currency) Payment Processor Type out into its own… #18349
Conversation
|
(Standard links)
|
seamuslee001
force-pushed
the
eway_extension
branch
from
4f4b162 to
591a8a9
Compare
|
It might be easier to get it under testing first & then do this as there are a few steps in that and then we have to figure those out in an extension |
|
I would prefer to get the move done then figure out testing tbqh |
| @@ -6,7 +6,7 @@ | |||
| * | |||
| * Generated from xml/schema/CRM/Event/Event.xml | |||
| * DO NOT EDIT. Generated by CRM_Core_CodeGen | |||
| * (GenCodeChecksum:164ee1a507b9244536114d1569ccff1e) | |||
| * (GenCodeChecksum:82ba48cbb804cf6f4b26fa50f07d44db) | |||
There was a problem hiding this comment.
@seamuslee001 Why is this in here?
There was a problem hiding this comment.
Because it is there I'll spin that off separately
ext/ewaysingle/README.md
Outdated
| (*FIXME: In one or two paragraphs, describe what the extension does and why one would download it. *) | ||
|
|
||
| The extension is licensed under [AGPL-3.0](LICENSE.txt). | ||
|
|
||
| ## Requirements | ||
|
|
||
| * PHP v7.0+ | ||
| * CiviCRM (*FIXME: Version number*) |
There was a problem hiding this comment.
Can we fix the FIXMEs here :-)
ext/ewaysingle/info.xml
Outdated
| <url desc="Main Extension Page">http://FIXME</url> | ||
| <url desc="Documentation">http://FIXME</url> | ||
| <url desc="Support">http://FIXME</url> |
There was a problem hiding this comment.
@seamuslee001 Please update these lines
ext/ewaysingle/info.xml
Outdated
| <version>1.0</version> | ||
| <develStage>alpha</develStage> | ||
| <compatibility> | ||
| <ver>5.0</ver> |
ext/ewaysingle/info.xml
Outdated
| </urls> | ||
| <releaseDate>2020-09-04</releaseDate> | ||
| <version>1.0</version> | ||
| <develStage>alpha</develStage> |
ext/ewaysingle/info.xml
Outdated
| <compatibility> | ||
| <ver>5.0</ver> | ||
| </compatibility> | ||
| <comments>This is a new, undeveloped module</comments> |
|
@seamuslee001 This looks good. I've made some comments about fixing up the xml/readme - I hate seeing FIXME on production systems! 996 style warnings... |
seamuslee001
force-pushed
the
eway_extension
branch
2 times, most recently
from
d70c553 to
63bb954
Compare
|
@eileenmcnaughton @mattwire is there anything blocking this from proceeding now? |
|
@seamuslee001 given you are the only one with the credentials to test this I'm happy to proceed if you promise to do one more test after it's merged |
|
Sure |
….xml and add in test of eWAY single Currency payment processor Update README
seamuslee001
force-pushed
the
eway_extension
branch
from
63bb954 to
68f481e
Compare
|
@seamuslee001 @eileenmcnaughton is there a reason for keeping this EWAY extension at all? Why not just replace with EWAY Recurring? https://lab.civicrm.org/extensions/ewayrecurring This one uses the old EWAY APIs compared to EWAY Recurring which uses the new EWAY Rapid API and is maintained. Why have two, especially now since this one is moving to a Core extension and EWAY Recurring is approved for bundling. |
|
Some sites may still use the original processor, there is a desire to modulise the code more which is what this is doing. The idea is to not break sites at all that require the original processor but also allow for sites to get rid of it if they care. |
|
So let's merge the functionality with EWAY Recurring and just have the one processor. I don't see the sense in maintaining two. And if I find it confusing, what do you think a new Civi implementor or developer will think? They'd be confused too. It's like having a Stripe APIv3 extension and also have a Stripe APIv4 extension. Pick which one too use. |
|
Or conversely, take EWAY Recurring and merge it back into EWAY Single. I don't mind either way! |
|
@agileware-justin yep - the goal is to eventually move this out of core & stop maintaining it - ditto the other core processors over time where a better alternative exists or they are just too niche |
|
As I see it these are the options, maybe there's others: Option 1. Merge EWAY Recurring into EWAY Single to have one EWAY extension. |
|
We have 3 not 2 :-). 2 extensions and the existing core code. In this PR the existing core code is being moved from the CRM directory to a hidden extension (@seamuslee001 I'm not sure you made it hidden yet). So this change doesn't alter the fact that there are 3 alternatives - it just moves the code internally to separate it from the CRM dir - so it's part of option 4 - continuing the process of deprecating this code fully out of core. |
|
PR to remove from packages - civicrm/civicrm-packages#306 |
|
@eileenmcnaughton I'll see your 3 and raise you 4. Is it worth raising a Gitlab to discuss merging the sprawling EWAY extensions into a single source and deprecating the others? Or am I just whistling in the wind? |
|
@agileware-justin you can do - but the focus here is just part of the larger 'move core processors into extensions' plan - not any particular eway focus. It's the second off the block (the first was payment express but that turned out to be long time broken) |
… extension
Overview
This shifts the eWAY (Single Currency) Payment Processor Type out from being within core into a core extension. Also brings across the eway libraries from Packages to be in with the eway code by its self.
Before
Eway is part of Core Payment Processors
After
Eway Single Currency isn't in the core payment processors but can be added as necessary
Technical Details
This auto inserts the managed entity row if necessary if eway is already present in the payment processor type table on upgrade so there is no breakage
ping @eileenmcnaughton