Merge branch 'feature/track-file-downloads' into staging

Author: tsubik

app/controllers/private_uploads_controller.rb

@@ -32,7 +32,7 @@ def authenticate_user!
     end
   end
 
-  def allowed_filepath
+  def allowed_directory
     return File.join(Rails.root, "tmp", "private", "uploads") if Rails.env.test?
 
     File.join(Rails.root, "private", "uploads")

app/controllers/uploads_controller.rb

@@ -73,8 +73,9 @@ def ensure_valid_db_record
   end
 
   def allowed_models
-    Dir.entries(Rails.root.join("uploads"))
-      .select { |entry| File.directory?(Rails.root.join("uploads", entry)) }
+    uploads_root = ApplicationUploader.new.public_root.join("uploads")
+    Dir.entries(uploads_root)
+      .select { |entry| File.directory?(uploads_root.join(entry)) }
       .reject { |entry| entry.start_with?(".") || entry == "tmp" }
       .map { |entry| MODELS_OVERRIDES[entry.downcase] || entry }
   end

app/models/concerns/moveable_attachment.rb

@@ -12,8 +12,10 @@ module MoveableAttachment
         return if uploader.nil?
         return if uploader.file.nil?
 
-        from = File.dirname(uploader.file.file.gsub("/public/", "/private/"))
-        to = File.dirname(from.gsub("/private/", "/public/"))
+        public_uploads = uploader.public_root.join("uploads").to_s
+        private_uploads = uploader.private_root.join("uploads").to_s
+        from = File.dirname(uploader.file.file.gsub(public_uploads, private_uploads))
+        to = File.dirname(from.gsub(private_uploads, public_uploads))
         move_attachment(from, to)
       end
 
@@ -22,8 +24,10 @@ module MoveableAttachment
         return if uploader.nil?
         return if uploader.file.nil?
 
-        from = File.dirname(uploader.file.file.gsub("/private/", "/public/"))
-        to = File.dirname(from.gsub("/public/", "/private/"))
+        public_uploads = uploader.public_root.join("uploads").to_s
+        private_uploads = uploader.private_root.join("uploads").to_s
+        from = File.dirname(uploader.file.file.gsub(private_uploads, public_uploads))
+        to = File.dirname(from.gsub(public_uploads, private_uploads))
         move_attachment(from, to)
       end
     end

app/models/gov_document.rb

@@ -80,9 +80,8 @@ def has_data?
   end
 
   def reset_to_not_provided!
-    remove_attachment!
     update!(
-      status: OperatorDocument.statuses[:doc_not_provided],
+      status: OperatorDocument.statuses[:doc_not_provided], attachment: nil,
       expire_date: nil, start_date: nil, uploaded_by: nil, user_id: nil,
       value: nil, link: nil
     )
@@ -102,8 +101,8 @@ def move_previous_attachment_to_private_directory
     previous_attachment_filename = previous_changes[:attachment][0]
     return if previous_attachment_filename.blank?
 
-    from = File.join(attachment.root, attachment.store_dir, previous_attachment_filename)
-    to = from.gsub("/public/", "/private/")
+    from = File.join(attachment.public_root, attachment.store_dir, previous_attachment_filename)
+    to = File.join(attachment.private_root, attachment.store_dir, previous_attachment_filename)
     FileUtils.makedirs(File.dirname(to))
     system "mv #{Shellwords.escape(from)} #{Shellwords.escape(to)}"
   end
@@ -113,9 +112,9 @@ def move_attachment_to_public_directory
     attachment_attr = self[:attachment]
     return if attachment_attr.nil?
 
-    to = File.join(attachment.root, attachment.store_dir, attachment_attr)
-    from = to.gsub("/public/", "/private/")
-    FileUtils.makedirs(File.dirname(from))
+    from = File.join(attachment.private_root, attachment.store_dir, attachment_attr)
+    to = File.join(attachment.public_root, attachment.store_dir, attachment_attr)
+    FileUtils.makedirs(File.dirname(to))
     system "mv #{Shellwords.escape(from)} #{Shellwords.escape(to)}"
   end
 

app/uploaders/application_uploader.rb

@@ -4,10 +4,9 @@ class ApplicationUploader < CarrierWave::Uploader::Base
   storage :file
 
   def root
