From 3cb802ba415e80ac4778de1b71daa4a73e954449 Mon Sep 17 00:00:00 2001 From: Kevin O'Sullivan Date: Fri, 22 Apr 2022 16:46:55 -0400 Subject: [PATCH] Fixes `to_hash` to include readonly attributes 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 --- CHANGELOG.md | 1 + lib/shopify_api/rest/base.rb | 8 ++++---- test/clients/base_rest_resource_test.rb | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14beaeb4d..6f8963a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/shopify_api/rest/base.rb b/lib/shopify_api/rest/base.rb index a8127ca80..ce131411d 100644 --- a/lib/shopify_api/rest/base.rb +++ b/lib/shopify_api/rest/base.rb @@ -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 [ @@ -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) @@ -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) diff --git a/test/clients/base_rest_resource_test.rb b/test/clients/base_rest_resource_test.rb index 59923ef8a..d0aa41dc2 100644 --- a/test/clients/base_rest_resource_test.rb +++ b/test/clients/base_rest_resource_test.rb @@ -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