From 8b4b0621db04689f6058454fe68ed6006389635b Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 14 Aug 2019 12:55:10 +0200 Subject: [PATCH 1/3] Fix Rubocop violations in Stripe connect tests --- .rubocop_manual_todo.yml | 1 - spec/requests/checkout/stripe_connect_spec.rb | 122 ++++++++++++++---- 2 files changed, 99 insertions(+), 24 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index a561f065740..5e13b5ce2b7 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -341,7 +341,6 @@ Metrics/LineLength: - spec/performance/orders_controller_spec.rb - spec/performance/shop_controller_spec.rb - spec/requests/checkout/failed_checkout_spec.rb - - spec/requests/checkout/stripe_connect_spec.rb - spec/requests/embedded_shopfronts_headers_spec.rb - spec/requests/shop_spec.rb - spec/serializers/admin/customer_serializer_spec.rb diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb index d1ec739df94..d25fd74ea1a 100644 --- a/spec/requests/checkout/stripe_connect_spec.rb +++ b/spec/requests/checkout/stripe_connect_spec.rb @@ -6,8 +6,23 @@ let!(:order_cycle) { create(:simple_order_cycle) } let!(:enterprise) { create(:distributor_enterprise) } - let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: order_cycle.coordinator, receiver: enterprise, incoming: false, pickup_time: "Monday") } - let!(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 0), distributors: [enterprise]) } + let!(:exchange) do + create( + :exchange, + order_cycle: order_cycle, + sender: order_cycle.coordinator, + receiver: enterprise, + incoming: false, + pickup_time: "Monday" + ) + end + let!(:shipping_method) do + create( + :shipping_method, + calculator: Spree::Calculator::FlatRate.new(preferred_amount: 0), + distributors: [enterprise] + ) + end let!(:payment_method) { create(:stripe_payment_method, distributors: [enterprise]) } let!(:stripe_account) { create(:stripe_account, enterprise: enterprise) } let!(:line_item) { create(:line_item, price: 12.34) } @@ -17,19 +32,48 @@ let(:new_token) { "newtoken123" } let(:card_id) { "card_XyZ456" } let(:customer_id) { "cus_A123" } + let(:payments_attributes) do + { + payment_method_id: payment_method.id, + source_attributes: { + gateway_payment_profile_id: token, + cc_type: "visa", + last_digits: "4242", + month: 10, + year: 2025, + first_name: 'Jill', + last_name: 'Jeffreys' + } + } + end + let(:allowed_address_attributes) do + [ + "firstname", + "lastname", + "address1", + "address2", + "phone", + "city", + "zipcode", + "state_id", + "country_id" + ] + end let(:params) do - { format: :json, order: { - shipping_method_id: shipping_method.id, - payments_attributes: [{ payment_method_id: payment_method.id, source_attributes: { gateway_payment_profile_id: token, cc_type: "visa", last_digits: "4242", month: 10, year: 2025, first_name: 'Jill', last_name: 'Jeffreys' } }], - bill_address_attributes: address.attributes.slice("firstname", "lastname", "address1", "address2", "phone", "city", "zipcode", "state_id", "country_id"), - ship_address_attributes: address.attributes.slice("firstname", "lastname", "address1", "address2", "phone", "city", "zipcode", "state_id", "country_id") - } } + { + format: :json, order: { + shipping_method_id: shipping_method.id, + payments_attributes: [payments_attributes], + bill_address_attributes: address.attributes.slice(*allowed_address_attributes), + ship_address_attributes: address.attributes.slice(*allowed_address_attributes) + } + } end before do order_cycle_distributed_variants = double(:order_cycle_distributed_variants) - allow(OrderCycleDistributedVariants).to receive(:new).and_return(order_cycle_distributed_variants) - allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?).and_return(true) + allow(OrderCycleDistributedVariants).to receive(:new) { order_cycle_distributed_variants } + allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?) { true } allow(Stripe).to receive(:api_key) { "sk_test_12345" } order.update_attributes(distributor_id: enterprise.id, order_cycle_id: order_cycle.id) @@ -38,9 +82,22 @@ end context "when a new card is submitted" do - let(:store_response_mock) { { status: 200, body: JSON.generate(id: customer_id, default_card: card_id, sources: { data: [{ id: "1" }] }) } } - let(:token_response_mock) { { status: 200, body: JSON.generate(id: new_token) } } - let(:charge_response_mock) { { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } } + let(:store_response_mock) do + { + status: 200, + body: JSON.generate( + id: customer_id, + default_card: card_id, + sources: { data: [{ id: "1" }] } + ) + } + end + let(:token_response_mock) do + { status: 200, body: JSON.generate(id: new_token) } + end + let(:charge_response_mock) do + { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } + end context "and the user doesn't request that the card is saved for later" do before do @@ -67,7 +124,9 @@ end context "when the charge request returns an error message" do - let(:charge_response_mock) { { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } } + let(:charge_response_mock) do + { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } + end it "should not process the payment" do put update_checkout_path, params @@ -81,7 +140,8 @@ context "and the customer requests that the card is saved for later" do before do - params[:order][:payments_attributes][0][:source_attributes][:save_requested_by_customer] = '1' + source_attributes = params[:order][:payments_attributes][0][:source_attributes] + source_attributes[:save_requested_by_customer] = '1' # Saves the card against the user stub_request(:post, "https://api.stripe.com/v1/customers") @@ -95,7 +155,10 @@ # Charges the card stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""], body: /#{token}.*#{order.number}/).to_return(charge_response_mock) + .with( + basic_auth: ["sk_test_12345", ""], + body: /#{token}.*#{order.number}/ + ).to_return(charge_response_mock) end context "and the store, token and charge requests are successful" do @@ -115,19 +178,24 @@ end context "when the store request returns an error message" do - let(:store_response_mock) { { status: 402, body: JSON.generate(error: { message: "store-failure" }) } } + let(:store_response_mock) do + { status: 402, body: JSON.generate(error: { message: "store-failure" }) } + end it "should not process the payment" do put update_checkout_path, params expect(response.status).to be 400 json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq I18n.t(:spree_gateway_error_flash_for_checkout, error: 'store-failure') + expect(json_response["flash"]["error"]) + .to eq(I18n.t(:spree_gateway_error_flash_for_checkout, error: 'store-failure')) expect(order.payments.completed.count).to be 0 end end context "when the charge request returns an error message" do - let(:charge_response_mock) { { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } } + let(:charge_response_mock) do + { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } + end it "should not process the payment" do put update_checkout_path, params @@ -139,7 +207,9 @@ end context "when the token request returns an error message" do - let(:token_response_mock) { { status: 402, body: JSON.generate(error: { message: "token-failure" }) } } + let(:token_response_mock) do + { status: 402, body: JSON.generate(error: { message: "token-failure" }) } + end # Note, no requests have been stubbed it "should not process the payment" do @@ -169,7 +239,9 @@ end let(:token_response_mock) { { status: 200, body: JSON.generate(id: new_token) } } - let(:charge_response_mock) { { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } } + let(:charge_response_mock) do + { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } + end before do params[:order][:existing_card_id] = credit_card.id @@ -203,7 +275,9 @@ end context "when the charge request returns an error message" do - let(:charge_response_mock) { { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } } + let(:charge_response_mock) do + { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } + end it "should not process the payment" do put update_checkout_path, params @@ -215,7 +289,9 @@ end context "when the token request returns an error message" do - let(:token_response_mock) { { status: 402, body: JSON.generate(error: { message: "token-error" }) } } + let(:token_response_mock) do + { status: 402, body: JSON.generate(error: { message: "token-error" }) } + end it "should not process the payment" do put update_checkout_path, params From ffde7a38dfe7b6a4c4d9498773c72051aa46125c Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 14 Aug 2019 13:22:57 +0200 Subject: [PATCH 2/3] Add spacing to increase readability --- spec/requests/checkout/stripe_connect_spec.rb | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb index d25fd74ea1a..3cfdea4be82 100644 --- a/spec/requests/checkout/stripe_connect_spec.rb +++ b/spec/requests/checkout/stripe_connect_spec.rb @@ -110,10 +110,14 @@ context "and the charge request is successful" do it "should process the payment without storing card details" do put update_checkout_path, params + json_response = JSON.parse(response.body) + expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 + card = order.payments.completed.first.source + expect(card.gateway_customer_profile_id).to eq nil expect(card.gateway_payment_profile_id).to eq token expect(card.cc_type).to eq "visa" @@ -130,8 +134,11 @@ it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -164,10 +171,14 @@ context "and the store, token and charge requests are successful" do it "should process the payment, and stores the card/customer details" do put update_checkout_path, params + json_response = JSON.parse(response.body) + expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 + card = order.payments.completed.first.source + expect(card.gateway_customer_profile_id).to eq customer_id expect(card.gateway_payment_profile_id).to eq card_id expect(card.cc_type).to eq "visa" @@ -184,8 +195,11 @@ it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]) .to eq(I18n.t(:spree_gateway_error_flash_for_checkout, error: 'store-failure')) expect(order.payments.completed.count).to be 0 @@ -199,8 +213,11 @@ it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -214,8 +231,11 @@ # Note, no requests have been stubbed it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]).to eq "token-failure" expect(order.payments.completed.count).to be 0 end @@ -261,10 +281,14 @@ context "and the charge and token requests are accepted" do it "should process the payment, and keep the profile ids and other card details" do put update_checkout_path, params + json_response = JSON.parse(response.body) + expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 + card = order.payments.completed.first.source + expect(card.gateway_customer_profile_id).to eq customer_id expect(card.gateway_payment_profile_id).to eq card_id expect(card.cc_type).to eq "master" @@ -281,8 +305,11 @@ it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -295,8 +322,11 @@ it "should not process the payment" do put update_checkout_path, params + expect(response.status).to be 400 + json_response = JSON.parse(response.body) + expect(json_response["flash"]["error"]).to eq "token-error" expect(order.payments.completed.count).to be 0 end From 1a450733a3abba77049d6b3890608c1b0de39787 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 14 Aug 2019 13:25:01 +0200 Subject: [PATCH 3/3] Use ApiHelper to DRY calls to JSON.parse in spec --- spec/requests/checkout/stripe_connect_spec.rb | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb index 3cfdea4be82..a64bb44ab8c 100644 --- a/spec/requests/checkout/stripe_connect_spec.rb +++ b/spec/requests/checkout/stripe_connect_spec.rb @@ -3,6 +3,7 @@ describe "checking out an order with a Stripe Connect payment method", type: :request do include ShopWorkflow include AuthenticationWorkflow + include OpenFoodNetwork::ApiHelper let!(:order_cycle) { create(:simple_order_cycle) } let!(:enterprise) { create(:distributor_enterprise) } @@ -111,8 +112,6 @@ it "should process the payment without storing card details" do put update_checkout_path, params - json_response = JSON.parse(response.body) - expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 @@ -137,8 +136,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -172,8 +169,6 @@ it "should process the payment, and stores the card/customer details" do put update_checkout_path, params - json_response = JSON.parse(response.body) - expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 @@ -198,8 +193,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]) .to eq(I18n.t(:spree_gateway_error_flash_for_checkout, error: 'store-failure')) expect(order.payments.completed.count).to be 0 @@ -216,8 +209,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -234,8 +225,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq "token-failure" expect(order.payments.completed.count).to be 0 end @@ -282,8 +271,6 @@ it "should process the payment, and keep the profile ids and other card details" do put update_checkout_path, params - json_response = JSON.parse(response.body) - expect(json_response["path"]).to eq spree.order_path(order) expect(order.payments.completed.count).to be 1 @@ -308,8 +295,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq "charge-failure" expect(order.payments.completed.count).to be 0 end @@ -325,8 +310,6 @@ expect(response.status).to be 400 - json_response = JSON.parse(response.body) - expect(json_response["flash"]["error"]).to eq "token-error" expect(order.payments.completed.count).to be 0 end