CLDC-3700 Update csv downloads (#2785)

* Save a csv download record

* Allow downloading CSVs

* Make links expire

* Send correct download link

* Fix world

* Make csv downloads work locally

* Set expiration time on the record

* lint

* Update download urls

* Update expiration time

* Update expired link content

* Add a page to view the download

* lint

* lint again

app/controllers/csv_downloads_controller.rb

@@ -0,0 +1,27 @@
+class CsvDownloadsController < ApplicationController
+  before_action :authenticate_user!
+
+  def show
+    @csv_download = CsvDownload.find(params[:id])
+    authorize @csv_download
+
+    return render "errors/download_link_expired" if @csv_download.expired?
+  end
+
+  def download
+    csv_download = CsvDownload.find(params[:id])
+    authorize csv_download
+
+    return render "errors/download_link_expired" if csv_download.expired?
+
+    downloader = Csv::Downloader.new(csv_download:)
+
+    if Rails.env.development?
+      downloader.call
+      send_file downloader.path, filename: csv_download.filename, type: "text/csv"
+    else
+      presigned_url = downloader.presigned_url
+      redirect_to presigned_url, allow_other_host: true
+    end
+  end
+end

app/jobs/email_csv_job.rb

@@ -1,9 +1,10 @@
 class EmailCsvJob < ApplicationJob
+  include Rails.application.routes.url_helpers
   queue_as :default
 
   BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8
 
-  EXPIRATION_TIME = 24.hours.to_i
+  EXPIRATION_TIME = 48.hours.to_i
 
   def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false, log_type = "lettings", year = nil) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
     export_type = codes_only_export ? "codes" : "labels"
@@ -20,10 +21,16 @@ class EmailCsvJob < ApplicationJob
 
     filename = "#{[log_type, 'logs', organisation&.name, Time.zone.now].compact.join('-')}.csv"
 
-    storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+    storage_service = if FeatureToggle.upload_enabled?
+                        Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+                      else
+                        Storage::LocalDiskService.new
+                      end
+
     storage_service.write_file(filename, BYTE_ORDER_MARK + csv_string)
+    csv_download = CsvDownload.create!(user:, organisation: user.organisation, filename:, download_type: log_type, expiration_time: EXPIRATION_TIME)
 
-    url = storage_service.get_presigned_url(filename, EXPIRATION_TIME)
+    url = csv_download_url(csv_download.id, host: ENV["APP_HOST"])
 
     CsvDownloadMailer.new.send_csv_download_mail(user, url, EXPIRATION_TIME)
   end

app/jobs/scheme_email_csv_job.rb

@@ -1,9 +1,10 @@
 class SchemeEmailCsvJob < ApplicationJob
+  include Rails.application.routes.url_helpers
   queue_as :default
 
   BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8
 
-  EXPIRATION_TIME = 24.hours.to_i
+  EXPIRATION_TIME = 48.hours.to_i
 
   def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, download_type = "combined") # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
     unfiltered_schemes = if organisation.present? && user.support?
@@ -23,10 +24,16 @@ class SchemeEmailCsvJob < ApplicationJob
       filename = "#{['schemes-and-locations', organisation&.name, Time.zone.now].compact.join('-')}.csv"
     end
 
-    storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+    storage_service = if FeatureToggle.upload_enabled?
+                        Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+                      else
+                        Storage::LocalDiskService.new
+                      end
+
     storage_service.write_file(filename, BYTE_ORDER_MARK + csv_string)
+    csv_download = CsvDownload.create!(user:, organisation: user.organisation, filename:, download_type:, expiration_time: EXPIRATION_TIME)
 
-    url = storage_service.get_presigned_url(filename, EXPIRATION_TIME)
+    url = csv_download_url(csv_download.id, host: ENV["APP_HOST"])
 
     CsvDownloadMailer.new.send_csv_download_mail(user, url, EXPIRATION_TIME)
   end

app/models/csv_download.rb

@@ -0,0 +1,10 @@
+class CsvDownload < ApplicationRecord
+  enum download_type: { lettings: "lettings", sales: "sales", schemes: "schemes", locations: "locations", combined: "combined" }
+
+  belongs_to :user
+  belongs_to :organisation
+
+  def expired?
+    created_at < expiration_time.seconds.ago
+  end
+end

app/policies/csv_download_policy.rb

