From e71b2aeb9f687c9ac41d2b3e5993d5867a6f4690 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 23 Oct 2024 10:21:41 +0100
Subject: [PATCH] CLDC-3638 Allow adding custom resources (#2690)

* Stub some collection resource requests

* Rebase changes

* Add upload new resource page

* Allow adding custom collection resources

* Display and validate additional resources

* Allow downloading additional collection resources

* Allow updating additional resources

* Display additional resources on the homepage

* Add delete link to additional resources

* Update hint text

* Update tests

* Allow unauthenticated users to donwload additional collection resources

* Add some changes to copy and styling

* Add content type to the files

* Validate short display name at the same time as file
---
 .../collection_resources_controller.rb        | 122 +++++--
 app/controllers/start_controller.rb           |   2 +
 app/helpers/collection_resources_helper.rb    |   2 +-
 app/models/collection_resource.rb             |  31 +-
 app/services/collection_resources_service.rb  |   3 +-
 .../mandatory_collection_resources_service.rb |   1 +
 app/services/storage/local_disk_service.rb    |   2 +-
 app/services/storage/s3_service.rb            |  21 +-
 .../_collection_resource_summary_list.erb     |  19 +-
 app/views/collection_resources/edit.html.erb  |  21 +-
 app/views/collection_resources/index.html.erb |   4 +-
 app/views/collection_resources/new.html.erb   |  36 ++
 .../layouts/_collection_resources.html.erb    |   4 +-
 config/locales/en.yml                         |   7 +-
 config/routes.rb                              |   8 +-
 spec/factories/collection_resource.rb         |  10 +
 spec/features/collection_resources_spec.rb    |  73 ++++
 .../collection_resources_helper_spec.rb       |  12 +-
 .../collection_resources_controller_spec.rb   | 342 +++++++++++++++++-
 spec/requests/start_controller_spec.rb        |   2 +
 .../collection_resources_service_spec.rb      |   2 +-
 21 files changed, 668 insertions(+), 56 deletions(-)
 create mode 100644 app/views/collection_resources/new.html.erb

diff --git a/app/controllers/collection_resources_controller.rb b/app/controllers/collection_resources_controller.rb
index ed080c749..5203ceea1 100644
--- a/app/controllers/collection_resources_controller.rb
+++ b/app/controllers/collection_resources_controller.rb
@@ -1,13 +1,15 @@
 class CollectionResourcesController < ApplicationController
   include CollectionResourcesHelper
 
-  before_action :authenticate_user!, except: %i[download_mandatory_collection_resource]
+  before_action :authenticate_user!, except: %i[download_mandatory_collection_resource download_additional_collection_resource]
 
   def index
     render_not_found unless current_user.support?
 
     @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
@@ -23,7 +25,16 @@ class CollectionResourcesController < ApplicationController
     download_resource(resource.download_filename)
   end
 
-  def edit
+  def download_additional_collection_resource
+    resource = CollectionResource.find_by(id: params[:collection_resource_id])
+
+    return render_not_found unless resource
+    return render_not_found unless resource_for_year_can_be_downloaded?(resource.year)
+
+    download_resource(resource.download_filename)
+  end
+
+  def edit_mandatory_collection_resource
     return render_not_found unless current_user.support?
 
     year = params[:year].to_i
@@ -39,7 +50,18 @@ class CollectionResourcesController < ApplicationController
     render "collection_resources/edit"
   end
 
-  def update
+  def edit_additional_collection_resource
+    return render_not_found unless current_user.support?
+
+    @collection_resource = CollectionResource.find_by(id: params[:collection_resource_id])
+
+    return render_not_found unless @collection_resource
+    return render_not_found unless resource_for_year_can_be_updated?(@collection_resource.year)
+
+    render "collection_resources/edit"
+  end
+
+  def update_mandatory_collection_resource
     return render_not_found unless current_user.support?
 
     year = resource_params[:year].to_i
@@ -52,7 +74,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?
 
@@ -68,6 +91,36 @@ class CollectionResourcesController < ApplicationController
     redirect_to collection_resources_path
   end
 
+  def update_additional_collection_resource
+    return render_not_found unless current_user.support?
+
+    @collection_resource = CollectionResource.find_by(id: params[:collection_resource_id])
+
+    return render_not_found unless @collection_resource
+    return render_not_found unless resource_for_year_can_be_updated?(@collection_resource.year)
+
+    @collection_resource.file = resource_params[:file]
+    @collection_resource.validate_attached_file
+    @collection_resource.validate_short_display_name
+    return render "collection_resources/edit" if @collection_resource.errors.any?
+
+    @collection_resource.short_display_name = resource_params[:short_display_name]
+    @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)})"
+    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.downcase} has been updated."
+        redirect_to collection_resources_path
+      rescue StandardError
+        @collection_resource.errors.add(:file, :error_uploading)
+        render "collection_resources/edit"
+      end
+    else
+      render "collection_resources/edit"
+    end
+  end
+
   def confirm_mandatory_collection_resources_release
     return render_not_found unless current_user.support?
 
@@ -91,10 +144,50 @@ class CollectionResourcesController < ApplicationController
     redirect_to collection_resources_path
   end
 
+  def new
+    return render_not_found unless current_user.support?
+
+    year = params[:year].to_i
+    log_type = params[:log_type]
+
+    return render_not_found unless editable_collection_resource_years.include?(year)
+
+    @collection_resource = CollectionResource.new(year:, log_type:)
+  end
+
+  def create
+    return render_not_found unless current_user.support? && editable_collection_resource_years.include?(resource_params[:year].to_i)
+
+    @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)})"
+
+    @collection_resource.validate_attached_file
+    @collection_resource.validate_short_display_name
+    return render "collection_resources/new" if @collection_resource.errors.any?
+
+    if @collection_resource.save
+      begin
+        CollectionResourcesService.new.upload_collection_resource(@collection_resource.download_filename, @collection_resource.file)
+        flash[:notice] = if displayed_collection_resource_years.include?(@collection_resource.year)
+                           "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name} is now available to users."
+                         else
+                           "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name} has been uploaded."
+                         end
+        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
+  end
+
 private
 
   def resource_params
-    params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file)
+    params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file, :mandatory, :short_display_name)
   end
 
   def download_resource(filename)
@@ -113,23 +206,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
diff --git a/app/controllers/start_controller.rb b/app/controllers/start_controller.rb
index 5bd49df3f..b65da4d44 100644
--- a/app/controllers/start_controller.rb
+++ b/app/controllers/start_controller.rb
@@ -4,6 +4,8 @@ class StartController < ApplicationController
   def index
     @mandatory_lettings_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("lettings", displayed_collection_resource_years)
     @mandatory_sales_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("sales", displayed_collection_resource_years)
+    @additional_lettings_collection_resources_per_year = CollectionResource.where(log_type: "lettings", mandatory: false, year: displayed_collection_resource_years).group_by(&:year)
+    @additional_sales_collection_resources_per_year = CollectionResource.where(log_type: "sales", mandatory: false, year: displayed_collection_resource_years).group_by(&:year)
     if current_user
       @homepage_presenter = HomepagePresenter.new(current_user)
       render "home/index"
diff --git a/app/helpers/collection_resources_helper.rb b/app/helpers/collection_resources_helper.rb
index 9deb13887..1a5887744 100644
--- a/app/helpers/collection_resources_helper.rb
+++ b/app/helpers/collection_resources_helper.rb
@@ -49,7 +49,7 @@ module CollectionResourcesHelper
   def document_list_component_items(resources)
     resources.map do |resource|
       {
-        name: "Download the #{resource.display_name}",
+        name: "Download the #{resource.display_name.downcase}",
         href: resource.download_path,
         metadata: file_type_size_and_pages(resource.download_filename),
       }
diff --git a/app/models/collection_resource.rb b/app/models/collection_resource.rb
index 81665ccf6..cc217b8cd 100644
--- a/app/models/collection_resource.rb
+++ b/app/models/collection_resource.rb
@@ -3,7 +3,36 @@ 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
+      collection_resource_download_path(self)
+    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
+
+  def validate_short_display_name
+    errors.add(:short_display_name, :blank) if short_display_name.blank?
   end
 end
diff --git a/app/services/collection_resources_service.rb b/app/services/collection_resources_service.rb
index 81ab08254..82247f7a0 100644
--- a/app/services/collection_resources_service.rb
+++ b/app/services/collection_resources_service.rb
@@ -24,6 +24,7 @@ class CollectionResourcesService
   end
 
   def upload_collection_resource(filename, file)
-    @storage_service.write_file(filename, file)
+    content_type = MiniMime.lookup_by_filename(filename)&.content_type
+    @storage_service.write_file(filename, file, content_type:)
   end
 end
diff --git a/app/services/mandatory_collection_resources_service.rb b/app/services/mandatory_collection_resources_service.rb
index 8b34153e9..397e4b5d0 100644
--- a/app/services/mandatory_collection_resources_service.rb
+++ b/app/services/mandatory_collection_resources_service.rb
@@ -27,6 +27,7 @@ class MandatoryCollectionResourcesService
       year:,
       log_type:,
       download_filename: download_filename(resource_type, year, log_type),
+      mandatory: true,
     )
   end
 
diff --git a/app/services/storage/local_disk_service.rb b/app/services/storage/local_disk_service.rb
index bb5825340..ad3cc9608 100644
--- a/app/services/storage/local_disk_service.rb
+++ b/app/services/storage/local_disk_service.rb
@@ -19,7 +19,7 @@ module Storage
       File.open(path, "r")
     end
 
-    def write_file(filename, data)
+    def write_file(filename, data, _content_type: nil)
       path = Rails.root.join("tmp/storage", filename)
 
       FileUtils.mkdir_p(path.dirname)
diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb
index 88199c0a0..2e8daa719 100644
--- a/app/services/storage/s3_service.rb
+++ b/app/services/storage/s3_service.rb
@@ -36,12 +36,21 @@ module Storage
              .body.read
     end
 
-    def write_file(file_name, data)
-      @client.put_object(
-        body: data,
-        bucket: @configuration.bucket_name,
-        key: file_name,
-      )
+    def write_file(file_name, data, content_type: nil)
+      if content_type.nil?
+        @client.put_object(
+          body: data,
+          bucket: @configuration.bucket_name,
+          key: file_name,
+        )
+      else
+        @client.put_object(
+          body: data,
+          bucket: @configuration.bucket_name,
+          key: file_name,
+          content_type:,
+        )
+      end
     end
 
     def get_file_metadata(file_name)
diff --git a/app/views/collection_resources/_collection_resource_summary_list.erb b/app/views/collection_resources/_collection_resource_summary_list.erb
index 8ef588fd5..0e630f0aa 100644
--- a/app/views/collection_resources/_collection_resource_summary_list.erb
+++ b/app/views/collection_resources/_collection_resource_summary_list.erb
@@ -23,9 +23,26 @@
           <% end %>
         <% end %>
       <% end %>
+      <% additional_resources&.each do |resource| %>
+        <% summary_list.with_row do |row| %>
+          <% row.with_key { resource.short_display_name } %>
+          <% row.with_value do %>
+            <%= render DocumentListComponent.new(items: document_list_edit_component_items([resource]), label: "") %>
+          <% end %>
+          <% row.with_action(
+            text: "Change",
+            href: collection_resource_edit_path(resource),
+          ) %>
+          <% row.with_action(
+            text: "Delete",
+            href: "/",
+            classes: "app-!-colour-red"
+          ) %>
+        <% 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: "/" %>
+      <%= 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) %>
     </div>
     <hr class="govuk-section-break govuk-section-break--visible govuk-section-break--m">
   </div>
diff --git a/app/views/collection_resources/edit.html.erb b/app/views/collection_resources/edit.html.erb
index e5b2a6d33..6100256c8 100644
--- a/app/views/collection_resources/edit.html.erb
+++ b/app/views/collection_resources/edit.html.erb
@@ -5,7 +5,7 @@
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <% resource_exists = file_exists_on_s3?(@collection_resource.download_filename) %>
-    <%= form_with model: @collection_resource, url: update_mandatory_collection_resource_path, method: :patch do |f| %>
+    <%= form_with model: @collection_resource, url: @collection_resource.mandatory ? update_mandatory_collection_resource_path : collection_resource_update_path(@collection_resource), method: :patch do |f| %>
       <%= f.hidden_field :year %>
       <%= f.hidden_field :log_type %>
       <%= f.hidden_field :resource_type %>
@@ -13,7 +13,7 @@
       <%= f.govuk_error_summary %>
 
       <span class="govuk-caption-l"><%= "#{@collection_resource.log_type.humanize} #{text_year_range_format(@collection_resource.year)}" %></span>
-      <h1 class="govuk-heading-l"><%= resource_exists ? "Change" : "Upload" %> the <%= @collection_resource.resource_type.humanize.downcase %></h1>
+      <h1 class="govuk-heading-l"><%= resource_exists ? "Change" : "Upload" %> the <%= @collection_resource.short_display_name.downcase %></h1>
 
       <p class="govuk-body">
         This file will be available for all users to download.
@@ -21,9 +21,22 @@
 
       <%= f.govuk_file_field :file,
         label: { text: "Upload file", size: "m" } %>
+      <% if resource_exists %>
+        <p class="govuk-body">Current file: <%= govuk_link_to @collection_resource.download_filename, href: @collection_resource.download_path %></p>
+      <% end %>
 
-      <%= f.govuk_submit resource_exists ? "Save changes" : "Upload" %>
-      <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %>
+      <% unless @collection_resource.mandatory %>
+        <%= f.govuk_text_field :short_display_name,
+                  label: { text: "Resource type", size: "m" },
+                  hint: { text: safe_join(["This will be used in the download link on the homepage. Do not include the log type or collection year.",
+                                           content_tag(:br),
+                                           "For example, if you enter “bulk upload change log”, the download link will say “Download the #{@collection_resource.log_type} bulk upload change log (#{text_year_range_format(@collection_resource.year)})”."]) } %>
+                                          <% end %>
+
+      <div class="govuk-button-group">
+        <%= f.govuk_submit resource_exists ? "Save changes" : "Upload" %>
+        <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %>
+      </div>
     <% end %>
   </div>
 </div>
diff --git a/app/views/collection_resources/index.html.erb b/app/views/collection_resources/index.html.erb
index f6785eb46..207a63991 100644
--- a/app/views/collection_resources/index.html.erb
+++ b/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 %>
diff --git a/app/views/collection_resources/new.html.erb b/app/views/collection_resources/new.html.erb
new file mode 100644
index 000000000..f66c56b94
--- /dev/null
+++ b/app/views/collection_resources/new.html.erb
@@ -0,0 +1,36 @@
+<% content_for :before_content do %>
+  <%= govuk_back_link href: collection_resources_path %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_with model: @collection_resource, url: collection_resources_path, method: :post do |f| %>
+      <%= f.hidden_field :year %>
+      <%= f.hidden_field :log_type %>
+      <%= f.hidden_field :mandatory, value: false %>
+
+      <%= f.govuk_error_summary %>
+
+      <span class="govuk-caption-l"><%= "#{@collection_resource.log_type.humanize} #{text_year_range_format(@collection_resource.year)}" %></span>
+      <h1 class="govuk-heading-l">Add a new collection resource</h1>
+
+      <p class="govuk-body">
+        This file will be available for all users to download.
+      </p>
+
+      <%= f.govuk_file_field :file,
+        label: { text: "Upload file", size: "m" } %>
+
+      <%= f.govuk_text_field :short_display_name,
+                      label: { text: "Resource type", size: "m" },
+                      hint: { text: safe_join(["This will be used in the download link on the homepage. Do not include the log type or collection year.",
+                                               content_tag(:br),
+                                               "For example, if you enter “bulk upload change log”, the download link will say “Download the #{@collection_resource.log_type} bulk upload change log (#{text_year_range_format(@collection_resource.year)})”."]) } %>
+
+      <div class="govuk-button-group">
+        <%= f.govuk_submit "Add resource" %>
+        <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %>
+      </div>
+    <% end %>
+  </div>
+</div>
diff --git a/app/views/layouts/_collection_resources.html.erb b/app/views/layouts/_collection_resources.html.erb
index 935f1806c..07648e5c0 100644
--- a/app/views/layouts/_collection_resources.html.erb
+++ b/app/views/layouts/_collection_resources.html.erb
@@ -12,12 +12,12 @@
   <%= govuk_tabs(title: "Collection resources", classes: %w[app-tab__small-headers]) do |c| %>
     <% @mandatory_lettings_collection_resources_per_year.each do |year, resources| %>
       <% c.with_tab(label: "Lettings #{year_range_format(year)}") do %>
-          <%= render DocumentListComponent.new(items: document_list_component_items(resources), label: "Lettings #{text_year_range_format(year)}") %>
+          <%= render DocumentListComponent.new(items: document_list_component_items(resources.concat(@additional_lettings_collection_resources_per_year[year] || [])), label: "Lettings #{text_year_range_format(year)}") %>
       <% end %>
     <% end %>
     <% @mandatory_sales_collection_resources_per_year.each do |year, resources| %>
       <% c.with_tab(label: "Sales #{year_range_format(year)}") do %>
