Skip to content

Commit

Permalink
Backport 'Fix the Diff Render output' to v0.29 (decidim#13752)
Browse files Browse the repository at this point in the history
  • Loading branch information
alecslupu authored Dec 12, 2024
1 parent 74efe47 commit 2b3e962
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions decidim-core/app/cells/decidim/diff_cell.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 26 additions & 2 deletions decidim-core/app/services/decidim/base_diff_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -108,14 +129,17 @@ 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:)
else
locale
end

locale_name = I18n.t(postfix, locale_name:, locale:) if postfix.present?

"#{label} (#{locale_name})"
end

Expand Down
2 changes: 2 additions & 0 deletions decidim-core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 18 additions & 0 deletions decidim-core/lib/decidim/diffy_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 /^(---|\+\+\+|\\\\)/
" <li class=\"diff-comment\"><div>#{line.chomp}</div></li>"
when /^\+/
" <li class=\"ins\"><ins>#{cleaned}</ins></li>"
when /^-/
" <li class=\"del\"><del>#{cleaned}</del></li>"
when /^ /
" <li class=\"unchanged\"><div>#{cleaned}</div></li>"
when /^@@/
" <li class=\"diff-block-info\"><div>#{line.chomp}</div></li>"
end
end
end

# Adding a new method to Diffy::Format so we can pass the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
28 changes: 14 additions & 14 deletions decidim-proposals/spec/system/amendable/amendment_diff_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,23 @@
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
expect(page).to have_content("Amended long enough title")
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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -143,24 +143,24 @@
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
expect(page).to have_content("Amended long enough title")
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
Expand Down
12 changes: 6 additions & 6 deletions decidim-proposals/spec/system/proposals_versions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit 2b3e962

Please sign in to comment.