Hey @nporteschaikin!

I am glad you are finding the v4 API wonderful - I think you’ve found a less-than-wonderful gap in the information that’s available. Looking at it today, I think the way to get this information would be to do a lot of work using the reviews connection with a query like this:

query ($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest: issueOrPullRequest(number: $number) {
      ... on PullRequest {
        reviews(last: 100) {
          totalCount
          nodes {
            id
            author {
              login
            }
            url
            comments(first: 100) {
              nodes {
                id
                replyTo {
                  id
                }
                body
                author {
                  login
                }
              }
            }
          }
        }
      }
    }
  }
} 

If you look at the id of the `eplyTo field on the comments, you should be able to take a look at what comments and reviews are “top level”, and what comments are attributed to them.

Unfortunately, this is not a great UX! As you’ve discovered in those earlier issues, this has been an issue for quite some time (sorry @rnystrom and @yakov116, @hsborges and everyone else who we let down this time and others).

I’ve got some GraphQL Schema in development that would allow someone to query something like this:

query ($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest: issueOrPullRequest(number: $number) {
      __typename
      ... on PullRequest {
        reviewThreads(first: 100) {
          edges {
            node {
              __typename
              comments(first: 10) {
                nodes {
                  pullRequestReview {
                    id
                  }
                  body
                }
              }
            }
          }
        }
      }
    }
  }
}

Which, for the infamous issue GitHawkApp/GitHawk#555 as mentioned in other threads, would return something like this:

{
  "data": {
    "repository": {
      "pullRequest": {
        "__typename": "PullRequest",
        "reviewThreads": {
          "edges": [
            {
              "node": {
                "__typename": "PullRequestReviewThread",
                "comments": {
                  "nodes": [
                    {
                      "pullRequestReview": {
                        "id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NjkyNTI5NzE="
                      },
                      "body": "Can we remove the double space, and if it fails it should default to just returning `fileName`"
                    },
                    {
                      "pullRequestReview": {
                        "id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NjkyNTkxMTk="
                      },
                      "body": "I have tidied up the code and removed the `potentialLanguage`. I was planning to use that for the 'you know what' PR as Highlight.js is not correctly automatically detected the Swift language 😅\r\n\r\nAt GitHub they are using their own Ruby library [github/linguist](https://github.com/github/linguist) can't ask the Github API to send the language?"
                    },
                    {
                      "pullRequestReview": {
                        "id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NjkzOTkwNTE="
                      },
                      "body": "Leaving this reply as a test for PR review threads, demonstrating how broken they are 😞 "
                    }
                  ]
                }
              }
            },
            {
              "node": {
                "__typename": "PullRequestReviewThread",
                "comments": {
                  "nodes": [
                    {
                      "pullRequestReview": {
                        "id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NjkyNTMwMTc="
                      },
                      "body": "Probably want to extract a list of known languages to an enum + separate file? And maybe we can already find a list like that somewhere?\r\n\r\nThen we can `return Language(rawValue: fileExtension)` (or something similar, eg. when there are multiple extensions for one language) instead of this weird `switch`. :D"
                    }
                  ]
                }
              }
            },
            {
              "node": {
                "__typename": "PullRequestReviewThread",
                "comments": {
                  "nodes": [
                    {
                      "pullRequestReview": {
                        "id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NjkyNTI5NzE="
                      },
                      "body": "Not sure this is necessary, at least yet? ~Also it shouldn't **only** support Swift, even if we do keep it~\r\n\r\nEdit// See BasThomas comment"
                    }
                  ]
                }
              }
            }
          ]
        }
      }
    }
  }
}

 How does that sound? I’ll keep this thread up to date as we look to release this.

Separately, to people I’ve mentioned in this issue (including you @nporteschaikin): I am sure you’ve got more schema suggestions, frustrations, or anything of the sort. If you’re interested in chatting about them with me, I’d love to sit down over a video chat and go through what’s got you down with our public GraphQL API.

@k0shk0sh do you have any input for us?

Hi @nickvanw,

Thanks for getting back to me and for your detailed response! We’re heavy users of your GraphQL API at Code Climate and it’s been mostly a pleasant experience :slight_smile:

@nickvanw wrote:

If you look at the id of the `eplyTo field on the comments, you should be able to take a look at what comments and reviews are “top level”, and what comments are attributed to them.

In my experience, this wouldn’t solve the issue we’re running into. It looks like the response we receive from GraphQL (mostly) does not include replies to comments - rather, we’re receiving all top-level comments, which we can’t connect to their replies. Please correct me if I’m misunderstanding something! A replies connection would be another (inelegant) option, but so would the reviewThreads connection you proposed below. 

For now, we’re using the v3 API, although - as you could imagine - it’s significantly more challenging to extract this data at scale via the traditional REST API. I look forward to seeing the reviewThreads connection released!

As for additional feedback on the GraphQL API, I’d love to hop on a call and chat! As alluded to above, we’re extracting a lot of data, regularly, from the v4 API and I think we’ve collected a lot of wisdom, best practices, and feedback for GitHub. I’d love to shoot you an email - let me know how to reach you privately!

Hey @nporteschaikin,

I’m happy to hear that! Perhaps we may be speaking about different things - looking at the query I posted earlier that works now, I believe one could re-create the reviewThreads connection with the data that’s returned using that query. By using the id returned in replyTo, you should be able to find the parent comment ID that the reply was to, and build the discussion that way. 

