vcatalano / py-authorize

A full-featured Python API for the Authorize.net payment gateway.
MIT License
42 stars 35 forks source link

List of customer payment profiles #15

Closed clearcode closed 10 years ago

clearcode commented 10 years ago

Hi, we wanted to draw attention on this issue:

When sending request "Customer.details", eg.

authorize.Customer.details('19086352')

in response parse by py-authorize we always get only last added payment_profile, so if customer has eg. two cards (two payment_id), we get only one. Authorize.net API sent all payment profiles in response, but function parse_response from py-authorize always create dict, not list from these responses.

tylerhjones commented 10 years ago

I see the same problem with adding a bank account as well as a credit card. The details request only shows the last created payment method.

The results are the same if I create a customer and list the payment methods in the params or if I use each CreditCard.create and BankAccount.create seperately.

vcatalano commented 10 years ago

I believe this problem is caused by how I'm parsing the response for the "getCustomerProfileResponse". It appears that I am not treating the "paymentProfiles" element as a list of payment profiles. I will look into pushing a fix to this within the next day or two.

tylerhjones commented 10 years ago

Yes it is. I printed out the response from authorize in the make_call function and it has all the information in it. So somehow the parsing is overwriting.

tylerhjones commented 10 years ago

so it looks like when you ref a child element in your parse response function it looks like it uses the child name as the dictionary reference.

So when you add to the dictionary multiple children with the same tag name,

for example

<paymentProfiles>
  <blah></blah>
</paymentProfiles>
<paymentProfiles>
  <blah></blah>
</paymentProfiles>

the returned dictionary only has one of the payment profiles if the dictionary key is "paymentProfiles" dict["paymentProfiles"].

Side questions why do you jump to slice 41 (line 64 in response_parser) and why all the renaming of elements?

tylerhjones commented 10 years ago

This type of conversion to json seems to handle multiple subelements with the same name as lists

import xmltodict
from lxml import etree

root = etree.fromstring(xml)
o = xmltodict.parse(etree.tostring(root))
j = json.dumps(o)

How ever it is not in the format your library uses (still has the xml schema stuff in the json, and is not renaming elements / adding underscores as you are.

vcatalano commented 10 years ago

Side questions why do you jump to slice 41 (line 64 in response_parser) and why all the renaming of elements?

This is because the ElementTree library prefixes all node tag values with the namespace. ElementTree doesn't provide a way to remove it so I'm forced to ignore the first N characters of the namespace.

There were a couple of reasons why I decided to rename certain elements. One, is that Authorize.net is very inconsistent with their nomenclature and there are instances where different XML elements that contain the same data do not have the same name. I wanted to attempt to normalize the response data as much as possible for the end users. Another reason is that Authorize.net's naming conventions are just bad. For example, I felt like it's less verbose and just as clear to use "address_ids" instead of "customerShippingAddressIdList". Looking back, I don't know if this was the best approach, but it aims to make Authorize.net's responses just a little bit better.

This type of conversion to json seems to handle multiple subelements with the same name as lists...

The reason I did not go with the xmltodict library (or any other XML to dictionary conversion libraries) was due to inherent ambiguity that exists with XML. Take the following two XML snippets:

<items>
    <item>one</item>
    <item>two</item>
    <item>trhee</item>
    <item>four</item>
</items>
<items>
    <item>one</item>
</items>

Unless you know that the items element is a list explicitly, one will be parsed as a list while the other will be parsed as a nested dictionary.

vcatalano commented 10 years ago

This should now be fixed. I rewrote the response parser to properly handle lists of response items.