Skip to content

Commit 8b6ccce

Browse files
committed
[WHIT-2724] Fix minor inconsistencies in block_content
Make the data that is stored in block_content consistent with the original translations that it is being copied from. Instead of directly getting the values from the edition, we instead use the translation, so that we avoid getting fallback values and including data that is not part of the original translation, or only relevant to the primary translation. Also ensures we can't save anything in the `body` column for a StandardEdition, as we only care about the value in block_content. This will hopefully prevent any confusion about where values originate from.
1 parent cdd99f2 commit 8b6ccce

3 files changed

Lines changed: 260 additions & 4 deletions

File tree

app/models/concerns/edition/translatable.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ class Trait < Edition::Traits::Trait
55
def process_associations_before_save(edition)
66
@edition.translations.each do |translation|
77
I18n.with_locale(translation.locale) do
8-
edition.title = @edition.title
9-
edition.summary = @edition.summary
10-
edition.body = @edition.body
11-
edition.block_content = @edition.block_content
8+
edition.title = translation.title
9+
edition.summary = translation.summary
10+
edition.body = translation.body
11+
edition.write_attribute(:block_content, translation.block_content)
1212
end
1313
end
1414
end

app/models/standard_edition.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ def body
4343
block_content["body"]
4444
end
4545

46+
def body=(_)
47+
nil
48+
end
49+
4650
def can_set_previously_published?
4751
type_instance.settings["backdating_enabled"]
4852
end
Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
require "test_helper"
2+
3+
class StandardEditionTranslationBlockContentConsistencyTest < ActiveSupport::TestCase
4+
def english_body_content = "English body content"
5+
def translated_body_content(locale) = "Body content for #{locale}"
6+
7+
setup do
8+
ConfigurableDocumentType.setup_test_types(
9+
build_configurable_document_type(
10+
"test", {
11+
"schema" => {
12+
"properties" => {
13+
"body" => { "type" => "string", "format" => "govspeak", "title" => "Body" },
14+
"image" => { "type" => "integer", "format" => "lead_image_select", "title" => "Lead image" },
15+
},
16+
},
17+
"settings" => { "translations_enabled" => true },
18+
}
19+
),
20+
)
21+
end
22+
23+
test "creating a new draft from a published StandardEdition with translations should not duplicate old body field data" do
24+
translated_edition = create_standard_edition(translate_for: %w[cy])
25+
26+
new_draft = translated_edition.create_draft(create(:writer))
27+
28+
english_translation = new_draft.translation_for(:en)
29+
welsh_translation = new_draft.translation_for(:cy)
30+
31+
assert_nil english_translation[:body],
32+
"English translation body column should be nil, but got: #{english_translation[:body]}"
33+
assert_nil welsh_translation[:body],
34+
"Welsh translation body column should be nil, but got: #{welsh_translation[:body]}"
35+
36+
assert_equal english_body_content, english_translation.block_content["body"],
37+
"English body should be in block_content.body"
38+
assert_equal translated_body_content("cy"), welsh_translation.block_content["body"],
39+
"Welsh body should be in block_content.body"
40+
end
41+
42+
test "editions without images should not have image key in block_content" do
43+
translated_edition = create_standard_edition(translate_for: %w[cy])
44+
45+
assert_not_includes translated_edition.translation_for(:en).block_content.keys, "image",
46+
"English translation should not have image key"
47+
assert_not_includes translated_edition.translation_for(:cy).block_content.keys, "image",
48+
"Welsh translation should not have image key"
49+
50+
new_draft = translated_edition.create_draft(create(:writer))
51+
52+
assert_not_includes new_draft.translation_for(:en).block_content.keys, "image",
53+
"Draft English translation should not have image key"
54+
assert_not_includes new_draft.translation_for(:cy).block_content.keys, "image",
55+
"Draft Welsh translation should not have image key"
56+
end
57+
58+
test "primary translation with image should not clone image to non-primary translations" do
59+
standard_edition = create_standard_edition(with_image: true)
60+
create_translated_edition(standard_edition, locale: "cy", with_image: false)
61+
62+
english_translation = standard_edition.translation_for(:en)
63+
welsh_translation = standard_edition.translation_for(:cy)
64+
image_id = english_translation.block_content["image"]
65+
66+
assert_equal english_body_content, english_translation.block_content["body"],
67+
"English body should have correct content"
68+
assert_equal translated_body_content("cy"), welsh_translation.block_content["body"],
69+
"Welsh body should have correct content"
70+
assert_not_includes welsh_translation.block_content.keys, "image",
71+
"Welsh translation should not have image key"
72+
73+
new_draft = standard_edition.create_draft(create(:writer))
74+
75+
assert_equal image_id, new_draft.translation_for(:en).block_content["image"],
76+
"Draft English translation should have image value"
77+
assert_equal english_body_content, new_draft.translation_for(:en).block_content["body"],
78+
"Draft English body should have correct content"
79+
80+
assert new_draft.translation_for(:cy).block_content["image"].blank?,
81+
"Draft Welsh translation should not have a value but found Image ID: #{new_draft.translation_for(:cy).block_content['image']}"
82+
assert_equal translated_body_content("cy"), new_draft.translation_for(:cy).block_content["body"],
83+
"Draft Welsh body should have correct content"
84+
end
85+
86+
test "translated edition with image should not clone to primary translation" do
87+
standard_edition = create_standard_edition(with_image: false)
88+
create_translated_edition(standard_edition, locale: "cy", with_image: true)
89+
90+
english_translation = standard_edition.translation_for(:en)
91+
welsh_translation = standard_edition.translation_for(:cy)
92+
image_id = welsh_translation.block_content["image"]
93+
94+
new_draft = standard_edition.create_draft(create(:writer))
95+
96+
assert english_translation.block_content["image"].blank?,
97+
"Draft English translation should not have image value"
98+
assert_equal english_body_content, new_draft.translation_for(:en).block_content["body"],
99+
"Draft English body should have correct content"
100+
101+
assert_equal image_id, new_draft.translation_for(:cy).block_content["image"],
102+
"Draft Welsh translation should have image value"
103+
assert_equal translated_body_content("cy"), new_draft.translation_for(:cy).block_content["body"],
104+
"Draft Welsh body should have correct content"
105+
end
106+
107+
test "new draft editions with images get the translated image IDs from the parent" do
108+
standard_edition = create_standard_edition(with_image: true)
109+
create_translated_edition(standard_edition, locale: "cy", with_image: true)
110+
111+
english_image_id = standard_edition.translation_for(:en).block_content["image"]
112+
welsh_image_id = standard_edition.translation_for(:cy).block_content["image"]
113+
114+
new_draft = standard_edition.create_draft(create(:writer))
115+
116+
assert_equal english_image_id, new_draft.translation_for(:en).block_content["image"],
117+
"Draft English translation should have image value"
118+
assert_equal english_body_content, new_draft.translation_for(:en).block_content["body"],
119+
"Draft English body should have correct content"
120+
121+
assert_equal welsh_image_id, new_draft.translation_for(:cy).block_content["image"],
122+
"Draft Welsh translation should have image value"
123+
assert_equal translated_body_content("cy"), new_draft.translation_for(:cy).block_content["body"],
124+
"Draft Welsh body should have correct content"
125+
end
126+
127+
test "new draft with multiple translations each having images retains correct data per locale" do
128+
standard_edition = create_standard_edition(with_image: true)
129+
create_translated_edition(standard_edition, locale: "cy", with_image: true)
130+
create_translated_edition(standard_edition, locale: "fr", with_image: true)
131+
132+
english_image_id = standard_edition.translation_for(:en).block_content["image"]
133+
welsh_image_id = standard_edition.translation_for(:cy).block_content["image"]
134+
french_image_id = standard_edition.translation_for(:fr).block_content["image"]
135+
136+
# Ensure all images are distinct
137+
assert_not_equal english_image_id, welsh_image_id, "English and Welsh should have different images"
138+
assert_not_equal english_image_id, french_image_id, "English and French should have different images"
139+
assert_not_equal welsh_image_id, french_image_id, "Welsh and French should have different images"
140+
141+
new_draft = standard_edition.create_draft(create(:writer))
142+
143+
# Verify each locale retains its own image
144+
assert_equal english_image_id, new_draft.translation_for(:en).block_content["image"],
145+
"Draft English translation should have its original image"
146+
assert_equal welsh_image_id, new_draft.translation_for(:cy).block_content["image"],
147+
"Draft Welsh translation should have its original image"
148+
assert_equal french_image_id, new_draft.translation_for(:fr).block_content["image"],
149+
"Draft French translation should have its original image"
150+
151+
# Verify body content is also correct per locale
152+
assert_equal english_body_content, new_draft.translation_for(:en).block_content["body"],
153+
"Draft English body should have correct content"
154+
assert_equal translated_body_content("cy"), new_draft.translation_for(:cy).block_content["body"],
155+
"Draft Welsh body should have correct content"
156+
assert_equal translated_body_content("fr"), new_draft.translation_for(:fr).block_content["body"],
157+
"Draft French body should have correct content"
158+
end
159+
160+
test "partial updates to block_content merge correctly within primary translation" do
161+
draft_standard_edition = create_standard_edition(with_image: false, factory_name: :draft_standard_edition)
162+
163+
I18n.with_locale(:en) do
164+
draft_standard_edition.block_content = { "image" => 123 }
165+
draft_standard_edition.save!
166+
end
167+
168+
draft_standard_edition.reload
169+
primary_translation = draft_standard_edition.translation_for(:en)
170+
171+
assert_equal english_body_content, primary_translation.block_content["body"],
172+
"Body should be preserved after partial update"
173+
assert_equal 123, primary_translation.block_content["image"],
174+
"Image should be added via partial update"
175+
end
176+
177+
test "partial updates to block_content merge correctly within secondary translation" do
178+
draft_standard_edition = create_standard_edition(with_image: false, factory_name: :draft_standard_edition)
179+
create_translated_edition(draft_standard_edition, locale: "cy", with_image: false)
180+
181+
I18n.with_locale(:cy) do
182+
draft_standard_edition.block_content = { "image" => 123 }
183+
draft_standard_edition.save!
184+
end
185+
186+
draft_standard_edition.reload
187+
secondary_translation = draft_standard_edition.translation_for(:cy)
188+
189+
assert_equal translated_body_content(:cy), secondary_translation.block_content["body"],
190+
"Body should be preserved after partial update"
191+
assert_equal 123, secondary_translation.block_content["image"],
192+
"Image should be added via partial update"
193+
end
194+
195+
test "partial updates to block_content in translated locales do not affect primary translation" do
196+
draft_standard_edition = create_standard_edition(with_image: false, factory_name: :draft_standard_edition)
197+
create_translated_edition(draft_standard_edition, locale: "cy", with_image: false)
198+
199+
I18n.with_locale(:cy) do
200+
draft_standard_edition.block_content = { "body" => "this is the updated translated version" }
201+
draft_standard_edition.save!
202+
end
203+
204+
draft_standard_edition.reload
205+
206+
primary_translation = draft_standard_edition.translation_for(:en)
207+
assert_equal english_body_content, primary_translation.block_content["body"],
208+
"Body should be preserved after partial update"
209+
end
210+
211+
test "partial updates to block_content in primary locale do not affect secondary translations" do
212+
draft_standard_edition = create_standard_edition(with_image: false, factory_name: :draft_standard_edition)
213+
create_translated_edition(draft_standard_edition, locale: "cy", with_image: false)
214+
215+
I18n.with_locale(:en) do
216+
draft_standard_edition.block_content = { "body" => "this is the updated english version" }
217+
draft_standard_edition.save!
218+
end
219+
220+
draft_standard_edition.reload
221+
222+
secondary_translation = draft_standard_edition.translation_for(:cy)
223+
assert_equal translated_body_content(:cy), secondary_translation.block_content["body"],
224+
"Body should be preserved after partial update"
225+
end
226+
227+
private
228+
229+
def create_standard_edition(with_image: false, translate_for: [], factory_name: :published_standard_edition)
230+
block_content = { "body" => english_body_content }
231+
block_content["image"] = create(:image).image_data.id if with_image
232+
233+
standard_edition = create(
234+
factory_name,
235+
configurable_document_type: "test",
236+
title: "English title",
237+
summary: "English summary",
238+
block_content:,
239+
)
240+
241+
translate_for.each { |locale| create_translated_edition(standard_edition, locale:) }
242+
243+
standard_edition
244+
end
245+
246+
def create_translated_edition(standard_edition, locale:, with_image: false)
247+
block_content = { "body" => translated_body_content(locale) }
248+
block_content["image"] = create(:image).image_data.id if with_image
249+
250+
I18n.with_locale(locale) { standard_edition.translations.create!(locale:, block_content:) }
251+
end
252+
end

0 commit comments

Comments
 (0)