Merge pull request #369 from gep13/feature/GH113

(#113) Return all issues that have errors

Author: gep13

src/GitReleaseManager.Core/Exceptions/InvalidIssuesException.cs

@@ -0,0 +1,38 @@
+using System;
+using System.Collections.Generic;
+
+namespace GitReleaseManager.Core.Exceptions
+{
+    [Serializable]
+    public class InvalidIssuesException : Exception
+    {
+        public InvalidIssuesException()
+        {
+        }
+
+        public InvalidIssuesException(List<string> errors)
+            : base(string.Join(Environment.NewLine, errors))
+        {
+            Errors = errors;
+        }
+
+        public InvalidIssuesException(List<string> errors, string message)
+            : base(message)
+        {
+            Errors = errors;
+        }
+
+        public InvalidIssuesException(List<string> errors, string message, Exception inner)
+            : base(message, inner)
+        {
+            Errors = errors;
+        }
+
+        public List<string> Errors { get; set; }
+
+        protected InvalidIssuesException(
+            System.Runtime.Serialization.SerializationInfo info,
+            System.Runtime.Serialization.StreamingContext context)
+            : base(info, context) { }
+    }
+}

src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Threading.Tasks;
 using GitReleaseManager.Core.Configuration;
+using GitReleaseManager.Core.Exceptions;
 using GitReleaseManager.Core.Helpers;
 using GitReleaseManager.Core.Model;
 using GitReleaseManager.Core.Provider;
@@ -124,36 +125,43 @@ private string GetValidLabel(string label, int issuesCount)
             return newLabel;
         }
 
-        private bool CheckForValidLabels(Issue issue)
+        private (List<Issue> IssuesWithValidLabel, List<string> Errors) CheckIssuesForValidLabels(IEnumerable<Issue> issues)
         {
-            var includedIssuesCount = 0;
-            var excludedIssuesCount = 0;
+            var validIssues = new List<Issue>();
+            var errors = new List<string>();
 
-            foreach (var issueLabel in issue.Labels)
+            foreach (var issue in issues)
             {
-                includedIssuesCount += _configuration.IssueLabelsInclude.Count(issueToInclude => issueLabel.Name.ToUpperInvariant() == issueToInclude.ToUpperInvariant());
+                var includedIssuesCount = 0;
+                var excludedIssuesCount = 0;
 
-                excludedIssuesCount += _configuration.IssueLabelsExclude.Count(issueToExclude => issueLabel.Name.ToUpperInvariant() == issueToExclude.ToUpperInvariant());
-            }
+                foreach (var issueLabel in issue.Labels)
+                {
+                    includedIssuesCount += _configuration.IssueLabelsInclude.Count(issueToInclude => issueLabel.Name.ToUpperInvariant() == issueToInclude.ToUpperInvariant());
 
-            if (includedIssuesCount + excludedIssuesCount != 1)
-            {
-                var allIssueLabels = _configuration.IssueLabelsInclude.Union(_configuration.IssueLabelsExclude).ToList();
-                var allIssuesExceptLast = allIssueLabels.Take(allIssueLabels.Count - 1);
-                var lastLabel = allIssueLabels.Last();
+                    excludedIssuesCount += _configuration.IssueLabelsExclude.Count(issueToExclude => issueLabel.Name.ToUpperInvariant() == issueToExclude.ToUpperInvariant());
+                }
 
-                var allIssuesExceptLastString = string.Join(", ", allIssuesExceptLast);
+                if (includedIssuesCount + excludedIssuesCount != 1)
+                {
+                    var allIssueLabels = _configuration.IssueLabelsInclude.Union(_configuration.IssueLabelsExclude).ToList();
+                    var allIssuesExceptLast = allIssueLabels.Take(allIssueLabels.Count - 1);
+                    var lastLabel = allIssueLabels.Last();
 
-                var message = string.Format(CultureInfo.InvariantCulture, "Bad Issue {0} expected to find a single label with either {1} or {2}.", issue.HtmlUrl, allIssuesExceptLastString, lastLabel);
-                throw new InvalidOperationException(message);
-            }
+                    var allIssuesExceptLastString = string.Join(", ", allIssuesExceptLast);
 
-            if (includedIssuesCount > 0)
-            {
-                return true;
+                    var message = string.Format(CultureInfo.InvariantCulture, "Bad Issue {0} expected to find a single label with either {1} or {2}.", issue.HtmlUrl, allIssuesExceptLastString, lastLabel);
+                    errors.Add(message);
+                    continue;
+                }
+
+                if (includedIssuesCount > 0)
+                {
+                    validIssues.Add(issue);
+                }
             }
 
-            return false;
+            return (validIssues, errors);
         }
 
         private Milestone GetPreviousMilestone()
