Skip to content

Commit 7a53e43

Browse files
ChrisBAshtonlauraghiorghisor-tw
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 f5ee6c0 commit 7a53e43

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)