From 055fd5a940c55acc04e5585f9b097095141df478 Mon Sep 17 00:00:00 2001
From: Toastie <toastie@toastiet0ast.com>
Date: Fri, 27 Dec 2024 03:39:00 +1300
Subject: [PATCH] Improved error handling during template validation

---
 Commands/InterviewTemplateCommands.cs     |  58 ++++++-----
 Interviews/Interview.cs                   | 113 +++++++++++-----------
 Interviews/interview_template.schema.json |   6 +-
 Utilities.cs                              |  12 +++
 4 files changed, 104 insertions(+), 85 deletions(-)

diff --git a/Commands/InterviewTemplateCommands.cs b/Commands/InterviewTemplateCommands.cs
index 9a21210..e628dfd 100644
--- a/Commands/InterviewTemplateCommands.cs
+++ b/Commands/InterviewTemplateCommands.cs
@@ -38,7 +38,7 @@ public class InterviewTemplateCommands
             return;
         }
 
-        if (!Database.TryGetCategory(category.Id, out Database.Category categoryData))
+        if (!Database.TryGetCategory(category.Id, out Database.Category _))
         {
             await command.RespondAsync(new DiscordEmbedBuilder
             {
@@ -99,17 +99,15 @@ public class InterviewTemplateCommands
             return;
         }
 
+        Stream stream = await new HttpClient().GetStreamAsync(file.Url);
         try
         {
-            List<string> errors = [];
-
-            Stream stream = await new HttpClient().GetStreamAsync(file.Url);
             JSchemaValidatingReader validatingReader = new(new JsonTextReader(new StreamReader(stream)));
             validatingReader.Schema = JSchema.Parse(jsonSchema);
 
             // The schema seems to throw an additional error with incorrect information if an invalid parameter is included
             // in the template. Throw here in order to only show the first correct error to the user, also skips unnecessary validation further down.
-            validatingReader.ValidationEventHandler += (o, a) => throw new JsonException(a.Message);
+            validatingReader.ValidationEventHandler += (_, a) => throw new JsonException(a.Message);
 
             JsonSerializer serializer = new();
             Template template = serializer.Deserialize<Template>(validatingReader);
@@ -135,24 +133,15 @@ public class InterviewTemplateCommands
                 return;
             }
 
-            template.interview.Validate(ref errors, out int summaryCount, out int summaryMaxLength);
-
-            if (summaryCount > 25)
-            {
-                errors.Add("A summary cannot contain more than 25 fields, but you have " + summaryCount + " fields in at least one of your interview branches.");
-            }
-
-            if (summaryMaxLength >= 6000)
-            {
-                errors.Add("A summary cannot contain more than 6000 characters, but at least one of your branches has the possibility of its summary reaching " + summaryMaxLength + " characters.");
-            }
-
+            List<string> errors = [];
+            List<string> warnings = [];
+            template.interview.Validate(ref errors, ref warnings, "interview", 0, 0);
             if (errors.Count != 0)
             {
-                string errorString = string.Join("\n\n", errors);
-                if (errorString.Length > 1500)
+                string errorString = string.Join("```\n```", errors);
+                if (errorString.Length > 4000)
                 {
-                    errorString = errorString.Substring(0, 1500);
+                    errorString = errorString.Substring(0, 4000);
                 }
 
                 await command.RespondAsync(new DiscordEmbedBuilder
@@ -173,6 +162,29 @@ public class InterviewTemplateCommands
                 return;
             }
 
+            if (warnings.Count == 0)
+            {
+                await command.RespondAsync(new DiscordEmbedBuilder
+                {
+                    Color = DiscordColor.Green,
+                    Description = "Uploaded interview template."
+                }, true);
+            }
+            else
+            {
+                string warningString = string.Join("```\n```", warnings);
+                if (warningString.Length > 4000)
+                {
+                    warningString = warningString.Substring(0, 4000);
+                }
+
+                await command.RespondAsync(new DiscordEmbedBuilder
+                {
+                    Color = DiscordColor.Orange,
+                    Description = "Uploaded interview template.\n\n**Warnings:**\n```\n" + warningString + "\n```"
+                }, true);
+            }
+
             try
             {
                 MemoryStream memStream = new(Encoding.UTF8.GetBytes(Database.GetInterviewTemplateJSON(template.categoryID)));
@@ -204,12 +216,6 @@ public class InterviewTemplateCommands
             }, true);
             return;
         }
-
-        await command.RespondAsync(new DiscordEmbedBuilder
-        {
-            Color = DiscordColor.Green,
-            Description = "Uploaded interview template."
-        }, true);
     }
 
     [RequireGuild]
