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

fix: idempotencyKey should be optional in header cause it is a optional params in all API #651

Conversation

madison890000
Copy link

Resolves #649

@madison890000
Copy link
Author

Description

As described in issue 649, in the xero-node 4.36.0 version, the optional parameter idempotencyKey has been added to all APIs (This allows you to safely retry requests without the risk of duplicate processing. 128 character max.). However, the order of adding idempotencyKey in different APIs is not consistent, and not all APIs have added the idempotencyKey parameter as the last optional parameter. This resulted in a break change when upgrading from xero-node 4.35.0 to 4.36.0, which was not reflected in #648. This may have been an oversight.

Based on the following two reasons, I am trying to submit this Pull Request to fix the issue:

  1. 4.36.0 has already been released, and reordering it to the end will cause a break change in 4.36.0.
  2. It is mentioned in idempotencyKey parameter introduced in 4.36.0 breaks existing calls to accounting api #649 (comment) that passing null/undefined does not solve the problem.

My proposed solution in this Pull Request is to not adjust the existing order of the idempotencyKey parameter, but to check for null values and then include the value in the header, in accordance with the definition of optional.

Please note that I have not fixed all APIs currently, but I have only fixed the updateOrCreateInvoices function. If you accept this solution, I will synchronize the fix for all APIs and add corresponding unit tests.

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

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.

idempotencyKey parameter introduced in 4.36.0 breaks existing calls to accounting api
1 participant