diff --git a/lib/stripe/api_operations/update.rb b/lib/stripe/api_operations/update.rb index c9a78f71..e9811154 100644 --- a/lib/stripe/api_operations/update.rb +++ b/lib/stripe/api_operations/update.rb @@ -2,11 +2,7 @@ module Stripe module APIOperations module Update def save(params={}) - values = serialize_params(self).merge(params) - - if @values[:metadata] - values[:metadata] = serialize_metadata - end + values = self.class.serialize_params(self).merge(params) if values.length > 0 values.delete(:id) @@ -16,42 +12,6 @@ module Stripe end self end - - def serialize_metadata - if @unsaved_values.include?(:metadata) - # the metadata object has been reassigned - # i.e. as object.metadata = {key => val} - metadata_update = @values[:metadata] # new hash - new_keys = metadata_update.keys.map(&:to_sym) - # remove keys at the server, but not known locally - keys_to_unset = @previous_metadata.keys - new_keys - keys_to_unset.each {|key| metadata_update[key] = ''} - - metadata_update - else - # metadata is a StripeObject, and can be serialized normally - serialize_params(@values[:metadata]) - end - end - - def serialize_params(obj) - case obj - when nil - '' - when StripeObject - unsaved_keys = obj.instance_variable_get(:@unsaved_values) - obj_values = obj.instance_variable_get(:@values) - update_hash = {} - - unsaved_keys.each do |k| - update_hash[k] = serialize_params(obj_values[k]) - end - - update_hash - else - obj - end - end end end end diff --git a/lib/stripe/stripe_object.rb b/lib/stripe/stripe_object.rb index 645a0c36..f4954d0c 100644 --- a/lib/stripe/stripe_object.rb +++ b/lib/stripe/stripe_object.rb @@ -43,7 +43,7 @@ module Stripe def refresh_from(values, opts, partial=false) @opts = opts - @previous_metadata = values[:metadata] + @original_values = Marshal.load(Marshal.dump(values)) # deep copy removed = partial ? Set.new : Set.new(@values.keys - values.keys) added = Set.new(values.keys - @values.keys) # Wipe old state before setting new. This is useful for e.g. updating a @@ -118,6 +118,75 @@ module Stripe end end + + def serialize_nested_object(key) + new_value = @values[key] + if @unsaved_values.include?(key) + # the object has been reassigned + # e.g. as object.key = {foo => bar} + update = new_value + new_keys = update.keys.map(&:to_sym) + # remove keys at the server, but not known locally + keys_to_unset = @original_values[key].keys - new_keys + keys_to_unset.each {|key| update[key] = ''} + + update + else + # can be serialized normally + self.class.serialize_params(new_value) + end + end + + def self.serialize_params(obj) + case obj + when nil + '' + when StripeObject + unsaved_keys = obj.instance_variable_get(:@unsaved_values) + obj_values = obj.instance_variable_get(:@values) + update_hash = {} + + unsaved_keys.each do |k| + update_hash[k] = serialize_params(obj_values[k]) + end + + obj_values.each do |k, v| + if v.is_a?(StripeObject) || v.is_a?(Hash) + update_hash[k] = obj.serialize_nested_object(k) + elsif v.is_a?(Array) + original_value = obj.instance_variable_get(:@original_values)[k] + if original_value && original_value.length > v.length + # url params provide no mechanism for deleting an item in an array, + # just overwriting the whole array or adding new items. So let's not + # allow deleting without a full overwrite until we have a solution. + raise ArgumentError.new( + "You cannot delete an item from an array, you must instead set a new array" + ) + end + update_hash[k] = serialize_params(v) + end + end + + update_hash + when Array + update_hash = {} + obj.each_with_index do |value, index| + update = serialize_params(value) + if update != {} + update_hash[index] = update + end + end + + if update_hash == {} + nil + else + update_hash + end + else + obj + end + end + protected def metaclass diff --git a/test/stripe/account_test.rb b/test/stripe/account_test.rb index 30b34238..1ea1888b 100644 --- a/test/stripe/account_test.rb +++ b/test/stripe/account_test.rb @@ -26,6 +26,31 @@ module Stripe assert !a.details_submitted end + should "be updatable" do + resp = { + :id => 'acct_foo', + :legal_entity => { + :address => { + :line1 => '1 Two Three' + } + } + } + @mock.expects(:get). + once. + with('https://api.stripe.com/v1/accounts/acct_foo', nil, nil). + returns(test_response(resp)) + + @mock.expects(:post). + once. + with('https://api.stripe.com/v1/accounts/acct_foo', nil, 'legal_entity[first_name]=Bob&legal_entity[address][line1]=2%20Three%20Four'). + returns(test_response(resp)) + + a = Stripe::Account.retrieve('acct_foo') + a.legal_entity.first_name = 'Bob' + a.legal_entity.address.line1 = '2 Three Four' + a.save + end + should "be able to deauthorize an account" do resp = {:id => 'acct_1234', :email => "test+bindings@stripe.com", :charge_enabled => false, :details_submitted => false} @mock.expects(:get).once.returns(test_response(resp)) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 6d06bcab..7a5783e0 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -416,6 +416,115 @@ module Stripe assert_equal true, rescued end end + + should 'add key to nested objects' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :size => 'l', + :score => 4, + :height => 10 + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, 'legal_entity[first_name]=Bob').returns(test_response({"id" => "myid"})) + + acct.legal_entity.first_name = 'Bob' + acct.save + end + + should 'save nothing if nothing changes' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :metadata => { + :key => 'value' + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, '').returns(test_response({"id" => "myid"})) + + acct.save + end + + should 'correctly handle replaced nested objects' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :last_name => 'Smith', + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, 'legal_entity[first_name]=Bob&legal_entity[last_name]=').returns(test_response({"id" => "myid"})) + + acct.legal_entity = {:first_name => 'Bob'} + acct.save + end + + should 'correctly handle array setting' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => {} + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, 'legal_entity[additional_owners][0][first_name]=Bob').returns(test_response({"id" => "myid"})) + + acct.legal_entity.additional_owners = [{:first_name => 'Bob'}] + acct.save + end + + should 'correctly handle array insertion' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :additional_owners => [] + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, 'legal_entity[additional_owners][0][first_name]=Bob').returns(test_response({"id" => "myid"})) + + acct.legal_entity.additional_owners << {:first_name => 'Bob'} + acct.save + end + + should 'correctly handle array updates' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :additional_owners => [{:first_name => 'Bob'}, {:first_name => 'Jane'}] + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, 'legal_entity[additional_owners][1][first_name]=Janet').returns(test_response({"id" => "myid"})) + + acct.legal_entity.additional_owners[1].first_name = 'Janet' + acct.save + end + + should 'correctly handle array noops' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :additional_owners => [{:first_name => 'Bob'}] + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, '').returns(test_response({"id" => "myid"})) + + acct.save + end + + should 'correctly handle hash noops' do + acct = Stripe::Account.construct_from({ + :id => 'myid', + :legal_entity => { + :address => {:line1 => '1 Two Three'} + } + }) + + @mock.expects(:post).once.with("#{Stripe.api_base}/v1/accounts/myid", nil, '').returns(test_response({"id" => "myid"})) + + acct.save + end end end end