Ignore new comments

This removes the handling of `r?`.

Author: Mark-Simulacrum

highfive/newpr.py

@@ -68,9 +68,6 @@ def run(self, event):
         elif event == "pull_request" and self.payload["action"] == "opened":
             self.new_pr()
             return 'OK\n'
-        elif event == "issue_comment" and self.payload["action"] == "created":
-            self.new_comment()
-            return 'OK\n'
         else:
             return 'Unsupported webhook event.\n'
 
@@ -398,37 +395,3 @@ def new_pr(self):
 
         if self.repo_config.get("new_pr_labels"):
             self.add_labels(owner, repo, issue)
-
-    def new_comment(self):
-        # Check the issue is a PR and is open.
-        if self.payload['issue', 'state'] != 'open' \
-                or 'pull_request' not in self.payload['issue']:
-            return
-
-        commenter = self.payload['comment', 'user', 'login']
-        # Ignore our own comments.
-        if commenter == self.integration_user:
-            return
-
-        owner = self.payload['repository', 'owner', 'login']
-        repo = self.payload['repository', 'name']
-
-        # Check the commenter is the submitter of the PR or the previous assignee.
-        author = self.payload['issue', 'user', 'login']
-        if not (author == commenter or (
-                self.payload['issue', 'assignee'] \
-                and commenter == self.payload['issue', 'assignee', 'login']
-        )):
-            # Check if commenter is a collaborator.
-            if not self.is_collaborator(commenter, owner, repo):
-                return
-
-        # Check for r? and set the assignee.
-        msg = self.payload['comment', 'body']
-        reviewer = self.find_reviewer(msg)
-        if reviewer:
-            issue = str(self.payload['issue', 'number'])
-            self.set_assignee(
-                reviewer, owner, repo, issue, self.integration_user,
-                author, None
-            )

highfive/tests/fakes.py

@@ -65,12 +65,6 @@ def get_global_configs():
 
 
 class Payload(object):
-    @staticmethod
-    def new_comment():
-        with open(get_fake_filename('create-comment.payload'), 'r') as fin:
-            p = json.load(fin)
-        return payload.Payload(p)
-
     @staticmethod
     def new_pr(
             number=7, pr_body='The PR comment.', pr_url='https://the.url/',

highfive/tests/fakes/create-comment.payload

@@ -258,45 +258,3 @@ def make_mocks(cls, patcherize):
         config_mock = mock.Mock()
         config_mock.get.side_effect = ('integration-user', 'integration-token')
         cls.mocks['ConfigParser'].RawConfigParser.return_value = config_mock
-
-    def test_author_is_commenter(self):
-        payload = fakes.Payload.new_comment()
-        handler = newpr.HighfiveHandler(payload, dummy_config())
-        api_req_mock = ApiReqMocker([
-            (
-                (
-                    'PATCH', newpr.issue_url % ('rust-lang', 'rust', '1'),
-                    {'assignee': 'davidalber'}
-                ),
-                {'body': {}},
-            ),
-        ])
-        handler.new_comment()
-        api_req_mock.verify_calls()
-
-    def test_author_not_commenter_is_collaborator(self):
-        payload = fakes.Payload.new_comment()
-        payload._payload['issue']['user']['login'] = 'foouser'
-
-        handler = newpr.HighfiveHandler(payload, dummy_config())
-        api_req_mock = ApiReqMocker([
-            (
-                (
-                    "GET",
-                    newpr.user_collabo_url % (
-                        'rust-lang', 'rust', 'davidalber'
-                    ),
-                    None
-                ),
-                {'body': {}},
-            ),
-            (
-                (
-                    'PATCH', newpr.issue_url % ('rust-lang', 'rust', '1'),
-                    {'assignee': 'davidalber'}
-                ),
-                {'body': {}},
-            ),
-        ])
-        handler.new_comment()
-        api_req_mock.verify_calls()

highfive/tests/test_integration_tests.py

@@ -937,23 +937,20 @@ def make_handler(
     def test_not_open(self):
         handler = self.make_handler(state='closed')
 
-        assert handler.new_comment() is None
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_not_called()
         self.mocks['set_assignee'].assert_not_called()
 
     def test_not_pr(self):
         handler = self.make_handler(is_pull_request=False)
 
-        handler.new_comment() is None
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_not_called()
         self.mocks['set_assignee'].assert_not_called()
 
     def test_commenter_is_integration_user(self):
         handler = self.make_handler(commenter='integrationUser')
 
-        assert handler.new_comment() is None
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_not_called()
         self.mocks['set_assignee'].assert_not_called()
@@ -964,7 +961,6 @@ def test_unauthorized_assigner(self):
         )
 
         self.mocks['is_collaborator'].return_value = False
-        assert handler.new_comment() is None
         self.mocks['is_collaborator'].assert_called_with(
             'userB', 'repo-owner', 'repo-name'
         )
@@ -978,7 +974,6 @@ def test_authorized_assigner_author_is_commenter(self):
             author='userA', commenter='userA', assignee='userC'
         )
 
-        handler.new_comment()
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_called()
 
@@ -987,7 +982,6 @@ def test_authorized_assigner_commenter_is_assignee(self):
             author='userA', commenter='userB', assignee='userB'
         )
 
-        handler.new_comment()
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_called()
 
@@ -997,7 +991,6 @@ def test_authorized_assigner_commenter_is_collaborator(self):
         )
 
         self.mocks['is_collaborator'].return_value = True
-        handler.new_comment()
         self.mocks['is_collaborator'].assert_called_with(
             'userB', 'repo-owner', 'repo-name'
         )
@@ -1007,7 +1000,6 @@ def test_no_reviewer(self):
         handler = self.make_handler(author='userA', commenter='userA')
 
         self.mocks['find_reviewer'].return_value = None
-        handler.new_comment()
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_called_with('comment!')
         self.mocks['set_assignee'].assert_not_called()
@@ -1016,7 +1008,6 @@ def test_has_reviewer(self):
         handler = self.make_handler(author='userA', commenter='userA')
 
         self.mocks['find_reviewer'].return_value = 'userD'
-        handler.new_comment()
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_called_with('comment!')
         self.mocks['set_assignee'].assert_called_with(
@@ -1197,7 +1188,6 @@ class TestRun(TestNewPR):
     def make_mocks(cls, patcherize):
         cls.mocks = patcherize((
             ('new_pr', 'highfive.newpr.HighfiveHandler.new_pr'),
-            ('new_comment', 'highfive.newpr.HighfiveHandler.new_comment'),
         ))
 
     def handler_mock(self, payload):
@@ -1210,18 +1200,9 @@ def test_newpr(self):
         m = self.handler_mock(payload)
         assert m.handler.run('pull_request') == 'OK\n'
         self.mocks['new_pr'].assert_called_once_with()
-        self.mocks['new_comment'].assert_not_called()
-
-    def test_new_comment(self):
-        payload = Payload({'action': 'created'})
-        m = self.handler_mock(payload)
-        assert m.handler.run('issue_comment') == 'OK\n'
-        self.mocks['new_pr'].assert_not_called()
-        self.mocks['new_comment'].assert_called_once_with()
 
     def test_unsupported_payload(self):
         payload = Payload({'action': 'something-not-supported'})
         m = self.handler_mock(payload)
         assert m.handler.run('issue_comment') != 'OK\n'
         self.mocks['new_pr'].assert_not_called()
-        self.mocks['new_comment'].assert_not_called()