From dea41027db7aa3dfadf9a3d8a340f5df51ac5c33 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Tue, 21 Jan 2025 18:38:22 +0100 Subject: [PATCH] extract model changes to module Differ::Model --- .../journable/with_historic_attributes.rb | 2 +- .../lib/acts/journalized/differ/model.rb | 68 +++++++++++ .../lib/acts/journalized/journable_differ.rb | 31 ----- .../lib/acts_as_journalized.rb | 2 +- .../lib/journal_changes.rb | 2 +- .../lib/acts/journalized/differ/model_spec.rb | 110 ++++++++++++++++++ .../journable_differ_spec.rb | 77 ------------ 7 files changed, 181 insertions(+), 111 deletions(-) create mode 100644 lib_static/plugins/acts_as_journalized/lib/acts/journalized/differ/model.rb create mode 100644 spec/lib/acts/journalized/differ/model_spec.rb diff --git a/app/models/journable/with_historic_attributes.rb b/app/models/journable/with_historic_attributes.rb index 2a1d3482b85..727aeec48ee 100644 --- a/app/models/journable/with_historic_attributes.rb +++ b/app/models/journable/with_historic_attributes.rb @@ -236,7 +236,7 @@ class Journable::WithHistoricAttributes < SimpleDelegator return unless historic_journable - changes = ::Acts::Journalized::JournableDiffer.changes(__getobj__, historic_journable) + changes = ::Acts::Journalized::Differ::Model.changes(__getobj__, historic_journable) # In the other occurrences of JournableDiffer.association_changes calls, we are using the plural # of the association name (`custom_fields` in this instance), to map the association fields. That diff --git a/lib_static/plugins/acts_as_journalized/lib/acts/journalized/differ/model.rb b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/differ/model.rb new file mode 100644 index 00000000000..95a8078670c --- /dev/null +++ b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/differ/model.rb @@ -0,0 +1,68 @@ +# 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. +# ++ + +module Acts::Journalized::Differ + module Model + class << self + def changes(original, changed) + original_data = original ? normalize_newlines(journaled_attributes(original)) : {} + + normalize_newlines(journaled_attributes(changed)) + .to_h { |attribute, new_value| [attribute, [original_data[attribute], new_value]] } + .reject { |_, (old_value, new_value)| equal_ignoring_empty_string?(old_value, new_value) } + .with_indifferent_access + end + + private + + def journaled_attributes(object) + if object.is_a?(Journal::BaseJournal) + object.journaled_attributes.stringify_keys + else + object.attributes.slice(*object.class.journal_class.journaled_attributes.map(&:to_s)) + end + end + + def normalize_newlines(data) + data.transform_values do |value| + value.is_a?(String) ? value.gsub("\r\n", "\n") : value + end + end + + def equal_ignoring_empty_string?(old_value, new_value) + ignoring_empty_string(old_value) == ignoring_empty_string(new_value) + end + + def ignoring_empty_string(value) + value unless value == "" + end + end + end +end diff --git a/lib_static/plugins/acts_as_journalized/lib/acts/journalized/journable_differ.rb b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/journable_differ.rb index 064ef9e3d82..2058950e5a9 100644 --- a/lib_static/plugins/acts_as_journalized/lib/acts/journalized/journable_differ.rb +++ b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/journable_differ.rb @@ -31,15 +31,6 @@ module Acts::Journalized class JournableDiffer class << self - def changes(original, changed) - original_data = original ? normalize_newlines(journaled_attributes(original)) : {} - - normalize_newlines(journaled_attributes(changed)) - .to_h { |attribute, new_value| [attribute, [original_data[attribute], new_value]] } - .reject { |_, (old_value, new_value)| equal_ignoring_empty_string?(old_value, new_value) } - .with_indifferent_access - end - def association_changes(original, changed, *) get_association_changes(original, changed, *) end @@ -62,28 +53,6 @@ module Acts::Journalized private - def normalize_newlines(data) - data.transform_values do |value| - value.is_a?(String) ? value.gsub("\r\n", "\n") : value - end - end - - def equal_ignoring_empty_string?(old_value, new_value) - ignoring_empty_string(old_value) == ignoring_empty_string(new_value) - end - - def ignoring_empty_string(value) - value unless value == "" - end - - def journaled_attributes(object) - if object.is_a?(Journal::BaseJournal) - object.journaled_attributes.stringify_keys - else - object.attributes.slice(*object.class.journal_class.journaled_attributes.map(&:to_s)) - end - end - def get_association_changes(original, changed, association, association_name, key, value) original_journals = original&.send(association)&.map(&:attributes) || [] changed_journals = changed.send(association).map(&:attributes) diff --git a/lib_static/plugins/acts_as_journalized/lib/acts_as_journalized.rb b/lib_static/plugins/acts_as_journalized/lib/acts_as_journalized.rb index dc5d76e6ea9..dc9376b69b6 100644 --- a/lib_static/plugins/acts_as_journalized/lib/acts_as_journalized.rb +++ b/lib_static/plugins/acts_as_journalized/lib/acts_as_journalized.rb @@ -52,7 +52,7 @@ require "cause_of_change" module Acts end -Dir[File.expand_path("acts/journalized/*.rb", __dir__)].sort.each { |f| require f } +Dir[File.expand_path("acts/journalized/{,*/}*.rb", __dir__)].each { |f| require f } module Acts module Journalized 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 ad2d6e5b4af..85069220d48 100644 --- a/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb +++ b/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb @@ -50,7 +50,7 @@ module JournalChanges end def get_data_changes - ::Acts::Journalized::JournableDiffer.changes(predecessor&.data, data) + ::Acts::Journalized::Differ::Model.changes(predecessor&.data, data) end def get_attachments_changes diff --git a/spec/lib/acts/journalized/differ/model_spec.rb b/spec/lib/acts/journalized/differ/model_spec.rb new file mode 100644 index 00000000000..76efc8617df --- /dev/null +++ b/spec/lib/acts/journalized/differ/model_spec.rb @@ -0,0 +1,110 @@ +# 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 Acts::Journalized::Differ::Model do + describe ".changes" do + context "when the objects are work packages" do + let(:original) do + build_stubbed(:work_package, + subject: "The original work package title", + description: "The description\n", + assigned_to_id: nil, + schedule_manually: false, + ignore_non_working_days: true, + estimated_hours: 1) + end + let(:changed) do + build_stubbed(:work_package, + subject: "The changed work package title", + description: "The description\r\n", + priority: original.priority, + type: original.type, + project: original.project, + schedule_manually: nil, + ignore_non_working_days: false, + estimated_hours: nil) + end + + it "returns the changes" do + expect(described_class.changes(original, changed)) + .to eq( + "subject" => [original.subject, changed.subject], + "author_id" => [original.author_id, changed.author_id], + "status_id" => [original.status_id, changed.status_id], + "schedule_manually" => [false, nil], + "ignore_non_working_days" => [true, false], + "estimated_hours" => [1.0, nil] + ) + end + end + + context "when the objects are WorkPackageJournal" do + let(:original) do + build_stubbed(:journal_work_package_journal, + subject: "The original work package title", + description: nil, + priority_id: 5, + type_id: 89, + project_id: 12, + status_id: 45, + schedule_manually: nil, + ignore_non_working_days: false, + estimated_hours: nil) + end + let(:changed) do + build_stubbed(:journal_work_package_journal, + subject: "The changed work package title", + description: "", + priority_id: original.priority_id, + type_id: original.type_id, + project_id: original.project_id, + status_id: original.status_id + 12, + schedule_manually: false, + ignore_non_working_days: true, + estimated_hours: 1) + end + + it "returns the changes" do + # The description field changes from nil to '', but we want filter those transitions out, + # hence the expected hash does not contain the description related change. + expect(described_class.changes(original, changed)) + .to eq( + "subject" => [original.subject, changed.subject], + "status_id" => [original.status_id, changed.status_id], + "schedule_manually" => [nil, false], + "ignore_non_working_days" => [false, true], + "estimated_hours" => [nil, 1.0] + ) + end + end + end +end diff --git a/spec/lib/acts_as_journalized/journable_differ_spec.rb b/spec/lib/acts_as_journalized/journable_differ_spec.rb index 2c655ff775c..b48525ed38b 100644 --- a/spec/lib/acts_as_journalized/journable_differ_spec.rb +++ b/spec/lib/acts_as_journalized/journable_differ_spec.rb @@ -31,83 +31,6 @@ require "spec_helper" RSpec.describe Acts::Journalized::JournableDiffer do - describe ".changes" do - context "when the objects are work packages" do - let(:original) do - build_stubbed(:work_package, - subject: "The original work package title", - description: "The description\n", - assigned_to_id: nil, - schedule_manually: false, - ignore_non_working_days: true, - estimated_hours: 1) - end - let(:changed) do - build_stubbed(:work_package, - subject: "The changed work package title", - description: "The description\r\n", - priority: original.priority, - type: original.type, - project: original.project, - schedule_manually: nil, - ignore_non_working_days: false, - estimated_hours: nil) - end - - it "returns the changes" do - expect(described_class.changes(original, changed)) - .to eq( - "subject" => [original.subject, changed.subject], - "author_id" => [original.author_id, changed.author_id], - "status_id" => [original.status_id, changed.status_id], - "schedule_manually" => [false, nil], - "ignore_non_working_days" => [true, false], - "estimated_hours" => [1.0, nil] - ) - end - end - - context "when the objects are WorkPackageJournal" do - let(:original) do - build_stubbed(:journal_work_package_journal, - subject: "The original work package title", - description: nil, - priority_id: 5, - type_id: 89, - project_id: 12, - status_id: 45, - schedule_manually: nil, - ignore_non_working_days: false, - estimated_hours: nil) - end - let(:changed) do - build_stubbed(:journal_work_package_journal, - subject: "The changed work package title", - description: "", - priority_id: original.priority_id, - type_id: original.type_id, - project_id: original.project_id, - status_id: original.status_id + 12, - schedule_manually: false, - ignore_non_working_days: true, - estimated_hours: 1) - end - - it "returns the changes" do - # The description field changes from nil to '', but we want filter those transitions out, - # hence the expected hash does not contain the description related change. - expect(described_class.changes(original, changed)) - .to eq( - "subject" => [original.subject, changed.subject], - "status_id" => [original.status_id, changed.status_id], - "schedule_manually" => [nil, false], - "ignore_non_working_days" => [false, true], - "estimated_hours" => [nil, 1.0] - ) - end - end - end - describe ".association_changes" do context "when the objects are work packages" do let(:original) do