calcinai / xero-php

A php library for the Xero API, with a cleaner OAuth interface and ORM-like abstraction.
MIT License
359 stars 262 forks source link

Unable to create new TrackingOptions #871

Open willpower232 opened 2 years ago

willpower232 commented 2 years ago

I would like to create a TrackingOption as part of preparing a Demo Company for development.

Unfortunately the following code does nothing

$option = new TrackingOption($xeroConnection);
$option->setName($email);
$trackingCategory->addOption($option);
$trackingCategory->save();

Having inspected the savePropertiesDirectly method, I extracted the following XML body

<s><s><Name>email@example.com</Name></s></s>

I can see that in #801 @philipdarlo set TrackingOption::getRootNodeName to an empty string for some reason.

I can obviously PR to restore the word "Option" but was there any particular reason for blanking the return?

willpower232 commented 2 years ago

a quick solution is to use your own class when creating the option but that doesn't help you if you want to edit them

<?php

namespace App\Xero\Models;

use XeroPHP\Models\Accounting\TrackingCategory\TrackingOption as ParentTrackingOption;

class TrackingOption extends ParentTrackingOption
{
    /**
     * @inheritdoc
     */
    public static function getRootNodeName()
    {
        return 'Option';
    }
}
calcinai commented 2 years ago

@willpower232does this work? Over the years there has been inconstant behaviour handling Tracking Categories and options.

willpower232 commented 2 years ago

I can definitely create tracking options now and the XML is clearly broken without it so I'd say this is definitely needed.

Unless there was some wildly different intention from that PR, I'm not sure how they're possibly using the model although the addition of methods in getSupportedMethods implies they're doing something weird since tracking options don't get saved directly https://developer.xero.com/documentation/api/accounting/trackingcategories

calcinai commented 2 years ago

Yeah maybe something has changed, or we've finally found a solution? Let's merge this because, as you say, it's not working as it is.

philipdarlo commented 1 year ago

Apologies, for not getting a reply to the tag above on this sooner.

When I created the original MR, I was taking someones older work and just making sure it had no conflicts so could be merged.

I ended up not using this feature in my project and instead doing a manual work around.

I apologies @willpower232 as I did not test this well enough as has been clearly shown in this issue.