thallium205 / xero

Xero Library for Private Applications in Node
48 stars 55 forks source link

Cannot post multiple items in an invoice request #15

Open chiscab opened 8 years ago

chiscab commented 8 years ago

Hi Thallium,

thank you for your library! I am having issues with posting multiple Line items in an invoice. I have the same issue posting multiple tracking categories within the line item.

Is this a known issue? Would you be able to guide me towards a solution?

Many thanks, Chiara

andrew-wharton commented 8 years ago

Hi!

Could you describe what exactly the issue is that you're having please? An example request showing what data you are sending and the response you receive would be great.

To help finding the cause of the problem you're having, it helps to keep in mind that this library is essentially a thin wrapper that takes care of the OAuth request signing and conversion of JS <-> XML and that's all. There's nothing fancier going on, so depending on what the issue is that you're experiencing, the Xero API docs might be able to provide an answer as well.

chiscab commented 8 years ago

Hi Andrew,

thank you for your reply. I have managed to post multiple line items, however for the tracking categories, I don't seem to be able to send , and as required. My code is the following (it doesn't post any categories but doesn't break it either):

request = {
Type: 'xxxxxxx',
//etc
LineItems:{
     Lineitem: {
     ItemCode: xxxxxx, 
     TrackingCategories: [
     {
          TrackingCategoryID: 'xxxxxxxxxxxxx',
          Name: 'xxxxxxx',
          Option: 'xxxxxx'
     },
    {
          TrackingCategoryID: 'xxxxxxxxxxxxx',
          Name: 'xxxxxxx',
          Option: 'xxxxxx'
     }]
     }
  }
}

Thanks, Chiara

andrew-wharton commented 8 years ago

Hi Chiara,

