Display and validate additional resources

app/controllers/collection_resources_controller.rb

@@ -8,6 +8,8 @@ class CollectionResourcesController < ApplicationController
 
     @mandatory_lettings_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("lettings", editable_collection_resource_years)
     @mandatory_sales_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("sales", editable_collection_resource_years)
+    @additional_lettings_collection_resources_per_year = CollectionResource.where(log_type: "lettings", mandatory: false).group_by(&:year)
+    @additional_sales_collection_resources_per_year = CollectionResource.where(log_type: "sales", mandatory: false).group_by(&:year)
   end
 
   def download_mandatory_collection_resource
@@ -52,7 +54,8 @@ class CollectionResourcesController < ApplicationController
     @collection_resource = MandatoryCollectionResourcesService.generate_resource(log_type, year, resource_type)
     render_not_found unless @collection_resource
 
-    validate_file(file)
+    @collection_resource.file = file
+    @collection_resource.validate_attached_file
 
     return render "collection_resources/edit" if @collection_resource.errors.any?
 
@@ -107,26 +110,29 @@ class CollectionResourcesController < ApplicationController
 
     @collection_resource = CollectionResource.new(resource_params)
     @collection_resource.download_filename ||= @collection_resource.file&.original_filename
+    @collection_resource.display_name = "#{@collection_resource.log_type} #{@collection_resource.short_display_name} (#{text_year_range_format(@collection_resource.year)})"
 
-    validate_file(@collection_resource.file)
+    @collection_resource.validate_attached_file
     return render "collection_resources/new" if @collection_resource.errors.any?
 
-    begin
-      CollectionResourcesService.new.upload_collection_resource(@collection_resource.download_filename, @collection_resource.file)
-      @collection_resource.save!
-    rescue StandardError
-      @collection_resource.errors.add(:file, :error_uploading)
-      return render "collection_resources/new"
+    if @collection_resource.save
+      begin
+        CollectionResourcesService.new.upload_collection_resource(@collection_resource.download_filename, @collection_resource.file)
+        flash[:notice] = "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name} is now available to users."
+        redirect_to collection_resources_path
+      rescue StandardError
+        @collection_resource.errors.add(:file, :error_uploading)
+        render "collection_resources/new"
+      end
+    else
+      render "collection_resources/new"
     end
-
-    flash[:notice] = "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.display_name} is now available to users."
-    redirect_to collection_resources_path
   end
 
 private
 
   def resource_params
-    params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file, :mandatory, :display_name)
+    params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file, :mandatory, :short_display_name)
   end
 
   def download_resource(filename)
@@ -145,23 +151,4 @@ 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

@@ -3,7 +3,32 @@ class CollectionResource < ApplicationRecord
 
   attr_accessor :file
 
+  validates :short_display_name, presence: true
+
   def download_path
-    download_mandatory_collection_resource_path(log_type:, year:, resource_type:)
+    if mandatory
+      download_mandatory_collection_resource_path(log_type:, year:, resource_type:)
+    else
+      "/"
+    end
+  end
+
+  def validate_attached_file
+    return errors.add(:file, :blank) unless file
+    return errors.add(:file, :above_100_mb) if file.size > 100.megabytes
+
+    argv = %W[file --brief --mime-type -- #{file.path}]
+    output = `#{argv.shelljoin}`
+
+    case resource_type
+    when "paper_form"
+      unless output.match?(/application\/pdf/)
+        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/)
+        errors.add(:file, :must_be_xlsx, resource: short_display_name.downcase)
+      end
+    end
   end
 end

app/views/collection_resources/_collection_resource_summary_list.erb

@@ -23,6 +23,28 @@
           <% end %>
         <% end %>
       <% end %>
+      <% additional_resources&.each do |resource| %>
+        <% summary_list.with_row do |row| %>
+          <% row.with_key { resource.short_display_name } %>
+          <% if file_exists_on_s3?(resource.download_filename) %>
+            <% row.with_value do %>
+              <%= render DocumentListComponent.new(items: document_list_edit_component_items([resource]), label: "") %>
+            <% end %>
+            <% row.with_action(
+              text: "Change",
+              href: "/",
+            ) %>
+          <% else %>
+            <% row.with_value do %>
+              <p class="govuk-body app-!-colour-muted">No file uploaded</p>
+            <% end %>
+            <% row.with_action(
+              text: "Upload",
+              href: "/",
+            ) %>
+          <% end %>
+        <% end %>
+      <% end %>
     <% end %>
     <div class="govuk-!-margin-bottom-6">
       <%= govuk_link_to "Add new #{mandatory_resources.first.log_type} #{text_year_range_format(mandatory_resources.first.year)} resource", href: new_collection_resource_path(year: mandatory_resources.first.year, log_type: mandatory_resources.first.log_type) %>

app/views/collection_resources/index.html.erb

@@ -18,12 +18,12 @@
   <h2 class="govuk-heading-m">
     Lettings <%= text_year_range_format(year) %>
   </h2>
-  <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources: } %>
+  <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources:, additional_resources: @additional_lettings_collection_resources_per_year[year] } %>
 <% end %>
 
 <% @mandatory_sales_collection_resources_per_year.each do |year, mandatory_resources| %>
   <h2 class="govuk-heading-m">
     Sales <%= text_year_range_format(year) %>
   </h2>
-  <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources: } %>
+  <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources:, additional_resources: @additional_sales_collection_resources_per_year[year] } %>
 <% end %>

app/views/collection_resources/new.html.erb

@@ -21,7 +21,7 @@
       <%= f.govuk_file_field :file,
         label: { text: "Upload file", size: "m" } %>
 
-      <%= f.govuk_text_field :display_name,
+      <%= f.govuk_text_field :short_display_name,
                       label: nil,
                       hint: { text: "This will be used in the download link on the homepage. Do not include the log type or collection year. For example, if you enter “bulk upload change log”, the download link will say “Download the lettings bulk upload change log (2024 to 2025)”." } %>
 

config/locales/en.yml

@@ -215,6 +215,9 @@ en:
               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.
+            short_display_name:
+              blank: "You must answer resource type."
+
   notification:
     logs_deleted:
       one: "%{count} log has been deleted."

spec/factories/collection_resource.rb

@@ -6,5 +6,14 @@ FactoryBot.define do
     year { 2024 }
     log_type { "lettings" }
     download_filename { "24_25_lettings_paper_form.pdf" }
+    trait(:additional) do
+      resource_type { nil }
+      display_name { "additional resource" }
+      short_display_name { nil }
+      year { 2024 }
+      log_type { "lettings" }
+      download_filename { "additional.pdf" }
+      mandatory { false }
+    end
   end
 end

spec/features/collection_resources_spec.rb

@@ -174,12 +174,9 @@ RSpec.describe "Collection resources" do
       expect(CollectionResource.count).to eq(0)
 
       visit(new_collection_resource_path(year: 2025, log_type: "sales"))
-
-      click_button("Add resource")
-      expect(page).to have_content("Select which file to upload")
-
+      fill_in("collection_resource[short_display_name]", with: "some file")
       attach_file "file", file_fixture("pdf_file.pdf")
-      fill_in("collection_resource[display_name]", with: "some file")
+
       click_button("Add resource")
       expect(collection_resources_service).to have_received(:upload_collection_resource).with("pdf_file.pdf", anything)
       expect(CollectionResource.count).to eq(1)
@@ -188,8 +185,25 @@ RSpec.describe "Collection resources" do
       expect(CollectionResource.first.resource_type).to be_nil
       expect(CollectionResource.first.mandatory).to be_falsey
       expect(CollectionResource.first.released_to_user).to be_nil
-      expect(CollectionResource.first.display_name).to eq("some file")
+      expect(CollectionResource.first.display_name).to eq("sales some file (2025 to 2026)")
+      expect(CollectionResource.first.short_display_name).to eq("some file")
       expect(page).to have_content("The sales 2025 to 2026 some file is now available to users.")
     end
+
+    it "validates file is attached" do
+      visit(new_collection_resource_path(year: 2025, log_type: "sales"))
+
+      fill_in("collection_resource[short_display_name]", with: "some file")
+      click_button("Add resource")
+      expect(page).to have_content("Select which file to upload.")
+    end
+
+    it "validates resource type is given" do
+      visit(new_collection_resource_path(year: 2025, log_type: "sales"))
+
+      attach_file "file", file_fixture("pdf_file.pdf")
+      click_button("Add resource")
+      expect(page).to have_content("You must answer resource type.")
+    end
   end
 end

spec/requests/collection_resources_controller_spec.rb

@@ -156,6 +156,23 @@ RSpec.describe CollectionResourcesController, type: :request do
           expect(page).to have_content("Once you have uploaded all the required 2025 to 2026 collection resources, you will be able to release them to users.")
         end
       end
+
+      context "when there are additional resources" do
+        before do
+          # rubocop:disable RSpec/AnyInstance
+          allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025])
+          # rubocop:enable RSpec/AnyInstance
+          create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource")
+          create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource 2")
+        end
+
+        it "displays additional resources for editable years" do
+          get collection_resources_path
+
+          expect(page).to have_content("additional resource")
+          expect(page).not_to have_content("additional resource 2")
+        end
+      end
     end
   end