From 2acf23acb70eb009798960bd9162601aa45df247 Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Wed, 6 Jun 2018 23:06:00 -0700 Subject: [PATCH] github: switch creating comments from GraphQL to REST API for snarfed/bridgy#824 --- README.md | 3 ++- granary/github.py | 18 ++++++++++++------ granary/test/test_github.py | 31 ++++++++++++++----------------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 2b46f0f5..1068aa06 100644 --- a/README.md +++ b/README.md @@ -308,13 +308,14 @@ Changelog ### 1.13 - unreleased * Twitter: * Support ISO 8601 formatted created_at timestamps, which the [archive download uses](https://help.twitter.com/en/managing-your-account/how-to-download-your-twitter-archive), as well as RFC 2822 from the API. - * `create()` and `preview_create()`: support RSVPs. Tweet them as normal tweets with the RSVP content. ([#](818)) + * `create()` and `preview_create()`: support RSVPs. Tweet them as normal tweets with the RSVP content. ([#818](https://github.com/snarfed/bridgy/issues/818)) * Instagram: * Add global rate limiting lock for scraping. If a scraping HTTP request gets a 429 or 503 response, we refuse to make more requests for 5m, and instead short circuit and return the same error. This can be overridden with a new `ignore_rate_limit` kwarg to `get_activities()`. * GitHub: * Escape HTML characters (`<`, `>`, and `&`) in content in `create()` and `preview_create()` ([snarfed/bridgy#810](https://github.com/snarfed/bridgy/issues/810)). * `get_activities()` and `get_comment()` now return `ValueError` instead of `AssertionError` on malformed `activity_id` and `comment_id` args, respectively. * `get_activities()` bug fix for issues/PRs with no body text. + * Switch from GraphQL to REST API for creating comments and reactions, since GraphQL hits authorization errors on many org repos. ([snarfed/bridgy#824](https://github.com/snarfed/bridgy/issues/824)) * Atom: * Shorten and ellipsize feed title when necessary ([#144](https://github.com/snarfed/granary/issues/144)). diff --git a/granary/github.py b/granary/github.py index 83385e13..eb26b12a 100644 --- a/granary/github.py +++ b/granary/github.py @@ -3,6 +3,7 @@ API docs: https://developer.github.com/v4/ +https://developer.github.com/v3/ https://developer.github.com/apps/building-oauth-apps/authorization-options-for-oauth-apps/#web-application-flow """ from __future__ import absolute_import @@ -26,7 +27,6 @@ REST_API_BASE = 'https://api.github.com' REST_API_ISSUE = REST_API_BASE + '/repos/%s/%s/issues/%s' REST_API_CREATE_ISSUE = REST_API_BASE + '/repos/%s/%s/issues' -# currently unused; we use 'comments_url' in the issue or PR instead REST_API_COMMENTS = REST_API_BASE + '/repos/%s/%s/issues/%s/comments' REST_API_REACTIONS = REST_API_BASE + '/repos/%s/%s/issues/%s/reactions' REST_API_COMMENT = REST_API_BASE + '/repos/%s/%s/issues/comments/%s' @@ -614,11 +614,17 @@ def _create(self, obj, preview=None, include_link=source.OMIT_LINK, else: try: - resp = self.graphql(GRAPHQL_ADD_COMMENT, { - 'subject_id': issue['id'], - 'body': content, - }) - return source.creation_result(resp['addComment']['commentEdge']['node']) + # we originally used the GraphQL API here, but it often gets + # rejected against org repos due to access controls. oddly, the REST + # API works fine in those same cases. + # https://github.com/snarfed/bridgy/issues/824 + api_url = REST_API_COMMENTS % (owner, repo, number) + commented = self.rest(api_url, data={'body': content}).json() + return source.creation_result({ + 'id': commented.get('id'), + 'url': commented.get('html_url'), + 'type': 'comment', + }) except ValueError as e: return source.creation_result(abort=True, error_plain=str(e)) diff --git a/granary/test/test_github.py b/granary/test/test_github.py index 2855b635..fb9136fc 100644 --- a/granary/test/test_github.py +++ b/granary/test/test_github.py @@ -3,6 +3,7 @@ """ import copy import json +from unittest import skip from mox3 import mox from oauth_dropins.webutil import testutil @@ -631,26 +632,20 @@ def test_reaction_to_object_rest(self): def test_create_comment(self): self.expect_graphql_issue() - self.expect_graphql(json={ - 'query': github.GRAPHQL_ADD_COMMENT % { - 'subject_id': ISSUE_GRAPHQL['id'], + self.expect_requests_post( + REST_API_COMMENTS % ('foo', 'bar', 123), headers=EXPECTED_HEADERS, + json={ 'body': 'i have something to say here', - }, - }, response={ - 'addComment': { - 'commentEdge': { - 'node': { - 'id': '456', - 'url': 'https://github.com/foo/bar/pull/123#issuecomment-456', - }, - }, - }, - }) + }, response={ + 'id': 456, + 'html_url': 'https://github.com/foo/bar/pull/123#issuecomment-456', + }) self.mox.ReplayAll() result = self.gh.create(COMMENT_OBJ) self.assert_equals({ - 'id': '456', + 'type': 'comment', + 'id': 456, 'url': 'https://github.com/foo/bar/pull/123#issuecomment-456', }, result.content, result) @@ -663,6 +658,7 @@ def test_preview_comment(self): self.assertEquals(rendered, preview.content, preview) self.assertIn('comment on foo/bar#123, an issue title:', preview.description, preview) + @skip('only needed for GraphQL, and we currently use REST to create comments') def test_create_comment_escape_quotes(self): self.expect_graphql_issue() self.expect_graphql(json={ @@ -794,7 +790,8 @@ def test_preview_issue_tags_to_labels(self): 'create a new issue on foo/bar and attempt to add labels label 3, label_1:', preview.description, preview) - def test_create_comment_without_in_reply_to(self): + @skip('only needed for GraphQL, and we currently use REST to create comments') + def test_create_comment_org_access_forbidden(self): msg = 'Although you appear to have the correct authorization credentials,\nthe `whatwg` organization has enabled OAuth App access restrictions, meaning that data\naccess to third-parties is limited. For more information on these restrictions, including\nhow to whitelist this app, visit\nhttps://help.github.com/articles/restricting-access-to-your-organization-s-data/\n' self.expect_graphql_issue() @@ -827,7 +824,7 @@ def test_create_comment_without_in_reply_to(self): self.assertTrue(result.abort) self.assertEquals(msg, result.error_plain) - def test_create_comment_org_access_forbidden(self): + def test_create_comment_without_in_reply_to(self): """https://github.com/snarfed/bridgy/issues/824""" obj = copy.deepcopy(COMMENT_OBJ) obj['inReplyTo'] = [{'url': 'http://foo.com/bar'}]