Taking the output of that query and plugging it into this hastily written python:

import json
import collections
# d = Raw GraphQL query output
resp = json.loads(d, strict=False)
reviews = resp['data']['repository']['pullRequest']['reviews']['nodes']
toplevel, replies = {}, collections.defaultdict(list)
for r in reviews:

for c in r['comments']['nodes']:

if c['replyTo']:

replies[c['replyTo']['id']].append(c)

else:

toplevel[c['id']] = c
for c in toplevel:

print('Thread:\n')

print(toplevel[c]['body'] + '\n')

for r in replies[c]:

print(' Reply: %s\n' % r['body'])

Using the Issue I used earlier, GitHawkApp/GitHawk#555, I get the following:

Thread:
Probably want to extract a list of known languages to an enum + separate file? And maybe we can already find a list like that somewhere?
Then we can return Language(rawValue: fileExtension) (or something similar, eg. when there are multiple extensions for one language) instead of this weird switch. :D
Thread:
Can we remove the double space, and if it fails it should default to just returning fileName
Reply: I have tidied up the code and removed the potentialLanguage. I was planning to use that for the 'you know what' PR as Highlight.js is not correctly automatically detected the Swift language 😅
At GitHub they are using their own Ruby library github/linguist can't ask the Github API to send the language?
Reply: Leaving this reply as a test for PR review threads, demonstrating how broken they are 😞
Thread:
Not sure this is necessary, at least yet? Also it shouldn't only support Swift, even if we do keep it
Edit// See BasThomas comment

As a rough cut, this seems to me to be the equivalent of the reviewThreads connection I talked about releasing earlier - three total review request threads, one of which having three comments and the other only having the top-level comment. I haven’t tested it exhaustively, though I think the approach would still work.

If I’ve mischaracterized anything, please let me know! I’m working on getting the reviewThreads through code review now, but I want to make sure it will solve your needs before releasing it! 

Separately, please reach out to partnerengineering@github.com and mention this thread - we’ll be expecting you, and we can set up a chat! 

@nickvanw:

Hmm. The problem for us is that the replies are not being returned, so the replyTo field isn’t helpful. Take this thread for example:

• Review comment A, by me

• Review comment B, by someone else

• Review comment C, by me

The only comment being returned by the GraphQL API is comment A, the “parent” comment.

@nporteschaikin: 

Do you have an example of a public PR where you’re experiencing this? 

The top level reviews connection will only return the top level comments, but the comments connection inside of that seems to give me all of the information I expect. Specifically, I want to make sure you’re trying the reviews query that I linked to in my response, and not any of the examples that you gave in your original post. 

I’d love to find an example where I can reproduce what you’re talking about, but I’ve been going around to a few PRs and it seems to be returning what I’d expect. 

Thanks for getting back to me so quickly, @nickvanw. I’m experiencing this exclusively with private repositories, so I’m going to dig into the payload further and see if I’m missing something. Will report back.

Hi @nickvanw! I’m going to take this for a spin. Are these objects also available via the PullRequestTimelineConnection connection?

At present moment, you can only fetch these threads by looking at this particular connection. I believe re-creating the timeline exactly as it looks on GitHub will take some munging and additional work – if something looks amiss, please let me know. 

If any other issues persist, or something doesn’t seem right, please don’t hesitate to reach out!

Is there any reason why it’s

PullRequest {
    reviewThreads
}

and not

PullRequest {
    reviews {
        nodes {
            threads
        }
    }
}

?

Also it would be great to have review ID in the thread itself.

Hi! I don’t think reviewThreads serves the purpose properly when someone is trying to build a timeline from timelineItems until the review comment objects returned under it also have an option to include the node ID of the reviewThreads node it is a part of.

Currently, you have to paginate through all the data under reviewThreads and compare the top level comment ID of the review comment given in timelineItems through the results to be able to consistently fetch the replies to that comment. That is honestly, not really the best method, UX wise, and you end up having to wait longer for the replies to a review comment in the timeline to load the further you go into the timeline as you have to paginate through more data of reviewThreads until you find the comment.

You have to use hacky approaches like in the following:

  <a href="https://github.com/NamanShergill/diohub/blob/69b13ab65f5ffef13e448d4f3241ae997a3bfdc9/lib/services/pulls/pulls_service.dart#L149" target="_blank" rel="noopener nofollow ugc">github.com</a>

NamanShergill/diohub/blob/69b13ab65f5ffef13e448d4f3241ae997a3bfdc9/lib/services/pulls/pulls_service.dart#L149

    
      

          
      name: name,
          
    ),
          
  ),
          
  cacheOptions: CacheManager.defaultGQLCache(refresh: refresh),
          
);
          
final parsed = ReviewThreadFirstCommentQuery$Query.fromJson(res.data!);
          

          
if (parsed.repository!.pullRequest!.reviewThreads.edges!.isNotEmpty) {
          
  for (final thread
          
      in parsed.repository!.pullRequest!.reviewThreads.edges!) {
          
    if (thread?.node?.comments.nodes?.first?.id == commentID) {
          
      return thread!;
          
    }
          
  }
          
  return getPRReviewThreadID(commentID,
          
      name: name,
          
      owner: owner,
          
      number: number,
          
      cursor:
          
          parsed.repository!.pullRequest!.reviewThreads.edges!.last!.cursor,
          
      refresh: refresh);