From 2b3e9622adccf46372bdc3889a3738c4cb3d4d37 Mon Sep 17 00:00:00 2001 From: Alexandru Emil Lupu Date: Thu, 12 Dec 2024 10:49:07 +0200 Subject: [PATCH] Backport 'Fix the Diff Render output' to v0.29 (#13752) --- .../accountability/diff_renderer_spec.rb | 10 +++++-- decidim-core/app/cells/decidim/diff_cell.rb | 4 +++ .../services/decidim/base_diff_renderer.rb | 28 +++++++++++++++++-- decidim-core/config/locales/en.yml | 2 ++ decidim-core/lib/decidim/diffy_extension.rb | 18 ++++++++++++ .../decidim/meetings/diff_renderer_spec.rb | 18 ++++++++++-- .../collaborative_draft_diff_renderer.rb | 22 +++++++++++++++ .../decidim/proposals/diff_renderer.rb | 2 ++ .../system/amendable/amendment_diff_spec.rb | 28 +++++++++---------- .../spec/system/proposals_versions_spec.rb | 12 ++++---- 10 files changed, 116 insertions(+), 28 deletions(-) create mode 100644 decidim-proposals/app/services/decidim/proposals/collaborative_draft_diff_renderer.rb diff --git a/decidim-accountability/spec/services/decidim/accountability/diff_renderer_spec.rb b/decidim-accountability/spec/services/decidim/accountability/diff_renderer_spec.rb index 278026b2a5625..481177310103d 100644 --- a/decidim-accountability/spec/services/decidim/accountability/diff_renderer_spec.rb +++ b/decidim-accountability/spec/services/decidim/accountability/diff_renderer_spec.rb @@ -26,7 +26,7 @@ it "calculates the fields that have changed" do expect(subject.keys) - .to contain_exactly(:title_en, :description_ca, :progress, :start_date) + .to contain_exactly(:title_en, :description_ca, :description_machine_translations_es, :progress, :start_date, :title_machine_translations_es) end it "has the old and new values for each field" do @@ -37,9 +37,11 @@ it "has the type of each field" do expected_types = { description_ca: :i18n_html, + description_machine_translations_es: :i18n_html, progress: :percentage, start_date: :date, - title_en: :i18n + title_en: :i18n, + title_machine_translations_es: :i18n } types = subject.to_h { |attribute, data| [attribute.to_sym, data[:type]] } expect(types).to eq expected_types @@ -48,9 +50,11 @@ it "generates the labels correctly" do expected_labels = { description_ca: "Description (Català)", + description_machine_translations_es: "Description (automatic translation in Castellano)", progress: "Progress", start_date: "Start date", - title_en: "Title (English)" + title_en: "Title (English)", + title_machine_translations_es: "Title (automatic translation in Castellano)" } labels = subject.to_h { |attribute, data| [attribute.to_sym, data[:label]] } expect(labels).to eq expected_labels diff --git a/decidim-core/app/cells/decidim/diff_cell.rb b/decidim-core/app/cells/decidim/diff_cell.rb index d2d76214d0937..cfdb411668e47 100644 --- a/decidim-core/app/cells/decidim/diff_cell.rb +++ b/decidim-core/app/cells/decidim/diff_cell.rb @@ -70,6 +70,10 @@ def item # DiffRenderer class for the current_version's item; falls back to `BaseDiffRenderer`. def diff_renderer_class + renderer_class = "#{current_version.item_type}DiffRenderer".safe_constantize + + return renderer_class if renderer_class + if current_version.item_type.deconstantize == "Decidim" "#{current_version.item_type.pluralize}::DiffRenderer".constantize else diff --git a/decidim-core/app/services/decidim/base_diff_renderer.rb b/decidim-core/app/services/decidim/base_diff_renderer.rb index 7552f8575960f..a85eb6be6c856 100644 --- a/decidim-core/app/services/decidim/base_diff_renderer.rb +++ b/decidim-core/app/services/decidim/base_diff_renderer.rb @@ -41,7 +41,7 @@ def attribute_types end def parse_i18n_changeset(attribute, values, type, diff) - values.last.keys.each do |locale, _value| + (values.last.keys - ["machine_translations"]).each do |locale, _value| first_value = values.first.try(:[], locale) last_value = values.last.try(:[], locale) next if first_value == last_value @@ -56,6 +56,27 @@ def parse_i18n_changeset(attribute, values, type, diff) } ) end + + return diff unless values.last.has_key?("machine_translations") + + values.last.fetch("machine_translations").each_key do |locale, _value| + next unless I18n.available_locales.include?(locale.to_sym) + + first_value = values.first.try(:[], "machine_translations").try(:[], locale) + last_value = values.last.try(:[], "machine_translations").try(:[], locale) + + attribute_locale = :"#{attribute}_machine_translations_#{locale}" + + diff.update( + attribute_locale => { + type:, + label: generate_i18n_label(attribute, locale, "decidim.machine_translations.automatic"), + old_value: first_value, + new_value: last_value + } + ) + end + diff end @@ -108,7 +129,8 @@ def parse_changeset(attribute, values, type, diff) end # Returns a String. - def generate_i18n_label(attribute, locale) + # i18n-tasks-use t("decidim.machine_translations.automatic") + def generate_i18n_label(attribute, locale, postfix = "") label = I18n.t(attribute, scope: i18n_scope) locale_name = if I18n.available_locales.include?(locale.to_sym) I18n.t("locale.name", locale:) @@ -116,6 +138,8 @@ def generate_i18n_label(attribute, locale) locale end + locale_name = I18n.t(postfix, locale_name:, locale:) if postfix.present? + "#{label} (#{locale_name})" end diff --git a/decidim-core/config/locales/en.yml b/decidim-core/config/locales/en.yml index 9a6dc57669a13..b2df071d0100a 100644 --- a/decidim-core/config/locales/en.yml +++ b/decidim-core/config/locales/en.yml @@ -1091,6 +1091,8 @@ en: not_found: 'The scope was not found on the database (ID: %{id})' scope_type_presenter: not_found: 'The scope type was not found on the database (ID: %{id})' + machine_translations: + automatic: automatic translation in %{locale_name} managed_users: expired_session: The current administration session of a participant has expired. map: diff --git a/decidim-core/lib/decidim/diffy_extension.rb b/decidim-core/lib/decidim/diffy_extension.rb index e8acb71be319b..62c66367e279d 100644 --- a/decidim-core/lib/decidim/diffy_extension.rb +++ b/decidim-core/lib/decidim/diffy_extension.rb @@ -13,6 +13,24 @@ def to_s str = wrap_lines(@diff.map { |line| wrap_line(line) }) ActionView::Base.new(ActionView::LookupContext.new(nil), {}, nil).sanitize(str, tags: TAGS) end + + private + + def wrap_line(line) + cleaned = clean_line(line) + case line + when /^(---|\+\+\+|\\\\)/ + "
  • #{line.chomp}
  • " + when /^\+/ + "
  • #{cleaned}
  • " + when /^-/ + "
  • #{cleaned}
  • " + when /^ / + "
  • #{cleaned}
  • " + when /^@@/ + "
  • #{line.chomp}
  • " + end + end end # Adding a new method to Diffy::Format so we can pass the diff --git a/decidim-meetings/spec/services/decidim/meetings/diff_renderer_spec.rb b/decidim-meetings/spec/services/decidim/meetings/diff_renderer_spec.rb index 6b947637e5f9b..067f4788c8042 100644 --- a/decidim-meetings/spec/services/decidim/meetings/diff_renderer_spec.rb +++ b/decidim-meetings/spec/services/decidim/meetings/diff_renderer_spec.rb @@ -37,7 +37,11 @@ it "calculates the fields that have changed" do expect(subject.keys) - .to contain_exactly(:title_en, :description_ca, :address, :location_ca, :location_en, :location_hints_ca, :location_hints_en, :start_time, :end_time, :decidim_scope_id) + .to contain_exactly( + :title_en, :description_ca, :address, :location_ca, :location_en, :location_hints_ca, :location_hints_en, + :start_time, :end_time, :decidim_scope_id, :description_machine_translations_es, + :location_hints_machine_translations_es, :location_machine_translations_es, :title_machine_translations_es + ) end it "has the old and new values for each field" do @@ -76,14 +80,18 @@ expected_types = { title_en: :i18n, description_ca: :i18n_html, + description_machine_translations_es: :i18n_html, address: :string, location_ca: :i18n, location_en: :i18n, location_hints_ca: :i18n, location_hints_en: :i18n, + location_hints_machine_translations_es: :i18n, + location_machine_translations_es: :i18n, start_time: :date, end_time: :date, - decidim_scope_id: :scope + decidim_scope_id: :scope, + title_machine_translations_es: :i18n } types = subject.to_h { |attribute, data| [attribute.to_sym, data[:type]] } expect(types).to eq expected_types @@ -93,14 +101,18 @@ expected_labels = { title_en: "Title (English)", description_ca: "Description (Català)", + description_machine_translations_es: "Description (automatic translation in Castellano)", address: "Address", location_ca: "Location (Català)", location_en: "Location (English)", location_hints_ca: "Location hints (Català)", location_hints_en: "Location hints (English)", + location_hints_machine_translations_es: "Location hints (automatic translation in Castellano)", + location_machine_translations_es: "Location (automatic translation in Castellano)", start_time: "Start time", end_time: "End time", - decidim_scope_id: "Scope" + decidim_scope_id: "Scope", + title_machine_translations_es: "Title (automatic translation in Castellano)" } labels = subject.to_h { |attribute, data| [attribute.to_sym, data[:label]] } expect(labels).to eq expected_labels diff --git a/decidim-proposals/app/services/decidim/proposals/collaborative_draft_diff_renderer.rb b/decidim-proposals/app/services/decidim/proposals/collaborative_draft_diff_renderer.rb new file mode 100644 index 0000000000000..3994e4e02775d --- /dev/null +++ b/decidim-proposals/app/services/decidim/proposals/collaborative_draft_diff_renderer.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Decidim + module Proposals + class CollaborativeDraftDiffRenderer < DiffRenderer + private + + def attribute_types + { + title: :string, + body: :string, + decidim_category_id: :category, + decidim_scope_id: :scope, + address: :string, + latitude: :string, + longitude: :string, + decidim_proposals_proposal_state_id: :string + } + end + end + end +end diff --git a/decidim-proposals/app/services/decidim/proposals/diff_renderer.rb b/decidim-proposals/app/services/decidim/proposals/diff_renderer.rb index c9225e574a7e0..4c577da199c75 100644 --- a/decidim-proposals/app/services/decidim/proposals/diff_renderer.rb +++ b/decidim-proposals/app/services/decidim/proposals/diff_renderer.rb @@ -21,8 +21,10 @@ def attribute_types # Parses the values before parsing the changeset. def parse_changeset(attribute, values, type, diff) + return parse_i18n_changeset(attribute, values, type, diff) if [:i18n, :i18n_html].include?(type) return parse_scope_changeset(attribute, values, type, diff) if type == :scope return parse_state_changeset(attribute, values, type, diff) if type == :state + return parse_user_group_changeset(attribute, values, type, diff) if type == :user_group values = parse_values(attribute, values) old_value = values[0] diff --git a/decidim-proposals/spec/system/amendable/amendment_diff_spec.rb b/decidim-proposals/spec/system/amendable/amendment_diff_spec.rb index c6d721c3e0d6a..ed41603dba231 100644 --- a/decidim-proposals/spec/system/amendable/amendment_diff_spec.rb +++ b/decidim-proposals/spec/system/amendable/amendment_diff_spec.rb @@ -30,11 +30,11 @@ it "shows the changed attributes compared to the last version of the amended proposal" do expect(page).to have_content('Amendment to "Updated long enough title"') - within "#diff-for-title" do + within "#diff-for-title-english" do expect(page).to have_content("Title") within ".diff > ul > .del" do - expect(page).to have_content("Updated long enough title") + expect(page).to have_content("Original long enough title") end within ".diff > ul > .ins" do @@ -42,11 +42,11 @@ end end - within "#diff-for-body" do + within "#diff-for-body-english" do expect(page).to have_content("Body") within ".diff > ul > .del" do - expect(page).to have_content("Updated one liner body") + expect(page).to have_content("Original one liner body") end within ".diff > ul > .ins" do @@ -68,7 +68,7 @@ it "shows the changed attributes compared to the version of the amended proposal at the moment of making the amendment" do expect(page).to have_content('Amendment to "Updated long enough title"') - within "#diff-for-title" do + within "#diff-for-title-english" do expect(page).to have_content("Title") within ".diff > ul > .del" do @@ -80,7 +80,7 @@ end end - within "#diff-for-body" do + within "#diff-for-body-english" do expect(page).to have_content("Body") within ".diff > ul > .del" do @@ -105,14 +105,14 @@ it "shows NO changes in the body" do expect(page).to have_content('Amendment to "Original long enough title"') - within "#diff-for-body" do + within "#diff-for-body-english" do expect(page).to have_content("Body") - within all(".diff > ul > .unchanged").first do + within all(".diff > ul > .ins").first do expect(page).to have_content("One liner body") end - within all(".diff > ul > .unchanged").last do + within all(".diff > ul > .ins").last do expect(page).to have_content("Amended") end end @@ -143,12 +143,12 @@ visit decidim.review_amend_path(amendment) end - it "shows the changed attributes compared to the last version of the amended proposal" do - within "#diff-for-title" do + it "shows the changed attributes compared to the last version of the amended proposal", versioning: true do + within "#diff-for-title-english" do expect(page).to have_content("Title") within ".diff > ul > .del" do - expect(page).to have_content("Updated long enough title") + expect(page).to have_content("Original long enough title") end within ".diff > ul > .ins" do @@ -156,11 +156,11 @@ end end - within "#diff-for-body" do + within "#diff-for-body-english" do expect(page).to have_content("Body") within ".diff > ul > .del" do - expect(page).to have_content("Updated one liner body") + expect(page).to have_content("Original one liner body") end within ".diff > ul > .ins" do diff --git a/decidim-proposals/spec/system/proposals_versions_spec.rb b/decidim-proposals/spec/system/proposals_versions_spec.rb index d3778de35e45b..ce824264d44a9 100644 --- a/decidim-proposals/spec/system/proposals_versions_spec.rb +++ b/decidim-proposals/spec/system/proposals_versions_spec.rb @@ -78,20 +78,20 @@ it "shows the changed attributes" do expect(page).to have_content("Changes at") - within "#diff-for-title" do - expect(page).to have_content("Title") + within "#diff-for-title-english" do + expect(page).to have_content("Title (English)") within ".diff > ul > .del" do - expect(page).to have_content(translated(proposal.title).dump) + expect(page).to have_content(translated(proposal.title)) end within ".diff > ul > .ins" do - expect(page).to have_content(translated(emendation.title).dump) + expect(page).to have_content(translated(emendation.title)) end end - within "#diff-for-body" do - expect(page).to have_content("Body") + within "#diff-for-body-english" do + expect(page).to have_content("Body (English)") within ".diff > ul > .del" do expect(page).to have_content(translated(proposal.body))