From f6a9a4720046c15bfbff7f53b9220388407ba8d7 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Sun, 15 Mar 2020 11:33:32 -0300 Subject: [PATCH] Graduate review requests to what eventually shipped (#2153) --- ...servablePullRequestReviewRequestsClient.cs | 25 +--- ...servablePullRequestReviewRequestsClient.cs | 41 +----- .../PullRequestReviewRequestsClientTests.cs | 128 ++++-------------- .../PullRequestReviewRequestsClientTests.cs | 108 +++------------ ...blePullRequestReviewRequestsClientTests.cs | 74 ++-------- .../IPullRequestReviewRequestsClient.cs | 28 +--- .../PullRequestReviewRequestsClient.cs | 67 ++------- Octokit/Helpers/AcceptHeaders.cs | 5 +- .../Request/PullRequestReviewRequest.cs | 18 ++- Octokit/Models/Response/RequestedReviews.cs | 40 ++++++ 10 files changed, 140 insertions(+), 394 deletions(-) create mode 100644 Octokit/Models/Response/RequestedReviews.cs diff --git a/Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs b/Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs index c1831760d1..23bf93da4e 100644 --- a/Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs +++ b/Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs @@ -18,25 +18,7 @@ public interface IObservablePullRequestReviewRequestsClient /// The owner of the repository /// The name of the repository /// The pull request number - IObservable GetAll(string owner, string name, int number); - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The owner of the repository - /// The name of the repository - /// The pull request number - /// Options for changing the API response - IObservable GetAll(string owner, string name, int number, ApiOptions options); - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The Id of the repository - /// The pull request number - IObservable GetAll(long repositoryId, int number); + IObservable Get(string owner, string name, int number); /// /// Gets review requests for a specified pull request. @@ -44,8 +26,7 @@ public interface IObservablePullRequestReviewRequestsClient /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests /// The Id of the repository /// The pull request number - /// Options for changing the API response - IObservable GetAll(long repositoryId, int number, ApiOptions options); + IObservable Get(long repositoryId, int number); /// /// Creates review requests on a pull request for specified users. @@ -85,4 +66,4 @@ public interface IObservablePullRequestReviewRequestsClient /// List of logins of users that will be not longer requested for review IObservable Delete(long repositoryId, int number, PullRequestReviewRequest users); } -} \ No newline at end of file +} diff --git a/Octokit.Reactive/Clients/ObservablePullRequestReviewRequestsClient.cs b/Octokit.Reactive/Clients/ObservablePullRequestReviewRequestsClient.cs index 46467c046a..b6a2df841c 100644 --- a/Octokit.Reactive/Clients/ObservablePullRequestReviewRequestsClient.cs +++ b/Octokit.Reactive/Clients/ObservablePullRequestReviewRequestsClient.cs @@ -25,29 +25,12 @@ public ObservablePullRequestReviewRequestsClient(IGitHubClient client) /// The owner of the repository /// The name of the repository /// The pull request number - public IObservable GetAll(string owner, string name, int number) + public IObservable Get(string owner, string name, int number) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); - return _connection.GetAndFlattenAllPages(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview); - } - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The owner of the repository - /// The name of the repository - /// The pull request number - /// Options for changing the API response - public IObservable GetAll(string owner, string name, int number, ApiOptions options) - { - Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); - Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); - Ensure.ArgumentNotNull(options, nameof(options)); - - return _connection.GetAndFlattenAllPages(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options); + return _client.Get(owner, name, number).ToObservable(); } /// @@ -56,23 +39,9 @@ public IObservable GetAll(string owner, string name, int number, ApiOption /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests /// The Id of the repository /// The pull request number - public IObservable GetAll(long repositoryId, int number) + public IObservable Get(long repositoryId, int number) { - return _connection.GetAndFlattenAllPages(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview); - } - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The Id of the repository - /// The pull request number - /// Options for changing the API response - public IObservable GetAll(long repositoryId, int number, ApiOptions options) - { - Ensure.ArgumentNotNull(options, nameof(options)); - - return _connection.GetAndFlattenAllPages(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options); + return _client.Get(repositoryId, number).ToObservable(); } /// @@ -137,4 +106,4 @@ public IObservable Delete(long repositoryId, int number, PullRequestReview return _client.Delete(repositoryId, number, users).ToObservable(); } } -} \ No newline at end of file +} diff --git a/Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs b/Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs index b139b3b145..4c20d2e90f 100644 --- a/Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs @@ -46,10 +46,11 @@ public async Task GetsNoRequestsWhenNoneExist() { var number = await CreateTheWorld(_github, _context, createReviewRequests: false); - var reviewRequests = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number); + var reviewRequests = await _client.Get(_context.RepositoryOwner, _context.RepositoryName, number); Assert.NotNull(reviewRequests); - Assert.Empty(reviewRequests); + Assert.Empty(reviewRequests.Users); + Assert.Empty(reviewRequests.Teams); } [IntegrationTest] @@ -57,10 +58,11 @@ public async Task GetsNoRequestsWhenNoneExistWithRepositoryId() { var number = await CreateTheWorld(_github, _context, createReviewRequests: false); - var reviewRequests = await _client.GetAll(_context.RepositoryId, number); + var reviewRequests = await _client.Get(_context.RepositoryId, number); Assert.NotNull(reviewRequests); - Assert.Empty(reviewRequests); + Assert.Empty(reviewRequests.Users); + Assert.Empty(reviewRequests.Teams); } [IntegrationTest] @@ -68,9 +70,9 @@ public async Task GetsRequests() { var number = await CreateTheWorld(_github, _context); - var reviewRequests = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number); + var reviewRequests = await _client.Get(_context.RepositoryOwner, _context.RepositoryName, number); - Assert.Equal(_collaboratorLogins, reviewRequests.Select(rr => rr.Login)); + Assert.Equal(_collaboratorLogins, reviewRequests.Users.Select(rr => rr.Login)); } [IntegrationTest] @@ -78,91 +80,9 @@ public async Task GetsRequestsWithRepositoryId() { var number = await CreateTheWorld(_github, _context); - var reviewRequests = await _client.GetAll(_context.RepositoryId, number); + var reviewRequests = await _client.Get(_context.RepositoryId, number); - Assert.Equal(_collaboratorLogins, reviewRequests.Select(rr => rr.Login)); - } - - [IntegrationTest] - public async Task ReturnsCorrectCountOfReviewRequestsWithStart() - { - var number = await CreateTheWorld(_github, _context); - - var options = new ApiOptions - { - PageSize = 1, - PageCount = 1, - StartPage = 2 - }; - var reviewRequests = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number, options); - - Assert.Equal(1, reviewRequests.Count); - } - - [IntegrationTest] - public async Task ReturnsCorrectCountOfReviewRequestsWithStartWithRepositoryId() - { - var number = await CreateTheWorld(_github, _context); - - var options = new ApiOptions - { - PageSize = 1, - PageCount = 1, - StartPage = 2 - }; - var reviewRequests = await _client.GetAll(_context.RepositoryId, number, options); - - Assert.Equal(1, reviewRequests.Count); - } - - [IntegrationTest] - public async Task ReturnsDistinctResultsBasedOnStartPage() - { - var number = await CreateTheWorld(_github, _context); - - var startOptions = new ApiOptions - { - PageSize = 1, - PageCount = 1 - }; - var firstPage = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number, startOptions); - - var skipStartOptions = new ApiOptions - { - PageSize = 1, - PageCount = 1, - StartPage = 2 - }; - var secondPage = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number, skipStartOptions); - - Assert.Equal(1, firstPage.Count); - Assert.Equal(1, secondPage.Count); - Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); - } - - [IntegrationTest] - public async Task ReturnsDistinctResultsBasedOnStartPageWithRepositoryId() - { - var number = await CreateTheWorld(_github, _context); - - var startOptions = new ApiOptions - { - PageSize = 1, - PageCount = 1 - }; - var firstPage = await _client.GetAll(_context.RepositoryId, number, startOptions); - - var skipStartOptions = new ApiOptions - { - PageSize = 1, - PageCount = 1, - StartPage = 2 - }; - var secondPage = await _client.GetAll(_context.RepositoryId, number, skipStartOptions); - - Assert.Equal(1, firstPage.Count); - Assert.Equal(1, secondPage.Count); - Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + Assert.Equal(_collaboratorLogins, reviewRequests.Users.Select(rr => rr.Login)); } } @@ -173,13 +93,13 @@ public async Task DeletesRequests() { var number = await CreateTheWorld(_github, _context); - var reviewRequestsBeforeDelete = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number); - var reviewRequestToCreate = new PullRequestReviewRequest(_collaboratorLogins); + var reviewRequestsBeforeDelete = await _client.Get(_context.RepositoryOwner, _context.RepositoryName, number); + var reviewRequestToCreate = PullRequestReviewRequest.ForReviewers(_collaboratorLogins); await _client.Delete(_context.RepositoryOwner, _context.RepositoryName, number, reviewRequestToCreate); - var reviewRequestsAfterDelete = await _client.GetAll(_context.RepositoryOwner, _context.RepositoryName, number); + var reviewRequestsAfterDelete = await _client.Get(_context.RepositoryOwner, _context.RepositoryName, number); - Assert.NotEmpty(reviewRequestsBeforeDelete); - Assert.Empty(reviewRequestsAfterDelete); + Assert.NotEmpty(reviewRequestsBeforeDelete.Users); + Assert.Empty(reviewRequestsAfterDelete.Users); } [IntegrationTest] @@ -187,13 +107,13 @@ public async Task DeletesRequestsWithRepositoryId() { var number = await CreateTheWorld(_github, _context); - var reviewRequestsBeforeDelete = await _client.GetAll(_context.RepositoryId, number); - var reviewRequestToCreate = new PullRequestReviewRequest(_collaboratorLogins); + var reviewRequestsBeforeDelete = await _client.Get(_context.RepositoryId, number); + var reviewRequestToCreate = PullRequestReviewRequest.ForReviewers(_collaboratorLogins); await _client.Delete(_context.RepositoryId, number, reviewRequestToCreate); - var reviewRequestsAfterDelete = await _client.GetAll(_context.RepositoryId, number); + var reviewRequestsAfterDelete = await _client.Get(_context.RepositoryId, number); - Assert.NotEmpty(reviewRequestsBeforeDelete); - Assert.Empty(reviewRequestsAfterDelete); + Assert.NotEmpty(reviewRequestsBeforeDelete.Users); + Assert.Empty(reviewRequestsAfterDelete.Users); } } @@ -203,7 +123,7 @@ public class TheCreateMethod : PullRequestReviewRequestClientTestsBase, IDisposa public async Task CreatesRequests() { var number = await CreateTheWorld(_github, _context, createReviewRequests: false); - var reviewRequestToCreate = new PullRequestReviewRequest(_collaboratorLogins); + var reviewRequestToCreate = PullRequestReviewRequest.ForReviewers(_collaboratorLogins); var pr = await _client.Create(_context.RepositoryOwner, _context.RepositoryName, number, reviewRequestToCreate); @@ -214,7 +134,7 @@ public async Task CreatesRequests() public async Task CreatesRequestsWithRepositoryId() { var number = await CreateTheWorld(_github, _context, createReviewRequests: false); - var reviewRequestToCreate = new PullRequestReviewRequest(_collaboratorLogins); + var reviewRequestToCreate = PullRequestReviewRequest.ForReviewers(_collaboratorLogins); var pr = await _client.Create(_context.RepositoryId, number, reviewRequestToCreate); @@ -250,7 +170,7 @@ static async Task CreateTheWorld(IGitHubClient github, RepositoryContext co // Create review requests (optional) if (createReviewRequests) { - var reviewRequest = new PullRequestReviewRequest(_collaboratorLogins); + var reviewRequest = PullRequestReviewRequest.ForReviewers(_collaboratorLogins); await github.PullRequest.ReviewRequest.Create(context.RepositoryOwner, context.RepositoryName, createdPullRequest.Number, reviewRequest); } @@ -293,4 +213,4 @@ static async Task CreateCommit(IGitHubClient github, RepositoryContext c var newCommit = new NewCommit(message, sha, parent); return await github.Git.Commit.Create(context.RepositoryOwner, context.RepositoryName, newCommit); } -} \ No newline at end of file +} diff --git a/Octokit.Tests/Clients/PullRequestReviewRequestsClientTests.cs b/Octokit.Tests/Clients/PullRequestReviewRequestsClientTests.cs index 5e0000c82c..a06c4438b7 100644 --- a/Octokit.Tests/Clients/PullRequestReviewRequestsClientTests.cs +++ b/Octokit.Tests/Clients/PullRequestReviewRequestsClientTests.cs @@ -25,12 +25,10 @@ public async Task RequestsCorrectUrl() var connection = Substitute.For(); var client = new PullRequestReviewRequestsClient(connection); - await client.GetAll("owner", "name", 7); + await client.Get("owner", "name", 7); - connection.Received().GetAll( - Arg.Is(u => u.ToString() == "repos/owner/name/pulls/7/requested_reviewers"), - null, - "application/vnd.github.black-cat-preview+json"); + connection.Received().Get( + Arg.Is(u => u.ToString() == "repos/owner/name/pulls/7/requested_reviewers")); } [Fact] @@ -39,56 +37,10 @@ public async Task RequestsCorrectUrlWithRepositoryId() var connection = Substitute.For(); var client = new PullRequestReviewRequestsClient(connection); - await client.GetAll(42, 7); + await client.Get(42, 7); - connection.Received().GetAll( - Arg.Is(u => u.ToString() == "repositories/42/pulls/7/requested_reviewers"), - null, - "application/vnd.github.black-cat-preview+json"); - } - - [Fact] - public async Task RequestsCorrectUrlWithApiOptions() - { - var connection = Substitute.For(); - var client = new PullRequestReviewRequestsClient(connection); - - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1, - PageSize = 1 - }; - - await client.GetAll("owner", "name", 7, options); - - connection.Received().GetAll( - Arg.Is(u => u.ToString() == "repos/owner/name/pulls/7/requested_reviewers"), - null, - "application/vnd.github.black-cat-preview+json", - options); - } - - [Fact] - public async Task RequestsCorrectUrlWithApiOptionsWithRepositoryId() - { - var connection = Substitute.For(); - var client = new PullRequestReviewRequestsClient(connection); - - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1, - PageSize = 1 - }; - - await client.GetAll(42, 7, options); - - connection.Received().GetAll( - Arg.Is(u => u.ToString() == "repositories/42/pulls/7/requested_reviewers"), - null, - "application/vnd.github.black-cat-preview+json", - options); + connection.Received().Get( + Arg.Is(u => u.ToString() == "repositories/42/pulls/7/requested_reviewers")); } [Fact] @@ -97,21 +49,11 @@ public async Task EnsuresNonNullArguments() var connection = Substitute.For(); var client = new PullRequestReviewRequestsClient(connection); - await Assert.ThrowsAsync(() => client.GetAll(null, "name", 1)); - await Assert.ThrowsAsync(() => client.GetAll("owner", null, 1)); - - await Assert.ThrowsAsync(() => client.GetAll(null, "name", 1, ApiOptions.None)); - await Assert.ThrowsAsync(() => client.GetAll("owner", null, 1, ApiOptions.None)); - await Assert.ThrowsAsync(() => client.GetAll("owner", "name", 1, null)); - - - await Assert.ThrowsAsync(() => client.GetAll("", "name", 1)); - await Assert.ThrowsAsync(() => client.GetAll("owner", "", 1)); - - await Assert.ThrowsAsync(() => client.GetAll("", "name", 1, ApiOptions.None)); - await Assert.ThrowsAsync(() => client.GetAll("owner", "", 1, ApiOptions.None)); + await Assert.ThrowsAsync(() => client.Get(null, "name", 1)); + await Assert.ThrowsAsync(() => client.Get("owner", null, 1)); - await Assert.ThrowsAsync(() => client.GetAll(42, 1, null)); + await Assert.ThrowsAsync(() => client.Get("", "name", 1)); + await Assert.ThrowsAsync(() => client.Get("owner", "", 1)); } } @@ -124,15 +66,13 @@ public void PostsToCorrectUrl() var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); client.Create("fakeOwner", "fakeRepoName", 13, pullRequestReviewRequest); - connection.Connection.Received().Post( + connection.Received().Post( Arg.Is(u => u.ToString() == "repos/fakeOwner/fakeRepoName/pulls/13/requested_reviewers"), - pullRequestReviewRequest, - "application/vnd.github.black-cat-preview+json", - null); + pullRequestReviewRequest); } [Fact] @@ -141,15 +81,13 @@ public void PostsToCorrectUrlWithRepositoryId() var connection = Substitute.For(); var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); client.Create(42, 13, pullRequestReviewRequest); - connection.Connection.Received().Post( + connection.Received().Post( Arg.Is(u => u.ToString() == "repositories/42/pulls/13/requested_reviewers"), - pullRequestReviewRequest, - "application/vnd.github.black-cat-preview+json", - null); + pullRequestReviewRequest); } [Fact] @@ -159,7 +97,7 @@ public async Task EnsuresNonNullArguments() var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await Assert.ThrowsAsync(() => client.Create(null, "fakeRepoName", 1, pullRequestReviewRequest)); await Assert.ThrowsAsync(() => client.Create("fakeOwner", null, 1, pullRequestReviewRequest)); @@ -180,14 +118,13 @@ public async Task PostsToCorrectUrl() var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await client.Delete("owner", "name", 13, pullRequestReviewRequest); connection.Received().Delete( Arg.Is(u => u.ToString() == "repos/owner/name/pulls/13/requested_reviewers"), - pullRequestReviewRequest, - "application/vnd.github.black-cat-preview+json"); + pullRequestReviewRequest); } [Fact] @@ -197,14 +134,13 @@ public async Task PostsToCorrectUrlWithRepositoryId() var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await client.Delete(43, 13, pullRequestReviewRequest); connection.Received().Delete( Arg.Is(u => u.ToString() == "repositories/43/pulls/13/requested_reviewers"), - pullRequestReviewRequest, - "application/vnd.github.black-cat-preview+json"); + pullRequestReviewRequest); } [Fact] @@ -214,7 +150,7 @@ public async Task EnsuresNonNullArguments() var client = new PullRequestReviewRequestsClient(connection); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await Assert.ThrowsAsync(() => client.Delete(null, "name", 1, pullRequestReviewRequest)); await Assert.ThrowsAsync(() => client.Delete("owner", null, 1, pullRequestReviewRequest)); diff --git a/Octokit.Tests/Reactive/ObservablePullRequestReviewRequestsClientTests.cs b/Octokit.Tests/Reactive/ObservablePullRequestReviewRequestsClientTests.cs index 7d98870154..9c3e9dd627 100644 --- a/Octokit.Tests/Reactive/ObservablePullRequestReviewRequestsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservablePullRequestReviewRequestsClientTests.cs @@ -29,9 +29,9 @@ public async Task RequestsCorrectUrl() var gitHubClient = Substitute.For(); var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); - client.GetAll("owner", "name", 7); + client.Get("owner", "name", 7); - gitHubClient.Received().PullRequest.ReviewRequest.GetAll("owner", "name", 7); + gitHubClient.Received().PullRequest.ReviewRequest.Get("owner", "name", 7); } [Fact] @@ -40,45 +40,9 @@ public async Task RequestsCorrectUrlWithRepositoryId() var gitHubClient = Substitute.For(); var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); - client.GetAll(42, 7); + client.Get(42, 7); - gitHubClient.Received().PullRequest.ReviewRequest.GetAll(42, 7); - } - - [Fact] - public async Task RequestsCorrectUrlWithApiOptions() - { - var gitHubClient = Substitute.For(); - var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); - - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1, - PageSize = 1 - }; - - client.GetAll("owner", "name", 7, options); - - gitHubClient.Received().PullRequest.ReviewRequest.GetAll("owner", "name", 7, options); - } - - [Fact] - public async Task RequestsCorrectUrlWithApiOptionsWithRepositoryId() - { - var gitHubClient = Substitute.For(); - var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); - - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1, - PageSize = 1 - }; - - client.GetAll(42, 7, options); - - gitHubClient.Received().PullRequest.ReviewRequest.GetAll(42, 7, options); + gitHubClient.Received().PullRequest.ReviewRequest.Get(42, 7); } [Fact] @@ -87,20 +51,12 @@ public async Task EnsuresNonNullArguments() var gitHubClient = Substitute.For(); var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); - Assert.Throws(() => client.GetAll(null, "name", 1)); - Assert.Throws(() => client.GetAll("owner", null, 1)); - - Assert.Throws(() => client.GetAll(null, "name", 1, ApiOptions.None)); - Assert.Throws(() => client.GetAll("owner", null, 1, ApiOptions.None)); - Assert.Throws(() => client.GetAll("owner", "name", 1, null)); - - Assert.Throws(() => client.GetAll("", "name", 1)); - Assert.Throws(() => client.GetAll("owner", "", 1)); + Assert.Throws(() => client.Get(null, "name", 1)); + Assert.Throws(() => client.Get("owner", null, 1)); - Assert.Throws(() => client.GetAll("", "name", 1, ApiOptions.None)); - Assert.Throws(() => client.GetAll("owner", "", 1, ApiOptions.None)); + Assert.Throws(() => client.Get("", "name", 1)); + Assert.Throws(() => client.Get("owner", "", 1)); - Assert.Throws(() => client.GetAll(42, 1, null)); } } @@ -113,7 +69,7 @@ public void PostsToCorrectUrl() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); client.Create("fakeOwner", "fakeRepoName", 13, pullRequestReviewRequest); @@ -127,7 +83,7 @@ public void PostsToCorrectUrlWithRepositoryId() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); client.Create(42, 13, pullRequestReviewRequest); @@ -141,7 +97,7 @@ public async Task EnsuresNonNullArguments() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); Assert.Throws(() => client.Create(null, "fakeRepoName", 1, pullRequestReviewRequest)); Assert.Throws(() => client.Create("fakeOwner", null, 1, pullRequestReviewRequest)); @@ -162,7 +118,7 @@ public async Task PostsToCorrectUrl() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await client.Delete("owner", "name", 13, pullRequestReviewRequest); @@ -176,7 +132,7 @@ public async Task PostsToCorrectUrlWithRepositoryId() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); await client.Delete(42, 13, pullRequestReviewRequest); @@ -190,7 +146,7 @@ public async Task EnsuresNonNullArguments() var client = new ObservablePullRequestReviewRequestsClient(gitHubClient); IReadOnlyList fakeReviewers = new List { "zxc", "asd" }; - var pullRequestReviewRequest = new PullRequestReviewRequest(fakeReviewers); + var pullRequestReviewRequest = PullRequestReviewRequest.ForReviewers(fakeReviewers); Assert.Throws(() => client.Delete(null, "name", 1, pullRequestReviewRequest)); Assert.Throws(() => client.Delete("owner", null, 1, pullRequestReviewRequest)); @@ -202,4 +158,4 @@ public async Task EnsuresNonNullArguments() } } } -} \ No newline at end of file +} diff --git a/Octokit/Clients/IPullRequestReviewRequestsClient.cs b/Octokit/Clients/IPullRequestReviewRequestsClient.cs index 55d7973fa3..9f2c68167a 100644 --- a/Octokit/Clients/IPullRequestReviewRequestsClient.cs +++ b/Octokit/Clients/IPullRequestReviewRequestsClient.cs @@ -1,5 +1,4 @@ -using System.Collections.Generic; -using System.Threading.Tasks; +using System.Threading.Tasks; namespace Octokit { @@ -18,25 +17,7 @@ public interface IPullRequestReviewRequestsClient /// The owner of the repository /// The name of the repository /// The pull request number - Task> GetAll(string owner, string name, int number); - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The owner of the repository - /// The name of the repository - /// The pull request number - /// Options for changing the API response - Task> GetAll(string owner, string name, int number, ApiOptions options); - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The Id of the repository - /// The pull request number - Task> GetAll(long repositoryId, int number); + Task Get(string owner, string name, int number); /// /// Gets review requests for a specified pull request. @@ -44,8 +25,7 @@ public interface IPullRequestReviewRequestsClient /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests /// The Id of the repository /// The pull request number - /// Options for changing the API response - Task> GetAll(long repositoryId, int number, ApiOptions options); + Task Get(long repositoryId, int number); /// /// Creates review requests on a pull request for specified users. @@ -85,4 +65,4 @@ public interface IPullRequestReviewRequestsClient /// List of logins of users that will be not longer requested for review Task Delete(long repositoryId, int number, PullRequestReviewRequest users); } -} \ No newline at end of file +} diff --git a/Octokit/Clients/PullRequestReviewRequestsClient.cs b/Octokit/Clients/PullRequestReviewRequestsClient.cs index 9fda5bddcc..e215bcf448 100644 --- a/Octokit/Clients/PullRequestReviewRequestsClient.cs +++ b/Octokit/Clients/PullRequestReviewRequestsClient.cs @@ -25,30 +25,12 @@ public PullRequestReviewRequestsClient(IApiConnection apiConnection) /// The name of the repository /// The pull request number [ManualRoute("GET", "/repos/{owner}/{name}/pulls/{number}/requested_reviewers")] - public Task> GetAll(string owner, string name, int number) + public Task Get(string owner, string name, int number) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); - return ApiConnection.GetAll(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview); - } - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The owner of the repository - /// The name of the repository - /// The pull request number - /// Options for changing the API response - [ManualRoute("GET", "/repos/{owner}/{name}/pulls/{number}/requested_reviewers")] - public Task> GetAll(string owner, string name, int number, ApiOptions options) - { - Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); - Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); - Ensure.ArgumentNotNull(options, nameof(options)); - - return ApiConnection.GetAll(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options); + return ApiConnection.Get(ApiUrls.PullRequestReviewRequests(owner, name, number)); } /// @@ -58,24 +40,9 @@ public Task> GetAll(string owner, string name, int number, A /// The Id of the repository /// The pull request number [ManualRoute("GET", "/repositories/{id}/pulls/{number}/requested_reviewers")] - public Task> GetAll(long repositoryId, int number) + public Task Get(long repositoryId, int number) { - return ApiConnection.GetAll(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview); - } - - /// - /// Gets review requests for a specified pull request. - /// - /// https://developer.github.com/v3/pulls/review_requests/#list-review-requests - /// The Id of the repository - /// The pull request number - /// Options for changing the API response - [ManualRoute("GET", "/repositories/{id}/pulls/{number}/requested_reviewers")] - public Task> GetAll(long repositoryId, int number, ApiOptions options) - { - Ensure.ArgumentNotNull(options, nameof(options)); - - return ApiConnection.GetAll(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options); + return ApiConnection.Get(ApiUrls.PullRequestReviewRequests(repositoryId, number)); } /// @@ -87,21 +54,14 @@ public Task> GetAll(long repositoryId, int number, ApiOption /// The Pull Request number /// List of logins of user will be requested for review [ManualRoute("POST", "/repos/{owner}/{name}/pulls/{number}/requested_reviewers")] - public async Task Create(string owner, string name, int number, PullRequestReviewRequest users) + public Task Create(string owner, string name, int number, PullRequestReviewRequest users) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); Ensure.ArgumentNotNull(users, nameof(users)); var endpoint = ApiUrls.PullRequestReviewRequests(owner, name, number); - var response = await ApiConnection.Connection.Post(endpoint, users, AcceptHeaders.PullRequestReviewsApiPreview, null).ConfigureAwait(false); - - if (response.HttpResponse.StatusCode != HttpStatusCode.Created) - { - throw new ApiException("Invalid Status Code returned. Expected a 201", response.HttpResponse.StatusCode); - } - - return response.Body; + return ApiConnection.Post(endpoint, users); } /// @@ -112,19 +72,12 @@ public async Task Create(string owner, string name, int number, Pul /// The Pull Request number /// List of logins of user will be requested for review [ManualRoute("POST", "/repositories/{id}/pulls/{number}/requested_reviewers")] - public async Task Create(long repositoryId, int number, PullRequestReviewRequest users) + public Task Create(long repositoryId, int number, PullRequestReviewRequest users) { Ensure.ArgumentNotNull(users, nameof(users)); var endpoint = ApiUrls.PullRequestReviewRequests(repositoryId, number); - var response = await ApiConnection.Connection.Post(endpoint, users, AcceptHeaders.PullRequestReviewsApiPreview, null).ConfigureAwait(false); - - if (response.HttpResponse.StatusCode != HttpStatusCode.Created) - { - throw new ApiException("Invalid Status Code returned. Expected a 201", response.HttpResponse.StatusCode); - } - - return response.Body; + return ApiConnection.Post(endpoint, users); } /// @@ -142,7 +95,7 @@ public Task Delete(string owner, string name, int number, PullRequestReviewReque Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); Ensure.ArgumentNotNull(users, nameof(users)); - return ApiConnection.Delete(ApiUrls.PullRequestReviewRequests(owner, name, number), users, AcceptHeaders.PullRequestReviewsApiPreview); + return ApiConnection.Delete(ApiUrls.PullRequestReviewRequests(owner, name, number), users); } /// @@ -157,7 +110,7 @@ public Task Delete(long repositoryId, int number, PullRequestReviewRequest users { Ensure.ArgumentNotNull(users, nameof(users)); - return ApiConnection.Delete(ApiUrls.PullRequestReviewRequests(repositoryId, number), users, AcceptHeaders.PullRequestReviewsApiPreview); + return ApiConnection.Delete(ApiUrls.PullRequestReviewRequests(repositoryId, number), users); } } } diff --git a/Octokit/Helpers/AcceptHeaders.cs b/Octokit/Helpers/AcceptHeaders.cs index 6e9f38bf95..d7ab7953f3 100644 --- a/Octokit/Helpers/AcceptHeaders.cs +++ b/Octokit/Helpers/AcceptHeaders.cs @@ -1,4 +1,5 @@ -using System.Diagnostics.CodeAnalysis; +using System; +using System.Diagnostics.CodeAnalysis; namespace Octokit { @@ -47,8 +48,6 @@ public static class AcceptHeaders public const string RepositoryTrafficApiPreview = "application/vnd.github.spiderman-preview"; - public const string PullRequestReviewsApiPreview = "application/vnd.github.black-cat-preview+json"; - public const string DraftPullRequestApiPreview = "application/vnd.github.shadow-cat-preview+json"; public const string ProjectsApiPreview = "application/vnd.github.inertia-preview+json"; diff --git a/Octokit/Models/Request/PullRequestReviewRequest.cs b/Octokit/Models/Request/PullRequestReviewRequest.cs index ae0ede36f3..601664e683 100644 --- a/Octokit/Models/Request/PullRequestReviewRequest.cs +++ b/Octokit/Models/Request/PullRequestReviewRequest.cs @@ -14,16 +14,28 @@ namespace Octokit [DebuggerDisplay("{DebuggerDisplay,nq}")] public class PullRequestReviewRequest { - public PullRequestReviewRequest(IReadOnlyList reviewers) + public PullRequestReviewRequest(IReadOnlyList reviewers, IReadOnlyList teamReviewers) { Reviewers = reviewers; + TeamReviewers = teamReviewers; } - public IReadOnlyList Reviewers { get; set; } + public IReadOnlyList Reviewers { get; private set; } + public IReadOnlyList TeamReviewers { get; private set; } internal string DebuggerDisplay { get { return string.Format(CultureInfo.InvariantCulture, "Reviewers: {0}", string.Join(", ", Reviewers)); } } + + public static PullRequestReviewRequest ForReviewers(IReadOnlyList reviewers) + { + return new PullRequestReviewRequest(reviewers, new List()); + } + + public static PullRequestReviewRequest ForTeamReviewers(IReadOnlyList teamReviewers) + { + return new PullRequestReviewRequest(new List(), teamReviewers); + } } -} \ No newline at end of file +} diff --git a/Octokit/Models/Response/RequestedReviews.cs b/Octokit/Models/Response/RequestedReviews.cs new file mode 100644 index 0000000000..cbceff3469 --- /dev/null +++ b/Octokit/Models/Response/RequestedReviews.cs @@ -0,0 +1,40 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Globalization; +using System.Linq; + +namespace Octokit +{ + /// + /// Users and teams requested to review a pull request + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class RequestedReviews + { + public RequestedReviews() + { + Users = new List(); + Teams = new List(); + } + + public RequestedReviews(IReadOnlyList users, IReadOnlyList teams) + { + Users = users; + Teams = teams; + } + public IReadOnlyList Users { get; protected set; } + public IReadOnlyList Teams { get; protected set; } + + internal string DebuggerDisplay + { + get + { + return string.Format( + CultureInfo.InvariantCulture, + "Users: {0}, Teams: {1}", + string.Join(", ", Users.Select(u => u.Login)), + string.Join(", ", Teams.Select(t => t.Slug))); + } + } + } +}