new trades endpoint #111
new trades endpoint #111
Conversation
src/trades_call_builder.js
Outdated
| } | ||
| return this; | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like it doesn't allow to set all the query params (offer_id, cursor, order, limit...).
There was a problem hiding this comment.
Adding offer_id. Paging params come for free from the super class CallBuilder but adding a test anyway.
src/trades_call_builder.js
Outdated
| /** | ||
| * Creates a new {@link TradesCallBuilder} pointed to server defined by serverUrl. | ||
| * | ||
| * Do not create this object directly, use {@link Server#orderbook}. |
| this.url.addQuery("counter_asset_issuer", counter.getIssuer()); | ||
| } else { | ||
| this.url.addQuery("counter_asset_type", 'native'); | ||
| } |
There was a problem hiding this comment.
Not in a scope of this PR but I see this code a lot. Would be great to move it to a helper function.
src/trades_call_builder.js
Outdated
| * @param offerId | ||
| * @returns {TradesCallBuilder} | ||
| */ | ||
| forOffer(offerId) { |
There was a problem hiding this comment.
Extra space in before function name?
There was a problem hiding this comment.
weird. will fix
test/unit/server_test.js
Outdated
|
|
||
| describe("TradesCallBuilder", function() { | ||
| it("trades() requests the correct endpoint (no filters)", function (done) { | ||
| let response = "trades_no_filters"; |
There was a problem hiding this comment.
I've seen your comment about mocked responses in other PR but I think we should make them similar to real ones. For example we should test if functions are correctly created from _links.
|
@bartekn pushed fixes. The |
remove old
/orderbook/tradesadd
/tradeswith optional asset filters