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

Taxjar::Base has confusing mixed data states #79

Open
mintyfresh opened this issue Jul 6, 2022 · 0 comments
Open

Taxjar::Base has confusing mixed data states #79

mintyfresh opened this issue Jul 6, 2022 · 0 comments

Comments

@mintyfresh
Copy link

Taxjar::Base and all the data models that currently inherit from it use a confusing mix of data states, stored both directly into an @attrs instance variable, and also managed by ModelAttributes (which stores each attribute in its own instance variable).

While these states are initially in-sync when the object is constructed, using any of the attribute writers causes these states to go out of sync, and data read from the attributes to no longer match the data to be sent in an API request.

Expected Behaviour

order = Taxjar::Order.new(to_city: 'Toronto')
assert_equal order.to_city, 'Toronto' # Passes
assert_equal order.to_hash['to_city'], 'Toronto' # Passes

order.to_city = 'New York'
assert_equal order.to_city, 'New York' # Passes
assert_equal order.to_hash['to_city'], 'New York' # Passes

assert_equal order.to_json, '{"to_city":"New York"}' # Passes

Actual Behaviour

order = Taxjar::Order.new(to_city: 'Toronto')
assert_equal order.to_city, 'Toronto' # Passes
assert_equal order.to_hash['to_city'], 'Toronto' # Passes

order.to_city = 'New York'
assert_equal order.to_city, 'New York' # Passes
assert_equal order.to_hash['to_city'], 'New York' # Fails! `@attrs['to_city']` is still equal to 'Toronto'!

assert_equal order.to_json, '{"to_city":"New York"}' # Fails! `#to_json` still returns `{"to_city":"Toronto"}`!

This is caused because Taxjar::Base stores two separate data states:

  • @attrs, which is only assigned in the constructor, and delegates #to_hash
  • ModelAttributes, which is initialized in the constructor, and whenever attribute writers are used, but never updates @attrs state

Another Example

order = Taxjar::Order.new

order.to_country = 'CA'
order.to_state = 'ON'
order.to_city = 'Toronto'

order.to_json # => "{}" - Empty JSON object!

This happens because Taxjar::Base#to_h and Taxjar::Base#to_hash delegate to @attrs, which is not updated by attribute writers.

Suggested Fix

All usage of @attrs should be removed, instead using the #attributes method provided by ModelAttributes.

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

1 participant