Implement changes to Checks API for Annotations models and re-request…

… endpoint (#1857)

* Attempt to handle both old and new annotations models so we support the changes on github.com as well as still support GHE2.14
add Path and AnnotationLevel fields
flag Filename and WarningLevel as deprecated/obsolete
also flag BlobHref as deprecated on NewCheckRunAnnotation
Adjust ctors to handle new and legacy field options

* adjust tests to remove use of obsoleted fields

* fix a couple of other tests using unrelated obsoleted fields

* Mark check suite Request method and request object as obsolete

* Add Rerequest() method to normal and observable clients
Add unit and integration tests

* add StartColumn and EndColumn as optional fields for CheckRunAnnotation response and NewCheckRunAnnotation request

* remove integration tests for Request() method as they no longer work on github.com anyway

Octokit.Reactive/Clients/IObservableCheckSuitesClient.cs

@@ -151,6 +151,7 @@ public interface IObservableCheckSuitesClient
         /// <param name="owner">The owner of the repository</param>
         /// <param name="name">The name of the repository</param>
         /// <param name="request">Details of the Check Suite request</param>
+        [Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
         IObservable<bool> Request(string owner, string name, CheckSuiteTriggerRequest request);
 
         /// <summary>
@@ -161,6 +162,28 @@ public interface IObservableCheckSuitesClient
         /// </remarks>
         /// <param name="repositoryId">The Id of the repository</param>
         /// <param name="request">Details of the Check Suite request</param>
+        [Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
         IObservable<bool> Request(long repositoryId, CheckSuiteTriggerRequest request);
+
+        /// <summary>
+        /// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
+        /// </summary>
+        /// <remarks>
+        /// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
+        /// </remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="checkSuiteId">The Id of the check suite</param>
+        IObservable<bool> Rerequest(string owner, string name, long checkSuiteId);
+
+        /// <summary>
+        /// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
+        /// </summary>
+        /// <remarks>
+        /// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
+        /// </remarks>
+        /// <param name="repositoryId">The Id of the repository</param>
+        /// <param name="checkSuiteId">The Id of the check suite</param>
+        IObservable<bool> Rerequest(long repositoryId, long checkSuiteId);
     }
 }

Octokit.Reactive/Clients/ObservableCheckSuitesClient.cs

@@ -243,6 +243,7 @@ public IObservable<CheckSuite> Create(long repositoryId, NewCheckSuite newCheckS
         /// <param name="owner">The owner of the repository</param>
         /// <param name="name">The name of the repository</param>
         /// <param name="request">Details of the Check Suite request</param>
+        [Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
         public IObservable<bool> Request(string owner, string name, CheckSuiteTriggerRequest request)
         {
             Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
@@ -260,12 +261,42 @@ public IObservable<bool> Request(string owner, string name, CheckSuiteTriggerReq
         /// </remarks>
         /// <param name="repositoryId">The Id of the repository</param>
         /// <param name="request">Details of the Check Suite request</param>
-
+        [Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
         public IObservable<bool> Request(long repositoryId, CheckSuiteTriggerRequest request)
         {
             Ensure.ArgumentNotNull(request, nameof(request));
 
             return _client.Request(repositoryId, request).ToObservable();
         }
+
+        /// <summary>
+        /// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
+        /// </summary>
+        /// <remarks>
+        /// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
+        /// </remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="checkSuiteId">The Id of the check suite</param>
+        public IObservable<bool> Rerequest(string owner, string name, long checkSuiteId)
+        {
+            Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
+            Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
+
+            return _client.Rerequest(owner, name, checkSuiteId).ToObservable();
+        }
+
+        /// <summary>
+        /// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
+        /// </summary>
+        /// <remarks>
+        /// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
+        /// </remarks>
+        /// <param name="repositoryId">The Id of the repository</param>
+        /// <param name="checkSuiteId">The Id of the check suite</param>
+        public IObservable<bool> Rerequest(long repositoryId, long checkSuiteId)
+        {
+            return _client.Rerequest(repositoryId, checkSuiteId).ToObservable();
+        }
     }
 }

Octokit.Tests.Integration/Clients/CheckRunsClientTests.cs

@@ -409,7 +409,7 @@ public async Task GetsAllAnnotations()
                         {
                             Annotations = new[]
                             {
-                                new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
+                                new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
                             }
                         }
                     };
@@ -421,7 +421,7 @@ public async Task GetsAllAnnotations()
                     // Check result
                     Assert.Equal(1, annotations.Count);
                     Assert.Equal("this is a warning", annotations.First().Message);
-                    Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
+                    Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
                 }
             }
 
@@ -442,7 +442,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
                         {
                             Annotations = new[]
                             {
-                                new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
+                                new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
                             }
                         }
                     };
