From 8b3b27a55e4e98849a3d9a570b4cbe6f531e3f21 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 | 26 ++++++++++++++++++------- test/clients/base_rest_resource_test.rb | 18 +++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 156867ad1..cb0a353ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,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..974ae5b90 100644 --- a/lib/shopify_api/rest/base.rb +++ b/lib/shopify_api/rest/base.rb @@ -116,11 +116,17 @@ def has_many?(attribute) @has_many.include?(attribute) end + sig { returns(T::Hash[Symbol, Class]) } + attr_reader :has_many + sig { params(attribute: Symbol).returns(T::Boolean) } def has_one?(attribute) @has_one.include?(attribute) end + sig { returns(T::Hash[Symbol, Class]) } + attr_reader :has_one + sig { returns(T.nilable(T::Array[Symbol])) } def read_only_attributes @read_only_attributes&.map { |a| :"@#{a}" } @@ -261,8 +267,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 +279,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) @@ -283,10 +289,16 @@ def to_hash end.to_sym if self.class.has_many?(attribute) - hash[attribute.to_s] = get_property(attribute).map(&:to_hash).to_a if get_property(attribute) + hash[attribute.to_s] = get_property(attribute).map do |element| + data = element + data = self.class.has_many[attribute].create_instance(session: session, data: element) if data.is_a?(Hash) + data.to_hash(saving) unless data.nil? + end.to_a if get_property(attribute) elsif self.class.has_one?(attribute) - element_hash = get_property(attribute)&.to_hash - hash[attribute.to_s] = element_hash if element_hash || @forced_nils[attribute.to_s] + data = get_property(attribute) + data = self.class.has_one[attribute].create_instance(session: session, data: data) if data.is_a?(Hash) + data = data.to_hash(saving) if data + hash[attribute.to_s] = data if data || @forced_nils[attribute.to_s] elsif !get_property(attribute).nil? || @forced_nils[attribute.to_s] hash[attribute.to_s] = get_property(attribute) @@ -313,7 +325,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..dc3585f69 100644 --- a/test/clients/base_rest_resource_test.rb +++ b/test/clients/base_rest_resource_test.rb @@ -163,6 +163,24 @@ 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, "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" + + assert_includes(resource.to_hash(true), "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