Solves OP#42368 and OP#49452

Fix the formatting for Activity messages on Attachments and File Links
Guarantees that work package journals will be updated with with the file link data.

app/services/journals/create_service.rb

@@ -428,13 +428,15 @@ def insert_storable_sql
           storages_file_links_journals (
             journal_id,
             file_link_id,
-            link_name
+            link_name,
+            storage_name
           )
         SELECT
           #{id_from_inserted_journal_sql},
           file_links.id,
-          file_links.origin_name
-        FROM file_links
+          file_links.origin_name,
+          storages.name
+        FROM file_links left join storages ON file_links.storage_id = storages.id
         WHERE
           #{only_if_created_sql}
           AND file_links.container_id = :journable_id

config/locales/en.yml

@@ -2915,12 +2915,16 @@ en:
   text_work_packages_destroy_confirmation: "Are you sure you want to delete the selected work package(s)?"
   text_work_packages_ref_in_commit_messages: "Referencing and fixing work packages in commit messages"
   text_journal_added: "%{label} %{value} added"
+  text_journal_attachment_added: "%{label} %{value} added as attachment"
+  text_journal_attachment_deleted: "%{label} %{old} removed as attachment"
   text_journal_changed_plain: "%{label} changed from %{old} %{linebreak}to %{new}"
   text_journal_changed_no_detail: "%{label} updated"
   text_journal_changed_with_diff: "%{label} changed (%{link})"
   text_journal_deleted: "%{label} deleted (%{old})"
   text_journal_deleted_subproject: "%{label} %{old}"
   text_journal_deleted_with_diff: "%{label} deleted (%{link})"
+  text_journal_file_link_added: "%{label} link to %{value} (%{storage}) added"
+  text_journal_file_link_deleted: "%{label} link to %{old} (%{storage}) removed"
   text_journal_of: "%{label} %{value}"
   text_journal_set_to: "%{label} set to %{value}"
   text_journal_set_with_diff: "%{label} set (%{link})"

db/migrate/20230725165505_add_storage_name_to_storages_file_links_journals.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class AddStorageNameToStoragesFileLinksJournals < ActiveRecord::Migration[7.0]
+  def change
+    add_column :storages_file_links_journals, :storage_name, :string
+  end
+end

db/migrate/20230731153909_add_file_link_journals_to_existing_containers.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class AddFileLinkJournalsToExistingContainers < ActiveRecord::Migration[7.0]
+  def up
+    system_user = SystemUser.first
+    containers = Storages::FileLink.includes(:container).map(&:container).uniq
+
+    containers.each do |container|
+      next unless container.class.journaled?
+
+      Journals::CreateService.new(container, system_user).call(notes: "File link activity added by a system update")
+    end
+  end
+
+  def down; end
+end

lib/open_project/journal_formatter/attachment.rb

@@ -44,11 +44,19 @@ def render(key, values, options = { html: true })
       value = format_html_attachment_detail(key.to_s.sub('attachments_', ''), value)
     end
 
-    render_binary_detail_text(label, value, old_value)
+    render_attachment_detail_text(label, value, old_value)
   end
 
   private
 
+  def render_attachment_detail_text(label, value, old_value)
+    if value.blank?
+      I18n.t(:text_journal_attachment_deleted, label:, old: old_value)
+    else
+      I18n.t(:text_journal_attachment_added, label:, value:)
+    end
+  end
+
   def label(_key)
     Attachment.model_name.human
   end

lib/open_project/journal_formatter/file_link.rb

@@ -32,27 +32,64 @@ class OpenProject::JournalFormatter::FileLink < JournalFormatter::Base
   include OpenProject::ObjectLinking
 
   def render(key, values, options = { html: true })
-    id = key.to_s.sub('file_links_', '')
-    label, old_value, value = format_details(id, values)
+    id = key.to_s.sub('file_links_', '').to_i
+    label, old_value, value, storage = format_details(id, values)
 
     if options[:html]
-      label, old_value, value = *format_html_details(label, old_value, value)
+      label, old_value, value = format_html_details(label, old_value, value)
       value = format_html_file_link_detail(id, value)
     end
 
-    render_binary_detail_text(label, value, old_value)
+    render_file_link_detail_text(label, value, old_value, storage)
   end
 
   private
 