@@ -454,7 +454,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
                     // Check result
                     Assert.Equal(1, annotations.Count);
                     Assert.Equal("this is a warning", annotations.First().Message);
-                    Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
+                    Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
                 }
             }
         }

Octokit.Tests.Integration/Clients/CheckSuitesClientTests.cs

@@ -213,12 +213,12 @@ public async Task CreatesCheckSuiteWithRepositoryId()
             }
         }
 
-        public class TheRequestMethod
+        public class TheRerequestMethod
         {
             IGitHubClient _github;
             IGitHubClient _githubAppInstallation;
 
-            public TheRequestMethod()
+            public TheRerequestMethod()
             {
                 _github = Helper.GetAuthenticatedClient();
 
@@ -227,26 +227,32 @@ public TheRequestMethod()
             }
 
             [GitHubAppsTest]
-            public async Task RequestsCheckSuite()
+            public async Task RerequestsCheckSuite()
             {
                 using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
                 {
-                    var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryOwner, repoContext.RepositoryName, "master");
+                    // Need to get a CheckSuiteId so we can test the Get method
+                    var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
+                    var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();
 
-                    var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryOwner, repoContext.RepositoryName, new CheckSuiteTriggerRequest(headCommit.Sha));
+                    // Get Check Suite by Id
+                    var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryOwner, repoContext.RepositoryName, checkSuite.Id);
 
                     Assert.True(result);
                 }
             }
 
             [GitHubAppsTest]
-            public async Task RequestsCheckSuiteWithRepositoryId()
+            public async Task RerequestsCheckSuiteWithRepositoryId()
             {
                 using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
                 {
+                    // Need to get a CheckSuiteId so we can test the Get method
                     var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
+                    var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();
 
-                    var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryId, new CheckSuiteTriggerRequest(headCommit.Sha));
+                    // Get Check Suite by Id
+                    var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryId, checkSuite.Id);
 
                     Assert.True(result);
                 }

Octokit.Tests.Integration/Reactive/ObservableCheckRunsClientTests.cs

@@ -410,7 +410,7 @@ public async Task GetsAllAnnotations()
                         {
                             Annotations = new[]
                             {
-                                new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
+                                new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
                             }
                         }
                     };
@@ -422,7 +422,7 @@ public async Task GetsAllAnnotations()
                     // Check result
                     Assert.Equal(1, annotations.Count);
                     Assert.Equal("this is a warning", annotations.First().Message);
-                    Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
+                    Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
                 }
             }
 
@@ -443,7 +443,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
                         {
                             Annotations = new[]
                             {
-                                new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
+                                new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
                             }
                         }
                     };
@@ -455,7 +455,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
                     // Check result
                     Assert.Equal(1, annotations.Count);
                     Assert.Equal("this is a warning", annotations.First().Message);
-                    Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
+                    Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
                 }
             }
         }

Octokit.Tests.Integration/Reactive/ObservableCheckSuitesClientTests.cs

@@ -215,12 +215,12 @@ public async Task CreatesCheckSuiteWithRepositoryId()
             }
         }
 
-        public class TheRequestMethod
+        public class TheRerequestMethod
         {
             IObservableGitHubClient _github;
             IObservableGitHubClient _githubAppInstallation;
 
-            public TheRequestMethod()
+            public TheRerequestMethod()
             {
                 _github = new ObservableGitHubClient(Helper.GetAuthenticatedClient());
 
@@ -229,26 +229,30 @@ public TheRequestMethod()
             }
 
             [GitHubAppsTest]
-            public async Task RequestsCheckSuite()
+            public async Task RerequestsCheckSuite()
             {
                 using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
                 {
-                    var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryOwner, repoContext.RepositoryName, "master");
+                    // Need to get a CheckSuiteId so we can test the Get method
+                    var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
+                    var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();
 
-                    var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryOwner, repoContext.RepositoryName, new CheckSuiteTriggerRequest(headCommit.Sha));
+                    var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryOwner, repoContext.RepositoryName, checkSuite.Id);
 
                     Assert.True(result);
                 }
             }
 
             [GitHubAppsTest]
