From a879c02b3d7a0ac388dfefd3c704303dfadc2160 Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Mon, 17 Aug 2020 16:45:08 -0500 Subject: [PATCH 1/6] Issue #3455 - Part 1. Move process_issue_action into WebHookIssue model --- tests/unit/test_webhook.py | 138 ------------------------------- tests/unit/test_webhook_model.py | 137 ++++++++++++++++++++++++++++++ webcompat/webhooks/__init__.py | 3 +- webcompat/webhooks/helpers.py | 69 ---------------- webcompat/webhooks/model.py | 71 ++++++++++++++++ 5 files changed, 209 insertions(+), 209 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 1001d0bd2..b18afc906 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -7,7 +7,6 @@ """Tests for our webhooks: HTTP handling and helper methods.""" import json -import logging import os import unittest from unittest.mock import patch @@ -21,7 +20,6 @@ from webcompat.db import Site from webcompat.helpers import to_bytes from webcompat.webhooks import helpers -from webcompat.webhooks.helpers import process_issue_action from webcompat.webhooks.model import WebHookIssue @@ -29,15 +27,6 @@ # The key needs to be a bytes object key = to_bytes(webcompat.app.config['HOOK_SECRET_KEY']) -# Some expected responses as tuples -accepted = ('Moderated issue accepted', 200, {'Content-Type': 'text/plain'}) -rejected = ('Moderated issue rejected', 200, {'Content-Type': 'text/plain'}) -boring = ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) -gracias = ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) -wrong_repo = ('Wrong repository', 403, {'Content-Type': 'text/plain'}) -oops = ('oops', 400, {'Content-Type': 'text/plain'}) -comment_added = ('public url added', 200, {'Content-Type': 'text/plain'}) - # Some machinery for opening our test files def event_data(filename): @@ -436,132 +425,5 @@ def test_prepare_rejected_issue(self): self.assertEqual(actual, expected) -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_right_repo(mock_mr): - """Test that repository_url matches the CONFIG for public repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('new_event_valid.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == gracias - - -def test_process_issue_action_wrong_repo(): - """Test when repository_url differs from the CONFIG for public repo.""" - json_event, signature = event_data('wrong_repo.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == wrong_repo - - -def test_process_issue_action_wrong_repo(): - """Test for issues in the wrong repo.""" - json_event, signature = event_data( - 'private_milestone_accepted_wrong_repo.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == wrong_repo - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_acceptable_issue(mock_mr): - """Test for acceptable issues from private repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_accepted.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == accepted - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_private_issue_moderated_ok(mock_mr): - """Test for private issue successfully moderated.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_accepted.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == accepted - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_reject_issue(mock_mr): - """Test for rejected issues from private repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_closed.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == rejected - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_acceptable_issue_closed(mock_mr): - """Test for accepted issues being closed.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data( - 'private_milestone_accepted_closed.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == boring - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_comment_public_uri(mock_mr): - """Test we are getting the right message on public uri comment.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_issue_opened.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == comment_added - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_github_api_exception(mock_mr, caplog): - """Test GitHub API exception handling. - - Each of the test scenarios have the following: - issue_payload, expected_log, method - method is unused in the test, but is meant to provide context to - the reader for where the exception is happening. - """ - caplog.set_level(logging.INFO) - mock_mr.side_effect = HTTPError() - mock_mr.status_code = 400 - scenarios = [ - ('private_milestone_accepted.json', - 'private:moving to public failed', 'moderate_private_issue'), - ('private_issue_no_source.json', 'comment failed', - 'comment_public_uri'), - ('new_event_valid.json', 'public:opened labels failed', - 'tag_as_public'), - ('private_milestone_closed.json', - 'public rejection failed', 'reject_private_issue') - ] - for scenario in scenarios: - issue_payload, expected_log, method = scenario - json_event, signature = event_data(issue_payload) - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = process_issue_action(issue) - assert rv == oops - assert expected_log in caplog.text - - if __name__ == '__main__': unittest.main() diff --git a/tests/unit/test_webhook_model.py b/tests/unit/test_webhook_model.py index 65145c223..91b97c137 100644 --- a/tests/unit/test_webhook_model.py +++ b/tests/unit/test_webhook_model.py @@ -8,6 +8,7 @@ from dataclasses import asdict import json +import logging from unittest.mock import patch import pytest @@ -18,6 +19,15 @@ from tests.unit.test_webhook import event_data from webcompat.webhooks.model import WebHookIssue +# Some expected responses as tuples +accepted = ('Moderated issue accepted', 200, {'Content-Type': 'text/plain'}) +rejected = ('Moderated issue rejected', 200, {'Content-Type': 'text/plain'}) +boring = ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) +gracias = ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) +wrong_repo = ('Wrong repository', 403, {'Content-Type': 'text/plain'}) +oops = ('oops', 400, {'Content-Type': 'text/plain'}) +comment_added = ('public url added', 200, {'Content-Type': 'text/plain'}) + issue_info1 = { 'action': 'opened', 'state': 'open', 'milestoned_with': '', 'milestone': '', 'body': '\n\n\n\n\n**URL**: https://www.netflix.com/', # noqa @@ -193,3 +203,130 @@ def test_prepare_accepted_issue(mock_priority): 'labels': ['browser-firefox', 'priority-critical', 'engine-gecko'], 'title': 'www.netflix.com - test private issue accepted'} assert expected == actual + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_right_repo(mock_mr): + """Test that repository_url matches the CONFIG for public repo.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('new_event_valid.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == gracias + + +def test_process_issue_action_wrong_repo(): + """Test when repository_url differs from the CONFIG for public repo.""" + json_event, signature = event_data('wrong_repo.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == wrong_repo + + +def test_process_issue_action_wrong_repo(): + """Test for issues in the wrong repo.""" + json_event, signature = event_data( + 'private_milestone_accepted_wrong_repo.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == wrong_repo + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_acceptable_issue(mock_mr): + """Test for acceptable issues from private repo.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('private_milestone_accepted.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == accepted + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_private_issue_moderated_ok(mock_mr): + """Test for private issue successfully moderated.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('private_milestone_accepted.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == accepted + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_reject_issue(mock_mr): + """Test for rejected issues from private repo.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('private_milestone_closed.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == rejected + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_acceptable_issue_closed(mock_mr): + """Test for accepted issues being closed.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data( + 'private_milestone_accepted_closed.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == boring + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_comment_public_uri(mock_mr): + """Test we are getting the right message on public uri comment.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('private_issue_opened.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == comment_added + + +@patch('webcompat.webhooks.model.make_request') +def test_process_issue_action_github_api_exception(mock_mr, caplog): + """Test GitHub API exception handling. + + Each of the test scenarios have the following: + issue_payload, expected_log, method + method is unused in the test, but is meant to provide context to + the reader for where the exception is happening. + """ + caplog.set_level(logging.INFO) + mock_mr.side_effect = HTTPError() + mock_mr.status_code = 400 + scenarios = [ + ('private_milestone_accepted.json', + 'private:moving to public failed', 'moderate_private_issue'), + ('private_issue_no_source.json', 'comment failed', + 'comment_public_uri'), + ('new_event_valid.json', 'public:opened labels failed', + 'tag_as_public'), + ('private_milestone_closed.json', + 'public rejection failed', 'reject_private_issue') + ] + for scenario in scenarios: + issue_payload, expected_log, method = scenario + json_event, signature = event_data(issue_payload) + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == oops + assert expected_log in caplog.text diff --git a/webcompat/webhooks/__init__.py b/webcompat/webhooks/__init__.py index b66cfa1e0..30decdf8f 100644 --- a/webcompat/webhooks/__init__.py +++ b/webcompat/webhooks/__init__.py @@ -15,7 +15,6 @@ from webcompat.webhooks.helpers import is_github_hook from webcompat.webhooks.helpers import make_response -from webcompat.webhooks.helpers import process_issue_action from webcompat.webhooks.model import WebHookIssue from webcompat import app @@ -36,7 +35,7 @@ def hooklistener(): if event_type == 'issues': webhook_issue = WebHookIssue.from_dict(payload) # we process the action - return process_issue_action(webhook_issue) + return webhook_issue.process_issue_action() elif event_type == 'ping': return make_response('pong', 200) # If nothing worked as expected, the default response is 403. diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 8e2d8130f..be5ffc22e 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -185,75 +185,6 @@ def oops(): return make_response('oops', 400) -def process_issue_action(issue): - """Route the actions and provide different responses. - - There are two possible known scopes: - * public repo - * private repo - - Currently the actions we are handling are (for now): - * opened (public repo only) - Aka newly issues created and - need to be assigned labels and milestones - * milestoned (private repo only) - When the issue is being moderated with a milestone: accepted - """ - source_repo = issue.repository_url - scope = repo_scope(source_repo) - issue_number = issue.number - # We do not process further in case - # we don't know what we are dealing with - if scope == 'unknown': - return make_response('Wrong repository', 403) - if issue.action == 'opened' and scope == 'public': - # we are setting labels on each new open issues - try: - issue.tag_as_public() - except HTTPError as e: - msg_log(f'public:opened labels failed ({e})', issue_number) - return oops() - else: - return make_response('gracias, amigo.', 200) - elif issue.action == 'opened' and scope == 'private': - # webcompat-bot needs to comment on this issue with the URL - try: - issue.comment_public_uri() - except HTTPError as e: - msg_log(f'comment failed ({e})', issue_number) - return oops() - else: - return make_response('public url added', 200) - - elif (issue.action == 'milestoned' and scope == 'private' and - issue.milestoned_with == 'accepted'): - # private issue have been moderated and we will make it public - try: - issue.moderate_private_issue() - except HTTPError as e: - msg_log('private:moving to public failed', issue.number) - return oops() - else: - # we didn't get exceptions, so it's safe to close it - issue.close_private_issue() - return make_response('Moderated issue accepted', 200) - elif (scope == 'private' and issue.state == 'closed' and - not issue.milestone == 'accepted'): - # private issue has been closed. It is rejected - # We need to patch with a template. - try: - issue.reject_private_issue() - except HTTPError as e: - msg_log('public rejection failed', issue.number) - return oops() - else: - # we didn't get exceptions, so it's safe to close it - issue.close_private_issue() - return make_response('Moderated issue rejected', 200) - else: - return make_response('Not an interesting hook', 403) - - def repo_scope(source_repo): """Check the scope nature of the repository. diff --git a/webcompat/webhooks/model.py b/webcompat/webhooks/model.py index 6ddfee63c..62566d97b 100644 --- a/webcompat/webhooks/model.py +++ b/webcompat/webhooks/model.py @@ -17,8 +17,12 @@ from webcompat import app from webcompat.webhooks.helpers import extract_metadata from webcompat.webhooks.helpers import get_issue_labels +from webcompat.webhooks.helpers import make_response from webcompat.webhooks.helpers import make_request +from webcompat.webhooks.helpers import msg_log +from webcompat.webhooks.helpers import oops from webcompat.webhooks.helpers import prepare_rejected_issue +from webcompat.webhooks.helpers import repo_scope PUBLIC_REPO = app.config['ISSUES_REPO_URI'] PRIVATE_REPO = app.config['PRIVATE_REPO_URI'] @@ -181,3 +185,70 @@ def get_public_issue_number(self): if url: url = self.public_url.strip().rsplit('/', 1)[1] return url + + def process_issue_action(self): + """Route the actions and provide different responses. + + There are two possible known scopes: + * public repo + * private repo + + Currently the actions we are handling are (for now): + * opened (public repo only) + Aka newly issues created and + need to be assigned labels and milestones + * milestoned (private repo only) + When the issue is being moderated with a milestone: accepted + """ + source_repo = self.repository_url + scope = repo_scope(source_repo) + # We do not process further in case + # we don't know what we are dealing with + if scope == 'unknown': + return make_response('Wrong repository', 403) + if self.action == 'opened' and scope == 'public': + # we are setting labels on each new open issues + try: + self.tag_as_public() + except HTTPError as e: + msg_log(f'public:opened labels failed ({e})', self.number) + return oops() + else: + return make_response('gracias, amigo.', 200) + elif self.action == 'opened' and scope == 'private': + # webcompat-bot needs to comment on this issue with the URL + try: + self.comment_public_uri() + except HTTPError as e: + msg_log(f'comment failed ({e})', self.number) + return oops() + else: + return make_response('public url added', 200) + + elif (self.action == 'milestoned' and scope == 'private' and + self.milestoned_with == 'accepted'): + # private issue have been moderated and we will make it public + try: + self.moderate_private_issue() + except HTTPError as e: + msg_log('private:moving to public failed', self.number) + return oops() + else: + # we didn't get exceptions, so it's safe to close it + self.close_private_issue() + return make_response('Moderated issue accepted', 200) + elif (scope == 'private' and self.state == 'closed' and + not self.milestone == 'accepted'): + # private issue has been closed. It is rejected + # We need to patch with a template. + try: + self.reject_private_issue() + except HTTPError as e: + msg_log('public rejection failed', self.number) + return oops() + else: + # we didn't get exceptions, so it's safe to close it + self.close_private_issue() + return make_response('Moderated issue rejected', 200) + else: + return make_response('Not an interesting hook', 403) From ebded108bc31506d029f1df5e1e5fc59a7d5abfb Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Mon, 17 Aug 2020 17:40:49 -0500 Subject: [PATCH 2/6] Issue #3445 - Part 2. Add machinery for "accepted: invalid" milestone --- .../private_milestone_accepted_invalid.json | 41 +++++++++++++++++++ tests/unit/test_webhook.py | 15 +++++++ tests/unit/test_webhook_model.py | 20 ++++++++- webcompat/issues.py | 22 +++++++--- webcompat/webhooks/helpers.py | 19 +++++++++ webcompat/webhooks/model.py | 22 ++++++++++ 6 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/webhooks/private_milestone_accepted_invalid.json diff --git a/tests/fixtures/webhooks/private_milestone_accepted_invalid.json b/tests/fixtures/webhooks/private_milestone_accepted_invalid.json new file mode 100644 index 000000000..34cf758cd --- /dev/null +++ b/tests/fixtures/webhooks/private_milestone_accepted_invalid.json @@ -0,0 +1,41 @@ +{ + "action": "milestoned", + "issue": { + "title": "www.netflix.com - test private issue accepted", + "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-private", + "number": 600, + "body": "\n\n\n\n\n**URL**: https://www.netflix.com/", + "labels": [ + { + "id": 1788251357, + "node_id": "MDU6TGFiZWwxNzg4MjUxMzU3", + "url": "https://api.github.com/repos/webcompat/webcompat-tests/labels/action-needsmoderation", + "name": "action-needsmoderation", + "color": "d36200", + "default": false, + "description": "issue in the process of being moderated" + } + ], + "state": "open", + "milestone": { + "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2", + "html_url": "https://github.com/webcompat/webcompat-tests-private/milestone/2", + "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2/labels", + "id": 5092463, + "node_id": "MDk6TWlsZXN0b25lNTA5MjQ2Mw==", + "number": 2, + "title": "accepted: invalid", + "description": "This is when a bug has been accepted for moderation", + "open_issues": 0, + "closed_issues": 2, + "state": "open", + "created_at": "2020-02-11T00:37:18Z", + "updated_at": "2020-02-11T18:34:04Z", + "due_on": null, + "closed_at": null + } + }, + "milestone": { + "title": "accepted: invalid" + } + } diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index b18afc906..01f5b5d4f 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -411,6 +411,21 @@ def test_repo_scope_unknown(self): actual = helpers.repo_scope(url) self.assertEqual(expected, actual) + def test_prepare_invalid_issue(self): + """Test we prepare the right payload for the invalid issue.""" + expected = {'body': '

