From 7c9627d5e0a0388ed00a2695edfa030f482d78a7 Mon Sep 17 00:00:00 2001 From: Annie Li Date: Fri, 18 Nov 2022 14:46:13 -0800 Subject: [PATCH 1/3] Add tests for update --- test/stripe/api_resource_test.rb | 180 +++++++++++++++++++++++++++++-- 1 file changed, 169 insertions(+), 11 deletions(-) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 706801eee..ddc7334f6 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -314,7 +314,7 @@ class NestedTestAPIResource < APIResource assert_equal c.created, 12_345 end - should "updating an object should issue a POST request with only the changed properties" do + should "saving an object should issue a POST request with only the changed properties" do stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") .with(body: { "description" => "another_mn" }) .to_return(body: JSON.generate(customer_fixture)) @@ -323,7 +323,15 @@ class NestedTestAPIResource < APIResource c.save end - should "updating should merge in returned properties" do + should "updating an object should issue a POST request with only the changed properties" do + stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") + .with(body: { "description" => "another_mn" }) + .to_return(body: JSON.generate(customer_fixture)) + Stripe::Customer.construct_from(customer_fixture) + Stripe::Customer.update("cus_123", { description: "another_mn" }) + end + + should "saving should merge in returned properties" do stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") .with(body: { "description" => "another_mn" }) .to_return(body: JSON.generate(customer_fixture)) @@ -333,14 +341,21 @@ class NestedTestAPIResource < APIResource assert_equal false, c.livemode end - should "updating should fail if api_key is overwritten with nil" do + should "saving should fail if api_key is overwritten with nil" do c = Stripe::Customer.new assert_raises TypeError do c.save({}, api_key: nil) end end - should "updating should use the supplied api_key" do + should "updating should fail if api_key is overwritten with nil" do + Stripe::Customer.new("cus_123") + assert_raises TypeError do + Stripe::Customer.update("cus_123", {}, { api_key: nil }) + end + end + + should "saving should use the supplied api_key" do stub_request(:post, "#{Stripe.api_base}/v1/customers") .with(headers: { "Authorization" => "Bearer sk_test_local" }) .to_return(body: JSON.generate(customer_fixture)) @@ -349,6 +364,14 @@ class NestedTestAPIResource < APIResource assert_equal false, c.livemode end + should "updating should use the supplied api_key" do + stub_request(:post, "#{Stripe.api_base}/v1/customers") + .with(headers: { "Authorization" => "Bearer sk_test_local" }) + .to_return(body: JSON.generate(customer_fixture)) + Stripe::Customer.new("cus_123") + Stripe::Customer.update("cus_123", {}, api_key: "sk_test_local") + end + should "deleting should send no props and result in an object that has no props other deleted" do stub_request(:delete, "#{Stripe.api_base}/v1/customers/cus_123") .to_return(body: JSON.generate("id" => "cus_123", "deleted" => true)) @@ -385,7 +408,19 @@ class NestedTestAPIResource < APIResource c.save end - should "add key to nested objects" do + should "passing in a stripe_account header should pass it through on update" do + stub_request(:get, "#{Stripe.api_base}/v1/customers/cus_123") + .with(headers: { "Stripe-Account" => "acct_123" }) + .to_return(body: JSON.generate(customer_fixture)) + Stripe::Customer.retrieve("cus_123", stripe_account: "acct_123") + + stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") + .with(headers: { "Stripe-Account" => "acct_123" }) + .to_return(body: JSON.generate(customer_fixture)) + Stripe::Customer.update("cus_123", { description: "FOO" }) + end + + should "add key to nested objects on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { size: "l", @@ -401,6 +436,20 @@ class NestedTestAPIResource < APIResource acct.save end + should "add key to nested objects on update" do + Stripe::Account.construct_from(id: "myid", + business_profile: { + url: "example.com", + support_email: "test@example.com", + }) + + stub_request(:post, "#{Stripe.api_base}/v1/accounts/myid") + .with(body: { business_profile: { name: "Bob" } }) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Account.update("myid", { business_profile: { name: "Bob" } }) + end + should "save nothing if nothing changes" do acct = Stripe::Account.construct_from(id: "acct_id", metadata: { @@ -414,6 +463,19 @@ class NestedTestAPIResource < APIResource acct.save end + should "update nothing if nothing changes" do + Stripe::Account.construct_from(id: "acct_id", + metadata: { + key: "value", + }) + + stub_request(:post, "#{Stripe.api_base}/v1/accounts/acct_id") + .with(body: {}) + .to_return(body: JSON.generate("id" => "acct_id")) + + Stripe::Account.update("acct_id") + end + should "not save nested API resources" do ch = Stripe::Charge.construct_from(id: "ch_id", customer: { @@ -429,7 +491,21 @@ class NestedTestAPIResource < APIResource ch.save end - should "correctly handle replaced nested objects" do + should "not update nested API resources" do + Stripe::Charge.construct_from(id: "ch_id", + customer: { + object: "customer", + id: "customer_id", + }) + + stub_request(:post, "#{Stripe.api_base}/v1/charges/ch_id") + .with(body: {}) + .to_return(body: JSON.generate("id" => "ch_id")) + + Stripe::Charge.update("ch_id", { customer: { description: "Bob" } }) + end + + should "correctly handle replaced nested objects on save" do acct = Stripe::Account.construct_from( id: "acct_123", company: { @@ -449,7 +525,26 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array setting" do + should "correctly handle replaced nested objects on update" do + Stripe::Account.construct_from( + id: "acct_123", + company: { + name: "company_name", + address: { + line1: "test", + city: "San Francisco", + }, + } + ) + + stub_request(:post, "#{Stripe.api_base}/v1/accounts/acct_123") + .with(body: { company: { address: { line1: "Test2", city: "" } } }) + .to_return(body: JSON.generate("id" => "my_id")) + + Stripe::Account.update("acct_123", { company: { address: { line1: "Test2" } } }) + end + + should "correctly handle array setting on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: {}) @@ -461,7 +556,17 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array insertion" do + should "correctly handle array setting on update" do + Stripe::Subscription.construct_from(id: "myid") + + stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") + .with(body: { items: [{ plan: "foo", quantity: 2 }] }) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Subscription.update("myid", { items: [{ plan: "foo", quantity: 2 }] }) + end + + should "correctly handle array insertion on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { additional_owners: [], @@ -478,7 +583,20 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array updates" do + should "correctly handle array insertion on update" do + Stripe::Subscription.construct_from(id: "myid", items: []) + + # Note that this isn't a perfect check because we're using webmock's + # data decoding, which isn't aware of the Stripe array encoding that we + # use here. + stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") + .with(body: { items: [{ plan: "foo", quantity: 2 }] }) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Subscription.update("myid", { items: [{ plan: "foo", quantity: 2 }] }) + end + + should "correctly handle array updates on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { additional_owners: [{ first_name: "Bob" }, { first_name: "Jane" }], @@ -495,7 +613,24 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array noops" do + should "correctly handle array updates on update" do + Stripe::Subscription.construct_from(id: "myid", + items: [ + { plan: "foo", quantity: 2 }, + { plan: "bar", quantity: 2 }, + ]) + + # Note that this isn't a perfect check because we're using webmock's + # data decoding, which isn't aware of the Stripe array encoding that we + # use here. + stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") + .with(body: { items: [{ plan: "foos", quantity: 3 }, { plan: "bar", quantity: 3 }] }) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Subscription.update("myid", { items: [{ plan: "foos", quantity: 3 }, { plan: "bar", quantity: 3 }] }) + end + + should "correctly handle array noops on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { additional_owners: [{ first_name: "Bob" }], @@ -509,7 +644,17 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle hash noops" do + should "correctly handle array noops on update" do + Stripe::Subscription.construct_from(id: "myid", items: [{ plan: "foo", quantity: 2 }]) + + stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") + .with(body: {}) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Subscription.update("myid", {}) + end + + should "correctly handle hash noops on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { address: { line1: "1 Two Three" }, @@ -522,6 +667,19 @@ class NestedTestAPIResource < APIResource acct.save end + should "correctly handle hash noops on update" do + Stripe::Account.construct_from(id: "myid", + legal_entity: { + address: { line1: "1 Two Three" }, + }) + + stub_request(:post, "#{Stripe.api_base}/v1/accounts/myid") + .with(body: {}) + .to_return(body: JSON.generate("id" => "myid")) + + Stripe::Account.update("myid", {}) + end + should "should create a new resource when an object without an id is saved" do account = Stripe::Account.construct_from(id: nil, display_name: nil) From 981849270650f33de82b1069d86e747764ecebc8 Mon Sep 17 00:00:00 2001 From: anniel-stripe <97691964+anniel-stripe@users.noreply.github.com> Date: Mon, 21 Nov 2022 17:32:01 -0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Richard Marmorstein <52928443+richardm-stripe@users.noreply.github.com> --- test/stripe/api_resource_test.rb | 130 +------------------------------ 1 file changed, 3 insertions(+), 127 deletions(-) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index ddc7334f6..983bc90d1 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -323,7 +323,7 @@ class NestedTestAPIResource < APIResource c.save end - should "updating an object should issue a POST request with only the changed properties" do + should "updating an object should issue a POST request with the specified properties" do stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") .with(body: { "description" => "another_mn" }) .to_return(body: JSON.generate(customer_fixture)) @@ -348,7 +348,7 @@ class NestedTestAPIResource < APIResource end end - should "updating should fail if api_key is overwritten with nil" do + should "updating should fail if api_key is nil" do Stripe::Customer.new("cus_123") assert_raises TypeError do Stripe::Customer.update("cus_123", {}, { api_key: nil }) @@ -408,17 +408,6 @@ class NestedTestAPIResource < APIResource c.save end - should "passing in a stripe_account header should pass it through on update" do - stub_request(:get, "#{Stripe.api_base}/v1/customers/cus_123") - .with(headers: { "Stripe-Account" => "acct_123" }) - .to_return(body: JSON.generate(customer_fixture)) - Stripe::Customer.retrieve("cus_123", stripe_account: "acct_123") - - stub_request(:post, "#{Stripe.api_base}/v1/customers/cus_123") - .with(headers: { "Stripe-Account" => "acct_123" }) - .to_return(body: JSON.generate(customer_fixture)) - Stripe::Customer.update("cus_123", { description: "FOO" }) - end should "add key to nested objects on save" do acct = Stripe::Account.construct_from(id: "myid", @@ -436,13 +425,7 @@ class NestedTestAPIResource < APIResource acct.save end - should "add key to nested objects on update" do - Stripe::Account.construct_from(id: "myid", - business_profile: { - url: "example.com", - support_email: "test@example.com", - }) - + should "update with a nested object" do stub_request(:post, "#{Stripe.api_base}/v1/accounts/myid") .with(body: { business_profile: { name: "Bob" } }) .to_return(body: JSON.generate("id" => "myid")) @@ -463,19 +446,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "update nothing if nothing changes" do - Stripe::Account.construct_from(id: "acct_id", - metadata: { - key: "value", - }) - - stub_request(:post, "#{Stripe.api_base}/v1/accounts/acct_id") - .with(body: {}) - .to_return(body: JSON.generate("id" => "acct_id")) - - Stripe::Account.update("acct_id") - end - should "not save nested API resources" do ch = Stripe::Charge.construct_from(id: "ch_id", customer: { @@ -491,20 +461,6 @@ class NestedTestAPIResource < APIResource ch.save end - should "not update nested API resources" do - Stripe::Charge.construct_from(id: "ch_id", - customer: { - object: "customer", - id: "customer_id", - }) - - stub_request(:post, "#{Stripe.api_base}/v1/charges/ch_id") - .with(body: {}) - .to_return(body: JSON.generate("id" => "ch_id")) - - Stripe::Charge.update("ch_id", { customer: { description: "Bob" } }) - end - should "correctly handle replaced nested objects on save" do acct = Stripe::Account.construct_from( id: "acct_123", @@ -525,24 +481,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle replaced nested objects on update" do - Stripe::Account.construct_from( - id: "acct_123", - company: { - name: "company_name", - address: { - line1: "test", - city: "San Francisco", - }, - } - ) - - stub_request(:post, "#{Stripe.api_base}/v1/accounts/acct_123") - .with(body: { company: { address: { line1: "Test2", city: "" } } }) - .to_return(body: JSON.generate("id" => "my_id")) - - Stripe::Account.update("acct_123", { company: { address: { line1: "Test2" } } }) - end should "correctly handle array setting on save" do acct = Stripe::Account.construct_from(id: "myid", @@ -556,16 +494,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array setting on update" do - Stripe::Subscription.construct_from(id: "myid") - - stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") - .with(body: { items: [{ plan: "foo", quantity: 2 }] }) - .to_return(body: JSON.generate("id" => "myid")) - - Stripe::Subscription.update("myid", { items: [{ plan: "foo", quantity: 2 }] }) - end - should "correctly handle array insertion on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { @@ -583,19 +511,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array insertion on update" do - Stripe::Subscription.construct_from(id: "myid", items: []) - - # Note that this isn't a perfect check because we're using webmock's - # data decoding, which isn't aware of the Stripe array encoding that we - # use here. - stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") - .with(body: { items: [{ plan: "foo", quantity: 2 }] }) - .to_return(body: JSON.generate("id" => "myid")) - - Stripe::Subscription.update("myid", { items: [{ plan: "foo", quantity: 2 }] }) - end - should "correctly handle array updates on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { @@ -613,22 +528,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array updates on update" do - Stripe::Subscription.construct_from(id: "myid", - items: [ - { plan: "foo", quantity: 2 }, - { plan: "bar", quantity: 2 }, - ]) - - # Note that this isn't a perfect check because we're using webmock's - # data decoding, which isn't aware of the Stripe array encoding that we - # use here. - stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") - .with(body: { items: [{ plan: "foos", quantity: 3 }, { plan: "bar", quantity: 3 }] }) - .to_return(body: JSON.generate("id" => "myid")) - - Stripe::Subscription.update("myid", { items: [{ plan: "foos", quantity: 3 }, { plan: "bar", quantity: 3 }] }) - end should "correctly handle array noops on save" do acct = Stripe::Account.construct_from(id: "myid", @@ -644,16 +543,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array noops on update" do - Stripe::Subscription.construct_from(id: "myid", items: [{ plan: "foo", quantity: 2 }]) - - stub_request(:post, "#{Stripe.api_base}/v1/subscriptions/myid") - .with(body: {}) - .to_return(body: JSON.generate("id" => "myid")) - - Stripe::Subscription.update("myid", {}) - end - should "correctly handle hash noops on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { @@ -667,19 +556,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle hash noops on update" do - Stripe::Account.construct_from(id: "myid", - legal_entity: { - address: { line1: "1 Two Three" }, - }) - - stub_request(:post, "#{Stripe.api_base}/v1/accounts/myid") - .with(body: {}) - .to_return(body: JSON.generate("id" => "myid")) - - Stripe::Account.update("myid", {}) - end - should "should create a new resource when an object without an id is saved" do account = Stripe::Account.construct_from(id: nil, display_name: nil) From f9318226b2ec5150a88a48f89482c4504cbdb26f Mon Sep 17 00:00:00 2001 From: Annie Li Date: Mon, 21 Nov 2022 17:34:13 -0800 Subject: [PATCH 3/3] Lint --- test/stripe/api_resource_test.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 983bc90d1..948bfbbbe 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -408,7 +408,6 @@ class NestedTestAPIResource < APIResource c.save end - should "add key to nested objects on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: { @@ -481,7 +480,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array setting on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: {}) @@ -528,7 +526,6 @@ class NestedTestAPIResource < APIResource acct.save end - should "correctly handle array noops on save" do acct = Stripe::Account.construct_from(id: "myid", legal_entity: {