Merge pull request #311 from stripe/brandur-mass-assign

Add attribute mass assignment and use it in #save

lib/stripe/api_operations/update.rb

@@ -16,6 +16,15 @@ module Stripe
       def save(params={})
         # Let the caller override the URL but avoid serializing it.
         req_url = params.delete(:req_url) || save_url
+
+        # We started unintentionally (sort of) allowing attributes send to
+        # +save+ to override values used during the update. So as not to break
+        # the API, this makes that official here.
+        update_attributes_with_options(params, :raise_error => false)
+
+        # Now remove any parameters that look like object attributes.
+        params = params.reject { |k, _| respond_to?(k) }
+
         values = self.class.serialize_params(self).merge(params)
 
         if values.length > 0

lib/stripe/stripe_object.rb

@@ -56,8 +56,8 @@ module Stripe
         @unsaved_values.delete(k)
       end
 
-      values.each do |k, v|
-        @values[k] = Util.convert_to_stripe_object(v, @opts)
+      update_attributes_with_options(values, :opts => opts)
+      values.each do |k, _|
         @transient_values.delete(k)
         @unsaved_values.delete(k)
       end
@@ -65,6 +65,11 @@ module Stripe
       return self
     end
 
+    # Mass assigns attributes on the model.
+    def update_attributes(values)
+      update_attributes_with_options(values, {})
+    end
+
     def [](k)
       @values[k.to_sym]
     end
@@ -273,5 +278,36 @@ module Stripe
     def respond_to_missing?(symbol, include_private = false)
       @values && @values.has_key?(symbol) || super
     end
+
+    # Mass assigns attributes on the model.
+    #
+    # This is a version of +update_attributes+ that takes some extra options
+    # for internal use.
+    #
+    # ==== Options
+    #
+    # * +:opts:+ Options for StripeObject like an API key.
+    # * +:raise_error:+ Set to false to suppress ArgumentErrors on keys that
+    #   don't exist.
+    def update_attributes_with_options(values, options={})
+      # `opts` are StripeObject options
+      opts        = options.fetch(:opts, {})
+      raise_error = options.fetch(:raise_error, true)
+
+      values.each do |k, v|
+        if !@@permanent_attributes.include?(k) && !self.respond_to?(:"#{k}=")
+          if raise_error
+            raise ArgumentError,
+              "#{k} is not an attribute that can be assigned on this object"
+          else
+            next
+          end
+        end
+
+        @values[k] = Util.convert_to_stripe_object(v, opts)
+        @unsaved_values.add(k)
+      end
+      self
+    end
   end
 end

test/stripe/api_resource_test.rb

@@ -634,11 +634,24 @@ module Stripe
           :display_name => nil,
         })
 
-        @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts", nil, 'display_name=stripe').returns(make_response({"id" => "charge_id"}))
+        @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts", nil, 'display_name=stripe').
+          returns(make_response({"id" => "charge_id"}))
 
         account.display_name = 'stripe'
         account.save
       end
+
+      should 'set attributes as part of save' do
+        account = Stripe::Account.construct_from({
+          :id => nil,
+          :display_name => nil,
+        })
+
+        @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts", nil, 'display_name=stripe').
+          returns(make_response({"id" => "charge_id"}))
+
+        account.save(:display_name => 'stripe')
+      end
     end
   end
 end

test/stripe/stripe_object_test.rb

@@ -38,5 +38,16 @@ module Stripe
       assert obj.bool?
       refute obj.respond_to?(:not_bool?)
     end
+
+    should "mass assign values with #update_attributes" do
+      obj = Stripe::StripeObject.construct_from({ :id => 1, :name => 'Stripe' })
+      obj.update_attributes(:name => 'STRIPE')
+      assert_equal "STRIPE", obj.name
+
+      e = assert_raises(ArgumentError) do
+        obj.update_attributes(:foo => 'bar')
+      end
+      assert_equal "foo is not an attribute that can be assigned on this object", e.message
+    end
   end
 end