Thanks for the report, but this is not a Compatibility\nissue.

For this project we try to focus our effort on layouts, features\nor content that works as expected in one browser but not in another.\nClosing the issue as Invalid.

', # noqa + 'milestone': 8, + 'state': 'closed', + 'title': 'Test title!'} + actual = helpers.prepare_invalid_issue('Test title!') + self.assertEqual(type(actual), dict) + self.assertEqual(actual, expected) + + def test_prepare_invalid_issue_no_title(self): + """Test we raise for a missing title arguement.""" + with pytest.raises(ValueError) as excinfo: + helpers.prepare_invalid_issue() + def test_prepare_rejected_issue(self): """Test we prepare the right payload for the rejected issue.""" expected = {'body': "

The content of this issue doesn't meet our\n" diff --git a/tests/unit/test_webhook_model.py b/tests/unit/test_webhook_model.py index 91b97c137..395642d5d 100644 --- a/tests/unit/test_webhook_model.py +++ b/tests/unit/test_webhook_model.py @@ -142,6 +142,21 @@ def test_reject_private_issue(mock_mr): assert issue.get_public_issue_number() in uri +@patch('webcompat.webhooks.model.make_request') +def test_reject_invalid_issue(mock_mr): + """Test issue state and API request that is sent to GitHub.""" + mock_mr.return_value.status_code == 200 + json_event, signature = event_data('private_issue_opened.json') + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + issue.reject_invalid_issue() + method, uri, data = mock_mr.call_args[0] + # make sure we sent a patch with the right data to GitHub + assert method == 'patch' + assert type(data) == dict + assert issue.get_public_issue_number() in uri + + def test_prepare_public_comment(): """Test we prepare the right comment body.""" expected_payload = '{"body": "[Original issue 1](https://github.com/webcompat/webcompat-tests/issues/1)"}' # noqa @@ -319,7 +334,10 @@ def test_process_issue_action_github_api_exception(mock_mr, caplog): ('new_event_valid.json', 'public:opened labels failed', 'tag_as_public'), ('private_milestone_closed.json', - 'public rejection failed', 'reject_private_issue') + 'public rejection failed', 'reject_private_issue'), + ('private_milestone_accepted_invalid.json', + 'private:closing public issue as invalid failed', + 'reject_invalid_issue') ] for scenario in scenarios: issue_payload, expected_log, method = scenario diff --git a/webcompat/issues.py b/webcompat/issues.py index 7891dc2b6..e25bee083 100644 --- a/webcompat/issues.py +++ b/webcompat/issues.py @@ -23,10 +23,10 @@ PRIVATE_REPO_URI = app.config['PRIVATE_REPO_URI'] PRIVATE_REPO_MILESTONE = app.config['PRIVATE_REPO_MILESTONE'] -REJECTED_TITLE = 'Issue rejected.' -REJECTED_BODY = '''

The content of this issue doesn't meet our -acceptable use -guidelines. Its original content has been deleted.

''' +INVALID_BODY = '''

Thanks for the report, but this is not a Compatibility +issue.

For this project we try to focus our effort on layouts, features +or content that works as expected in one browser but not in another. +Closing the issue as Invalid.

''' ONGOING_TITLE = 'In the moderation queue.' ONGOING_BODY = '''

This issue has been put in the moderation queue. A human will review if the message meets our current @@ -35,20 +35,30 @@ It will probably take a couple of days depending on the backlog. Once it has been reviewed, the content will be either made public or deleted.

''' +REJECTED_TITLE = 'Issue rejected.' +REJECTED_BODY = '''

The content of this issue doesn't meet our +acceptable use +guidelines. Its original content has been deleted.

''' -def moderation_template(choice='ongoing'): +def moderation_template(choice='ongoing', title=None): """Gets the placeholder data to send for unmoderated issues. - The moderation is for now two types: + The moderation is for now these types: - ongoing: the issue is in the moderation queue. - rejected: the issue has been rejected. + - invalid: the issue is invalid (but not rejected) The default is 'ongoing' even with unknown keywords. """ if choice == 'rejected': title = REJECTED_TITLE body = REJECTED_BODY + elif choice == 'invalid': + if not title: + raise ValueError("A title must be passed in for invalid issues") + title = title + body = INVALID_BODY else: title = ONGOING_TITLE body = ONGOING_BODY diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index be5ffc22e..2ed97c1b2 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -209,6 +209,25 @@ def msg_log(msg, issue_number): log.info(msg) +def prepare_invalid_issue(title=None): + """Create the payload for the invalid moderated issue. + + When the issue has been moderated as "accepted:invalid", + we need to change a couple of things in the public space + + - change body + - close the issue + - remove the action-needsmoderation label + - change the milestone to invalid + """ + # Extract the relevant information + invalid_id = app.config['STATUSES']['invalid']['id'] + payload_request = moderation_template('invalid', title) + payload_request['state'] = 'closed' + payload_request['milestone'] = invalid_id + return payload_request + + def prepare_rejected_issue(): """Create the payload for the rejected moderated issue. diff --git a/webcompat/webhooks/model.py b/webcompat/webhooks/model.py index 62566d97b..37eebd344 100644 --- a/webcompat/webhooks/model.py +++ b/webcompat/webhooks/model.py @@ -21,6 +21,7 @@ from webcompat.webhooks.helpers import make_request from webcompat.webhooks.helpers import msg_log from webcompat.webhooks.helpers import oops +from webcompat.webhooks.helpers import prepare_invalid_issue from webcompat.webhooks.helpers import prepare_rejected_issue from webcompat.webhooks.helpers import repo_scope @@ -143,6 +144,14 @@ def prepare_public_comment(self): # prepare the payload return f'[Original issue {public_number}]({self.public_url})' + def reject_invalid_issue(self): + """Send a passed-moderation-yet-invalid PATCH to the public issue.""" + payload_request = prepare_invalid_issue(self.title) + public_number = self.get_public_issue_number() + # Preparing the proxy request + path = f'repos/{PUBLIC_REPO}/{public_number}' + make_request('patch', path, payload_request) + def reject_private_issue(self): """Send a rejected moderation PATCH on the public issue.""" payload_request = prepare_rejected_issue() @@ -237,6 +246,19 @@ def process_issue_action(self): # we didn't get exceptions, so it's safe to close it self.close_private_issue() return make_response('Moderated issue accepted', 200) + elif (self.action == 'milestoned' and scope == 'private' and + self.milestoned_with == 'accepted: invalid'): + try: + self.reject_invalid_issue() + except HTTPError as e: + msg_log( + 'private:closing public issue as invalid failed', + self.number) + return oops() + else: + # we didn't get exceptions, so it's safe to close it + self.close_private_issue() + return make_response('Moderated issue closed as invalid', 200) elif (scope == 'private' and self.state == 'closed' and not self.milestone == 'accepted'): # private issue has been closed. It is rejected From e3711105d54b8b4af701d5e0075229e6ed98a227 Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Mon, 17 Aug 2020 20:11:49 -0500 Subject: [PATCH 3/6] Issue #3445 - Part 3. Add machinery for "accepted: incomplete" milestone --- ...private_milestone_accepted_incomplete.json | 41 +++++++++++++++++++ tests/unit/test_webhook.py | 19 +++++++-- tests/unit/test_webhook_model.py | 37 +++++++---------- webcompat/issues.py | 10 +++++ webcompat/webhooks/helpers.py | 19 +++++++++ webcompat/webhooks/model.py | 23 +++++++++++ 6 files changed, 122 insertions(+), 27 deletions(-) create mode 100644 tests/fixtures/webhooks/private_milestone_accepted_incomplete.json diff --git a/tests/fixtures/webhooks/private_milestone_accepted_incomplete.json b/tests/fixtures/webhooks/private_milestone_accepted_incomplete.json new file mode 100644 index 000000000..a4cbe6593 --- /dev/null +++ b/tests/fixtures/webhooks/private_milestone_accepted_incomplete.json @@ -0,0 +1,41 @@ +{ + "action": "milestoned", + "issue": { + "title": "www.netflix.com - test private issue accepted", + "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-private", + "number": 600, + "body": "\n\n\n\n\n**URL**: https://www.netflix.com/", + "labels": [ + { + "id": 1788251357, + "node_id": "MDU6TGFiZWwxNzg4MjUxMzU3", + "url": "https://api.github.com/repos/webcompat/webcompat-tests/labels/action-needsmoderation", + "name": "action-needsmoderation", + "color": "d36200", + "default": false, + "description": "issue in the process of being moderated" + } + ], + "state": "open", + "milestone": { + "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2", + "html_url": "https://github.com/webcompat/webcompat-tests-private/milestone/2", + "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2/labels", + "id": 5092463, + "node_id": "MDk6TWlsZXN0b25lNTA5MjQ2Mw==", + "number": 2, + "title": "accepted: incomplete", + "description": "This is when a bug has been accepted for moderation", + "open_issues": 0, + "closed_issues": 2, + "state": "open", + "created_at": "2020-02-11T00:37:18Z", + "updated_at": "2020-02-11T18:34:04Z", + "due_on": null, + "closed_at": null + } + }, + "milestone": { + "title": "accepted: incomplete" + } + } diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 01f5b5d4f..755f97d38 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -411,20 +411,31 @@ def test_repo_scope_unknown(self): actual = helpers.repo_scope(url) self.assertEqual(expected, actual) + def test_prepare_incomplete_issue(self): + """Test we prepare the right payload for the incomplete issue.""" + expected = {'body': '

Thanks for the report. Unfortunately without any\ndetail about the issue you experienced, we cannot help with this bug.\nPlease leave a comment with more detail, or file a new report and we will\ngladly investigate this further.

', # noqa + 'milestone': 7, + 'state': 'closed', + 'title': 'Test incomplete title!'} + actual = helpers.prepare_incomplete_issue('Test incomplete title!') + self.assertEqual(type(actual), dict) + self.assertEqual(actual, expected) + def test_prepare_invalid_issue(self): """Test we prepare the right payload for the invalid issue.""" expected = {'body': '

Thanks for the report, but this is not a Compatibility\nissue.

For this project we try to focus our effort on layouts, features\nor content that works as expected in one browser but not in another.\nClosing the issue as Invalid.

', # noqa 'milestone': 8, 'state': 'closed', - 'title': 'Test title!'} - actual = helpers.prepare_invalid_issue('Test title!') + 'title': 'Test invalid title!'} + actual = helpers.prepare_invalid_issue('Test invalid title!') self.assertEqual(type(actual), dict) self.assertEqual(actual, expected) - def test_prepare_invalid_issue_no_title(self): - """Test we raise for a missing title arguement.""" + def test_prepare_issue_no_title(self): + """Test we raise for missing title arguements.""" with pytest.raises(ValueError) as excinfo: helpers.prepare_invalid_issue() + helpers.prepare_incomplete_issue() def test_prepare_rejected_issue(self): """Test we prepare the right payload for the rejected issue.""" diff --git a/tests/unit/test_webhook_model.py b/tests/unit/test_webhook_model.py index 395642d5d..7b0f84977 100644 --- a/tests/unit/test_webhook_model.py +++ b/tests/unit/test_webhook_model.py @@ -128,33 +128,21 @@ def test_moderate_public_issue(mock_mr): @patch('webcompat.webhooks.model.make_request') -def test_reject_private_issue(mock_mr): +def test_closing_public_issues(mock_mr): """Test issue state and API request that is sent to GitHub.""" mock_mr.return_value.status_code == 200 json_event, signature = event_data('private_issue_opened.json') payload = json.loads(json_event) issue = WebHookIssue.from_dict(payload) - issue.reject_private_issue() - method, uri, data = mock_mr.call_args[0] - # make sure we sent a patch with the right data to GitHub - assert method == 'patch' - assert type(data) == dict - assert issue.get_public_issue_number() in uri - - -@patch('webcompat.webhooks.model.make_request') -def test_reject_invalid_issue(mock_mr): - """Test issue state and API request that is sent to GitHub.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_issue_opened.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - issue.reject_invalid_issue() - method, uri, data = mock_mr.call_args[0] - # make sure we sent a patch with the right data to GitHub - assert method == 'patch' - assert type(data) == dict - assert issue.get_public_issue_number() in uri + test_functions = [issue.reject_incomplete_issue, + issue.reject_invalid_issue, issue.reject_private_issue] + for fn in test_functions: + fn() + method, uri, data = mock_mr.call_args[0] + # make sure we sent a patch with the right data to GitHub + assert method == 'patch' + assert type(data) == dict + assert issue.get_public_issue_number() in uri def test_prepare_public_comment(): @@ -337,7 +325,10 @@ def test_process_issue_action_github_api_exception(mock_mr, caplog): 'public rejection failed', 'reject_private_issue'), ('private_milestone_accepted_invalid.json', 'private:closing public issue as invalid failed', - 'reject_invalid_issue') + 'reject_invalid_issue'), + ('private_milestone_accepted_incomplete.json', + 'private:closing public issue as incomplete failed', + 'reject_incomplete_issue') ] for scenario in scenarios: issue_payload, expected_log, method = scenario diff --git a/webcompat/issues.py b/webcompat/issues.py index e25bee083..f141942ee 100644 --- a/webcompat/issues.py +++ b/webcompat/issues.py @@ -27,6 +27,10 @@ issue.

For this project we try to focus our effort on layouts, features or content that works as expected in one browser but not in another. Closing the issue as Invalid.

''' +INCOMPLETE_BODY = '''

Thanks for the report. Unfortunately without any +detail about the issue you experienced, we cannot help with this bug. +Please leave a comment with more detail, or file a new report and we will +gladly investigate this further.

''' ONGOING_TITLE = 'In the moderation queue.' ONGOING_BODY = '''

This issue has been put in the moderation queue. A human will review if the message meets our current @@ -47,6 +51,7 @@ def moderation_template(choice='ongoing', title=None): The moderation is for now these types: - ongoing: the issue is in the moderation queue. - rejected: the issue has been rejected. + - incomplete: the issue is incomplete (but not rejected) - invalid: the issue is invalid (but not rejected) The default is 'ongoing' even with unknown keywords. @@ -59,6 +64,11 @@ def moderation_template(choice='ongoing', title=None): raise ValueError("A title must be passed in for invalid issues") title = title body = INVALID_BODY + elif choice == 'incomplete': + if not title: + raise ValueError("A title must be passed in for incomplete issues") + title = title + body = INCOMPLETE_BODY else: title = ONGOING_TITLE body = ONGOING_BODY diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 2ed97c1b2..4d764e4e5 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -209,6 +209,25 @@ def msg_log(msg, issue_number): log.info(msg) +def prepare_incomplete_issue(title=None): + """Create the payload for the incomplete moderated issue. + + When the issue has been moderated as "accepted:incomplete", + we need to change a couple of things in the public space + + - change body + - close the issue + - remove the action-needsmoderation label + - change the milestone to invalid + """ + # Extract the relevant information + incomplete_id = app.config['STATUSES']['incomplete']['id'] + payload_request = moderation_template('incomplete', title) + payload_request['state'] = 'closed' + payload_request['milestone'] = incomplete_id + return payload_request + + def prepare_invalid_issue(title=None): """Create the payload for the invalid moderated issue. diff --git a/webcompat/webhooks/model.py b/webcompat/webhooks/model.py index 37eebd344..3e24717c1 100644 --- a/webcompat/webhooks/model.py +++ b/webcompat/webhooks/model.py @@ -21,6 +21,7 @@ from webcompat.webhooks.helpers import make_request from webcompat.webhooks.helpers import msg_log from webcompat.webhooks.helpers import oops +from webcompat.webhooks.helpers import prepare_incomplete_issue from webcompat.webhooks.helpers import prepare_invalid_issue from webcompat.webhooks.helpers import prepare_rejected_issue from webcompat.webhooks.helpers import repo_scope @@ -144,6 +145,14 @@ def prepare_public_comment(self): # prepare the payload return f'[Original issue {public_number}]({self.public_url})' + def reject_incomplete_issue(self): + """Send a passed-moderation-yet-incomplete PATCH to public issue.""" + payload_request = prepare_incomplete_issue(self.title) + public_number = self.get_public_issue_number() + # Preparing the proxy request + path = f'repos/{PUBLIC_REPO}/{public_number}' + make_request('patch', path, payload_request) + def reject_invalid_issue(self): """Send a passed-moderation-yet-invalid PATCH to the public issue.""" payload_request = prepare_invalid_issue(self.title) @@ -246,6 +255,20 @@ def process_issue_action(self): # we didn't get exceptions, so it's safe to close it self.close_private_issue() return make_response('Moderated issue accepted', 200) + elif (self.action == 'milestoned' and scope == 'private' and + self.milestoned_with == 'accepted: incomplete'): + try: + self.reject_incomplete_issue() + except HTTPError as e: + msg_log( + 'private:closing public issue as incomplete failed', + self.number) + return oops() + else: + # we didn't get exceptions, so it's safe to close it + self.close_private_issue() + return make_response('Moderated issue closed as incomplete', + 200) elif (self.action == 'milestoned' and scope == 'private' and self.milestoned_with == 'accepted: invalid'): try: From 9c71b101823307a9f3e06c53d78740c837dd15e1 Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Mon, 17 Aug 2020 20:26:04 -0500 Subject: [PATCH 4/6] Issue #3445 - Part 4. Create a common close_public_issue method --- tests/unit/test_webhook_model.py | 13 ++++++----- webcompat/webhooks/model.py | 37 ++++++++++++++------------------ 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/tests/unit/test_webhook_model.py b/tests/unit/test_webhook_model.py index 7b0f84977..1947e0a35 100644 --- a/tests/unit/test_webhook_model.py +++ b/tests/unit/test_webhook_model.py @@ -134,10 +134,9 @@ def test_closing_public_issues(mock_mr): json_event, signature = event_data('private_issue_opened.json') payload = json.loads(json_event) issue = WebHookIssue.from_dict(payload) - test_functions = [issue.reject_incomplete_issue, - issue.reject_invalid_issue, issue.reject_private_issue] - for fn in test_functions: - fn() + reasons = ['incomplete', 'invalid', 'rejected'] + for reason in reasons: + issue.close_public_issue(reason=reason) method, uri, data = mock_mr.call_args[0] # make sure we sent a patch with the right data to GitHub assert method == 'patch' @@ -322,13 +321,13 @@ def test_process_issue_action_github_api_exception(mock_mr, caplog): ('new_event_valid.json', 'public:opened labels failed', 'tag_as_public'), ('private_milestone_closed.json', - 'public rejection failed', 'reject_private_issue'), + 'public rejection failed', 'close_public_issue'), ('private_milestone_accepted_invalid.json', 'private:closing public issue as invalid failed', - 'reject_invalid_issue'), + 'close_public_issue'), ('private_milestone_accepted_incomplete.json', 'private:closing public issue as incomplete failed', - 'reject_incomplete_issue') + 'close_public_issue') ] for scenario in scenarios: issue_payload, expected_log, method = scenario diff --git a/webcompat/webhooks/model.py b/webcompat/webhooks/model.py index 3e24717c1..30ec3ab9c 100644 --- a/webcompat/webhooks/model.py +++ b/webcompat/webhooks/model.py @@ -145,25 +145,20 @@ def prepare_public_comment(self): # prepare the payload return f'[Original issue {public_number}]({self.public_url})' - def reject_incomplete_issue(self): - """Send a passed-moderation-yet-incomplete PATCH to public issue.""" - payload_request = prepare_incomplete_issue(self.title) - public_number = self.get_public_issue_number() - # Preparing the proxy request - path = f'repos/{PUBLIC_REPO}/{public_number}' - make_request('patch', path, payload_request) + def close_public_issue(self, reason='rejected'): + """Close a public issue for the given reason. - def reject_invalid_issue(self): - """Send a passed-moderation-yet-invalid PATCH to the public issue.""" - payload_request = prepare_invalid_issue(self.title) - public_number = self.get_public_issue_number() - # Preparing the proxy request - path = f'repos/{PUBLIC_REPO}/{public_number}' - make_request('patch', path, payload_request) - - def reject_private_issue(self): - """Send a rejected moderation PATCH on the public issue.""" - payload_request = prepare_rejected_issue() + Right now the accepted reasons are: + 'incomplete' + 'invalid' + 'rejected' (default) + """ + if reason == 'incomplete': + payload_request = prepare_incomplete_issue(self.title) + elif reason == 'invalid': + payload_request = prepare_invalid_issue(self.title) + else: + payload_request = prepare_rejected_issue() public_number = self.get_public_issue_number() # Preparing the proxy request path = f'repos/{PUBLIC_REPO}/{public_number}' @@ -258,7 +253,7 @@ def process_issue_action(self): elif (self.action == 'milestoned' and scope == 'private' and self.milestoned_with == 'accepted: incomplete'): try: - self.reject_incomplete_issue() + self.close_public_issue(reason='incomplete') except HTTPError as e: msg_log( 'private:closing public issue as incomplete failed', @@ -272,7 +267,7 @@ def process_issue_action(self): elif (self.action == 'milestoned' and scope == 'private' and self.milestoned_with == 'accepted: invalid'): try: - self.reject_invalid_issue() + self.close_public_issue(reason='invalid') except HTTPError as e: msg_log( 'private:closing public issue as invalid failed', @@ -287,7 +282,7 @@ def process_issue_action(self): # private issue has been closed. It is rejected # We need to patch with a template. try: - self.reject_private_issue() + self.close_public_issue(reason='rejected') except HTTPError as e: msg_log('public rejection failed', self.number) return oops() From 80d600756243110d930f6933dcd3d6b0c98fec3d Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Mon, 17 Aug 2020 20:29:15 -0500 Subject: [PATCH 5/6] Issue #3445 - Part 5. Add process_issue_actions tests for new milestones And remove all the redundancy. --- tests/unit/test_webhook_model.py | 114 +++++++------------------------ 1 file changed, 24 insertions(+), 90 deletions(-) diff --git a/tests/unit/test_webhook_model.py b/tests/unit/test_webhook_model.py index 1947e0a35..8e3e1ded4 100644 --- a/tests/unit/test_webhook_model.py +++ b/tests/unit/test_webhook_model.py @@ -22,6 +22,10 @@ # Some expected responses as tuples accepted = ('Moderated issue accepted', 200, {'Content-Type': 'text/plain'}) rejected = ('Moderated issue rejected', 200, {'Content-Type': 'text/plain'}) +incomplete = ('Moderated issue closed as incomplete', + 200, {'Content-Type': 'text/plain'}) +invalid = ('Moderated issue closed as invalid', + 200, {'Content-Type': 'text/plain'}) boring = ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) gracias = ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) wrong_repo = ('Wrong repository', 403, {'Content-Type': 'text/plain'}) @@ -208,97 +212,27 @@ def test_prepare_accepted_issue(mock_priority): @patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_right_repo(mock_mr): - """Test that repository_url matches the CONFIG for public repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('new_event_valid.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == gracias - - -def test_process_issue_action_wrong_repo(): - """Test when repository_url differs from the CONFIG for public repo.""" - json_event, signature = event_data('wrong_repo.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == wrong_repo - - -def test_process_issue_action_wrong_repo(): - """Test for issues in the wrong repo.""" - json_event, signature = event_data( - 'private_milestone_accepted_wrong_repo.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == wrong_repo - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_acceptable_issue(mock_mr): - """Test for acceptable issues from private repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_accepted.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == accepted - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_private_issue_moderated_ok(mock_mr): - """Test for private issue successfully moderated.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_accepted.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == accepted - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_reject_issue(mock_mr): - """Test for rejected issues from private repo.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_milestone_closed.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == rejected - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_acceptable_issue_closed(mock_mr): - """Test for accepted issues being closed.""" - mock_mr.return_value.status_code == 200 - json_event, signature = event_data( - 'private_milestone_accepted_closed.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == boring - - -@patch('webcompat.webhooks.model.make_request') -def test_process_issue_action_comment_public_uri(mock_mr): - """Test we are getting the right message on public uri comment.""" +def test_process_issue_action_scenarios(mock_mr): + """Test we are getting the right response for each scenario.""" + test_data = [ + ('new_event_valid.json', gracias), + ('wrong_repo.json', wrong_repo), + ('private_milestone_accepted_wrong_repo.json', wrong_repo), + ('private_milestone_accepted.json', accepted), + ('private_milestone_closed.json', rejected), + ('private_milestone_accepted_incomplete.json', incomplete), + ('private_milestone_accepted_invalid.json', invalid), + ('private_milestone_accepted_closed.json', boring), + ('private_issue_opened.json', comment_added) + ] mock_mr.return_value.status_code == 200 - json_event, signature = event_data('private_issue_opened.json') - payload = json.loads(json_event) - issue = WebHookIssue.from_dict(payload) - with webcompat.app.test_request_context(): - rv = issue.process_issue_action() - assert rv == comment_added + for issue_event, expected_rv in test_data: + json_event, signature = event_data(issue_event) + payload = json.loads(json_event) + issue = WebHookIssue.from_dict(payload) + with webcompat.app.test_request_context(): + rv = issue.process_issue_action() + assert rv == expected_rv @patch('webcompat.webhooks.model.make_request') From 5cf7bba88bd4d02ce1e63cfa9df5c771ad355720 Mon Sep 17 00:00:00 2001 From: Mike Taylor Date: Fri, 21 Aug 2020 10:45:23 -0500 Subject: [PATCH 6/6] Issue #3445 - Review Feedback. Remove stray blank line. --- webcompat/webhooks/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/webcompat/webhooks/model.py b/webcompat/webhooks/model.py index 30ec3ab9c..eba2c7d6d 100644 --- a/webcompat/webhooks/model.py +++ b/webcompat/webhooks/model.py @@ -237,7 +237,6 @@ def process_issue_action(self): return oops() else: return make_response('public url added', 200) - elif (self.action == 'milestoned' and scope == 'private' and self.milestoned_with == 'accepted'): # private issue have been moderated and we will make it public