-          <%= render DocumentListComponent.new(items: document_list_component_items(resources), label: "Sales #{text_year_range_format(year)}") %>
+          <%= render DocumentListComponent.new(items: document_list_component_items(resources.concat(@additional_sales_collection_resources_per_year[year] || [])), label: "Sales #{text_year_range_format(year)}") %>
       <% end %>
     <% end %>
   <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index ff73bfdcb..d490fd8ae 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -122,7 +122,7 @@ en:
             file:
               error_uploading: There was an error uploading this file.
               blank: Select which file to upload.
-              above_100_mb: The file is above 100MB.
+              above_100_mb: File must be 100MB or less.
               must_be_pdf: The paper form must be a PDF.
               must_be_xlsx: The %{resource} must be a Microsoft Excel file.
 
@@ -212,9 +212,12 @@ en:
             file:
               error_uploading: There was an error uploading this file.
               blank: Select which file to upload.
-              above_100_mb: The file is above 100MB.
+              above_100_mb: File must be 100MB or less.
               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."
diff --git a/config/routes.rb b/config/routes.rb
index db4b2a8a2..cbac4894a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -42,13 +42,15 @@ Rails.application.routes.draw do
 
   get "collection-resources", to: "collection_resources#index"
   get "/collection-resources/:log_type/:year/:resource_type/download", to: "collection_resources#download_mandatory_collection_resource", as: :download_mandatory_collection_resource
-  get "/collection-resources/:log_type/:year/:resource_type/edit", to: "collection_resources#edit", as: :edit_mandatory_collection_resource
-  patch "/collection-resources", to: "collection_resources#update", as: :update_mandatory_collection_resource
+  get "/collection-resources/:log_type/:year/:resource_type/edit", to: "collection_resources#edit_mandatory_collection_resource", as: :edit_mandatory_collection_resource
+  patch "/collection-resources", to: "collection_resources#update_mandatory_collection_resource", as: :update_mandatory_collection_resource
   get "/collection-resources/:year/release", to: "collection_resources#confirm_mandatory_collection_resources_release", as: :confirm_mandatory_collection_resources_release
   patch "/collection-resources/:year/release", to: "collection_resources#release_mandatory_collection_resources", as: :release_mandatory_collection_resources
 
   resources :collection_resources, path: "/collection-resources" do
-    get "/download", to: "collection_resources#download_additional_collection_resource" # when we get to adding them
+    get "/download", to: "collection_resources#download_additional_collection_resource"
+    get "/edit", to: "collection_resources#edit_additional_collection_resource"
+    patch "/update", to: "collection_resources#update_additional_collection_resource"
   end
 
   get "clear-filters", to: "sessions#clear_filters"
diff --git a/spec/factories/collection_resource.rb b/spec/factories/collection_resource.rb
index c352b70a9..0282e33e3 100644
--- a/spec/factories/collection_resource.rb
+++ b/spec/factories/collection_resource.rb
@@ -6,5 +6,15 @@ FactoryBot.define do
     year { 2024 }
     log_type { "lettings" }
     download_filename { "24_25_lettings_paper_form.pdf" }
+    mandatory { true }
+    trait(:additional) do
+      resource_type { nil }
+      display_name { "lettings additional resource (2024 to 2025)" }
+      short_display_name { "additional resource" }
+      year { 2024 }
+      log_type { "lettings" }
+      download_filename { "additional.pdf" }
+      mandatory { false }
+    end
   end
 end
diff --git a/spec/features/collection_resources_spec.rb b/spec/features/collection_resources_spec.rb
index 643f3537e..ad9a370b6 100644
--- a/spec/features/collection_resources_spec.rb
+++ b/spec/features/collection_resources_spec.rb
@@ -5,6 +5,9 @@ RSpec.describe "Collection resources" do
   let(:collection_resources_service) { instance_double(CollectionResourcesService, file_exists_on_s3?: true) }
 
   before do
+    # rubocop:disable RSpec/AnyInstance
+    allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2024, 2025])
+    # rubocop:enable RSpec/AnyInstance
     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 })
@@ -165,4 +168,74 @@ RSpec.describe "Collection resources" do
       expect(page).to have_content("There was an error uploading this file.")
     end
   end
