Skip to content

Commit a53f4f8

Browse files
danlivings-dxwdanlivings-dxw
authored
Improve user-facing messages for collection flows (#92)
* Move `GenerateSuccessMessage` to `DisplayHelpers` static class Moving reused pieces of functionality into their own modules reduces the cognitive load when understanding the rendering logic which makes it easier to make changes to. It also allows for testing on this functionality to be done in isolation, which means that tests can be more specific to the behaviours and therefore the team can be more confident that changes won't accidentally introduce new bugs or regressions. Tests have been added for this method to document its current behaviour. * Introduce `JsonElement.EvaluatePath` extension method This allows `JsonElement`s representing objects to be walked to get a specific property from them. Using a fully compliant JSON Path implementation would be better, but `System.Text.Json` doesn't have one yet and DfE discourages the use of third-party libraries such as JsonPath.Net. As this is a greatly simplified implementation, it only supports dot-based notation, i.e. `foo.bar.baz`. As such, it doesn't allow addressing of items within arrays. * Allow custom success messages to interpolate values from `JsonElement`s Custom success messages can now use the "{key.subkey}" syntax to provide user-facing messages that take values from complex data fields represented using `JsonElement`s. For example, when adding an academy, the success message can be set to "{academiesSearch.name} has been added" and the `name` property will be extracted from the complex field named `academiesSearch` and used to build the final message. * Add `ExpandEncodedJson` method to `DisplayHelpers` This acts as a workaround for complex field data being passed around as a JSON-string-of-JSON rather than as the JSON itself. To clarify, using adding an academy as an example: The message "{academiesSearch.name} has been added" expects `itemData` to have the following shape: ```jsonc { "academiesSearch": { "name": "Some Academy", "ukprn": "123456" }, // ... } ``` However, the form engine is providing `itemData` with this shape (note that `academiesSearch` is a `JsonElement` of kind `String`, rather than a C# `string`, or a JSON object proper): ```jsonc { "academiesSearch": "{\"name\":\"Some Academy\",\"ukprn\":\"123456\"}", // ... } ``` The `ExpandEncodedJson` method turns `JsonElement` strings like the latter into `JsonElement` values like the former, while leaving other values unchanged. After performing this transformation, the `GenerateSuccessMessage` can perform the correct interpolation on values given to it by the form engine. * Ensure all field data is available for success message when updating When a collection item is updated, the user can press the "Change" button on any of the fields. If they click (for example) the third button, the values for the first two fields aren't included in `accumulated`, which results in a bug where success messages show placeholders instead of the interpolated values. Merging in the original values using `TryAdd` ensures that all fields are available regardless of whether they were changed or not. * Allow custom pluralisation when no items are in collection flow Currently, the `_SingleCollectionFlow.cshtml` view mechanically pluralises the item label by adding "s" onto the end. In some situations this is inappropriate: for example, with a task where the `itemTitleBinding` is "Academy", this results in the incorrect pluralisation "academys" rather than "academies". This fixes that problem by allowing tasks in the template schema to optionally provide a value for `itemTitleBindingPlural`, which if present will be used instead. When the value is `null` then the view will fallback to concatenating the item label with "s" as before.
1 parent 8946fd7 commit a53f4f8

8 files changed

Lines changed: 821 additions & 68 deletions

File tree

‎src/DfE.ExternalApplications.Domain/Models/Task.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public class MultiCollectionFlowConfiguration
7373
[JsonPropertyName("minItems")] public int? MinItems { get; set; }
7474
[JsonPropertyName("maxItems")] public int? MaxItems { get; set; }
7575
[JsonPropertyName("itemTitleBinding")] public string? ItemTitleBinding { get; set; }
76+
[JsonPropertyName("itemTitleBindingPlural")] public string? ItemTitleBindingPlural { get; set; }
7677
[JsonPropertyName("summaryColumns")] public List<FlowSummaryColumn>? SummaryColumns { get; set; }
7778
[JsonPropertyName("addItemMessage")] public string? AddItemMessage { get; set; }
7879
[JsonPropertyName("updateItemMessage")] public string? UpdateItemMessage { get; set; }

‎src/DfE.ExternalApplications.Web/Pages/FormEngine/DisplayHelpers.cs‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
using System.Text.Json;
2+
using System.Text.RegularExpressions;
3+
4+
namespace DfE.ExternalApplications.Web.Pages.FormEngine;
5+
6+
public static class DisplayHelpers
7+
{
8+
/// <summary>
9+
/// Expands JSON strings encoded within a dictionary's values into their corresponding JSON objects.
10+
/// When a value is a JsonElement of type String containing valid JSON, it will be deserialized into a JsonElement.
11+
/// Other values remain unchanged.
12+
/// </summary>
13+
/// <param name="itemData">The dictionary containing potentially encoded JSON strings as values</param>
14+
/// <returns>A new dictionary with the same keys but with encoded JSON strings expanded into JsonElement objects.
15+
/// Returns null if the input dictionary is null.</returns>
16+
public static Dictionary<string, object>? ExpandEncodedJson(Dictionary<string, object>? itemData)
17+
{
18+
return itemData?.Select(kvp => (kvp.Key, TransformEncodedJsonString(kvp.Value))).ToDictionary();
19+
}
20+
21+
private static object TransformEncodedJsonString(object value)
22+
{
23+
switch (value)
24+
{
25+
case JsonElement { ValueKind: JsonValueKind.String } jsonString:
26+
try
27+
{
28+
return JsonSerializer.Deserialize<JsonElement>(jsonString.GetString() ?? "");
29+
}
30+
catch (JsonException)
31+
{
32+
return value;
33+
}
34+
default:
35+
return value;
36+
}
37+
}
38+
39+
/// <summary>
40+
/// Generates a success message using custom template or fallback default
41+
/// </summary>
42+
/// <param name="customMessage">Custom message template from configuration</param>
43+
/// <param name="operation">Operation type: "add", "update", or "delete"</param>
44+
/// <param name="itemData">Dictionary containing all field values for the item</param>
45+
/// <param name="flowTitle">Title of the flow</param>
46+
/// <returns>Formatted success message</returns>
47+
public static string GenerateSuccessMessage(string? customMessage, string operation, Dictionary<string, object>? itemData, string? flowTitle)
48+
{
49+
if (!string.IsNullOrEmpty(customMessage))
50+
{
51+
return InterpolateCustomMessage(customMessage, itemData, flowTitle);
52+
}
53+
54+
var displayName = GetDisplayNameFromItemData(itemData);
55+
56+
var lowerFlowTitle = flowTitle?.ToLowerInvariant() ?? "collection";
57+
58+
return operation switch
59+
{
60+
"add" => $"{displayName} has been added to {lowerFlowTitle}",
61+
"update" => $"{displayName} has been updated",
62+
"delete" => $"{displayName} has been removed from {lowerFlowTitle}",
63+
_ => $"{displayName} has been processed"
64+
};
65+
}
66+
67+
private static string InterpolateCustomMessage(string message, Dictionary<string, object>? itemData, string? flowTitle)
68+
{
69+
message = message.Replace("{flowTitle}", flowTitle ?? "collection");
70+
71+
if (itemData == null) return message;
72+
73+
foreach (var (key, value) in itemData)
74+
{
75+
message = value switch
76+
{
77+
JsonElement jsonElement => InterpolateJsonValue(message, key, jsonElement),
78+
_ => InterpolateBasicValue(message, key, value)
79+
};
80+
}
81+
82+
return message;
83+
}
84+
85+
private static string InterpolateJsonValue(string message, string key, JsonElement jsonElement)
86+
{
87+
// This regex matches interpolation expressions in the form of "{key.subkey}". It doesn't attempt to parse a
88+
// valid JSON path from the subkey.
89+
var matches = Regex.Matches(message, @$"\{{{Regex.Escape(key)}\.([^}}]+)}}");
90+
91+
foreach (var subkey in matches.Select(c => c.Groups[1].Value))
92+
{
93+
var result = jsonElement.EvaluatePath(subkey);
94+
95+
if (result is null) continue;
96+
97+
var placeholder = $"{{{key}.{subkey}}}";
98+
message = message.Replace(placeholder, result.Value.ToString());
99+
}
100+
101+
message = message.Replace($"{{{key}}}", jsonElement.ToString());
102+
103+
return message;
104+
}
105+
106+
private static string InterpolateBasicValue(string message, string key, object value)
107+
{
108+
var placeholder = $"{{{key}}}";
109+
var valueString = value.ToString() ?? "";
110+
message = message.Replace(placeholder, valueString);
111+
return message;
112+
}
113+
114+
private static string GetDisplayNameFromItemData(Dictionary<string, object>? itemData)
115+
{
116+
var displayName = "Item";
117+
if (itemData == null || itemData.Count == 0) return displayName;
118+
119+
// Try common name fields first, then fall back to any non-empty value
120+
var nameFields = new[] { "firstName", "name", "title", "label" };
121+
var nameField = nameFields.FirstOrDefault(field => itemData.ContainsKey(field) && !string.IsNullOrEmpty(itemData[field].ToString()));
122+
123+
if (nameField != null)
124+
{
125+
displayName = itemData[nameField].ToString() ?? "Item";
126+
}
127+
else
128+
{
129+
// Use the first non-empty field value
130+
var firstValue = itemData.Values.FirstOrDefault(v => !string.IsNullOrEmpty(v.ToString()));
131+
if (firstValue != null)
132+
{
133+
displayName = firstValue.ToString() ?? "Item";
134+
}
135+
}
136+
137+
return displayName;
138+
}
139+
}

‎src/DfE.ExternalApplications.Web/Pages/FormEngine/JsonHelpers.cs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
using System.Text.Json;
2+
3+
namespace DfE.ExternalApplications.Web.Pages.FormEngine;
4+
5+
public static class JsonHelpers
6+
{
7+
/// <summary>
8+
/// <para>
9+
/// Evaluates a JSON path on a given JSON element and retrieves the corresponding nested JSON element.
10+
/// </para>
11+
/// <para>
12+
/// Note: This method is not a compliant implementation of the JSONPath specification.
13+
/// Only dot-separated paths are supported, e.g. <c>"foo.bar.baz"</c>.
14+
/// </para>
15+
/// </summary>
16+
/// <param name="element">
17+
/// The JSON element on which the path evaluation is performed. Must have a <c>ValueKind</c> of <c>Object</c>.
18+
/// </param>
19+
/// <param name="path">
20+
/// A string specifying the path to a nested JSON property (e.g., <c>"property.subproperty"</c>).
21+
/// </param>
22+
/// <returns>
23+
/// The JSON element corresponding to the specified path if it exists and is valid; otherwise, null.
24+
/// </returns>
25+
/// <exception cref="ArgumentException">
26+
/// Thrown if the method is invoked on a JSON element whose <c>ValueKind</c> is not <c>Object</c>.
27+
/// </exception>
28+
public static JsonElement? EvaluatePath(this JsonElement element, string path)
29+
{
30+
if (element.ValueKind != JsonValueKind.Object)
31+
{
32+
throw new ArgumentException(
33+
$"This method should only be used on JSON objects, but the element is a {element.ValueKind.DisplayName()}",
34+
nameof(element)
35+
);
36+
}
37+
38+
while (true)
39+
{
40+
if (element.ValueKind != JsonValueKind.Object)
41+
{
42+
return null;
43+
}
44+
45+
var pathSegments = path.Split('.', 2);
46+
if (!element.TryGetProperty(pathSegments[0], out var result))
47+
{
48+
return null;
49+
}
50+
51+
if (pathSegments.Length == 1) return result;
52+
element = result;
53+
path = pathSegments[1];
54+
}
55+
}
56+
57+
private static string DisplayName(this JsonValueKind kind) => kind switch
58+
{
59+
JsonValueKind.Undefined => "undefined",
60+
JsonValueKind.Object => "object",
61+
JsonValueKind.Array => "array",
62+
JsonValueKind.String => "string",
63+
JsonValueKind.Number => "number",
64+
JsonValueKind.True or JsonValueKind.False => "boolean",
65+
JsonValueKind.Null => "null",
66+
_ => throw new ArgumentOutOfRangeException(nameof(kind))
67+
};
68+
}

‎src/DfE.ExternalApplications.Web/Pages/FormEngine/RenderForm.cshtml.cs‎

Lines changed: 22 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using GovUK.Dfe.CoreLibs.Contracts.ExternalApplications.Models.Request;
1414
using GovUK.Dfe.CoreLibs.Contracts.ExternalApplications.Enums;
1515
using DfE.ExternalApplications.Web.Interfaces;
16+
using static DfE.ExternalApplications.Web.Pages.FormEngine.DisplayHelpers;
1617

1718
namespace DfE.ExternalApplications.Web.Pages.FormEngine
1819
{
@@ -1093,11 +1094,30 @@ public async Task<IActionResult> OnPostPageAsync()
10931094
// Use the accumulated data (all fields from the item)
10941095
if (isNewItem)
10951096
{
1097+
accumulated = ExpandEncodedJson(accumulated);
10961098
SuccessMessage = GenerateSuccessMessage(flow.AddItemMessage, "add", accumulated, flow.Title);
10971099
}
10981100
else
10991101
{
1100-
SuccessMessage = GenerateSuccessMessage(flow.UpdateItemMessage, "update", accumulated, flow.Title);
1102+
// When a collection item is updated, the user can press the "Change" button on any
1103+
// of the fields. If they click (for example) the third button, the values for the
1104+
// first two fields aren't included in `accumulated`, which results in a bug where
1105+
// success messages show placeholders instead of the interpolated values.
1106+
// Merging in the original values using `TryAdd` ensures that all fields are
1107+
// available regardless of whether they were changed or not.
1108+
var itemData = accumulated;
1109+
var existingData = _applicationResponseService.GetAccumulatedFormData(HttpContext.Session);
1110+
if (existingData.TryGetValue(flowFieldId, out var existingValue))
1111+
{
1112+
var contents = JsonSerializer.Deserialize<List<Dictionary<string, object>>>(existingValue.ToString() ?? "[]") ?? [];
1113+
foreach (var (key, value) in contents.FirstOrDefault() ?? new Dictionary<string, object>())
1114+
{
1115+
itemData.TryAdd(key, value);
1116+
}
1117+
}
1118+
itemData = ExpandEncodedJson(itemData);
1119+
1120+
SuccessMessage = GenerateSuccessMessage(flow.UpdateItemMessage, "update", itemData, flow.Title);
11011121
}
11021122
}
11031123

@@ -1583,6 +1603,7 @@ public async Task<IActionResult> OnPostRemoveCollectionItemAsync(string fieldId,
15831603
}
15841604

15851605
// Generate success message using custom message or fallback
1606+
itemData = ExpandEncodedJson(itemData);
15861607
SuccessMessage = GenerateSuccessMessage(flow.DeleteItemMessage, "delete", itemData, flowTitle);
15871608
}
15881609
}
@@ -2279,72 +2300,6 @@ private void LoadExistingFlowItemData(string flowId, string instanceId)
22792300
}
22802301
}
22812302

2282-
/// <summary>
2283-
/// Generates a success message using custom template or fallback default
2284-
/// </summary>
2285-
/// <param name="customMessage">Custom message template from configuration</param>
2286-
/// <param name="operation">Operation type: "add", "update", or "delete"</param>
2287-
/// <param name="itemData">Dictionary containing all field values for the item</param>
2288-
/// <param name="flowTitle">Title of the flow</param>
2289-
/// <returns>Formatted success message</returns>
2290-
private string GenerateSuccessMessage(string? customMessage, string operation, Dictionary<string, object>? itemData, string? flowTitle)
2291-
{
2292-
// If custom message is provided, use it with placeholder substitution
2293-
if (!string.IsNullOrEmpty(customMessage))
2294-
{
2295-
var message = customMessage;
2296-
2297-
// Replace {flowTitle} placeholder
2298-
message = message.Replace("{flowTitle}", flowTitle ?? "collection");
2299-
2300-
// Replace field-based placeholders like {firstName}, {gender}, etc.
2301-
if (itemData != null)
2302-
{
2303-
foreach (var kvp in itemData)
2304-
{
2305-
var placeholder = $"{{{kvp.Key}}}";
2306-
var value = kvp.Value?.ToString() ?? "";
2307-
message = message.Replace(placeholder, value);
2308-
}
2309-
}
2310-
2311-
return message;
2312-
}
2313-
2314-
// Fallback to default messages - try to use itemTitleBinding or first available field
2315-
string displayName = "Item";
2316-
if (itemData != null && itemData.Any())
2317-
{
2318-
// Try common name fields first, then fall back to any non-empty value
2319-
var nameFields = new[] { "firstName", "name", "title", "label" };
2320-
var nameField = nameFields.FirstOrDefault(field => itemData.ContainsKey(field) && !string.IsNullOrEmpty(itemData[field]?.ToString()));
2321-
2322-
if (nameField != null)
2323-
{
2324-
displayName = itemData[nameField]?.ToString() ?? "Item";
2325-
}
2326-
else
2327-
{
2328-
// Use the first non-empty field value
2329-
var firstValue = itemData.Values.FirstOrDefault(v => !string.IsNullOrEmpty(v?.ToString()));
2330-
if (firstValue != null)
2331-
{
2332-
displayName = firstValue.ToString() ?? "Item";
2333-
}
2334-
}
2335-
}
2336-
2337-
var lowerFlowTitle = flowTitle?.ToLowerInvariant() ?? "collection";
2338-
2339-
return operation switch
2340-
{
2341-
"add" => $"{displayName} has been added to {lowerFlowTitle}",
2342-
"update" => $"{displayName} has been updated",
2343-
"delete" => $"{displayName} has been removed from {lowerFlowTitle}",
2344-
_ => $"{displayName} has been processed"
2345-
};
2346-
}
2347-
23482303
/// <summary>
23492304
/// Check if a field should be hidden based on conditional logic
23502305
/// </summary>

‎src/DfE.ExternalApplications.Web/Views/Shared/FormEngine/_SingleCollectionFlow.cshtml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
List<Dictionary<string, object>> items = new();
1616
try { items = System.Text.Json.JsonSerializer.Deserialize<List<Dictionary<string, object>>>(json) ?? new(); } catch { }
1717
var itemLabel = flow.ItemTitleBinding ?? "Item";
18+
var itemLabelPlural = flow.ItemTitleBindingPlural ?? $"{itemLabel}s";
1819
}
1920

2021
<div class="govuk-!-margin-bottom-8">
@@ -463,7 +464,7 @@
463464
var noItemsHintId = flow.FlowId + "-no-items-added-hint";
464465

465466
<div class="govuk-inset-text" id="@noItemsHintId">
466-
No @(itemLabel.ToLowerInvariant())s have been added.
467+
No @(itemLabelPlural.ToLowerInvariant()) have been added.
467468
</div>
468469
}
469470

0 commit comments

Comments
 (0)