Skip to content

Commit 92988c6

Browse files
ChrisBAshtoneYinka
authored andcommitted
Remove obsolete 'nested' code
We tried testing setting an object value under the new 'flattened attributes' structure, and the object was only ever being set as `{}`. This is because we had special case handling for type "object" which is no longer necessary - we blindly set the value to the object, no type casting necessary. We also no longer have the concept of a nested 'validations' or nested 'schema' property on a nested attribute. All validations are declared at the root level now and can incorporate multiple attributes, as we're doing with topical event date validation: https://github.com/alphagov/whitehall/blob/25addb1ce63d150731560b7a99efc89e731ecf21/app/models/configurable_document_types/topical_event.json#L60-L62
1 parent 4401f72 commit 92988c6

3 files changed

Lines changed: 45 additions & 35 deletions

File tree

app/models/standard_edition/block_content.rb

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ class StandardEdition::BlockContent
44
include DateValidation
55

66
validate :valid_instance_of_document_type_attributes
7-
validate :valid_nested_attributes
87

98
VALIDATORS = {
109
"embedded_contacts_exist" => GovspeakContactEmbedValidator,
@@ -26,11 +25,7 @@ def attributes=(values)
2625
values = values.to_h
2726
@attributes_config.each do |key, nested_schema|
2827
setter = "#{key}="
29-
if nested_schema["type"] == "object"
30-
nested_attributes = self.class.new(nested_schema, @path.push(key))
31-
nested_attributes.assign_attributes(values[key])
32-
public_send(setter, nested_attributes)
33-
elsif nested_schema["type"] == "date"
28+
if nested_schema["type"] == "date"
3429
public_send(setter, pre_validate_date_attribute(key, values[key]))
3530
else
3631
public_send(setter, values[key])
@@ -57,19 +52,6 @@ def valid_instance_of_document_type_attributes
5752
end
5853
end
5954

60-
def valid_nested_attributes
61-
@attributes_config.each do |key, nested_schema|
62-
next unless nested_schema["type"] == "object"
63-
64-
nested_attribute_values = attributes.public_send(key)
65-
next if nested_attribute_values.valid?
66-
67-
nested_attribute_values.errors.each do |error|
68-
errors.import(error, { attribute: "#{@path.push(key).validation_error_attribute}.#{error.attribute}" })
69-
end
70-
end
71-
end
72-
7355
def method_missing(symbol, *args)
7456
if attributes.class.instance_methods.include?(symbol)
7557
attributes.public_send(symbol, *args)

test/unit/app/models/standard_edition_block_content_merge_test.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ class StandardEditionBlockContentMergeTest < ActiveSupport::TestCase
115115
"body" => { "type" => "string" },
116116
"summary" => { "type" => "string" },
117117
"audience" => { "type" => "string" },
118+
"meta" => { "type" => "object" },
118119
"count" => { "type" => "integer" },
119120
},
120121
},
@@ -128,6 +129,11 @@ class StandardEditionBlockContentMergeTest < ActiveSupport::TestCase
128129
"body" => "Start",
129130
"summary" => "old summary",
130131
"audience" => "public",
132+
"meta" => {
133+
"info" => "some info",
134+
"details" => "some details",
135+
"bool" => true,
136+
},
131137
"count" => 1,
132138
"junk" => "should be removed", # invalid per schema
133139
},
@@ -137,9 +143,14 @@ class StandardEditionBlockContentMergeTest < ActiveSupport::TestCase
137143
params = ActionController::Parameters.new(
138144
"block_content" => {
139145
"summary" => "new summary", # overwrite nested valid key
140-
"count" => 2, # overwrite nested-nested valid key
141-
"invalid_nested" => "ignore me", # invalid nested key
146+
"count" => 2, # overwrite nested-nested valid key
147+
"invalid_nested" => "ignore me", # invalid nested key
142148
"unknown" => "ignore me too", # invalid nested key at level 1
149+
"meta" => {
150+
"info" => "updated info",
151+
"details" => "updated details",
152+
"bool" => false,
153+
},
143154
"not_in_schema" => "nope", # invalid top-level key
144155
},
145156
).permit!
@@ -155,6 +166,12 @@ class StandardEditionBlockContentMergeTest < ActiveSupport::TestCase
155166
assert_equal "public", bc["audience"] # preserved
156167
assert_equal 2, bc["count"] # updated
157168

169+
assert_equal({
170+
"info" => "updated info",
171+
"details" => "updated details",
172+
"bool" => false,
173+
}, bc["meta"].to_h)
174+
158175
# invalid keys filtered out
159176
assert_nil bc["junk"]
160177
assert_nil bc["unknown"]

test/unit/app/models/standard_edition_test.rb

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,29 +156,40 @@ class StandardEditionTest < ActiveSupport::TestCase
156156
configurable_document_type =
157157
build_configurable_document_type(
158158
test_type, {
159-
"schema" => {
160-
"attributes" => {
161-
"test_object_attribute" => {
162-
"type" => "object",
163-
"attributes" => {
164-
"test_nested_attribute" => {
165-
"type" => "string",
166-
},
167-
},
168-
"validations" => {
169-
"presence" => {
170-
"attributes" => %w[test_nested_attribute],
159+
"forms" => {
160+
"documents" => {
161+
"fields" => {
162+
"test_object_attribute" => {
163+
"title" => "Test object attribute",
164+
"block" => "default_object",
165+
"fields" => {
166+
"test_nested_attribute" => {
167+
"title" => "Test nested attribute",
168+
"block" => "default_string",
169+
},
171170
},
172171
},
173172
},
174173
},
175174
},
175+
"schema" => {
176+
"attributes" => {
177+
"test_nested_attribute" => {
178+
"type" => "string",
179+
},
180+
},
181+
"validations" => {
182+
"presence" => {
183+
"attributes" => %w[test_nested_attribute],
184+
},
185+
},
186+
},
176187
}
177188
)
178189
ConfigurableDocumentType.setup_test_types(configurable_document_type)
179-
page = build(:standard_edition, { configurable_document_type: test_type, block_content: { test_object_attribute: { test_nested_attribute: "" } } })
190+
page = build(:standard_edition, { configurable_document_type: test_type, block_content: { test_nested_attribute: "" } })
180191
assert page.invalid?
181-
assert_not page.errors.where("test_object_attribute.test_nested_attribute", :blank).empty?
192+
assert_not page.errors.where("test_nested_attribute", :blank).empty?
182193
end
183194

184195
test "it allows translations if the configurable document type settings permit them" do

0 commit comments

Comments
 (0)