diff --git a/app/controllers/collection_resources_controller.rb b/app/controllers/collection_resources_controller.rb index 8e2f0bbcc..f1fd6de07 100644 --- a/app/controllers/collection_resources_controller.rb +++ b/app/controllers/collection_resources_controller.rb @@ -58,7 +58,7 @@ class CollectionResourcesController < ApplicationController filename = @collection_resource.download_filename begin - UploadCollectionResourcesService.upload_collection_resource(filename, file) + CollectionResourcesService.new.upload_collection_resource(filename, file) rescue StandardError @collection_resource.errors.add(:file, :error_uploading) return render "collection_resources/edit" @@ -74,11 +74,11 @@ private params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file) end - def download_resource(filename, download_filename) + def download_resource(filename) file = CollectionResourcesService.new.get_file(filename) return render_not_found unless file - send_data(file, disposition: "attachment", filename: download_filename) + send_data(file, disposition: "attachment", filename:) end def resource_for_year_can_be_downloaded?(year) diff --git a/app/helpers/collection_resources_helper.rb b/app/helpers/collection_resources_helper.rb index 9db134e7a..e1fb6cf2b 100644 --- a/app/helpers/collection_resources_helper.rb +++ b/app/helpers/collection_resources_helper.rb @@ -64,12 +64,6 @@ module CollectionResourcesHelper end def file_exists_on_s3?(file) - url = "https://#{Rails.application.config.collection_resources_s3_bucket_name}.s3.amazonaws.com/#{file}" - uri = URI.parse(url) - - response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |http| - http.request_head(uri) - end - response.is_a?(Net::HTTPSuccess) || response.is_a?(Net::HTTPRedirection) + CollectionResourcesService.new.file_exists_on_s3?(file) end end diff --git a/app/services/collection_resources_service.rb b/app/services/collection_resources_service.rb index 50e1fc59a..42667bb78 100644 --- a/app/services/collection_resources_service.rb +++ b/app/services/collection_resources_service.rb @@ -18,4 +18,18 @@ class CollectionResourcesService rescue StandardError nil end + + def file_exists_on_s3?(file) + url = "https://#{@storage_service.configuration.bucket_name}.s3.amazonaws.com/#{file}" + uri = URI.parse(url) + + response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |http| + http.request_head(uri) + end + response.is_a?(Net::HTTPSuccess) || response.is_a?(Net::HTTPRedirection) + end + + def upload_collection_resource(filename, file) + @storage_service.write_file(filename, file) + end end diff --git a/app/services/upload_collection_resources_service.rb b/app/services/upload_collection_resources_service.rb deleted file mode 100644 index e137531df..000000000 --- a/app/services/upload_collection_resources_service.rb +++ /dev/null @@ -1,6 +0,0 @@ -class UploadCollectionResourcesService - def self.upload_collection_resource(filename, file) - storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["COLLECTION_RESOURCES_BUCKET"]) - storage_service.write_file(filename, file) - end -end diff --git a/spec/features/collection_resources_spec.rb b/spec/features/collection_resources_spec.rb index 452d76b91..aaf4b30d4 100644 --- a/spec/features/collection_resources_spec.rb +++ b/spec/features/collection_resources_spec.rb @@ -2,9 +2,12 @@ require "rails_helper" RSpec.describe "Collection resources" do let(:user) { create(:user, :support) } + let(:collection_resources_service) { instance_double(CollectionResourcesService, file_exists_on_s3?: true) } before do - allow(UploadCollectionResourcesService).to receive(:upload_collection_resource) + allow(CollectionResourcesService).to receive(:new).and_return(collection_resources_service) + allow(collection_resources_service).to receive(:upload_collection_resource) + allow(collection_resources_service).to receive(:get_file_metadata).and_return({ "Content-Type" => "application/pdf", "Content-Length" => 1000 }) allow(user).to receive(:need_two_factor_authentication?).and_return(false) sign_in user end @@ -29,7 +32,7 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The paper form must be a PDF.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("2024_25_lettings_paper_form.pdf", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("2024_25_lettings_paper_form.pdf", anything) expect(page).to have_content("The lettings 2024 to 2025 paper form has been updated") end @@ -52,7 +55,7 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The paper form must be a PDF.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("2024_25_sales_paper_form.pdf", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("2024_25_sales_paper_form.pdf", anything) expect(page).to have_content("The sales 2024 to 2025 paper form has been updated") end end @@ -77,7 +80,7 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The bulk upload template must be a Microsoft Excel file.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("bulk-upload-lettings-template-2024-25.xlsx", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("bulk-upload-lettings-template-2024-25.xlsx", anything) expect(page).to have_content("The lettings 2024 to 2025 bulk upload template has been updated") end @@ -100,7 +103,7 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The bulk upload template must be a Microsoft Excel file.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("bulk-upload-sales-template-2024-25.xlsx", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("bulk-upload-sales-template-2024-25.xlsx", anything) expect(page).to have_content("The sales 2024 to 2025 bulk upload template has been updated") end end @@ -125,7 +128,7 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The bulk upload specification must be a Microsoft Excel file.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("bulk-upload-lettings-specification-2024-25.xlsx", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("bulk-upload-lettings-specification-2024-25.xlsx", anything) expect(page).to have_content("The lettings 2024 to 2025 bulk upload specification has been updated") end @@ -148,12 +151,12 @@ RSpec.describe "Collection resources" do click_button("Save changes") expect(page).not_to have_content("The bulk upload specification must be a Microsoft Excel file.") - expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("bulk-upload-sales-specification-2024-25.xlsx", anything) + expect(collection_resources_service).to have_received(:upload_collection_resource).with("bulk-upload-sales-specification-2024-25.xlsx", anything) expect(page).to have_content("The sales 2024 to 2025 bulk upload specification has been updated") end it "displays error message if the upload fails" do - allow(UploadCollectionResourcesService).to receive(:upload_collection_resource).and_raise(StandardError) + allow(collection_resources_service).to receive(:upload_collection_resource).and_raise(StandardError) visit("/collection-resources/sales/2024/bulk_upload_specification/edit") attach_file "file", file_fixture("excel_file.xlsx") diff --git a/spec/requests/collection_resources_controller_spec.rb b/spec/requests/collection_resources_controller_spec.rb index cee39febf..fb8b7ab6f 100644 --- a/spec/requests/collection_resources_controller_spec.rb +++ b/spec/requests/collection_resources_controller_spec.rb @@ -2,6 +2,12 @@ require "rails_helper" RSpec.describe CollectionResourcesController, type: :request do let(:page) { Capybara::Node::Simple.new(response.body) } + let(:storage_service) { instance_double(Storage::S3Service) } + + before do + allow(Storage::S3Service).to receive(:new).and_return(storage_service) + allow(storage_service).to receive(:configuration).and_return(OpenStruct.new(bucket_name: "core-test-collection-resources")) + end describe "GET #index" do context "when user is not signed in" do diff --git a/spec/services/upload_collection_resources_service_spec.rb b/spec/services/collection_resources_service_spec.rb similarity index 88% rename from spec/services/upload_collection_resources_service_spec.rb rename to spec/services/collection_resources_service_spec.rb index bb5cabc46..3786b70c9 100644 --- a/spec/services/upload_collection_resources_service_spec.rb +++ b/spec/services/collection_resources_service_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" -describe UploadCollectionResourcesService do - let(:service) { described_class } +describe CollectionResourcesService do + let(:service) { described_class.new } let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) } let(:storage_service) { instance_double(Storage::S3Service) }