@@ -0,0 +1,16 @@
+class CsvDownloadPolicy
+  attr_reader :current_user, :csv_download
+
+  def initialize(current_user, csv_download)
+    @current_user = current_user
+    @csv_download = csv_download
+  end
+
+  def show?
+    @current_user == @csv_download.user || @current_user.support? || @current_user.organisation == @csv_download.organisation
+  end
+
+  def download?
+    @current_user == @csv_download.user || @current_user.support? || @current_user.organisation == @csv_download.organisation
+  end
+end

app/services/csv/downloader.rb

@@ -0,0 +1,50 @@
+class Csv::Downloader
+  attr_reader :csv_download
+
+  delegate :path, to: :file
+
+  def initialize(csv_download:)
+    @csv_download = csv_download
+  end
+
+  def call
+    download
+  end
+
+  def delete_local_file!
+    file.unlink
+  end
+
+  def presigned_url
+    s3_storage_service.get_presigned_url(csv_download.filename, 60, response_content_disposition: "attachment; filename=#{csv_download.filename}")
+  end
+
+private
+
+  def download
+    io = storage_service.get_file_io(csv_download.filename)
+    file.write(io.read)
+    io.close
+    file.close
+  end
+
+  def file
+    @file ||= Tempfile.new
+  end
+
+  def storage_service
+    @storage_service ||= if FeatureToggle.upload_enabled?
+                           s3_storage_service
+                         else
+                           local_disk_storage_service
+                         end
+  end
+
+  def s3_storage_service
+    Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+  end
+
+  def local_disk_storage_service
+    Storage::LocalDiskService.new
+  end
+end

app/views/csv_downloads/show.html.erb

@@ -0,0 +1,10 @@
+<% title = "Downlaod CSV file" %>
+<% content_for :title, title %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <h1 class="govuk-heading-l govuk-!-margin-bottom-6">You are about to download a CSV file</h1>
+    <p class="govuk-body-m">Filename: <%= @csv_download.filename %></p>
+    <%= govuk_button_link_to "Download CSV", download_csv_download_path(@csv_download) %>
+  </div>
+</div>

app/views/errors/download_link_expired.html.erb

@@ -0,0 +1,8 @@
+<% content_for :title, "This link has expired" %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <h1 class="govuk-heading-xl govuk-!-margin-bottom-6">This link has expired.</h1>
+    <p class="govuk-body-m">Download the logs again to get a new link.</p>
+  </div>
+</div>

config/routes.rb

@@ -382,6 +382,13 @@ Rails.application.routes.draw do
     end
   end
 
+  resources :csv_downloads, path: "csv-downloads" do
+    member do
+      get "/", to: "csv_downloads#show", as: "show"
+      get "download", to: "csv_downloads#download"
+    end
+  end
+
   scope via: :all do
     match "/404", to: "errors#not_found"
     match "/429", to: "errors#too_many_requests", status: 429

db/migrate/20241118104046_add_csv_download_table.rb

@@ -0,0 +1,12 @@
+class AddCsvDownloadTable < ActiveRecord::Migration[7.0]
+  def change
+    create_table :csv_downloads do |t|
+      t.column :download_type, :string
+      t.column :filename, :string
+      t.column :expiration_time, :integer
+      t.timestamps
+      t.references :user
+      t.references :organisation
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_10_31_102744) do
+ActiveRecord::Schema[7.0].define(version: 2024_11_18_104046) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -64,6 +64,18 @@ ActiveRecord::Schema[7.0].define(version: 2024_10_31_102744) do
     t.datetime "discarded_at"
   end
 
+  create_table "csv_downloads", force: :cascade do |t|
+    t.string "download_type"
+    t.string "filename"
+    t.integer "expiration_time"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.bigint "user_id"
+    t.bigint "organisation_id"
+    t.index ["organisation_id"], name: "index_csv_downloads_on_organisation_id"
+    t.index ["user_id"], name: "index_csv_downloads_on_user_id"
+  end
+
   create_table "csv_variable_definitions", force: :cascade do |t|
     t.string "variable", null: false
     t.string "definition", null: false

spec/factories/csv_download.rb

@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :csv_download do
+    download_type { "lettings" }
+    user { create(:user) }
+    organisation { user.organisation }
+    filename { "lettings.csv" }
+    expiration_time { 24.hours.to_i }
+  end
+end

spec/jobs/email_csv_job_spec.rb

@@ -3,8 +3,6 @@ require "rails_helper"
 describe EmailCsvJob do
   include Helpers
 
-  test_url = :test_url
-
   let(:job) { described_class.new }
   let(:user) { FactoryBot.create(:user) }
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -22,7 +20,6 @@ describe EmailCsvJob do
   before do
     allow(Storage::S3Service).to receive(:new).and_return(storage_service)
     allow(storage_service).to receive(:write_file)
-    allow(storage_service).to receive(:get_presigned_url).and_return(test_url)
 
     allow(Csv::SalesLogCsvService).to receive(:new).and_return(sales_log_csv_service)
     allow(sales_log_csv_service).to receive(:prepare_csv).and_return("")
@@ -67,6 +64,16 @@ describe EmailCsvJob do
       expect(lettings_log_csv_service).to receive(:prepare_csv).with(lettings_logs)
       job.perform(user, nil, {}, nil, nil, codes_only_export)
     end
+
+    it "creates a CsvDownload record" do
+      job.perform(user, nil, {}, nil, nil, codes_only_export, "lettings")
+      expect(CsvDownload.count).to eq(1)
+      expect(CsvDownload.first.user).to eq(user)
+      expect(CsvDownload.first.organisation).to eq(user.organisation)
+      expect(CsvDownload.first.filename).to match(/lettings-logs-.*\.csv/)
+      expect(CsvDownload.first.download_type).to eq("lettings")
+      expect(CsvDownload.first.expiration_time).to eq(172_800)
+    end
   end
 
   context "when exporting sales logs" do
@@ -102,10 +109,20 @@ describe EmailCsvJob do
       expect(sales_log_csv_service).to receive(:prepare_csv).with(sales_logs)
       job.perform(user, nil, {}, nil, nil, codes_only_export, "sales")
     end
+
+    it "creates a CsvDownload record" do
+      job.perform(user, nil, {}, nil, nil, codes_only_export, "sales")
+      expect(CsvDownload.count).to eq(1)
+      expect(CsvDownload.first.user).to eq(user)
+      expect(CsvDownload.first.organisation).to eq(user.organisation)
+      expect(CsvDownload.first.filename).to match(/sales-logs-.*\.csv/)
+      expect(CsvDownload.first.download_type).to eq("sales")
+      expect(CsvDownload.first.expiration_time).to eq(172_800)
+    end
   end
 
   it "sends an E-mail with the presigned URL and duration" do
-    expect(mailer).to receive(:send_csv_download_mail).with(user, test_url, instance_of(Integer))
+    expect(mailer).to receive(:send_csv_download_mail).with(user, /csv-downloads/, instance_of(Integer))
     job.perform(user)
   end
 end

spec/jobs/scheme_email_csv_job_spec.rb

@@ -3,10 +3,8 @@ require "rails_helper"
 describe SchemeEmailCsvJob do
   include Helpers
 
-  test_url = :test_url
-
   let(:job) { described_class.new }
-  let(:storage_service) { instance_double(Storage::S3Service, write_file: nil, get_presigned_url: test_url) }
+  let(:storage_service) { instance_double(Storage::S3Service, write_file: nil) }
   let(:mailer) { instance_double(CsvDownloadMailer, send_csv_download_mail: nil) }
   let(:user) { FactoryBot.create(:user) }
 
@@ -53,6 +51,16 @@ describe SchemeEmailCsvJob do
           job.perform(user, nil, {}, nil, nil, download_type)
         end
       end
+
+      it "creates a CsvDownload record" do
+        job.perform(user, nil, {}, nil, nil, download_type)
+        expect(CsvDownload.count).to eq(1)
+        expect(CsvDownload.first.user).to eq(user)
+        expect(CsvDownload.first.organisation).to eq(user.organisation)
+        expect(CsvDownload.first.filename).to match(/schemes-.*\.csv/)
+        expect(CsvDownload.first.download_type).to eq("schemes")
+        expect(CsvDownload.first.expiration_time).to eq(172_800)
+      end
     end
 
     context "when download type locations" do
@@ -62,6 +70,16 @@ describe SchemeEmailCsvJob do
         expect(storage_service).to receive(:write_file).with(/locations-.*\.csv/, anything)
         job.perform(user, nil, {}, nil, nil, download_type)
       end
+
+      it "creates a CsvDownload record" do
+        job.perform(user, nil, {}, nil, nil, download_type)
+        expect(CsvDownload.count).to eq(1)
+        expect(CsvDownload.first.user).to eq(user)
+        expect(CsvDownload.first.organisation).to eq(user.organisation)
+        expect(CsvDownload.first.filename).to match(/locations-.*\.csv/)
+        expect(CsvDownload.first.download_type).to eq("locations")
+        expect(CsvDownload.first.expiration_time).to eq(172_800)
+      end
     end
 
     context "when download type combined" do
