Mercado Pago: add idempotency key field

[yunnydang]

Adds the idempotency key to the headers of a request. Decided to use an instance variable to store the idempotency key (if provided) so that there is less wrangling of the options object and changes to the endpoint methods.

Local:
5995 tests, 80210 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit:
45 tests, 209 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
45 tests, 107 assertions, 17 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
62.2222% passed

Note: the tests failing are msotly due to payer email related issues.

[jcreiff]

Hey @yunnydang I think the instance variable is one possible clever way to solve this without having to update the params sent to commit and headers, but I also notice that we used a different convention to achieve this for the other header that gets added, by the way that we handle options[:device_id] - it gets added to the post for the purpose of getting sent to commit, but then is omitted by the logic in post_data

Do you think we should stick with that type of convention, for the sake of consistency?

[jcreiff]

Although now that I look at it again, I see that the add_additional_data method that references the device_id is only getting called for purchase, and we need to be able to send the idempotency key on other txn types as well 🤔 So maybe it's not a simpler/cleaner way to accomplish this after all. Maybe there's some kind of hybrid approach that makes sense?

[yunnydang]

Hey @yunnydang I think the instance variable is one possible clever way to solve this without having to update the params sent to commit and headers, but I also notice that we used a different convention to achieve this for the other header that gets added, by the way that we handle options[:device_id] - it gets added to the post for the purpose of getting sent to commit, but then is omitted by the logic in post_data

Do you think we should stick with that type of convention, for the sake of consistency?

Hey @jcreiff I actually tried this initially and wehen we add x_idempotency_key to the post object and send it along, the gateway returns an error that states that the field name (idempotency_key) is invalid. For some reason its accepting the device_id but it refuses the idempotency key field in the post object

[jcreiff]

@yunnydang Could you update the post_data method to also delete the idempotency key? I think that's why there's no issue with the device_id - it's getting pulled out of the final request body

[yunnydang]

Although now that I look at it again, I see that the add_additional_data method that references the device_id is only getting called for purchase, and we need to be able to send the idempotency key on other txn types as well 🤔 So maybe it's not a simpler/cleaner way to accomplish this after all. Maybe there's some kind of hybrid approach that makes sense?

Yes, in line with my other reply, this would then be needed for the other endpoints. I believe that we could try to do a hybrid approach but the issue is that I think we would still run into to the error in my other comment. Im inclined to believe that the gateway does not accept this field unless its only in the header

[yunnydang]

@yunnydang Could you update the post_data method to also delete the idempotency key? I think that's why there's no issue with the device_id - it's getting pulled out of the final request body

Hmm i think this could work! Let me see what tinkering I can do

[yunnydang]

@yunnydang Could you update the post_data method to also delete the idempotency key? I think that's why there's no issue with the device_id - it's getting pulled out of the final request body

Thanks for this suggestion! I was able to make a hybridish way for this to work and definitely prefer keeping conventions! I removed the refund remote test because i noticed it would sometimes work and sometimes not. It would fail and state that the refund was not found

[jcreiff]

Since this ultimately calls the authorize method, I don't think you actually need to touch anything within the verify block - the changes you made to purchase_request should do the trick 😁

[jcreiff]

Just making sure you saw this comment as well @yunnydang - I think we can remove these additions here - you can test it yourself if you change your unit test to be a verify (you'll have to change the # of arguments passed in) - it should still pass even without these updates within this method

[yunnydang]

ahh gotcha I was unsure if we needed to send it on the auth and verify as well since they bot go to the payments endpoint!

[yunnydang]

Even though its hitting the auth endpoint first, we dont need to send the key on the subsequent post to verify?

[jcreiff]

Looks good!