Thanks for this. After a bit of investigation, it looks like this is due to the way that EasyXML converts Arrays into XML nodes. There is a configurable option called "singularizeChildren" (which we don't set and defaults to true in the EasyXML library) that will take a JS Array and produce an XML node for each object in the array, using the singularised version of the Array name as the node name. Eg.

request = {
  Type: "ACCREC",
  // etc.
  LineItems: [
    {
      Description: "Consulting services as agreed",
      // etc.
      TrackingCategories: [
        {
            Name: "Activity/Workstream",
            Option: "Onsite consultancy"
        }
      ]
    }
  ]  
}

will produce XML looking like

<?xml version='1.0' encoding='utf-8'?>
<Invoice>
  <Type>ACCREC</Type>
  <LineItems>
    <LineItem>
      <Description>Consulting services as agreed</Description>
      <TrackingCategories>
        <TrackingCategory>
          <Name>Activity/Workstream</Name>
          <Option>Onsite consultancy</Option>
        </TrackingCategory>
      </TrackingCategories>
    </LineItem>
  </LineItems>
</Invoice>

See how the Arrays are singularized? Unfortunately in the Xero API, they expect the Tracking instead of TrackingCategories (http://developer.xero.com/documentation/api/invoices/), so this won't work.

With "singularizeChildren" set to true, there's no way that I'm aware of to override this behaviour.

To fix this will require an update, but it'll take a little investigation to find out how to change this behaviour without breaking functionality which relies on this behaviour. Perhaps @thallium205 or @jesucarr have some ideas? I tried setting singularizeChildren to false, but it looks like there is a problem with the EasyXML library when you have nested nodes (https://github.com/tlhunter/node-easyxml/issues/8).

andrew-wharton commented 8 years ago

Having said that, you can do:

request = {
  Type: "ACCREC",
  // etc.
  LineItems: [
    {
      Description: "Consulting services as agreed",
      // etc.
      Tracking: {
        TrackingCategory: {
            Name: "Activity/Workstream",
            Option: "Onsite consultancy"
        }
      }
    }
  ]  
}

which will give

<?xml version='1.0' encoding='utf-8'?>
<Invoice>
  <Type>ACCREC</Type>
  <LineItems>
    <LineItem>
      <Description>Consulting services as agreed</Description>
      <Tracking>
        <TrackingCategory>
          <Name>Activity/Workstream</Name>
          <Option>Onsite consultancy</Option>
        </TrackingCategory>
      </Tracking>
    </LineItem>
  </LineItems>
</Invoice>

But you can't do multiple child nodes.

jesucarr commented 8 years ago

So it looks like the singularized wrapper is not consistent in the Xero API for nested elements. I think this would need to manually check the keys and if it's TrackingCategory convert it to Tracking.

lpil commented 8 years ago

:+1:

andrew-wharton commented 8 years ago

Ideally (IMHO) the JSON <-> XML conversion wouldn't need this sort of magic, under-the-hood conversion, as I can see potential for confusion and head scratching by devs trying to figure out why this is happening. If we do add this key conversion, they need to be at least documented in the README.

Personally, I'd prefer to change the API for this library so that the nested elements are named explicitly, and probably also remove the implicit naming of the root node based on the path at the same time. This would be a fairly major change though, but I think it would save a lot of confusion and make the API both simpler and in the long run easier to use for developers (ie. less WTF moments), even if it does involve a bit more typing...

rogchap commented 8 years ago

I ended up adding a cheeky hack to get Tracking working: https://github.com/kayla-tech/xero/blob/master/index.js#L39

awb99 commented 8 years ago

Gents! This error seems to happen frequently in XML-JSON converson.. For example it also happens in LineItems.LineItem

{"Contact":{"ContactID":"9f1e449d-fa45-4b4a-b111-cf9521358925","ContactStatus":"ACTIVE","Name":"Demo Corp","FirstName":"Demania","LastName":"Demoissimo","EmailAddress":"","Addresses":{"Address":[{"AddressType":"STREET"},{"AddressType":"POBOX","Region":"Alaska","PostalCode":"10500","Country":"USA"}]},"Phones":{"Phone":[{"PhoneType":"DEFAULT"},{"PhoneType":"DDI"},{"PhoneType":"FAX"},{"PhoneType":"MOBILE"}]},"UpdatedDateUTC":"2016-08-08T08:10:04.34","IsSupplier":"false","IsCustomer":"true"},"Date":"2016-09-03T00:00:00","DueDate":"2016-10-03T00:00:00","BrandingThemeID":"eecc8221-4e02-45a6-839c-9e668b8dc3f9","Status":"AUTHORISED","LineAmountTypes":"Exclusive","LineItems":{"LineItem":[{"ItemCode":"TMM","Description":"15\" Cleaning Machine","UnitAmount":"2676.97","TaxAmount":"0.00","Quantity":"1.0000","DiscountRate":"100.00","LineItemID":"3c855eeb-ffdb-48f3-9ff2-d7a4ec944be5"},{"ItemCode":"B751","Description":"TM BRUSH ","UnitAmount":"50.79","TaxType":"OUTPUT","TaxAmount":"0.00","AccountCode":"00021","Quantity":"1.0000","LineItemID":"7e0d70ba-be0c-460f-a904-bcd090071974"}]},"SubTotal":"50.79","TotalTax":"0.00","Total":"50.79","UpdatedDateUTC":"2016-09-06T17:34:05.16","CurrencyCode":"USD","Type":"ACCREC","InvoiceID":"a1f00ea8-4ef6-4bf8-91b3-79ba8e06b34b","InvoiceNumber":"INV-ZZZZ","Reference":"#w test","AmountDue":"50.79","AmountPaid":"0.00","SentToContact":"false","CurrencyRate":"1.000000","HasAttachments":"false"}

awb99 commented 8 years ago

@andrew-wharton I was thinking of using a macro to transform all JSON received to a correct format.

Example for invoice LineItems:

current behavior:

Example with multiple entries: invoice LineItems LineItem (array) LineItem (item) LineItem (item)

Example with single entry invoice LineItems LineItem (item)

Both versions should be converted to this format: Conversion with multiple entries: invoice LineItems (array) LineItem (item) LineItem (item)

conversion with single entry invoice LineItems (array) LineItem (item)

With this format, the conversion JSON -> XML works. So I think this format is the one that should be used. All GET calls shoudl have a postProcess call.

postProcess hould search for:

  1. child that has the same name of the parent, but misses the last "s".
  2. convert the parent to an array
  3. Add all child items to the new parent array.

This is all that needs to be done, and then all the requests are perfect.

A generic feature is a must. This behavior is all over: Contact - Phone numbers, ...

So when we have a child element that is either

andrew-wharton commented 8 years ago

To be honest, I'm not actually using this library in my projects anymore, as I wasn't happy with some of the 'magic' which was going on. Too many WTF situations and time wasted debugging.

Instead, I made myself a lightweight wrapper around the Xero API which handles the OAuth signing, and that's about all. I basically ripped the OAuth code out of this library and got rid everything else.

I then use jstoxml (https://github.com/davidcalhoun/jstoxml) to handle the, well, js to xml conversion for requests and set the Accept header to "application/json" to get JSON back in the response from Xero.

Been working well for me, basically taking ownership of all the conversions in my own code base.

I'm happy to share this if anyone is interested, just let me know.

lpil commented 8 years ago

I did the same after having encountering man baffling bugs with this library.

Amusingly I discovered that the OAuth code was a copy/paste from another xero library, which was a copy/paste from a twitter client library.