octokit.graphql.net icon indicating copy to clipboard operation
octokit.graphql.net copied to clipboard

Consider AutoPaging Optionally Returning Edges

Open jamisonhyatt opened this issue 5 years ago • 3 comments

A lot of times there is important information on the Edge (E.g., LanguageEdge has size in bytes, CollaboratorEdge has RepositoryPermisson) and the implementation of autopaging only returns the underlying node. Unless I'm missing something, this means anytime you need information from the edge you are required to fall back to manual paging.

It looks like AllPages could be modified to optionally return an IQueryableList of Edges rather than Nodes. Is that modification something that you would be in favor of? Was this considered and discarded?

jamisonhyatt avatar Jun 16 '19 19:06 jamisonhyatt

Totally agree! It seems GraphQL has much better performance (compared to Octokit) but things like this make it pretty hard to migrate 😢

terrajobst avatar Nov 19 '19 06:11 terrajobst

I just hit this scenario too, also on the RepositoryCollaboratorEdge scenario. I was trying find the collaborator edge associated with a pull request's author. Here was my workaround:

var collaboratorQuery = new Query()
    .Repository(Var("repo"), Var("owner"))
    .Collaborators(query: Var("author"))
    .Select(c => new
    {
        c.PageInfo.HasNextPage,
        c.PageInfo.EndCursor,
        Users = c.Edges.Select(e => new
        {
            e.Node.Login,
            e.Permission
        }).ToList()
    })
    .Compile();

while (true)
{
    string? afterCursor = null;

    var collaborators = await connection.Run(collaboratorQuery, new Dictionary<string, object?>
    {
        { "owner", owner },
        { "repo", repo },
        { "author", data.PullRequest.Author },
        { "after_cursor", afterCursor }
    });

    var author = collaborators.Users.SingleOrDefault(user => string.Equals(user.Login, data.PullRequest.Author, StringComparison.InvariantCultureIgnoreCase));

    if (author is not null)
    {
        Console.WriteLine($"Collaborator:");
        Console.WriteLine($"  Login:      {author.Login}");
        Console.WriteLine($"  Permission: {author.Permission}");
        break;
    }

    if (collaborators.HasNextPage)
    {
        afterCursor = collaborators.EndCursor;
    }
    else
    {
        Console.WriteLine($"Author '{data.PullRequest.Author}' is not a collaborator in this repository.");
        break;
    }
}

jeffhandley avatar Aug 04 '22 05:08 jeffhandley

I'm also comping up against this. Would be keen to pick this up if that's OK (I see it's 'up for grabs')?

In my head the solution I was going to suggest was to add an optional argument to AllPages to specify whether to paginate over edges or nodes (.AllPages(paginationTarget: PaginationTarget.Edges)?) and then in the QueryBuilder.CreateNodeQuery (when the AllPages expression gets converted into a .Nodes().Cast<>().... expression tree - https://github.com/octokit/octokit.graphql.net/blob/main/Octokit.GraphQL.Core/Core/Builders/QueryBuilder.cs#L1069) to examine that argument and emit an Edges() call instead.

jMarkP avatar Jul 11 '23 10:07 jMarkP