Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graduate review requests to what eventually shipped #2153

Merged
merged 2 commits into from
Mar 15, 2020
+140 −394
Merged

Graduate review requests to what eventually shipped #2153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f08d4a4
rewrite Review Requests API to match what has shipped
shiftkey Mar 15, 2020
c229261
drop unused preview API
shiftkey Mar 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 3 additions & 22 deletions Octokit.Reactive/Clients/IObservablePullRequestReviewRequestsClient.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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