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

Implement filter by ids #300

Merged
merged 4 commits into from
Aug 30, 2020

Conversation

bartonip
Copy link
Contributor

Requested by: #267
Xero offers a fast way to select multiple invoices by using the IDs param. This has been added to the _filter method.

https://developer.xero.com/documentation/api/requests-and-responses

Usage:

xero.invoices.filter(IDs='f3ecf55f-b7b0-47d2-a0f3-97df84890af2,f3ecf55f-b7b0-47d2-a0f3-97df84890af2f3ecf55f-b7b0-47d2-a0f3-97df84890af2')

@bartonip
Copy link
Contributor Author

@freakboy3742 bump

Copy link
Owner

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we can provide a much more Pythonic API here. The Xero API may expose "IDs" as a comma separated string, but that doesn't mean PyXero's API needs to be the same. Otherwise, there's no benefit over using raw.

I'd expect the public API for IDs to be a list of strings, concatenated internally to a single string.

@bartonip
Copy link
Contributor Author

@freakboy3742 done

Copy link
Owner

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - thanks for the patch!

@freakboy3742 freakboy3742 merged commit d9478ab into freakboy3742:master Aug 30, 2020
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

Successfully merging this pull request may close these issues.

2 participants