Feature/reviewer improvements (#153)

* Throw exception if OCR fails (instead of reporting as a Failure)
* Increased progress report to every 5 seconds
* Added conneciton testing prior to running
* Added injection for redlist, ignore and targets
* Support for custom patterns from gui
* Added default CLI setting for RulesOnly (avoids unessesary updates)
* Updated changelog

Co-authored-by: abrooks <abrooks>

CHANGELOG.md

@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased]
 
+- Improved usability of IsIdentifiableReviewer
+
 ## [1.5.0] - 2020-03-05
 
 - \[Breaking\] Updated RabbitMQ extraction config to match extraction plan v2

src/applications/IsIdentifiableReviewer/IsIdentifiableReviewerOptions.cs

@@ -1,4 +1,5 @@
 using CommandLine;
+using IsIdentifiableReviewer.Out;
 
 namespace IsIdentifiableReviewer
 {
@@ -16,5 +17,34 @@ public class IsIdentifiableReviewerOptions
             HelpText = "[Optional] Runs the application automatically processing existing update/ignore rules.  Failures not matching either are written to a new file with this path"
         )]
         public string UnattendedOutputPath { get; set; }
+
+        [Option('t', "targets",
+            Required = false,
+            Default = "Targets.yaml",
+            HelpText = "Location of database connection strings file (for issuing UPDATE statements)"
+        )]
+        public string TargetsFile { get; set; }
+
+        [Option('i', "ignore",
+            Required = false,
+            Default = IgnoreRuleGenerator.DefaultFileName,
+            HelpText = "File containing rules for ignoring validation errors"
+        )]
+        public string IgnoreList { get; set; }
+
+        [Option('r', "redlist",
+            Required = false,
+            Default = RowUpdater.DefaultFileName,
+            HelpText = "File containing rules for when to issue UPDATE statements"
+        )]
+        public string RedList { get; set; }
+
+
+        [Option('o', "only-rules",
+            Required = false,
+            Default = false,
+            HelpText = "Specify to make GUI UPDATE choices only create new rules instead of going to database"
+        )]
+        public bool OnlyRules { get; set; }
     }
 }

src/applications/IsIdentifiableReviewer/MainWindow.cs

@@ -5,22 +5,22 @@
 using System.Text;
 using System.Text.RegularExpressions;
 using IsIdentifiableReviewer.Out;
-using Microservices.IsIdentifiable.Rules;
+using Microservices.IsIdentifiable.Reporting;
 using Terminal.Gui;
 using Attribute = Terminal.Gui.Attribute;
 
 namespace IsIdentifiableReviewer
 {
-    class MainWindow : View
+    class MainWindow : View,IRulePatternFactory
     {
         private readonly List<Target> _targets;
 
         public Target CurrentTarget { get; set; }
         public ReportReader CurrentReport { get; set; }
 
-        public IgnoreRuleGenerator Ignorer { get; set; } = new IgnoreRuleGenerator();
+        public IgnoreRuleGenerator Ignorer { get; }
 
-        public RowUpdater Updater { get; set; } = new RowUpdater();
+        public RowUpdater Updater { get;  } 
 
         public int DlgWidth = 78;
         public int DlgHeight = 18;
@@ -29,9 +29,12 @@ class MainWindow : View
         private Label _info;
         private TextField _gotoTextField;
 
-        public MainWindow(List<Target> targets, IsIdentifiableReviewerOptions opts)
+        public MainWindow(List<Target> targets, IsIdentifiableReviewerOptions opts, IgnoreRuleGenerator ignorer, RowUpdater updater)
         {
             _targets = targets;
+            Ignorer = ignorer;
+            Updater = updater;
+
             X = 0;
             Y = 1;
             Width = Dim.Fill();
@@ -105,7 +108,17 @@ public MainWindow(List<Target> targets, IsIdentifiableReviewerOptions opts)
                 Clicked = ()=>GoToRelative(1)
             });
 
-            var cbRulesOnly = new CheckBox(23,1,"Rules Only",false);
+            var cbCustomPattern = new CheckBox(23,1,"Custom Patterns",false);
+            cbCustomPattern.Toggled += (c, s) =>
+            {
+                Updater.RulesFactory = cbCustomPattern.Checked ? this : (IRulePatternFactory)new MatchWholeStringRulePatternFactory();
+                Ignorer.RulesFactory = cbCustomPattern.Checked ? this : (IRulePatternFactory)new MatchWholeStringRulePatternFactory();
+            };
+            frame.Add(cbCustomPattern);
+
+            var cbRulesOnly = new CheckBox(23,2,"Rules Only",opts.OnlyRules);
+            Updater.RulesOnly = opts.OnlyRules;
+
             cbRulesOnly.Toggled += (c, s) => { Updater.RulesOnly = cbRulesOnly.Checked;};
             frame.Add(cbRulesOnly);
 
@@ -218,7 +231,7 @@ private void Update()
 
             try
             {
-                Updater.Update(CurrentTarget,_valuePane.CurrentFailure);
+                Updater.Update(CurrentTarget,_valuePane.CurrentFailure,true);
             }
             catch (Exception e)
             {
@@ -365,12 +378,90 @@ bool RunDialog<T>(string title, string message,out T chosen, params T[] options)
             chosen = result;
             return optionChosen;
         }
+         private bool GetText(string title, string message, string initialValue, out string chosen)
+         {
+            bool optionChosen = false;
+
+            var dlg = new Dialog(title, Math.Min(Console.WindowWidth,DlgWidth), DlgHeight);
+
+            var line = DlgHeight - (DlgBoundary)*2 - 2;
+
+            if (!string.IsNullOrWhiteSpace(message))
+            {
+                int width = Math.Min(Console.WindowWidth,DlgWidth) - (DlgBoundary * 2);
+
+                var msg = Wrap(message, width-1).TrimEnd();
+
+                var text = new Label(0, 0, msg)
+                {
+                    Height = line - 1, Width = width
+                };
+
+                //if it is too long a message
+                int newlines = msg.Count(c => c == '\n');
+                if (newlines > line - 1)
+                {
+                    var view = new ScrollView(new Rect(0, 0, width, line - 1))
+                    {
+                        ContentSize = new Size(width, newlines + 1),
+                        ContentOffset = new Point(0, 0),
+                        ShowVerticalScrollIndicator = true,
+                        ShowHorizontalScrollIndicator = false
+                    };
+                    view.Add(text);
+                    dlg.Add(view);
+                }
+                else
+                    dlg.Add(text);
+            }
+
+            var txt = new TextField(0, line++, DlgWidth -4 ,initialValue ?? "");
+            dlg.Add(txt);
+
+            var btn = new Button(0, line, "Ok")
+            {
+                IsDefault = true,
+                Clicked = () =>
+                {
+                    dlg.Running = false;
+                    optionChosen = true;
+                }
+            };
+            dlg.Add(btn);
+
+            var btnClear = new Button(15, line, "Clear")
+            {
+                Clicked = () => { txt.Text = ""; }
+            };
+            dlg.Add(btnClear);
+
+
+
+            dlg.FocusFirst();
+
+
+            Application.Run(dlg);
+
+            chosen = txt.Text?.ToString();
+            return optionChosen;
+         }
 
          public static string Wrap(string s, int width)
          {
              var r = new Regex(@"(?:((?>.{1," + width + @"}(?:(?<=[^\S\r\n])[^\S\r\n]?|(?=\r?\n)|$|[^\S\r\n]))|.{1,16})(?:\r?\n)?|(?:\r?\n|$))");
              return r.Replace(s, "$1\n");
          }
-
+
+         public string GetPattern(Failure failure)
+         {
+             var defaultFactory = new MatchWholeStringRulePatternFactory();
+             var recommendedPattern = defaultFactory.GetPattern(failure);
+
+             if(GetText("Pattern","Enter pattern to match failure",recommendedPattern, out string chosen))
+                return chosen;
+
+             throw new Exception("User chose not to enter a pattern");
+         }
+
     }
 }

src/applications/IsIdentifiableReviewer/Out/IRulePatternFactory.cs

@@ -0,0 +1,14 @@
+using Microservices.IsIdentifiable.Reporting;
+
+namespace IsIdentifiableReviewer.Out
+{
+    public interface IRulePatternFactory
+    {
+        /// <summary>
+        /// Returns a Regex for picking up the provided <paramref name="failure"/>
+        /// </summary>
+        /// <param name="failure"></param>
+        /// <returns></returns>
+        string GetPattern(Failure failure);
+    }
+}

src/applications/IsIdentifiableReviewer/Out/MatchWholeStringRulePatternFactory.cs

@@ -0,0 +1,13 @@
+using System.Text.RegularExpressions;
+using Microservices.IsIdentifiable.Reporting;
+
+namespace IsIdentifiableReviewer.Out
+{
+    public class MatchWholeStringRulePatternFactory: IRulePatternFactory
+    {
+        public string GetPattern(Failure failure)
+        {
+            return "^" + Regex.Escape(failure.ProblemValue) + "$";
+        }
+    }
+}

src/applications/IsIdentifiableReviewer/Out/OutBase.cs

@@ -14,6 +14,8 @@ public abstract class OutBase
     {
         protected List<IsIdentifiableRule> Rules { get;}
         public FileInfo RulesFile { get; }
+
+        public IRulePatternFactory RulesFactory { get; set; } = new MatchWholeStringRulePatternFactory();
 
         protected OutBase(FileInfo rulesFile)
         {
@@ -53,7 +55,7 @@ protected void Add(Failure f, RuleAction action)
             {
                 Action = action,
                 IfColumn = f.ProblemField,
-                IfPattern = "^" + Regex.Escape(f.ProblemValue) + "$",
+                IfPattern = RulesFactory.GetPattern(f),
                 As = 
                     action == RuleAction.Ignore? 
                         FailureClassification.None : 

src/applications/IsIdentifiableReviewer/Out/RowUpdater.cs

@@ -30,15 +30,16 @@ public RowUpdater() : this(new FileInfo(DefaultFileName))
         {
         }
 
-        public void Update(Target target, Failure failure)
+        public void Update(Target target, Failure failure, bool addRule)
         {
-            Update(target.Discover(),failure);
+            Update(target.Discover(),failure,addRule);
         }
 
-        public void Update(DiscoveredServer server, Failure failure)
+        public void Update(DiscoveredServer server, Failure failure, bool addRule)
         {
             //add the update rule to the redlist
-            Add(failure,RuleAction.Report);
+            if(addRule)
+                Add(failure,RuleAction.Report);
 
             //if we are running in rules only mode we don't need to also update the database
             if(RulesOnly)
@@ -104,7 +105,7 @@ public bool OnLoad(DiscoveredServer server,Failure failure)
             if (IsCoveredByExistingRule(failure))
             {
                 //since user has issued an update for this exact problem before we can update this one too
-                Update(server,failure);
+                Update(server,failure,false);
 
                 //and return false to indicate that it is not a novel issue
                 return false;