Bug in pagination of orders

[vlad-kurdesov]

Current implementation of Orders.GetAllPagesAsync method doesn't really give you a way to work around any exception in process - if you received 10 pages and then you hit api limit, you will loose all data because ClientBase.ExecuteRequest will throw ApiLimitReachedException.

I think having these options might be helpful:

• return exception with already received data, so at least you would have some data returned
• have a way to tell client to retry multiple times (if ApiLimitReached, client could wait till next minute and try get same page again).

When I was trying to research problem, I found that GetPageAsync has critical bug. Page = 1 always.
please look at line 75 of https://github.com/nla-brandonjames/ShipStation4Net/blob/master/ShipStation4Net/Clients/Orders.cs

• I was trying to see if I could work around doing pagination via GetPageAsync method and I was expected to get error when I try to access page 1,000,000 with page size 500.. I was surprised to get 500 items.

Also, I have some other questions that I would like to ask:

1. do you plan to add integration with Create/Update multiple orders? (if I'm not mistaken, it's not in place currently) this functionality could help to work around api limitations for clients that have higher volume of orders.

[vlad-kurdesov]

there is also bug with page size on line 76 of same file

[vlad-kurdesov]

Shipments & Products most likely have bug in pagination as well
If you pass filter, then page & pagesize variables are ignored
line 59: https://github.com/nla-brandonjames/ShipStation4Net/blob/master/ShipStation4Net/Clients/Shipments.cs
and line 64: https://github.com/nla-brandonjames/ShipStation4Net/blob/master/ShipStation4Net/Clients/Products.cs

[vlad-kurdesov]

Question #2 that i had - do you plan to maintain this project or it is just something which was built as 'example of work'. I understand that if shipstation will provide their own client package on nuget, then there will be no reason to maintain different implementation of Api client. Just trying to get better idea what i could rely on.

[nla-brandonjames]

Hey,
Thanks for using this library. I created this in a week, and I am intending to upgrade the core ClientBase and related code as a separate REST Client along the way some time. I was expecting for there to be some issues. I was rushing. I also have a Zoho Client that I should be publishing sometime too. Also, I have seen your Pull Request and will review it shortly. I will be using the library to integrate into some internal applications for my business. Eventually ShipStation will be phased out here and this Client was made just for that purpose to make it easier to sever the tie. I have not implemented the bulk update yet. The core code doesn't have file support yet.

[vlad-kurdesov]

I think that error has to do with missing 'configuration.json' file in test project. When I got latest of this repo, this file was missing, and I assumed it's because this file contained some api credentials that were not supposed to be exposed.
on the other hand, even if file would be present, I think that master branch would still fail to pass all tests - current build status of master branch is "failing": https://ci.appveyor.com/project/nla-brandonjames/shipstation4net/branch/master
in this log you might see that ApiLimitReachedException is thrown in 4 test cases.
2 errors because [shipDate] should be nullable on order DTO..
ShipStation4Net.Tests.TestStores.TestGetStoreAsync test throws 'Bad Request' exception.

[nla-brandonjames]

OK. Yeah, the appveyor.yml file has credentials encrypted in it. I'm assuming it throws because it can't read the API Limit from the file in the tests. Most of the failed tests from the main branch were the reasons due to the API limit. The configuration.json file has the details. I can tell for the most part you just changed the affected files.

[nla-brandonjames]

Accepted PR #3.