inakiabt / etsy-php

Etsy API wrapper for PHP
74 stars 59 forks source link

Time not accepted in params #31

Open booni3 opened 7 years ago

booni3 commented 7 years ago

Hey,

I am unable to get any time parameters to be accepted within findAllShopReceipts. I have tried a few things and in each case the time parameter is ignored and results from all time are returned. e.g.

$api->findAllShopReceipts(array(
    'params' => array(
        'shop_id' => 'shopid',
        'min_created' => strtotime("-1 days"),  //times not accepted?!
        'limit' => 25,
        'page' => 1
    )))

In the response you can see the time has not been accepted;

shop_id =>  shopid
min_created =>  null
max_created =>  null
min_last_modified   =>  null
max_last_modified   =>  null
limit   =>  25
offset  =>  0
page    =>  1
was_paid    =>  null
was_shipped =>  null

Does anyone have any ideas?

booni3 commented 7 years ago

I found a solution... not 100% if this is the correct way to do it but its working for me at least. Within EtsyAPI.php, add the required parameters for your call.

private function prepareParameters($params) {
        $query_pairs = array();
        $allowed = array(
            "limit",
            "offset",
            "page",
            "sort_on",
            "sort_order",
            "include_private",
            "language",
            "min_created",
            "max_created",
            "max_last_modified",
            "min_last_modified",
            "was_paid",
            "was_shipped"
            );
inakiabt commented 7 years ago

You are right, that wasn't considered at the time this library was built. I don't think hardcode parameters is the best approach anymore. I'll try to do the required change but have some help would be great.

I'm thinking to merge data into params (would be the same to have one, the other or both for backwards compatibility) and validate only that as I'm doing it for data. If the method requested is a GET set all params as query params if it is not, set it as the body. I guess it will require add more validations for param types like user_id_or_name or epoch (min_created has that type in findAllShopReceipts for example and that type wasn't considered either).

inakiabt commented 7 years ago

I did a quick fix in this branch to show the idea. If you can play with it would be great. Thanks!

FYI, I didn't even try it once

inakiabt commented 7 years ago

@lambooni can you try to set min_created in data and the rest in params?

booni3 commented 6 years ago

@inakiabt Sorry for such a long delay on this. I came back to using the library recently. I did try your fork but I got some errors and as I do not have much time, I am using this as a quick fix.

7355b5b93b6763413e226c687ec37efb1206558f

I realise this may not be 100% correct but its working for me at least for my current needs.

booni3 commented 6 years ago

Sorry... maybe I missed something here? In your main branch, using this also works...

$receipts = $api->findAllShopReceipts(array(
        'params' => array(
            'shop_id' => $apikeys->shop_id,
            'limit' => $limit,
            'page' => $page
        ),
        'data' => array(
            'min_last_modified' => strtotime("-1 days"), //modified in the last day
        )));

Seems like you have either fixed it, or I was not using it correctly in the first place!

inakiabt commented 6 years ago

Yes! I thought it could work. Great. I’ll update the README, but I think I’ll go also with the idea I started in the branch I created for this issue (whenever I have some time to do it).

Thanks

pavvel11 commented 3 years ago

Hi, I am trying to use findAllShopReceipts method with min_created and max_created params and I get Exception: Invalid params for method "findAllShopReceipts": Invalid data param type "min_created" (1610215615: int): required type "epoch".

Params min_last_modified and max_last_modified work fine and I guess this is because in methods.json file those params are defined as int not epoch.

For now, I use "local" copy of methods.json file with int instead of epoch and it works as expected.

Regards, Pawel.