Merge pull request #563 from wri/fix/soft-deleting-annex-should-not-remove-file

Fix/soft deleting annex should not remove file

Author: tsubik

app/admin/operator_document_annex.rb

@@ -9,15 +9,6 @@
 
   active_admin_paranoia
 
-  # To include the deleted operator document annexes
-  scope_to do
-    Class.new do
-      def self.operator_document_annexes
-        OperatorDocumentAnnex.unscoped.distinct
-      end
-    end
-  end
-
   controller do
     def scoped_collection
       end_of_association_chain.includes([:user, annex_documents: [documentable: [:operator, required_operator_document: :translations]]])
@@ -45,11 +36,7 @@ def scoped_collection
     :attachment, :uploaded_by
 
   csv do
-    column I18n.t("active_admin.required_operator_document_page.exists") do |annex|
-      annex.deleted_at.nil?
-    end
     column :status
-
     column I18n.t("active_admin.operator_page.documents") do |annex|
       documents = []
       annex.annex_documents.each do |ad|
@@ -60,7 +47,6 @@ def scoped_collection
     column I18n.t("active_admin.dashboard_page.columns.operator") do |annex|
       annex.annex_documents.first&.documentable&.operator&.name
     end
-
     column I18n.t("activerecord.models.user") do |annex|
       annex.user&.name
     end
@@ -71,9 +57,6 @@ def scoped_collection
   end
 
   index do
-    bool_column I18n.t("active_admin.required_operator_document_page.exists") do |od|
-      od.deleted_at.nil?
-    end
     tag_column :status
     column I18n.t("active_admin.operator_page.documents") do |od|
       next if od.annex_document.nil?
@@ -93,17 +76,24 @@ def scoped_collection
       fmu = doc.documentable_type.constantize.unscoped.find(doc.documentable_id).fmu
       link_to(fmu.name, admin_fmu_path(fmu.id)) if fmu
     end
-
     column :user, sortable: "users.name"
     column :expire_date
     column :start_date
     column :created_at
     column :uploaded_by
     column :attachment do |o|
-      link_to o.attachment&.identifier, o.attachment&.url
+      if o.attachment&.identifier.present?
+        name = o.attachment.identifier
+        name += " (Missing file)" if o.attachment.blank?
+        link_to name, o.attachment.url
+      end
+    end
+    if params[:scope] == "archived"
+      column :deleted_at
+    else
+      column(I18n.t("active_admin.approve")) { |annex| link_to I18n.t("active_admin.approve"), approve_admin_operator_document_annex_path(annex), method: :put }
+      column(I18n.t("active_admin.reject")) { |annex| link_to I18n.t("active_admin.reject"), reject_admin_operator_document_annex_path(annex), method: :put }
     end
-    column(I18n.t("active_admin.approve")) { |annex| link_to I18n.t("active_admin.approve"), approve_admin_operator_document_annex_path(annex), method: :put }
-    column(I18n.t("active_admin.reject")) { |annex| link_to I18n.t("active_admin.reject"), reject_admin_operator_document_annex_path(annex), method: :put }
     actions
   end
 

app/models/operator_document_annex.rb

@@ -32,6 +32,9 @@ class OperatorDocumentAnnex < ApplicationRecord
     class_name: "AnnexDocument", inverse_of: :operator_document_annex
   has_many :operator_document_histories, through: :annex_documents_history, source: :documentable, source_type: "OperatorDocumentHistory"
 
+  skip_callback :commit, :after, :remove_attachment!
+  after_real_destroy :remove_attachment!
+
   before_validation(on: :create) do
     self.status = OperatorDocumentAnnex.statuses[:doc_pending]
   end

spec/factories/operator_document_annexes.rb

@@ -21,6 +21,7 @@
     start_date { Date.yesterday }
     expire_date { Date.tomorrow }
     name { "annex name" }
+    attachment { Rack::Test::UploadedFile.new(File.join(Rails.root, "spec", "support", "files", "doc.pdf")) }
 
     transient do
       force_status { nil }

spec/models/operator_document_annex_spec.rb

@@ -25,6 +25,30 @@
     expect(operator_document_annex).to be_valid
   end
 
+  describe "deletion" do
+    let!(:annex) { create(:operator_document_annex) }
+
+    context "when soft deleting record" do
+      it "does not delete the original file" do
+        original_file_path = annex.attachment.file.file
+        expect(File.exist?(original_file_path)).to be true
+        annex.destroy!
+        expect(annex.deleted?).to be true
+        expect(File.exist?(original_file_path)).to be true
+        expect(annex.attachment.file.file).to eq(original_file_path)
+      end
+    end
+
+    context "when hard deleting record" do
+      it "deletes the original file" do
+        original_file_path = annex.attachment.file.file
+        expect(File.exist?(original_file_path)).to be true
+        annex.really_destroy!
+        expect(File.exist?(original_file_path)).to be false
+      end
+    end
+  end
+
   describe "Instance methods" do
     describe "#expire_document_annex" do
       it "set status to doc_expired" do