From 3cb950a9746d729113a9df92226c24b60e0189aa Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Sep 2026 19:00:03 +0300 Subject: [PATCH 01/14] Journal work package labels Snapshots the label set per journal so the activity tab renders label changes and baseline comparison loads historic labels. Snapshot rows keep no foreign key to labels so history survives a hard delete. https://community.openproject.org/wp/79963 --- .../journable/with_historic_attributes.rb | 32 +++-- .../with_historic_attributes/loader.rb | 29 +++- app/models/journal.rb | 2 + app/models/journal/label_journal.rb | 35 +++++ app/models/work_package/journalized.rb | 1 + .../journals/create_service/association.rb | 2 +- .../journals/create_service/labelable.rb | 86 +++++++++++ config/locales/en.yml | 3 + .../20260914130000_create_label_journals.rb | 39 +++++ .../journal_formatter/joined_association.rb | 45 ++++++ .../journal_formatter/joined_versions.rb | 19 +-- lib/open_project/journal_formatter/labels.rb | 34 +++++ .../lib/journal_changes.rb | 15 ++ .../journal/label_journal_factory.rb | 33 +++++ .../journal_formatter/labels_spec.rb | 100 +++++++++++++ .../with_historic_attributes_spec.rb | 103 +++++++++++++ spec/models/journal/label_journal_spec.rb | 51 +++++++ .../work_package_acts_as_journalized_spec.rb | 136 ++++++++++++++++++ .../create_service/association_spec.rb | 9 ++ 19 files changed, 742 insertions(+), 32 deletions(-) create mode 100644 app/models/journal/label_journal.rb create mode 100644 app/services/journals/create_service/labelable.rb create mode 100644 db/migrate/20260914130000_create_label_journals.rb create mode 100644 lib/open_project/journal_formatter/joined_association.rb create mode 100644 lib/open_project/journal_formatter/labels.rb create mode 100644 spec/factories/journal/label_journal_factory.rb create mode 100644 spec/lib/open_project/journal_formatter/labels_spec.rb create mode 100644 spec/models/journal/label_journal_spec.rb diff --git a/app/models/journable/with_historic_attributes.rb b/app/models/journable/with_historic_attributes.rb index d18fefd33a40..4a136c61e43e 100644 --- a/app/models/journable/with_historic_attributes.rb +++ b/app/models/journable/with_historic_attributes.rb @@ -263,6 +263,7 @@ def changes_at_timestamp(timestamp) merge_target_versions_changes!(changes, historic_journable) merge_observed_in_versions_changes!(changes, historic_journable) + merge_labels_changes!(changes, historic_journable) changes end @@ -275,12 +276,12 @@ def merge_target_versions_changes!(changes, historic_journable) end def target_versions_changes(historic_journable) - old_ids = sorted_version_ids(historic_journable, :target_versions) - new_ids = sorted_version_ids(__getobj__, :target_versions) + old_ids = sorted_association_ids(historic_journable, :target_versions) + new_ids = sorted_association_ids(__getobj__, :target_versions) {}.tap do |changes| changes["version_id"] = [old_ids.first, new_ids.first] if old_ids.first != new_ids.first - changes["target_versions"] = [old_ids, new_ids].map { joined_version_ids(it) } if old_ids != new_ids + changes["target_versions"] = [old_ids, new_ids].map { joined_ids(it) } if old_ids != new_ids end end @@ -291,19 +292,34 @@ def merge_observed_in_versions_changes!(changes, historic_journable) end def observed_in_versions_changes(historic_journable) - old_ids = sorted_version_ids(historic_journable, :observed_in_versions) - new_ids = sorted_version_ids(__getobj__, :observed_in_versions) + old_ids = sorted_association_ids(historic_journable, :observed_in_versions) + new_ids = sorted_association_ids(__getobj__, :observed_in_versions) {}.tap do |changes| - changes["observed_in_versions"] = [old_ids, new_ids].map { joined_version_ids(it) } if old_ids != new_ids + changes["observed_in_versions"] = [old_ids, new_ids].map { joined_ids(it) } if old_ids != new_ids end end - def sorted_version_ids(work_package, association) + def merge_labels_changes!(changes, historic_journable) + return unless __getobj__.respond_to?(:labels) + + changes.merge!(labels_changes(historic_journable)) + end + + def labels_changes(historic_journable) + old_ids = sorted_association_ids(historic_journable, :labels) + new_ids = sorted_association_ids(__getobj__, :labels) + + {}.tap do |changes| + changes["labels"] = [old_ids, new_ids].map { joined_ids(it) } if old_ids != new_ids + end + end + + def sorted_association_ids(work_package, association) work_package.public_send(association).map(&:id).sort end - def joined_version_ids(ids) + def joined_ids(ids) ids.join(",").presence end diff --git a/app/models/journable/with_historic_attributes/loader.rb b/app/models/journable/with_historic_attributes/loader.rb index d4ddb2969b6b..c06864bf7e02 100644 --- a/app/models/journable/with_historic_attributes/loader.rb +++ b/app/models/journable/with_historic_attributes/loader.rb @@ -68,6 +68,7 @@ def load_journal_associations(journalized = journables) load_custom_value_associations(journalized, journal_ids) load_versions_associations(journalized, journal_ids, kind: "target", association: :target_versions) load_versions_associations(journalized, journal_ids, kind: "observed_in", association: :observed_in_versions) + load_labels_association(journalized, journal_ids) journalized end @@ -90,7 +91,20 @@ def load_versions_associations(journalized, journal_ids, kind:, association:) journalized.each do |work_package| version_journals = Array(version_journals_by_journal_id[work_package.journal_id]) - set_versions_association_from_journal!(work_package:, version_journals:, association:) + historic_versions = version_journals.filter_map(&:version).sort_by(&:id) + set_association_from_journal!(work_package:, records: historic_versions, association:) + end + end + + def load_labels_association(journalized, journal_ids) + return unless journalized_class.method_defined?(:labels) + + label_journals_by_journal_id = load_label_journals_by_journal_id(journal_ids) + + journalized.each do |work_package| + label_journals = Array(label_journals_by_journal_id[work_package.journal_id]) + historic_labels = label_journals.filter_map(&:label).sort_by(&:id) + set_association_from_journal!(work_package:, records: historic_labels, association: :labels) end end @@ -143,6 +157,13 @@ def load_version_journals_by_journal_id(journal_ids, kind:) .group_by(&:journal_id) end + def load_label_journals_by_journal_id(journal_ids) + Journal::LabelJournal + .where(journal_id: journal_ids) + .includes(:label) + .group_by(&:journal_id) + end + def set_custom_value_association_from_journal!(work_package:, customizable_journals:) # Build the associated customizable_journals as custom values, this way the historic work packages # will behave just as the normal ones. Additionally set the reverse customized association @@ -155,11 +176,9 @@ def set_custom_value_association_from_journal!(work_package:, customizable_journ work_package.association(:custom_values).target = historic_custom_values end - def set_versions_association_from_journal!(work_package:, version_journals:, association:) - historic_versions = version_journals.filter_map(&:version).sort_by(&:id) - + def set_association_from_journal!(work_package:, records:, association:) work_package.association(association).loaded! - work_package.association(association).target = historic_versions + work_package.association(association).target = records end attr_accessor :journables diff --git a/app/models/journal.rb b/app/models/journal.rb index 97684afdf8c4..4d43339bdd80 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -56,6 +56,7 @@ class Journal < ApplicationRecord register_journal_formatter OpenProject::JournalFormatter::Diff register_journal_formatter OpenProject::JournalFormatter::FileLink register_journal_formatter OpenProject::JournalFormatter::IgnoreNonWorkingDays + register_journal_formatter OpenProject::JournalFormatter::Labels register_journal_formatter OpenProject::JournalFormatter::MeetingStartTime register_journal_formatter OpenProject::JournalFormatter::MeetingState register_journal_formatter OpenProject::JournalFormatter::MeetingWorkPackageId @@ -136,6 +137,7 @@ class Journal < ApplicationRecord has_many :custom_comment_journals, class_name: "Journal::CustomCommentJournal", dependent: :delete_all has_many :project_phase_journals, class_name: "Journal::ProjectPhaseJournal", dependent: :delete_all has_many :storable_journals, class_name: "Journal::StorableJournal", dependent: :delete_all + has_many :label_journals, class_name: "Journal::LabelJournal", dependent: :delete_all has_many :work_package_version_journals, class_name: "Journal::WorkPackageVersionJournal", dependent: :delete_all # Row lifecycle is owned by work_package_version_journals above. # rubocop:disable Rails/HasManyOrHasOneDependent diff --git a/app/models/journal/label_journal.rb b/app/models/journal/label_journal.rb new file mode 100644 index 000000000000..f83523b30bb7 --- /dev/null +++ b/app/models/journal/label_journal.rb @@ -0,0 +1,35 @@ +# 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, 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 Journal::LabelJournal < Journal::AssociatedJournal + self.table_name = "label_journals" + + belongs_to :label +end diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb index 4a1c4edf92af..05aa4b1d7c03 100644 --- a/app/models/work_package/journalized.rb +++ b/app/models/work_package/journalized.rb @@ -107,6 +107,7 @@ def self.event_url register_journal_formatted_fields "project_phase_definition_id", formatter_key: :project_phase_definition register_journal_formatted_fields "target_versions", formatter_key: :target_versions register_journal_formatted_fields "observed_in_versions", formatter_key: :observed_in_versions + register_journal_formatted_fields "labels", formatter_key: :labels # Joined register_journal_formatted_fields :parent_id, :project_id, diff --git a/app/services/journals/create_service/association.rb b/app/services/journals/create_service/association.rb index 964c4ecae338..eb1295de190b 100644 --- a/app/services/journals/create_service/association.rb +++ b/app/services/journals/create_service/association.rb @@ -33,7 +33,7 @@ class Association # Core associations are defined here. Module-specific associations can be defined in engines # using `Journals::CreateService::Association.register`. - @registry = Set.new(%i[Attachable CustomComment Customizable ProjectPhase WorkPackageVersion]) + @registry = Set.new(%i[Attachable CustomComment Customizable Labelable ProjectPhase WorkPackageVersion]) class << self def register(*names) diff --git a/app/services/journals/create_service/labelable.rb b/app/services/journals/create_service/labelable.rb new file mode 100644 index 000000000000..60fa1e70badb --- /dev/null +++ b/app/services/journals/create_service/labelable.rb @@ -0,0 +1,86 @@ +# 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, 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 Journals::CreateService + class Labelable < Association + def associated? + journable.respond_to?(:labels) + end + + def cleanup_predecessor(predecessor, notes, cause) + cleanup_predecessor_for(predecessor, + notes, + cause, + "label_journals", + :journal_id, + :id) + end + + def insert_sql + sanitize(<<~SQL.squish, journable_id:, labelable_type: journable_class_name) + INSERT INTO + label_journals ( + journal_id, + label_id + ) + SELECT + #{id_from_inserted_journal_sql}, + labelings.label_id + FROM labelings + WHERE + #{only_if_created_sql} + AND labelings.labelable_type = :labelable_type + AND labelings.labelable_id = :journable_id + SQL + end + + def changes_sql + sanitize(<<~SQL.squish, journable_id:, labelable_type: journable_class_name) + SELECT + :journable_id AS journable_id + FROM + ( + SELECT ARRAY_AGG(label_id ORDER BY label_id) AS labels + FROM labelings + WHERE labelings.labelable_type = :labelable_type + AND labelings.labelable_id = :journable_id + ) current_labels + CROSS JOIN + ( + SELECT ARRAY_AGG(label_id ORDER BY label_id) AS labels + FROM label_journals + WHERE journal_id IN (SELECT id FROM max_journals) + ) journal_labels + WHERE + current_labels.labels IS DISTINCT FROM journal_labels.labels + SQL + end + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index b37c902ad40f..893a56914787 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1208,6 +1208,9 @@ en: issue_priority: one: "Priority" other: "Priorities" + label: + one: "Label" + other: "Labels" ldap_auth_source: one: "LDAP connection" other: "LDAP connections" diff --git a/db/migrate/20260914130000_create_label_journals.rb b/db/migrate/20260914130000_create_label_journals.rb new file mode 100644 index 000000000000..8af04d0e0b16 --- /dev/null +++ b/db/migrate/20260914130000_create_label_journals.rb @@ -0,0 +1,39 @@ +# 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, 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 CreateLabelJournals < ActiveRecord::Migration[8.1] + def change + create_table :label_journals do |t| # rubocop:disable Rails/CreateTableWithTimestamps + t.belongs_to :journal, null: false, foreign_key: true + # No foreign key: journal snapshots must outlive deleted labels. + t.belongs_to :label, null: false + end + end +end diff --git a/lib/open_project/journal_formatter/joined_association.rb b/lib/open_project/journal_formatter/joined_association.rb new file mode 100644 index 000000000000..6a822d655b2d --- /dev/null +++ b/lib/open_project/journal_formatter/joined_association.rb @@ -0,0 +1,45 @@ +# 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 OpenProject::JournalFormatter::JoinedAssociation < JournalFormatter::NamedAssociation + private + + def format_values(values, key) + klass = class_from_field(key) + + values.map do |value| + next if value.blank? || klass.nil? + + value.to_s.split(",") + .filter_map { |id| name_or_placeholder(associated_object(klass, id.to_i)) } + .join(", ") + .presence + end + end +end diff --git a/lib/open_project/journal_formatter/joined_versions.rb b/lib/open_project/journal_formatter/joined_versions.rb index bf2a4904c0cb..8d4a41584de3 100644 --- a/lib/open_project/journal_formatter/joined_versions.rb +++ b/lib/open_project/journal_formatter/joined_versions.rb @@ -27,26 +27,9 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# Base for formatters rendering the change to a set of versions referenced by a -# work package. Each value is the sorted, comma-joined version ids (see -# JournalChanges); every id is resolved to the version's name, with a -# placeholder for versions that have been deleted or are no longer visible to -# the reader. -class OpenProject::JournalFormatter::JoinedVersions < JournalFormatter::NamedAssociation +class OpenProject::JournalFormatter::JoinedVersions < OpenProject::JournalFormatter::JoinedAssociation private - def format_values(values, key) - klass = class_from_field(key) - - values.map do |value| - next if value.blank? || klass.nil? - - value.to_s.split(",") - .map { |id| name_or_placeholder(associated_object(klass, id.to_i)) } - .join(", ") - end - end - def name_or_placeholder(object) return I18n.t(:label_deleted_version) if object.nil? diff --git a/lib/open_project/journal_formatter/labels.rb b/lib/open_project/journal_formatter/labels.rb new file mode 100644 index 000000000000..4603302f137f --- /dev/null +++ b/lib/open_project/journal_formatter/labels.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, 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. +#++ + +# Renders the change to the set of labels attached to a work package +# (see JournalChanges#get_labels_changes). +class OpenProject::JournalFormatter::Labels < OpenProject::JournalFormatter::JoinedAssociation +end diff --git a/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb b/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb index 16085802f4eb..e782cb32997f 100644 --- a/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb +++ b/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb @@ -47,6 +47,7 @@ def all_changes get_project_phases_changes, get_target_versions_changes, get_observed_in_versions_changes, + get_labels_changes, get_file_links_changes, get_participants_changes, get_agenda_items_changes @@ -155,6 +156,16 @@ def get_observed_in_versions_changes { observed_in_versions: [old_value, new_value] } end + def get_labels_changes + return unless journable.respond_to?(:labels) + + old_value = predecessor && joined_label_ids(predecessor) + new_value = joined_label_ids(self) + return if old_value == new_value + + { labels: [old_value, new_value] } + end + def get_file_links_changes return unless has_file_links? @@ -216,6 +227,10 @@ def joined_observed_in_version_ids(journal) journal.observed_in_version_journals.map(&:version_id).sort.join(",").presence end + def joined_label_ids(journal) + journal.label_journals.map(&:label_id).sort.join(",").presence + end + def participant_baseline_journal journals = journable.journals.to_a current_index = journals.index { |entry| entry.id == id } diff --git a/spec/factories/journal/label_journal_factory.rb b/spec/factories/journal/label_journal_factory.rb new file mode 100644 index 000000000000..207d4091c9eb --- /dev/null +++ b/spec/factories/journal/label_journal_factory.rb @@ -0,0 +1,33 @@ +# 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, 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. +#++ + +FactoryBot.define do + factory :journal_label_journal, class: "Journal::LabelJournal" +end diff --git a/spec/lib/open_project/journal_formatter/labels_spec.rb b/spec/lib/open_project/journal_formatter/labels_spec.rb new file mode 100644 index 000000000000..b600aa5a19a3 --- /dev/null +++ b/spec/lib/open_project/journal_formatter/labels_spec.rb @@ -0,0 +1,100 @@ +# 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, 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. +#++ + +require "spec_helper" + +RSpec.describe OpenProject::JournalFormatter::Labels do + describe "#render" do + let(:label) { build_stubbed(:label, name: "A") } + let(:other_label) { build_stubbed(:label, name: "B") } + let(:work_package) { build_stubbed(:work_package) } + let(:journal) { build_stubbed(:work_package_journal, journable: work_package) } + let(:instance) { described_class.new(journal) } + let(:label_text) { "Labels" } + + before do + allow(Label).to receive(:find_by).and_return(nil) + + [label, other_label].each do |l| + allow(Label).to receive(:find_by).with(id: l.id).and_return(l) + end + end + + context "when setting labels" do + it "renders the label names as the new value" do + expect(instance.render(:labels, [nil, "#{label.id},#{other_label.id}"])) + .to eq(I18n.t(:text_journal_set_to, label: label_text, value: "A, B")) + end + end + + context "when changing labels" do + it "renders the old and new label names" do + expect(instance.render(:labels, [label.id.to_s, other_label.id.to_s])) + .to eq(I18n.t(:text_journal_changed_plain, + label: label_text, + linebreak: nil, + old: "A", + new: "B")) + end + end + + context "when removing all labels" do + it "renders the old label names as deleted" do + expect(instance.render(:labels, ["#{label.id},#{other_label.id}", nil])) + .to eq(I18n.t(:text_journal_deleted, label: label_text, old: "A, B")) + end + end + + context "with a label that no longer exists" do + it "renders only the existing label names" do + expect(instance.render(:labels, [nil, "#{label.id},99999"])) + .to eq(I18n.t(:text_journal_set_to, label: label_text, value: "A")) + end + end + + context "when setting labels that no longer exist" do + it "renders no detail" do + expect(instance.render(:labels, [nil, "99999,99998"])) + .to eq(I18n.t(:text_journal_deleted_no_detail, label: label_text)) + end + end + + context "with html: false" do + it "renders plain text" do + expect(instance.render(:labels, [label.id.to_s, other_label.id.to_s], html: false)) + .to eq(I18n.t(:text_journal_changed_plain, + label: "Labels", + linebreak: nil, + old: "A", + new: "B")) + end + end + end +end diff --git a/spec/models/journable/with_historic_attributes_spec.rb b/spec/models/journable/with_historic_attributes_spec.rb index a112800b19f1..0dace097f1af 100644 --- a/spec/models/journable/with_historic_attributes_spec.rb +++ b/spec/models/journable/with_historic_attributes_spec.rb @@ -819,4 +819,107 @@ def update_observed_in_versions(*versions) end end end + + describe "labels" do + shared_let(:label_a) { create(:label) } + shared_let(:label_b) { create(:label) } + + let(:work_package) do + create(:work_package, + subject: "The labelled work package", + project:, + journals: { + created_at => {}, + 1.day.ago => {} + }) + end + + subject { described_class.wrap(work_package, timestamps:) } + + def update_labels(*labels) + work_package.labels = labels + work_package.save! + end + + def seed_baseline_label_journal(label) + create(:journal_label_journal, journal: work_package.journals.first, label:) + end + + describe "#at_timestamp" do + it "returns the labels from the journal snapshot, not the current ones" do + seed_baseline_label_journal(label_a) + update_labels(label_a, label_b) + + expect(subject.at_timestamp(Timestamp.parse("2022-01-01T00:00:00Z")).labels) + .to contain_exactly(label_a) + expect(work_package.reload.labels) + .to contain_exactly(label_a, label_b) + end + + it "omits snapshotted labels that were deleted since" do + update_labels(label_a, label_b) + label_b.destroy! + + # 1 hour in the future so the journal written by the update above is + # unambiguously in the past, regardless of sub-second timing. + expect(subject.at_timestamp(Timestamp.parse(1.hour.from_now.iso8601)).labels) + .to contain_exactly(label_a) + end + end + + describe "#changed_at_timestamp" do + context "when a label was added" do + it "marks the labels as changed" do + update_labels(label_a, label_b) + + expect(subject.changed_at_timestamp(Timestamp.parse("2022-01-01T00:00:00Z"))) + .to contain_exactly("labels") + end + end + + context "when one of several labels was removed" do + it "marks only the labels as changed" do + # Journal rows are stamped by the database clock, so a past removal + # can't be journalled directly; seed the baseline snapshot instead. + seed_baseline_label_journal(label_a) + seed_baseline_label_journal(label_b) + update_labels(label_a) + + expect(subject.changed_at_timestamp(Timestamp.parse("2022-01-01T00:00:00Z"))) + .to contain_exactly("labels") + expect(subject.send(:changes_at_timestamp, Timestamp.parse("2022-01-01T00:00:00Z"))["labels"]) + .to eq([[label_a.id, label_b.id].sort.join(","), label_a.id.to_s]) + end + end + + context "when all labels were removed" do + it "marks the labels as changed" do + seed_baseline_label_journal(label_a) + update_labels + + expect(subject.changed_at_timestamp(Timestamp.parse("2022-01-01T00:00:00Z"))) + .to contain_exactly("labels") + expect(subject.send(:changes_at_timestamp, Timestamp.parse("2022-01-01T00:00:00Z"))["labels"]) + .to eq([label_a.id.to_s, nil]) + end + end + + context "when the label set is unchanged" do + it "reports no label change" do + expect(subject.changed_at_timestamp(Timestamp.parse("2022-01-01T00:00:00Z"))) + .to be_empty + end + end + end + + describe "#changes_at_timestamp" do + it "reports the joined old and new label ids" do + seed_baseline_label_journal(label_a) + update_labels(label_a, label_b) + + expect(subject.send(:changes_at_timestamp, Timestamp.parse("2022-01-01T00:00:00Z"))["labels"]) + .to eq([label_a.id.to_s, [label_a.id, label_b.id].sort.join(",")]) + end + end + end end diff --git a/spec/models/journal/label_journal_spec.rb b/spec/models/journal/label_journal_spec.rb new file mode 100644 index 000000000000..6d2ea252406f --- /dev/null +++ b/spec/models/journal/label_journal_spec.rb @@ -0,0 +1,51 @@ +# 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, 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. +#++ + +require "spec_helper" + +RSpec.describe Journal::LabelJournal do + it "belongs to a label" do + expect(described_class.reflect_on_association(:label).macro).to eq(:belongs_to) + end + + it "uses the label_journals table" do + expect(described_class.table_name).to eq("label_journals") + end + + it "keeps its rows when the label is destroyed" do + journal = create(:work_package_journal) + label = create(:label) + label_journal = described_class.create!(journal:, label:) + + label.destroy! + + expect(label_journal.reload.label_id).to eq(label.id) + end +end 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 0df4ac2555af..229081b4ea61 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 @@ -575,6 +575,142 @@ def set_target_versions(versions) end end + context "on label changes", with_settings: { journal_aggregation_time_minutes: 0 } do + shared_let(:label) { create(:label) } + shared_let(:other_label) { create(:label) } + + shared_let(:journable) do + create(:work_package) + end + + def set_labels(labels) + journable.labelings.destroy_all + labels.each { journable.labelings.create!(label: it) } + journable.save! + end + + context "when setting labels" do + it "creates a new journal listing the labels in the details" do + expect { set_labels([label, other_label]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["labels"]) + .to eq([nil, [label.id, other_label.id].sort.join(",")]) + end + + it "touches the journable to match the journal's timestamp" do + expect { set_labels([label]) } + .to change { journable.reload.updated_at } + + expect(journable.updated_at).to eq(journable.last_journal.updated_at) + end + end + + context "on work package creation" do + it "creates an initial journal listing the labels in the details" do + work_package = create(:work_package, labels: [label]) + + expect(work_package.journals.count).to eq(1) + expect(work_package.last_journal.details["labels"]) + .to eq([nil, label.id.to_s]) + expect(Journal::LabelJournal.where(journal_id: work_package.last_journal.id).count) + .to eq(1) + end + end + + context "when replacing a label" do + before do + set_labels([label]) + end + + it "creates a new journal with the old and new labels in the details" do + expect { set_labels([other_label]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["labels"]) + .to eq([label.id.to_s, other_label.id.to_s]) + end + end + + context "when removing all labels" do + before do + set_labels([label]) + end + + it "creates a new journal with an empty new value in the details" do + expect { set_labels([]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["labels"]) + .to eq([label.id.to_s, nil]) + end + end + + context "when saving with unchanged labels" do + before do + set_labels([label]) + end + + it "creates no journal and does not touch the journable" do + expect { set_labels([label]) } + .to not_change { journable.journals.count } + .and(not_change { journable.reload.updated_at }) + end + end + + context "when another labelable shares the work package's id" do + it "journals only the work package's own labels" do + Labeling.create!(label:, labelable_type: "Project", labelable_id: journable.id) + + expect { set_labels([other_label]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["labels"]) + .to eq([nil, other_label.id.to_s]) + end + end + + context "within aggregation time", with_settings: { journal_aggregation_time_minutes: 5 } do + # The initial journal lies outside the aggregation window so that only + # the journals created by the examples can aggregate with each other. + shared_let(:journable) do + create(:work_package, + subject: "Initial subject", + journals: { 10.minutes.ago => { user: } }) + end + + context "when changing labels again" do + before do + set_labels([label]) + end + + it "aggregates into the previous journal with cumulative details" do + expect { set_labels([label, other_label]) } + .not_to change { journable.journals.count } + + expect(journable.last_journal.details["labels"]) + .to eq([nil, [label.id, other_label.id].sort.join(",")]) + end + end + + context "when changing labels shortly after another change" do + before do + journable.update!(subject: "Changed subject") + end + + it "aggregates into one journal capturing both changes" do + expect { set_labels([label]) } + .not_to change { journable.journals.count } + + expect(journable.last_journal.details["subject"]) + .to eq(["Initial subject", "Changed subject"]) + expect(journable.last_journal.details["labels"]) + .to eq([nil, label.id.to_s]) + end + end + end + end + context "on observed in version changes", with_settings: { journal_aggregation_time_minutes: 0 } do shared_let(:journable) do create(:work_package) diff --git a/spec/services/journals/create_service/association_spec.rb b/spec/services/journals/create_service/association_spec.rb index e4162aeeba3c..3249b128c11b 100644 --- a/spec/services/journals/create_service/association_spec.rb +++ b/spec/services/journals/create_service/association_spec.rb @@ -64,11 +64,20 @@ allow(journable).to receive(:respond_to?).with(:file_links).and_return(false) allow(journable).to receive(:respond_to?).with(:agenda_items).and_return(false) allow(journable).to receive(:respond_to?).with(:phases).and_return(false) + allow(journable).to receive(:respond_to?).with(:labels).and_return(false) associations = described_class.for(journable) expect(associations.map(&:class)).to include(Journals::CreateService::Attachable) end + it "includes Labelable for a journable that responds to labels" do + journable = instance_double(WorkPackage, customizable?: false, respond_to?: false) + allow(journable).to receive(:respond_to?).with(:labels).and_return(true) + + associations = described_class.for(journable) + expect(associations.map(&:class)).to include(Journals::CreateService::Labelable) + end + it "excludes associations whose #associated? returns false" do journable = instance_double(WorkPackage, customizable?: false, respond_to?: false) allow(journable).to receive(:respond_to?).and_return(false) From 1c544f8bcad6b6dd8d1b3c1911aa876ab5614fa0 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 10:07:35 +0300 Subject: [PATCH 02/14] Move label journals migration after the current dev migrations Keeps the migration ordered after the ones merged since the branch was opened. --- ..._label_journals.rb => 20261021090000_create_label_journals.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/migrate/{20260914130000_create_label_journals.rb => 20261021090000_create_label_journals.rb} (100%) diff --git a/db/migrate/20260914130000_create_label_journals.rb b/db/migrate/20261021090000_create_label_journals.rb similarity index 100% rename from db/migrate/20260914130000_create_label_journals.rb rename to db/migrate/20261021090000_create_label_journals.rb From 963aae4a61bf9275727e12fac3677e9da42719fa Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 10:13:34 +0300 Subject: [PATCH 03/14] Render a placeholder for deleted labels in journal entries A label can be hard-deleted while journals still reference it. Dropping it made the entry read as if the labels had been cleared. --- config/locales/en.yml | 1 + .../journal_formatter/joined_association.rb | 3 +-- lib/open_project/journal_formatter/labels.rb | 7 +++++++ spec/lib/open_project/journal_formatter/labels_spec.rb | 10 ++++++---- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 893a56914787..7e436b902052 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4049,6 +4049,7 @@ en: label_deleted_custom_field: "(deleted custom field)" label_deleted_custom_item: "(deleted item)" label_deleted_custom_option: "(deleted option)" + label_deleted_label: "(deleted label)" label_deleted_version: "(deleted version)" label_departments: "Organization" label_departments_description_html: > diff --git a/lib/open_project/journal_formatter/joined_association.rb b/lib/open_project/journal_formatter/joined_association.rb index 6a822d655b2d..84dbd4ce44e4 100644 --- a/lib/open_project/journal_formatter/joined_association.rb +++ b/lib/open_project/journal_formatter/joined_association.rb @@ -37,9 +37,8 @@ def format_values(values, key) next if value.blank? || klass.nil? value.to_s.split(",") - .filter_map { |id| name_or_placeholder(associated_object(klass, id.to_i)) } + .map { |id| name_or_placeholder(associated_object(klass, id.to_i)) } .join(", ") - .presence end end end diff --git a/lib/open_project/journal_formatter/labels.rb b/lib/open_project/journal_formatter/labels.rb index 4603302f137f..dcbf74002f89 100644 --- a/lib/open_project/journal_formatter/labels.rb +++ b/lib/open_project/journal_formatter/labels.rb @@ -31,4 +31,11 @@ # Renders the change to the set of labels attached to a work package # (see JournalChanges#get_labels_changes). class OpenProject::JournalFormatter::Labels < OpenProject::JournalFormatter::JoinedAssociation + private + + def name_or_placeholder(object) + return I18n.t(:label_deleted_label) if object.nil? + + super + end end diff --git a/spec/lib/open_project/journal_formatter/labels_spec.rb b/spec/lib/open_project/journal_formatter/labels_spec.rb index b600aa5a19a3..8f528ac37b8e 100644 --- a/spec/lib/open_project/journal_formatter/labels_spec.rb +++ b/spec/lib/open_project/journal_formatter/labels_spec.rb @@ -73,16 +73,18 @@ end context "with a label that no longer exists" do - it "renders only the existing label names" do + it "renders a placeholder next to the existing label names" do expect(instance.render(:labels, [nil, "#{label.id},99999"])) - .to eq(I18n.t(:text_journal_set_to, label: label_text, value: "A")) + .to eq(I18n.t(:text_journal_set_to, label: label_text, value: "A, #{I18n.t(:label_deleted_label)}")) end end context "when setting labels that no longer exist" do - it "renders no detail" do + it "renders a placeholder for each missing label" do + placeholder = I18n.t(:label_deleted_label) + expect(instance.render(:labels, [nil, "99999,99998"])) - .to eq(I18n.t(:text_journal_deleted_no_detail, label: label_text)) + .to eq(I18n.t(:text_journal_set_to, label: label_text, value: "#{placeholder}, #{placeholder}")) end end From b25b8e77670239dd3217bcfa6fc21153a89a300f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:06:57 +0300 Subject: [PATCH 04/14] Snapshot current labels onto the latest journal when adding the table Existing journals predate the snapshot table. Without a baseline, the next unchanged save of a labelled work package journals a spurious change and a first-edit removal of all labels journals nothing. --- .../20261021090000_create_label_journals.rb | 25 ++++++ spec/migrations/create_label_journals_spec.rb | 88 +++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 spec/migrations/create_label_journals_spec.rb diff --git a/db/migrate/20261021090000_create_label_journals.rb b/db/migrate/20261021090000_create_label_journals.rb index 8af04d0e0b16..f75600cd116d 100644 --- a/db/migrate/20261021090000_create_label_journals.rb +++ b/db/migrate/20261021090000_create_label_journals.rb @@ -35,5 +35,30 @@ def change # No foreign key: journal snapshots must outlive deleted labels. t.belongs_to :label, null: false end + + # Existing journals predate this table, so only the latest journal of each + # labelled work package gets a snapshot of its current labels. That keeps + # the next save from journaling a spurious change; older journals cannot be + # reconstructed and stay empty. + reversible do |dir| + dir.up do + say_with_time "Snapshotting current work package labels onto each latest journal" do + execute(<<~SQL.squish) + INSERT INTO label_journals (journal_id, label_id) + SELECT latest.id, labelings.label_id + FROM labelings + INNER JOIN LATERAL ( + SELECT journals.id + FROM journals + WHERE journals.journable_type = 'WorkPackage' + AND journals.journable_id = labelings.labelable_id + ORDER BY journals.version DESC + LIMIT 1 + ) latest ON TRUE + WHERE labelings.labelable_type = 'WorkPackage' + SQL + end + end + end end end diff --git a/spec/migrations/create_label_journals_spec.rb b/spec/migrations/create_label_journals_spec.rb new file mode 100644 index 000000000000..a8b0557b0256 --- /dev/null +++ b/spec/migrations/create_label_journals_spec.rb @@ -0,0 +1,88 @@ +# 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" +require Rails.root.join("db/migrate/20261021090000_create_label_journals.rb") + +RSpec.describe CreateLabelJournals, type: :model, + with_settings: { journal_aggregation_time_minutes: 0 } do + subject(:migrate) { ActiveRecord::Migration.suppress_messages { described_class.new.migrate(:up) } } + + let(:label) { create(:label) } + let(:other_label) { create(:label) } + + let!(:labelled_work_package) { create(:work_package, labels: [label, other_label]) } + let!(:unlabelled_work_package) { create(:work_package) } + + before do + labelled_work_package.update!(subject: "Second journal") + + # Journals and labelings exist but the snapshot table does not yet. + ActiveRecord::Base.connection.drop_table(:label_journals) + end + + it "snapshots the current labels onto the latest journal only" do + migrate + + first_journal, latest_journal = labelled_work_package.journals.order(:version).to_a + + expect(Journal::LabelJournal.pluck(:journal_id, :label_id)) + .to contain_exactly([latest_journal.id, label.id], [latest_journal.id, other_label.id]) + expect(Journal::LabelJournal.where(journal_id: first_journal.id)).to be_empty + end + + it "creates no rows for work packages without labels" do + migrate + + expect(Journal::LabelJournal.where(journal: unlabelled_work_package.journals)).to be_empty + end + + it "does not journal a change when a labelled work package is saved unchanged" do + migrate + labelled_work_package.reload + + expect do + labelled_work_package.label_id_replacements = [label.id, other_label.id] + labelled_work_package.save! + end.not_to change { labelled_work_package.journals.count } + end + + it "journals the removal when all labels are removed as the first edit" do + migrate + labelled_work_package.reload + + expect do + labelled_work_package.label_id_replacements = [] + labelled_work_package.save! + end.to change { labelled_work_package.journals.count }.by(1) + + expect(labelled_work_package.last_journal.details["labels"]) + .to eq([[label.id, other_label.id].sort.join(","), nil]) + end +end From e916b9998496b75598a715e0f0679770ac49e68b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:08:53 +0300 Subject: [PATCH 05/14] Include label changes in the activity tab's only-changes filter Label-only journals were hidden because the filter compared every other snapshot table with the predecessor but not the label snapshots. --- .../paginator/journal_changes_filter.rb | 10 ++++ .../activities_tab/paginator_spec.rb | 48 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/app/services/work_packages/activities_tab/paginator/journal_changes_filter.rb b/app/services/work_packages/activities_tab/paginator/journal_changes_filter.rb index e8561f37a8d0..09074e404f78 100644 --- a/app/services/work_packages/activities_tab/paginator/journal_changes_filter.rb +++ b/app/services/work_packages/activities_tab/paginator/journal_changes_filter.rb @@ -35,6 +35,7 @@ # * Custom field changes (compares customizable_journals with predecessor) # * File link changes (compares storages_file_links_journals with predecessor) # * Target/observed-in version changes (compares work_package_version_journals with predecessor) +# * Label changes (compares label_journals with predecessor) # * Cause metadata (system-triggered changes) # * Attribute/data changes (compares work_package_journals columns with immediate predecessor) # @@ -87,6 +88,7 @@ def changes_condition_sql OR EXISTS (#{custom_field_changes_condition_sql}) OR EXISTS (#{file_link_changes_condition_sql}) OR EXISTS (#{version_changes_condition_sql}) + OR EXISTS (#{label_changes_condition_sql}) SQL end @@ -136,6 +138,14 @@ def version_changes_condition_sql ) end + def label_changes_condition_sql + association_changes_condition_sql( + table: Journal::LabelJournal.table_name, + join_columns: ["label_id"], + value_columns: [] + ) + end + def data_changes_condition_sql data_change_columns = Journal::WorkPackageJournal.column_names - ["id"] diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb index 4e33ddbdb997..76636affa86f 100644 --- a/spec/services/work_packages/activities_tab/paginator_spec.rb +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -822,6 +822,54 @@ end end + context "with label changes" do + let(:label_a) { create(:label, name: "alpha") } + let(:label_b) { create(:label, name: "beta") } + + def save_labels(*labels) + work_package.reload.label_id_replacements = labels.map(&:id) + work_package.save! + work_package.journals.order(:version).last + end + + it "includes journal where a label was added" do + journal = save_labels(label_a) + + _pagy, records = paginator.call + expect(records.map(&:id)).to include(journal.id) + end + + it "includes journal where a label was replaced" do + save_labels(label_a) + journal = save_labels(label_b) + + _pagy, records = paginator.call + expect(records.map(&:id)).to include(journal.id) + end + + it "includes journal where a label was removed" do + save_labels(label_a) + journal = save_labels + + _pagy, records = paginator.call + expect(records.map(&:id)).to include(journal.id) + end + + it "excludes journal with only a label snapshot" do + save_labels(label_a) + work_package.reload.add_journal(notes: "Unrelated change") + work_package.save! + snapshot = work_package.journals.order(:version).last + + _pagy, records = paginator.call + expect(snapshot.reload.label_journals.count).to eq(1) + changes = snapshot.reload.get_changes + expect(changes).to eq({}) # no changes + + expect(records.map(&:id)).not_to include(snapshot.id) + end + end + context "with target version changes" do let!(:journal_with_target_version_added) do version = create(:version, project:) From 14fffab020ecf539dad078f99e19b2bcd52dec48 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:08:57 +0300 Subject: [PATCH 06/14] Preload label snapshots with the other journal associations Rendering label changes otherwise queries label_journals once per entry and once per predecessor. --- app/models/activities/fetcher.rb | 2 +- app/services/work_packages/activities_tab/paginator.rb | 2 +- lib/api/v3/activities/activity_eager_loading_wrapper.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/activities/fetcher.rb b/app/models/activities/fetcher.rb index 7058d022373d..53c63f64582f 100644 --- a/app/models/activities/fetcher.rb +++ b/app/models/activities/fetcher.rb @@ -123,7 +123,7 @@ def journals_of_event_set(events) journal_ids = events.map(&:event_id) Journal - .includes(:data, :customizable_journals, :attachable_journals, :target_version_journals, + .includes(:data, :customizable_journals, :attachable_journals, :target_version_journals, :label_journals, :observed_in_version_journals, :bcf_comment) .find(journal_ids) .then { |journals| ::API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals) } diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb index 07978d7a180e..d116ebdedf91 100644 --- a/app/services/work_packages/activities_tab/paginator.rb +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -179,7 +179,7 @@ def with_changesets(scope) def page_journals(page_relation) page_relation .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :target_version_journals, - :observed_in_version_journals, :notifications, :attachments) + :label_journals, :observed_in_version_journals, :notifications, :attachments) .to_a end diff --git a/lib/api/v3/activities/activity_eager_loading_wrapper.rb b/lib/api/v3/activities/activity_eager_loading_wrapper.rb index 0f992c2e148c..34f507388c44 100644 --- a/lib/api/v3/activities/activity_eager_loading_wrapper.rb +++ b/lib/api/v3/activities/activity_eager_loading_wrapper.rb @@ -150,7 +150,7 @@ def predecessor_journals(journals) SQL ) .includes(:attachable_journals, :customizable_journals, :storable_journals, :target_version_journals, - :observed_in_version_journals) + :label_journals, :observed_in_version_journals) end end end From 6ef5885636450c635089c573cdaf43739977e169 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:10:25 +0300 Subject: [PATCH 07/14] Render historic labels in the timestamp API The baseline comparison computed the label change but the timestamp representer dropped it, since labels were not among its supported links. --- .../work_package_at_timestamp_representer.rb | 5 +- ...at_timestamp_representer_rendering_spec.rb | 46 ++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/api/v3/work_packages/work_package_at_timestamp_representer.rb b/lib/api/v3/work_packages/work_package_at_timestamp_representer.rb index 63045e32118f..7175501e6c1d 100644 --- a/lib/api/v3/work_packages/work_package_at_timestamp_representer.rb +++ b/lib/api/v3/work_packages/work_package_at_timestamp_representer.rb @@ -54,9 +54,12 @@ class WorkPackageAtTimestampRepresenter < WorkPackageRepresenter version target_versions observed_in_versions + labels parent ].freeze + MULTI_VALUED_LINK_PROPERTIES = %w[target_versions observed_in_versions labels].freeze + SUPPORTED_CUSTOM_PROPERTIES = [/^custom_field_\d+$/].freeze SUPPORTED_PROPERTIES = (SUPPORTED_NON_LINK_PROPERTIES + SUPPORTED_LINK_PROPERTIES).freeze @@ -104,7 +107,7 @@ def rendered_properties # contains the lower camel-cased names. def rendered_properties_for_links @rendered_properties_for_links ||= rendered_properties.map do |property| - if property.starts_with?("custom_field_") || %w[target_versions observed_in_versions].include?(property) + if property.starts_with?("custom_field_") || MULTI_VALUED_LINK_PROPERTIES.include?(property) API::Utilities::PropertyNameConverter.from_ar_name(property) else property diff --git a/spec/lib/api/v3/work_packages/work_package_at_timestamp_representer_rendering_spec.rb b/spec/lib/api/v3/work_packages/work_package_at_timestamp_representer_rendering_spec.rb index 0f25cf18a788..3754c7a98bce 100644 --- a/spec/lib/api/v3/work_packages/work_package_at_timestamp_representer_rendering_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_at_timestamp_representer_rendering_spec.rb @@ -50,6 +50,9 @@ let(:version) { build_stubbed(:version) } let(:target_versions) { [version] } let(:observed_in_versions) { [] } + let(:label_a) { build_stubbed(:label) } + let(:label_b) { build_stubbed(:label) } + let(:labels) { [label_a, label_b] } let(:parent) do build_stubbed(:work_package).tap do |wp| allow(wp) @@ -99,7 +102,8 @@ .to receive_messages(available_custom_fields:, custom_field_values: [custom_value], effective_target_versions: target_versions, - effective_observed_in_versions: observed_in_versions) + effective_observed_in_versions: observed_in_versions, + effective_labels: labels) end end let(:timestamp) { Timestamp.new(1.day.ago) } @@ -323,6 +327,46 @@ end end + context "with only the labels changed", with_flag: { work_package_labels: true } do + let(:attributes_changed_to_baseline) { %w[labels] } + + let(:expected_json) do + { + "_meta" => { + "matchesFilters" => true, + "exists" => true, + "timestamp" => timestamp.to_s + }, + "_links" => { + "labels" => [ + { "href" => api_v3_paths.label(label_a.id), "title" => label_a.name }, + { "href" => api_v3_paths.label(label_b.id), "title" => label_b.name } + ], + "self" => { + "href" => api_v3_paths.work_package(work_package.id, timestamps: timestamp), + "title" => work_package.subject + }, + "schema" => { + "href" => api_v3_paths.work_package_schema(work_package.project_id, work_package.type_id) + } + } + }.to_json + end + + it "renders as expected" do + expect(subject) + .to be_json_eql(expected_json) + end + end + + context "with the labels unchanged", with_flag: { work_package_labels: true } do + let(:attributes_changed_to_baseline) { %w[subject] } + + it "does not render the labels" do + expect(subject).not_to have_json_path("_links/labels") + end + end + context "with only the observed in versions changed" do let(:observed_in_versions) { [version] } let(:attributes_changed_to_baseline) { %w[observed_in_versions] } From a67f7301d6734684335822ba94a578d44ac1a08e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:10:27 +0300 Subject: [PATCH 08/14] Copy historic labels onto the work package at a past timestamp When the last requested timestamp is historic, the work package took its labels from the current record while every other attribute came from the journal. --- .../eager_loading/historic_attributes.rb | 11 ++++---- .../v3/work_packages/index_resource_spec.rb | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/api/v3/work_packages/eager_loading/historic_attributes.rb b/lib/api/v3/work_packages/eager_loading/historic_attributes.rb index 0321d87f0b59..bd0cb6d7ef02 100644 --- a/lib/api/v3/work_packages/eager_loading/historic_attributes.rb +++ b/lib/api/v3/work_packages/eager_loading/historic_attributes.rb @@ -105,16 +105,17 @@ def timestamps def override_attributes(work_package, source) work_package.attributes = source.attributes.except("timestamp", "journal_id") - override_target_versions(work_package, source) + override_association(work_package, source, :target_versions) + override_association(work_package, source, :labels) work_package.clear_changes_information work_package.readonly! end - def override_target_versions(work_package, source) - return unless work_package.respond_to?(:target_versions) + def override_association(work_package, source, name) + return unless work_package.respond_to?(name) - work_package.association(:target_versions).loaded! - work_package.association(:target_versions).target = source.target_versions.to_a + work_package.association(name).loaded! + work_package.association(name).target = source.public_send(name).to_a end def set_timestamp_attributes(work_package, source, timestamp) diff --git a/spec/requests/api/v3/work_packages/index_resource_spec.rb b/spec/requests/api/v3/work_packages/index_resource_spec.rb index af33db81785a..a3c1f058e21b 100644 --- a/spec/requests/api/v3/work_packages/index_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/index_resource_spec.rb @@ -464,6 +464,34 @@ def create_customizable_journal(journal:, custom_field:, value:) end end + context "when the labels changed and the last timestamp is historic", + with_flag: { work_package_labels: true } do + let(:timestamps) { [Timestamp.parse("2015-01-01T00:00:00Z")] } + let(:label_a) { create(:label) } + let(:label_b) { create(:label) } + let(:label_hrefs) do + JSON.parse(subject.body).dig("_embedded", "elements", 0, "_links", "labels").pluck("href") + end + + before do + create(:labeling, labelable: work_package, label: label_a) + create(:labeling, labelable: work_package, label: label_b) + create(:journal_label_journal, journal: current_journal, label: label_a) + create(:journal_label_journal, journal: current_journal, label: label_b) + end + + it "renders the historic labels on the work package" do + create(:journal_label_journal, journal: original_journal, label: label_a) + + expect(label_hrefs) + .to contain_exactly(api_v3_paths.label(label_a.id)) + end + + it "renders no labels when the work package had none at the timestamp" do + expect(label_hrefs).to eq([]) + end + end + context "when a custom value changes" do before do custom_value From 5dd5b34c0104532b6e1dbe8b92006cf9af8ef9d6 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:14:26 +0300 Subject: [PATCH 09/14] Cover the baseline entry's label links in the timestamp API spec The historic last-timestamp case was covered; the baseline comparison entry was not. --- .../api/v3/work_packages/index_resource_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/requests/api/v3/work_packages/index_resource_spec.rb b/spec/requests/api/v3/work_packages/index_resource_spec.rb index a3c1f058e21b..3940a0212e2c 100644 --- a/spec/requests/api/v3/work_packages/index_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/index_resource_spec.rb @@ -490,6 +490,20 @@ def create_customizable_journal(journal:, custom_field:, value:) it "renders no labels when the work package had none at the timestamp" do expect(label_hrefs).to eq([]) end + + context "when comparing the baseline with the current state" do + let(:timestamps) { [Timestamp.parse("2015-01-01T00:00:00Z"), Timestamp.now] } + + it "embeds the historic labels in the baseline entry only" do + create(:journal_label_journal, journal: original_journal, label: label_a) + + expect(subject.body) + .to be_json_eql([{ href: api_v3_paths.label(label_a.id), title: label_a.name }].to_json) + .at_path("_embedded/elements/0/_embedded/attributesByTimestamp/0/_links/labels") + expect(subject.body) + .not_to have_json_path("_embedded/elements/0/_embedded/attributesByTimestamp/1/_links/labels") + end + end end context "when a custom value changes" do From 15975092a9b5c1a5572619a4a9e3a2f0ed4f26f5 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 12:14:29 +0300 Subject: [PATCH 10/14] Trim comments in the label journals migration and paginator spec --- db/migrate/20261021090000_create_label_journals.rb | 5 +---- spec/services/work_packages/activities_tab/paginator_spec.rb | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/db/migrate/20261021090000_create_label_journals.rb b/db/migrate/20261021090000_create_label_journals.rb index f75600cd116d..c44b7e5c9331 100644 --- a/db/migrate/20261021090000_create_label_journals.rb +++ b/db/migrate/20261021090000_create_label_journals.rb @@ -36,10 +36,7 @@ def change t.belongs_to :label, null: false end - # Existing journals predate this table, so only the latest journal of each - # labelled work package gets a snapshot of its current labels. That keeps - # the next save from journaling a spurious change; older journals cannot be - # reconstructed and stay empty. + # Only the latest journal of each labelled work package gets a snapshot; older journals cannot be reconstructed. reversible do |dir| dir.up do say_with_time "Snapshotting current work package labels onto each latest journal" do diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb index 76636affa86f..b401faed8b9a 100644 --- a/spec/services/work_packages/activities_tab/paginator_spec.rb +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -864,7 +864,7 @@ def save_labels(*labels) _pagy, records = paginator.call expect(snapshot.reload.label_journals.count).to eq(1) changes = snapshot.reload.get_changes - expect(changes).to eq({}) # no changes + expect(changes).to eq({}) expect(records.map(&:id)).not_to include(snapshot.id) end From 60eeef1df2c329f513a809fca61fd6983f9aa45f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 15:32:43 +0300 Subject: [PATCH 11/14] Move the label journals migration past the newest one on dev dev gained a migration with the version 20261021090000 after the rebase, and CI refuses duplicate versions. --- ...abel_journals.rb => 20261021120000_create_label_journals.rb} | 0 spec/migrations/create_label_journals_spec.rb | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename db/migrate/{20261021090000_create_label_journals.rb => 20261021120000_create_label_journals.rb} (100%) diff --git a/db/migrate/20261021090000_create_label_journals.rb b/db/migrate/20261021120000_create_label_journals.rb similarity index 100% rename from db/migrate/20261021090000_create_label_journals.rb rename to db/migrate/20261021120000_create_label_journals.rb diff --git a/spec/migrations/create_label_journals_spec.rb b/spec/migrations/create_label_journals_spec.rb index a8b0557b0256..6dad9a7e7629 100644 --- a/spec/migrations/create_label_journals_spec.rb +++ b/spec/migrations/create_label_journals_spec.rb @@ -28,7 +28,7 @@ #++ require "spec_helper" -require Rails.root.join("db/migrate/20261021090000_create_label_journals.rb") +require Rails.root.join("db/migrate/20261021120000_create_label_journals.rb") RSpec.describe CreateLabelJournals, type: :model, with_settings: { journal_aggregation_time_minutes: 0 } do From 604dd85fc6f8c4cbd8cd3eea140429b8fb59d932 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 15:38:59 +0300 Subject: [PATCH 12/14] Drop the label snapshot backfill from the migration Labels are unreleased and behind a feature flag, so no database has label history from before the snapshot table. --- .../20261021120000_create_label_journals.rb | 22 ----- spec/migrations/create_label_journals_spec.rb | 88 ------------------- 2 files changed, 110 deletions(-) delete mode 100644 spec/migrations/create_label_journals_spec.rb diff --git a/db/migrate/20261021120000_create_label_journals.rb b/db/migrate/20261021120000_create_label_journals.rb index c44b7e5c9331..8af04d0e0b16 100644 --- a/db/migrate/20261021120000_create_label_journals.rb +++ b/db/migrate/20261021120000_create_label_journals.rb @@ -35,27 +35,5 @@ def change # No foreign key: journal snapshots must outlive deleted labels. t.belongs_to :label, null: false end - - # Only the latest journal of each labelled work package gets a snapshot; older journals cannot be reconstructed. - reversible do |dir| - dir.up do - say_with_time "Snapshotting current work package labels onto each latest journal" do - execute(<<~SQL.squish) - INSERT INTO label_journals (journal_id, label_id) - SELECT latest.id, labelings.label_id - FROM labelings - INNER JOIN LATERAL ( - SELECT journals.id - FROM journals - WHERE journals.journable_type = 'WorkPackage' - AND journals.journable_id = labelings.labelable_id - ORDER BY journals.version DESC - LIMIT 1 - ) latest ON TRUE - WHERE labelings.labelable_type = 'WorkPackage' - SQL - end - end - end end end diff --git a/spec/migrations/create_label_journals_spec.rb b/spec/migrations/create_label_journals_spec.rb deleted file mode 100644 index 6dad9a7e7629..000000000000 --- a/spec/migrations/create_label_journals_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# 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" -require Rails.root.join("db/migrate/20261021120000_create_label_journals.rb") - -RSpec.describe CreateLabelJournals, type: :model, - with_settings: { journal_aggregation_time_minutes: 0 } do - subject(:migrate) { ActiveRecord::Migration.suppress_messages { described_class.new.migrate(:up) } } - - let(:label) { create(:label) } - let(:other_label) { create(:label) } - - let!(:labelled_work_package) { create(:work_package, labels: [label, other_label]) } - let!(:unlabelled_work_package) { create(:work_package) } - - before do - labelled_work_package.update!(subject: "Second journal") - - # Journals and labelings exist but the snapshot table does not yet. - ActiveRecord::Base.connection.drop_table(:label_journals) - end - - it "snapshots the current labels onto the latest journal only" do - migrate - - first_journal, latest_journal = labelled_work_package.journals.order(:version).to_a - - expect(Journal::LabelJournal.pluck(:journal_id, :label_id)) - .to contain_exactly([latest_journal.id, label.id], [latest_journal.id, other_label.id]) - expect(Journal::LabelJournal.where(journal_id: first_journal.id)).to be_empty - end - - it "creates no rows for work packages without labels" do - migrate - - expect(Journal::LabelJournal.where(journal: unlabelled_work_package.journals)).to be_empty - end - - it "does not journal a change when a labelled work package is saved unchanged" do - migrate - labelled_work_package.reload - - expect do - labelled_work_package.label_id_replacements = [label.id, other_label.id] - labelled_work_package.save! - end.not_to change { labelled_work_package.journals.count } - end - - it "journals the removal when all labels are removed as the first edit" do - migrate - labelled_work_package.reload - - expect do - labelled_work_package.label_id_replacements = [] - labelled_work_package.save! - end.to change { labelled_work_package.journals.count }.by(1) - - expect(labelled_work_package.last_journal.details["labels"]) - .to eq([[label.id, other_label.id].sort.join(","), nil]) - end -end From 5b32c808664dc4fe37ebfd52c5eaa3761b094e5c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 17:08:25 +0300 Subject: [PATCH 13/14] Pin label journaling through the label_id_replacements path Covers the callback order that lets the snapshot see labels written in the same save, and drops two examples that only restated Rails conventions. --- spec/models/journal/label_journal_spec.rb | 8 -------- .../work_package_acts_as_journalized_spec.rb | 12 ++++++++++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/models/journal/label_journal_spec.rb b/spec/models/journal/label_journal_spec.rb index 6d2ea252406f..26852b0242c1 100644 --- a/spec/models/journal/label_journal_spec.rb +++ b/spec/models/journal/label_journal_spec.rb @@ -31,14 +31,6 @@ require "spec_helper" RSpec.describe Journal::LabelJournal do - it "belongs to a label" do - expect(described_class.reflect_on_association(:label).macro).to eq(:belongs_to) - end - - it "uses the label_journals table" do - expect(described_class.table_name).to eq("label_journals") - end - it "keeps its rows when the label is destroyed" do journal = create(:work_package_journal) label = create(:label) 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 229081b4ea61..e03f40996e7e 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 @@ -598,6 +598,18 @@ def set_labels(labels) .to eq([nil, [label.id, other_label.id].sort.join(",")]) end + it "journals labels assigned through label_id_replacements in the same save" do + journable.label_id_replacements = [label.id, other_label.id] + + expect { journable.save! } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["labels"]) + .to eq([nil, [label.id, other_label.id].sort.join(",")]) + expect(Journal::LabelJournal.where(journal: journable.last_journal).pluck(:label_id)) + .to contain_exactly(label.id, other_label.id) + end + it "touches the journable to match the journal's timestamp" do expect { set_labels([label]) } .to change { journable.reload.updated_at } From 65b57c6d41e3825d22da94fefaa1e69a5eac562f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 21 Oct 2026 17:08:29 +0300 Subject: [PATCH 14/14] Drop a comment that only restated the class name --- lib/open_project/journal_formatter/labels.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/open_project/journal_formatter/labels.rb b/lib/open_project/journal_formatter/labels.rb index dcbf74002f89..ce89795284fa 100644 --- a/lib/open_project/journal_formatter/labels.rb +++ b/lib/open_project/journal_formatter/labels.rb @@ -28,8 +28,6 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# Renders the change to the set of labels attached to a work package -# (see JournalChanges#get_labels_changes). class OpenProject::JournalFormatter::Labels < OpenProject::JournalFormatter::JoinedAssociation private