Skip to content

Conversation

@jamiestamp
Copy link
Contributor

@jamiestamp jamiestamp commented Dec 23, 2025

What

This PR refactors the schema design for config-driven document types by separating concerns that were previously conflated under edit_screens. We now:

  • removed settings.edit_screens, in favour of a new top-level forms hash. It describes things at the UI level: which fields appear on which forms.
  • adds new top-level presenters field, describing which field values get sent to Publishing API (and how to map them)
  • removes schema.properties in favour of schema.attributes: effectively a slimmed down version of the former.
  • schema.validations remains unchanged, but we no longer have 'nested' validation (e.g. schema.properties.duration.validations) - the 'nested' validation has moved up into the schema.validations (in this example, schema.validations.duration). This isn't represented in the overview below as we only had one example and it's more of a side-effect than a deliberate change.

Overview of schema changes:

"schema": {
-  "properties": {
-    "body": {
-      "title": "Body",
-      "type": "string",
-      "format": "govspeak"
-    }
+  "attributes": {
+    "body": {
+      "type": "string",
+    }
+  }
}
+ "forms": {
+  "documents": {
+    "fields": {
+      "body": {
+        "title": "Body",
+        "description": "The main content of the page",
+        "block": "govspeak"
+      }
+    }
+  }
+ },
+ "presenters": {
+   "publishing_api": {
+    "body": "govspeak"
+  }
+}
"settings": {
-  "edit_screens": {
-   "document": ["body"]
- }

In order to not break anything, we're following the expand-and-contract methodology to support both the old and new schema kinds, branching our templates with conditional logic and duplicating our tests to exercise both.

Once merged, we'll migrate each standard edition config file in isolation, from the old format to the new. We'll then be able to delete the branching and the original tests.

Why

Bunching up properties into 'edit screens' was a good first step in #10786, allowing us to update different block content fields in isolation on separate tabs. But it was too restrictive: we had no way of denoting which fields should be 'grouped' (e.g. start & end date fields for an overall 'duration') and also we were having to wrestle with wanting to present the fields in one way but present them differently when presenting to Publishing API. The previous properties and edit_screens concepts were conflating too much.

The refactoring of the schemas we've done here should make it quicker and easier to iterate document types in future.

Additional notes

We have converted Topical Events to use the new forms schema. Once this is deployed and with sense checks done, we will migrate the other existing configurable document types.

Jira: https://gov-uk.atlassian.net/browse/WHIT-2788


⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

This application is owned by the Whitehall Experience team. Please let us know in #govuk-whitehall-experience-tech when you raise any PRs.

Follow these steps if you are doing a Rails upgrade.

@jamiestamp jamiestamp marked this pull request as draft December 23, 2025 10:18
@jamiestamp jamiestamp changed the title Configurable document form objects [WHIT-2788] Configurable document form objects Dec 23, 2025
@jamiestamp jamiestamp force-pushed the configurable-document-form-objects branch 2 times, most recently from 2371e0a to d7196f7 Compare December 23, 2025 10:47
@eYinka eYinka force-pushed the configurable-document-form-objects branch 2 times, most recently from 178592f to 97f4f3b Compare January 2, 2026 10:58
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, the Render standard edition with a new schema format commit is quite hefty! Is there any way it can be broken up? Also that commit in general could do with a bit of a rewrite of the message (to rewrite confusing shorthand like "this includes st ed controller").

Currently I'm putting a lot of faith in the idea that tests have been copied over verbatim, with only the 'setup' code tweaked accordingly. It's a shame there's no way we can configure Minitest to just run the same exact existing tests twice but with two different setups. (Like RSpec's it_behaves_like de-duplicator?)

But as an overall approach, I like it 👍 good job, folks! 👏

eYinka and others added 6 commits January 5, 2026 13:18
This ensures we can define a different type setup in future commits, one that is aware of new properties such as `presenters` and `forms`.
A schema that includes `attributes` specifying `type` and `validations`, rather than a hash of properties.

This will help us separate the model schema from what we want to render, and send to publishing API, respectively.
`build_block` will be used in future commits to branch block building logic.

It builds uniquely named blocks and abandons the `type > format` nested structure we had in the `blocks` method.

Due to future changes to how we compute pub api payloads and schema shape, the `wrapper` object has been dropped in the new method `build_block` as it will be no longer needed.
This now replaces the need for all the `publishing_api_payload` methods defined on all the config driven blocks. There is no need to define a payload behaviour for the `default_object` block, as we always send a flat details hash to publishing API. Only "leaf" blocks will be included in the payload.

We plan to maintain both implementations in the code to de-risk the release. At a later point, we will be removing all the `publishing_api_payload` methods and only use the new flow.

All the tests covering the blocks' `publishing_api_payload` behaviour have been ported over.
We want to specify which attributes we're going to be including in the presenter, separately from defining the model attributes, or the forms to render.

Note that the check for empty on the details hash is not very precise. We could in theory have blank `details`, if there is no content to send, or if the `presenters` hash is blank (does not specify any `attributes`), but in reality we always require at least one block in the details hash for our existing types.
@eYinka eYinka force-pushed the configurable-document-form-objects branch 2 times, most recently from fb00d11 to aaa2b99 Compare January 5, 2026 16:56
lauraghiorghisor-tw and others added 3 commits January 5, 2026 17:43
- Test all block types in feature tests for both schema formats (with, and without forms)
- Branch render logic to support both schema formats (with forms, and without). Where we encounter a schema style that uses forms, we render "old" style blocks (using the `build` method), otherwise we use the `build_block` method.
- We now "skip" nested objects when rendering; this means that whereas before the form name would have looked like `edition[block_content][wrapper_object][leaf_block]`, it now looks like `edition[block_content][leaf_block]`.
- The way we compose the path when rendering child blocks in the new schema style has also changed.
- Rendering the "required" labels require accessing the `validations`. Validations now live in a different segment of the schema; previously we were fetching them straight from the top-level `schema`; now the "schema" passed for rendering a block only contains `fields` with title/block/description. This was fixed by passing the array of required fields through any `default_object` block, so it's available down the tree. We compute the required fields values in the `ConfigurableDocumentType` model - `required_attributes` method.
- Duplicate all block tests + added required behaviours in the form views (example in app/views/admin/standard_editions/_form.html.erb#L76 and app/views/admin/configurable_content_blocks/_default_object.html.erb#L20)
- The required attrs are now being passed for the images and the translations edit form. As-is behaviour was missing that too
- Duplicated test/components/admin/editions/language_select_form_control_test.rb to cover for the new schema builder with forms
- Checked that we have not missed duplicating some of the tests "with forms" -> really the main thing is to support a nested object like "address" - but some tests don't need to cover that
- Duplicated Standard edition controller tests -> this includes standard edition controller, edition translations controller or other places where we use the old builder build_configurable_document_type -> should also validate that we use the new builder (build_configurable_document_type_with_forms)
- Duplicated all relevant tests from StandardEditionTest that are affected by the schema change

Notes:

- Some behaviour for newly introduced nested attribute was broken for as is -> we do not want to fix these though, since we are replacing the schema and the tests pass in the new world -> Removed todos
- Some tests have "with forms" names on some of the steps - probably good enough for now, we just need to be able to differentiate the legacy steps somehow. We will eventually get rid of the "with forms" after removing the old schema code anyway.
- In some tests, you'll find "contexts" to seperate the old world from the new world - mostly in controller tests. Otherwise, you'll find an entirely new class with "WithForms" prefix. This should help us easily get rid of unused code once we retire the old schema's code.
- In `test/unit/app/sidekiq/standard_edition_migrator_worker_test.rb`, we have directly used the `with_forms` builder because the migrator worker isn't in active usage. So it's probably fine to let it use the new way of doing things.

Co-authored-by:       Olayinka Oladele <[email protected]>
Co-authored-by:       Jamie Stamp <[email protected]>
…ith attributes

Included new schema style rules:
- simultaneously support old schema style (`schema` with `properties`) and new style (`forms` with `fields`; `schema` with `attributes` and `validations`; `presenters`)
- validate that the `presenters` values map to existing builder methods
Also removed the wrapper object as it is not used anywhere anymore.
@eYinka eYinka force-pushed the configurable-document-form-objects branch from aaa2b99 to 2bcd3a7 Compare January 5, 2026 17:43
@eYinka eYinka marked this pull request as ready for review January 5, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants