[COMMS-1067] Journal admin deletion of a label

app/models/journal.rb

@@ -89,6 +89,8 @@ class Journal < ApplicationRecord
                    status_name
                    status_id
                    status_changes
+                   label_ids
+                   label_names
                    meeting_id
                    source_meeting_id
                    message_id
@@ -107,6 +109,7 @@ class Journal < ApplicationRecord
     default_attribute_written
     forum_message
     import
+    label_deleted
     progress_mode_changed_to_status_based
     status_changed
     system_update

app/models/journal/caused_by_default_attribute_written.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 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, see <https://www.gnu.org/licenses/>.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class Journal::CausedByDefaultAttributeWritten < CauseOfChange::Base
+  def initialize
+    super("default_attribute_written")
+  end
+end

app/models/journal/caused_by_label_deletion.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 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, see <https://www.gnu.org/licenses/>.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class Journal::CausedByLabelDeletion < CauseOfChange::Base
+  def initialize(label_ids:, label_names:)
+    super("label_deleted", "label_ids" => label_ids, "label_names" => label_names)
+  end
+
+  # Labels deleted in quick succession by the same user are merged into the same cause
+  def merge_into(predecessor_cause)
+    return unless label_deletion?(predecessor_cause)
+
+    merged = names_by_id(predecessor_cause).merge(names_by_id(to_hash))
+
+    self.class.new(label_ids: merged.keys, label_names: merged.values)
+  end
+
+  private
+
+  def label_deletion?(cause)
+    cause["type"] == type
+  end
+
+  def names_by_id(cause)
+    cause["label_ids"].zip(cause["label_names"]).to_h
+  end
+end

app/services/journals/create_service.rb

@@ -70,7 +70,7 @@ def call(notes: "", internal: false, cause: CauseOfChange::NoCause.new)
     #
     # * The journalizing happens within the configured aggregation time
     # * The journalizing is carried out by the same user
-    # * The cause is identical
+    # * The cause is identical or extends the predecessor's cause
     # * ONLY one note exists between the predecessor and the journal to be created
     # * The predecessor and the journal to be created have the same restriction
     #
@@ -84,6 +84,7 @@ def aggregatable_predecessor(notes, internal, cause)
 
     def create_journal(notes, internal, cause)
       predecessor = aggregatable_predecessor(notes, internal, cause)
+      cause = aggregate_cause(predecessor, cause)
 
       log_journal_creation(predecessor)
 
@@ -611,7 +612,13 @@ def same_user?(predecessor)
     end
 
     def only_one_or_same_cause?(predecessor, cause)
-      predecessor.cause.empty? || predecessor.cause == cause
+      predecessor.cause.empty? || predecessor.cause == cause || cause.mergeable_into?(predecessor.cause)
+    end
+
+    def aggregate_cause(predecessor, cause)
+      return cause unless predecessor
+
+      cause.merge_into(predecessor.cause) || cause
     end
 
     def only_one_note?(predecessor, notes)

app/services/labels/delete_service.rb

@@ -29,5 +29,24 @@
 
 module Labels
   class DeleteService < ::BaseServices::Delete
+    protected
+
+    def persist(service_result)
+      ids = labeled_work_package_ids
+      super.tap { |result| enqueue_journalization(ids) if result.success? && ids.any? }
+    end
+
+    private
+
+    def labeled_work_package_ids
+      Labeling.where(label_id: model.id, labelable_type: WorkPackage.name).pluck(:labelable_id)
+    end
+
+    def enqueue_journalization(work_package_ids)
+      Labels::JournalizeDeletionJob.perform_later(work_package_ids:,
+                                                  label_id: model.id,
+                                                  label_name: model.name,
+                                                  user_id: user.id)
+    end
   end
 end

app/services/work_packages/set_attributes_service.rb

@@ -549,8 +549,6 @@ def parent_due_later_than_start?
   def set_cause_for_readonly_attributes
     return unless work_package.changes.keys.intersect?(%w(created_at updated_at author))
 
-    work_package.journal_cause = {
-      "type" => "default_attribute_written"
-    }
+    work_package.journal_cause = Journal::CausedByDefaultAttributeWritten.new
   end
 end

app/workers/labels/journalize_deletion_job.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 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, see <https://www.gnu.org/licenses/>.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class Labels::JournalizeDeletionJob < ApplicationJob
+  queue_with_priority :below_normal
+
+  def perform(work_package_ids:, label_id:, label_name:, user_id:)
+    user = User.find_by(id: user_id) || DeletedUser.first
+    cause = Journal::CausedByLabelDeletion.new(label_ids: [label_id], label_names: [label_name])
+
+    WorkPackage.where(id: work_package_ids).find_each(batch_size: 100) do |work_package|
+      journalize(work_package, user, cause, label_id)
+    end
+  end
+
+  private
+
+  def journalize(work_package, user, cause, label_id)
+    OpenProject::Mutex.with_advisory_lock_transaction(work_package) do
+      next if journalized?(work_package, label_id)
+
+      Journals::CreateService.new(work_package, user).call(cause:)
+    end
+  end
+
+  def journalized?(work_package, label_id)
+    work_package.journals.exists?(["journals.cause @> ?", { type: "label_deleted", label_ids: [label_id] }.to_json])
+  end
+end

config/locales/en.yml

@@ -3917,6 +3917,9 @@ en:
         header: "changes by %{author}"
         migrated: "from Jira"
         set_with_diff: "%{field} set (%{link})"
