Improve error message for social media [WHIT-3156] by GDSNewt · Pull Request #11360 · alphagov/whitehall

GDSNewt · 2026-04-14T12:47:55Z

Overview

This update improves the clarity, consistency, and usability of validation errors when publishers add social media accounts.

Why

Previously, validation feedback for social media accounts was unclear and inconsistent:

Errors only appeared in the summary, not inline
It wasn’t obvious which field (Channel or URL) needed fixing
Terminology used “service” instead of “channel”
Only one error appeared even when multiple fields were invalid

What Changed

Validation behaviour and messaging have been enhanced:

Terminology updated from service → channel to match the UI
Errors are now attached to individual fields (Channel, URL) instead of a shared attribute
Errors display both inline and in the summary
Validation runs independently per field
Multiple errors are shown when applicable (e.g. both fields blank)
Invalid fields are highlighted with a red border

ACs

Meets the ACs as described in the Jira ticket

UI Error Examples

1. Both fields blank

2. Channel selected, URL missing

3. URL entered, channel missing

4. Multiple accounts, mixed errors

Alex attempted the nested validation approach in #11381.

It worked, but we discussed as a team in Sit Down recently. As per ADR 009, we recently made the decision to move away from 'nested' validation as it added a lot of complexity for little benefit. We felt that writing a custom validator and declaring it at the root "validations" property level was the right level of abstraction / complexity at this stage.

Therefore we're re-exploring #11360 as the solution to better error messages for the array block.

ChrisBAshton

I'm as keen to get this over the line as you are :D let's see if we can remove some of the code smells first - comments below.

ChrisBAshton

Getting there 👏

ChrisBAshton · 2026-05-05T15:03:37Z

 private

+  def find_field_title(config, segments)
+    return config["title"] if segments.empty?


Looks like we're missing a test for this behaviour ^

I'm not sure we're testing this behaviour yet? segments is "field_attribute" in your first unit test. The only situation I see this line returning anything is if we basically call title_for_attribute(nil), which shouldn't ever be a thing, right? Can we just delete this line?

I tried removing it, and it breaks several tests (including the one that expects "Test Attribute"), because the final recursive call with [] no longer returns the title.

For title_for_attribute("field_attribute.0"), we remove the index and get ["field_attribute"], match the field, then recurse with []. That recursive call hits this line because segments.empty? is true, and that’s where we return the title.

So it’s not just for nil input—it’s the stop condition for valid lookups as well. Without it, the recursion wouldn’t know when to stop and would end up returning nil.

I don’t think we can safely delete it for that reason, even though it’s not obvious from the tests. We could perhaps add a test for the empty case?

Yep I'd add a test for the empty case 👍

ChrisBAshton · 2026-05-05T15:13:49Z

+    # Errors on array item fields use dotted attribute names like
+    # :"social_media_links.0.url" so they can be targeted inline per
+    # field. Rails calls these as methods during error processing;
+    # returning nil here prevents a NoMethodError.


Did you consider my suggestion that we switch from test_array_attribute.0.social_media_service_id to something like test_array_attribute__0__social_media_service_id so that we don't need to fight Rails's default method name construction? (Could do it in a refactor commit at the end if you want to avoid a painful rebase)

I suggested this on the assumption that it means you don't need to define these method_missing / respond_to_missing method definitions...? (On the basis that method names can't include a . in them, but can include a __).

If all this suggestion does is a find-and-replace across the code then it's not a useful suggestion 😅 Did you check what else could be cleaned up as a result? 🙏

From a quick look, I'm hoping the move to __ means we can delete the WithNestedAttributeErrors helper entirely?

So I don't think it actually eliminates the need for method_missing guard or WithNestedAttributeErrors — Rails calls the attribute as a method on the model during error processing regardless of whether the separator is . or __, so the guard is needed either way.

I think it ends up being a pure find-and-replace.

One small improvement we could make as a follow-up: BlockContent currently has its own inline copy of the method_missing/respond_to_missing? logic, while WithNestedAttributeErrors has an identical copy for use in test doubles. We could de-duplicate by moving the helper to concerns and having BlockContent include it, rather than defining it inline. Wanted to check you were happy with that first, given it's making WithNestedAttributeErrors even more needed than before 😬

OK - thanks for removing the "." to "__" replacement.

De-duplicating feels like a good move 👍

ChrisBAshton · 2026-05-11T15:39:45Z

+    # Errors on array item fields use dotted attribute names like
+    # :"social_media_links.0.url" so they can be targeted inline per
+    # field. Rails calls these as methods during error processing;
+    # returning nil here prevents a NoMethodError.


From a quick look, I'm hoping the move to __ means we can delete the WithNestedAttributeErrors helper entirely?

Translates dotted error keys like "social_media_links.0.url" into a human-readable field title by stripping numeric index segments and walking the form config.

Hooks into Rails error message generation to return human-readable field titles for dotted attribute names on configurable document types.

Replaces inline method_missing definitions in test classes with a shared module, avoiding repetition across the codebase.

Validates uniqueness of channel and URL across social media link array entries, adding per-field errors with dotted attribute keys.

Covers the case where an error is added with a dotted attribute key like "social_media_links.0.url", ensuring the component renders the correct link text and href anchor.

Return nil for dotted attribute names in method_missing to prevent NoMethodError during Rails error processing.

Without segments.first(path.length) != path, looking up an attribute would incorrectly return the title of the first field encountered rather than the one whose path actually matches.

ChrisBAshton

Have spent half an hour spiking trying to remove the dot notation. It should be doable but wasn't doable in that time! Happy to proceed and maybe we can have a backlog item to explore refactoring it back out again later.

A couple of comments above where we've agreed to add a couple more things ^ but otherwise this is good to go.

GDSNewt changed the title ~~Improve error message social media wip~~ Improve error message for social media [WHIT-3156] Apr 14, 2026

GDSNewt force-pushed the Improve-error-message-social-media-wip branch from e63ba07 to 5a70f75 Compare April 14, 2026 12:50

This comment was marked as outdated.

Sign in to view

GDSNewt force-pushed the Improve-error-message-social-media-wip branch 6 times, most recently from 1bd6059 to 6ba1fa3 Compare April 17, 2026 10:51

GDSNewt closed this Apr 22, 2026

GDSNewt reopened this Apr 27, 2026

ChrisBAshton reviewed Apr 28, 2026

View reviewed changes

GDSNewt force-pushed the Improve-error-message-social-media-wip branch 3 times, most recently from a828a6d to d5cd09f Compare May 5, 2026 10:27

ChrisBAshton reviewed May 5, 2026

View reviewed changes

GDSNewt force-pushed the Improve-error-message-social-media-wip branch 6 times, most recently from 8cfd894 to c5cd04c Compare May 11, 2026 13:27

ChrisBAshton reviewed May 11, 2026

View reviewed changes

GDSNewt added 6 commits May 13, 2026 14:05

Add title_for_attribute to ConfigurableDocumentType

af6809c

Translates dotted error keys like "social_media_links.0.url" into a human-readable field title by stripping numeric index segments and walking the form config.

Add human_attribute_name to StandardEdition

78d7b42

Hooks into Rails error message generation to return human-readable field titles for dotted attribute names on configurable document types.

Extract WithNestedAttributeErrors test helper

a44dcdf

Replaces inline method_missing definitions in test classes with a shared module, avoiding repetition across the codebase.

Add SocialMediaLinksValidator

08a0a6b

Validates uniqueness of channel and URL across social media link array entries, adding per-field errors with dotted attribute keys.

Add test for error summary component with dotted attribute names

ffbce7c

Covers the case where an error is added with a dotted attribute key like "social_media_links.0.url", ensuring the component renders the correct link text and href anchor.

Handle dotted attribute method calls in BlockContent

5d845f1

Return nil for dotted attribute names in method_missing to prevent NoMethodError during Rails error processing.

Add test for path-matching behaviour in find_field_title

63f5426

Without segments.first(path.length) != path, looking up an attribute would incorrectly return the title of the first field encountered rather than the one whose path actually matches.

GDSNewt force-pushed the Improve-error-message-social-media-wip branch from c5cd04c to 63f5426 Compare May 13, 2026 13:07

GDSNewt requested a review from ChrisBAshton May 13, 2026 13:08

ChrisBAshton approved these changes May 14, 2026

View reviewed changes

Conversation

GDSNewt commented Apr 14, 2026

Overview

Why

What Changed

ACs

UI Error Examples

1. Both fields blank

2. Channel selected, URL missing

3. URL entered, channel missing

4. Multiple accounts, mixed errors

Uh oh!

This comment was marked as outdated.

Uh oh!

ChrisBAshton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChrisBAshton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrisBAshton left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants