Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Querying ALL Orders #431

Closed
jonathansampson opened this issue Mar 13, 2021 · 5 comments
Closed

Querying ALL Orders #431

jonathansampson opened this issue Mar 13, 2021 · 5 comments

Comments

@jonathansampson
Copy link

jonathansampson commented Mar 13, 2021

An opportunity to improve documentation:

Related to #339, I was curious how to query for all orders (rather than for open orders alone). It seemed like the only parameters we could pass with the request were pagination values. After a quick scan of the source I realized a general query-string is created from whatever object is passed as the pagination object (thanks to loose restrictions on the interface). As such, getOpenOrders({ status: 'all' }) would suffice to retrieve all orders.

async getOpenOrders(pagination?: Pagination): Promise<PaginatedData<Order>> {
const resource = OrderAPI.URL.ORDERS;
const response = await this.apiClient.get<Order[]>(resource, {params: pagination});
return {
data: response.data,
pagination: {
after: response.headers['cb-after'],
before: response.headers['cb-before'],
},
};
}

@bennycode
Copy link
Owner

Hi @jonathansampson, can you point me to Coinbase's documentation for { status: 'all' }? If it is a valid query string, then I will add it to the list of accepted params for getOpenOrders.

@jonathansampson
Copy link
Author

jonathansampson commented Mar 13, 2021

@bennycode Certainly, see https://docs.pro.coinbase.com/#list-orders.

image

I would suggest a getOrders method in addition to (for backwards compat, assuming others have also been using the method in this manner) modifying getOpenOrders. getOrders would then look for status, product_id, and/or pagination values.

@bennycode
Copy link
Owner

I think extending the existing parameter is a good idea. What's your take on that? https://github.com/bennycode/coinbase-pro-node/pull/432/files

@jonathansampson
Copy link
Author

jonathansampson commented Mar 14, 2021

@bennycode Thanks! I added a couple comments for your consideration.

@bennycode
Copy link
Owner

Thank you! 🤝 Fixed with #432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants