invoiceninja / sdk-php

PHP wrapper for Invoice Ninja's REST API
https://www.invoiceninja.com
83 stars 41 forks source link

is it possible to use this to build custom query, example get tasks by user_id ? #29

Closed vesper8 closed 4 years ago

vesper8 commented 6 years ago

I guess this is possible since I just tried it, but I had to make this custom function which is just a replicate of the findByClientId

    /**
    * @return \InvoiceNinja\Models\AbstractModel
    */
    public static function findByUserId($id)
    {
        $url = sprintf('%s?user_id=%s', static::getRoute(), $id);
        $data = static::sendRequest($url);

        $result = [];
        foreach ($data as $item) {
            $result[] = static::hydrate($item);
        }

        return $result;
    }    

But instead of having to fork this sdk and start creating other functions.. shouldn't it be possible to use a flexible method to achieve this? Similar to a where() method

I would love to be able to use something like eloquent

$tasks = Tasks::where('user_id', 1)->skip(0)->take(10)->get();

This would become very useful once the number of tasks becomes too big.

In any case the skip/take limit would be a bonus, but at least being able to query by user_id without requiring a function dedicated to this would be a good improvement

hillelcoren commented 6 years ago

Are you sure this works? I didn't think the API supports filtering by user.

Agreed, the SDK could definitely be improved. Only so many hours in the day...

vesper8 commented 6 years ago

actually.. you're right.. upon trying to make this function myself.. which would look like this:

    /**
    * @return \InvoiceNinja\Models\AbstractModel
    */
    public static function findBy($where)
    {
        $url = static::getRoute();

        $columns = array_keys($where);
        $values = array_values($where);

        for ($i = 0; $i < count($where) ; $i++) {
            $url = sprintf("%s?{$columns[$i]}=%s", $url, $values[$i]);
        }

        $data = static::sendRequest($url);

        $result = [];
        foreach ($data as $item) {
            $result[] = static::hydrate($item);
        }

        return $result;
    }

        // call like this
        $tasks = Task::findBy([
            'user_id' => 2,
        ]);

I realized that actually.. my above method doesn't work...

So there's no way to filter by anything other than client_id ? Well.. that's.. pretty lame

Will have to do the filtering on my side then.. which will become a problem when I end up with too many records

vesper8 commented 6 years ago

This sounds like it would be a fairly easy thing to add.. if there's willingness.. If I understand correctly, it's not so much the SDK that lacks flexibility but the API itself ? Well it's both but.. adding my above method to the SDK would only solve half the problem

Still the other half should be pretty simple to add too?

vesper8 commented 6 years ago

Ugh.. I see I can't even filter on my side since the user_id is not returned at all...

hillelcoren commented 6 years ago

Feel free to create a PR (on the develop branch), here's the relevant code:

https://github.com/invoiceninja/invoiceninja/blob/master/app/Http/Controllers/BaseAPIController.php#L92

hillelcoren commented 6 years ago

Actually, this change is trickier than it looks (due to multi-tenancy). We probably wouldn't be able to merge your PR. You'd need to copy how the client_id is handled. You may want to create an issue/feature request.