diff --git a/Interviews/Interview.cs b/Interviews/Interview.cs
index da31542..fc5fe19 100644
--- a/Interviews/Interview.cs
+++ b/Interviews/Interview.cs
@@ -115,7 +115,7 @@ public class InterviewQuestion
         }
 
         // Check children.
-        foreach (KeyValuePair<string, InterviewQuestion> path in paths)
+        foreach (KeyValuePair<string,InterviewQuestion> path in paths)
         {
             // This child either is the one we are looking for or contains the one we are looking for.
             if (path.Value.TryGetCurrentQuestion(out question))
@@ -188,73 +188,29 @@ public class InterviewQuestion
     {
         return buttonStyle switch
         {
-            ButtonType.PRIMARY => DiscordButtonStyle.Primary,
+            ButtonType.PRIMARY   => DiscordButtonStyle.Primary,
             ButtonType.SECONDARY => DiscordButtonStyle.Secondary,
-            ButtonType.SUCCESS => DiscordButtonStyle.Success,
-            ButtonType.DANGER => DiscordButtonStyle.Danger,
-            _ => DiscordButtonStyle.Secondary
+            ButtonType.SUCCESS   => DiscordButtonStyle.Success,
+            ButtonType.DANGER    => DiscordButtonStyle.Danger,
+            _                    => DiscordButtonStyle.Secondary
         };
     }
 
-    public void Validate(ref List<string> errors, out int summaryCount, out int summaryMaxLength)
+    public void Validate(ref List<string> errors, ref List<string> warnings, string stepID, int summaryFieldCount, int summaryMaxLength)
     {
-        if (string.IsNullOrWhiteSpace(message))
-        {
-            errors.Add("Message cannot be empty.");
-        }
-
-        if (type is QuestionType.ERROR or QuestionType.END_WITH_SUMMARY or QuestionType.END_WITHOUT_SUMMARY)
-        {
-            if (paths.Count > 0)
-            {
-                errors.Add("'" + type + "' questions cannot have child paths.");
-            }
-        }
-        else if (paths.Count == 0)
-        {
-            errors.Add("'" + type + "' questions must have at least one child path.");
-        }
-
-        List<int> summaryCounts = [];
-        Dictionary<string, int> childMaxLengths = new Dictionary<string, int>();
-        foreach (KeyValuePair<string, InterviewQuestion> path in paths)
-        {
-            path.Value.Validate(ref errors, out int summaries, out int maxLen);
-            summaryCounts.Add(summaries);
-            childMaxLengths.Add(path.Key, maxLen);
-        }
-
-        summaryCount = summaryCounts.Count == 0 ? 0 : summaryCounts.Max();
-
-        string childPathString = "";
-        int childMaxLength = 0;
-        if (childMaxLengths.Count != 0)
-        {
-            (childPathString, childMaxLength) = childMaxLengths.ToArray().MaxBy(x => x.Key.Length + x.Value);
-        }
-
-        summaryMaxLength = childMaxLength;
-
-        if (string.IsNullOrWhiteSpace(summaryField))
-        {
-            ++summaryCount;
-        }
-
-        // Only count summaries that end in a summary question.
-        if (type == QuestionType.END_WITH_SUMMARY)
-        {
-            summaryMaxLength = message?.Length ?? 0;
-            summaryMaxLength += title?.Length ?? 0;
-        }
-        // Only add to the total max length if the summary field is not empty. That way we know this branch ends in a summary.
-        else if (summaryMaxLength > 0 && !string.IsNullOrEmpty(summaryField))
+        if (!string.IsNullOrWhiteSpace(summaryField))
         {
+            ++summaryFieldCount;
             summaryMaxLength += summaryField.Length;
             switch (type)
             {
                 case QuestionType.BUTTONS:
                 case QuestionType.TEXT_SELECTOR:
-                    summaryMaxLength += childPathString.Length;
+                    // Get the longest button/selector text
+                    if (paths.Count > 0)
+                    {
+                        summaryMaxLength += paths.Max(kv => kv.Key.Length);
+                    }
                     break;
                 case QuestionType.USER_SELECTOR:
                 case QuestionType.ROLE_SELECTOR:
@@ -273,6 +229,49 @@ public class InterviewQuestion
                     break;
             }
         }
+
+        if (type is QuestionType.ERROR or QuestionType.END_WITH_SUMMARY or QuestionType.END_WITHOUT_SUMMARY)
+        {
+            if (paths.Count > 0)
+            {
+                warnings.Add("'" + type + "' paths cannot have child paths.\n\n" + stepID + ".message-type");
+            }
+
+            if (!string.IsNullOrWhiteSpace(summaryField))
+            {
+                warnings.Add("'" + type + "' paths cannot have summary field names.\n\n" + stepID + ".summary-field");
+            }
+        }
+        else if (paths.Count == 0)
+        {
+            errors.Add("'" + type + "' paths must have at least one child path.\n\n" + stepID + ".message-type");
+        }
+
+        if (type is QuestionType.END_WITH_SUMMARY)
+        {
+            summaryMaxLength += message?.Length ?? 0;
+            summaryMaxLength += title?.Length ?? 0;
+            if (summaryFieldCount > 25)
+            {
+                errors.Add("A summary cannot contain more than 25 fields, but you have " + summaryFieldCount + " fields in this branch.\n\n" + stepID);
+            }
+            else if (summaryMaxLength >= 6000)
+            {
+                warnings.Add("A summary cannot contain more than 6000 characters, but this branch may reach " + summaryMaxLength + " characters.\n" +
+                             "Use the \"max-length\" parameter to limit text input field lengths, or shorten other parts of the summary message.\n\n" + stepID);
+            }
+        }
+
+        foreach (KeyValuePair<string,InterviewQuestion> path in paths)
+        {
+            // The JSON schema error messages use this format for the JSON path, so we use it here too.
+            string nextStepID = stepID;
+            nextStepID += path.Key.ContainsAny('.', ' ', '[', ']', '(', ')', '/', '\\')
+                ? ".paths['" + path.Key + "']"
+                : ".paths." + path.Key;
+
+            path.Value.Validate(ref errors, ref warnings, nextStepID, summaryFieldCount, summaryMaxLength);
+        }
     }
 
     public class StripInternalPropertiesResolver : DefaultContractResolver
diff --git a/Interviews/interview_template.schema.json b/Interviews/interview_template.schema.json
index e3ab224..9e0fe4e 100644
--- a/Interviews/interview_template.schema.json
+++ b/Interviews/interview_template.schema.json
@@ -124,12 +124,14 @@
         "max-length": {
           "type": "number",
           "title": "Max Length",
-          "description": "The maximum length of the text input."
+          "description": "The maximum length of the text input.",
+          "maximum": 1024
         },
         "min-length": {
           "type": "number",
           "title": "Min Length",
-          "description": "The minimum length of the text input."
+          "description": "The minimum length of the text input.",
+          "minimum": 0
         }
       },
       "required": [ "message", "message-type", "color", "paths" ],
diff --git a/Utilities.cs b/Utilities.cs
index 8d368b8..44ee39f 100644
--- a/Utilities.cs
+++ b/Utilities.cs
@@ -8,6 +8,18 @@ using DSharpPlus.Entities;
 
 namespace SupportChild;
 
+public static class Extensions
+{
+    public static bool ContainsAny(this string haystack, params string[] needles)
+    {
+        return needles.Any(haystack.Contains);
+    }
+    public static bool ContainsAny(this string haystack, params char[] needles)
+    {
+        return needles.Any(haystack.Contains);
+    }
+}
+
 public static class Utilities
 {
     private static readonly Random rng = new Random();