XeroAPI / xero-php-oauth2

Xero PHP SDK for oAuth 2 generated from Xero API OpenAPI Spec 3.0
MIT License
91 stars 65 forks source link

Xero Account codes can be longer than 10 #159

Closed fooman closed 4 years ago

fooman commented 4 years ago

SDK you're using (please complete the following information):

Describe the bug The SDK validates received data from Xero. It looks like the rules are different for accounts created within Xero as the result included an account with code length > 10 causing the SDK to bail out.

To Reproduce Steps to reproduce the behavior:

  1. Retrieve list of accounts from Xero
  2. See error 'invalid length for $code when calling Account., must be smaller than or equal to 10.' https://github.com/XeroAPI/xero-php-oauth2/blob/c07ed298d2ec11dae02fd9d5954214b436ded060/lib/Models/Accounting/Account.php#L493

Expected behavior All accounts are listed.

Additional context I believe the offending account may have been a bank account (American Express FX Bank was the code).

SidneyAllen commented 4 years ago

@fooman - I'm sorry but I can't confirm the "code" property on account can be greater than 10 characters.

I've looked at Xero's UI and tried to POST and create an account with a code greater than 10 characters - both are not allowed.

Screen Shot 2020-09-10 at 9 37 41 AM

{ "ErrorNumber": 10, "Type": "ValidationException", "Message": "A validation exception occurred", "Elements": [ { "AccountID": "00000000-0000-0000-0000-000000000000", "Code": "12345ABCDE6789FGHIJ", "Name": "Bye82452", "Type": "EXPENSE", "Description": "Foo boo", "HasAttachments": false, "ValidationErrors": [ { "Message": "Account Code is too long. Maximum of 10 characters allowed" } ] } ] }

fooman commented 4 years ago

Thanks @SidneyAllen for investigating. The accounts are pre existing so don't anticipate them having been created via the api. One further thing which may be related there were multiple fake bank accounts, ie with all zeroes. Wondering if maybe some fallback happens in that case to use the name as code.

I'll try if I can get access to the client's account again to get full debug output.

SidneyAllen commented 4 years ago

@fooman - can you share the raw JSON payload? Is this the Demo company or a real customer's data?

fooman commented 4 years ago

@SidneyAllen this is a real account. Here is the relevant excerpt coming from the Xero response

    {
      "AccountID": "d8549e19-8d78-4909-8df5-8f07a267a3bd",
      "Code": "American Express FX Bank",
      "Name": "American Express FX Bank",
      "Status": "ACTIVE",
      "Type": "BANK",
      "TaxType": "NONE",
      "Description": "",
      "Class": "ASSET",
      "EnablePaymentsToAccount": false,
      "ShowInExpenseClaims": false,
      "BankAccountNumber": "0000000000000000",
      "BankAccountType": "BANK",
      "CurrencyCode": "GBP",
      "ReportingCode": "ASS",
      "ReportingCodeName": "Assets",
      "HasAttachments": false,
      "UpdatedDateUTC": "\/Date(1550486930180+0000)\/",
      "AddToWatchlist": false
    },

taken from the $content variable in AccountApi.php after

            $responseBody = $response->getBody();
            switch($statusCode) {
                case 200:
                    if ('\XeroAPI\XeroPHP\Models\Accounting\Accounts' === '\SplFileObject') {
                        $content = $responseBody; //stream goes to serializer
                    } else {
                        $content = $responseBody->getContents();
                    }
{
  "Id": "1c931b3f-3a86-47f7-a271-f552eb8e5c09",
  "Status": "OK",
  "ProviderName": "Fooman",
  "DateTimeUTC": "\/Date(1599787025974)\/",
  "Accounts": [
SidneyAllen commented 4 years ago

@fooman - Thanks for raising this issue. While the API does have business logic that limits the length of the code property to 10, during a conversion where data is imported in bulk that max length is not enforced. During serialization and deserialization models are used and validation of data is checked. Therefore, an error was resulting from the returned data.

For this reason, I've removed the maxlength from the code property in our Open API spec and updated the sdk to version 2.1.3. Cheers