Merge pull request #16630 from opf/bug/file-link-journal-issues

Ignore FileLink renames when rendering activities

lib/open_project/journal_formatter/file_link.rb

@@ -35,6 +35,8 @@ def render(key, values, options = { html: true })
     id = key.to_s.sub("file_links_", "").to_i
     label, old_value, value, storage = format_details(id, values)
 
+    return if old_value && value
+
     if options[:html]
       label, old_value, value = format_html_details(label, old_value, value)
       value = format_html_file_link_detail(id, value)
@@ -57,11 +59,10 @@ def format_details(id, values)
 
   def storage(id, old, current)
     non_nil_value = [old, current].compact.first
-    file_link = file_link_for(id, non_nil_value)
 
-    non_nil_value["storage_name"] ||
-    file_link&.storage&.name ||
-    I18n.t("unknown_storage", scope: "my_account.access_tokens.storages")
+    storage_name(id, non_nil_value) ||
+      non_nil_value["storage_name"] ||
+      I18n.t("unknown_storage", scope: "my_account.access_tokens.storages")
   end
 
   def render_file_link_detail_text(label, value, old_value, storage)
@@ -75,8 +76,12 @@ def render_file_link_detail_text(label, value, old_value, storage)
   # Based this off the Attachment formatter. Not sure if it is the best approach
   def label(_key) = I18n.t("activerecord.models.file_link")
 
+  def storage_name(id, value)
+    file_link_for(id, value)&.storage&.name
+  end
+
   def file_link_for(key, value)
-    value.present? && ::Storages::FileLink.find_by(id: key)
+    value.present? && ::Storages::FileLink.includes(:storage).find_by(id: key)
   end
 
   def format_html_file_link_detail(key, value)

spec/lib/journal_formatter/file_link_spec.rb

@@ -41,6 +41,16 @@
   subject(:instance) { described_class.new(journal) }
 
   describe "#render" do
+    context "having both a predecessor and a current value" do
+      let(:old) do
+        { "link_name" => "this_is_the_old_file_name.jiff", "storage_name" => nil }
+      end
+
+      it "does not render anything" do
+        expect(instance.render(key, [old, changes])).to be_nil
+      end
+    end
+
     context "having the origin_name as nil" do
       let(:changes) { { "link_name" => file_link.origin_name, "storage_name" => nil } }