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

@@ -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
 

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)

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