thephpleague / omnipay-authorizenet

Authorize.Net driver for the Omnipay payment processing library
MIT License
57 stars 92 forks source link

Invalid paths for AIM query messages #125

Closed judgej closed 5 years ago

judgej commented 5 years ago

From @dmgctrlr on issue #79


Hi @eileenmcnaughton I've been trying to use your $gateway->query(array('startTimestamp' => strtotime('yesterday'), 'endTimeStamp' => 'strtotime('now'))); example. I'm receiving a InvalidRequestException. I believe this is due to the AIMAbstractRequest class adding a transaction type. Do we perhaps need to add a method override for addTransactionType in the QueryRequest class?

In addition, it appears that in the AIMGateway class, calls to queryBatch, queryBatchDetail and queryDetail are not pointing to the correct files.

I may be misusing this, so please forgive my provisional understanding!

Sorry if this is not the correct place to post this, feel free to move it if required.

judgej commented 5 years ago

Path fixes in branch issue125. I've not looked at the first bit of the query yet. If you have any code examples to replicate this, that would be useful.

dmgctrlr commented 5 years ago

Hi @judgej thanks for your input. To replicate the error I am simply doing the following:

` $gateway = Omnipay::create('AuthorizeNet_AIM'); $gateway->initialize(array_merge(['ApiLoginId' => '', 'TransactionKey' => ''], ['developerMode' => true, 'testMode' => true]));

$request = $gateway->query([
    'startTimestamp' => strtotime('yesterday'), 'endTimeStamp' => strtotime('now')
]);

$request->send();`

Which will trigger the InvalidRequestException, apparently here: Omnipay\\AuthorizeNet\\Message\\AIMAbstractRequest->addTransactionType(Object(SimpleXMLElement))

I noticed in the AIMPaymentPlansQueryRequest there is an empty addTransactionType.

judgej commented 5 years ago

I've disabled the validation check by overriding the transaction type method in the "query" functions, if you would like to give it a try. I have not read up on the query functionality, so no no idea if it is actually working, i.e. doing what it was designed to do.

dmgctrlr commented 5 years ago

@judgej Yes, thats working well now! Thanks! When do you think that could be merged?

eileenmcnaughton commented 5 years ago

I think this makes sense - I haven't been in the code for a while but the logic above makes sense

judgej commented 5 years ago

I'll merge it into the master branch now, and look at a release perhaps tomorrow.

judgej commented 5 years ago

In master now. If you could try it and let me know if it's working, I'll tag it for release. It'll probably be a patch version, since it fixes functionality that didn't work and nobody could possibly have been using as it stood.

dmgctrlr commented 5 years ago

@judgej Sorry for the delay, yes, it appears to be working as expected in master.

thanks again!

judgej commented 5 years ago

In master now. If you could try it and let me know if it's working, I'll tag it for release. It'll probably be a patch version, since it fixes functionality that didn't work and nobody could possibly have been using as it stood.

judgej commented 5 years ago

Release 3.2.0

Given these particular bugs, I can see others will be found in the query/query batch functions, as they are explored. Please raise new issues for these.

Thank you for your help :-)