From 2d6312c57628f0891ec8c5f5f19355678eab3464 Mon Sep 17 00:00:00 2001 From: Farshad Dashti <78855469+FrostyApeOne@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:37:41 +0100 Subject: [PATCH] Added validator to the template to check if fieldId set in applicationmetadata exist (#68) Co-authored-by: FrostyApeOne (cherry picked from commit c045d65e9a256ebfeb1fbe42702d92d79927d85f) --- docs/Form-Template-Designer-Manual.md | 8 + .../ApplicationMetadataBindingValidator.cs | 276 ++++++++++++++++++ .../Services/TemplateValidationService.cs | 15 +- ...pplicationMetadataBindingValidatorTests.cs | 232 +++++++++++++++ .../TemplateValidationServiceTests.cs | 34 +++ 5 files changed, 563 insertions(+), 2 deletions(-) create mode 100644 src/GovUK.Dfe.FlexForms.Application/FormEngine/ApplicationMetadataBindingValidator.cs create mode 100644 src/Tests/GovUK.Dfe.FlexForms.Application.Tests/FormEngine/ApplicationMetadataBindingValidatorTests.cs diff --git a/docs/Form-Template-Designer-Manual.md b/docs/Form-Template-Designer-Manual.md index 9e23c89..266bba3 100644 --- a/docs/Form-Template-Designer-Manual.md +++ b/docs/Form-Template-Designer-Manual.md @@ -273,6 +273,14 @@ The block under **Your application** (grey left border) can list up to **10** la Property names are **case-sensitive** in template JSON (`label` and `fieldId`, lowercase). +**Validation on save:** Template Manager won't save a version whose `applicationMetaData` paths don't match the template, because those rows would show blank on the task list. Each problem is listed with the item index, label and a suggested fix. A version is rejected when: + +- an `application.*` value isn't one of those listed above; +- a field or collection ID isn't defined on any task page or flow page (the closest match is suggested); +- a collection path names a field that isn't on that collection's pages (the collection's fields are listed); +- a child property (`.name`) or index (`[0]`) is used on a plain-value field such as text, email, radios, select, checkboxes or date. Child properties of lookup (`autocomplete` / `complexField`) and upload fields come from external data, so only the field itself is checked; +- the path is malformed, for example `a..b` or `items[x]`. + --- ## 4. Task groups diff --git a/src/GovUK.Dfe.FlexForms.Application/FormEngine/ApplicationMetadataBindingValidator.cs b/src/GovUK.Dfe.FlexForms.Application/FormEngine/ApplicationMetadataBindingValidator.cs new file mode 100644 index 0000000..f8bb07a --- /dev/null +++ b/src/GovUK.Dfe.FlexForms.Application/FormEngine/ApplicationMetadataBindingValidator.cs @@ -0,0 +1,276 @@ +using System.Globalization; +using System.Text.RegularExpressions; +using GovUK.Dfe.FlexForms.Domain.Models; + +namespace GovUK.Dfe.FlexForms.Application.FormEngine; + +/// +/// Checks that each applicationMetaData binding path points at something the template defines, using the +/// same path rules as . A path the resolver can't match renders as an empty +/// row on the task list, so these problems are reported when the template is saved instead. +/// +public static partial class ApplicationMetadataBindingValidator +{ + public static readonly IReadOnlyList ApplicationKeys = + [ + "reference", + "version", + "leadApplicant", + "leadApplicantName", + "leadApplicantEmail", + ]; + + private const int MaxListedFieldIds = 15; + + /// + /// Field types whose answer is a single plain value, so it has no child properties and no list items to index. + /// Other types (autocomplete, complexField, upload, and any type not listed) may store objects or lists. + /// + private static readonly HashSet PlainValueFieldTypes = new(StringComparer.OrdinalIgnoreCase) + { + "text", "textarea", "text-area", "email", "character-count", + "radios", "select", "checkboxes", + "date", "datetime", "date-time", + "number", "integer", "decimal", "boolean", + }; + + [GeneratedRegex(@"^(?[^\[\]]+)(\[(?\d+)\])?$")] + private static partial Regex SegmentPattern(); + + private readonly record struct PathSegment(string Name, int? Index, string Text); + + /// + /// Returns a description of what is wrong with , or null when it is valid. + /// + public static string? Validate(string binding, FormTemplate template) + { + var path = binding.Trim(); + + if (path.StartsWith("application.", StringComparison.OrdinalIgnoreCase)) + { + var key = path["application.".Length..]; + if (key.StartsWith("field.", StringComparison.OrdinalIgnoreCase)) + return ValidateFieldPath(key["field.".Length..], template); + + if (ApplicationKeys.Contains(key, StringComparer.OrdinalIgnoreCase)) + return null; + + return $"'{key}' is not a recognised application value. Use one of " + + string.Join(", ", ApplicationKeys.Select(k => $"'application.{k}'")) + + ", or 'field.{fieldId}' for an answer" + + Suggestion(path, ApplicationKeys.Select(k => $"application.{k}")); + } + + if (path.StartsWith("field.", StringComparison.OrdinalIgnoreCase)) + path = path["field.".Length..]; + + return ValidateFieldPath(path, template); + } + + private static string? ValidateFieldPath(string path, FormTemplate template) + { + if (path.Length == 0) + return "the path does not name a field. Add a field ID, for example 'field.trustName'"; + + var fields = TemplateFields.From(template); + + // The resolver matches the whole path as a key first, so field IDs that contain dots still work. + if (!path.Contains('[') && (fields.TopLevel.ContainsKey(path) || fields.Collections.ContainsKey(path))) + return null; + + if (!TryParse(path, out var segments, out var badSegment)) + { + return $"'{badSegment}' is not a valid path. Separate field IDs and properties with single dots and " + + "use an optional zero-based index, for example 'detailsOfIncomingTrust[0].trustSearch.name'"; + } + + var first = segments[0]; + var remainder = segments.Skip(1).ToList(); + + if (fields.Collections.TryGetValue(first.Name, out var collectionFields)) + return ValidateCollectionPath(first, remainder, collectionFields); + + if (fields.TopLevel.TryGetValue(first.Name, out var topLevelField)) + { + if (first.Index is not null && IsPlainValue(topLevelField)) + return IndexOnPlainValueMessage(first, topLevelField); + + return ValidatePropertyPath(topLevelField, remainder); + } + + // The resolver also looks inside every collection item for a field with this ID. + var nestedField = fields.Collections.Values + .Select(inner => inner.GetValueOrDefault(first.Name)) + .FirstOrDefault(field => field is not null); + if (nestedField is not null) + return ValidatePropertyPath(nestedField, remainder); + + return $"no field or collection with ID '{first.Name}' exists in the template" + Suggestion(first.Name, fields.AllIds()); + } + + private static string? ValidateCollectionPath( + PathSegment collection, + List remainder, + IReadOnlyDictionary collectionFields) + { + if (remainder.Count == 0) + return null; + + var nested = remainder[0]; + if (!collectionFields.TryGetValue(nested.Name, out var field)) + { + var available = collectionFields.Count == 0 + ? " That collection has no fields" + : $" Fields in that collection: {FormatIds(collectionFields.Keys)}"; + return $"'{nested.Name}' is not a field in collection '{collection.Name}'." + + available + + Suggestion(nested.Name, collectionFields.Keys); + } + + if (nested.Index is not null && IsPlainValue(field)) + return IndexOnPlainValueMessage(nested, field); + + return ValidatePropertyPath(field, remainder.Skip(1).ToList()); + } + + private static string? ValidatePropertyPath(Field field, List propertyPath) + { + // Lookup and upload answers come from external data, so their child properties can't be checked here. + if (propertyPath.Count == 0 || !IsPlainValue(field)) + return null; + + var property = string.Join(".", propertyPath.Select(s => s.Name)); + return $"'{field.FieldId}' is a '{field.Type}' field, which stores a single value with no child properties, " + + $"so '.{property}' cannot be read. Remove '.{property}', or point at a lookup (autocomplete) or upload field"; + } + + private static string IndexOnPlainValueMessage(PathSegment segment, Field field) => + $"'{segment.Text}' cannot use an index because '{field.FieldId}' is a '{field.Type}' field, which stores a single value. " + + $"Indexes only apply to collections and lookup fields that hold a list of items. Use '{segment.Name}' instead"; + + private static bool IsPlainValue(Field field) => + field.ComplexField is null && PlainValueFieldTypes.Contains(field.Type?.Trim() ?? string.Empty); + + private static bool TryParse(string path, out List segments, out string badSegment) + { + segments = []; + badSegment = string.Empty; + + foreach (var raw in path.Split('.')) + { + var text = raw.Trim(); + var match = SegmentPattern().Match(text); + if (!match.Success || string.IsNullOrWhiteSpace(match.Groups["name"].Value)) + { + badSegment = text.Length == 0 ? path : text; + return false; + } + + int? index = match.Groups["index"].Success + ? int.Parse(match.Groups["index"].Value, NumberStyles.None, CultureInfo.InvariantCulture) + : null; + segments.Add(new PathSegment(match.Groups["name"].Value.Trim(), index, text)); + } + + return segments.Count > 0; + } + + private static string Suggestion(string id, IEnumerable candidates) + { + var best = candidates + .Distinct(StringComparer.OrdinalIgnoreCase) + .Select(candidate => (Candidate: candidate, Distance: EditDistance(id.ToLowerInvariant(), candidate.ToLowerInvariant()))) + .Where(x => x.Distance <= Math.Max(2, id.Length / 3)) + .OrderBy(x => x.Distance) + .FirstOrDefault(); + + return best.Candidate is null ? string.Empty : $". Did you mean '{best.Candidate}'?"; + } + + private static string FormatIds(IEnumerable ids) + { + var list = ids.ToList(); + var shown = string.Join(", ", list.Take(MaxListedFieldIds).Select(id => $"'{id}'")); + return list.Count > MaxListedFieldIds ? $"{shown} and {list.Count - MaxListedFieldIds} more" : shown; + } + + private static int EditDistance(string a, string b) + { + var previous = new int[b.Length + 1]; + var current = new int[b.Length + 1]; + for (var j = 0; j <= b.Length; j++) + previous[j] = j; + + for (var i = 1; i <= a.Length; i++) + { + current[0] = i; + for (var j = 1; j <= b.Length; j++) + { + var cost = a[i - 1] == b[j - 1] ? 0 : 1; + current[j] = Math.Min(Math.Min(current[j - 1] + 1, previous[j] + 1), previous[j - 1] + cost); + } + + (previous, current) = (current, previous); + } + + return previous[b.Length]; + } + + /// + /// Fields on task pages, and collections (the fieldId of each multi- or derived-collection flow) with the + /// fields on their pages. Answer lookups ignore case, so these do too. + /// + private sealed class TemplateFields + { + public Dictionary TopLevel { get; } = new(StringComparer.OrdinalIgnoreCase); + + public Dictionary> Collections { get; } = new(StringComparer.OrdinalIgnoreCase); + + public IEnumerable AllIds() => + TopLevel.Keys + .Concat(Collections.Keys) + .Concat(Collections.Values.SelectMany(inner => inner.Keys)); + + public static TemplateFields From(FormTemplate template) + { + var result = new TemplateFields(); + + foreach (var task in (template.TaskGroups ?? []).SelectMany(group => group.Tasks ?? [])) + { + AddPageFields(task.Pages, result.TopLevel); + + var summary = task.Summary; + if (summary is null) + continue; + + var flows = (summary.Flows ?? []).Select(flow => (flow.FieldId, flow.Pages)) + .Concat((summary.DerivedFlows ?? []).Select(flow => (flow.FieldId, flow.Pages))); + + foreach (var (collectionId, pages) in flows) + { + if (string.IsNullOrWhiteSpace(collectionId)) + continue; + + if (!result.Collections.TryGetValue(collectionId.Trim(), out var inner)) + { + inner = new Dictionary(StringComparer.OrdinalIgnoreCase); + result.Collections[collectionId.Trim()] = inner; + } + + AddPageFields(pages, inner); + } + } + + return result; + } + + private static void AddPageFields(IEnumerable? pages, Dictionary target) + { + foreach (var field in (pages ?? []).SelectMany(page => page.Fields ?? [])) + { + if (!string.IsNullOrWhiteSpace(field.FieldId)) + target.TryAdd(field.FieldId.Trim(), field); + } + } + } +} diff --git a/src/GovUK.Dfe.FlexForms.Infrastructure/Services/TemplateValidationService.cs b/src/GovUK.Dfe.FlexForms.Infrastructure/Services/TemplateValidationService.cs index 88774a5..0d145ff 100644 --- a/src/GovUK.Dfe.FlexForms.Infrastructure/Services/TemplateValidationService.cs +++ b/src/GovUK.Dfe.FlexForms.Infrastructure/Services/TemplateValidationService.cs @@ -1,3 +1,4 @@ +using GovUK.Dfe.FlexForms.Application.FormEngine; using GovUK.Dfe.FlexForms.Application.Interfaces; using GovUK.Dfe.FlexForms.Domain.Models; using Microsoft.Extensions.Logging; @@ -122,11 +123,12 @@ private void ValidateTemplate(FormTemplate template, List errors) ValidateTaskGroup(group, i, errors); } - ValidateApplicationMetaData(template.ApplicationMetaData, errors); + ValidateApplicationMetaData(template, errors); } - private static void ValidateApplicationMetaData(ApplicationMetadataConfiguration? configuration, List errors) + private static void ValidateApplicationMetaData(FormTemplate template, List errors) { + var configuration = template.ApplicationMetaData; if (configuration is null || configuration.Items is null || configuration.Items.Count == 0) return; @@ -145,7 +147,16 @@ private static void ValidateApplicationMetaData(ApplicationMetadataConfiguration errors.Add($"{context}: 'label' is required"); if (string.IsNullOrWhiteSpace(item.FieldId)) + { errors.Add($"{context}: 'fieldId' is required"); + continue; + } + + if (ApplicationMetadataBindingValidator.Validate(item.FieldId, template) is { } problem) + { + var label = string.IsNullOrWhiteSpace(item.Label) ? string.Empty : $" ('{item.Label.Trim()}')"; + errors.Add($"{context}{label}: fieldId '{item.FieldId.Trim()}' is invalid: {problem}"); + } } } diff --git a/src/Tests/GovUK.Dfe.FlexForms.Application.Tests/FormEngine/ApplicationMetadataBindingValidatorTests.cs b/src/Tests/GovUK.Dfe.FlexForms.Application.Tests/FormEngine/ApplicationMetadataBindingValidatorTests.cs new file mode 100644 index 0000000..bdac94e --- /dev/null +++ b/src/Tests/GovUK.Dfe.FlexForms.Application.Tests/FormEngine/ApplicationMetadataBindingValidatorTests.cs @@ -0,0 +1,232 @@ +using GovUK.Dfe.FlexForms.Application.FormEngine; +using GovUK.Dfe.FlexForms.Domain.Models; + +namespace GovUK.Dfe.FlexForms.Application.Tests.FormEngine; + +public class ApplicationMetadataBindingValidatorTests +{ + private static readonly FormTemplate Template = new() + { + TemplateId = "t1", + TemplateName = "Test", + Description = "d", + TaskGroups = + [ + new TaskGroup + { + GroupId = "g1", + GroupName = "Group", + GroupOrder = 1, + GroupStatus = nameof(Domain.Models.TaskStatus.NotStarted), + Tasks = + [ + new Domain.Models.Task + { + TaskId = "task1", + TaskName = "Task", + TaskOrder = 1, + TaskStatusString = nameof(Domain.Models.TaskStatus.NotStarted), + Pages = + [ + Page( + Field("trustName", "text"), + Field("trustLookup", "complexField", new ComplexField { Id = "trusts" }), + Field("transfer.date", "date")) + ], + Summary = new TaskSummaryConfiguration + { + Mode = "multiCollectionFlow", + Flows = + [ + new MultiCollectionFlowConfiguration + { + FlowId = "trusts", + FieldId = "detailsOfIncomingTrust", + Pages = + [ + Page( + Field("incomingTrustsSearch-field-flow", "autocomplete"), + Field("incomingTrustContact", "email")) + ] + } + ], + DerivedFlows = + [ + new DerivedCollectionFlowConfiguration + { + FlowId = "declarations", + FieldId = "trustDeclarations", + Pages = [Page(Field("signatoryName", "text"))] + } + ] + } + } + ] + } + ] + }; + + [Theory] + [InlineData("application.reference")] + [InlineData("application.version")] + [InlineData("application.leadApplicant")] + [InlineData("application.leadApplicantName")] + [InlineData("APPLICATION.LEADAPPLICANTEMAIL")] + [InlineData("application.field.trustName")] + [InlineData("field.trustName")] + [InlineData("trustName")] + [InlineData("TRUSTNAME")] + [InlineData("transfer.date")] + [InlineData("trustLookup.ukprn")] + [InlineData("trustLookup[0].name")] + [InlineData("detailsOfIncomingTrust")] + [InlineData("detailsOfIncomingTrust[1]")] + [InlineData("detailsOfIncomingTrust.incomingTrustsSearch-field-flow")] + [InlineData("detailsOfIncomingTrust[0].incomingTrustsSearch-field-flow.name")] + [InlineData("detailsOfIncomingTrust.incomingTrustContact")] + [InlineData("incomingTrustsSearch-field-flow.name")] + [InlineData("incomingTrustContact")] + [InlineData("trustDeclarations[0].signatoryName")] + [InlineData(" field.trustName ")] + public void Validate_accepts_paths_that_exist_in_the_template(string binding) + { + Assert.Null(ApplicationMetadataBindingValidator.Validate(binding, Template)); + } + + [Fact] + public void Validate_rejects_unknown_application_key_and_suggests_closest() + { + var problem = ApplicationMetadataBindingValidator.Validate("application.refrence", Template); + + Assert.NotNull(problem); + Assert.Contains("'refrence' is not a recognised application value", problem); + Assert.Contains("'application.leadApplicantEmail'", problem); + Assert.Contains("Did you mean 'application.reference'?", problem); + } + + [Theory] + [InlineData("field.trustNam")] + [InlineData("trustNam")] + [InlineData("application.field.trustNam")] + public void Validate_rejects_unknown_field_and_suggests_closest(string binding) + { + var problem = ApplicationMetadataBindingValidator.Validate(binding, Template); + + Assert.Equal("no field or collection with ID 'trustNam' exists in the template. Did you mean 'trustName'?", problem); + } + + [Fact] + public void Validate_rejects_unknown_field_without_suggestion_when_nothing_is_close() + { + var problem = ApplicationMetadataBindingValidator.Validate("field.completelyDifferent", Template); + + Assert.Equal("no field or collection with ID 'completelyDifferent' exists in the template", problem); + } + + [Fact] + public void Validate_rejects_field_missing_from_named_collection_and_lists_its_fields() + { + var problem = ApplicationMetadataBindingValidator.Validate( + "detailsOfIncomingTrust[0].incomingTrustSearch-field-flow.name", + Template); + + Assert.NotNull(problem); + Assert.Contains("'incomingTrustSearch-field-flow' is not a field in collection 'detailsOfIncomingTrust'", problem); + Assert.Contains("'incomingTrustsSearch-field-flow', 'incomingTrustContact'", problem); + Assert.Contains("Did you mean 'incomingTrustsSearch-field-flow'?", problem); + } + + [Theory] + [InlineData("field.trustName.name", "trustName", "text", ".name")] + [InlineData("detailsOfIncomingTrust.incomingTrustContact.address", "incomingTrustContact", "email", ".address")] + [InlineData("incomingTrustContact.a.b", "incomingTrustContact", "email", ".a.b")] + public void Validate_rejects_child_property_on_plain_value_field(string binding, string fieldId, string type, string property) + { + var problem = ApplicationMetadataBindingValidator.Validate(binding, Template); + + Assert.NotNull(problem); + Assert.Contains($"'{fieldId}' is a '{type}' field", problem); + Assert.Contains($"'{property}' cannot be read", problem); + } + + [Theory] + [InlineData("trustName[0]", "trustName[0]")] + [InlineData("detailsOfIncomingTrust.incomingTrustContact[1]", "incomingTrustContact[1]")] + public void Validate_rejects_index_on_plain_value_field(string binding, string segment) + { + var problem = ApplicationMetadataBindingValidator.Validate(binding, Template); + + Assert.NotNull(problem); + Assert.StartsWith($"'{segment}' cannot use an index", problem); + } + + [Theory] + [InlineData("field.")] + [InlineData("application.field.")] + public void Validate_rejects_path_without_field(string binding) + { + var problem = ApplicationMetadataBindingValidator.Validate(binding, Template); + + Assert.NotNull(problem); + Assert.StartsWith("the path does not name a field", problem); + } + + [Theory] + [InlineData("trustName..name", "trustName..name")] + [InlineData("detailsOfIncomingTrust[x].name", "detailsOfIncomingTrust[x]")] + [InlineData("detailsOfIncomingTrust[-1]", "detailsOfIncomingTrust[-1]")] + [InlineData("detailsOfIncomingTrust[0", "detailsOfIncomingTrust[0")] + [InlineData("field.trustName.", "trustName.")] + public void Validate_rejects_malformed_path(string binding, string reported) + { + var problem = ApplicationMetadataBindingValidator.Validate(binding, Template); + + Assert.NotNull(problem); + Assert.StartsWith($"'{reported}' is not a valid path", problem); + } + + [Fact] + public void Validate_accepts_every_application_key_the_resolver_maps() + { + var template = new FormTemplate + { + TemplateId = "t1", + TemplateName = "Test", + Description = "d", + TaskGroups = Template.TaskGroups, + ApplicationMetaData = new ApplicationMetadataConfiguration + { + Items = ApplicationMetadataBindingValidator.ApplicationKeys + .Select(key => new ApplicationMetadataItemDefinition { Label = key, FieldId = $"application.{key}" }) + .ToList() + } + }; + var context = new ApplicationMetadataResolveContext( + "REF-1", "1.0.0", "Lead", "lead@example.com", "Lead applicant", "Application", null); + + var rows = new ApplicationMetadataResolver().Resolve(template, context); + + Assert.All(rows, row => Assert.False(string.IsNullOrEmpty(row.DisplayValue), $"{row.Label} resolved to empty")); + } + + private static Page Page(params Field[] fields) => + new() + { + PageId = "p1", + Slug = "p1", + Title = "Page", + Description = "d", + PageOrder = 1, + Fields = [.. fields] + }; + + private static Field Field(string fieldId, string type, ComplexField? complexField = null) => + new() + { + FieldId = fieldId, + Type = type, + Label = new Label { Value = fieldId }, + Order = 1, + ComplexField = complexField + }; +} diff --git a/src/Tests/GovUK.Dfe.FlexForms.Infrastructure.UnitTests/Services/TemplateValidationServiceTests.cs b/src/Tests/GovUK.Dfe.FlexForms.Infrastructure.UnitTests/Services/TemplateValidationServiceTests.cs index d72add2..8ed6b54 100644 --- a/src/Tests/GovUK.Dfe.FlexForms.Infrastructure.UnitTests/Services/TemplateValidationServiceTests.cs +++ b/src/Tests/GovUK.Dfe.FlexForms.Infrastructure.UnitTests/Services/TemplateValidationServiceTests.cs @@ -376,6 +376,40 @@ public void ValidateTemplateJson_ShouldFail_WhenApplicationMetaDataItemMissingLa Assert.Contains(errors, e => e.Contains("applicationMetaData.items[1]: 'fieldId' is required", StringComparison.Ordinal)); } + [Fact] + public void ValidateTemplateJson_ShouldFail_WhenApplicationMetaDataFieldIdNotInTemplate() + { + var template = CreateMinimalValidTemplate(); + template.TaskGroups[0].Tasks[0].Pages = + [ + new Page + { + PageId = "p1", + Slug = "p1", + Title = "Page", + Description = "d", + PageOrder = 1, + Fields = [new Field { FieldId = "trustName", Type = "text", Label = new Label { Value = "Trust" }, Order = 1 }] + } + ]; + template.ApplicationMetaData = new ApplicationMetadataConfiguration + { + Items = + [ + new ApplicationMetadataItemDefinition { Label = "Trust", FieldId = "field.trustName" }, + new ApplicationMetadataItemDefinition { Label = "Second trust", FieldId = "field.trustNmae" } + ] + }; + + var (isValid, errors) = _service.ValidateTemplateJson(JsonSerializer.Serialize(template)); + + Assert.False(isValid); + Assert.Equal( + "applicationMetaData.items[1] ('Second trust'): fieldId 'field.trustNmae' is invalid: " + + "no field or collection with ID 'trustNmae' exists in the template. Did you mean 'trustName'?", + Assert.Single(errors)); + } + private static FormTemplate CreateMinimalValidTemplate() => new() {