CLDC-3518 Add file validation to collection resources (#2678)

* Add file validation to collection resources

* Move some tests to feature tests

* Extract validations into translations file

app/controllers/collection_resources_controller.rb

@@ -52,11 +52,15 @@ class CollectionResourcesController < ApplicationController
     @collection_resource = MandatoryCollectionResourcesService.generate_resource(log_type, year, resource_type)
     render_not_found unless @collection_resource
 
+    validate_file(file)
+
+    return render "collection_resources/edit" if @collection_resource.errors.any?
+
     filename = @collection_resource.download_filename
     begin
       UploadCollectionResourcesService.upload_collection_resource(filename, file)
     rescue StandardError
-      @collection_resource.errors.add(:file, "There was an error uploading this file.")
+      @collection_resource.errors.add(:file, :error_uploading)
       return render "collection_resources/edit"
     end
 
@@ -86,4 +90,23 @@ private
   def resource_for_year_can_be_updated?(year)
     editable_collection_resource_years.include?(year)
   end
+
+  def validate_file(file)
+    return @collection_resource.errors.add(:file, :blank) unless file
+    return @collection_resource.errors.add(:file, :above_100_mb) if file.size > 100.megabytes
+
+    argv = %W[file --brief --mime-type -- #{file.path}]
+    output = `#{argv.shelljoin}`
+
+    case @collection_resource.resource_type
+    when "paper_form"
+      unless output.match?(/application\/pdf/)
+        @collection_resource.errors.add(:file, :must_be_pdf)
+      end
+    when "bulk_upload_template", "bulk_upload_specification"
+      unless output.match?(/application\/vnd\.ms-excel|application\/vnd\.openxmlformats-officedocument\.spreadsheetml\.sheet/)
+        @collection_resource.errors.add(:file, :must_be_xlsx, resource: @collection_resource.short_display_name.downcase)
+      end
+    end
+  end
 end

app/models/collection_resource.rb

@@ -2,7 +2,7 @@ class CollectionResource
   include ActiveModel::Model
   include Rails.application.routes.url_helpers
 
-  attr_accessor :resource_type, :display_name, :short_display_name, :year, :log_type, :download_filename
+  attr_accessor :resource_type, :display_name, :short_display_name, :year, :log_type, :download_filename, :file
 
   def download_path
     download_mandatory_collection_resource_path(log_type:, year:, resource_type:)

config/locales/en.yml

@@ -117,6 +117,14 @@ en:
           attributes:
             confirm_soft_errors:
               blank: "You must select if there are errors in these fields."
+        collection_resource:
+          attributes:
+            file:
+              error_uploading: There was an error uploading this file.
+              blank: Select which file to upload.
+              above_100_mb: The file is above 100MB.
+              must_be_pdf: The paper form must be a PDF.
+              must_be_xlsx: The %{resource} must be a Microsoft Excel file.
 
   activerecord:
     attributes:

spec/features/collection_resources_spec.rb

@@ -0,0 +1,165 @@
+require "rails_helper"
+
+RSpec.describe "Collection resources" do
+  let(:user) { create(:user, :support) }
+
+  before do
+    allow(UploadCollectionResourcesService).to receive(:upload_collection_resource)
+    allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+    sign_in user
+  end
+
+  context "when uploading paper form" do
+    it "only allows pdf files for lettings" do
+      visit("/collection-resources/lettings/2024/paper_form/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the paper form")
+      expect(page).to have_content("Lettings 2024 to 2025")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      click_button("Save changes")
+
+      expect(page).to have_content("The paper form must be a PDF.")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      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(page).to have_content("The lettings 2024 to 2025 paper form has been updated")
+    end
+
+    it "only allows pdf files for sales" do
+      visit("/collection-resources/sales/2024/paper_form/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the paper form")
+      expect(page).to have_content("Sales 2024 to 2025")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      click_button("Save changes")
+
+      expect(page).to have_content("The paper form must be a PDF.")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      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(page).to have_content("The sales 2024 to 2025 paper form has been updated")
+    end
+  end
+
+  context "when uploading bu template" do
+    it "only allows excel files for lettings" do
+      visit("/collection-resources/lettings/2024/bulk_upload_template/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the bulk upload template")
+      expect(page).to have_content("Lettings 2024 to 2025")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      click_button("Save changes")
+
+      expect(page).to have_content("The bulk upload template must be a Microsoft Excel file.")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      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(page).to have_content("The lettings 2024 to 2025 bulk upload template has been updated")
+    end
+
+    it "only allows excel files for sales" do
+      visit("/collection-resources/sales/2024/bulk_upload_template/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the bulk upload template")
+      expect(page).to have_content("Sales 2024 to 2025")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      click_button("Save changes")
+
+      expect(page).to have_content("The bulk upload template must be a Microsoft Excel file.")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      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(page).to have_content("The sales 2024 to 2025 bulk upload template has been updated")
+    end
+  end
+
+  context "when uploading bu specification" do
+    it "only allows excel files for lettings" do
+      visit("/collection-resources/lettings/2024/bulk_upload_specification/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the bulk upload specification")
+      expect(page).to have_content("Lettings 2024 to 2025")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      click_button("Save changes")
+
+      expect(page).to have_content("The bulk upload specification must be a Microsoft Excel file.")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      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(page).to have_content("The lettings 2024 to 2025 bulk upload specification has been updated")
+    end
+
+    it "only allows excel files for sales" do
+      visit("/collection-resources/sales/2024/bulk_upload_specification/edit")
+
+      click_button("Save changes")
+
+      expect(page).to have_content("Select which file to upload")
+
+      expect(page).to have_content("Change the bulk upload specification")
+      expect(page).to have_content("Sales 2024 to 2025")
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      click_button("Save changes")
+
+      expect(page).to have_content("The bulk upload specification must be a Microsoft Excel file.")
+
+      attach_file "file", file_fixture("excel_file.xlsx")
+      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(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)
+
+      visit("/collection-resources/sales/2024/bulk_upload_specification/edit")
+      attach_file "file", file_fixture("excel_file.xlsx")
+      click_button("Save changes")
+
+      expect(page).to have_content("There was an error uploading this file.")
+    end
+  end
+end

spec/requests/collection_resources_controller_spec.rb

@@ -293,45 +293,5 @@ RSpec.describe CollectionResourcesController, type: :request do
         expect(response).to have_http_status(:not_found)
       end
     end
-
-    context "when user is signed in as a support user" do
-      let(:user) { create(:user, :support) }
-
-      before do
-        allow(Time.zone).to receive(:today).and_return(Time.zone.local(2025, 1, 8))
-        allow(user).to receive(:need_two_factor_authentication?).and_return(false)
-        # rubocop:disable RSpec/AnyInstance
-        allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2024, 2025])
-        # rubocop:enable RSpec/AnyInstance
-        sign_in user
-      end
-
-      it "correcty updates a sales file" do
-        params = { collection_resource: { year: 2024, log_type: "sales", resource_type: "bulk_upload_template", file: some_file } }
-        patch update_mandatory_collection_resource_path, params: params
-
-        expect(response).to redirect_to(collection_resources_path)
-        expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("bulk-upload-sales-template-2024-25.xlsx", anything)
-        expect(flash[:notice]).to eq("The sales 2024 to 2025 bulk upload template has been updated")
-      end
-
-      it "correcty updates a lettings file" do
-        params = { collection_resource: { year: 2025, log_type: "lettings", resource_type: "paper_form", file: some_file } }
-        patch update_mandatory_collection_resource_path, params: params
-
-        expect(response).to redirect_to(collection_resources_path)
-        expect(UploadCollectionResourcesService).to have_received(:upload_collection_resource).with("2025_26_lettings_paper_form.pdf", anything)
-        expect(flash[:notice]).to eq("The lettings 2025 to 2026 paper form has been updated")
-      end
-
-      it "displays error message if the upload fails" do
-        allow(UploadCollectionResourcesService).to receive(:upload_collection_resource).and_raise(StandardError)
-
-        params = { collection_resource: { year: 2025, log_type: "lettings", resource_type: "paper_form", file: some_file } }
-        patch update_mandatory_collection_resource_path, params: params
-
-        expect(page).to have_content("There was an error uploading this file.")
-      end
-    end
   end
 end