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

Feedback on GraphQL support #476

Closed
bkeepers opened this issue Mar 22, 2018 · 8 comments

Comments

@bkeepers
Copy link
Contributor

commented Mar 22, 2018

#472 introduced GraphQL support through context.github.query. It'll likely take a while to figure out patterns for efficiently building Probot apps with GraphQL. Your feedback is an important part of that. Feel free to comment here with ideas, friction, or suggestions on how we can make building Probot apps with GraphQL an amazing experience.

@bkeepers

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2018

One thing that is awkward is that the node id is needed for mutations in GraphQL, but webhooks don't include them…yet. This could change within the next week as this change to the REST API to include node_id in all payloads is about to be taken out of preview. Webhooks use the same serializers as the REST API, so payloads will include node_id once that ships.

If that doesn't look like it's going to ship in the next few days, I'd be interested in adding a helper that turns a webhook payload into a node_id. It probably would make most sense to define it on context. It'd look something like this:

async function node(resource) {
  // GraphQL query to get Node id for any resource, which is needed for mutations
  const getResource = `
    query getResource($url: URI!) {
      resource(url: $url) {
        ... on Node {
          id
        }
      }
    }
  `

  return this.github.query(getResource, {url: resource.html_url})
}

So then the hello world example would look something like this:

// GraphQL query to add a comment
const addComment = `
  mutation comment($id: ID!, $body: String!) {
    addComment(input: {subjectId: $id, body: $body}) {
      clientMutationId
    }
  }
`

module.exports = robot => {
  robot.on('issues.opened', async context => {
    // Get the node id of the issue
    const id = await context.node(context.payload.issue)
    const body = 'Hello World'

    // Post a comment on the issue
    await context.github.query(addComment, {id, body})
  })
}
@ocombe

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2018

2 ideas:

  • add an helper function to paginate graphql queries
  • add some "defaults" for graphQL queries (eg: issues, PRs, ...), because it's just annoying to always have to wrap my queries in repository > pullRequest (or issues), it is just boilerplate that I don't really want to add everytime. Example:
async queryPR<T>(context: Context, query: string, params: { [key: string]: any, owner: string, repo: string, number: number }): Promise<T> {
  return (await context.github.query(context, `query($owner: String!, $repo: String!, $number: Int!) {
    repository(owner: $owner, name: $repo) {
      pullRequest(number: $number) {
        ${query}
      }
    }
  }`, params)).repository.pullRequest;
}
@jeffrafter

This comment has been minimized.

Copy link

commented Apr 9, 2018

This pattern is working really well for me... except when it doesn't. I've noticed that some objects have "odd" html_url. One example is PullRequestReview:

https://github.com/<owner>/<repo>/pull/28#pullrequestreview-110303313

The fragment identifier is ignored when looking up the resource id, so you instead get the PullRequest global ID. I considered manually constructing the IDs (they are just Base64), but the version is volatile, I think.

Instead I fetched the PullRequestReview id by looking it up:

let issue = context.issue()
let reviewResponses = await getPullRequestReviews(context, issue.owner, issue.repo, issue.number)
let reviews = reviewResponses.repository.pullRequest.reviews.nodes

 // ...

let getPullRequestReviews = async (context, owner, name, number) => {
  const pullRequestReviewsQuery = `
    query getPullRequestReviews($owner: String!, $name: String!, $number: Int!) {
      repository(owner: $owner, name: $name) {
        pullRequest(number: $number) {
          reviews(first: 5) {
            nodes {
              id
              url
              viewerDidAuthor
            }
          }
        }
      }
    }`
  let response = await context.github.query(pullRequestReviewsQuery, {owner, name, number})
  return await response  
}

I only grab the first 5 as that is enough for me. You could subsequently loop through those to find the matching one.

GitHub
GitHub is where people build software. More than 27 million people use GitHub to discover, fork, and contribute to over 80 million projects.
@stale

This comment has been minimized.

Copy link

commented Jun 8, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 8, 2018

@bkeepers

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2018

It looks like there's not a lot of feedback on the GraphQL support. #552 simplified this a bit, and #484 documents the only other thing I think immediately needs improvement, so I'm going to close this out.

@bkeepers bkeepers closed this Jun 10, 2018

@ehabdevel

This comment has been minimized.

Copy link

commented Sep 22, 2018

Actually GraphQL is more easy for me as the Doc inside the Explorer helps a lot, but looks like that there is a lot of missing in GraphQL Docs or not explained, plus the "less examples",

for example how to ( createFile ) wish there is an example for this ...
and the confusion between (AddPullRequestReview) and (submitPullRequestReview)

then the tree became more complex with the (commitOID: GitObjectID) which is A Git object ID for (AddPullRequestReview)

Or (pullRequestReviewId: PullRequestReview!) for (submitPullRequestReview) which is going to Change on 2019-01-01

@maticzav

This comment has been minimized.

Copy link

commented Oct 30, 2018

Maybe you could add support for graphql-bindings, I genuinely enjoy using these in my other GraphQL projects. And there's already an existing one made especially for Github! 🎉

@Jake-Gillberg

This comment has been minimized.

Copy link

commented Nov 14, 2018

Ooo, I would love to use graphql-binding/graphql-binding-github in a probot app. @maticzav, have you done this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.