+  def format_details(id, values)
+    old_value, current_value = values
+    [
+      label(nil),
+      old_value&.fetch('link_name'),
+      current_value&.fetch('link_name'),
+      storage(id, old_value, current_value)
+    ]
+  end
+
+  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('storages.unknown_storage')
+  end
+
+  def render_file_link_detail_text(label, value, old_value, storage)
+    if value.blank?
+      I18n.t(:text_journal_file_link_deleted, label:, old: old_value, storage:)
+    else
+      I18n.t(:text_journal_file_link_added, label:, value:, storage:)
+    end
+  end
+
   # Based this off the Attachment formatter. Not sure if it is the best approach
-  def label(_key) = Storages::FileLink.model_name.human
+  def label(_key) = I18n.t('activerecord.models.file_link')
+
+  def file_link_for(key, value)
+    value.present? && ::Storages::FileLink.find_by(id: key)
+  end
 
   def format_html_file_link_detail(key, value)
-    if value.present? && file_link = ::Storages::FileLink.find_by(id: key.to_i)
+    if file_link = file_link_for(key, value)
       link_to_file_link(file_link, only_path: false)
     elsif value.present?
       value
     end
   end
+
+  def link_to_file_link(file_link, options = {})
+    text = options.delete(:text) || file_link.origin_name
+
+    link_to text,
+            url_to_file_link(file_link, only_path: options.delete(:only_path) { true }),
+            options
+  end
 end

lib/open_project/object_linking.rb

@@ -79,14 +79,6 @@ def link_to_attachment(attachment, options = {})
               options
     end
 
-    def link_to_file_link(file_link, options = {})
-      text = options.delete(:text) || file_link.origin_name
-
-      link_to text,
-              url_to_file_link(file_link, only_path: options.delete(:only_path) { true }),
-              options
-    end
-
     # Generates a link to a SCM revision
     # Options:
     # * :text - Link text (default to the formatted revision)

lib_static/plugins/acts_as_journalized/lib/acts/journalized/file_link_journal_differ.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module Acts::Journalized
+  module FileLinkJournalDiffer
+    class << self
+      def get_changes_to_file_links(predecessor, storable_journals)
+        if predecessor.nil?
+          storable_journals.each_with_object({}) do |journal, hash|
+            change_key = "file_links_#{journal.file_link_id}"
+            new_values = { link_name: journal.link_name, storage_name: journal.storage_name }
+            hash[change_key] = [nil, new_values]
+          end
+        else
+          current_storables = storable_journals.map(&:attributes)
+          previous_storables = predecessor.storable_journals.map(&:attributes)
+
+          changes_on_file_links(previous_storables, current_storables)
+        end
+      end
+
+      def changes_on_file_links(previous, current)
+        ids = all_file_link_ids(previous, current)
+
+        cleanup_changes(
+          pair_changes(ids, previous, current)
+        ).transform_keys! { |key| "file_links_#{key}" }
+      end
+
+      def all_file_link_ids(previous, current)
+        current.pluck('file_link_id') | previous.pluck('file_link_id')
+      end
+
+      def cleanup_changes(changes) = changes.reject { |_, (first, last)| first == last }
+
+      def pair_changes(ids, previous, current)
+        ids.index_with do |id|
+          [select_journals(previous.select { |attributes| attributes['file_link_id'] == id }),
+           select_journals(current.select { |attributes| attributes['file_link_id'] == id })]
+        end
+      end
+
+      def select_journals(journals)
+        return if journals.empty?
+
+        journals.sort.map { |hash| hash.slice('link_name', 'storage_name') }.last
+      end
+    end
+  end
+end

lib_static/plugins/acts_as_journalized/lib/journal_changes.rb

@@ -63,13 +63,9 @@ def get_changes
 
     if has_file_links?
       @changes.merge!(
-        ::Acts::Journalized::JournableDiffer.association_changes(
+        ::Acts::Journalized::FileLinkJournalDiffer.get_changes_to_file_links(
           predecessor,
-          self,
-          'storable_journals',
-          'file_links',
-          :file_link_id,
-          :link_name
+          storable_journals
         )
       )
     end

modules/storages/config/locales/en.yml

@@ -18,7 +18,7 @@ en:
 
   activerecord:
     models:
-      file_link: "File link"
+      file_link: "File"
       storages/storage: "Storage"
     attributes:
       storages/storage:
@@ -58,6 +58,7 @@ en:
       too_many_elements_created_at_once: "Too many elements created at once. Expected %{max} at most, got %{actual}."
 
   storages:
+    unknown_storage: "Unknown storage"
     buttons:
       done_continue_setup: "Done. Continue setup"
       done_complete_setup: "Done, complete setup"