github: switch creating comments from GraphQL to REST API

for snarfed/bridgy#824

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)).
 

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))
 

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('<span class="verb">comment</span> on <a href="https://github.com/foo/bar/pull/123">foo/bar#123, <em>an issue title</em></a>:', 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):
       '<span class="verb">create a new issue</span> on <a href="https://github.com/foo/bar/issues">foo/bar</a> and attempt to add labels <span class="verb">label 3, label_1</span>:',
       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'}]