-            public async Task RequestsCheckSuiteWithRepositoryId()
+            public async Task RerequestsCheckSuiteWithRepositoryId()
             {
                 using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
                 {
+                    // Need to get a CheckSuiteId so we can test the Get method
                     var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
+                    var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();
 
-                    var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryId, new CheckSuiteTriggerRequest(headCommit.Sha));
+                    var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryId, checkSuite.Id);
 
                     Assert.True(result);
                 }

Octokit.Tests/Clients/CheckSuitesClientTests.cs

@@ -332,6 +332,7 @@ public async Task EnsuresNonEmptyArguments()
             }
         }
 
+#pragma warning disable CS0618 // Type or member is obsolete
         public class TheRequestMethod
         {
             [Fact]
@@ -393,5 +394,57 @@ public async Task EnsuresNonEmptyArguments()
                 await Assert.ThrowsAsync<ArgumentException>(() => client.Request("fake", "", request));
             }
         }
+#pragma warning restore CS0618 // Type or member is obsolete
+
+        public class TheRerequestMethod
+        {
+            [Fact]
+            public async Task RequestsCorrectUrl()
+            {
+                var connection = MockedIApiConnection.PostReturnsHttpStatus(HttpStatusCode.Created);
+                var client = new CheckSuitesClient(connection);
+
+                await client.Rerequest("fake", "repo", 1);
+
+                connection.Connection.Received().Post(
+                    Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/check-suites/1/rerequest"),
+                    Args.Object,
+                    "application/vnd.github.antiope-preview+json");
+            }
+
+            [Fact]
+            public async Task RequestsCorrectUrlWithRepositoryId()
+            {
+                var connection = MockedIApiConnection.PostReturnsHttpStatus(HttpStatusCode.Created);
+                var client = new CheckSuitesClient(connection);
+
+                await client.Rerequest(1, 1);
+
+                connection.Connection.Received().Post(
+                    Arg.Is<Uri>(u => u.ToString() == "repositories/1/check-suites/1/rerequest"),
+                    Args.Object,
+                    "application/vnd.github.antiope-preview+json");
+            }
+
+            [Fact]
+            public async Task EnsuresNonNullArguments()
+            {
+                var connection = Substitute.For<IApiConnection>();
+                var client = new CheckSuitesClient(connection);
+
+                await Assert.ThrowsAsync<ArgumentNullException>(() => client.Rerequest(null, "repo", 1));
+                await Assert.ThrowsAsync<ArgumentNullException>(() => client.Rerequest("fake", null, 1));
+            }
+
+            [Fact]
+            public async Task EnsuresNonEmptyArguments()
+            {
+                var connection = Substitute.For<IApiConnection>();
+                var client = new CheckSuitesClient(connection);
+
+                await Assert.ThrowsAsync<ArgumentException>(() => client.Rerequest("", "repo", 1));
+                await Assert.ThrowsAsync<ArgumentException>(() => client.Rerequest("fake", "", 1));
+            }
+        }
     }
 }

Octokit.Tests/Clients/ProjectCardsClientTests.cs

@@ -115,7 +115,10 @@ public async Task PostsToCorrectURL()
             {
                 var connection = Substitute.For<IApiConnection>();
                 var client = new ProjectCardsClient(connection);
-                var updateCard = new ProjectCardUpdate("someNewNote");
+                var updateCard = new ProjectCardUpdate
+                {
+                    Note = "someNewNote"
+                };
 
                 await client.Update(1, updateCard);
 
@@ -129,7 +132,10 @@ public async Task PostsToCorrectURL()
             public async Task EnsuresNonNullArguments()
             {
                 var client = new ProjectCardsClient(Substitute.For<IApiConnection>());
-                var updateCard = new ProjectCardUpdate("someNewNote");
+                var updateCard = new ProjectCardUpdate
+                {
+                    Note = "someNewNote"
+                };
 
                 await Assert.ThrowsAsync<ArgumentNullException>(() => client.Update(1, null));
             }