Rename files on upload (#355)

Author: dcollie2

app/models/topic.rb

@@ -57,6 +57,16 @@ def doc_prefix
     id
   end
 
+  def custom_file_name(document)
+    [
+      id,
+      provider.file_name_prefix.present? ? provider.file_name_prefix.parameterize : provider.name.parameterize(separator: "_"),
+      published_at_year,
+      published_at_month,
+      document.filename.base.parameterize(separator: "_"),
+    ].compact.join("_") + "." + document.filename.extension
+  end
+
   class << self
     def by_year(year)
       where("extract(year from published_at) = ?", year)

app/services/file_manager.rb

@@ -77,21 +77,9 @@ def file_content
     @file_content ||= document.download
   end
 
-  # naming convention described here: https://github.com/rubyforgood/skillrx/issues/305
-  # [topic.id]_[provider.provider_name_for_file]_[topic.published_at_year]_[topic.published_at_month][document.filename.parameterize].[extension]
+  # naming convention now enforced on upload
   def file_name
-    provider = topic.provider
-    @file_name ||= [].tap do |parts|
-      parts << topic.id
-      if provider.file_name_prefix.present?
-        parts << provider.file_name_prefix
-      else
-        parts << provider.name
-      end
-      parts << topic.published_at_year
-      parts << format("%02d", topic.published_at_month)
-      parts << document.filename
-    end.join("_")
+    document.filename.to_s
   end
 
   def language

app/services/topics/mutator.rb

@@ -55,6 +55,7 @@ def mutate
       attach_files(document_signed_ids)
       # topic.persisted? means that we are updating existing topic and documents can be removed
       shadow_delete_documents(docs_to_delete) if topic.persisted?
+      rename_files(document_signed_ids)
       # document_signed_ids.any? means that some new documents were attached and we need to sync them
       sync_docs_for_topic_updates if document_signed_ids.any?
       [ :ok, topic ]
@@ -71,6 +72,30 @@ def attach_files(signed_ids)
     end
   end
 
+  def rename_files(signed_ids)
+    return if signed_ids.blank?
+    signed_ids.each do |signed_id|
+    document = topic.documents.find { |doc| doc.blob.signed_id == signed_id }
+      next unless document
+
+      new_filename = topic.custom_file_name(document)
+      rename_document(document, new_filename)
+    end
+  end
+
+  def rename_document(document, new_filename)
+    return unless document.blob.filename.to_s.split("_").first == "rename"
+    return if document.filename == new_filename
+
+    file_io = StringIO.new(document.download)
+      topic.documents.attach(
+        io: file_io,
+        filename: new_filename,
+        content_type: document.content_type
+      )
+      document.purge
+  end
+
   def sync_docs_for_topic_updates
     topic.documents_attachments.each do |doc|
       DocumentsSyncJob.perform_later(

spec/jobs/documents_sync_job_spec.rb

@@ -4,15 +4,7 @@
   let(:topic) { create(:topic, :with_documents) }
   let(:provider) { topic.provider }
   let(:document) { topic.documents.first }
-  let(:file_name) do
-    [
-      topic.id,
-      provider.file_name_prefix,
-      topic.published_at_year,
-      format("%02d", topic.published_at_month),
-      document.filename,
-    ].join("_")
-  end
+  let(:file_name) { document.filename }
 
   before { provider.update(file_name_prefix: "MyPrefix") }
 

spec/models/topic_spec.rb

@@ -43,4 +43,29 @@
   context "tagging" do
     it_behaves_like "taggable"
   end
+
+  describe "#custom_file_name" do
+    let(:document) { double("Document", filename: ActiveStorage::Filename.new("Document name.pdf")) }
+    let(:topic) { create(:topic, provider: provider, published_at: Time.new(2023, 12, 22)) }
+
+    context "when provider has a file name prefix" do
+      let(:provider) { create(:provider, file_name_prefix: "prefix") }
+
+      it "returns a custom file name based on topic attributes" do
+        expect(topic.custom_file_name(document)).to eq(
+          "#{topic.id}_prefix_2023_12_document_name.pdf"
+        )
+      end
+    end
+
+    context "when provider does not have a file name prefix" do
+      let(:provider) { create(:provider, file_name_prefix: nil, name: "Provider Name") }
+
+      it "returns a custom file name based on topic attributes" do
+        expect(topic.custom_file_name(document)).to eq(
+          "#{topic.id}_provider_name_2023_12_document_name.pdf"
+        )
+      end
+    end
+  end
 end

spec/services/file_manager_spec.rb

@@ -29,14 +29,14 @@
     it "initializes file workers with correct parameters" do
       expect(FileWorker).to receive(:new).with(
         share:,
-        name: "#{topic.id}_MyPrefix_#{topic.published_at_year}_#{format('%02d', topic.published_at_month)}_dummy.pdf",
+        name: "dummy.pdf",
         path: "#{topic.language.file_storage_prefix}CMES-Pi/assets/Content",
         file: document.download,
         new_path: nil,
       ).and_call_original
       expect(FileWorker).to receive(:new).with(
         share:,
-        name: "#{topic.id}_MyPrefix_#{topic.published_at_year}_#{format('%02d', topic.published_at_month)}_dummy.pdf",
+        name: "dummy.pdf",
         path: "#{topic.language.file_storage_prefix}CMES-v2/assets/Content",
         file: document.download,
         new_path: nil,
@@ -64,7 +64,7 @@
       it "initializes file worker with parameters for video" do
         expect(FileWorker).to receive(:new).with(
           share:,
-          name: "#{topic.id}_MyPrefix_#{topic.published_at_year}_#{format('%02d', topic.published_at_month)}_video_file.mp4",
+          name: "video_file.mp4",
           path: "#{topic.language.file_storage_prefix}CMES-v2/assets/VideoContent",
           file: document.download,
           new_path: nil,