Skip to content

Commit

Permalink
Fix IssueCommentClient #1500 (#1501)
Browse files Browse the repository at this point in the history 
* Make GetAllForRepo to accept optional args

* Fix tests

* add missing interfaces

* fix tests

* Fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* fix tests

* fix

* fix tests

* fix tests
  • Loading branch information
@pjc0247 @ryangribble
pjc0247 authored and ryangribble committed Jan 7, 2017
1 parent 49c2d52 commit cf0eddd
Show file tree
Hide file tree
Showing 14 changed files with 304 additions and 19 deletions.
36 changes: 36 additions & 0 deletions Octokit.Reactive/Clients/IObservableIssueCommentsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,42 @@ public interface IObservableIssueCommentsClient
/// <param name="options">Options for changing the API response</param>
IObservable<IssueComment> GetAllForRepository(long repositoryId, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
IObservable<IssueComment> GetAllForRepository(string owner, string name, IssueCommentRequest request);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
IObservable<IssueComment> GetAllForRepository(long repositoryId, IssueCommentRequest request);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
IObservable<IssueComment> GetAllForRepository(string owner, string name, IssueCommentRequest request, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
IObservable<IssueComment> GetAllForRepository(long repositoryId, IssueCommentRequest request, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a specified Issue.
/// </summary>
Expand Down
66 changes: 64 additions & 2 deletions Octokit.Reactive/Clients/ObservableIssueCommentsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public IObservable<IssueComment> GetAllForRepository(string owner, string name,
Ensure.ArgumentNotNullOrEmptyString(name, "name");
Ensure.ArgumentNotNull(options, "options");

return _connection.GetAndFlattenAllPages<IssueComment>(ApiUrls.IssueComments(owner, name), null, AcceptHeaders.ReactionsPreview, options);
return GetAllForRepository(owner, name, new IssueCommentRequest(), options);
}

/// <summary>
Expand All @@ -100,7 +100,69 @@ public IObservable<IssueComment> GetAllForRepository(long repositoryId, ApiOptio
{
Ensure.ArgumentNotNull(options, "options");

return _connection.GetAndFlattenAllPages<IssueComment>(ApiUrls.IssueComments(repositoryId), options);
return GetAllForRepository(repositoryId, new IssueCommentRequest(), options);
}

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
public IObservable<IssueComment> GetAllForRepository(string owner, string name, IssueCommentRequest request)
{
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");
Ensure.ArgumentNotNull(request, "request");

return GetAllForRepository(owner, name, request, ApiOptions.None);
}

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
public IObservable<IssueComment> GetAllForRepository(long repositoryId, IssueCommentRequest request)
{
Ensure.ArgumentNotNull(request, "request");

return GetAllForRepository(repositoryId, request, ApiOptions.None);
}

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
public IObservable<IssueComment> GetAllForRepository(string owner, string name, IssueCommentRequest request, ApiOptions options)
{
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");
Ensure.ArgumentNotNull(request, "request");
Ensure.ArgumentNotNull(options, "options");

return _connection.GetAndFlattenAllPages<IssueComment>(ApiUrls.IssueComments(owner, name), request.ToParametersDictionary(), AcceptHeaders.ReactionsPreview, options);
}

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
public IObservable<IssueComment> GetAllForRepository(long repositoryId, IssueCommentRequest request, ApiOptions options)
{
Ensure.ArgumentNotNull(request, "request");
Ensure.ArgumentNotNull(options, "options");

return _connection.GetAndFlattenAllPages<IssueComment>(ApiUrls.IssueComments(repositoryId), request.ToParametersDictionary(), AcceptHeaders.ReactionsPreview, options);
}

/// <summary>
Expand Down
6 changes: 4 additions & 2 deletions Octokit.Tests/Clients/IssueCommentsClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ public async Task EnsuresNonNullArguments()
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository("owner", null));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository(null, "name", ApiOptions.None));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository("owner", null, ApiOptions.None));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", null));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", options: null));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", null, ApiOptions.None));

await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository(1, null));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository(1, options: null));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAllForRepository(1, null, ApiOptions.None));

await Assert.ThrowsAsync<ArgumentException>(() => client.GetAllForRepository("", "name"));
await Assert.ThrowsAsync<ArgumentException>(() => client.GetAllForRepository("owner", ""));
Expand Down
44 changes: 34 additions & 10 deletions Octokit.Tests/Reactive/ObservableIssueCommentsClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ public void RequestsCorrectUrl()
client.GetAllForRepository("fake", "repo");

gitHubClient.Connection.Received(1).Get<List<IssueComment>>(
new Uri("repos/fake/repo/issues/comments", UriKind.Relative),
Args.EmptyDictionary,
new Uri("repos/fake/repo/issues/comments", UriKind.Relative),
Arg.Any<IDictionary<string, string>>(),
"application/vnd.github.squirrel-girl-preview");
}

