-
Notifications
You must be signed in to change notification settings - Fork 12
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 typo in address parameter helper #47
Conversation
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.
Changelog should reflect the bug fix please!
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.
LGTM pending the changelog update!
This change adds a failing spec which exposes a typo in our API params helper method. In the next change we'll fix the issue and un-pend the spec.
This change fixes a typo in the params helper for addresses without a `state` association. This was a previously untested behaviour so we never caught this. This change also marks the failing test as no longer pending.
124bdbc
to
b1b67da
Compare
This links to the PR where we fixed the issue from the changelog file, so we can know what is in the next release of the gem.
b1b67da
to
3b47180
Compare
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.
LGTM! Merge at will
What is the goal of this PR?
Fixes a typo in the helper method which transforms a
Spree::Address
into the payload parameters we send to TaxJar.How do you manually test these changes? (if applicable)
state
association, but with astate_name
instead.state_name
field should be serialized and sent to Tax Jar.Merge Checklist
Update the changelog(I am not sure if this needs to be done for this change?)