diff --git a/app/views/errors/download_link_expired.html.erb b/app/views/errors/download_link_expired.html.erb
new file mode 100644
index 000000000..7477ee6b3
--- /dev/null
+++ b/app/views/errors/download_link_expired.html.erb
@@ -0,0 +1,8 @@
+<% content_for :title, "This link has expired" %>
+
+
+
+
This link has expired.
+
Download the logs again to get a new link.
+
+
diff --git a/config/routes.rb b/config/routes.rb
index 6ac7b3f34..1c7af8c59 100644
--- a/config/routes.rb
+++ b/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
diff --git a/db/migrate/20241118104046_add_csv_download_table.rb b/db/migrate/20241118104046_add_csv_download_table.rb
new file mode 100644
index 000000000..9b4f73f0b
--- /dev/null
+++ b/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
diff --git a/db/schema.rb b/db/schema.rb
index ef635628c..5b6ddacdb 100644
--- a/db/schema.rb
+++ b/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
diff --git a/spec/factories/csv_download.rb b/spec/factories/csv_download.rb
new file mode 100644
index 000000000..415f69de6
--- /dev/null
+++ b/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
diff --git a/spec/jobs/email_csv_job_spec.rb b/spec/jobs/email_csv_job_spec.rb
index 8e8a027ea..59cf7bf7d 100644
--- a/spec/jobs/email_csv_job_spec.rb
+++ b/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
diff --git a/spec/jobs/scheme_email_csv_job_spec.rb b/spec/jobs/scheme_email_csv_job_spec.rb
index 5ddaa91a6..efb75b698 100644
--- a/spec/jobs/scheme_email_csv_job_spec.rb
+++ b/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
diff --git a/spec/requests/csv_downloads_controller_spec.rb b/spec/requests/csv_downloads_controller_spec.rb
new file mode 100644
index 000000000..982077a12
--- /dev/null
+++ b/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