@@ -71,6 +89,16 @@ describe SchemeEmailCsvJob do
         expect(storage_service).to receive(:write_file).with(/schemes-and-locations.*\.csv/, anything)
         job.perform(user, nil, {}, nil, nil, download_type)
       end
+
+      it "creates a CsvDownload record" do
+        job.perform(user, nil, {}, nil, nil, download_type)
+        expect(CsvDownload.count).to eq(1)
+        expect(CsvDownload.first.user).to eq(user)
+        expect(CsvDownload.first.organisation).to eq(user.organisation)
+        expect(CsvDownload.first.filename).to match(/schemes-and-locations-.*\.csv/)
+        expect(CsvDownload.first.download_type).to eq("combined")
+        expect(CsvDownload.first.expiration_time).to eq(172_800)
+      end
     end
 
     it "includes the organisation name in the filename when one is provided" do
@@ -117,7 +145,7 @@ describe SchemeEmailCsvJob do
   end
 
   it "sends an E-mail with the presigned URL and duration" do
-    expect(mailer).to receive(:send_csv_download_mail).with(user, test_url, instance_of(Integer))
+    expect(mailer).to receive(:send_csv_download_mail).with(user, /csv-downloads/, instance_of(Integer))
     job.perform(user)
   end
 end

spec/requests/csv_downloads_controller_spec.rb

@@ -0,0 +1,136 @@
+require "rails_helper"
+
+RSpec.describe CsvDownloadsController, type: :request do
+  describe "GET #show" do
+    let(:page) { Capybara::Node::Simple.new(response.body) }
+    let(:csv_user) { create(:user) }
+    let(:csv_download) { create(:csv_download, user: csv_user, organisation: csv_user.organisation) }
+    let(:get_file_io) do
+      io = StringIO.new
+      io.write("hello")
+      io.rewind
+      io
+    end
+    let(:mock_storage_service) { instance_double(Storage::S3Service, get_file_io:, get_presigned_url: "https://example.com") }
+
+    before do
+      allow(Storage::S3Service).to receive(:new).and_return(mock_storage_service)
+    end
+
+    context "when user is not signed in" do
+      it "redirects to sign in page" do
+        get "/csv-downloads/#{csv_download.id}"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when user is signed in" do
+      before do
+        sign_in user
+      end
+
+      context "and the user is from a different organisation" do
+        let(:user) { create(:user) }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}"
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "and is the user who generated the csv" do
+        let(:user) { csv_user }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}"
+        end
+
+        it "allows downloading the csv" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_link("Download CSV", href: "/csv-downloads/#{csv_download.id}/download")
+        end
+      end
+
+      context "and is the user is from the same organisation" do
+        let(:user) { create(:user, organisation: csv_user.organisation) }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}"
+        end
+
+        it "allows downloading the csv" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_link("Download CSV", href: "/csv-downloads/#{csv_download.id}/download")
+        end
+      end
+    end
+  end
+
+  describe "GET #download" do
+    let(:csv_user) { create(:user) }
+    let(:csv_download) { create(:csv_download, user: csv_user, organisation: csv_user.organisation) }
+    let(:get_file_io) do
+      io = StringIO.new
+      io.write("hello")
+      io.rewind
+      io
+    end
+    let(:mock_storage_service) { instance_double(Storage::S3Service, get_file_io:, get_presigned_url: "https://example.com") }
+
+    before do
+      allow(Storage::S3Service).to receive(:new).and_return(mock_storage_service)
+    end
+
+    context "when user is not signed in" do
+      it "redirects to sign in page" do
+        get "/csv-downloads/#{csv_download.id}/download"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when user is signed in" do
+      before do
+        sign_in user
+      end
+
+      context "and the user is from a different organisation" do
+        let(:user) { create(:user) }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}/download"
+        end
+
+        it "returns page not found" do
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "and is the user who generated the csv" do
+        let(:user) { csv_user }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}/download"
+        end
+
+        it "allows downloading the csv" do
+          expect(response).to have_http_status(:found)
+        end
+      end
+
+      context "and is the user is from the same organisation" do
+        let(:user) { create(:user, organisation: csv_user.organisation) }
+
+        before do
+          get "/csv-downloads/#{csv_download.id}/download"
+        end
+
+        it "allows downloading the csv" do
+          expect(response).to have_http_status(:found)
+        end
+      end
+    end
+  end
+end