Custom send data parameters #162

Merged
merged 2 commits into from Nov 10, 2013

Conversation

Projects
None yet
4 participants
@amacneil
Member

amacneil commented Nov 8, 2013

This is my attempt at solving #82

Now you can adjust the data generated by any gateway before it is sent:

$data = $request->getData();
$data['customParameter'] = true;

$response = $request->sendData($data);

Of course you can still just call send if you don't want to modify the data:

$response = $request->send();

Feedback welcome.

@aderuwe

This comment has been minimized.

Show comment
Hide comment
@aderuwe

aderuwe Nov 8, 2013

Contributor

👍

Contributor

aderuwe commented Nov 8, 2013

👍

amacneil added a commit that referenced this pull request Nov 10, 2013

@amacneil amacneil merged commit f79a14f into master Nov 10, 2013

1 check passed

default The Travis CI build passed
Details

@amacneil amacneil deleted the custom-send-data branch Nov 10, 2013

@anushr

This comment has been minimized.

Show comment
Hide comment
@anushr

anushr Nov 24, 2013

@adrianmacneil This implementation may be problematic because it does not abstract the data sufficiently. Specifically, some gateways' getData() return a SimpleXMLElement and others an array. I think the onus of maintaining data in the appropriate format should rest with Omnipay and not with the application.

My suggestion would be for Omnipay to always accept data as an array and have the Omnipay drivers do the appropriate data format conversion.

I have a feeling @phpmathan had the right idea in #82 by accepting any additional custom data in the send method and merging them in before sendData().

(sorry for the late feedback -- I only recently started using Omnipay)

anushr commented Nov 24, 2013

@adrianmacneil This implementation may be problematic because it does not abstract the data sufficiently. Specifically, some gateways' getData() return a SimpleXMLElement and others an array. I think the onus of maintaining data in the appropriate format should rest with Omnipay and not with the application.

My suggestion would be for Omnipay to always accept data as an array and have the Omnipay drivers do the appropriate data format conversion.

I have a feeling @phpmathan had the right idea in #82 by accepting any additional custom data in the send method and merging them in before sendData().

(sorry for the late feedback -- I only recently started using Omnipay)

@amacneil

This comment has been minimized.

Show comment
Hide comment
@amacneil

amacneil Nov 24, 2013

Member

No problem. I don't see how merging in send() vs sendData() avoids the problem with some gateways dealing with XML and others dealing with arrays. It's also not possible to consistently represent XML as an array so it's not practical to require that all gateways return an array from their getData() method (unless you have any ideas here). The only other alternative would be to always return a string, which would make it even more difficult to modify the data before sending it.

Presumably if you are messing with the request data before it is sent, then you have read the API documentation for a specific gateway and therefore I don't think it's a big deal whether it returns a SimpleXMLElement or an array - it's not like it would make any sense to modify the raw request without knowing in advance which gateway you are dealing with.

Always happy to discuss and hear thoughts on how to improve though.

Member

amacneil commented Nov 24, 2013

No problem. I don't see how merging in send() vs sendData() avoids the problem with some gateways dealing with XML and others dealing with arrays. It's also not possible to consistently represent XML as an array so it's not practical to require that all gateways return an array from their getData() method (unless you have any ideas here). The only other alternative would be to always return a string, which would make it even more difficult to modify the data before sending it.

Presumably if you are messing with the request data before it is sent, then you have read the API documentation for a specific gateway and therefore I don't think it's a big deal whether it returns a SimpleXMLElement or an array - it's not like it would make any sense to modify the raw request without knowing in advance which gateway you are dealing with.

Always happy to discuss and hear thoughts on how to improve though.

@anushr

This comment has been minimized.

Show comment
Hide comment
@anushr

anushr Nov 24, 2013

I see your point -- if the app is doing gateway specific actions, it should know how to deal with it. Makes sense.

Then perhaps the notion of "custom" data is in itself questionable. If a certain gateway supports specific parameters, shouldn't the gateway driver implement the setters for each of those gateway specific parameters?

I guess I'm wondering what the use case is for custom data? Is it something an app can use (as a workaround) until the gateway driver implements support for that particular parameter?

anushr commented Nov 24, 2013

I see your point -- if the app is doing gateway specific actions, it should know how to deal with it. Makes sense.

Then perhaps the notion of "custom" data is in itself questionable. If a certain gateway supports specific parameters, shouldn't the gateway driver implement the setters for each of those gateway specific parameters?

I guess I'm wondering what the use case is for custom data? Is it something an app can use (as a workaround) until the gateway driver implements support for that particular parameter?

@amacneil

This comment has been minimized.

Show comment
Hide comment
@amacneil

amacneil Nov 24, 2013

Member

Basically, yes. In most cases it would be better to just add a new getter/setter to the gateway, and send a pull request so we can have official support for that parameter. However, that all takes time, and sometimes you are up against a deadline and don't want to bother forking people's libraries just to bend them to your will. So by allowing you to edit the data before sending it, we save you time not having to fork the library or subclass the requests.

You're right though, the vast majority of people using Omnipay are not expected to need this feature, it's more just a nice-to-have.

Member

amacneil commented Nov 24, 2013

Basically, yes. In most cases it would be better to just add a new getter/setter to the gateway, and send a pull request so we can have official support for that parameter. However, that all takes time, and sometimes you are up against a deadline and don't want to bother forking people's libraries just to bend them to your will. So by allowing you to edit the data before sending it, we save you time not having to fork the library or subclass the requests.

You're right though, the vast majority of people using Omnipay are not expected to need this feature, it's more just a nice-to-have.

@BeingTomGreen

This comment has been minimized.

Show comment
Hide comment
@BeingTomGreen

BeingTomGreen Dec 12, 2014

Ignore me, I found a better example here.

@adrianmacneil can you provide a little more information on how to get this working?

I've got a basic WorldPay app using $handle->purchase($data)->send(); but how would I tweak this to use extra parameters?

Ignore me, I found a better example here.

@adrianmacneil can you provide a little more information on how to get this working?

I've got a basic WorldPay app using $handle->purchase($data)->send(); but how would I tweak this to use extra parameters?

barryvdh pushed a commit that referenced this pull request Feb 13, 2016

@lukeholder lukeholder referenced this pull request in thephpleague/omnipay-stripe Jul 7, 2017

Open

Move header auth setting to sendData #87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment