Skip to content

Commit

Permalink
Fixes to_hash to include readonly attributes
Browse files Browse the repository at this point in the history
Adds parameter to `to_hash` so that default behaviour includes readonly
attributes; setting the parameter to `true` indicates that `to_hash` is
being used to serialize the object for saving (and thus excludes
readonly attributes).

Fixes issue #930
  • Loading branch information
mkevinosullivan committed Apr 25, 2022
1 parent 71bab21 commit 3cb802b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- [#935](https://github.com/Shopify/shopify_api/pull/935) Fix issue [#931](https://github.com/Shopify/shopify_api/pull/931), weight of variant should be float
- [#944](https://github.com/Shopify/shopify_api/pull/944) Deprecated the `validate_shop` method from the JWT class since we can trust the token payload, since it comes from Shopify.
- [#941](https://github.com/Shopify/shopify_api/pull/941) Fix `to_hash` to return readonly attributes, unless being used for serialize the object for saving - fix issue [#930](https://github.com/Shopify/shopify_api/issues/930)

## Version 10.0.2

Expand Down
8 changes: 4 additions & 4 deletions lib/shopify_api/rest/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ def respond_to_missing?(meth_id, *args)
match.nil? ? true : super
end

sig { returns(T::Hash[String, T.untyped]) }
def to_hash
sig { params(saving: T::Boolean).returns(T::Hash[String, T.untyped]) }
def to_hash(saving = false)
hash = {}
instance_variables.each do |var|
next if [
Expand All @@ -273,7 +273,7 @@ def to_hash
:"@errors",
:"@aliased_properties",
].include?(var)
next if self.class.read_only_attributes&.include?(var)
next if saving && self.class.read_only_attributes&.include?(var)

var = var.to_s.delete("@")
attribute = if @aliased_properties.value?(var)
Expand Down Expand Up @@ -313,7 +313,7 @@ def save!

sig { params(update_object: T::Boolean).void }
def save(update_object: false)
hash = HashDiff::Comparison.new(original_state, to_hash).left_diff
hash = HashDiff::Comparison.new(original_state, to_hash(true)).left_diff
method = hash[self.class.primary_key] ? :put : :post

path = self.class.get_path(http_method: method, operation: method, entity: self)
Expand Down
16 changes: 16 additions & 0 deletions test/clients/base_rest_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,22 @@ def test_save_ignores_unsaveable_attributes
assert_nil(resource.id)
end

def test_to_hash_includes_unsaveable_attributes
resource = TestHelpers::FakeResource.new(session: @session)
resource.attribute = "attribute"
resource.unsaveable_attribute = "this is an attribute"

assert_includes(resource.to_hash, "unsaveable_attribute")
end

def test_to_hash_for_saving_excludes_unsaveable_attributes
resource = TestHelpers::FakeResource.new(session: @session)
resource.attribute = "attribute"
resource.unsaveable_attribute = "this is an attribute"

refute_includes(resource.to_hash(true), "unsaveable_attribute")
end

def test_deletes_existing_resource_and_fails_on_deleting_nonexistent_resource
resource = TestHelpers::FakeResource.new(session: @session)
resource.id = 1
Expand Down

0 comments on commit 3cb802b

Please sign in to comment.