search

ChristopherHaws / gdax-dotnet

DotNet Core GDAX Exchange API
MIT License
7 stars 7 forks source link

GetFills_ShouldAllowOlderAndNewerPage Gdax.GdaxException : query must contained either product_id or order_id #10

Open markwkjr opened 5 years ago

markwkjr commented 5 years ago

Guessing this is perhaps an API change since they've added so many new Products. But need Product now to perform this.

Fixing this with client.GetFills(productId: "BTC-USD", paging: new PagingOptions<Int32?>. Working on PR.

Test Name:  Gdax.Fills.GetFillsQueryTests.GetFills_ShouldAllowOlderAndNewerPage
Test FullName:  Gdax.Fills.GetFillsQueryTests.GetFills_ShouldAllowOlderAndNewerPage
Test Source:    C:\gdax-dotnet\Gdax.Tests\Fills\GetFillsQueryTests.cs : line 26
Test Outcome:   Failed
Test Duration:  0:00:24.669

Result StackTrace:  
at Gdax.GdaxClient.GetResponse(GdaxRequest request) in C:\gdax-dotnet\Gdax\GdaxClient.cs:line 75
   at Gdax.GdaxClient.GetResponse[TResponse](GdaxRequest request) in C:\gdax-dotnet\Gdax\GdaxClient.cs:line 50
   at Gdax.GdaxClient.GetFills(String orderId, String productId, PagingOptions`1 paging) in C:\gdax-dotnet\Gdax\GdaxClient.Fills.cs:line 18
   at Gdax.Fills.GetFillsQueryTests.GetFills_ShouldAllowOlderAndNewerPage() in C:\gdax-dotnet\Gdax.Tests\Fills\GetFillsQueryTests.cs:line 51
--- End of stack trace from previous location where exception was thrown ---
Result Message: Gdax.GdaxException : query must contained either product_id or order_id
markwkjr commented 5 years ago

PR #12 submitted to fix this issue I only fixed this by updating the Tests to include the product_id

Thought it might be good to have discussion around if GdaxClient.GetFills should be updated though? Since query must contain either product_id or order_id should an IF statement to check if BOTH of these are null be inserted and throw if that's the case? Why not throw immediately in this library rather than waiting for failure from the API. Thoughts?