Expand All @@ -71,7 +71,9 @@ public void RequestsCorrectUrlWithRepositoryId()
client.GetAllForRepository(1);

gitHubClient.Connection.Received(1).Get<List<IssueComment>>(
new Uri("repositories/1/issues/comments", UriKind.Relative), Args.EmptyDictionary, null);
new Uri("repositories/1/issues/comments", UriKind.Relative),
Arg.Any<IDictionary<string, string>>(),
"application/vnd.github.squirrel-girl-preview");
}

[Fact]
Expand All @@ -80,18 +82,27 @@ public void RequestsCorrectUrlWithApiOptions()
var gitHubClient = Substitute.For<IGitHubClient>();
var client = new ObservableIssueCommentsClient(gitHubClient);

var request = new IssueCommentRequest()
{
Direction = SortDirection.Descending,
Since = new DateTimeOffset(2016, 11, 23, 11, 11, 11, 00, new TimeSpan()),
Sort = PullRequestReviewCommentSort.Updated
};
var options = new ApiOptions
{
StartPage = 1,
PageSize = 1,
PageCount = 1
};

client.GetAllForRepository("fake", "repo", options);
client.GetAllForRepository("fake", "repo", request, options);

gitHubClient.Connection.Received(1).Get<List<IssueComment>>(
new Uri("repos/fake/repo/issues/comments", UriKind.Relative),
Arg.Any<Dictionary<string, string>>(),
new Uri("repos/fake/repo/issues/comments", UriKind.Relative),
Arg.Is<Dictionary<string, string>>(d => d.Count == 5
&& d["direction"] == "desc"
&& d["since"] == "2016-11-23T11:11:11Z"
&& d["sort"] == "updated"),
"application/vnd.github.squirrel-girl-preview");
}

Expand All @@ -101,17 +112,28 @@ public void RequestsCorrectUrlWithRepositoryIdWithApiOptions()
var gitHubClient = Substitute.For<IGitHubClient>();
var client = new ObservableIssueCommentsClient(gitHubClient);

var request = new IssueCommentRequest()
{
Direction = SortDirection.Descending,
Since = new DateTimeOffset(2016, 11, 23, 11, 11, 11, 00, new TimeSpan()),
Sort = PullRequestReviewCommentSort.Updated
};
var options = new ApiOptions
{
StartPage = 1,
PageSize = 1,
PageCount = 1
};

client.GetAllForRepository(1, options);
client.GetAllForRepository(1, request, options);

gitHubClient.Connection.Received(1).Get<List<IssueComment>>(
new Uri("repositories/1/issues/comments", UriKind.Relative), Arg.Is<IDictionary<string, string>>(d => d.Count == 2), null);
new Uri("repositories/1/issues/comments", UriKind.Relative),
Arg.Is<Dictionary<string, string>>(d => d.Count == 5
&& d["direction"] == "desc"
&& d["since"] == "2016-11-23T11:11:11Z"
&& d["sort"] == "updated"),
"application/vnd.github.squirrel-girl-preview");
}

[Fact]
Expand All @@ -124,9 +146,11 @@ public async Task EnsuresNonNullArguments()
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository("owner", null));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository(null, "name", ApiOptions.None));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository("owner", null, ApiOptions.None));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", null));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", request: null));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository("owner", "name", options: null));

Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository(1, null));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository(1, request: null));
Assert.Throws<ArgumentNullException>(() => client.GetAllForRepository(1, options: null));

Assert.Throws<ArgumentException>(() => client.GetAllForRepository("", "name"));
Assert.Throws<ArgumentException>(() => client.GetAllForRepository("owner", ""));
Expand Down
36 changes: 36 additions & 0 deletions Octokit/Clients/IIssueCommentsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,42 @@ public interface IIssueCommentsClient
/// <param name="options">Options for changing the API response</param>
Task<IReadOnlyList<IssueComment>> GetAllForRepository(long repositoryId, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
Task<IReadOnlyList<IssueComment>> GetAllForRepository(string owner, string name, IssueCommentRequest request);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
Task<IReadOnlyList<IssueComment>> GetAllForRepository(long repositoryId, IssueCommentRequest request);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
Task<IReadOnlyList<IssueComment>> GetAllForRepository(string owner, string name, IssueCommentRequest request, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a repository.
/// </summary>
/// <remarks>http://developer.github.com/v3/issues/comments/#list-comments-in-a-repository</remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">The sorting <see cref="IssueCommentRequest">parameters</see></param>
/// <param name="options">Options for changing the API response</param>
Task<IReadOnlyList<IssueComment>> GetAllForRepository(long repositoryId, IssueCommentRequest request, ApiOptions options);

/// <summary>
/// Gets Issue Comments for a specified Issue.
/// </summary>
Expand Down
Loading

0 comments on commit cf0eddd

Please sign in to comment.