+
+  context "when uploading an additional resource" do
+    it "allows valid files" do
+      expect(CollectionResource.count).to eq(0)
+
+      visit(new_collection_resource_path(year: 2025, log_type: "sales"))
+      fill_in("collection_resource[short_display_name]", with: "some file")
+      attach_file "file", file_fixture("pdf_file.pdf")
+
+      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)
+      expect(CollectionResource.first.year).to eq(2025)
+      expect(CollectionResource.first.log_type).to eq("sales")
+      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("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 has been uploaded.")
+    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
+
+  context "when updating an additional resource" do
+    let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales") }
+
+    it "only allows valid files" do
+      expect(CollectionResource.count).to eq(1)
+
+      visit(collection_resource_edit_path(collection_resource))
+      fill_in("collection_resource[short_display_name]", with: "some updated file")
+      attach_file "file", file_fixture("pdf_file.pdf")
+
+      click_button("Save changes")
+      expect(collection_resources_service).to have_received(:upload_collection_resource).with("pdf_file.pdf", anything)
+      expect(CollectionResource.count).to eq(1)
+      expect(CollectionResource.first.year).to eq(2025)
+      expect(CollectionResource.first.log_type).to eq("sales")
+      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("sales some updated file (2025 to 2026)")
+      expect(CollectionResource.first.short_display_name).to eq("some updated file")
+      expect(page).to have_content("The sales 2025 to 2026 some updated file has been updated.")
+    end
+
+    it "validates file is attached" do
+      visit(collection_resource_edit_path(collection_resource))
+
+      fill_in("collection_resource[short_display_name]", with: "some file")
+      click_button("Save changes")
+      expect(page).to have_content("Select which file to upload.")
+    end
+  end
 end
diff --git a/spec/helpers/collection_resources_helper_spec.rb b/spec/helpers/collection_resources_helper_spec.rb
index 859d14cd1..9fc77a4c9 100644
--- a/spec/helpers/collection_resources_helper_spec.rb
+++ b/spec/helpers/collection_resources_helper_spec.rb
@@ -94,9 +94,9 @@ RSpec.describe CollectionResourcesHelper do
 
       context "and next year resources were manually released" do
         before do
