-
Notifications
You must be signed in to change notification settings - Fork 125
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
Update GET TaxRates spec #579
Conversation
PETOSS-386 |
Thanks for raising an issue, a ticket has been created to track your request |
- OAuth2: [accounting.settings, accounting.settings.read] | ||
tags: | ||
- Accounting | ||
operationId: getTaxRates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jvcudis - can you update the operationId to "getTaxRateByTaxType"? This is how the other GETs by a given field are labelled in the spec. Also, "getTaxRates" causes an error because it is already used as the operationId for GET /TaxRates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@0GH4J - thanks for reviewing the PR 😄
I updated the operationId value as requested.
"ProviderName": "Xero API Partner", | ||
"DateTimeUTC": "\/Date(1550797359081)\/", | ||
"TaxRates": [ | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please indent the example JSON?
It's reported by users (see CX0014522641 CX ticket) that the TaxType query param is not working. This query param is not supported by the Accounting API. What is supported is record filtering which allows users to append the
TaxType
in the endpoint's URL to filter for TaxRate with specific TaxType.Does not work ->
/TaxRates?TaxType={TaxType}
Works ->
/TaxRates/{TaxType}
Description
TaxType
query param in GET TaxRatesTaxRates/{TaxTypes}
Release Notes
We need to make this change to avoid confusing users since I few have used
TaxType
as query param.Screenshots (if appropriate):
Types of Changes