Merge pull request #559 from stripe/brandur-include-saved-resources

Include IDs of resources set into properties

lib/stripe/stripe_object.rb

@@ -176,7 +176,7 @@ module Stripe
         unsaved = @unsaved_values.include?(k)
         if options[:force] || unsaved || v.is_a?(StripeObject)
           update_hash[k.to_sym] =
-            serialize_params_value(@values[k], @original_values[k], unsaved, options[:force])
+            serialize_params_value(@values[k], @original_values[k], unsaved, options[:force], key: k)
         end
       end
 
@@ -337,7 +337,7 @@ module Stripe
       self
     end
 
-    def serialize_params_value(value, original, unsaved, force)
+    def serialize_params_value(value, original, unsaved, force, key: nil)
       case true
       when value == nil
         ''
@@ -348,13 +348,32 @@ module Stripe
       # be updated from their proper endpoints, and therefore they are not
       # included when serializing even if they've been modified.
       #
-      # There are _some_ known exceptions though. For example, to save on API
-      # calls it's sometimes desirable to update a customer's default source by
-      # setting a new card (or other) object with `#source=` and then saving
-      # the customer. The `#save_with_parent` flag to override the default
-      # behavior allows us to handle these exceptions.
+      # There are _some_ known exceptions though.
+      #
+      # For example, if the value is unsaved (meaning the user has set it), and
+      # it looks like the API resource is persisted with an ID, then we include
+      # the object so that parameters are serialized with a reference to its
+      # ID.
+      #
+      # Another example is that on save API calls it's sometimes desirable to
+      # update a customer's default source by setting a new card (or other)
+      # object with `#source=` and then saving the customer. The
+      # `#save_with_parent` flag to override the default behavior allows us to
+      # handle these exceptions.
+      #
+      # We throw an error if a property was set explicitly but we can't do
+      # anything with it because the integration is probably not working as the
+      # user intended it to.
       when value.is_a?(APIResource) && !value.save_with_parent
-        nil
+        if !unsaved
+          nil
+        elsif value.respond_to?(:id) && value.id != nil
+          value
+        else
+          raise ArgumentError, "Cannot save property `#{key}` containing " \
+            "an API resource. It doesn't appear to be persisted and is " \
+            "not marked as `save_with_parent`."
+        end
 
       when value.is_a?(Array)
         update = value.map { |v| serialize_params_value(v, nil, true, force) }

test/stripe/stripe_object_test.rb

@@ -266,16 +266,29 @@ module Stripe
       assert_equal([{ :foo => "bar" }], serialized[:metadata])
     end
 
-    should "#serialize_params and remove embedded APIResources" do
+    should "#serialize_params and embed an API resource that's been set and has an ID" do
+      customer = Customer.construct_from({ :id => "cus_123" })
+      obj = Stripe::StripeObject.construct_from({})
+
+      # the key here is that the property is set explicitly (and therefore
+      # marked as unsaved), which is why it gets included below
+      obj.customer = customer
+
+      serialized = obj.serialize_params
+      assert_equal({ :customer => customer }, serialized)
+    end
+
+    should "#serialize_params and not include API resources that have not been set" do
+      customer = Customer.construct_from({ :id => "cus_123" })
       obj = Stripe::StripeObject.construct_from({
-        :customer => Customer.construct_from({})
+        :customer => customer
       })
 
       serialized = obj.serialize_params
       assert_equal({}, serialized)
     end
 
-    should "#serialize_params and remove embedded APIResources unless flagged with save_with_parent" do
+    should "#serialize_params serializes API resources flagged with save_with_parent" do
       c = Customer.construct_from({})
       c.save_with_parent = true
 
@@ -287,6 +300,23 @@ module Stripe
       assert_equal({ :customer => {} }, serialized)
     end
 
+    should "#serialize_params should raise an error on other embedded API resources" do
+      # This customer doesn't have an ID and therefore the library doesn't know
+      # what to do with it and throws an ArgumentError because it's probably
+      # not what the user expected to happen.
+      customer = Customer.construct_from({})
+
+      obj = Stripe::StripeObject.construct_from({})
+      obj.customer = customer
+
+      e = assert_raises(ArgumentError) do
+        obj.serialize_params
+      end
+      assert_equal "Cannot save property `customer` containing " \
+        "an API resource. It doesn't appear to be persisted and is " \
+        "not marked as `save_with_parent`.", e.message
+    end
+
     should "#serialize_params takes a force option" do
       obj = Stripe::StripeObject.construct_from({
         :id => 'id',