From 1a734aacf8ea383453c1eaf19ca32e31eca05b3d Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 17 Aug 2020 17:16:44 +0100 Subject: [PATCH 01/28] Allow user to upload terms and conditions PDF file to an enterprise --- app/models/enterprise.rb | 8 ++- .../api/admin/enterprise_serializer.rb | 2 +- .../permitted_attributes/enterprise.rb | 2 +- .../form/_business_details.html.haml | 10 +++ config/locales/en.yml | 2 + ...add_terms_and_conditions_to_enterprises.rb | 5 ++ db/schema.rb | 32 +++++----- .../features/admin/enterprises/images_spec.rb | 2 +- .../enterprises/terms_and_conditions_spec.rb | 61 +++++++++++++++++++ 9 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20200817150002_add_terms_and_conditions_to_enterprises.rb create mode 100644 spec/features/admin/enterprises/terms_and_conditions_spec.rb diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 407d0a71cd..971bcc4216 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -74,10 +74,16 @@ class Enterprise < ActiveRecord::Base }, url: '/images/enterprises/promo_images/:id/:style/:basename.:extension', path: 'public/images/enterprises/promo_images/:id/:style/:basename.:extension' - validates_attachment_content_type :logo, content_type: %r{\Aimage/.*\Z} validates_attachment_content_type :promo_image, content_type: %r{\Aimage/.*\Z} + has_attached_file :terms_and_conditions, + url: '/files/enterprises/terms_and_conditions/:id/:basename.:extension', + path: 'public/files/enterprises/terms_and_conditions/:id/:basename.:extension' + validates_attachment_content_type :terms_and_conditions, + content_type: "application/pdf", + message: I18n.t(:enterprise_terms_and_conditions_type_error) + include Spree::Core::S3Support supports_s3 :logo supports_s3 :promo_image diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb index 02eff79426..5c66621744 100644 --- a/app/serializers/api/admin/enterprise_serializer.rb +++ b/app/serializers/api/admin/enterprise_serializer.rb @@ -6,7 +6,7 @@ class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer :preferred_product_selection_from_inventory_only, :preferred_show_customer_names_to_suppliers, :owner, :contact, :users, :tag_groups, :default_tag_group, :require_login, :allow_guest_orders, :allow_order_changes, - :logo, :promo_image + :logo, :promo_image, :terms_and_conditions, :terms_and_conditions_file_name has_one :owner, serializer: Api::Admin::UserSerializer has_many :users, serializer: Api::Admin::UserSerializer diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb index dc71d76d86..0516c9fe3e 100644 --- a/app/services/permitted_attributes/enterprise.rb +++ b/app/services/permitted_attributes/enterprise.rb @@ -25,7 +25,7 @@ module PermittedAttributes [ :id, :name, :visible, :permalink, :owner_id, :contact_name, :email_address, :phone, :is_primary_producer, :sells, :website, :facebook, :instagram, :linkedin, :twitter, - :description, :long_description, :logo, :promo_image, + :description, :long_description, :logo, :promo_image, :terms_and_conditions, :allow_guest_orders, :allow_order_changes, :require_login, :enable_subscriptions, :abn, :acn, :charges_sales_tax, :display_invoice_logo, :invoice_text, :preferred_product_selection_from_inventory_only, :preferred_shopfront_message, diff --git a/app/views/admin/enterprises/form/_business_details.html.haml b/app/views/admin/enterprises/form/_business_details.html.haml index c5953ba5ef..84686fafbe 100644 --- a/app/views/admin/enterprises/form/_business_details.html.haml +++ b/app/views/admin/enterprises/form/_business_details.html.haml @@ -31,3 +31,13 @@ = f.label :invoice_text, t('.invoice_text') .omega.eight.columns = f.text_area :invoice_text, style: "width: 100%; height: 100px;" + +.row + .alpha.three.columns + = f.label :terms_and_conditions, t('.terms_and_conditions') + + .omega.eight.columns + %a{ href: '{{ Enterprise.terms_and_conditions }}', ng: { if: 'Enterprise.terms_and_conditions' } } + = '{{ Enterprise.terms_and_conditions_file_name }}' + %p + = f.file_field :terms_and_conditions diff --git a/config/locales/en.yml b/config/locales/en.yml index 217f78f7cf..8867d8f6f7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -690,6 +690,7 @@ en: acn_placeholder: eg. 123 456 789 display_invoice_logo: Display logo in invoices invoice_text: Add customized text at the end of invoices + terms_and_conditions: "Terms and Conditions" contact: name: Name name_placeholder: eg. Gustav Plum @@ -2433,6 +2434,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_name_error: "has already been taken. If this is your enterprise and you would like to claim ownership, or if you would like to trade with this enterprise please contact the current manager of this profile at %{email}." enterprise_owner_error: "^%{email} is not permitted to own any more enterprises (limit is %{enterprise_limit})." enterprise_role_uniqueness_error: "^That role is already present." + enterprise_terms_and_conditions_type_error: "Only PDFs are allowed" inventory_item_visibility_error: must be true or false product_importer_file_error: "error: no file uploaded" product_importer_spreadsheet_error: "could not process file: invalid filetype" diff --git a/db/migrate/20200817150002_add_terms_and_conditions_to_enterprises.rb b/db/migrate/20200817150002_add_terms_and_conditions_to_enterprises.rb new file mode 100644 index 0000000000..57b646abd6 --- /dev/null +++ b/db/migrate/20200817150002_add_terms_and_conditions_to_enterprises.rb @@ -0,0 +1,5 @@ +class AddTermsAndConditionsToEnterprises < ActiveRecord::Migration + def change + add_attachment :enterprises, :terms_and_conditions + end +end diff --git a/db/schema.rb b/db/schema.rb index f56c95f405..83e5d66fa6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20200721135726) do +ActiveRecord::Schema.define(version: 20200817150002) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -190,8 +190,8 @@ ActiveRecord::Schema.define(version: 20200721135726) do t.integer "address_id" t.text "pickup_times" t.string "next_collection_at" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.text "distributor_info" t.string "logo_file_name" t.string "logo_content_type" @@ -201,22 +201,26 @@ ActiveRecord::Schema.define(version: 20200721135726) do t.string "promo_image_content_type" t.integer "promo_image_file_size" t.datetime "promo_image_updated_at" - t.boolean "visible", default: true + t.boolean "visible", default: true t.string "facebook" t.string "instagram" t.string "linkedin" - t.integer "owner_id", null: false - t.string "sells", default: "none", null: false - t.boolean "producer_profile_only", default: false - t.string "permalink", null: false - t.boolean "charges_sales_tax", default: false, null: false + t.integer "owner_id", null: false + t.string "sells", default: "none", null: false + t.boolean "producer_profile_only", default: false + t.string "permalink", null: false + t.boolean "charges_sales_tax", default: false, null: false t.string "email_address" - t.boolean "require_login", default: false, null: false - t.boolean "allow_guest_orders", default: true, null: false + t.boolean "require_login", default: false, null: false + t.boolean "allow_guest_orders", default: true, null: false t.text "invoice_text" - t.boolean "display_invoice_logo", default: false - t.boolean "allow_order_changes", default: false, null: false - t.boolean "enable_subscriptions", default: false, null: false + t.boolean "display_invoice_logo", default: false + t.boolean "allow_order_changes", default: false, null: false + t.boolean "enable_subscriptions", default: false, null: false + t.string "terms_and_conditions_file_name" + t.string "terms_and_conditions_content_type" + t.integer "terms_and_conditions_file_size" + t.datetime "terms_and_conditions_updated_at" end add_index "enterprises", ["address_id"], name: "index_enterprises_on_address_id", using: :btree diff --git a/spec/features/admin/enterprises/images_spec.rb b/spec/features/admin/enterprises/images_spec.rb index 0d4644f8c7..4dafe3df17 100644 --- a/spec/features/admin/enterprises/images_spec.rb +++ b/spec/features/admin/enterprises/images_spec.rb @@ -15,7 +15,7 @@ feature "Managing enterprise images" do visit edit_admin_enterprise_path(distributor) end - describe "images for an enterprise", js: true do + describe "images for an enterprise" do def go_to_images within(".side_menu") do click_link "Images" diff --git a/spec/features/admin/enterprises/terms_and_conditions_spec.rb b/spec/features/admin/enterprises/terms_and_conditions_spec.rb new file mode 100644 index 0000000000..93c448ef6c --- /dev/null +++ b/spec/features/admin/enterprises/terms_and_conditions_spec.rb @@ -0,0 +1,61 @@ +require "spec_helper" + +feature "Uploading Terms and Conditions PDF" do + include WebHelper + include AuthenticationHelper + + context "as an Enterprise user", js: true do + let(:enterprise_user) { create(:user, enterprise_limit: 1) } + let(:distributor) { create(:distributor_enterprise, name: "First Distributor") } + + before do + enterprise_user.enterprise_roles.build(enterprise: distributor).save! + + login_as enterprise_user + visit edit_admin_enterprise_path(distributor) + end + + describe "images for an enterprise" do + def go_to_business_details + within(".side_menu") do + click_link "Business Details" + end + end + + let(:white_pdf_file_name) { Rails.root.join("app", "assets", "images", "logo-white.pdf") } + let(:black_pdf_file_name) { Rails.root.join("app", "assets", "images", "logo-black.pdf") } + + before do + # Create fake PDFs from PNG images + FileUtils.cp(Rails.root.join("app", "assets", "images", "logo-white.png"), white_pdf_file_name) + FileUtils.cp(Rails.root.join("app", "assets", "images", "logo-black.png"), black_pdf_file_name) + + go_to_business_details + end + + scenario "uploading terms and conditions" do + # Add PDF + attach_file "enterprise[terms_and_conditions]", white_pdf_file_name + click_button "Update" + expect(page).to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") + + go_to_business_details + expect(page).to have_selector("a[href*='logo-white.pdf']") + + # Replace PDF + attach_file "enterprise[terms_and_conditions]", black_pdf_file_name + click_button "Update" + expect(page).to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") + + go_to_business_details + expect(page).to have_selector("a[href*='logo-black.pdf']") + end + + after do + # Delete fake PDFs + FileUtils.rm_f(white_pdf_file_name) + FileUtils.rm_f(black_pdf_file_name) + end + end + end +end From 16a475d8afe4a349a083d4e44b7b6d77e0944dda Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 17 Aug 2020 18:22:19 +0100 Subject: [PATCH 02/28] Fix some rubocop issues --- .../features/admin/enterprises/images_spec.rb | 2 ++ .../enterprises/terms_and_conditions_spec.rb | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/spec/features/admin/enterprises/images_spec.rb b/spec/features/admin/enterprises/images_spec.rb index 4dafe3df17..4716ba2ded 100644 --- a/spec/features/admin/enterprises/images_spec.rb +++ b/spec/features/admin/enterprises/images_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "spec_helper" feature "Managing enterprise images" do diff --git a/spec/features/admin/enterprises/terms_and_conditions_spec.rb b/spec/features/admin/enterprises/terms_and_conditions_spec.rb index 93c448ef6c..d13893c0dc 100644 --- a/spec/features/admin/enterprises/terms_and_conditions_spec.rb +++ b/spec/features/admin/enterprises/terms_and_conditions_spec.rb @@ -1,7 +1,8 @@ +# frozen_string_literal: true + require "spec_helper" feature "Uploading Terms and Conditions PDF" do - include WebHelper include AuthenticationHelper context "as an Enterprise user", js: true do @@ -22,14 +23,14 @@ feature "Uploading Terms and Conditions PDF" do end end - let(:white_pdf_file_name) { Rails.root.join("app", "assets", "images", "logo-white.pdf") } - let(:black_pdf_file_name) { Rails.root.join("app", "assets", "images", "logo-black.pdf") } + let(:white_pdf_file_name) { Rails.root.join("app/assets/images/logo-white.pdf") } + let(:black_pdf_file_name) { Rails.root.join("app/assets/images/logo-black.pdf") } before do # Create fake PDFs from PNG images - FileUtils.cp(Rails.root.join("app", "assets", "images", "logo-white.png"), white_pdf_file_name) - FileUtils.cp(Rails.root.join("app", "assets", "images", "logo-black.png"), black_pdf_file_name) - + FileUtils.cp(Rails.root.join("app/assets/images/logo-white.png"), white_pdf_file_name) + FileUtils.cp(Rails.root.join("app/assets/images/logo-black.png"), black_pdf_file_name) + go_to_business_details end @@ -37,7 +38,8 @@ feature "Uploading Terms and Conditions PDF" do # Add PDF attach_file "enterprise[terms_and_conditions]", white_pdf_file_name click_button "Update" - expect(page).to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") + expect(page). + to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") go_to_business_details expect(page).to have_selector("a[href*='logo-white.pdf']") @@ -45,7 +47,8 @@ feature "Uploading Terms and Conditions PDF" do # Replace PDF attach_file "enterprise[terms_and_conditions]", black_pdf_file_name click_button "Update" - expect(page).to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") + expect(page). + to have_content("Enterprise \"#{distributor.name}\" has been successfully updated!") go_to_business_details expect(page).to have_selector("a[href*='logo-black.pdf']") From b9511d4f0736110874048f7bc1cd496fb94d3882 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 17 Aug 2020 22:27:49 +0100 Subject: [PATCH 03/28] Show terms and conditions on checkout if enterprise has an associated PDF file --- app/views/checkout/_form.html.haml | 1 + app/views/checkout/_terms_and_conditions.html.haml | 2 ++ config/locales/en.yml | 3 +++ 3 files changed, 6 insertions(+) create mode 100644 app/views/checkout/_terms_and_conditions.html.haml diff --git a/app/views/checkout/_form.html.haml b/app/views/checkout/_form.html.haml index 9a705ef4b5..f341a666d9 100644 --- a/app/views/checkout/_form.html.haml +++ b/app/views/checkout/_form.html.haml @@ -14,6 +14,7 @@ = render "checkout/shipping", f: f = render "checkout/payment", f: f = render "checkout/already_ordered", f: f if show_bought_items? + = render "checkout/terms_and_conditions", f: f %p %button.button.primary{type: :submit} = t :checkout_send diff --git a/app/views/checkout/_terms_and_conditions.html.haml b/app/views/checkout/_terms_and_conditions.html.haml new file mode 100644 index 0000000000..6811a738ae --- /dev/null +++ b/app/views/checkout/_terms_and_conditions.html.haml @@ -0,0 +1,2 @@ +%p + = t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions.url, target: '_blank')) if current_order.distributor.terms_and_conditions.present? diff --git a/config/locales/en.yml b/config/locales/en.yml index 8867d8f6f7..c90755e1d5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1224,6 +1224,9 @@ en: already_ordered: cart: "cart" message_html: "You have an order for this order cycle already. Check the %{cart} to see the items you ordered before. You can also cancel items as long as the order cycle is open." + terms_and_conditions: + message_html: "By placing this order you agree to the %{terms_and_conditions_link}" + link_text: "Terms of Service" failed: "The checkout failed. Please let us know so that we can process your order." shops: hubs: From 785f8ada4d72268cdccd4dc27caa1ebf88dd1668 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:07:46 +0100 Subject: [PATCH 04/28] Refactor checkout_spec by removing unnecessary initial describe section --- .../consumer/shopping/checkout_spec.rb | 718 +++++++++--------- 1 file changed, 357 insertions(+), 361 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index dd79d95958..26b10eb617 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -16,6 +16,19 @@ feature "As a consumer I want to check out my cart", js: true do let(:variant) { product.variants.first } let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor, bill_address_id: nil, ship_address_id: nil) } + let(:free_shipping) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow", calculator: Calculator::FlatRate.new(preferred_amount: 0.00)) } + let(:shipping_with_fee) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue", calculator: Calculator::FlatRate.new(preferred_amount: 4.56)) } + let(:tagged_shipping) { create(:shipping_method, require_ship_address: false, name: "Local", tag_list: "local") } + let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") } + let!(:check_with_fee) { create(:payment_method, distributors: [distributor], calculator: Calculator::FlatRate.new(preferred_amount: 5.67)) } + let!(:paypal) do + Spree::Gateway::PayPalExpress.create!(name: "Paypal", environment: 'test', distributor_ids: [distributor.id]).tap do |pm| + pm.preferred_login = 'devnull-facilitator_api1.rohanmitchell.com' + pm.preferred_password = '1406163716' + pm.preferred_signature = 'AFcWxV21C7fd0v3bYYYRCpSSRl31AaTntNJ-AjvUJkWf4dgJIvcLsf1V' + end + end + before do Spree::Config.shipment_inc_vat = true Spree::Config.shipping_tax_rate = 0.25 @@ -23,61 +36,70 @@ feature "As a consumer I want to check out my cart", js: true do add_enterprise_fee enterprise_fee set_order order add_product_to_cart order, product + + distributor.shipping_methods << free_shipping + distributor.shipping_methods << shipping_with_fee + distributor.shipping_methods << tagged_shipping end - describe "with shipping and payment methods" do - let(:free_shipping) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow", calculator: Calculator::FlatRate.new(preferred_amount: 0.00)) } - let(:shipping_with_fee) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue", calculator: Calculator::FlatRate.new(preferred_amount: 4.56)) } - let(:tagged_shipping) { create(:shipping_method, require_ship_address: false, name: "Local", tag_list: "local") } - let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") } - let!(:check_with_fee) { create(:payment_method, distributors: [distributor], calculator: Calculator::FlatRate.new(preferred_amount: 5.67)) } - let!(:paypal) do - Spree::Gateway::PayPalExpress.create!(name: "Paypal", environment: 'test', distributor_ids: [distributor.id]).tap do |pm| - pm.preferred_login = 'devnull-facilitator_api1.rohanmitchell.com' - pm.preferred_password = '1406163716' - pm.preferred_signature = 'AFcWxV21C7fd0v3bYYYRCpSSRl31AaTntNJ-AjvUJkWf4dgJIvcLsf1V' - end + describe "when I have an out of stock product in my cart" do + before do + variant.on_demand = false + variant.on_hand = 0 + variant.save! end + it "returns me to the cart with an error message" do + visit checkout_path + + expect(page).not_to have_selector 'closing', text: "Checkout now" + expect(page).to have_selector 'closing', text: "Your shopping cart" + expect(page).to have_content "An item in your cart has become unavailable" + end + end + + context 'login in as user' do + let(:user) { create(:user) } + before do - distributor.shipping_methods << free_shipping - distributor.shipping_methods << shipping_with_fee - distributor.shipping_methods << tagged_shipping + login_as(user) end - describe "when I have an out of stock product in my cart" do + context "with details filled out" do before do - variant.on_demand = false - variant.on_hand = 0 - variant.save! - end - - it "returns me to the cart with an error message" do visit checkout_path - - expect(page).not_to have_selector 'closing', text: "Checkout now" - expect(page).to have_selector 'closing', text: "Your shopping cart" - expect(page).to have_content "An item in your cart has become unavailable" - end - end - - context 'login in as user' do - let(:user) { create(:user) } - - before do - login_as(user) + fill_out_form end - context "with details filled out" do + it "creates a new default billing address and shipping address" do + expect(user.bill_address).to be_nil + expect(user.ship_address).to be_nil + + expect(order.bill_address).to be_nil + expect(order.ship_address).to be_nil + + place_order + expect(page).to have_content "Your order has been processed successfully" + + expect(order.reload.bill_address.address1).to eq '123 Your Head' + expect(order.reload.ship_address.address1).to eq '123 Your Head' + + expect(order.customer.bill_address.address1).to eq '123 Your Head' + expect(order.customer.ship_address.address1).to eq '123 Your Head' + + expect(user.reload.bill_address.address1).to eq '123 Your Head' + expect(user.reload.ship_address.address1).to eq '123 Your Head' + end + + context "when the user and customer have existing default addresses" do + let(:existing_address) { create(:address) } + before do - visit checkout_path - fill_out_form + user.bill_address = existing_address + user.ship_address = existing_address end - it "creates a new default billing address and shipping address" do - expect(user.bill_address).to be_nil - expect(user.ship_address).to be_nil - + it "updates billing address and shipping address" do expect(order.bill_address).to be_nil expect(order.ship_address).to be_nil @@ -93,365 +115,339 @@ feature "As a consumer I want to check out my cart", js: true do expect(user.reload.bill_address.address1).to eq '123 Your Head' expect(user.reload.ship_address.address1).to eq '123 Your Head' end - - context "when the user and customer have existing default addresses" do - let(:existing_address) { create(:address) } - - before do - user.bill_address = existing_address - user.ship_address = existing_address - end - - it "updates billing address and shipping address" do - expect(order.bill_address).to be_nil - expect(order.ship_address).to be_nil - - place_order - expect(page).to have_content "Your order has been processed successfully" - - expect(order.reload.bill_address.address1).to eq '123 Your Head' - expect(order.reload.ship_address.address1).to eq '123 Your Head' - - expect(order.customer.bill_address.address1).to eq '123 Your Head' - expect(order.customer.ship_address.address1).to eq '123 Your Head' - - expect(user.reload.bill_address.address1).to eq '123 Your Head' - expect(user.reload.ship_address.address1).to eq '123 Your Head' - end - end - - it "it doesn't tell about previous orders" do - expect(page).to have_no_content("You have an order for this order cycle already.") - end end - context "with previous orders" do - let!(:prev_order) { create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor, user: order.user) } + it "it doesn't tell about previous orders" do + expect(page).to have_no_content("You have an order for this order cycle already.") + end + end - before do - order.distributor.allow_order_changes = true - order.distributor.save - visit checkout_path - end + context "with previous orders" do + let!(:prev_order) { create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor, user: order.user) } - it "informs about previous orders" do - expect(page).to have_content("You have an order for this order cycle already.") - end + before do + order.distributor.allow_order_changes = true + order.distributor.save + visit checkout_path end - context "when the user has a preset shipping and billing address" do - before do - user.bill_address = build(:address) - user.ship_address = build(:address) - user.save! - end + it "informs about previous orders" do + expect(page).to have_content("You have an order for this order cycle already.") + end + end - it "checks out successfully" do - visit checkout_path + context "when the user has a preset shipping and billing address" do + before do + user.bill_address = build(:address) + user.ship_address = build(:address) + user.save! + end + + it "checks out successfully" do + visit checkout_path + choose shipping_with_fee.name + choose check_without_fee.name + + expect do + place_order + expect(page).to have_content "Your order has been processed successfully" + end.to enqueue_job ConfirmOrderJob + end + end + + context "with Stripe" do + let!(:stripe_pm) do + create(:stripe_payment_method, distributors: [distributor]) + end + + let!(:saved_card) do + create(:credit_card, + user_id: user.id, + month: "01", + year: "2025", + cc_type: "visa", + number: "1111111111111111", + payment_method_id: stripe_pm.id, + gateway_customer_profile_id: "i_am_saved") + end + + let!(:stripe_account) { create(:stripe_account, enterprise_id: distributor.id, stripe_user_id: 'some_id') } + + let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } + + before do + allow(Stripe).to receive(:api_key) { "sk_test_12345" } + allow(Stripe).to receive(:publishable_key) { "some_key" } + Spree::Config.set(stripe_connect_enabled: true) + stub_request(:post, "https://api.stripe.com/v1/charges") + .with(basic_auth: ["sk_test_12345", ""]) + .to_return(status: 200, body: JSON.generate(response_mock)) + + visit checkout_path + fill_out_form + choose stripe_pm.name + end + + it "allows use of a saved card" do + # shows the saved credit card dropdown + expect(page).to have_content I18n.t("spree.checkout.payment.stripe.used_saved_card") + + # default card is selected, form element is not shown + expect(page).to have_no_selector "#card-element.StripeElement" + expect(page).to have_select 'selected_card', selected: "Visa x-1111 Exp:01/2025" + + # allows checkout + place_order + expect(page).to have_content "Your order has been processed successfully" + end + end + end + + context "on the checkout page" do + before do + visit checkout_path + checkout_as_guest + end + + it "shows the current distributor" do + visit checkout_path + expect(page).to have_content distributor.name + end + + it 'does not show the save as default address checkbox' do + expect(page).not_to have_content "Save as default billing address" + expect(page).not_to have_content "Save as default shipping address" + end + + it "shows a breakdown of the order price" do + choose shipping_with_fee.name + + expect(page).to have_selector 'orderdetails .cart-total', text: with_currency(11.23) + expect(page).to have_selector 'orderdetails .shipping', text: with_currency(4.56) + expect(page).to have_selector 'orderdetails .total', text: with_currency(15.79) + + # Tax should not be displayed in checkout, as the customer's choice of shipping method + # affects the tax and we haven't written code to live-update the tax amount when they + # make a change. + expect(page).not_to have_content product.tax_category.name + end + + it "shows all shipping methods in order by name" do + within '#shipping' do + expect(page).to have_selector "label", count: 4 # Three shipping methods + instructions label + labels = page.all('label').map(&:text) + expect(labels[0]).to start_with("Donkeys") # shipping_with_fee + expect(labels[1]).to start_with("Frogs") # free_shipping + expect(labels[2]).to start_with("Local") # tagged_shipping + end + end + + context "when shipping method requires an address" do + before do + choose free_shipping.name + end + it "shows ship address forms when 'same as billing address' is unchecked" do + uncheck "Shipping address same as billing address?" + expect(find("#ship_address > div.visible").visible?).to be true + end + end + + it "filters out 'Back office only' shipping methods" do + expect(page).to have_content shipping_with_fee.name + shipping_with_fee.update_attribute :display_on, 'back_end' # Back office only + + visit checkout_path + checkout_as_guest + expect(page).not_to have_content shipping_with_fee.name + end + + context "using FilterShippingMethods" do + let(:user) { create(:user) } + let(:customer) { create(:customer, user: user, enterprise: distributor) } + + it "shows shipping methods allowed by the rule" do + # No rules in effect + expect(page).to have_content "Frogs" + expect(page).to have_content "Donkeys" + expect(page).to have_content "Local" + + create(:filter_shipping_methods_tag_rule, + enterprise: distributor, + preferred_customer_tags: "local", + preferred_shipping_method_tags: "local", + preferred_matched_shipping_methods_visibility: 'visible') + create(:filter_shipping_methods_tag_rule, + enterprise: distributor, + is_default: true, + preferred_shipping_method_tags: "local", + preferred_matched_shipping_methods_visibility: 'hidden') + visit checkout_path + checkout_as_guest + + # Default rule in effect, disallows access to 'Local' + expect(page).to have_content "Frogs" + expect(page).to have_content "Donkeys" + expect(page).not_to have_content "Local" + + login_as(user) + visit checkout_path + + # Default rule in still effect, disallows access to 'Local' + expect(page).to have_content "Frogs" + expect(page).to have_content "Donkeys" + expect(page).not_to have_content "Local" + + customer.update_attribute(:tag_list, "local") + visit checkout_path + + # #local Customer can access 'Local' shipping method + expect(page).to have_content "Frogs" + expect(page).to have_content "Donkeys" + expect(page).to have_content "Local" + end + end + end + + context "on the checkout page with payments open" do + before do + visit checkout_path + checkout_as_guest + end + + it "shows all available payment methods" do + expect(page).to have_content check_without_fee.name + expect(page).to have_content check_with_fee.name + expect(page).to have_content paypal.name + end + + describe "purchasing" do + it "takes us to the order confirmation page when we submit a complete form" do + fill_out_details + fill_out_billing_address + + within "#shipping" do choose shipping_with_fee.name - choose check_without_fee.name - - expect do - place_order - expect(page).to have_content "Your order has been processed successfully" - end.to enqueue_job ConfirmOrderJob + fill_in 'Any comments or special instructions?', with: "SpEcIaL NoTeS" end + + within "#payment" do + choose check_without_fee.name + end + + expect do + place_order + expect(page).to have_content "Your order has been processed successfully" + end.to enqueue_job ConfirmOrderJob + + # And the order's special instructions should be set + order = Spree::Order.complete.first + expect(order.special_instructions).to eq "SpEcIaL NoTeS" + + # And the Spree tax summary should not be displayed + expect(page).not_to have_content product.tax_category.name + + # And the total tax for the order, including shipping and fee tax, should be displayed + # product tax ($10.00 @ 10% = $0.91) + # + fee tax ($ 1.23 @ 10% = $0.11) + # + shipping tax ($ 4.56 @ 25% = $0.91) + # = $1.93 + expect(page).to have_content '(includes tax)' + expect(page).to have_content with_currency(1.93) + expect(page).to have_content 'Back To Store' end - context "with Stripe" do - let!(:stripe_pm) do - create(:stripe_payment_method, distributors: [distributor]) - end - - let!(:saved_card) do - create(:credit_card, - user_id: user.id, - month: "01", - year: "2025", - cc_type: "visa", - number: "1111111111111111", - payment_method_id: stripe_pm.id, - gateway_customer_profile_id: "i_am_saved") - end - - let!(:stripe_account) { create(:stripe_account, enterprise_id: distributor.id, stripe_user_id: 'some_id') } - - let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } - + context "with basic details filled" do before do - allow(Stripe).to receive(:api_key) { "sk_test_12345" } - allow(Stripe).to receive(:publishable_key) { "some_key" } - Spree::Config.set(stripe_connect_enabled: true) - stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""]) - .to_return(status: 200, body: JSON.generate(response_mock)) - - visit checkout_path - fill_out_form - choose stripe_pm.name + choose free_shipping.name + choose check_without_fee.name + fill_out_details + fill_out_billing_address + check "Shipping address same as billing address?" end - it "allows use of a saved card" do - # shows the saved credit card dropdown - expect(page).to have_content I18n.t("spree.checkout.payment.stripe.used_saved_card") - - # default card is selected, form element is not shown - expect(page).to have_no_selector "#card-element.StripeElement" - expect(page).to have_select 'selected_card', selected: "Visa x-1111 Exp:01/2025" - - # allows checkout + it "takes us to the order confirmation page when submitted with 'same as billing address' checked" do place_order expect(page).to have_content "Your order has been processed successfully" end - end - end - context "on the checkout page" do - before do - visit checkout_path - checkout_as_guest - end + it "takes us to the cart page with an error when a product becomes out of stock just before we purchase", js: true do + variant.on_demand = false + variant.on_hand = 0 + variant.save! - it "shows the current distributor" do - visit checkout_path - expect(page).to have_content distributor.name - end + place_order - it 'does not show the save as default address checkbox' do - expect(page).not_to have_content "Save as default billing address" - expect(page).not_to have_content "Save as default shipping address" - end - - it "shows a breakdown of the order price" do - choose shipping_with_fee.name - - expect(page).to have_selector 'orderdetails .cart-total', text: with_currency(11.23) - expect(page).to have_selector 'orderdetails .shipping', text: with_currency(4.56) - expect(page).to have_selector 'orderdetails .total', text: with_currency(15.79) - - # Tax should not be displayed in checkout, as the customer's choice of shipping method - # affects the tax and we haven't written code to live-update the tax amount when they - # make a change. - expect(page).not_to have_content product.tax_category.name - end - - it "shows all shipping methods in order by name" do - within '#shipping' do - expect(page).to have_selector "label", count: 4 # Three shipping methods + instructions label - labels = page.all('label').map(&:text) - expect(labels[0]).to start_with("Donkeys") # shipping_with_fee - expect(labels[1]).to start_with("Frogs") # free_shipping - expect(labels[2]).to start_with("Local") # tagged_shipping + expect(page).not_to have_content "Your order has been processed successfully" + expect(page).to have_selector 'closing', text: "Your shopping cart" + expect(page).to have_content "An item in your cart has become unavailable." end - end - context "when shipping method requires an address" do - before do - choose free_shipping.name - end - it "shows ship address forms when 'same as billing address' is unchecked" do - uncheck "Shipping address same as billing address?" - expect(find("#ship_address > div.visible").visible?).to be true - end - end + context "when we are charged a shipping fee" do + before { choose shipping_with_fee.name } - it "filters out 'Back office only' shipping methods" do - expect(page).to have_content shipping_with_fee.name - shipping_with_fee.update_attribute :display_on, 'back_end' # Back office only - - visit checkout_path - checkout_as_guest - expect(page).not_to have_content shipping_with_fee.name - end - - context "using FilterShippingMethods" do - let(:user) { create(:user) } - let(:customer) { create(:customer, user: user, enterprise: distributor) } - - it "shows shipping methods allowed by the rule" do - # No rules in effect - expect(page).to have_content "Frogs" - expect(page).to have_content "Donkeys" - expect(page).to have_content "Local" - - create(:filter_shipping_methods_tag_rule, - enterprise: distributor, - preferred_customer_tags: "local", - preferred_shipping_method_tags: "local", - preferred_matched_shipping_methods_visibility: 'visible') - create(:filter_shipping_methods_tag_rule, - enterprise: distributor, - is_default: true, - preferred_shipping_method_tags: "local", - preferred_matched_shipping_methods_visibility: 'hidden') - visit checkout_path - checkout_as_guest - - # Default rule in effect, disallows access to 'Local' - expect(page).to have_content "Frogs" - expect(page).to have_content "Donkeys" - expect(page).not_to have_content "Local" - - login_as(user) - visit checkout_path - - # Default rule in still effect, disallows access to 'Local' - expect(page).to have_content "Frogs" - expect(page).to have_content "Donkeys" - expect(page).not_to have_content "Local" - - customer.update_attribute(:tag_list, "local") - visit checkout_path - - # #local Customer can access 'Local' shipping method - expect(page).to have_content "Frogs" - expect(page).to have_content "Donkeys" - expect(page).to have_content "Local" - end - end - end - - context "on the checkout page with payments open" do - before do - visit checkout_path - checkout_as_guest - end - - it "shows all available payment methods" do - expect(page).to have_content check_without_fee.name - expect(page).to have_content check_with_fee.name - expect(page).to have_content paypal.name - end - - describe "purchasing" do - it "takes us to the order confirmation page when we submit a complete form" do - fill_out_details - fill_out_billing_address - - within "#shipping" do - choose shipping_with_fee.name - fill_in 'Any comments or special instructions?', with: "SpEcIaL NoTeS" - end - - within "#payment" do - choose check_without_fee.name - end - - expect do + it "creates a payment for the full amount inclusive of shipping" do place_order expect(page).to have_content "Your order has been processed successfully" - end.to enqueue_job ConfirmOrderJob - # And the order's special instructions should be set - order = Spree::Order.complete.first - expect(order.special_instructions).to eq "SpEcIaL NoTeS" - - # And the Spree tax summary should not be displayed - expect(page).not_to have_content product.tax_category.name - - # And the total tax for the order, including shipping and fee tax, should be displayed - # product tax ($10.00 @ 10% = $0.91) - # + fee tax ($ 1.23 @ 10% = $0.11) - # + shipping tax ($ 4.56 @ 25% = $0.91) - # = $1.93 - expect(page).to have_content '(includes tax)' - expect(page).to have_content with_currency(1.93) - expect(page).to have_content 'Back To Store' + # There are two orders - our order and our new cart + o = Spree::Order.complete.first + expect(o.adjustments.shipping.first.amount).to eq(4.56) + expect(o.payments.first.amount).to eq(10 + 1.23 + 4.56) # items + fees + shipping + end end - context "with basic details filled" do - before do - choose free_shipping.name - choose check_without_fee.name - fill_out_details - fill_out_billing_address - check "Shipping address same as billing address?" - end + context "when we are charged a payment method fee (transaction fee)" do + it "creates a payment including the transaction fee" do + # Selecting the transaction fee, it is displayed + expect(page).to have_selector ".transaction-fee td", text: with_currency(0.00) + expect(page).to have_selector ".total", text: with_currency(11.23) + + choose "#{check_with_fee.name} (#{with_currency(5.67)})" + + expect(page).to have_selector ".transaction-fee td", text: with_currency(5.67) + expect(page).to have_selector ".total", text: with_currency(16.90) - it "takes us to the order confirmation page when submitted with 'same as billing address' checked" do place_order expect(page).to have_content "Your order has been processed successfully" + + # There are two orders - our order and our new cart + o = Spree::Order.complete.first + expect(o.adjustments.payment_fee.first.amount).to eq 5.67 + expect(o.payments.first.amount).to eq(10 + 1.23 + 5.67) # items + fees + transaction end + end - it "takes us to the cart page with an error when a product becomes out of stock just before we purchase", js: true do - variant.on_demand = false - variant.on_hand = 0 - variant.save! + describe "credit card payments" do + ["Spree::Gateway::Bogus", "Spree::Gateway::BogusSimple"].each do |gateway_type| + context "with a credit card payment method using #{gateway_type}" do + let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: gateway_type) } - place_order + it "takes us to the order confirmation page when submitted with a valid credit card" do + fill_in 'Card Number', with: "4111111111111111" + select 'February', from: 'secrets.card_month' + select (Date.current.year + 1).to_s, from: 'secrets.card_year' + fill_in 'Security Code', with: '123' - expect(page).not_to have_content "Your order has been processed successfully" - expect(page).to have_selector 'closing', text: "Your shopping cart" - expect(page).to have_content "An item in your cart has become unavailable." - end + place_order + expect(page).to have_content "Your order has been processed successfully" - context "when we are charged a shipping fee" do - before { choose shipping_with_fee.name } + # Order should have a payment with the correct amount + o = Spree::Order.complete.first + expect(o.payments.first.amount).to eq(11.23) + end - it "creates a payment for the full amount inclusive of shipping" do - place_order - expect(page).to have_content "Your order has been processed successfully" + it "shows the payment processing failed message when submitted with an invalid credit card" do + fill_in 'Card Number', with: "9999999988887777" + select 'February', from: 'secrets.card_month' + select (Date.current.year + 1).to_s, from: 'secrets.card_year' + fill_in 'Security Code', with: '123' - # There are two orders - our order and our new cart - o = Spree::Order.complete.first - expect(o.adjustments.shipping.first.amount).to eq(4.56) - expect(o.payments.first.amount).to eq(10 + 1.23 + 4.56) # items + fees + shipping - end - end + place_order + expect(page).to have_content 'Bogus Gateway: Forced failure' - context "when we are charged a payment method fee (transaction fee)" do - it "creates a payment including the transaction fee" do - # Selecting the transaction fee, it is displayed - expect(page).to have_selector ".transaction-fee td", text: with_currency(0.00) - expect(page).to have_selector ".total", text: with_currency(11.23) - - choose "#{check_with_fee.name} (#{with_currency(5.67)})" - - expect(page).to have_selector ".transaction-fee td", text: with_currency(5.67) - expect(page).to have_selector ".total", text: with_currency(16.90) - - place_order - expect(page).to have_content "Your order has been processed successfully" - - # There are two orders - our order and our new cart - o = Spree::Order.complete.first - expect(o.adjustments.payment_fee.first.amount).to eq 5.67 - expect(o.payments.first.amount).to eq(10 + 1.23 + 5.67) # items + fees + transaction - end - end - - describe "credit card payments" do - ["Spree::Gateway::Bogus", "Spree::Gateway::BogusSimple"].each do |gateway_type| - context "with a credit card payment method using #{gateway_type}" do - let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: gateway_type) } - - it "takes us to the order confirmation page when submitted with a valid credit card" do - fill_in 'Card Number', with: "4111111111111111" - select 'February', from: 'secrets.card_month' - select (Date.current.year + 1).to_s, from: 'secrets.card_year' - fill_in 'Security Code', with: '123' - - place_order - expect(page).to have_content "Your order has been processed successfully" - - # Order should have a payment with the correct amount - o = Spree::Order.complete.first - expect(o.payments.first.amount).to eq(11.23) - end - - it "shows the payment processing failed message when submitted with an invalid credit card" do - fill_in 'Card Number', with: "9999999988887777" - select 'February', from: 'secrets.card_month' - select (Date.current.year + 1).to_s, from: 'secrets.card_year' - fill_in 'Security Code', with: '123' - - place_order - expect(page).to have_content 'Bogus Gateway: Forced failure' - - # Does not show duplicate shipping fee - visit checkout_path - expect(page).to have_selector "th", text: "Shipping", count: 1 - end + # Does not show duplicate shipping fee + visit checkout_path + expect(page).to have_selector "th", text: "Shipping", count: 1 end end end From d1f5828d13277ca5077bb98c914de00035be5e02 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:11:25 +0100 Subject: [PATCH 05/28] Rename checkout_workflow to checkout_helper --- spec/features/consumer/shopping/checkout_auth_spec.rb | 2 +- spec/features/consumer/shopping/checkout_paypal_spec.rb | 2 +- spec/features/consumer/shopping/checkout_spec.rb | 2 +- spec/features/consumer/shopping/embedded_shopfronts_spec.rb | 2 +- spec/features/consumer/shopping/variant_overrides_spec.rb | 2 +- .../request/{checkout_workflow.rb => checkout_helper.rb} | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename spec/support/request/{checkout_workflow.rb => checkout_helper.rb} (93%) diff --git a/spec/features/consumer/shopping/checkout_auth_spec.rb b/spec/features/consumer/shopping/checkout_auth_spec.rb index acbbabd970..a6986467e8 100644 --- a/spec/features/consumer/shopping/checkout_auth_spec.rb +++ b/spec/features/consumer/shopping/checkout_auth_spec.rb @@ -4,7 +4,7 @@ feature "As a consumer I want to check out my cart", js: true do include AuthenticationHelper include WebHelper include ShopWorkflow - include CheckoutWorkflow + include CheckoutHelper include UIComponentHelper describe "using the checkout" do diff --git a/spec/features/consumer/shopping/checkout_paypal_spec.rb b/spec/features/consumer/shopping/checkout_paypal_spec.rb index c32225dac7..244da29fa2 100644 --- a/spec/features/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/features/consumer/shopping/checkout_paypal_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" feature "Checking out with Paypal", js: true do include ShopWorkflow - include CheckoutWorkflow + include CheckoutHelper let(:distributor) { create(:distributor_enterprise) } let(:supplier) { create(:supplier_enterprise) } diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 26b10eb617..275bcb4105 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' feature "As a consumer I want to check out my cart", js: true do include AuthenticationHelper include ShopWorkflow - include CheckoutWorkflow + include CheckoutHelper include WebHelper include UIComponentHelper diff --git a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb index 075209769e..e587801540 100644 --- a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb +++ b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb @@ -5,7 +5,7 @@ feature "Using embedded shopfront functionality", js: true do include AuthenticationHelper include WebHelper include ShopWorkflow - include CheckoutWorkflow + include CheckoutHelper include UIComponentHelper describe "using iframes" do diff --git a/spec/features/consumer/shopping/variant_overrides_spec.rb b/spec/features/consumer/shopping/variant_overrides_spec.rb index d89990dc55..3fffbe61bc 100644 --- a/spec/features/consumer/shopping/variant_overrides_spec.rb +++ b/spec/features/consumer/shopping/variant_overrides_spec.rb @@ -4,7 +4,7 @@ feature "shopping with variant overrides defined", js: true do include AuthenticationHelper include WebHelper include ShopWorkflow - include CheckoutWorkflow + include CheckoutHelper include UIComponentHelper let(:hub) { create(:distributor_enterprise, with_payment_and_shipping: true) } diff --git a/spec/support/request/checkout_workflow.rb b/spec/support/request/checkout_helper.rb similarity index 93% rename from spec/support/request/checkout_workflow.rb rename to spec/support/request/checkout_helper.rb index c6863a1bf5..45204f13a1 100644 --- a/spec/support/request/checkout_workflow.rb +++ b/spec/support/request/checkout_helper.rb @@ -1,4 +1,4 @@ -module CheckoutWorkflow +module CheckoutHelper def have_checkout_details have_content "Your details" end From edfd0fd95c0e0ada8eaea46ecd40d1c098e57e58 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:12:59 +0100 Subject: [PATCH 06/28] Move checkout helpers to checkout_helper --- .../consumer/shopping/checkout_spec.rb | 32 ------------------- spec/support/request/checkout_helper.rb | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 275bcb4105..31a11b8d1d 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -455,36 +455,4 @@ feature "As a consumer I want to check out my cart", js: true do end end end - - def fill_out_details - within "#details" do - fill_in "First Name", with: "Will" - fill_in "Last Name", with: "Marshall" - fill_in "Email", with: "test@test.com" - fill_in "Phone", with: "0468363090" - end - end - - def fill_out_billing_address - within "#billing" do - fill_in "City", with: "Melbourne" - fill_in "Postcode", with: "3066" - fill_in "Address", with: "123 Your Head" - select "Australia", from: "Country" - select "Victoria", from: "State" - end - end - - def fill_out_form - choose free_shipping.name - choose check_without_fee.name - - fill_out_details - check "Save as default billing address" - - fill_out_billing_address - - check "Shipping address same as billing address?" - check "Save as default shipping address" - end end diff --git a/spec/support/request/checkout_helper.rb b/spec/support/request/checkout_helper.rb index 45204f13a1..d6c2a70d91 100644 --- a/spec/support/request/checkout_helper.rb +++ b/spec/support/request/checkout_helper.rb @@ -18,4 +18,36 @@ module CheckoutHelper def toggle_details toggle_accordion :details end + + def fill_out_details + within "#details" do + fill_in "First Name", with: "Will" + fill_in "Last Name", with: "Marshall" + fill_in "Email", with: "test@test.com" + fill_in "Phone", with: "0468363090" + end + end + + def fill_out_billing_address + within "#billing" do + fill_in "City", with: "Melbourne" + fill_in "Postcode", with: "3066" + fill_in "Address", with: "123 Your Head" + select "Australia", from: "Country" + select "Victoria", from: "State" + end + end + + def fill_out_form + choose free_shipping.name + choose check_without_fee.name + + fill_out_details + check "Save as default billing address" + + fill_out_billing_address + + check "Shipping address same as billing address?" + check "Save as default shipping address" + end end From 86ad31eb5c3eb59f11b1e84fe5e6c7507957ee7a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:23:19 +0100 Subject: [PATCH 07/28] Reuse checkout form filling code from CheckoutHelper in checkout paypal spec --- .../consumer/shopping/checkout_paypal_spec.rb | 29 ++----------------- .../consumer/shopping/checkout_spec.rb | 5 ++-- spec/support/request/checkout_helper.rb | 10 +++---- 3 files changed, 9 insertions(+), 35 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_paypal_spec.rb b/spec/features/consumer/shopping/checkout_paypal_spec.rb index 244da29fa2..ad21e46f54 100644 --- a/spec/features/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/features/consumer/shopping/checkout_paypal_spec.rb @@ -44,7 +44,8 @@ feature "Checking out with Paypal", js: true do describe "as a guest" do it "fails with an error message" do visit checkout_path - complete_the_form + checkout_as_guest + fill_out_form(free_shipping.name, paypal.name, false) paypal_response = double(:response, success?: false, errors: []) paypal_provider = double( @@ -59,30 +60,4 @@ feature "Checking out with Paypal", js: true do expect(page).to have_content "PayPal failed." end end - - def complete_the_form - checkout_as_guest - - within "#details" do - fill_in "First Name", with: "Will" - fill_in "Last Name", with: "Marshall" - fill_in "Email", with: "test@test.com" - fill_in "Phone", with: "0468363090" - end - - within "#billing" do - fill_in "City", with: "Melbourne" - fill_in "Postcode", with: "3066" - fill_in "Address", with: "123 Your Head" - select "Australia", from: "Country" - select "Victoria", from: "State" - end - - within "#shipping" do - choose free_shipping.name - end - within "#payment" do - choose paypal.name - end - end end diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 31a11b8d1d..074e6a66df 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -68,7 +68,7 @@ feature "As a consumer I want to check out my cart", js: true do context "with details filled out" do before do visit checkout_path - fill_out_form + fill_out_form(free_shipping.name, check_without_fee.name) end it "creates a new default billing address and shipping address" do @@ -184,8 +184,7 @@ feature "As a consumer I want to check out my cart", js: true do .to_return(status: 200, body: JSON.generate(response_mock)) visit checkout_path - fill_out_form - choose stripe_pm.name + fill_out_form(free_shipping.name, stripe_pm.name) end it "allows use of a saved card" do diff --git a/spec/support/request/checkout_helper.rb b/spec/support/request/checkout_helper.rb index d6c2a70d91..d3c92d1608 100644 --- a/spec/support/request/checkout_helper.rb +++ b/spec/support/request/checkout_helper.rb @@ -38,16 +38,16 @@ module CheckoutHelper end end - def fill_out_form - choose free_shipping.name - choose check_without_fee.name + def fill_out_form(shipping_method_name, payment_method_name, save_default_addresses = false) + choose shipping_method_name + choose payment_method_name fill_out_details - check "Save as default billing address" + check "Save as default billing address" if save_default_addresses fill_out_billing_address check "Shipping address same as billing address?" - check "Save as default shipping address" + check "Save as default shipping address" if save_default_addresses end end From 70e9ef93bb127589a5af35c232171ef44771d729 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:25:31 +0100 Subject: [PATCH 08/28] Extract stripe spec from checkout spec so we can expand stripe tests in checkout --- .../consumer/shopping/checkout_spec.rb | 46 --------- .../consumer/shopping/checkout_stripe_spec.rb | 98 +++++++++++++++++++ 2 files changed, 98 insertions(+), 46 deletions(-) create mode 100644 spec/features/consumer/shopping/checkout_stripe_spec.rb diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 074e6a66df..a7afc3b9a6 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -154,52 +154,6 @@ feature "As a consumer I want to check out my cart", js: true do end.to enqueue_job ConfirmOrderJob end end - - context "with Stripe" do - let!(:stripe_pm) do - create(:stripe_payment_method, distributors: [distributor]) - end - - let!(:saved_card) do - create(:credit_card, - user_id: user.id, - month: "01", - year: "2025", - cc_type: "visa", - number: "1111111111111111", - payment_method_id: stripe_pm.id, - gateway_customer_profile_id: "i_am_saved") - end - - let!(:stripe_account) { create(:stripe_account, enterprise_id: distributor.id, stripe_user_id: 'some_id') } - - let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } - - before do - allow(Stripe).to receive(:api_key) { "sk_test_12345" } - allow(Stripe).to receive(:publishable_key) { "some_key" } - Spree::Config.set(stripe_connect_enabled: true) - stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""]) - .to_return(status: 200, body: JSON.generate(response_mock)) - - visit checkout_path - fill_out_form(free_shipping.name, stripe_pm.name) - end - - it "allows use of a saved card" do - # shows the saved credit card dropdown - expect(page).to have_content I18n.t("spree.checkout.payment.stripe.used_saved_card") - - # default card is selected, form element is not shown - expect(page).to have_no_selector "#card-element.StripeElement" - expect(page).to have_select 'selected_card', selected: "Visa x-1111 Exp:01/2025" - - # allows checkout - place_order - expect(page).to have_content "Your order has been processed successfully" - end - end end context "on the checkout page" do diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb new file mode 100644 index 0000000000..fe866a76df --- /dev/null +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +feature "As a consumer I want to check out my cart", js: true do + include AuthenticationHelper + include ShopWorkflow + include CheckoutHelper + include WebHelper + include UIComponentHelper + + let!(:zone) { create(:zone_with_member) } + let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true) } + let(:supplier) { create(:supplier_enterprise) } + let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [variant]) } + let(:enterprise_fee) { create(:enterprise_fee, amount: 1.23, tax_category: product.tax_category) } + let(:product) { create(:taxed_product, supplier: supplier, price: 10, zone: zone, tax_rate_amount: 0.1) } + let(:variant) { product.variants.first } + let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor, bill_address_id: nil, ship_address_id: nil) } + + let(:free_shipping) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow", calculator: Calculator::FlatRate.new(preferred_amount: 0.00)) } + let(:shipping_with_fee) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue", calculator: Calculator::FlatRate.new(preferred_amount: 4.56)) } + let(:tagged_shipping) { create(:shipping_method, require_ship_address: false, name: "Local", tag_list: "local") } + let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") } + let!(:check_with_fee) { create(:payment_method, distributors: [distributor], calculator: Calculator::FlatRate.new(preferred_amount: 5.67)) } + let!(:paypal) do + Spree::Gateway::PayPalExpress.create!(name: "Paypal", environment: 'test', distributor_ids: [distributor.id]).tap do |pm| + pm.preferred_login = 'devnull-facilitator_api1.rohanmitchell.com' + pm.preferred_password = '1406163716' + pm.preferred_signature = 'AFcWxV21C7fd0v3bYYYRCpSSRl31AaTntNJ-AjvUJkWf4dgJIvcLsf1V' + end + end + + before do + Spree::Config.shipment_inc_vat = true + Spree::Config.shipping_tax_rate = 0.25 + + add_enterprise_fee enterprise_fee + set_order order + add_product_to_cart order, product + + distributor.shipping_methods << free_shipping + distributor.shipping_methods << shipping_with_fee + distributor.shipping_methods << tagged_shipping + end + + context 'login in as user' do + let(:user) { create(:user) } + + before do + login_as(user) + end + + context "with Stripe" do + let!(:stripe_pm) do + create(:stripe_payment_method, distributors: [distributor]) + end + + let!(:saved_card) do + create(:credit_card, + user_id: user.id, + month: "01", + year: "2025", + cc_type: "visa", + number: "1111111111111111", + payment_method_id: stripe_pm.id, + gateway_customer_profile_id: "i_am_saved") + end + + let!(:stripe_account) { create(:stripe_account, enterprise_id: distributor.id, stripe_user_id: 'some_id') } + + let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } + + before do + allow(Stripe).to receive(:api_key) { "sk_test_12345" } + allow(Stripe).to receive(:publishable_key) { "some_key" } + Spree::Config.set(stripe_connect_enabled: true) + stub_request(:post, "https://api.stripe.com/v1/charges") + .with(basic_auth: ["sk_test_12345", ""]) + .to_return(status: 200, body: JSON.generate(response_mock)) + + visit checkout_path + fill_out_form(free_shipping.name, stripe_pm.name) + end + + it "allows use of a saved card" do + # shows the saved credit card dropdown + expect(page).to have_content I18n.t("spree.checkout.payment.stripe.used_saved_card") + + # default card is selected, form element is not shown + expect(page).to have_no_selector "#card-element.StripeElement" + expect(page).to have_select 'selected_card', selected: "Visa x-1111 Exp:01/2025" + + # allows checkout + place_order + expect(page).to have_content "Your order has been processed successfully" + end + end + end +end From 685a5465f1de48e10752f4b002414cbeaa2ece42 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:32:14 +0100 Subject: [PATCH 09/28] Simplify checkout stripe spec --- .../consumer/shopping/checkout_stripe_spec.rb | 32 +++---------------- spec/support/request/checkout_helper.rb | 6 ++-- 2 files changed, 9 insertions(+), 29 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index fe866a76df..8ad75d552b 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -4,42 +4,20 @@ feature "As a consumer I want to check out my cart", js: true do include AuthenticationHelper include ShopWorkflow include CheckoutHelper - include WebHelper - include UIComponentHelper - let!(:zone) { create(:zone_with_member) } - let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true) } - let(:supplier) { create(:supplier_enterprise) } - let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [variant]) } - let(:enterprise_fee) { create(:enterprise_fee, amount: 1.23, tax_category: product.tax_category) } - let(:product) { create(:taxed_product, supplier: supplier, price: 10, zone: zone, tax_rate_amount: 0.1) } + let(:distributor) { create(:distributor_enterprise) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [variant]) } + let(:product) { create(:product, price: 10) } let(:variant) { product.variants.first } let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor, bill_address_id: nil, ship_address_id: nil) } - let(:free_shipping) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow", calculator: Calculator::FlatRate.new(preferred_amount: 0.00)) } - let(:shipping_with_fee) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue", calculator: Calculator::FlatRate.new(preferred_amount: 4.56)) } - let(:tagged_shipping) { create(:shipping_method, require_ship_address: false, name: "Local", tag_list: "local") } - let!(:check_without_fee) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") } + let(:shipping_with_fee) { create(:shipping_method, require_ship_address: false, name: "Donkeys", calculator: Calculator::FlatRate.new(preferred_amount: 4.56)) } let!(:check_with_fee) { create(:payment_method, distributors: [distributor], calculator: Calculator::FlatRate.new(preferred_amount: 5.67)) } - let!(:paypal) do - Spree::Gateway::PayPalExpress.create!(name: "Paypal", environment: 'test', distributor_ids: [distributor.id]).tap do |pm| - pm.preferred_login = 'devnull-facilitator_api1.rohanmitchell.com' - pm.preferred_password = '1406163716' - pm.preferred_signature = 'AFcWxV21C7fd0v3bYYYRCpSSRl31AaTntNJ-AjvUJkWf4dgJIvcLsf1V' - end - end before do - Spree::Config.shipment_inc_vat = true - Spree::Config.shipping_tax_rate = 0.25 - - add_enterprise_fee enterprise_fee set_order order add_product_to_cart order, product - - distributor.shipping_methods << free_shipping distributor.shipping_methods << shipping_with_fee - distributor.shipping_methods << tagged_shipping end context 'login in as user' do @@ -78,7 +56,7 @@ feature "As a consumer I want to check out my cart", js: true do .to_return(status: 200, body: JSON.generate(response_mock)) visit checkout_path - fill_out_form(free_shipping.name, stripe_pm.name) + fill_out_form(shipping_with_fee.name, stripe_pm.name) end it "allows use of a saved card" do diff --git a/spec/support/request/checkout_helper.rb b/spec/support/request/checkout_helper.rb index d3c92d1608..8844743b32 100644 --- a/spec/support/request/checkout_helper.rb +++ b/spec/support/request/checkout_helper.rb @@ -47,7 +47,9 @@ module CheckoutHelper fill_out_billing_address - check "Shipping address same as billing address?" - check "Save as default shipping address" if save_default_addresses + if save_default_addresses + check "Shipping address same as billing address?" + check "Save as default shipping address" + end end end From 4ef4a585324d7e3b50e525e3cd69549454da2def Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 14:41:07 +0100 Subject: [PATCH 10/28] Merge two describe sections with same before method and call it what it is: guest checkout --- spec/features/consumer/shopping/checkout_spec.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index a7afc3b9a6..bfad8b8f02 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -156,7 +156,7 @@ feature "As a consumer I want to check out my cart", js: true do end end - context "on the checkout page" do + context "guest checkout" do before do visit checkout_path checkout_as_guest @@ -259,13 +259,6 @@ feature "As a consumer I want to check out my cart", js: true do expect(page).to have_content "Local" end end - end - - context "on the checkout page with payments open" do - before do - visit checkout_path - checkout_as_guest - end it "shows all available payment methods" do expect(page).to have_content check_without_fee.name From 746533d3f6a353c3a33a239013b047c20eb868b2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 15:00:11 +0100 Subject: [PATCH 11/28] Improve spec titles --- spec/features/consumer/shopping/checkout_paypal_spec.rb | 2 +- spec/features/consumer/shopping/checkout_stripe_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_paypal_spec.rb b/spec/features/consumer/shopping/checkout_paypal_spec.rb index ad21e46f54..b10de71a7b 100644 --- a/spec/features/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/features/consumer/shopping/checkout_paypal_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -feature "Checking out with Paypal", js: true do +feature "Check out with Paypal", js: true do include ShopWorkflow include CheckoutHelper diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index 8ad75d552b..878ff2d146 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature "As a consumer I want to check out my cart", js: true do +feature "Check out with Stripe", js: true do include AuthenticationHelper include ShopWorkflow include CheckoutHelper From ad111e837e9042930e47c06eb8b5c1f952ac1eba Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 15:00:47 +0100 Subject: [PATCH 12/28] Add spec to test terms and conditions link on checkout page --- spec/features/consumer/shopping/checkout_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index bfad8b8f02..63be0e92ee 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -122,6 +122,21 @@ feature "As a consumer I want to check out my cart", js: true do end end + context "when distributor has terms and conditions" do + let(:fake_terms_and_conditions_path) { Rails.root.join("app/assets/images/logo-white.png") } + let(:terms_and_conditions_file) { Rack::Test::UploadedFile.new(fake_terms_and_conditions_path, "application/pdf") } + + before do + order.distributor.terms_and_conditions = terms_and_conditions_file + order.distributor.save + end + + it "shows a link to the terms and conditions" do + visit checkout_path + expect(page).to have_link('Terms of Service', href: order.distributor.terms_and_conditions.url) + end + end + context "with previous orders" do let!(:prev_order) { create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor, user: order.user) } From 12d18b2825113d28847120598f9dcd71d8379d20 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 15:04:38 +0100 Subject: [PATCH 13/28] Add specs to checkout_spec to validate terms and conditions link --- spec/features/consumer/shopping/checkout_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 63be0e92ee..26be7c62a5 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -117,9 +117,13 @@ feature "As a consumer I want to check out my cart", js: true do end end - it "it doesn't tell about previous orders" do + it "doesn't tell about previous orders" do expect(page).to have_no_content("You have an order for this order cycle already.") end + + it "doesn't show link to terms and conditions" do + expect(page).to have_no_link("Terms of Service") + end end context "when distributor has terms and conditions" do @@ -133,7 +137,7 @@ feature "As a consumer I want to check out my cart", js: true do it "shows a link to the terms and conditions" do visit checkout_path - expect(page).to have_link('Terms of Service', href: order.distributor.terms_and_conditions.url) + expect(page).to have_link("Terms of Service", href: order.distributor.terms_and_conditions.url) end end From 42d5344179d68a43a9f56e20a21670cb5d7bb4e2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 15:30:55 +0100 Subject: [PATCH 14/28] Fix checkout spec by fixing wrong default value --- spec/features/consumer/shopping/checkout_stripe_spec.rb | 2 +- spec/support/request/checkout_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index 878ff2d146..dc287b0fcb 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -56,7 +56,7 @@ feature "Check out with Stripe", js: true do .to_return(status: 200, body: JSON.generate(response_mock)) visit checkout_path - fill_out_form(shipping_with_fee.name, stripe_pm.name) + fill_out_form(shipping_with_fee.name, stripe_pm.name, false) end it "allows use of a saved card" do diff --git a/spec/support/request/checkout_helper.rb b/spec/support/request/checkout_helper.rb index 8844743b32..a68f1e753b 100644 --- a/spec/support/request/checkout_helper.rb +++ b/spec/support/request/checkout_helper.rb @@ -38,7 +38,7 @@ module CheckoutHelper end end - def fill_out_form(shipping_method_name, payment_method_name, save_default_addresses = false) + def fill_out_form(shipping_method_name, payment_method_name, save_default_addresses = true) choose shipping_method_name choose payment_method_name From 07cee32f0440feeeff1f00341340cd82cfaea60d Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 16:50:43 +0100 Subject: [PATCH 15/28] Move enterprisse_console to pages/enterprise_form --- .../{enterprise_console.scss => pages/enterprise_form.scss} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename app/assets/stylesheets/admin/{enterprise_console.scss => pages/enterprise_form.scss} (87%) diff --git a/app/assets/stylesheets/admin/enterprise_console.scss b/app/assets/stylesheets/admin/pages/enterprise_form.scss similarity index 87% rename from app/assets/stylesheets/admin/enterprise_console.scss rename to app/assets/stylesheets/admin/pages/enterprise_form.scss index d475f3dc25..94e1e67a90 100644 --- a/app/assets/stylesheets/admin/enterprise_console.scss +++ b/app/assets/stylesheets/admin/pages/enterprise_form.scss @@ -1,4 +1,4 @@ -@import "variables"; +@import "../variables"; span.unavailable, span.available { font-weight: bold; @@ -13,4 +13,4 @@ span.available { span.unavailable { color: $warning-red; -} \ No newline at end of file +} From fc4cc65e073d144e0bab222d89c6dbf06e10853b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 17:14:02 +0100 Subject: [PATCH 16/28] Merge typography files in css admin --- .../stylesheets/admin/shared/typography.scss | 18 +++++++++++++++++ app/assets/stylesheets/admin/typography.scss | 20 ------------------- 2 files changed, 18 insertions(+), 20 deletions(-) delete mode 100644 app/assets/stylesheets/admin/typography.scss diff --git a/app/assets/stylesheets/admin/shared/typography.scss b/app/assets/stylesheets/admin/shared/typography.scss index 9cd3a4c2c4..16d1b86b5d 100644 --- a/app/assets/stylesheets/admin/shared/typography.scss +++ b/app/assets/stylesheets/admin/shared/typography.scss @@ -132,3 +132,21 @@ dl { padding: 40px 0px; color: lighten($color-body-text, 15); } + +.text-normal { + font-size: 1.0rem; + font-weight: 300; +} + +.text-big { + font-size: 1.2rem; + font-weight: 300; +} + +.text-red { + color: $warning-red; +} + +input.text-big { + font-size: 1.1rem; +} diff --git a/app/assets/stylesheets/admin/typography.scss b/app/assets/stylesheets/admin/typography.scss deleted file mode 100644 index 12c683d820..0000000000 --- a/app/assets/stylesheets/admin/typography.scss +++ /dev/null @@ -1,20 +0,0 @@ -@import "variables"; - -.text-normal { - font-size: 1.0rem; - font-weight: 300; -} - -.text-big { - font-size: 1.2rem; - font-weight: 300; -} - -.text-red { - color: $warning-red; -} - - -input.text-big { - font-size: 1.1rem; -} From aedc12e0e3ce29ab49dbeff9f4cfe431f50ad55f Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 17:16:39 +0100 Subject: [PATCH 17/28] Add top padding to terms file upload input in enterprises form --- app/assets/stylesheets/admin/shared/typography.scss | 4 ++++ app/views/admin/enterprises/form/_business_details.html.haml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/admin/shared/typography.scss b/app/assets/stylesheets/admin/shared/typography.scss index 16d1b86b5d..79ecba0510 100644 --- a/app/assets/stylesheets/admin/shared/typography.scss +++ b/app/assets/stylesheets/admin/shared/typography.scss @@ -150,3 +150,7 @@ dl { input.text-big { font-size: 1.1rem; } + +.pad-top { + padding-top: 1em; +} diff --git a/app/views/admin/enterprises/form/_business_details.html.haml b/app/views/admin/enterprises/form/_business_details.html.haml index 84686fafbe..c275ca7f89 100644 --- a/app/views/admin/enterprises/form/_business_details.html.haml +++ b/app/views/admin/enterprises/form/_business_details.html.haml @@ -39,5 +39,5 @@ .omega.eight.columns %a{ href: '{{ Enterprise.terms_and_conditions }}', ng: { if: 'Enterprise.terms_and_conditions' } } = '{{ Enterprise.terms_and_conditions_file_name }}' - %p + .pad-top = f.file_field :terms_and_conditions From 5a10a2861e991e68828fda242747147a86b383b7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 17:21:41 +0100 Subject: [PATCH 18/28] Reduce the size of the terms and conditions message on the checkout page --- app/views/checkout/_terms_and_conditions.html.haml | 2 +- config/locales/en.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/checkout/_terms_and_conditions.html.haml b/app/views/checkout/_terms_and_conditions.html.haml index 6811a738ae..83e03821fc 100644 --- a/app/views/checkout/_terms_and_conditions.html.haml +++ b/app/views/checkout/_terms_and_conditions.html.haml @@ -1,2 +1,2 @@ -%p +%p.small = t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions.url, target: '_blank')) if current_order.distributor.terms_and_conditions.present? diff --git a/config/locales/en.yml b/config/locales/en.yml index c90755e1d5..c9e60c9767 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1225,7 +1225,7 @@ en: cart: "cart" message_html: "You have an order for this order cycle already. Check the %{cart} to see the items you ordered before. You can also cancel items as long as the order cycle is open." terms_and_conditions: - message_html: "By placing this order you agree to the %{terms_and_conditions_link}" + message_html: "By placing this order you agree to the %{terms_and_conditions_link}." link_text: "Terms of Service" failed: "The checkout failed. Please let us know so that we can process your order." shops: From 0974c4b2aca37821006dbb1ed64c63619c90c1fd Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 17:34:48 +0100 Subject: [PATCH 19/28] Move enterprise images translations to the correct place using lazylookup on the server and to main js: namespace for js translations --- .../controllers/enterprise_controller.js.coffee | 8 ++++---- .../admin/enterprises/form/_images.html.haml | 4 ++-- config/locales/en.yml | 17 +++++++++-------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index fe5fc34992..bf588bd876 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -69,25 +69,25 @@ angular.module("admin.enterprises") $scope.newUser = $scope.invite_errors = $scope.invite_success = null $scope.removeLogo = -> - return unless confirm(t("admin.enterprises.remove_logo.immediate_removal_warning")) + return unless confirm(t("js.admin.enterprises.form.images.immediate_logo_removal_warning")) Enterprises.removeLogo($scope.Enterprise).then (data) -> $scope.Enterprise = angular.copy(data) $scope.$emit("enterprise:updated", $scope.Enterprise) - StatusMessage.display("success", t("admin.enterprises.remove_logo.removed_successfully")) + StatusMessage.display("success", t("js.admin.enterprises.form.images.removed_logo_successfully")) , (response) -> if response.data.error? StatusMessage.display("failure", response.data.error) $scope.removePromoImage = -> - return unless confirm(t("admin.enterprises.remove_promo_image.immediate_removal_warning")) + return unless confirm(t("js.admin.enterprises.form.images.immediate_promo_image_removal_warning")) Enterprises.removePromoImage($scope.Enterprise).then (data) -> $scope.Enterprise = angular.copy(data) $scope.$emit("enterprise:updated", $scope.Enterprise) - StatusMessage.display("success", t("admin.enterprises.remove_promo_image.removed_successfully")) + StatusMessage.display("success", t("js.admin.enterprises.form.images.removed_promo_image_successfully")) , (response) -> if response.data.error? StatusMessage.display("failure", response.data.error) diff --git a/app/views/admin/enterprises/form/_images.html.haml b/app/views/admin/enterprises/form/_images.html.haml index 0f48fce322..4a5a54c8c0 100644 --- a/app/views/admin/enterprises/form/_images.html.haml +++ b/app/views/admin/enterprises/form/_images.html.haml @@ -7,7 +7,7 @@ %img{ class: 'image-field-group__preview-image', ng: { src: '{{ Enterprise.logo.medium }}', if: 'Enterprise.logo' } } = f.file_field :logo %a.button.red{ href: '', ng: {click: 'removeLogo()', if: 'Enterprise.logo'} } - = t('admin.enterprises.remove_logo.remove') + = t('.remove_logo') .row.page-admin-enterprises-form__promo-image-field-group.image-field-group .alpha.three.columns @@ -23,4 +23,4 @@ %img{ class: 'image-field-group__preview-image', ng: { src: '{{ Enterprise.promo_image.large }}', if: 'Enterprise.promo_image' } } = f.file_field :promo_image %a.button.red{ href: '', ng: {click: 'removePromoImage()', if: 'Enterprise.promo_image'} } - = t('admin.enterprises.remove_promo_image.remove') + = t('.remove_promo_image') diff --git a/config/locales/en.yml b/config/locales/en.yml index c9e60c9767..3d9d84fe34 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -713,6 +713,8 @@ en: promo_image_note1: 'PLEASE NOTE:' promo_image_note2: Any promo image uploaded here will be cropped to 1200 x 260. promo_image_note3: The promo image is displayed at the top of an enterprise's profile page and pop-ups. + remove_logo: "Remove Image" + remove_promo_image: "Remove Image" inventory_settings: text1: You may opt to manage stock levels and prices in via your inventory: inventory @@ -897,14 +899,6 @@ en: new: title: New Enterprise back_link: Back to enterprises list - remove_logo: - remove: "Remove Image" - removed_successfully: "Logo removed successfully" - immediate_removal_warning: "The logo will be removed immediately after you confirm." - remove_promo_image: - remove: "Remove Image" - removed_successfully: "Promo image removed successfully" - immediate_removal_warning: "The promo image will be removed immediately after you confirm." welcome: welcome_title: Welcome to the Open Food Network! welcome_text: You have successfully created a @@ -2703,6 +2697,13 @@ See the %{link} to find out more about %{sitename}'s features and to start using error_saving: "Error saving subscription" new: please_select_a_shop: "Please select a shop" + enterprises: + form: + images: + removed_logo_successfully: "Logo removed successfully" + immediate_logo_removal_warning: "The logo will be removed immediately after you confirm." + removed_promo_image_successfully: "Promo image removed successfully" + immediate_promo_image_removal_warning: "The promo image will be removed immediately after you confirm." insufficient_stock: "Insufficient stock available, only %{on_hand} remaining" out_of_stock: reduced_stock_available: Reduced stock available From a3e9226878d328152d16e28549b2c1b0446e187a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 17:48:28 +0100 Subject: [PATCH 20/28] Add option to remove existing terms and conditions file --- app/views/admin/enterprises/form/_business_details.html.haml | 3 +++ config/locales/en.yml | 1 + 2 files changed, 4 insertions(+) diff --git a/app/views/admin/enterprises/form/_business_details.html.haml b/app/views/admin/enterprises/form/_business_details.html.haml index c275ca7f89..d15e3803c0 100644 --- a/app/views/admin/enterprises/form/_business_details.html.haml +++ b/app/views/admin/enterprises/form/_business_details.html.haml @@ -41,3 +41,6 @@ = '{{ Enterprise.terms_and_conditions_file_name }}' .pad-top = f.file_field :terms_and_conditions + .pad-top + %a.button.red{ href: '', ng: {click: 'removeTermsAndConditions()', if: 'Enterprise.terms_and_conditions'} } + = t('.remove_terms_and_conditions') diff --git a/config/locales/en.yml b/config/locales/en.yml index 3d9d84fe34..b1fef1f039 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -691,6 +691,7 @@ en: display_invoice_logo: Display logo in invoices invoice_text: Add customized text at the end of invoices terms_and_conditions: "Terms and Conditions" + remove_terms_and_conditions: "Remove File" contact: name: Name name_placeholder: eg. Gustav Plum From 24cdd0c4673dc879fdf47ea77a6760f0f9ac90cc Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 18:14:03 +0100 Subject: [PATCH 21/28] Refactor enterprise controller to reduce code duplication --- .../enterprise_controller.js.coffee | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index bf588bd876..30a0d8cf41 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -69,25 +69,29 @@ angular.module("admin.enterprises") $scope.newUser = $scope.invite_errors = $scope.invite_success = null $scope.removeLogo = -> - return unless confirm(t("js.admin.enterprises.form.images.immediate_logo_removal_warning")) + return unless confirm($scope.translation("immediate_logo_removal_warning")) - Enterprises.removeLogo($scope.Enterprise).then (data) -> - $scope.Enterprise = angular.copy(data) - $scope.$emit("enterprise:updated", $scope.Enterprise) - - StatusMessage.display("success", t("js.admin.enterprises.form.images.removed_logo_successfully")) - , (response) -> - if response.data.error? - StatusMessage.display("failure", response.data.error) + Enterprises.removeLogo($scope.Enterprise) + .then $scope.removeImageSuccessCallback("removed_logo_successfully"), + $scope.removeImageSuccessCallback() $scope.removePromoImage = -> - return unless confirm(t("js.admin.enterprises.form.images.immediate_promo_image_removal_warning")) + return unless confirm($scope.translation("immediate_promo_image_removal_warning")) - Enterprises.removePromoImage($scope.Enterprise).then (data) -> + Enterprises.removePromoImage($scope.Enterprise) + .then $scope.removeImageSuccessCallback("removed_promo_image_successfully"), + $scope.removeImageSuccessCallback() + + $scope.removeImageSuccessCallback = (success_message_key) -> + (data) -> $scope.Enterprise = angular.copy(data) $scope.$emit("enterprise:updated", $scope.Enterprise) + StatusMessage.display("success", $scope.translation(success_message_key)) - StatusMessage.display("success", t("js.admin.enterprises.form.images.removed_promo_image_successfully")) - , (response) -> + $scope.removeImageErrorCallback = -> + (response) -> if response.data.error? StatusMessage.display("failure", response.data.error) + + $scope.translation = (key) -> + t('js.admin.enterprises.form.images.' + key) From 66587ccc006dd5bd3bb6d07173cedefb47433c13 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 19:20:11 +0100 Subject: [PATCH 22/28] Allow user to remove terms and conditions file --- .../enterprise_controller.js.coffee | 7 +++ .../resources/enterprise_resource.js.coffee | 3 ++ .../resources/services/enterprises.js.coffee | 1 + .../api/terms_and_conditions_controller.rb | 16 ++++++ app/models/spree/ability_decorator.rb | 2 +- .../api/admin/enterprise_serializer.rb | 6 +++ config/locales/en.yml | 4 ++ config/routes/api.rb | 1 + .../terms_and_conditions_controller_spec.rb | 50 +++++++++++++++++++ spec/models/spree/ability_spec.rb | 4 +- 10 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 app/controllers/api/terms_and_conditions_controller.rb create mode 100644 spec/controllers/api/terms_and_conditions_controller_spec.rb diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index 30a0d8cf41..f15c65e6e7 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -82,6 +82,13 @@ angular.module("admin.enterprises") .then $scope.removeImageSuccessCallback("removed_promo_image_successfully"), $scope.removeImageSuccessCallback() + $scope.removeTermsAndConditions = -> + return unless confirm($scope.translation("immediate_terms_and_conditions_removal_warning")) + + Enterprises.removeTermsAndConditions($scope.Enterprise) + .then $scope.removeImageSuccessCallback("removed_terms_and_conditions_successfully"), + $scope.removeImageSuccessCallback() + $scope.removeImageSuccessCallback = (success_message_key) -> (data) -> $scope.Enterprise = angular.copy(data) diff --git a/app/assets/javascripts/admin/resources/resources/enterprise_resource.js.coffee b/app/assets/javascripts/admin/resources/resources/enterprise_resource.js.coffee index ec89bbda36..e7e773a42a 100644 --- a/app/assets/javascripts/admin/resources/resources/enterprise_resource.js.coffee +++ b/app/assets/javascripts/admin/resources/resources/enterprise_resource.js.coffee @@ -14,4 +14,7 @@ angular.module("admin.resources").factory 'EnterpriseResource', ($resource) -> 'removePromoImage': url: '/api/enterprises/:id/promo_image.json' method: 'DELETE' + 'removeTermsAndConditions': + url: '/api/enterprises/:id/terms_and_conditions.json' + method: 'DELETE' }) diff --git a/app/assets/javascripts/admin/resources/services/enterprises.js.coffee b/app/assets/javascripts/admin/resources/services/enterprises.js.coffee index 435cc88500..7a51e52367 100644 --- a/app/assets/javascripts/admin/resources/services/enterprises.js.coffee +++ b/app/assets/javascripts/admin/resources/services/enterprises.js.coffee @@ -52,3 +52,4 @@ angular.module("admin.resources").factory 'Enterprises', ($q, EnterpriseResource removeLogo: performActionOnEnterpriseResource(EnterpriseResource.removeLogo) removePromoImage: performActionOnEnterpriseResource(EnterpriseResource.removePromoImage) + removeTermsAndConditions: performActionOnEnterpriseResource(EnterpriseResource.removeTermsAndConditions) diff --git a/app/controllers/api/terms_and_conditions_controller.rb b/app/controllers/api/terms_and_conditions_controller.rb new file mode 100644 index 0000000000..cae88a789e --- /dev/null +++ b/app/controllers/api/terms_and_conditions_controller.rb @@ -0,0 +1,16 @@ +module Api + class TermsAndConditionsController < Api::EnterpriseAttachmentController + private + + def attachment_name + :terms_and_conditions + end + + def enterprise_authorize_action + case action_name.to_sym + when :destroy + :remove_terms_and_conditions + end + end + end +end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 6eb4727c03..57d48c7b16 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -97,7 +97,7 @@ class AbilityDecorator end can [:admin, :index, :create], Enterprise - can [:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], Enterprise do |enterprise| + can [:read, :edit, :update, :remove_logo, :remove_promo_image, :remove_terms_and_conditions, :bulk_update, :resend_confirmation], Enterprise do |enterprise| OpenFoodNetwork::Permissions.new(user).editable_enterprises.include? enterprise end can [:welcome, :register], Enterprise do |enterprise| diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb index 5c66621744..2385d996d6 100644 --- a/app/serializers/api/admin/enterprise_serializer.rb +++ b/app/serializers/api/admin/enterprise_serializer.rb @@ -20,6 +20,12 @@ class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer attachment_urls(object.promo_image, [:thumb, :medium, :large]) end + def terms_and_conditions + return unless @object.terms_and_conditions.file? + + @object.terms_and_conditions.url + end + def tag_groups object.tag_rules.prioritised.reject(&:is_default).each_with_object([]) do |tag_rule, tag_groups| tag_group = find_match(tag_groups, tag_rule.preferred_customer_tags. diff --git a/config/locales/en.yml b/config/locales/en.yml index b1fef1f039..f5ca85f623 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1207,6 +1207,8 @@ en: destroy_attachment_does_not_exist: "Logo does not exist" enterprise_promo_image: destroy_attachment_does_not_exist: "Promo image does not exist" + enterprise_terms_and_conditions: + destroy_attachment_does_not_exist: "Terms and Conditions file does not exist" orders: failed_to_update: "Failed to update order" @@ -2705,6 +2707,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using immediate_logo_removal_warning: "The logo will be removed immediately after you confirm." removed_promo_image_successfully: "Promo image removed successfully" immediate_promo_image_removal_warning: "The promo image will be removed immediately after you confirm." + immediate_terms_and_conditions_removal_warning: "The Terms and Conditions file will be removed immediately after you confirm." + removed_terms_and_conditions_successfully: "Terms and Conditions file removed successfully" insufficient_stock: "Insufficient stock available, only %{on_hand} remaining" out_of_stock: reduced_stock_available: Reduced stock available diff --git a/config/routes/api.rb b/config/routes/api.rb index 1147076112..7bc81c03fe 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -33,6 +33,7 @@ Openfoodnetwork::Application.routes.draw do resource :logo, only: [:destroy] resource :promo_image, only: [:destroy] + resource :terms_and_conditions, only: [:destroy] end resources :shops, only: [:show] do diff --git a/spec/controllers/api/terms_and_conditions_controller_spec.rb b/spec/controllers/api/terms_and_conditions_controller_spec.rb new file mode 100644 index 0000000000..25eb3d83ad --- /dev/null +++ b/spec/controllers/api/terms_and_conditions_controller_spec.rb @@ -0,0 +1,50 @@ +require "spec_helper" + +module Api + describe TermsAndConditionsController, type: :controller do + include AuthenticationHelper + + let(:enterprise_owner) { create(:user) } + let(:enterprise) { create(:enterprise, owner: enterprise_owner ) } + let(:enterprise_manager) { create(:user, enterprises: [enterprise]) } + + describe "removing terms and conditions file" do + fake_terms_file_path = File.open(Rails.root.join("app", "assets", "images", "logo-black.png")) + let(:terms_and_conditions_file) { Rack::Test::UploadedFile.new(fake_terms_file_path, "application/pdf") } + let(:enterprise) { create(:enterprise, owner: enterprise_owner) } + + before do + allow(controller).to receive(:spree_current_user) { current_user } + enterprise.update terms_and_conditions: terms_and_conditions_file + end + + context "as manager" do + let(:current_user) { enterprise_manager } + + it "removes terms and conditions file" do + spree_delete :destroy, enterprise_id: enterprise + + expect(response).to be_success + expect(json_response["id"]).to eq enterprise.id + enterprise.reload + expect(enterprise.terms_and_conditions?).to be false + end + + context "when terms and conditions file does not exist" do + let(:enterprise) { create(:enterprise, owner: enterprise_owner) } + + before do + enterprise.update terms_and_conditions: nil + end + + it "responds with error" do + spree_delete :destroy, enterprise_id: enterprise + + expect(response.status).to eq(409) + expect(json_response["error"]).to eq I18n.t("api.enterprise_terms_and_conditions.destroy_attachment_does_not_exist") + end + end + end + end + end +end diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 4710f18743..355c5d2935 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -300,11 +300,11 @@ module Spree let!(:er_pd) { create(:enterprise_relationship, parent: d_related, child: d1, permissions_list: [:edit_profile]) } it "should be able to edit enterprises it manages" do - is_expected.to have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], for: d1) + is_expected.to have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :remove_terms_and_conditions, :bulk_update, :resend_confirmation], for: d1) end it "should be able to edit enterprises it has permission to" do - is_expected.to have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], for: d_related) + is_expected.to have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :remove_terms_and_conditions, :bulk_update, :resend_confirmation], for: d_related) end it "should be able to manage shipping methods, payment methods and enterprise fees for enterprises it manages" do From 8a75fe777c8ea8d29963109c7b6820724bfae0e2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Aug 2020 20:42:51 +0100 Subject: [PATCH 23/28] Refactor enterprises controller to reduce code duplication --- .../enterprise_controller.js.coffee | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index f15c65e6e7..51c38d0a3f 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -69,34 +69,22 @@ angular.module("admin.enterprises") $scope.newUser = $scope.invite_errors = $scope.invite_success = null $scope.removeLogo = -> - return unless confirm($scope.translation("immediate_logo_removal_warning")) - - Enterprises.removeLogo($scope.Enterprise) - .then $scope.removeImageSuccessCallback("removed_logo_successfully"), - $scope.removeImageSuccessCallback() + $scope.performEnterpriseAction("removeLogo", "immediate_logo_removal_warning", "removed_logo_successfully") $scope.removePromoImage = -> - return unless confirm($scope.translation("immediate_promo_image_removal_warning")) - - Enterprises.removePromoImage($scope.Enterprise) - .then $scope.removeImageSuccessCallback("removed_promo_image_successfully"), - $scope.removeImageSuccessCallback() + $scope.performEnterpriseAction("removePromoImage", "immediate_promo_image_removal_warning", "removed_promo_image_successfully") $scope.removeTermsAndConditions = -> - return unless confirm($scope.translation("immediate_terms_and_conditions_removal_warning")) + $scope.performEnterpriseAction("removeTermsAndConditions", "immediate_terms_and_conditions_removal_warning", "removed_terms_and_conditions_successfully") - Enterprises.removeTermsAndConditions($scope.Enterprise) - .then $scope.removeImageSuccessCallback("removed_terms_and_conditions_successfully"), - $scope.removeImageSuccessCallback() + $scope.performEnterpriseAction = (enterpriseActionName, warning_message_key, success_message_key) -> + return unless confirm($scope.translation(warning_message_key)) - $scope.removeImageSuccessCallback = (success_message_key) -> - (data) -> + Enterprises[enterpriseActionName]($scope.Enterprise).then (data) -> $scope.Enterprise = angular.copy(data) $scope.$emit("enterprise:updated", $scope.Enterprise) StatusMessage.display("success", $scope.translation(success_message_key)) - - $scope.removeImageErrorCallback = -> - (response) -> + , (response) -> if response.data.error? StatusMessage.display("failure", response.data.error) From d9a228e5ec199a66948feefc288acb4e580d10ff Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 31 Aug 2020 17:47:41 +0100 Subject: [PATCH 24/28] Replace before and after hook with an around hook --- .../enterprises/terms_and_conditions_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/features/admin/enterprises/terms_and_conditions_spec.rb b/spec/features/admin/enterprises/terms_and_conditions_spec.rb index d13893c0dc..ced7c5deea 100644 --- a/spec/features/admin/enterprises/terms_and_conditions_spec.rb +++ b/spec/features/admin/enterprises/terms_and_conditions_spec.rb @@ -26,15 +26,21 @@ feature "Uploading Terms and Conditions PDF" do let(:white_pdf_file_name) { Rails.root.join("app/assets/images/logo-white.pdf") } let(:black_pdf_file_name) { Rails.root.join("app/assets/images/logo-black.pdf") } - before do + around do |example| # Create fake PDFs from PNG images FileUtils.cp(Rails.root.join("app/assets/images/logo-white.png"), white_pdf_file_name) FileUtils.cp(Rails.root.join("app/assets/images/logo-black.png"), black_pdf_file_name) - go_to_business_details + example.run + + # Delete fake PDFs + FileUtils.rm_f(white_pdf_file_name) + FileUtils.rm_f(black_pdf_file_name) end scenario "uploading terms and conditions" do + go_to_business_details + # Add PDF attach_file "enterprise[terms_and_conditions]", white_pdf_file_name click_button "Update" @@ -53,12 +59,6 @@ feature "Uploading Terms and Conditions PDF" do go_to_business_details expect(page).to have_selector("a[href*='logo-black.pdf']") end - - after do - # Delete fake PDFs - FileUtils.rm_f(white_pdf_file_name) - FileUtils.rm_f(black_pdf_file_name) - end end end end From c7a5dd65cf32bcb49da4ef0815543857289b8054 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 31 Aug 2020 18:38:46 +0100 Subject: [PATCH 25/28] Ensure all specs that change stripe_connect_enable set the value back to what it was before, which should be the default value false This will speed up specs as it ensures the stripe is always disabled and its JS script is not loaded --- .../admin/stripe_accounts_controller_spec.rb | 6 ++++++ .../stripe_connect_settings_controller_spec.rb | 16 ++++++++++++---- spec/features/admin/payment_method_spec.rb | 6 ++++++ spec/features/consumer/account/cards_spec.rb | 6 ++++++ .../consumer/shopping/checkout_stripe_spec.rb | 6 ++++++ spec/helpers/enterprises_helper_spec.rb | 6 ++++++ 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb index 079ce59f0d..39dea8d9a4 100644 --- a/spec/controllers/admin/stripe_accounts_controller_spec.rb +++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb @@ -77,6 +77,12 @@ describe Admin::StripeAccountsController, type: :controller do describe "#status" do let(:params) { { format: :json, enterprise_id: enterprise.id } } + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) + end + before do allow(Stripe).to receive(:api_key) { "sk_test_12345" } Spree::Config.set(stripe_connect_enabled: false) diff --git a/spec/controllers/admin/stripe_connect_settings_controller_spec.rb b/spec/controllers/admin/stripe_connect_settings_controller_spec.rb index 73331a151a..3de185a882 100644 --- a/spec/controllers/admin/stripe_connect_settings_controller_spec.rb +++ b/spec/controllers/admin/stripe_connect_settings_controller_spec.rb @@ -4,8 +4,10 @@ describe Admin::StripeConnectSettingsController, type: :controller do let(:user) { create(:user) } let(:admin) { create(:admin_user) } - before do - Spree::Config.set(stripe_connect_enabled: true) + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config[:stripe_connect_enabled] = original_stripe_connect_enabled end describe "edit" do @@ -19,7 +21,10 @@ describe Admin::StripeConnectSettingsController, type: :controller do end context "as super admin" do - before { allow(controller).to receive(:spree_current_user) { admin } } + before do + Spree::Config.set(stripe_connect_enabled: true) + allow(controller).to receive(:spree_current_user) { admin } + end context "when a Stripe API key is not set" do before do @@ -81,7 +86,10 @@ describe Admin::StripeConnectSettingsController, type: :controller do end context "as super admin" do - before { allow(controller).to receive(:spree_current_user) { admin } } + before do + allow(controller).to receive(:spree_current_user) { admin } + Spree::Config.set(stripe_connect_enabled: true) + end it "sets global config to the specified values" do expect(Spree::Config.stripe_connect_enabled).to be true diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb index 851c1be1a0..b40674066e 100644 --- a/spec/features/admin/payment_method_spec.rb +++ b/spec/features/admin/payment_method_spec.rb @@ -37,6 +37,12 @@ feature ' let!(:disconnected_stripe_account) { create(:stripe_account, enterprise: revoked_account_enterprise, stripe_user_id: "acc_revoked123") } let!(:stripe_account_mock) { { id: "acc_connected123", business_name: "My Org", charges_enabled: true } } + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) + end + before do Spree::Config.set(stripe_connect_enabled: true) allow(Stripe).to receive(:api_key) { "sk_test_12345" } diff --git a/spec/features/consumer/account/cards_spec.rb b/spec/features/consumer/account/cards_spec.rb index e364d8057f..a65cdd045e 100644 --- a/spec/features/consumer/account/cards_spec.rb +++ b/spec/features/consumer/account/cards_spec.rb @@ -8,6 +8,12 @@ feature "Credit Cards", js: true do let!(:default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_AZNMJ', is_default: true) } let!(:non_default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_FDTG') } + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) + end + before do login_as user diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index dc287b0fcb..d33d8a17e4 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -47,6 +47,12 @@ feature "Check out with Stripe", js: true do let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) + end + before do allow(Stripe).to receive(:api_key) { "sk_test_12345" } allow(Stripe).to receive(:publishable_key) { "some_key" } diff --git a/spec/helpers/enterprises_helper_spec.rb b/spec/helpers/enterprises_helper_spec.rb index 2ad922a259..7049ef031e 100644 --- a/spec/helpers/enterprises_helper_spec.rb +++ b/spec/helpers/enterprises_helper_spec.rb @@ -241,6 +241,12 @@ describe EnterprisesHelper, type: :helper do let!(:pm4) { create(:stripe_payment_method, distributors: [distributor], preferred_enterprise_id: some_other_distributor.id) } let(:available_payment_methods) { helper.available_payment_methods } + around do |example| + original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] + example.run + Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) + end + before do allow(helper).to receive(:current_distributor) { distributor } end From de061b4c541ac13b53167ddacec2f85934b877a3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 31 Aug 2020 18:47:24 +0100 Subject: [PATCH 26/28] Make it a keyword argument so it's easier to read --- spec/features/consumer/shopping/checkout_paypal_spec.rb | 2 +- spec/features/consumer/shopping/checkout_stripe_spec.rb | 2 +- spec/support/request/checkout_helper.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_paypal_spec.rb b/spec/features/consumer/shopping/checkout_paypal_spec.rb index b10de71a7b..107d425761 100644 --- a/spec/features/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/features/consumer/shopping/checkout_paypal_spec.rb @@ -45,7 +45,7 @@ feature "Check out with Paypal", js: true do it "fails with an error message" do visit checkout_path checkout_as_guest - fill_out_form(free_shipping.name, paypal.name, false) + fill_out_form(free_shipping.name, paypal.name, save_default_addresses: false) paypal_response = double(:response, success?: false, errors: []) paypal_provider = double( diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index d33d8a17e4..a9b7cb3727 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -62,7 +62,7 @@ feature "Check out with Stripe", js: true do .to_return(status: 200, body: JSON.generate(response_mock)) visit checkout_path - fill_out_form(shipping_with_fee.name, stripe_pm.name, false) + fill_out_form(shipping_with_fee.name, stripe_pm.name, save_default_addresses: false) end it "allows use of a saved card" do diff --git a/spec/support/request/checkout_helper.rb b/spec/support/request/checkout_helper.rb index a68f1e753b..e969abce63 100644 --- a/spec/support/request/checkout_helper.rb +++ b/spec/support/request/checkout_helper.rb @@ -38,7 +38,7 @@ module CheckoutHelper end end - def fill_out_form(shipping_method_name, payment_method_name, save_default_addresses = true) + def fill_out_form(shipping_method_name, payment_method_name, save_default_addresses: true) choose shipping_method_name choose payment_method_name From 208be3ede653c8f696e3b264af4c31080fa5e717 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 31 Aug 2020 20:00:08 +0100 Subject: [PATCH 27/28] Fix rubocop issues --- app/assets/stylesheets/admin/pages/enterprise_form.scss | 4 +++- app/assets/stylesheets/admin/shared/typography.scss | 2 +- app/controllers/api/terms_and_conditions_controller.rb | 2 ++ app/models/spree/ability_decorator.rb | 4 +++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/admin/pages/enterprise_form.scss b/app/assets/stylesheets/admin/pages/enterprise_form.scss index 94e1e67a90..55572be07c 100644 --- a/app/assets/stylesheets/admin/pages/enterprise_form.scss +++ b/app/assets/stylesheets/admin/pages/enterprise_form.scss @@ -1,7 +1,9 @@ @import "../variables"; -span.unavailable, span.available { +span.unavailable, +span.available { font-weight: bold; + i { font-size: 150%; } diff --git a/app/assets/stylesheets/admin/shared/typography.scss b/app/assets/stylesheets/admin/shared/typography.scss index 79ecba0510..cacd0e9f74 100644 --- a/app/assets/stylesheets/admin/shared/typography.scss +++ b/app/assets/stylesheets/admin/shared/typography.scss @@ -134,7 +134,7 @@ dl { } .text-normal { - font-size: 1.0rem; + font-size: 1rem; font-weight: 300; } diff --git a/app/controllers/api/terms_and_conditions_controller.rb b/app/controllers/api/terms_and_conditions_controller.rb index cae88a789e..d49721c04a 100644 --- a/app/controllers/api/terms_and_conditions_controller.rb +++ b/app/controllers/api/terms_and_conditions_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Api class TermsAndConditionsController < Api::EnterpriseAttachmentController private diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 57d48c7b16..325d93839a 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -97,7 +97,9 @@ class AbilityDecorator end can [:admin, :index, :create], Enterprise - can [:read, :edit, :update, :remove_logo, :remove_promo_image, :remove_terms_and_conditions, :bulk_update, :resend_confirmation], Enterprise do |enterprise| + can [:read, :edit, :update, + :remove_logo, :remove_promo_image, :remove_terms_and_conditions, + :bulk_update, :resend_confirmation], Enterprise do |enterprise| OpenFoodNetwork::Permissions.new(user).editable_enterprises.include? enterprise end can [:welcome, :register], Enterprise do |enterprise| From e44efd3db2c39c600b7cc6243763ca6e636046c2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 4 Sep 2020 10:06:41 +0100 Subject: [PATCH 28/28] Change test of attachment from present? to file? --- app/views/checkout/_terms_and_conditions.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/checkout/_terms_and_conditions.html.haml b/app/views/checkout/_terms_and_conditions.html.haml index 83e03821fc..a99cf44785 100644 --- a/app/views/checkout/_terms_and_conditions.html.haml +++ b/app/views/checkout/_terms_and_conditions.html.haml @@ -1,2 +1,2 @@ %p.small - = t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions.url, target: '_blank')) if current_order.distributor.terms_and_conditions.present? + = t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions.url, target: '_blank')) if current_order.distributor.terms_and_conditions.file?