Allow updating additional resources

app/controllers/collection_resources_controller.rb

@@ -34,7 +34,7 @@ class CollectionResourcesController < ApplicationController
     download_resource(resource.download_filename)
   end
 
-  def edit
+  def edit_mandatory_collection_resource
     return render_not_found unless current_user.support?
 
     year = params[:year].to_i
@@ -50,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
@@ -80,6 +91,35 @@ 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.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)})"
+    @collection_resource.validate_attached_file
+    return render "collection_resources/edit" if @collection_resource.errors.any?
+
+    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?
 

app/views/collection_resources/_collection_resource_summary_list.erb

@@ -26,23 +26,13 @@
       <% 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 %>
-            <% row.with_value do %>
+            <%= render DocumentListComponent.new(items: document_list_edit_component_items([resource]), label: "") %>
-              <%= 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 %>
+          <% row.with_action(
+            text: "Change",
+            href: collection_resource_edit_path(resource),
+          ) %>
         <% end %>
       <% end %>
     <% end %>

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,6 +21,15 @@
 
       <%= 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 %>
+
+      <% unless @collection_resource.mandatory %>
+        <%= 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.</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)})”.".html_safe } %>
+      <% end %>
 
       <%= f.govuk_submit resource_exists ? "Save changes" : "Upload" %>
       <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %>

app/views/collection_resources/new.html.erb

@@ -23,7 +23,7 @@
 
       <%= 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)”." } %>
+                      hint: { text: "This will be used in the download link on the homepage. Do not include the log type or collection year.</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)})”.".html_safe } %>
 
       <%= f.govuk_submit "Add resource" %>
       <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %>

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
+  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", as: :update_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"

spec/factories/collection_resource.rb

@@ -8,8 +8,8 @@ FactoryBot.define do
     download_filename { "24_25_lettings_paper_form.pdf" }
     trait(:additional) do
       resource_type { nil }
-      display_name { "additional resource" }
+      display_name { "lettings additional resource (2024 to 2025)" }
-      short_display_name { nil }
+      short_display_name { "additional resource" }
       year { 2024 }
       log_type { "lettings" }
       download_filename { "additional.pdf" }

spec/features/collection_resources_spec.rb

@@ -170,7 +170,7 @@ RSpec.describe "Collection resources" do
   end
 
   context "when uploading an additional resource" do
-    it "only allows excel files for lettings" do
+    it "allows valid files" do
       expect(CollectionResource.count).to eq(0)
 
       visit(new_collection_resource_path(year: 2025, log_type: "sales"))
@@ -206,4 +206,36 @@ RSpec.describe "Collection resources" do
       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

spec/requests/collection_resources_controller_spec.rb

@@ -122,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
@@ -676,4 +686,113 @@ RSpec.describe CollectionResourcesController, type: :request do
       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