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.

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

Add ApiOptions overloads to methods on I(Observable)StarredClient #1336

Merged
merged 6 commits into from
Jun 2, 2016

Conversation

alexander-efremov
Copy link
Contributor

@alexander-efremov alexander-efremov commented Jun 2, 2016

Fixes #1163

To-do:

  • Add overloads on IStarredClient
  • Add overloads on IObservableStarredClient
  • Add unit tests for these new methods
  • Add integration tests for these new methods

/cc @shiftkey, @ryangribble

@@ -37,6 +37,15 @@ public static IObservable<T> GetAndFlattenAllPages<T>(this IConnection connectio
return GetPages(url, parameters, (pageUrl, pageParams) => connection.Get<List<T>>(pageUrl, pageParams, accepts).ToObservable());
}

public static IObservable<T> GetAndFlattenAllPages<T>(this IConnection connection, Uri url, IDictionary<string, string> parameters, string accepts, ApiOptions options)
{
return GetPagesWithOptions(url, parameters, options, (pageUrl, pageParams, o) =>
Copy link
Member

@shiftkey shiftkey Jun 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT: oops, I'd messed up the test and not specified PageCount. Amended below:

[IntegrationTest]
public async Task CanRetrievePagesOfPublicStarWatchers()
{
    var owner = "octokit";
    var name = "octokit.net";

    var firstPageOptions = new ApiOptions
    {
        PageSize = 5,
        PageCount = 1,
        StartPage = 1
    };

    var firstPage = await _fixture.GetAllStargazersWithTimestamps(owner, name, firstPageOptions);

    var secondPageOptions = new ApiOptions
    {
        PageSize = 5,
        PageCount = 1,
        StartPage = 2
    };

    var secondPage = await _fixture.GetAllStargazersWithTimestamps(owner, name, secondPageOptions);
    Assert.Equal(5, firstPage.Count);
    Assert.Equal(5, secondPage.Count);

    Assert.NotEqual(firstPage[0].User.Id, secondPage[0].User.Id);
    Assert.NotEqual(firstPage[1].User.Id, secondPage[1].User.Id);
    Assert.NotEqual(firstPage[2].User.Id, secondPage[2].User.Id);
    Assert.NotEqual(firstPage[3].User.Id, secondPage[3].User.Id);
    Assert.NotEqual(firstPage[4].User.Id, secondPage[4].User.Id);
}

Copy link
Contributor Author

@alexander-efremov alexander-efremov Jun 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shiftkey, sorry, I've forgotten push my commits with integration tests. There are a lot of new ones 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dampir good to hear ❤️

@shiftkey
Copy link
Member

shiftkey commented Jun 2, 2016

@dampir this looks good, add in that new integration test and this is good to 🚢

@alexander-efremov
Copy link
Contributor Author

@shiftkey please rerun Travis, it failed again 😢

@shiftkey shiftkey merged commit 85a87da into octokit:master Jun 2, 2016
@alexander-efremov alexander-efremov deleted the fix1180 branch June 2, 2016 10:17
@shiftkey shiftkey modified the milestone: Pagination Support Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants