diff --git a/app/components/work_packages/activities_tab/journals/item_component/details.rb b/app/components/work_packages/activities_tab/journals/item_component/details.rb
index 723bba06c9c7..46e0213d3f52 100644
--- a/app/components/work_packages/activities_tab/journals/item_component/details.rb
+++ b/app/components/work_packages/activities_tab/journals/item_component/details.rb
@@ -61,7 +61,13 @@ def journal_details
# A formatter may render a change as nothing, so what is worth heading is
# what comes out of them rather than what went in.
def rendered_details
- @rendered_details ||= journal_details.filter_map { |detail| journal.render_detail(detail).presence }
+ @rendered_details ||= journal_details
+ .reject { |attribute, _| hidden_by_cause?(attribute) }
+ .filter_map { |detail| journal.render_detail(detail).presence }
+ end
+
+ def hidden_by_cause?(attribute)
+ attribute.to_s == "labels" && journal.cause_type == "label_deleted"
end
def has_details?
diff --git a/app/models/journal.rb b/app/models/journal.rb
index 4d43339bdd80..9122ba7f0b99 100644
--- a/app/models/journal.rb
+++ b/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
diff --git a/app/models/journal/caused_by_label_deletion.rb b/app/models/journal/caused_by_label_deletion.rb
new file mode 100644
index 000000000000..ed0e170095eb
--- /dev/null
+++ b/app/models/journal/caused_by_label_deletion.rb
@@ -0,0 +1,49 @@
+# 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 .
+#
+# 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
+
+ def aggregatable_with?(predecessor_cause)
+ predecessor_cause["type"] == type
+ end
+
+ def aggregate_with(predecessor_cause)
+ self.class.new(label_ids: predecessor_cause["label_ids"] | label_ids,
+ label_names: predecessor_cause["label_names"] | label_names)
+ end
+
+ private
+
+ def label_ids = additional_attributes["label_ids"]
+
+ def label_names = additional_attributes["label_names"]
+end
diff --git a/app/services/journals/create_service.rb b/app/services/journals/create_service.rb
index b0d8c915b887..6baeb53a3ba5 100644
--- a/app/services/journals/create_service.rb
+++ b/app/services/journals/create_service.rb
@@ -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.try(:aggregatable_with?, predecessor.cause)
+ end
+
+ def aggregate_cause(predecessor, cause)
+ return cause unless predecessor && cause.try(:aggregatable_with?, predecessor.cause)
+
+ cause.aggregate_with(predecessor.cause)
end
def only_one_note?(predecessor, notes)
diff --git a/app/services/labels/delete_service.rb b/app/services/labels/delete_service.rb
index ce804fe88d90..75593794fb1f 100644
--- a/app/services/labels/delete_service.rb
+++ b/app/services/labels/delete_service.rb
@@ -29,5 +29,29 @@
module Labels
class DeleteService < ::BaseServices::Delete
+ protected
+
+ def persist(service_result)
+ @labeled_work_package_ids = labeled_work_package_ids
+ super
+ end
+
+ def after_persist(service_result)
+ enqueue_journalization if @labeled_work_package_ids.any?
+ super
+ end
+
+ private
+
+ def labeled_work_package_ids
+ Labeling.where(label_id: model.id, labelable_type: WorkPackage.name).pluck(:labelable_id)
+ end
+
+ def enqueue_journalization
+ Labels::JournalizeDeletionJob.perform_later(work_package_ids: @labeled_work_package_ids,
+ label_id: model.id,
+ label_name: model.name,
+ user_id: user.id)
+ end
end
end
diff --git a/app/workers/labels/journalize_deletion_job.rb b/app/workers/labels/journalize_deletion_job.rb
new file mode 100644
index 000000000000..a8addae8a1d3
--- /dev/null
+++ b/app/workers/labels/journalize_deletion_job.rb
@@ -0,0 +1,60 @@
+# 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 .
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+# Skips work packages that already carry the deletion journal, so a retried job adds no second entry.
+# Deletions by the same admin inside the aggregation window merge into one entry listing every label.
+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: pending_ids(work_package_ids, label_id)).find_each(batch_size: 100) do |work_package|
+ journalize(work_package, user, cause)
+ end
+ end
+
+ private
+
+ def pending_ids(ids, label_id)
+ journalized = Journal
+ .where(journable_type: WorkPackage.name, journable_id: ids)
+ .where("journals.cause @> ?", { type: "label_deleted", label_ids: [label_id] }.to_json)
+ .pluck(:journable_id)
+
+ ids - journalized
+ end
+
+ def journalize(work_package, user, cause)
+ OpenProject::Mutex.with_advisory_lock_transaction(work_package) do
+ Journals::CreateService.new(work_package, user).call(cause:)
+ end
+ end
+end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7e436b902052..37c5d2a9cde5 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -3793,6 +3793,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}"
@@ -3846,6 +3849,7 @@ en:
default_attribute_written: "Read-only attributes written"
forum_message_html: "Created from forum message %{message_information}"
import: "Imported"
+ label_deleted: "Label removed:"
meeting_agenda_item_added_html: "Added to meeting %{meeting_title_information}"
meeting_agenda_item_added_template_html: "Added to %{meeting_title_information}"
meeting_agenda_item_discussed_html: "Discussed in meeting %{meeting_title_information}"
diff --git a/lib/open_project/journal_formatter/cause.rb b/lib/open_project/journal_formatter/cause.rb
index 3afa2b04fdb9..0abae175c0db 100644
--- a/lib/open_project/journal_formatter/cause.rb
+++ b/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,13 @@ def system_update_message
html? ? message : Sanitize.fragment(message)
end
+ def label_deleted_message
+ names = Array(cause["label_names"])
+ 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)
diff --git a/lib_static/plugins/acts_as_journalized/lib/cause_of_change/base.rb b/lib_static/plugins/acts_as_journalized/lib/cause_of_change/base.rb
index ba34c78f96e9..30c20d61c669 100644
--- a/lib_static/plugins/acts_as_journalized/lib/cause_of_change/base.rb
+++ b/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 aggregatable_with?(_predecessor_cause)
+ false
+ end
+
+ def aggregate_with(_predecessor_cause)
+ self
+ end
end
end
diff --git a/spec/components/work_packages/activities_tab/journals/item_component/details_spec.rb b/spec/components/work_packages/activities_tab/journals/item_component/details_spec.rb
index 75a3dab5a773..f9a1e3f237a9 100644
--- a/spec/components/work_packages/activities_tab/journals/item_component/details_spec.rb
+++ b/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]] => "Labels deleted A", [: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
diff --git a/spec/features/admin/labels/manage_spec.rb b/spec/features/admin/labels/manage_spec.rb
index 98e83dba80ee..51c579371288 100644
--- a/spec/features/admin/labels/manage_spec.rb
+++ b/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
diff --git a/spec/lib/open_project/journal_formatter/cause_spec.rb b/spec/lib/open_project/journal_formatter/cause_spec.rb
index b7bc6fd85ba4..83eea3746b41 100644
--- a/spec/lib/open_project/journal_formatter/cause_spec.rb
+++ b/spec/lib/open_project/journal_formatter/cause_spec.rb
@@ -645,6 +645,50 @@ def render(cause, html:)
end
end
+ context "when the change was caused by the deletion of a label" do
+ subject(:cause) { Journal::CausedByLabelDeletion.new(label_ids: [42], label_names: ["Foo"]).to_hash }
+
+ it do
+ expect(cause).to render_html_variant(
+ "Label removed: 'Foo' was deleted"
+ )
+ end
+
+ it do
+ expect(cause).to render_raw_variant("Label removed: 'Foo' was deleted")
+ end
+
+ context "when several labels were deleted" do
+ subject(:cause) do
+ Journal::CausedByLabelDeletion.new(
+ label_ids: [1, 2, 3], label_names: %w[Frontend Backend Middleware]
+ ).to_hash
+ end
+
+ it "lists every label name and pluralises the verb" do
+ expect(cause).to render_html_variant(
+ "Label removed: 'Frontend', 'Backend', 'Middleware' were deleted"
+ )
+ end
+ end
+
+ context "when the label name contains HTML (XSS prevention)" do
+ subject(:cause) do
+ Journal::CausedByLabelDeletion.new(label_ids: [42], label_names: [""]).to_hash
+ end
+
+ it "escapes the label name in HTML output" do
+ expect(cause).to render_html_variant(
+ a_string_including("<script>alert('x')</script>")
+ )
+ end
+
+ it "does not escape the label name in raw text output" do
+ expect(cause).to render_raw_variant(a_string_including(""))
+ end
+ end
+ end
+
context "when the change was caused by an import" do
subject(:cause) do
{
diff --git a/spec/models/work_package/work_package_acts_as_journalized_spec.rb b/spec/models/work_package/work_package_acts_as_journalized_spec.rb
index e03f40996e7e..1dc5f972ecdc 100644
--- a/spec/models/work_package/work_package_acts_as_journalized_spec.rb
+++ b/spec/models/work_package/work_package_acts_as_journalized_spec.rb
@@ -1178,6 +1178,38 @@ def set_observed_in_versions(versions)
end
end
+ context "on consecutive label deletions within aggregation time", with_settings: { journal_aggregation_time_minutes: 5 } do
+ shared_let(:journable) do
+ create(:work_package, journals: { 10.minutes.ago => { user: } })
+ end
+
+ def journalize(cause)
+ Journals::CreateService.new(journable, user).call(cause:)
+ end
+
+ before do
+ journalize(Journal::CausedByLabelDeletion.new(label_ids: [1], label_names: ["A"]))
+ end
+
+ it "aggregates consecutive label deletions into one journal listing every label" do
+ expect { journalize(Journal::CausedByLabelDeletion.new(label_ids: [2], label_names: ["B"])) }
+ .not_to change { journable.journals.count }
+
+ expect(journable.journals.reload.last.cause)
+ .to eq("type" => "label_deleted", "label_ids" => [1, 2], "label_names" => %w[A B])
+ end
+
+ it "does not aggregate an unrelated cause into the label deletion journal" do
+ expect { journalize(Journal::CausedByWorkPackageParentDeletion.new) }
+ .to change { journable.journals.count }.by(1)
+
+ expect(journable.journals.reload[-2].cause)
+ .to eq("type" => "label_deleted", "label_ids" => [1], "label_names" => ["A"])
+ expect(journable.journals.reload.last.cause)
+ .to eq(Journal::CausedByWorkPackageParentDeletion.new.to_hash)
+ end
+ end
+
context "on a comment within aggregation time after a journal with a meeting cause" do
let(:meeting_cause) { { "type" => "meeting_agenda_item_added", "meeting_id" => 42 } }
diff --git a/spec/services/labels/delete_service_spec.rb b/spec/services/labels/delete_service_spec.rb
index 6d700b1b19db..897ea87a1599 100644
--- a/spec/services/labels/delete_service_spec.rb
+++ b/spec/services/labels/delete_service_spec.rb
@@ -47,6 +47,29 @@
expect(result).to be_success
expect(Label.where(id: label.id)).not_to exist
expect(Labeling.where(id: labeling.id)).not_to exist
+ expect(Labels::JournalizeDeletionJob)
+ .to have_been_enqueued.with(work_package_ids: [labeling.labelable_id],
+ label_id: label.id,
+ label_name: label.name,
+ user_id: admin.id)
+ end
+
+ it "enqueues nothing for an unused label" do
+ instance.call
+
+ expect(Labels::JournalizeDeletionJob).not_to have_been_enqueued
+ end
+
+ context "when the label cannot be destroyed" do
+ before do
+ create(:labeling, label:)
+ allow(label).to receive(:destroy).and_return(false)
+ end
+
+ it "fails and enqueues nothing" do
+ expect(instance.call).to be_failure
+ expect(Labels::JournalizeDeletionJob).not_to have_been_enqueued
+ end
end
context "with a non-admin user" do
@@ -58,6 +81,7 @@
expect(result).to be_failure
expect(result.errors.symbols_for(:base)).to include(:error_unauthorized)
expect(Label.where(id: label.id)).to exist
+ expect(Labels::JournalizeDeletionJob).not_to have_been_enqueued
end
end
end
diff --git a/spec/workers/labels/journalize_deletion_job_spec.rb b/spec/workers/labels/journalize_deletion_job_spec.rb
new file mode 100644
index 000000000000..14ac7547831d
--- /dev/null
+++ b/spec/workers/labels/journalize_deletion_job_spec.rb
@@ -0,0 +1,128 @@
+# 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 .
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe Labels::JournalizeDeletionJob, type: :job, with_settings: { journal_aggregation_time_minutes: 0 } do
+ shared_let(:admin) { create(:admin) }
+
+ let(:label) { create(:label, name: "Deprecated") }
+ let(:label_id) { label.id }
+ let(:label_name) { label.name }
+ let(:labels_on_work_packages) { [label] }
+ let(:work_packages) do
+ create_list(:work_package, 2, journals: { 10.minutes.ago => { user: admin } })
+ end
+ let(:user_id) { admin.id }
+ let(:cause) do
+ { "type" => "label_deleted", "label_ids" => [label_id], "label_names" => [label_name] }
+ end
+
+ def perform
+ described_class.perform_now(work_package_ids: work_packages.map(&:id), label_id:, label_name:, user_id:)
+ end
+
+ before do
+ work_packages.each do |work_package|
+ WorkPackages::UpdateService.new(user: admin, model: work_package).call(label_ids: labels_on_work_packages.map(&:id)).on_failure { raise it.message }
+ end
+ label.destroy!
+ end
+
+ it "writes one journal per work package, authored by the admin, recording the label as removed" do
+ expect { perform }.to change(Journal, :count).by(2)
+
+ work_packages.each do |work_package|
+ journal = work_package.reload.last_journal
+ expect(journal.user).to eq(admin)
+ expect(journal.cause).to eq(cause)
+ end
+
+ journal = work_packages.first.reload.last_journal
+ old_ids, new_ids = journal.details["labels"].map { it.to_s.split(",").map(&:to_i) }
+ expect(old_ids).to include(label_id)
+ expect(new_ids).not_to include(label_id)
+ expect(journal.render_detail(["labels", journal.details["labels"]], html: false))
+ .to include(I18n.t(:label_deleted_label))
+ end
+
+ it "adds no further journal when run again with the same arguments" do
+ perform
+
+ expect { perform }.not_to change(Journal, :count)
+ end
+
+ context "when the admin no longer exists" do
+ let(:user_id) { 0 }
+
+ it "authors the journals as the deleted user" do
+ perform
+
+ expect(work_packages.first.reload.last_journal.user).to eq(DeletedUser.first)
+ end
+ end
+
+ context "when the same admin deletes another label right after" do
+ let(:work_packages) { [create(:work_package, journals: { 10.minutes.ago => { user: admin } })] }
+ let(:other_label) { create(:label, name: "Obsolete") }
+ let(:labels_on_work_packages) { [label, other_label] }
+ let(:work_package) { work_packages.first }
+
+ def perform_for(deleted_label)
+ described_class.perform_now(work_package_ids: [work_package.id], label_id: deleted_label.id,
+ label_name: deleted_label.name, user_id:)
+ end
+
+ before do
+ WorkPackages::UpdateService.new(user: admin, model: work_package).call(subject: "x").on_failure { raise it.message }
+ with_settings(journal_aggregation_time_minutes: 5)
+ perform_for(label)
+ other_label.destroy!
+ end
+
+ it "merges both labels into one journal entry" do
+ expect { perform_for(other_label) }.not_to change { work_package.journals.count }
+
+ journal = work_package.reload.last_journal
+ expect(journal.cause).to eq(
+ "type" => "label_deleted",
+ "label_ids" => [label.id, other_label.id],
+ "label_names" => [label.name, other_label.name]
+ )
+ new_ids = journal.details["labels"].last.to_s.split(",").map(&:to_i)
+ expect(new_ids).not_to include(label.id, other_label.id)
+ end
+
+ it "adds nothing when the first label's job runs again after the merge" do
+ perform_for(other_label)
+
+ expect { perform_for(label) }.not_to change { work_package.journals.count }
+ end
+ end
+end