Move download resources to collection resources controller and refactor routes

app/controllers/collection_resources_controller.rb

@@ -9,4 +9,32 @@ 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)
   end
+
+  def download_mandatory_collection_resource
+    log_type = params[:log_type]
+    year = params[:year].to_i
+    resource_type = params[:resource_type]
+
+    return render_not_found unless resource_for_year_can_be_downloaded?(year)
+
+    resource = MandatoryCollectionResourcesService.generate_resource(log_type, year, resource_type)
+    return render_not_found unless resource
+
+    download_resource(resource.download_filename)
+  end
+
+private
+
+  def download_resource(filename, download_filename)
+    file = CollectionResourcesService.new.get_file(filename)
+    return render_not_found unless file
+
+    send_data(file, disposition: "attachment", filename: download_filename)
+  end
+
+  def resource_for_year_can_be_downloaded?(year)
+    return true if current_user&.support? && editable_collection_resource_years.include?(year)
+
+    displayed_collection_resource_years.include?(year)
+  end
 end

app/controllers/start_controller.rb

@@ -1,5 +1,4 @@
 class StartController < ApplicationController
-  include CollectionTimeHelper
   include CollectionResourcesHelper
 
   def index
@@ -10,57 +9,4 @@ class StartController < ApplicationController
       render "home/index"
     end
   end
-
-  FormHandler.instance.years_of_available_lettings_forms.each do |year|
-    short_underscored_year = "#{year % 100}_#{(year + 1) % 100}"
-    underscored_year = "#{year}_#{(year + 1) % 100}"
-    dasherised_year = underscored_year.dasherize
-
-    define_method "download_#{short_underscored_year}_lettings_form" do
-      download_resource("#{underscored_year}_lettings_paper_form.pdf", "#{dasherised_year}-lettings-paper-form.pdf")
-    end
-
-    define_method "download_#{short_underscored_year}_lettings_bulk_upload_template" do
-      download_resource("bulk-upload-lettings-template-#{dasherised_year}.xlsx", "#{dasherised_year}-lettings-bulk-upload-template.xlsx")
-    end
-
-    define_method "download_#{short_underscored_year}_lettings_bulk_upload_specification" do
-      download_resource("bulk-upload-lettings-specification-#{dasherised_year}.xlsx", "#{dasherised_year}-lettings-bulk-upload-specification.xlsx")
-    end
-  end
-
-  FormHandler.instance.years_of_available_sales_forms.each do |year|
-    short_underscored_year = "#{year % 100}_#{(year + 1) % 100}"
-    underscored_year = "#{year}_#{(year + 1) % 100}"
-    dasherised_year = underscored_year.dasherize
-
-    define_method "download_#{short_underscored_year}_sales_form" do
-      download_resource("#{underscored_year}_sales_paper_form.pdf", "#{dasherised_year}-sales-paper-form.pdf")
-    end
-
-    define_method "download_#{short_underscored_year}_sales_bulk_upload_template" do
-      download_resource("bulk-upload-sales-template-#{dasherised_year}.xlsx", "#{dasherised_year}-sales-bulk-upload-template.xlsx")
-    end
-
-    define_method "download_#{short_underscored_year}_sales_bulk_upload_specification" do
-      download_resource("bulk-upload-sales-specification-#{dasherised_year}.xlsx", "#{dasherised_year}-sales-bulk-upload-specification.xlsx")
-    end
-  end
-
-  def download_23_24_lettings_bulk_upload_legacy_template
-    download_resource("bulk-upload-lettings-legacy-template-2023-24.xlsx", "2023-24-lettings-bulk-upload-legacy-template.xlsx")
-  end
-
-  def download_23_24_sales_bulk_upload_legacy_template
-    download_resource("bulk-upload-sales-legacy-template-2023-24.xlsx", "2023-24-sales-bulk-upload-legacy-template.xlsx")
-  end
-
-private
-
-  def download_resource(filename, download_filename)
-    file = CollectionResourcesService.new.get_file(filename)
-    return render_not_found unless file
-
-    send_data(file, disposition: "attachment", filename: download_filename)
-  end
 end

app/helpers/collection_resources_helper.rb

@@ -30,7 +30,7 @@ module CollectionResourcesHelper
 
   def editable_collection_resource_years
     return [previous_collection_start_year, current_collection_start_year] if FormHandler.instance.in_edit_crossover_period?
-    return [current_collection_start_year, next_collection_start_year] if Time.zone.today >= Time.zone.local(Time.zone.today.year, 1, 1) && Time.zone.today < Time.zone.local(Time.zone.today.year, 4, 1)
+    return [next_collection_start_year, current_collection_start_year] if (Time.zone.today >= Time.zone.local(Time.zone.today.year, 1, 1) && Time.zone.today < Time.zone.local(Time.zone.today.year, 4, 1)) || FeatureToggle.allow_future_resource_updates?
 
     [current_collection_start_year]
   end
@@ -43,23 +43,11 @@ module CollectionResourcesHelper
     "#{year} to #{year + 1}"
   end
 
-  def underscored_file_year_format(year)
-    "#{year}_#{(year + 1) % 100}"
-  end
-
-  def dasherised_file_year_format(year)
-    underscored_file_year_format(year).dasherize
-  end
-
-  def short_underscored_year_range_format(year)
-    "#{year % 100}_#{(year + 1) % 100}"
-  end
-
   def document_list_component_items(resources)
     resources.map do |resource|
       {
         name: "Download the #{resource.display_name}",
-        href: send(resource.download_path),
+        href: resource.download_path,
         metadata: file_type_size_and_pages(resource.download_filename),
       }
     end
@@ -69,9 +57,19 @@ module CollectionResourcesHelper
     resources.map do |resource|
       {
         name: resource.download_filename,
-        href: send(resource.download_path),
+        href: resource.download_path,
         metadata: file_type_size_and_pages(resource.download_filename),
       }
     end
   end
+
+  def file_exists_on_s3?(file)
+    url = "https://#{Rails.application.config.collection_resources_s3_bucket_name}.s3.amazonaws.com/#{file}"
+    uri = URI.parse(url)
+
+    response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |http|
+      http.request_head(uri)
+    end
+    response.is_a?(Net::HTTPSuccess) || response.is_a?(Net::HTTPRedirection)
+  end
 end

app/models/collection_resource.rb

@@ -1,5 +1,10 @@
 class CollectionResource
   include ActiveModel::Model
+  include Rails.application.routes.url_helpers
 
-  attr_accessor :display_name, :short_display_name, :year, :log_type, :download_filename, :download_path
+  attr_accessor :resource_type, :display_name, :short_display_name, :year, :log_type, :download_filename
+
+  def download_path
+    download_mandatory_collection_resource_path(log_type:, year:, resource_type:)
+  end
 end

app/services/feature_toggle.rb

@@ -38,4 +38,8 @@ class FeatureToggle
   def self.local_storage?
     Rails.env.development?
   end
+
+  def self.allow_future_resource_updates?
+    !Rails.env.production? && !Rails.env.test?
+  end
 end

app/services/mandatory_collection_resources_service.rb

@@ -11,17 +11,23 @@ class MandatoryCollectionResourcesService
 
   def self.resources_per_year(year, log_type)
     MANDATORY_RESOURCES.map do |resource|
-      CollectionResource.new(
+      generate_resource(log_type, year, resource)
-        display_name: display_name(resource, year, log_type),
-        short_display_name: resource.humanize,
-        year:,
-        log_type:,
-        download_filename: download_filename(resource, year, log_type),
-        download_path: download_path(resource, year, log_type),
-      )
     end
   end
 
+  def self.generate_resource(log_type, year, resource_type)
+    return unless MANDATORY_RESOURCES.include?(resource_type)
+
+    CollectionResource.new(
+      resource_type:,
+      display_name: display_name(resource_type, year, log_type),
+      short_display_name: resource_type.humanize,
+      year:,
+      log_type:,
+      download_filename: download_filename(resource_type, year, log_type),
+    )
+  end
+
   def self.display_name(resource, year, log_type)
     year_range = "#{year} to #{year + 1}"
     case resource
@@ -34,18 +40,6 @@ class MandatoryCollectionResourcesService
     end
   end
 
-  def self.download_path(resource, year, log_type)
-    year_range = "#{year % 100}_#{(year + 1) % 100}"
-    case resource
-    when "paper_form"
-      "download_#{year_range}_#{log_type}_form_path"
-    when "bulk_upload_template"
-      "download_#{year_range}_#{log_type}_bulk_upload_template_path"
-    when "bulk_upload_specification"
-      "download_#{year_range}_#{log_type}_bulk_upload_specification_path"
-    end
-  end
-
   def self.download_filename(resource, year, log_type)
     year_range = "#{year}_#{(year + 1) % 100}"
     case resource

app/views/collection_resources/_collection_resource_summary_list.erb

@@ -2,13 +2,23 @@
   <% mandatory_resources.each do |resource| %>
     <% summary_list.with_row do |row| %>
       <% row.with_key { resource.short_display_name } %>
-      <% row.with_value do %>
+      <% if file_exists_on_s3?(resource.download_filename) %>
-        <%= render DocumentListComponent.new(items: document_list_edit_component_items([resource]), label: "") %>
+        <% 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 %>
-      <% row.with_action(
-        text: "Change",
-        href: "/",
-      ) %>
     <% end %>
   <% end %>
 <% end %>

app/views/layouts/_collection_resources.html.erb

@@ -23,4 +23,4 @@
   <% end %>
 </div>
 
-<%= govuk_button_link_to "Manage collection resources", collection_resources_path, secondary: true, class: "govuk-!-margin-bottom-2" if current_user.support? %>
+<%= govuk_button_link_to "Manage collection resources", collection_resources_path, secondary: true, class: "govuk-!-margin-bottom-2" if current_user&.support? %>

config/routes.rb

@@ -40,31 +40,15 @@ Rails.application.routes.draw do
   get "/service-moved", to: "maintenance#service_moved"
   get "/service-unavailable", to: "maintenance#service_unavailable"
 
-  get "/download-23-24-lettings-bulk-upload-legacy-template", to: "start#download_23_24_lettings_bulk_upload_legacy_template"
+  get "collection-resources", to: "collection_resources#index"
-  get "/download-23-24-sales-bulk-upload-legacy-template", to: "start#download_23_24_sales_bulk_upload_legacy_template"
+  get "/collection-resources/:log_type/:year/:resource_type/download", to: "collection_resources#download_mandatory_collection_resource", as: :download_mandatory_collection_resource
-
-  FormHandler.instance.years_of_available_lettings_forms.each do |year|
-    short_underscored_year = "#{year % 100}_#{(year + 1) % 100}"
-    short_dasherized_year = short_underscored_year.dasherize
-
-    get "/download-#{short_dasherized_year}-lettings-form", to: "start#download_#{short_underscored_year}_lettings_form"
-    get "/download-#{short_dasherized_year}-lettings-bulk-upload-template", to: "start#download_#{short_underscored_year}_lettings_bulk_upload_template"
-    get "/download-#{short_dasherized_year}-lettings-bulk-upload-specification", to: "start#download_#{short_underscored_year}_lettings_bulk_upload_specification"
-  end
-
-  FormHandler.instance.years_of_available_sales_forms.each do |year|
-    short_underscored_year = "#{year % 100}_#{(year + 1) % 100}"
-    short_dasherized_year = short_underscored_year.dasherize
 
-    get "/download-#{short_dasherized_year}-sales-form", to: "start#download_#{short_underscored_year}_sales_form"
+  resources :collection_resources do
-    get "/download-#{short_dasherized_year}-sales-bulk-upload-template", to: "start#download_#{short_underscored_year}_sales_bulk_upload_template"
+    get "/download", to: "collection_resources#download_additional_collection_resource" # when we get to adding them
-    get "/download-#{short_dasherized_year}-sales-bulk-upload-specification", to: "start#download_#{short_underscored_year}_sales_bulk_upload_specification"
   end
 
   get "clear-filters", to: "sessions#clear_filters"
 
-  get "collection-resources", to: "collection_resources#index"
-
   resource :account, only: %i[show edit], controller: "users" do
     get "edit/password", to: "users#edit_password"
   end

spec/factories/collection_resource.rb

@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :collection_resource, class: "CollectionResource" do
+    resource_type { "paper_form" }
+    display_name { "lettings log for tenants (2021 to 2022)" }
+    short_display_name { "Paper Form" }
+    year { 2024 }
+    log_type { "lettings" }
+    download_filename { "24_25_lettings_paper_form.pdf" }
+  end
+end

spec/helpers/collection_resources_helper_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe CollectionResourcesHelper do
         end
 
         it "returns current and next years" do
-          expect(editable_collection_resource_years).to eq([2024, 2025])
+          expect(editable_collection_resource_years).to match_array([2024, 2025])
         end
       end
 
@@ -106,21 +106,65 @@ RSpec.describe CollectionResourcesHelper do
     end
   end
 
-  describe "#underscored_file_year_format" do
+  describe "#document_list_component_items" do
-    it "returns formatted dasherised file year" do
+    let(:resources) do
-      expect(underscored_file_year_format(2023)).to eq("2023_24")
+      [
+        build(:collection_resource, year: 2023, resource_type: "paper_form", display_name: "lettings log for tenants (2023 to 2024)", download_filename: "2023_24_lettings_paper_form.pdf"),
+        build(:collection_resource, year: 2023, resource_type: "bulk_upload_template", display_name: "bulk upload template (2023 to 2024)", download_filename: "2023_24_lettings_bulk_upload_template.xlsx"),
+      ]
+    end
+
+    before do
+      WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_paper_form.pdf/)
+        .to_return(status: 200, body: "", headers: { "Content-Length" => 292_864, "Content-Type" => "application/pdf" })
+      WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_bulk_upload_template.xlsx/)
+        .to_return(status: 200, body: "", headers: { "Content-Length" => 19_456, "Content-Type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" })
     end
-  end
 
-  describe "#dasherised_file_year_format" do
+    it "returns component items" do
-    it "returns formatted dasherised file year" do
+      expect(document_list_component_items(resources)).to eq([
-      expect(dasherised_file_year_format(2023)).to eq("2023-24")
+        {
+          name: "Download the lettings log for tenants (2023 to 2024)",
+          href: "/collection-resources/lettings/2023/paper_form/download",
+          metadata: "PDF, 286 KB",
+        },
+        {
+          name: "Download the bulk upload template (2023 to 2024)",
+          href: "/collection-resources/lettings/2023/bulk_upload_template/download",
+          metadata: "Microsoft Excel, 19 KB",
+        },
+      ])
     end
   end
 
-  describe "#short_underscored_year_range_format" do
+  describe "#document_list_edit_component_items" do
-    it "returns formatted short underscored year range" do
+    let(:resources) do
-      expect(short_underscored_year_range_format(2023)).to eq("23_24")
+      [
+        build(:collection_resource, year: 2023, resource_type: "paper_form", display_name: "lettings log for tenants (2023 to 2024)", download_filename: "2023_24_lettings_paper_form.pdf"),
+        build(:collection_resource, year: 2023, resource_type: "bulk_upload_template", display_name: "bulk upload template (2023 to 2024)", download_filename: "2023_24_lettings_bulk_upload_template.xlsx"),
+      ]
+    end
+
+    before do
+      WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_paper_form.pdf/)
+        .to_return(status: 200, body: "", headers: { "Content-Length" => 292_864, "Content-Type" => "application/pdf" })
+      WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_bulk_upload_template.xlsx/)
+        .to_return(status: 200, body: "", headers: { "Content-Length" => 19_456, "Content-Type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" })
+    end
+
+    it "returns component items" do
+      expect(document_list_edit_component_items(resources)).to eq([
+        {
+          name: "2023_24_lettings_paper_form.pdf",
+          href: "/collection-resources/lettings/2023/paper_form/download",
+          metadata: "PDF, 286 KB",
+        },
+        {
+          name: "2023_24_lettings_bulk_upload_template.xlsx",
+          href: "/collection-resources/lettings/2023/bulk_upload_template/download",
+          metadata: "Microsoft Excel, 19 KB",
+        },
+      ])
     end
   end
 end

spec/requests/collection_resources_controller_spec.rb

@@ -44,10 +44,11 @@ RSpec.describe CollectionResourcesController, type: :request 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
-        get collection_resources_path
       end
 
       it "displays collection resources" do
+        get collection_resources_path
+
         expect(page).to have_content("Lettings 2024 to 2025")
         expect(page).to have_content("Lettings 2025 to 2026")
         expect(page).to have_content("Sales 2024 to 2025")
@@ -55,12 +56,18 @@ RSpec.describe CollectionResourcesController, type: :request do
       end
 
       it "displays mandatory filed" do
+        get collection_resources_path
+
         expect(page).to have_content("Paper form")
         expect(page).to have_content("Bulk upload template")
         expect(page).to have_content("Bulk upload specification")
       end
 
       context "when files are on S3" do
+        before do
+          get collection_resources_path
+        end
+
         it "displays file names with download links" do
           expect(page).to have_link("2024_25_lettings_paper_form.pdf", href: download_24_25_lettings_form_path)
           expect(page).to have_link("bulk-upload-lettings-template-2024-25.xlsx", href: download_24_25_lettings_bulk_upload_template_path)
@@ -79,6 +86,99 @@ RSpec.describe CollectionResourcesController, type: :request do
 
         it "displays change links" do
           expect(page).to have_selector(:link_or_button, "Change", count: 12)
+          expect(page).to have_link("Change", href: edit_24_25_lettings_form_path)
+        end
+      end
+
+      context "when files are not on S3" do
+        before do
+          WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/)
+          .to_return(status: 404, body: "", headers: {})
+          get collection_resources_path
+        end
+
+        it "displays No file uploaded" do
+          expect(page).to have_content("No file uploaded")
+        end
+
+        it "displays upload links" do
+          expect(page).to have_selector(:link_or_button, "Upload", count: 12)
+        end
+      end
+    end
+  end
+
+  describe "GET #download_mandatory_collection_resource" do
+    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
+      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+      sign_in user
+    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
+          WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/)
+          .to_return(status: 200, body: "file", headers: { "Content-Type" => "application/pdf", "Content-Length" => 1000 })
+          get download_mandatory_collection_resource_path(log_type: "lettings", year: 2025, resource_type: "paper_form")
+        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
+          WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/)
+            .to_return(status: 404, body: "", headers: {})
+          get download_mandatory_collection_resource_path(log_type: "lettings", year: 2024, resource_type: "paper_form")
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+
+      context "when resource isn't a mandatory resources" do
+        before do
+          get download_mandatory_collection_resource_path(log_type: "lettings", year: 2024, resource_type: "invalid_resource")
+        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
+        before do
+          get download_mandatory_collection_resource_path(log_type: "lettings", year: 2026, resource_type: "paper_form")
+        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(:user) { create(:user, :support) }
+
+      context "when year is in editable_collection_resource_years but not in displayed_collection_resource_years" do
+        before do
+          WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/)
+            .to_return(status: 200, body: "file", headers: { "Content-Type" => "application/pdf", "Content-Length" => 1000 })
+          get download_mandatory_collection_resource_path(log_type: "lettings", year: 2026, resource_type: "paper_form")
+        end
+
+        it "downloads the file" do
+          expect(response.status).to eq(200)
+          expect(response.body).to eq("file")
         end
       end
     end

spec/requests/start_controller_spec.rb

@@ -324,7 +324,7 @@ RSpec.describe StartController, type: :request do
 
       context "and 2023 collection window is open for editing" do
         before do
-          allow(Time).to receive(:now).and_return(Time.zone.local(2024, 1, 1))
+          allow(Time).to receive(:now).and_return(Time.zone.local(2024, 4, 1))
         end
 
         it "displays correct resources for 2023/24 and 2024/25 collection years" do

spec/services/mandatory_collection_resources_service_spec.rb

@@ -0,0 +1,42 @@
+require "rails_helper"
+
+describe MandatoryCollectionResourcesService do
+  let(:service) { described_class }
+
+  describe "#generate_resource" do
+    it "returns a CollectionResource object" do
+      resource = service.generate_resource("lettings", 2024, "paper_form")
+      expect(resource).to be_a(CollectionResource)
+    end
+
+    it "returns nil if resource type is not in the MANDATORY_RESOURCES list" do
+      resource = service.generate_resource("lettings", 2024, "invalid_resource")
+      expect(resource).to be_nil
+    end
+
+    it "returns a CollectionResource object with the correct attributes" do
+      resource = service.generate_resource("lettings", 2024, "paper_form")
+      expect(resource.resource_type).to eq("paper_form")
+      expect(resource.display_name).to eq("lettings log for tenants (2024 to 2025)")
+      expect(resource.short_display_name).to eq("Paper form")
+      expect(resource.year).to eq(2024)
+      expect(resource.log_type).to eq("lettings")
+      expect(resource.download_filename).to eq("2024_25_lettings_paper_form.pdf")
+    end
+  end
+
+  describe "#generate_resources" do
+    it "generates all mandatory resources for given years" do
+      resources = service.generate_resources("lettings", [2024, 2025])
+      expect(resources[2024].map(&:resource_type)).to eq(%w[paper_form bulk_upload_template bulk_upload_specification])
+      expect(resources[2025].map(&:resource_type)).to eq(%w[paper_form bulk_upload_template bulk_upload_specification])
+    end
+  end
+
+  describe "#resources_per_year" do
+    it "generates all mandatory resources for a specific year" do
+      resources = service.resources_per_year(2024, "lettings")
+      expect(resources.map(&:resource_type)).to eq(%w[paper_form bulk_upload_template bulk_upload_specification])
+    end
+  end
+end