-          CollectionResource.create!(year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true)
-          CollectionResource.create!(year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
-          CollectionResource.create!(year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
         end
 
         it "reutrns current and next years" do
@@ -199,9 +199,9 @@ RSpec.describe CollectionResourcesHelper do
 
       context "and the resources have been manually released" do
         before do
-          CollectionResource.create!(year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true)
-          CollectionResource.create!(year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
-          CollectionResource.create!(year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
+          create(:collection_resource, year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true)
         end
 
         it "returns false" do
diff --git a/spec/requests/collection_resources_controller_spec.rb b/spec/requests/collection_resources_controller_spec.rb
index 7c9bb0f05..a6d88f911 100644
--- a/spec/requests/collection_resources_controller_spec.rb
+++ b/spec/requests/collection_resources_controller_spec.rb
@@ -62,7 +62,7 @@ RSpec.describe CollectionResourcesController, type: :request do
         expect(page).to have_content("Sales 2025 to 2026")
       end
 
-      it "displays mandatory filed" do
+      it "displays mandatory files" do
         get collection_resources_path
 
         expect(page).to have_content("Paper form")
@@ -70,6 +70,15 @@ RSpec.describe CollectionResourcesController, type: :request do
         expect(page).to have_content("Bulk upload specification")
       end
 
+      it "allows uploading new resources" do
+        get collection_resources_path
+
+        expect(page).to have_link("Add new sales 2024 to 2025 resource", href: new_collection_resource_path(year: 2024, log_type: "sales"))
+        expect(page).to have_link("Add new lettings 2024 to 2025 resource", href: new_collection_resource_path(year: 2024, log_type: "lettings"))
+        expect(page).to have_link("Add new sales 2025 to 2026 resource", href: new_collection_resource_path(year: 2025, log_type: "sales"))
+        expect(page).to have_link("Add new lettings 2025 to 2026 resource", href: new_collection_resource_path(year: 2025, log_type: "lettings"))
+      end
+
       context "when files are on S3" do
         before do
           allow(storage_service).to receive(:file_exists?).and_return(true)
@@ -113,6 +122,16 @@ RSpec.describe CollectionResourcesController, type: :request do
           expect(page).to have_content("The 2025 to 2026 collection resources are not yet available to users.")
           expect(page).to have_link("Release the 2025 to 2026 collection resources to users", href: confirm_mandatory_collection_resources_release_path(year: 2025))
         end
+
+        context "when there are additional resources" do
+          let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource", download_filename: "additional.pdf") }
+
+          it "displays change links for additional resources" do
+            get collection_resources_path
+
+            expect(page).to have_link("Change", href: collection_resource_edit_path(collection_resource))
+          end
+        end
       end
 
       context "when files are not on S3" do
@@ -147,6 +166,26 @@ 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
+        let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource", download_filename: "additional.pdf") }
+
+        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: 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")
+          expect(page).to have_link("additional.pdf", href: collection_resource_download_path(collection_resource))
+          expect(page).to have_link("Delete")
+        end
+      end
     end
   end
 
@@ -473,4 +512,305 @@ RSpec.describe CollectionResourcesController, type: :request do
       end
     end
   end
+
+  describe "GET #new_collection_resource" do
+    context "when user is not signed in" do
+      it "redirects to the sign in page" do
+        get new_collection_resource_path(year: 2025, log_type: "sales")
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context "when user is signed in as a data coordinator" do
+      let(:user) { create(:user, :data_coordinator) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        get new_collection_resource_path(year: 2025, log_type: "sales")
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in as a data provider" do
+      let(:user) { create(:user, :data_provider) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        get new_collection_resource_path(year: 2025, log_type: "sales")
+        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
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025, 2026])
+        # rubocop:enable RSpec/AnyInstance
+        allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+        sign_in user
+      end
+
+      it "displays new collection resource page content" do
+        get new_collection_resource_path(year: 2025, log_type: "sales")
+
+        expect(page).to have_content("Sales 2025 to 2026")
+        expect(page).to have_content("Add a new collection resource")
+        expect(page).to have_content("Upload file")
+        expect(page).to have_button("Add resource")
+        expect(page).to have_link("Back", href: collection_resources_path)
+        expect(page).to have_link("Cancel", href: collection_resources_path)
+      end
+    end
+  end
+
+  describe "POST #collection_resources" do
+    let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) }
+    let(:params) { { collection_resource: { year: 2025, log_type: "sales", file: some_file, display_name: "some file" } } }
+
+    context "when user is not signed in" do
+      it "redirects to the sign in page" do
+        post collection_resources_path, params: params
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context "when user is signed in as a data coordinator" do
+      let(:user) { create(:user, :data_coordinator) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        post collection_resources_path, params: params
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in as a data provider" do
+      let(:user) { create(:user, :data_provider) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        post collection_resources_path, params: params
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+  end
+
+  describe "GET #download_additional_collection_resource" do
+    let(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource") }
+
+    before do
+      # rubocop:disable RSpec/AnyInstance
+      allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025, 2026])
+      allow_any_instance_of(CollectionResourcesHelper).to receive(:displayed_collection_resource_years).and_return([2025])
+      # rubocop:enable RSpec/AnyInstance
+    end
+
+    context "when the user is not signed in" do
+      context "when the file exists on S3" do
+        before do
+          allow(storage_service).to receive(:get_file).and_return("file")
+          get collection_resource_download_path(collection_resource)
+        end
+
+        it "downloads the file" do
+          expect(response.body).to eq("file")
+        end
+      end
+    end
+
+    context "when user is signed in as a data coordinator" do
+      let(:user) { create(:user, :data_coordinator) }
+
+      context "when the file exists on S3" do
+        before do
+          sign_in user
+          allow(storage_service).to receive(:get_file).and_return("file")
+          get collection_resource_download_path(collection_resource)
+        end
+
+        it "downloads the file" do
+          expect(response.body).to eq("file")
+        end
+      end
+
+      context "when the file does not exist on S3" do
+        before do
+          sign_in user
+          allow(storage_service).to receive(:get_file).and_return(nil)
+          get collection_resource_download_path(collection_resource)
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+
+      context "when resource id is invalid" do
+        before do
+          sign_in user
+          allow(storage_service).to receive(:get_file).and_return(nil)
+          get collection_resource_download_path(collection_resource_id: "invalid")
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+
+      context "when year not in displayed_collection_resource_years" do
+        let(:collection_resource) { create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource") }
+
+        before do
+          sign_in user
+          get collection_resource_download_path(collection_resource)
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+    end
+
+    context "when user is signed in as a support user" do
+      let(:collection_resource) { create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource") }
+      let(:user) { create(:user, :support) }
+
+      context "when year is in editable_collection_resource_years but not in displayed_collection_resource_years" do
+        before do
+          allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+          sign_in user
+          allow(storage_service).to receive(:get_file).and_return("file")
+          get collection_resource_download_path(collection_resource)
+        end
+
+        it "downloads the file" do
+          expect(response.status).to eq(200)
+          expect(response.body).to eq("file")
+        end
+      end
+    end
+  end
+
+  describe "GET #edit_additional_collection_resource" do
+    let(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales", short_display_name: "additional resource", download_filename: "additional.pdf") }
+
+    context "when user is not signed in" do
+      it "redirects to the sign in page" do
+        get collection_resource_edit_path(collection_resource)
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context "when user is signed in as a data coordinator" do
+      let(:user) { create(:user, :data_coordinator) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        get collection_resource_edit_path(collection_resource)
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in as a data provider" do
+      let(:user) { create(:user, :data_provider) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        get collection_resource_edit_path(collection_resource)
+        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)
+        sign_in user
+      end
+
+      context "and the file exists on S3" do
+        before do
+          allow(storage_service).to receive(:file_exists?).and_return(true)
+        end
+
+        it "displays update collection resources page content" do
+          get collection_resource_edit_path(collection_resource)
+
+          expect(page).to have_content("Sales 2025 to 2026")
+          expect(page).to have_content("Change the additional resource")
+          expect(page).to have_content("This file will be available for all users to download.")
+          expect(page).to have_content("Upload file")
+          expect(page).to have_button("Save changes")
+          expect(page).to have_link("Back", href: collection_resources_path)
+          expect(page).to have_link("Cancel", href: collection_resources_path)
+        end
+      end
+    end
+  end
+
+  describe "PATCH #update_additional_collection_resource" do
+    let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) }
+    let(:params) { { collection_resource: { short_display_name: "short name", file: some_file } } }
+    let(:collection_resource_service) { instance_double(CollectionResourcesService) }
+    let(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales", short_display_name: "additional resource", download_filename: "additional.pdf") }
+
+    before do
+      allow(CollectionResourcesService).to receive(:new).and_return(collection_resource_service)
+    end
+
+    context "when user is not signed in" do
+      it "redirects to the sign in page" do
+        patch collection_resource_update_path(collection_resource), params: params
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context "when user is signed in as a data coordinator" do
+      let(:user) { create(:user, :data_coordinator) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        patch collection_resource_update_path(collection_resource), params: params
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in as a data provider" do
+      let(:user) { create(:user, :data_provider) }
+
+      before do
+        sign_in user
+      end
+
+      it "returns page not found" do
+        patch collection_resource_update_path(collection_resource), params: params
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+  end
 end
diff --git a/spec/requests/start_controller_spec.rb b/spec/requests/start_controller_spec.rb
index db884d09f..b3846809c 100644
--- a/spec/requests/start_controller_spec.rb
+++ b/spec/requests/start_controller_spec.rb
@@ -324,6 +324,7 @@ RSpec.describe StartController, type: :request do
 
       context "and 2023 collection window is open for editing" do
         before do
+          create(:collection_resource, :additional, year: 2023, log_type: "sales", display_name: "sales additional resource (2023 to 2024)")
           allow(Time).to receive(:now).and_return(Time.zone.local(2024, 4, 1))
         end
 
@@ -337,6 +338,7 @@ RSpec.describe StartController, type: :request do
           expect(page).to have_content("Sales 23/24")
           expect(page).to have_content("Sales 2024 to 2025")
           expect(page).to have_content("Sales 2023 to 2024")
+          expect(page).to have_content("Download the sales additional resource (2023 to 2024)")
         end
       end
 
diff --git a/spec/services/collection_resources_service_spec.rb b/spec/services/collection_resources_service_spec.rb
index 3786b70c9..602e36c38 100644
--- a/spec/services/collection_resources_service_spec.rb
+++ b/spec/services/collection_resources_service_spec.rb
@@ -12,7 +12,7 @@ describe CollectionResourcesService do
     end
 
     it "calls write_file on S3 service" do
-      expect(storage_service).to receive(:write_file).with("2025_26_lettings_paper_form.pdf", some_file)
+      expect(storage_service).to receive(:write_file).with("2025_26_lettings_paper_form.pdf", some_file, content_type: "application/pdf")
       service.upload_collection_resource("2025_26_lettings_paper_form.pdf", some_file)
     end
   end