From 41112c14625dbe21e5bc2420740223d2540a69d7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 29 Nov 2023 12:14:00 +1100 Subject: [PATCH 1/4] DRY uploaded file use in specs --- spec/controllers/api/v0/logos_controller_spec.rb | 2 +- spec/controllers/api/v0/product_images_controller_spec.rb | 2 +- spec/controllers/api/v0/promo_images_controller_spec.rb | 2 +- spec/factories/order_cycle_factory.rb | 2 +- spec/support/file_helper.rb | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/controllers/api/v0/logos_controller_spec.rb b/spec/controllers/api/v0/logos_controller_spec.rb index 9aa9769d1d..b1bb65b187 100644 --- a/spec/controllers/api/v0/logos_controller_spec.rb +++ b/spec/controllers/api/v0/logos_controller_spec.rb @@ -18,7 +18,7 @@ module Api } describe "removing logo" do - let(:image) { Rack::Test::UploadedFile.new(black_logo_file, "image/png") } + let(:image) { black_logo_file } let(:enterprise) { create(:enterprise, owner: enterprise_owner, logo: image) } diff --git a/spec/controllers/api/v0/product_images_controller_spec.rb b/spec/controllers/api/v0/product_images_controller_spec.rb index 464e9025cc..a85db7771c 100644 --- a/spec/controllers/api/v0/product_images_controller_spec.rb +++ b/spec/controllers/api/v0/product_images_controller_spec.rb @@ -8,7 +8,7 @@ describe Api::V0::ProductImagesController, type: :controller do render_views describe "uploading an image" do - let(:image) { Rack::Test::UploadedFile.new(black_logo_file, 'image/png') } + let(:image) { black_logo_file } let(:pdf) { Rack::Test::UploadedFile.new(pdf_path, 'application/pdf') } let(:pdf_path) { Rails.public_path.join('Terms-of-service.pdf') } let(:product_without_image) { create(:product) } diff --git a/spec/controllers/api/v0/promo_images_controller_spec.rb b/spec/controllers/api/v0/promo_images_controller_spec.rb index dff570aba3..92fdce4940 100644 --- a/spec/controllers/api/v0/promo_images_controller_spec.rb +++ b/spec/controllers/api/v0/promo_images_controller_spec.rb @@ -18,7 +18,7 @@ module Api } describe "removing promo image" do - let(:image) { Rack::Test::UploadedFile.new(black_logo_file, "image/png") } + let(:image) { black_logo_file } let(:enterprise) { create(:enterprise, owner: enterprise_owner, promo_image: image) } diff --git a/spec/factories/order_cycle_factory.rb b/spec/factories/order_cycle_factory.rb index 8518bb68c1..18c1fd7181 100644 --- a/spec/factories/order_cycle_factory.rb +++ b/spec/factories/order_cycle_factory.rb @@ -35,7 +35,7 @@ FactoryBot.define do viewable_id: product.id, viewable_type: 'Spree::Product', alt: "position 1", - attachment: Rack::Test::UploadedFile.new(white_logo_path), + attachment: white_logo_file, position: 1 ) diff --git a/spec/support/file_helper.rb b/spec/support/file_helper.rb index 8f4ead848b..4ab44257d7 100644 --- a/spec/support/file_helper.rb +++ b/spec/support/file_helper.rb @@ -2,11 +2,11 @@ module FileHelper def black_logo_file - Rack::Test::UploadedFile.new(black_logo_path) + Rack::Test::UploadedFile.new(black_logo_path, "image/png") end def white_logo_file - Rack::Test::UploadedFile.new(white_logo_path) + Rack::Test::UploadedFile.new(white_logo_path, "image/png") end def black_logo_path From 6327f46733f38994209689b1809a307ebb91ea3f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 29 Nov 2023 12:20:51 +1100 Subject: [PATCH 2/4] Use fixture_file_upload helper where possible We can't use it in factories but in other places it's a nice shortcut. --- spec/controllers/api/v0/product_images_controller_spec.rb | 2 +- .../api/v0/terms_and_conditions_controller_spec.rb | 2 +- spec/models/terms_of_service_file_spec.rb | 2 +- spec/support/api_helper.rb | 2 +- spec/system/admin/tos_banner_spec.rb | 2 +- spec/system/consumer/checkout/summary_spec.rb | 4 ++-- spec/system/consumer/shopping/checkout_spec.rb | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/controllers/api/v0/product_images_controller_spec.rb b/spec/controllers/api/v0/product_images_controller_spec.rb index a85db7771c..a19ef98c00 100644 --- a/spec/controllers/api/v0/product_images_controller_spec.rb +++ b/spec/controllers/api/v0/product_images_controller_spec.rb @@ -9,7 +9,7 @@ describe Api::V0::ProductImagesController, type: :controller do describe "uploading an image" do let(:image) { black_logo_file } - let(:pdf) { Rack::Test::UploadedFile.new(pdf_path, 'application/pdf') } + let(:pdf) { fixture_file_upload(pdf_path, 'application/pdf') } let(:pdf_path) { Rails.public_path.join('Terms-of-service.pdf') } let(:product_without_image) { create(:product) } let(:product_with_image) { create(:product_with_image) } diff --git a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb index 89d0b6fc08..ed8978cd7f 100644 --- a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb +++ b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb @@ -14,7 +14,7 @@ module Api describe "removing terms and conditions file" do let(:terms_file_path) { Rails.public_path.join('Terms-of-service.pdf') } let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(terms_file_path, "application/pdf") + fixture_file_upload(terms_file_path, "application/pdf") } let(:enterprise) { create(:enterprise, owner: enterprise_owner) } diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index 21718a0e9b..d702a66365 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe TermsOfServiceFile do let(:pdf) { File.open(Rails.public_path.join('Terms-of-service.pdf')) } - let(:upload) { Rack::Test::UploadedFile.new(pdf, "application/pdf") } + let(:upload) { fixture_file_upload(pdf, "application/pdf") } describe ".current" do it "returns nil" do diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 7632c90c86..a0f8e10105 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -28,7 +28,7 @@ module OpenFoodNetwork end def image(filename) - Rack::Test::UploadedFile.new(Rails.root + "spec/support/fixtures" + filename) + fixture_file_upload(Rails.root + "spec/support/fixtures" + filename) end end end diff --git a/spec/system/admin/tos_banner_spec.rb b/spec/system/admin/tos_banner_spec.rb index 48808896f3..853c18384a 100644 --- a/spec/system/admin/tos_banner_spec.rb +++ b/spec/system/admin/tos_banner_spec.rb @@ -8,7 +8,7 @@ describe 'Terms of Service banner' do let(:admin_user) { create(:admin_user, terms_of_service_accepted_at: nil) } let(:test_file) { "Terms-of-service.pdf" } let(:pdf_upload) do - Rack::Test::UploadedFile.new(Rails.public_path.join(test_file), "application/pdf") + fixture_file_upload(Rails.public_path.join(test_file), "application/pdf") end before do diff --git a/spec/system/consumer/checkout/summary_spec.rb b/spec/system/consumer/checkout/summary_spec.rb index 2f2767d7dd..408acbbbfb 100644 --- a/spec/system/consumer/checkout/summary_spec.rb +++ b/spec/system/consumer/checkout/summary_spec.rb @@ -126,10 +126,10 @@ describe "As a consumer, I want to checkout my order" do let(:system_terms_path) { Rails.public_path.join('Terms-of-service.pdf') } let(:shop_terms_path) { Rails.public_path.join('Terms-of-ServiceUK.pdf') } let(:system_terms) { - Rack::Test::UploadedFile.new(system_terms_path, "application/pdf") + fixture_file_upload(system_terms_path, "application/pdf") } let(:shop_terms) { - Rack::Test::UploadedFile.new(shop_terms_path, "application/pdf") + fixture_file_upload(shop_terms_path, "application/pdf") } context "when none are required" do diff --git a/spec/system/consumer/shopping/checkout_spec.rb b/spec/system/consumer/shopping/checkout_spec.rb index 8b39be28f5..33332d8819 100644 --- a/spec/system/consumer/shopping/checkout_spec.rb +++ b/spec/system/consumer/shopping/checkout_spec.rb @@ -90,7 +90,7 @@ describe "As a consumer I want to check out my cart" do pending 'login in as user' do let(:user) { create(:user) } let(:pdf_upload) { - Rack::Test::UploadedFile.new( + fixture_file_upload( Rails.public_path.join('Terms-of-service.pdf'), "application/pdf" ) From dd639435f1a4e53bdab1a7c1415be4ff1b84c9c6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 8 Jan 2024 10:58:27 +1100 Subject: [PATCH 3/4] Remove unnecessary image file helper --- spec/controllers/api/v0/products_controller_spec.rb | 6 ++++-- .../fixtures => fixtures/files}/thinking-cat.jpg | Bin spec/support/api_helper.rb | 4 ---- spec/system/admin/products_spec.rb | 8 +++----- 4 files changed, 7 insertions(+), 11 deletions(-) rename spec/{support/fixtures => fixtures/files}/thinking-cat.jpg (100%) diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index c31e323896..b96081a57e 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -25,15 +25,17 @@ describe Api::V0::ProductsController, type: :controller do end context "as a normal user" do + let(:attachment) { fixture_file_upload("thinking-cat.jpg") } + before do allow(current_api_user) .to receive(:has_spree_role?).with("admin").and_return(false) end it "gets a single product" do - product.create_image!(attachment: image("thinking-cat.jpg")) + product.create_image!(attachment:) product.variants.create!(unit_value: "1", unit_description: "thing", price: 1) - product.variants.first.images.create!(attachment: image("thinking-cat.jpg")) + product.variants.first.images.create!(attachment:) product.set_property("spree", "rocks") api_get :show, id: product.to_param diff --git a/spec/support/fixtures/thinking-cat.jpg b/spec/fixtures/files/thinking-cat.jpg similarity index 100% rename from spec/support/fixtures/thinking-cat.jpg rename to spec/fixtures/files/thinking-cat.jpg diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index a0f8e10105..b9546b25e6 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -26,9 +26,5 @@ module OpenFoodNetwork expect(json_response).to eq("error" => "You are not authorized to perform that action.") expect(response.status).to eq 401 end - - def image(filename) - fixture_file_upload(Rails.root + "spec/support/fixtures" + filename) - end end end diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 47a5e58cef..0a83938ce0 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -340,6 +340,7 @@ describe ' context "as an enterprise user" do let!(:tax_category) { create(:tax_category) } let(:filter) { { producerFilter: 2 } } + let(:image_file_path) { Rails.root.join(file_fixture_path, "thinking-cat.jpg") } before do @new_user = create(:user) @@ -627,14 +628,13 @@ describe ' end it "upload a new product image including url filters" do - file_path = Rails.root + "spec/support/fixtures/thinking-cat.jpg" product = create(:simple_product, supplier: @supplier2) visit spree.admin_product_images_path(product, filter) page.find('a#new_image_link').click - attach_file('image_attachment', file_path) + attach_file('image_attachment', image_file_path) click_button "Create" uri = URI.parse(current_url) @@ -680,13 +680,11 @@ describe ' viewable_type: 'Spree::Product', alt: "position 1", attachment: image, position: 1) - file_path = Rails.root + "spec/support/fixtures/thinking-cat.jpg" - visit spree.admin_product_images_path(product, filter) page.find("a.icon-edit").click - attach_file('image_attachment', file_path) + attach_file('image_attachment', image_file_path) click_button "Update" uri = URI.parse(current_url) From 2699ae6ca70e9f701ea6be1afe8b2849b464d418 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 8 Jan 2024 11:28:47 +1100 Subject: [PATCH 4/4] DRY terms of service PDF file use in specs --- .../api/v0/product_images_controller_spec.rb | 3 +-- .../v0/terms_and_conditions_controller_spec.rb | 5 +---- spec/models/terms_of_service_file_spec.rb | 5 +++-- spec/support/file_helper.rb | 5 +++++ spec/system/admin/tos_banner_spec.rb | 8 +++----- spec/system/consumer/checkout/summary_spec.rb | 16 +++++----------- spec/system/consumer/shopping/checkout_spec.rb | 7 +------ 7 files changed, 19 insertions(+), 30 deletions(-) diff --git a/spec/controllers/api/v0/product_images_controller_spec.rb b/spec/controllers/api/v0/product_images_controller_spec.rb index a19ef98c00..2c0a380bee 100644 --- a/spec/controllers/api/v0/product_images_controller_spec.rb +++ b/spec/controllers/api/v0/product_images_controller_spec.rb @@ -9,8 +9,7 @@ describe Api::V0::ProductImagesController, type: :controller do describe "uploading an image" do let(:image) { black_logo_file } - let(:pdf) { fixture_file_upload(pdf_path, 'application/pdf') } - let(:pdf_path) { Rails.public_path.join('Terms-of-service.pdf') } + let(:pdf) { terms_pdf_file } let(:product_without_image) { create(:product) } let(:product_with_image) { create(:product_with_image) } let(:current_api_user) { create(:admin_user) } diff --git a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb index ed8978cd7f..d5e7f511f8 100644 --- a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb +++ b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb @@ -12,10 +12,7 @@ module Api let(:enterprise_manager) { create(:user, enterprises: [enterprise]) } describe "removing terms and conditions file" do - let(:terms_file_path) { Rails.public_path.join('Terms-of-service.pdf') } - let(:terms_and_conditions_file) { - fixture_file_upload(terms_file_path, "application/pdf") - } + let(:terms_and_conditions_file) { terms_pdf_file } let(:enterprise) { create(:enterprise, owner: enterprise_owner) } before do diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index d702a66365..19d9fb183d 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe TermsOfServiceFile do - let(:pdf) { File.open(Rails.public_path.join('Terms-of-service.pdf')) } - let(:upload) { fixture_file_upload(pdf, "application/pdf") } + include FileHelper + + let(:upload) { terms_pdf_file } describe ".current" do it "returns nil" do diff --git a/spec/support/file_helper.rb b/spec/support/file_helper.rb index 4ab44257d7..3e2feb2233 100644 --- a/spec/support/file_helper.rb +++ b/spec/support/file_helper.rb @@ -16,4 +16,9 @@ module FileHelper def white_logo_path Rails.root.join('app/webpacker/images/logo-white.png') end + + def terms_pdf_file + file_path = Rails.public_path.join("Terms-of-service.pdf") + fixture_file_upload(file_path, "application/pdf") + end end diff --git a/spec/system/admin/tos_banner_spec.rb b/spec/system/admin/tos_banner_spec.rb index 853c18384a..902d11071f 100644 --- a/spec/system/admin/tos_banner_spec.rb +++ b/spec/system/admin/tos_banner_spec.rb @@ -4,12 +4,10 @@ require 'system_helper' describe 'Terms of Service banner' do include AuthenticationHelper + include FileHelper let(:admin_user) { create(:admin_user, terms_of_service_accepted_at: nil) } - let(:test_file) { "Terms-of-service.pdf" } - let(:pdf_upload) do - fixture_file_upload(Rails.public_path.join(test_file), "application/pdf") - end + let(:pdf_upload) { terms_pdf_file } before do Spree::Config.enterprises_require_tos = true @@ -43,7 +41,7 @@ describe 'Terms of Service banner' do # Upload new ToS visit admin_terms_of_service_files_path - attach_file "Attachment", Rails.public_path.join(test_file) + attach_file "Attachment", pdf_upload.path click_button "Create Terms of service file" # check it has been uploaded diff --git a/spec/system/consumer/checkout/summary_spec.rb b/spec/system/consumer/checkout/summary_spec.rb index 408acbbbfb..f36dfa5f22 100644 --- a/spec/system/consumer/checkout/summary_spec.rb +++ b/spec/system/consumer/checkout/summary_spec.rb @@ -123,14 +123,8 @@ describe "As a consumer, I want to checkout my order" do describe "terms and conditions" do let(:customer) { create(:customer, enterprise: order.distributor, user:) } let(:tos_url) { "https://example.org/tos" } - let(:system_terms_path) { Rails.public_path.join('Terms-of-service.pdf') } - let(:shop_terms_path) { Rails.public_path.join('Terms-of-ServiceUK.pdf') } - let(:system_terms) { - fixture_file_upload(system_terms_path, "application/pdf") - } - let(:shop_terms) { - fixture_file_upload(shop_terms_path, "application/pdf") - } + let(:system_terms) { terms_pdf_file } + let(:shop_terms) { terms_pdf_file } context "when none are required" do it "doesn't show checkbox or links" do @@ -152,7 +146,7 @@ describe "As a consumer, I want to checkout my order" do describe "when customer has not accepted T&Cs before" do it "shows a link to the T&Cs and disables checkout button until terms are accepted" do visit checkout_step_path(:summary) - expect(page).to have_link "Terms and Conditions", href: /#{shop_terms_path.basename}$/ + expect(page).to have_link "Terms and Conditions", href: /Terms-of-service\.pdf/ expect(page).to have_field "order_accept_terms", checked: false end end @@ -243,8 +237,8 @@ describe "As a consumer, I want to checkout my order" do it "shows links to both terms and all need accepting" do visit checkout_step_path(:summary) - expect(page).to have_link "Terms and Conditions", href: /#{shop_terms_path.basename}$/ - expect(page).to have_link("Terms of service", href: /Terms-of-service.pdf/, count: 2) + expect(page).to have_link "Terms and Conditions", href: /Terms-of-service\.pdf/ + expect(page).to have_link("Terms of service", href: /Terms-of-service\.pdf/, count: 2) expect(page).to have_field "order_accept_terms", checked: false end end diff --git a/spec/system/consumer/shopping/checkout_spec.rb b/spec/system/consumer/shopping/checkout_spec.rb index 33332d8819..b897468232 100644 --- a/spec/system/consumer/shopping/checkout_spec.rb +++ b/spec/system/consumer/shopping/checkout_spec.rb @@ -89,12 +89,7 @@ describe "As a consumer I want to check out my cart" do pending 'login in as user' do let(:user) { create(:user) } - let(:pdf_upload) { - fixture_file_upload( - Rails.public_path.join('Terms-of-service.pdf'), - "application/pdf" - ) - } + let(:pdf_upload) { terms_pdf_file } before do login_as(user)