From 700f434b2e224f09af14f877727244e65890af90 Mon Sep 17 00:00:00 2001 From: Kat Date: Fri, 11 Oct 2024 13:58:00 +0100 Subject: [PATCH] Allow deleting resources --- .../collection_resources_controller.rb | 17 ++++- app/controllers/start_controller.rb | 4 +- app/models/collection_resource.rb | 7 ++ app/services/collection_resources_service.rb | 4 ++ app/services/storage/local_disk_service.rb | 6 ++ app/services/storage/s3_service.rb | 4 ++ app/services/storage/storage_service.rb | 8 +++ config/routes.rb | 1 + db/migrate/20241011112158_add_discarded_at.rb | 5 ++ db/schema.rb | 3 +- .../collection_resources_controller_spec.rb | 64 ++++++++++++++++++- 11 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20241011112158_add_discarded_at.rb diff --git a/app/controllers/collection_resources_controller.rb b/app/controllers/collection_resources_controller.rb index 1a9832a06..7d0a18652 100644 --- a/app/controllers/collection_resources_controller.rb +++ b/app/controllers/collection_resources_controller.rb @@ -8,8 +8,8 @@ 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) - @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) + @additional_lettings_collection_resources_per_year = CollectionResource.visible.where(log_type: "lettings", mandatory: false).group_by(&:year) + @additional_sales_collection_resources_per_year = CollectionResource.visible.where(log_type: "sales", mandatory: false).group_by(&:year) end def download_mandatory_collection_resource @@ -194,6 +194,19 @@ class CollectionResourcesController < ApplicationController render "collection_resources/delete_confirmation" end + def delete + 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 + + @collection_resource.discard! + + flash[:notice] = "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name.downcase} has been deleted." + redirect_to collection_resources_path + end + private def resource_params diff --git a/app/controllers/start_controller.rb b/app/controllers/start_controller.rb index b65da4d44..d6df81c39 100644 --- a/app/controllers/start_controller.rb +++ b/app/controllers/start_controller.rb @@ -4,8 +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) + @additional_lettings_collection_resources_per_year = CollectionResource.visible.where(log_type: "lettings", mandatory: false, year: displayed_collection_resource_years).group_by(&:year) + @additional_sales_collection_resources_per_year = CollectionResource.visible.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/models/collection_resource.rb b/app/models/collection_resource.rb index cc217b8cd..fc5808cec 100644 --- a/app/models/collection_resource.rb +++ b/app/models/collection_resource.rb @@ -1,8 +1,10 @@ class CollectionResource < ApplicationRecord include Rails.application.routes.url_helpers + has_paper_trail attr_accessor :file + scope :visible, -> { where(discarded_at: nil) } validates :short_display_name, presence: true def download_path @@ -35,4 +37,9 @@ class CollectionResource < ApplicationRecord def validate_short_display_name errors.add(:short_display_name, :blank) if short_display_name.blank? end + + def discard! + CollectionResourcesService.new.delete_collection_resource(download_filename) + update!(discarded_at: Time.zone.now) + end end diff --git a/app/services/collection_resources_service.rb b/app/services/collection_resources_service.rb index 82247f7a0..f6762116a 100644 --- a/app/services/collection_resources_service.rb +++ b/app/services/collection_resources_service.rb @@ -27,4 +27,8 @@ class CollectionResourcesService content_type = MiniMime.lookup_by_filename(filename)&.content_type @storage_service.write_file(filename, file, content_type:) end + + def delete_collection_resource(filename) + @storage_service.delete_file(filename) + end end diff --git a/app/services/storage/local_disk_service.rb b/app/services/storage/local_disk_service.rb index ad3cc9608..228f0339e 100644 --- a/app/services/storage/local_disk_service.rb +++ b/app/services/storage/local_disk_service.rb @@ -43,5 +43,11 @@ module Storage File.exist?(path) end + + def delete_file(filename) + path = Rails.root.join("tmp/storage", filename) + + File.delete(path) + end end end diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index 2e8daa719..a6eef7d49 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -64,6 +64,10 @@ module Storage false end + def delete_file(file_name) + @client.delete_object(bucket: @configuration.bucket_name, key: file_name) + end + private def create_configuration diff --git a/app/services/storage/storage_service.rb b/app/services/storage/storage_service.rb index afb3d4a58..37d4cc0fd 100644 --- a/app/services/storage/storage_service.rb +++ b/app/services/storage/storage_service.rb @@ -15,5 +15,13 @@ module Storage def write_file(_file_name, _data) raise NotImplementedError end + + def get_file(_file_name, _data) + raise NotImplementedError + end + + def delete_file(_file_name, _data) + raise NotImplementedError + end end end diff --git a/config/routes.rb b/config/routes.rb index e451ae034..6ac7b3f34 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,6 +52,7 @@ Rails.application.routes.draw do get "/edit", to: "collection_resources#edit_additional_collection_resource" patch "/update", to: "collection_resources#update_additional_collection_resource" get "/delete-confirmation", to: "collection_resources#delete_confirmation" + delete "/delete", to: "collection_resources#delete" end get "clear-filters", to: "sessions#clear_filters" diff --git a/db/migrate/20241011112158_add_discarded_at.rb b/db/migrate/20241011112158_add_discarded_at.rb new file mode 100644 index 000000000..d910b8797 --- /dev/null +++ b/db/migrate/20241011112158_add_discarded_at.rb @@ -0,0 +1,5 @@ +class AddDiscardedAt < ActiveRecord::Migration[7.0] + def change + add_column :collection_resources, :discarded_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 1f5cca5ca..84bf048af 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_08_100119) do +ActiveRecord::Schema[7.0].define(version: 2024_10_11_112158) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -61,6 +61,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_10_08_100119) do t.boolean "released_to_user" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.datetime "discarded_at" end create_table "csv_variable_definitions", force: :cascade do |t| diff --git a/spec/requests/collection_resources_controller_spec.rb b/spec/requests/collection_resources_controller_spec.rb index fe756d9e0..5bf1d2843 100644 --- a/spec/requests/collection_resources_controller_spec.rb +++ b/spec/requests/collection_resources_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe CollectionResourcesController, type: :request do let(:page) { Capybara::Node::Simple.new(response.body) } - let(:storage_service) { instance_double(Storage::S3Service, get_file_metadata: nil) } + let(:storage_service) { instance_double(Storage::S3Service, get_file_metadata: nil, delete_file: nil) } before do allow(Storage::S3Service).to receive(:new).and_return(storage_service) @@ -874,4 +874,66 @@ RSpec.describe CollectionResourcesController, type: :request do end end end + + describe "DELETE #collection_resource_delete" 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 + delete collection_resource_delete_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 + delete collection_resource_delete_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 + delete collection_resource_delete_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(storage_service).to receive(:file_exists?).and_return(true) + 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 + it "displays delete confirmation page content" do + expect(CollectionResource.visible.count).to eq(1) + delete collection_resource_delete_path(collection_resource) + + expect(CollectionResource.count).to eq(1) + expect(CollectionResource.visible.count).to eq(0) + expect(response).to redirect_to(collection_resources_path) + expect(storage_service).to have_received(:delete_file).with(collection_resource.download_filename) + follow_redirect! + expect(page).to have_content("The sales 2025 to 2026 additional resource has been deleted.") + end + end + end + end end