-    result = Rails.root
-    result = Rails.root.join("private") if private_upload?
-    result = result.to_s.gsub(Rails.root.to_s, Rails.root.join("tmp").to_s) if Rails.env.test?
-    result
+    return private_root if private_upload?
+
+    public_root
   end
 
   def cache_dir
@@ -34,6 +33,18 @@ def private_upload?
     false
   end
 
+  def public_root
+    return Rails.root.join("tmp") if Rails.env.test?
+
+    Rails.root
+  end
+
+  def private_root
+    return Rails.root.join("tmp", "private") if Rails.env.test?
+
+    Rails.root.join("private")
+  end
+
   def original_filename
     if file.present?
       file.filename

spec/integration/uploads_spec.rb

@@ -5,6 +5,7 @@
 RSpec.describe UploadsController, type: :request do
   before(:all) do
     @etc_dir = Rails.root.join("tmp", "etc")
+    FileUtils.mkdir_p(ApplicationUploader.new.public_root.join("uploads"))
     FileUtils.mkdir_p(@etc_dir)
 
     @observation_report = create(:observation_report)

spec/models/gov_document_spec.rb

@@ -28,25 +28,29 @@
       context "when removing attachment" do
         it "moves previous attachment to private folder" do
           expect(document.attachment.file.file).to match(Rails.root.join("tmp/uploads").to_s)
-          document.remove_attachment!
+          expect(document.attachment.file.exists?).to be(true)
+          document.attachment = nil
           document.save!
           document.reload
           expect(document.read_attribute(:attachment)).to be_nil
           prev_version = document.versions.last.reify
           expect(prev_version.attachment.file.file).to match("/private/uploads")
+          expect(prev_version.attachment.file.exists?).to be(true)
         end
       end
 
       context "when changing attachment" do
         it "moves previous attachment to private folder" do
           expect(document.attachment.file.file).to match(Rails.root.join("tmp/uploads").to_s)
+          expect(document.attachment.file.exists?).to be(true)
           document.attachment = Rack::Test::UploadedFile.new(File.join(Rails.root, "spec", "support", "files", "doc.pdf"))
           document.save!
           document.reload
           expect(document.read_attribute(:attachment)).to match(".pdf")
           prev_version = document.versions.last.reify
           expect(prev_version.attachment.file.file).to match("/private/uploads")
           expect(prev_version.attachment.file.file).to match(".png")
+          expect(prev_version.attachment.file.exists?).to be(true)
         end
       end
     end
@@ -98,6 +102,7 @@
           document.destroy!
           document.reload
           expect(document.attachment.file.file).to match("/private/uploads")
+          expect(document.attachment.file.exists?).to be(true)
         end
       end
 
@@ -112,12 +117,15 @@
 
         it "moves attachment back to public directory" do
           expect(document.attachment.file.file).to match("/private/uploads")
+          expect(document.attachment.file.exists?).to be(true)
           document.restore
           reloaded_doc = GovDocument.find(document.id) # as reload does not reload paper_trail.live? weird
           expect(reloaded_doc.attachment.file.file).to match(Rails.root.join("tmp/uploads").to_s)
+          expect(reloaded_doc.attachment.file.exists?).to be(true)
           # first version should stay in private directory
           prev_version = reloaded_doc.versions[-2].reify
           expect(prev_version.attachment.file.file).to match("/private/uploads")
+          expect(prev_version.attachment.file.exists?).to be(true)
         end
       end
     end

spec/models/observation_document_spec.rb

@@ -36,6 +36,7 @@
         report.destroy!
         report.reload
         expect(report.attachment.file.file).to match("/private/uploads")
+        expect(report.attachment.file.exists?).to be(true)
       end
     end
 
@@ -50,6 +51,7 @@
         report.restore
         report.reload
         expect(report.attachment.file.file).to match(Rails.root.join("tmp/uploads").to_s)
+        expect(report.attachment.file.exists?).to be(true)
       end
     end
   end

spec/models/observation_report_spec.rb

@@ -83,6 +83,7 @@
         report.destroy!
         report.reload
         expect(report.attachment.file.file).to match("/private/uploads")
+        expect(report.attachment.file.exists?).to be(true)
       end
     end
 
@@ -97,6 +98,7 @@
         report.restore
         report.reload
         expect(report.attachment.file.file).to match(Rails.root.join("tmp/uploads").to_s)
+        expect(report.attachment.file.exists?).to be(true)
       end
     end
   end