@@ -173,28 +181,25 @@ private async Task LoadMilestonesAsync()
 
         private async Task<List<Issue>> GetIssuesAsync(Milestone milestone)
         {
-            var issues = await _vcsProvider.GetIssuesAsync(_user, _repository, milestone.Number, ItemStateFilter.Closed).ConfigureAwait(false);
+            var allIssues = await _vcsProvider.GetIssuesAsync(_user, _repository, milestone.Number, ItemStateFilter.Closed).ConfigureAwait(false);
 
-            var hasIncludedIssues = false;
+            var result = CheckIssuesForValidLabels(allIssues);
 
-            foreach (var issue in issues)
+            if (result.Errors.Count > 0)
             {
-                if (CheckForValidLabels(issue))
-                {
-                    hasIncludedIssues = true;
-                }
+                throw new InvalidIssuesException(result.Errors);
             }
 
             // If there are no issues assigned to the milestone that have a label that is part
             // of the labels to include array, then that is essentially the same as having no
             // closed issues assigned to the milestone.  In this scenario, we want to raise an
             // error, so return an emtpy issues list.
-            if (!hasIncludedIssues)
+            if (!result.IssuesWithValidLabel.Any())
             {
                 return new List<Issue>();
             }
 
-            return issues.ToList();
+            return allIssues.ToList();
         }
 
         private void GetTargetMilestone()

src/GitReleaseManager.Tests/ReleaseNotesBuilderTests.cs

@@ -4,6 +4,7 @@
 using System.Threading.Tasks;
 using ApprovalTests;
 using GitReleaseManager.Core.Configuration;
+using GitReleaseManager.Core.Exceptions;
 using GitReleaseManager.Core.Helpers;
 using GitReleaseManager.Core.Model;
 using GitReleaseManager.Core.Provider;
@@ -150,14 +151,30 @@ public void CorrectlyUseFooterWithMilestoneWhenEnabled()
         public void NoCommitsWrongIssueLabel()
         {
             var exception = Assert.Throws<AggregateException>(() => AcceptTest(0, CreateIssue(1, "Test")));
-            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidOperationException>());
+            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidIssuesException>());
         }
 
         [Test]
         public void SomeCommitsWrongIssueLabel()
         {
             var exception = Assert.Throws<AggregateException>(() => AcceptTest(5, CreateIssue(1, "Test")));
-            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidOperationException>());
+            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidIssuesException>());
+        }
+
+        [Test]
+        public void NoCommitsMultipleWrongIssueLabel()
+        {
+            var exception = Assert.Throws<AggregateException>(() => AcceptTest(0, CreateIssue(1, "Test"), CreateIssue(2, "Test")));
+            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidIssuesException>());
+            Assert.That((exception.InnerException as InvalidIssuesException).Errors.Count, Is.EqualTo(2));
+        }
+
+        [Test]
+        public void SomeCommitsMultipleWrongIssueLabel()
+        {
+            var exception = Assert.Throws<AggregateException>(() => AcceptTest(5, CreateIssue(1, "Test"), CreateIssue(2, "Test"), CreateIssue(3, "Bob")));
+            Assert.That(exception.InnerException, Is.Not.Null.And.TypeOf<InvalidIssuesException>());
+            Assert.That((exception.InnerException as InvalidIssuesException).Errors.Count, Is.EqualTo(3));
         }
 
         [Test]