Skip to content

Commit

Permalink
Graduate review requests to what eventually shipped (#2153)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shiftkey
shiftkey authored Mar 15, 2020
1 parent 3918685 commit f6a9a47
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 394 deletions.
25 changes: 3 additions & 22 deletions Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,15 @@ public interface IObservablePullRequestReviewRequestsClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The pull request number</param>
IObservable<User> GetAll(string owner, string name, int number);

/// <summary>
/// Gets review requests for a specified pull request.
/// </summary>
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The pull request number</param>
/// <param name="options">Options for changing the API response</param>
IObservable<User> GetAll(string owner, string name, int number, ApiOptions options);

/// <summary>
/// Gets review requests for a specified pull request.
/// </summary>
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The pull request number</param>
IObservable<User> GetAll(long repositoryId, int number);
IObservable<RequestedReviews> Get(string owner, string name, int number);

/// <summary>
/// Gets review requests for a specified pull request.
/// </summary>
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The pull request number</param>
/// <param name="options">Options for changing the API response</param>
IObservable<User> GetAll(long repositoryId, int number, ApiOptions options);
IObservable<RequestedReviews> Get(long repositoryId, int number);

/// <summary>
/// Creates review requests on a pull request for specified users.
Expand Down Expand Up @@ -85,4 +66,4 @@ public interface IObservablePullRequestReviewRequestsClient
/// <param name="users">List of logins of users that will be not longer requested for review</param>
IObservable<Unit> Delete(long repositoryId, int number, PullRequestReviewRequest users);
}
}
}
41 changes: 5 additions & 36 deletions Octokit.Reactive/Clients/ObservablePullRequestReviewRequestsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,12 @@ public ObservablePullRequestReviewRequestsClient(IGitHubClient client)
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The pull request number</param>
public IObservable<User> GetAll(string owner, string name, int number)
public IObservable<RequestedReviews> Get(string owner, string name, int number)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));

return _connection.GetAndFlattenAllPages<User>(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview);
}

/// <summary>
/// Gets review requests for a specified pull request.
/// </summary>
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="number">The pull request number</param>
/// <param name="options">Options for changing the API response</param>
public IObservable<User> 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<User>(ApiUrls.PullRequestReviewRequests(owner, name, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options);
return _client.Get(owner, name, number).ToObservable();
}

/// <summary>
Expand All @@ -56,23 +39,9 @@ public IObservable<User> GetAll(string owner, string name, int number, ApiOption
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The pull request number</param>
public IObservable<User> GetAll(long repositoryId, int number)
public IObservable<RequestedReviews> Get(long repositoryId, int number)
{
return _connection.GetAndFlattenAllPages<User>(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview);
}

/// <summary>
/// Gets review requests for a specified pull request.
/// </summary>
/// <remarks>https://developer.github.com/v3/pulls/review_requests/#list-review-requests</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="number">The pull request number</param>
/// <param name="options">Options for changing the API response</param>
public IObservable<User> GetAll(long repositoryId, int number, ApiOptions options)
{
Ensure.ArgumentNotNull(options, nameof(options));

return _connection.GetAndFlattenAllPages<User>(ApiUrls.PullRequestReviewRequests(repositoryId, number), null, AcceptHeaders.PullRequestReviewsApiPreview, options);
return _client.Get(repositoryId, number).ToObservable();
}

/// <summary>
Expand Down Expand Up @@ -137,4 +106,4 @@ public IObservable<Unit> Delete(long repositoryId, int number, PullRequestReview
return _client.Delete(repositoryId, number, users).ToObservable();
}
}
}
}
128 changes: 24 additions & 104 deletions Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,123 +46,43 @@ 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]
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]
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]
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));
}
}

Expand All @@ -173,27 +93,27 @@ 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]
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);
}
}

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -250,7 +170,7 @@ static async Task<int> 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);
}

Expand Down Expand Up @@ -293,4 +213,4 @@ static async Task<Commit> CreateCommit(IGitHubClient github, RepositoryContext c
var newCommit = new NewCommit(message, sha, parent);
return await github.Git.Commit.Create(context.RepositoryOwner, context.RepositoryName, newCommit);
}
}
}
Loading

0 comments on commit f6a9a47

Please sign in to comment.