Add L2/L3 data for cybersource rest and worldpay #5117
Conversation
COMP-134 Adds support for L2 and L3 data to the Cybersource Rest gateway Test Summary Local: 5882 tests, 79430 assertions, 0 failures, 23 errors, 0 pendings, 0 omissions, 0 notifications 99.609% passed Unit: 36 tests, 189 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote:
COMP-134 Refactor L2/L3 data for Worldpay to be more in line with how active merchant gateways expect this data. It also lowers the burden for what merchants must provide to use L2/L3 data Test Summary Local: 5883 tests, 79441 assertions, 0 failures, 23 errors, 0 pendings, 0 omissions, 0 notifications 99.609% passed Unit: 117 tests, 668 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 103 tests, 444 assertions, 2 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 98.0583% passed
| # data[:shipping_courier][:tracking_number], | ||
| # data[:shipping_courier][:name] | ||
| # ) | ||
| # end |
There was a problem hiding this comment.
What is the reason that these are commented out?
There was a problem hiding this comment.
I was testing to see if these are actually a thing. They're not in WP's docs so I think I'm going to delete them.
There was a problem hiding this comment.
@aenand were you going to go ahead and remove these?
There was a problem hiding this comment.
Actually I'm half of the mind to leave them as-is; it wouldn't be the first time an undocumented field was functional and utilized. It's a bit of a gray area, but I'm on board with either.
There was a problem hiding this comment.
Ah I forgot to. Yeah I'm going to delete them
| elsif data.include?(:item) | ||
| add_items_into_level_three_data(xml, data[:item].symbolize_keys) | ||
| end | ||
| data[:item].each { |item| add_items_into_level_three_data(xml, item.symbolize_keys, data) } if data.include?(:item) |
There was a problem hiding this comment.
This implies that where data.item was once an optional hash OR array, it is now a required array. Is it so?
Also, does this possibly mean the key can be changed to :items? I've been this way before and have sad memories of the time I spent staring at data[:item].kind_of?(Array) like derrrr it's just item, not items
There was a problem hiding this comment.
Yeah I don't think it makes sense to treat line_items as a hash if there's only one item. I can update it to expect line_items to be more clear
| item_discount_amount: 'itemDiscountAmount', | ||
| tax_amount: 'taxAmount' | ||
| }.each do |key, tag| | ||
| next unless item.include?(key) |
There was a problem hiding this comment.
The old "each on a hardcoded hash" strategy checked to see if the param existed in item, at this next clause. That check appears to have been removed? is that cool?
Not for nothing but--while I see why you flattened the each out (there are two XML nodes that require special code) it's a little verbose. What would you think of keeping the original implementation for unitCost, itemTotalWithTax, itemDiscountAmount, and taxAmount... with less functionality, like I like how you moved to_i to the function that needs it... and then having special ones for itemTotal and unitOfMeasure
There was a problem hiding this comment.
I had to change this because Worldpay is particular about the order in which the xml attributes are added and i kept having to interrupt the hash to get the order "right"
Worldpay:
Refactor L2/L3 data for Worldpay to be more in line with how active merchant gateways expect this data. It also lowers the burden for what merchants must provide to use L2/L3 data
Test Summary
Local:
5883 tests, 79441 assertions, 0 failures, 23 errors, 0 pendings, 0 omissions, 0 notifications
99.609% passed
Unit:
117 tests, 668 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
103 tests, 444 assertions, 2 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
98.0583% passed
Cybersource Rest
Adds support for L2 and L3 data to the Cybersource Rest
Test Summary
Local:
5882 tests, 79430 assertions, 0 failures, 23 errors, 0 pendings, 0 omissions, 0 notifications
99.609% passed
Unit:
36 tests, 189 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
54 tests, 168 assertions, 7 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
87.037% passed