+      label_deleted:
+        one: "%{label_names} was deleted"
+        other: "%{label_names} were deleted"
       meeting_cancelled: "(cancelled)"
       meeting_deleted: "(deleted)"
       meeting_series_backlog_html: "backlog for meeting series %{link}"
@@ -3971,6 +3974,7 @@ en:
       default_attribute_written: "Read-only attributes written"
       forum_message_html: "<strong>Created from forum message</strong> %{message_information}"
       import: "Imported"
+      label_deleted: "Label removed:"
       meeting_agenda_item_added_html: "<strong>Added to meeting</strong> %{meeting_title_information}"
       meeting_agenda_item_added_template_html: "<strong>Added to</strong> %{meeting_title_information}"
       meeting_agenda_item_discussed_html: "<strong>Discussed in meeting</strong> %{meeting_title_information}"

lib/open_project/journal_formatter/cause.rb

@@ -101,6 +101,8 @@ def cause_description
       total_percent_complete_mode_changed_to_simple_average_message
     when "import"
       import_message
+    when "label_deleted"
+      label_deleted_message
     when "work_package_parent_deleted"
       I18n.t("journals.cause_descriptions.work_package_parent_deleted")
     else
@@ -128,6 +130,15 @@ def system_update_message
     html? ? message : Sanitize.fragment(message)
   end
 
+  def label_deleted_message
+    names = cause["label_names"]
+    return if names.blank?
+
+    label_names = names.map { |name| "'#{html? ? h(name) : name}'" }.join(", ")
+
+    I18n.t("journals.cause_descriptions.label_deleted", count: names.size, label_names:)
+  end
+
   def working_days_changed_message(changed_dates)
     day_changes_messages = changed_dates["working_days"].collect do |day, working|
       working_day_change_message(day.to_i, working)

lib/open_project/journal_formatter/labels.rb

@@ -29,8 +29,23 @@
 #++
 
 class OpenProject::JournalFormatter::Labels < OpenProject::JournalFormatter::JoinedAssociation
+  def render(key, values, options = { html: true })
+    return if only_deleted_labels_removed?(values)
+
+    super
+  end
+
   private
 
+  def only_deleted_labels_removed?(values)
+    return false unless @journal.cause_type == "label_deleted"
+
+    old_ids, new_ids = values.map { |value| value.to_s.split(",").map(&:to_i) }
+    added = new_ids - old_ids
+    removed = old_ids - new_ids
+    added.empty? && removed.to_set == Array(@journal.cause_label_ids).to_set(&:to_i)
+  end
+
   def name_or_placeholder(object)
     return I18n.t(:label_deleted_label) if object.nil?
 

lib_static/plugins/acts_as_journalized/lib/cause_of_change/base.rb

@@ -47,5 +47,13 @@ def blank?
     def ==(other)
       to_hash == other.to_hash
     end
+
+    def merge_into(_predecessor_cause)
+      nil
+    end
+
+    def mergeable_into?(predecessor_cause)
+      !merge_into(predecessor_cause).nil?
+    end
   end
 end

spec/components/work_packages/activities_tab/journals/item_component/details_spec.rb

@@ -75,6 +75,29 @@
     end
   end
 
+  context "with a labels change" do
+    let(:details) { { "labels" => ["1", nil] } }
+    let(:rendered) { { ["labels", ["1", nil]] => "Labels deleted A" } }
+
+    it "renders the labels line" do
+      expect(page).to have_text("Labels deleted A")
+    end
+
+    context "when written for a label deletion" do
+      let(:cause) { Journal::CausedByLabelDeletion.new(label_ids: [1], label_names: ["A"]).to_hash }
+      let(:journal) { build_stubbed(:work_package_journal, journable: work_package, user:, version: 2, cause:) }
+      let(:details) { { "labels" => ["1", nil], cause: [nil, cause] } }
+      let(:rendered) do
+        { ["labels", ["1", nil]] => nil, [:cause, [nil, cause]] => "Label removed: 'A' was deleted" }
+      end
+
+      it "renders the cause but not the labels line", :aggregate_failures do
+        expect(page).to have_text("Label removed: 'A' was deleted")
+        expect(page).to have_no_text("Labels deleted A")
+      end
+    end
+  end
+
   # A formatter that renders nothing leaves the entry with no change to show, so
   # a header would announce an author and a time above an empty entry.
   context "when every detail renders blank" do

spec/features/admin/labels/manage_spec.rb

@@ -114,6 +114,7 @@ def expect_row(label, usage:)
     end
 
     expect_flash(message: "Successful deletion.")
+    expect(Labels::JournalizeDeletionJob).to have_been_enqueued
     expect(page).to have_no_test_selector(row_selector(label))
     expect(Label.where(id: label.id)).not_to exist
     expect(Labeling.where(label_id: label.id)).not_to exist

spec/lib/api/v3/activities/activity_representer_rendering_spec.rb

@@ -160,6 +160,24 @@
           expect(subject).to have_json_path("details/#{x}/html")
         end
       end
+
+      context "when a label deletion cause already explains the removed label" do
+        let(:label) { build_stubbed(:label) }
+        let(:journal) do
+          build_stubbed(:work_package_journal,
+                        version: 2,
+                        notes: "",
+                        user: other_user,
+                        cause: Journal::CausedByLabelDeletion.new(label_ids: [label.id],
+                                                                  label_names: [label.name]).to_hash).tap do |journal|
+            allow(journal).to receive(:get_changes).and_return("labels" => [label.id.to_s, nil])
+          end
+        end
+
+        it "omits the labels detail" do
+          expect(subject).to have_json_size(0).at_path("details")
+        end
+      end
     end
   end