Added ability to change endpoint urls #16
Conversation
|
Hi @3dgroup - from a quick skim through this change looks like it could potentially break existing installations. Is the default Realex endpoint provided somewhere? |
|
No your right, let me change it. |
|
This should have resolved the issue you mentioned by adding the default payandshop urls into the default config array. |
src/Omnipay/Realex/RemoteGateway.php
Outdated
| '3dSecure' => 0, | ||
| 'authEndpoint'=>'https://epage.payandshop.com/epage-remote.cgi', | ||
| 'secureDataVaultEndpoint'=>'https://epage.payandshop.com/epage-remote-plugins.cgi', | ||
| '3DSecureEndpoint'=>'https://epage.payandshop.com/epage-3dsecure.cgi' |
There was a problem hiding this comment.
@3dgroup - just being picky, but can you match the case of this parameter to the existing 3dSecure one - ie. lowercase "d". It's just easier for other devs to remember then if it's consistent.
|
Hey, I just made a pull request, one part of which allows you to change one of the endpoints, is there any issues with this request as it stands? If not I will gladly take that section out of my pull request. |
| } | ||
| public function setAuthEndpoint($value) | ||
| { | ||
| return $this->setParameter('SecureDataVaultEndpoint', $value); |
There was a problem hiding this comment.
Should getters/setters not be using lower-camel-case parameter names to match the defaults defined in RemoteGateway.php? I'm not sure if the parameter bag is case-insenstive, but it would seem to make sense to be consistent with the other getters/setters...
|
@James-Byrne - no, I think your implementation actually looks cleaner, so I'll probably merge yours instead. I'm guessing that your endpoint changes are to allow you to use it with white-labelled versions of Realex? |
|
@coatesap To me there is more than one end point URL and @James-Byrne commit only seems to cover the Auth URL not the data vault end point url. |
|
@3dgroup - yes, good point. @James-Byrne - have you any thoughts how this could be addressed? |
|
I think the pr by @3dgroup is far more comprehensive than the one I have made and covers my current use case as well as use cases I may have in the future. If you can resolve the outstanding issues you have yourself (styling, backwards compatibility etc) then I think this is the better option. |
|
Thanks @James-Byrne, now that I've had chance to look properly at the commits, I'd agree. On that basis - @3dgroup - are you able to address the two comments I've added to the PR, and sort the indentation (tab is 4 spaces in Omnipay land, I believe), and I'll get this merged. |
|
I think the tab issue is from my college istvan when he resolved an XML issue regarding special characters. He will be updating this today. |
…s wrong, move to convertation to node content
|
What's the status of this PR? My team is working on Realex Hosted Payment Page / Redirect support as well, and it looks like some of the functionality will overlap what's being added here. |
|
My Fork is working fine, https://github.com/3dgroup/omnipay-realex |
nbproject/project.properties
Outdated
| @@ -0,0 +1,7 @@ | |||
| include.path=${php.global.include.path} | |||
There was a problem hiding this comment.
Could you remove any of these IDE-specific files from the PR (I imagine they probably need to be gitignored...)
| @@ -120,8 +130,22 @@ protected function createResponse($data) | |||
| return $response; | |||
| } | |||
|
|
|||
| // public function getEndpoint() | |||
There was a problem hiding this comment.
Commented-out code needs removing or a comment explaining why this is a temporary action adding.
# Conflicts: # src/Omnipay/Realex/Message/RemoteAbstractRequest.php
# Conflicts: # src/Omnipay/Realex/Message/EnrolmentRequest.php # src/Omnipay/Realex/Message/RemoteAbstractRequest.php # src/Omnipay/Realex/RemoteGateway.php
Elavon servers went down causing paying scripts to hang. Adding this frees the resources.
No description provided.