From 258335653d57a465ddc608adf70615f39cb27c14 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 12:54:28 +0900 Subject: [PATCH 01/22] Issue #3140 - Adds a process_issue_action for better flexibility This basically push the action processing on issues out of the route. It adds a generic function that can be further refined and handles multiple actions. Some goals: * easier to read * easier to test * more flexibility for growth --- tests/unit/test_webhook.py | 2 +- webcompat/webhooks/__init__.py | 27 ++++----------------------- webcompat/webhooks/helpers.py | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index b7349a152..345af7e90 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -323,7 +323,7 @@ def test_new_opened_issue(self): self.assertEqual(response.status_code, 401) self.assertTrue('Bad credentials' in response.content) - @patch('webcompat.webhooks.new_opened_issue') + @patch('webcompat.webhooks.helpers.new_opened_issue') def test_new_issue_right_repo(self, mock_proxy): """Test that repository_url matches the CONFIG for public repo. diff --git a/webcompat/webhooks/__init__.py b/webcompat/webhooks/__init__.py index d20447898..435b8c58f 100644 --- a/webcompat/webhooks/__init__.py +++ b/webcompat/webhooks/__init__.py @@ -16,13 +16,10 @@ from webcompat.webhooks.helpers import get_issue_info from webcompat.webhooks.helpers import is_github_hook -from webcompat.webhooks.helpers import new_opened_issue +from webcompat.webhooks.helpers import process_issue_action from webcompat import app -public_repo = app.config['ISSUES_REPO_URI'] -private_repo = app.config['PRIVATE_REPO_URI'] - @app.route('/webhooks/labeler', methods=['POST']) def hooklistener(): @@ -38,25 +35,9 @@ def hooklistener(): # Treating events related to issues if event_type == 'issues': issue = get_issue_info(payload) - source_repo = issue['repository_url'] - # A new issue has been created. - # In the future we can add new type of actions. - if issue['action'] == 'opened': - # We scope the opened action only to the public repo - if not source_repo.endswith(public_repo.rsplit('/issues')[0]): - return ('Wrong repository', 403, - {'Content-Type': 'text/plain'}) - # we are setting labels on each new open issues - response = new_opened_issue(payload) - if response.status_code == 200: - return ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) - else: - log = app.logger - log.setLevel(logging.INFO) - msg = 'failed to set labels on issue {issue}'.format( - issue=issue['number']) - log.info(msg) - return ('ooops', 400, {'Content-Type': 'text/plain'}) + # we process the action + response = process_issue_action(issue, payload) + return response elif event_type == 'ping': return ('pong', 200, {'Content-Type': 'text/plain'}) # If nothing worked as expected, the default response is 403. diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 32304e5ad..1b4065dce 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -27,6 +27,8 @@ 'browser-focus-geckoview', 'browser-geckoview', ] +public_repo = app.config['ISSUES_REPO_URI'] +private_repo = app.config['PRIVATE_REPO_URI'] def extract_metadata(body): @@ -206,3 +208,26 @@ def new_opened_issue(payload): headers=headers, data=json.dumps(payload_request)) return proxy_response + + +def process_issue_action(issue, payload): + """Route the actions and provide different responses.""" + source_repo = issue['repository_url'] + if issue['action'] == 'opened': + # We scope the opened action only to the public repo + if not source_repo.endswith(public_repo.rsplit('/issues')[0]): + return ('Wrong repository', 403, + {'Content-Type': 'text/plain'}) + # we are setting labels on each new open issues + response = new_opened_issue(payload) + if response.status_code == 200: + return ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) + else: + log = app.logger + log.setLevel(logging.INFO) + msg = 'failed to set labels on issue {issue}'.format( + issue=issue['number']) + log.info(msg) + return ('ooops', 400, {'Content-Type': 'text/plain'}) + else: + return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) From 32337c22fdeb58b041985e3c8d92865d9b4e7712 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 14:27:54 +0900 Subject: [PATCH 02/22] Issue #3140 - Adds repo_scope to know the scope of the repo Still in the process of making the code more readable. repo_scope will provide a way to know if we are dealing with a public or a private repo. It makes it easier to scope the actions to certain consequences and it makes it more readable and testable. --- tests/fixtures/webhooks/wrong_repo.json | 2 +- tests/unit/test_webhook.py | 23 ++++++++++++- webcompat/webhooks/helpers.py | 46 ++++++++++++++++++++----- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/tests/fixtures/webhooks/wrong_repo.json b/tests/fixtures/webhooks/wrong_repo.json index ef879f667..5ba3d2b46 100644 --- a/tests/fixtures/webhooks/wrong_repo.json +++ b/tests/fixtures/webhooks/wrong_repo.json @@ -2,7 +2,7 @@ "action": "opened", "issue": { "title": "www.netflix.com - test wrong repo", - "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-private", + "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-foobar", "number": 600, "body": "\n\n\n\n**URL**: https://www.netflix.com/", "labels": [ diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 345af7e90..00e34b6dd 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -359,7 +359,7 @@ def test_new_issue_wrong_repo(self): json_event, signature = event_data('wrong_repo.json') headers = { 'X-GitHub-Event': 'issues', - 'X-Hub-Signature': 'sha1=585e6a35199b5d6e7a5321ca9231aed0a104f41e', + 'X-Hub-Signature': 'sha1=650006ac2f518e2b4dd6201055c652cd82149821', } with webcompat.app.test_client() as c: rv = c.post( @@ -371,6 +371,27 @@ def test_new_issue_wrong_repo(self): self.assertEqual(rv.status_code, 403) self.assertEqual(rv.content_type, 'text/plain') + def test_repo_scope_public(self): + """Test the public scope of the repository.""" + url = 'https://api.github.com/repos/webcompat/webcompat-tests' + expected = 'public' + actual = helpers.repo_scope(url) + self.assertEqual(expected, actual) + + def test_repo_scope_private(self): + """Test the private scope of the repository.""" + url = 'https://api.github.com/repos/webcompat/webcompat-tests-private' + expected = 'private' + actual = helpers.repo_scope(url) + self.assertEqual(expected, actual) + + def test_repo_scope_unknown(self): + """Test the unknown of the repository.""" + url = 'https://api.github.com/repos/webcompat/webcompat-foobar' + expected = 'unknown' + actual = helpers.repo_scope(url) + self.assertEqual(expected, actual) + if __name__ == '__main__': unittest.main() diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 1b4065dce..5db573ee7 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -27,8 +27,8 @@ 'browser-focus-geckoview', 'browser-geckoview', ] -public_repo = app.config['ISSUES_REPO_URI'] -private_repo = app.config['PRIVATE_REPO_URI'] +PUBLIC_REPO = app.config['ISSUES_REPO_URI'] +PRIVATE_REPO = app.config['PRIVATE_REPO_URI'] def extract_metadata(body): @@ -211,13 +211,27 @@ def new_opened_issue(payload): def process_issue_action(issue, payload): - """Route the actions and provide different responses.""" + """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'] - if issue['action'] == 'opened': - # We scope the opened action only to the public repo - if not source_repo.endswith(public_repo.rsplit('/issues')[0]): - return ('Wrong repository', 403, - {'Content-Type': 'text/plain'}) + scope = repo_scope(source_repo) + # We do not process further in case + # we don't know what we are dealing with + print('SCOPE ', scope) + if scope == 'unknown': + return ('Wrong repository', 403, {'Content-Type': 'text/plain'}) + if issue['action'] == 'opened' and scope == 'public': # we are setting labels on each new open issues response = new_opened_issue(payload) if response.status_code == 200: @@ -231,3 +245,19 @@ def process_issue_action(issue, payload): return ('ooops', 400, {'Content-Type': 'text/plain'}) else: return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) + + +def repo_scope(source_repo): + """Check the scope nature of the repository. + + The repository can be a + * known public. + * known private. + * or completely unknown. + """ + scope = 'unknown' + if source_repo.endswith(PUBLIC_REPO.rsplit('/issues')[0]): + scope = 'public' + elif source_repo.endswith(PRIVATE_REPO.rsplit('/issues')[0]): + scope = 'private' + return scope From 938b84b89ac7535581e1508d205c022738b4f523 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 14:34:31 +0900 Subject: [PATCH 03/22] Issue #3140 - Adds tests placeholder for PATCH valid issues --- tests/unit/test_webhook.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 00e34b6dd..3ba257ba6 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -392,6 +392,33 @@ def test_repo_scope_unknown(self): actual = helpers.repo_scope(url) self.assertEqual(expected, actual) + def test_patch_acceptable_issue(self): + """Test for acceptable issues comes from private repo. + + payload: 'Moderated issue accepted' + status: 200 + content-type: text/plain + """ + raise unittest.SkipTest('TODO') + + def test_patch_not_acceptable_issue(self): + """Test for not acceptable issues from private repo. + + payload: 'Moderated issue rejected' + status: 200 + content-type: text/plain + """ + raise unittest.SkipTest('TODO') + + def test_patch_wrong_repo_for_moderation(self): + """Test for not acceptable issues from private repo. + + payload: 'Wrong repository' + status: 403 + content-type: text/plain + """ + raise unittest.SkipTest('TODO') + if __name__ == '__main__': unittest.main() From 36408c9fd05b42ee9d24190d5ffbb3cab0e21a23 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 15:00:48 +0900 Subject: [PATCH 04/22] Issue #3140 - Adds a more generic msg_log method --- tests/unit/test_webhook.py | 4 ++-- webcompat/webhooks/helpers.py | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 3ba257ba6..f782a431f 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -402,7 +402,7 @@ def test_patch_acceptable_issue(self): raise unittest.SkipTest('TODO') def test_patch_not_acceptable_issue(self): - """Test for not acceptable issues from private repo. + """Test for rejected issues from private repo. payload: 'Moderated issue rejected' status: 200 @@ -411,7 +411,7 @@ def test_patch_not_acceptable_issue(self): raise unittest.SkipTest('TODO') def test_patch_wrong_repo_for_moderation(self): - """Test for not acceptable issues from private repo. + """Test for issues in the wrong repo. payload: 'Wrong repository' status: 403 diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 5db573ee7..e93d59b50 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -226,9 +226,9 @@ def process_issue_action(issue, payload): """ 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 - print('SCOPE ', scope) if scope == 'unknown': return ('Wrong repository', 403, {'Content-Type': 'text/plain'}) if issue['action'] == 'opened' and scope == 'public': @@ -237,11 +237,7 @@ def process_issue_action(issue, payload): if response.status_code == 200: return ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) else: - log = app.logger - log.setLevel(logging.INFO) - msg = 'failed to set labels on issue {issue}'.format( - issue=issue['number']) - log.info(msg) + msg_log('public:opened labels failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) else: return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) @@ -261,3 +257,11 @@ def repo_scope(source_repo): elif source_repo.endswith(PRIVATE_REPO.rsplit('/issues')[0]): scope = 'private' return scope + + +def msg_log(msg, issue_number): + """Write a log with the reason and the issue number.""" + log = app.logger + log.setLevel(logging.INFO) + msg = 'issue {issue} {msg}'.format(issue=issue_number, msg=msg) + log.info(msg) From 8c65b53755ecad729e49ccef92e9f496c64b199b Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 16:07:32 +0900 Subject: [PATCH 05/22] Issue #3140 - Adds fixture for accepted milestone on private issue --- .../webhooks/private-milestone-accepted.json | 256 ++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 tests/fixtures/webhooks/private-milestone-accepted.json diff --git a/tests/fixtures/webhooks/private-milestone-accepted.json b/tests/fixtures/webhooks/private-milestone-accepted.json new file mode 100644 index 000000000..1539e0325 --- /dev/null +++ b/tests/fixtures/webhooks/private-milestone-accepted.json @@ -0,0 +1,256 @@ +{ + "action": "milestoned", + "issue": { + "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30", + "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-private", + "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/labels{/name}", + "comments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/comments", + "events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/events", + "html_url": "https://github.com/webcompat/webcompat-tests-private/issues/30", + "id": 550573102, + "node_id": "MDU6SXNzdWU1NTA1NzMxMDI=", + "number": 30, + "title": "In the moderation queue.", + "user": { + "login": "karlcow", + "id": 505230, + "node_id": "MDQ6VXNlcjUwNTIzMA==", + "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/karlcow", + "html_url": "https://github.com/karlcow", + "followers_url": "https://api.github.com/users/karlcow/followers", + "following_url": "https://api.github.com/users/karlcow/following{/other_user}", + "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", + "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", + "organizations_url": "https://api.github.com/users/karlcow/orgs", + "repos_url": "https://api.github.com/users/karlcow/repos", + "events_url": "https://api.github.com/users/karlcow/events{/privacy}", + "received_events_url": "https://api.github.com/users/karlcow/received_events", + "type": "User", + "site_admin": false + }, + "labels": [], + "state": "open", + "locked": false, + "assignee": null, + "assignees": [ + + ], + "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": 2744172, + "node_id": "MDk6TWlsZXN0b25lMjc0NDE3Mg==", + "number": 2, + "title": "accepted", + "description": "Issues which have been accepted", + "creator": { + "login": "karlcow", + "id": 505230, + "node_id": "MDQ6VXNlcjUwNTIzMA==", + "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/karlcow", + "html_url": "https://github.com/karlcow", + "followers_url": "https://api.github.com/users/karlcow/followers", + "following_url": "https://api.github.com/users/karlcow/following{/other_user}", + "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", + "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", + "organizations_url": "https://api.github.com/users/karlcow/orgs", + "repos_url": "https://api.github.com/users/karlcow/repos", + "events_url": "https://api.github.com/users/karlcow/events{/privacy}", + "received_events_url": "https://api.github.com/users/karlcow/received_events", + "type": "User", + "site_admin": false + }, + "open_issues": 634, + "closed_issues": 24, + "state": "open", + "created_at": "2017-09-05T03:39:06Z", + "updated_at": "2020-01-16T04:51:44Z", + "due_on": null, + "closed_at": null + }, + "comments": 0, + "created_at": "2020-01-16T04:51:43Z", + "updated_at": "2020-01-16T04:51:44Z", + "closed_at": null, + "author_association": "CONTRIBUTOR", + "body": "

This issue has been put in the moderation queue. A human\nwill review if the message meets our current\n\nacceptable use guidelines.\nIt will probably take a couple of days depending on the backlog.\nOnce it has been reviewed, the content will be either made public\nor deleted.

" + }, + "milestone": { + "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2", + "html_url": "https://github.com/webcompat/webcompat-tests-private/milestone/1", + "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2/labels", + "id": 2744172, + "node_id": "MDk6TWlsZXN0b25lMjc0NDE3Mg==", + "number": 2, + "title": "accepted", + "description": "Issues which needs to be triaged", + "creator": { + "login": "karlcow", + "id": 505230, + "node_id": "MDQ6VXNlcjUwNTIzMA==", + "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/karlcow", + "html_url": "https://github.com/karlcow", + "followers_url": "https://api.github.com/users/karlcow/followers", + "following_url": "https://api.github.com/users/karlcow/following{/other_user}", + "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", + "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", + "organizations_url": "https://api.github.com/users/karlcow/orgs", + "repos_url": "https://api.github.com/users/karlcow/repos", + "events_url": "https://api.github.com/users/karlcow/events{/privacy}", + "received_events_url": "https://api.github.com/users/karlcow/received_events", + "type": "User", + "site_admin": false + }, + "open_issues": 634, + "closed_issues": 24, + "state": "open", + "created_at": "2017-09-05T03:39:06Z", + "updated_at": "2020-01-16T04:51:44Z", + "due_on": null, + "closed_at": null + }, + "repository": { + "id": 17839063, + "node_id": "MDEwOlJlcG9zaXRvcnkxNzgzOTA2Mw==", + "name": "webcompat-tests-private", + "full_name": "webcompat/webcompat-tests-private", + "private": false, + "owner": { + "login": "webcompat", + "id": 6175168, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjYxNzUxNjg=", + "avatar_url": "https://avatars3.githubusercontent.com/u/6175168?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/webcompat", + "html_url": "https://github.com/webcompat", + "followers_url": "https://api.github.com/users/webcompat/followers", + "following_url": "https://api.github.com/users/webcompat/following{/other_user}", + "gists_url": "https://api.github.com/users/webcompat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/webcompat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/webcompat/subscriptions", + "organizations_url": "https://api.github.com/users/webcompat/orgs", + "repos_url": "https://api.github.com/users/webcompat/repos", + "events_url": "https://api.github.com/users/webcompat/events{/privacy}", + "received_events_url": "https://api.github.com/users/webcompat/received_events", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/webcompat/webcompat-tests-private", + "description": "Test issues for webcompat.com", + "fork": false, + "url": "https://api.github.com/repos/webcompat/webcompat-tests-private", + "forks_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/forks", + "keys_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/teams", + "hooks_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/hooks", + "issue_events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/events{/number}", + "events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/events", + "assignees_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/assignees{/user}", + "branches_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/branches{/branch}", + "tags_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/tags", + "blobs_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/statuses/{sha}", + "languages_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/languages", + "stargazers_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/stargazers", + "contributors_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/contributors", + "subscribers_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/subscribers", + "subscription_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/subscription", + "commits_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/contents/{+path}", + "compare_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/merges", + "archive_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/downloads", + "issues_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues{/number}", + "pulls_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/pulls{/number}", + "milestones_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones{/number}", + "notifications_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/labels{/name}", + "releases_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/releases{/id}", + "deployments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/deployments", + "created_at": "2014-03-17T18:39:50Z", + "updated_at": "2019-10-28T18:41:13Z", + "pushed_at": "2019-10-09T01:11:11Z", + "git_url": "git://github.com/webcompat/webcompat-tests-private.git", + "ssh_url": "git@github.com:webcompat/webcompat-tests-private.git", + "clone_url": "https://github.com/webcompat/webcompat-tests-private.git", + "svn_url": "https://github.com/webcompat/webcompat-tests-private", + "homepage": "", + "size": 7, + "stargazers_count": 7, + "watchers_count": 7, + "language": null, + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": false, + "has_pages": false, + "forks_count": 2, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 693, + "license": { + "key": "mpl-2.0", + "name": "Mozilla Public License 2.0", + "spdx_id": "MPL-2.0", + "url": "https://api.github.com/licenses/mpl-2.0", + "node_id": "MDc6TGljZW5zZTE0" + }, + "forks": 2, + "open_issues": 693, + "watchers": 7, + "default_branch": "master" + }, + "organization": { + "login": "webcompat", + "id": 6175168, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjYxNzUxNjg=", + "url": "https://api.github.com/orgs/webcompat", + "repos_url": "https://api.github.com/orgs/webcompat/repos", + "events_url": "https://api.github.com/orgs/webcompat/events", + "hooks_url": "https://api.github.com/orgs/webcompat/hooks", + "issues_url": "https://api.github.com/orgs/webcompat/issues", + "members_url": "https://api.github.com/orgs/webcompat/members{/member}", + "public_members_url": "https://api.github.com/orgs/webcompat/public_members{/member}", + "avatar_url": "https://avatars3.githubusercontent.com/u/6175168?v=4", + "description": "" + }, + "sender": { + "login": "anonymousbot", + "id": 8493563, + "node_id": "MDQ6VXNlcjg0OTM1NjM=", + "avatar_url": "https://avatars2.githubusercontent.com/u/8493563?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/anonymousbot", + "html_url": "https://github.com/anonymousbot", + "followers_url": "https://api.github.com/users/anonymousbot/followers", + "following_url": "https://api.github.com/users/anonymousbot/following{/other_user}", + "gists_url": "https://api.github.com/users/anonymousbot/gists{/gist_id}", + "starred_url": "https://api.github.com/users/anonymousbot/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/anonymousbot/subscriptions", + "organizations_url": "https://api.github.com/users/anonymousbot/orgs", + "repos_url": "https://api.github.com/users/anonymousbot/repos", + "events_url": "https://api.github.com/users/anonymousbot/events{/privacy}", + "received_events_url": "https://api.github.com/users/anonymousbot/received_events", + "type": "User", + "site_admin": false + } +} From 92153cb8cca200a709a7ae8c40cd34b301e17359 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 22 Jan 2020 16:09:20 +0900 Subject: [PATCH 06/22] Issue #3140 - Makes the tests adjustable I had forgotten that I didn't need to actually put the signature as I am already computing it. That makes things a lot simpler when changing the body of the fixture. --- tests/unit/test_webhook.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index f782a431f..12b241633 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -335,7 +335,7 @@ def test_new_issue_right_repo(self, mock_proxy): json_event, signature = event_data('new_event_valid.json') headers = { 'X-GitHub-Event': 'issues', - 'X-Hub-Signature': 'sha1=2fd56e551f8243a4c8094239916131535051f74b', + 'X-Hub-Signature': signature, } with webcompat.app.test_client() as c: mock_proxy.return_value.status_code = 200 @@ -359,7 +359,7 @@ def test_new_issue_wrong_repo(self): json_event, signature = event_data('wrong_repo.json') headers = { 'X-GitHub-Event': 'issues', - 'X-Hub-Signature': 'sha1=650006ac2f518e2b4dd6201055c652cd82149821', + 'X-Hub-Signature': signature, } with webcompat.app.test_client() as c: rv = c.post( From 96a2ed15f907d020d0ee74e0296b8b31ef6466eb Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Thu, 23 Jan 2020 11:38:56 +0900 Subject: [PATCH 07/22] Issue #3140 - Changes the method name for tagging new public issues That makes it more obvious. `new_opened_issue` was a bit generic. --- tests/unit/test_webhook.py | 8 ++++---- webcompat/webhooks/helpers.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 12b241633..f084f425e 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -305,25 +305,25 @@ def test_signature_check(self): post_signature = 'sha1=wrong' self.assertFalse(helpers.signature_check(key, post_signature, payload)) - def test_new_opened_issue(self): + def test_tag_new_public_issue(self): """Test the core actions on new opened issues for WebHooks.""" # A 200 response json_event, signature = event_data('new_event_valid.json') payload = json.loads(json_event) with patch('webcompat.webhooks.helpers.proxy_request') as proxy: proxy.return_value.status_code = 200 - response = helpers.new_opened_issue(payload) + response = helpers.tag_new_public_issue(payload) self.assertEqual(response.status_code, 200) # A 401 response proxy.return_value.status_code = 401 proxy.return_value.content = '{"message":"Bad credentials","documentation_url":"https://developer.github.com/v3"}' # noqa with patch.dict('webcompat.webhooks.helpers.app.config', {'OAUTH_TOKEN': ''}): - response = helpers.new_opened_issue(payload) + response = helpers.tag_new_public_issue(payload) self.assertEqual(response.status_code, 401) self.assertTrue('Bad credentials' in response.content) - @patch('webcompat.webhooks.helpers.new_opened_issue') + @patch('webcompat.webhooks.helpers.tag_new_public_issue') def test_new_issue_right_repo(self, mock_proxy): """Test that repository_url matches the CONFIG for public repo. diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index e93d59b50..2696dcea7 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -171,7 +171,7 @@ def get_issue_labels(issue_body): return labelslist -def new_opened_issue(payload): +def tag_new_public_issue(payload): """Set the core actions on new opened issues. When a new issue is opened, we set a couple of things. @@ -233,7 +233,7 @@ def process_issue_action(issue, payload): return ('Wrong repository', 403, {'Content-Type': 'text/plain'}) if issue['action'] == 'opened' and scope == 'public': # we are setting labels on each new open issues - response = new_opened_issue(payload) + response = tag_new_public_issue(payload) if response.status_code == 200: return ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) else: From 9d427febb673bd95111ed40806a004fe73e7a5a2 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Thu, 23 Jan 2020 16:13:59 +0900 Subject: [PATCH 08/22] Issue #3140 - Changes file name to our conventions --- ...te-milestone-accepted.json => private_milestone_accepted.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/fixtures/webhooks/{private-milestone-accepted.json => private_milestone_accepted.json} (100%) diff --git a/tests/fixtures/webhooks/private-milestone-accepted.json b/tests/fixtures/webhooks/private_milestone_accepted.json similarity index 100% rename from tests/fixtures/webhooks/private-milestone-accepted.json rename to tests/fixtures/webhooks/private_milestone_accepted.json From c8028bc765b1e8b6f78d29aec5f375828ea362c2 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Thu, 23 Jan 2020 16:24:00 +0900 Subject: [PATCH 09/22] Issue #3140 - Adds HTTP handling of accepted private issues There is a couple of things to unpack here. * Simplifies the json fixture so we focus on what we need. * Adds a test which is handling **only** the HTTP workflow of the webhook note they start to repeat a bit, and we probably need to functionalize this a bit at a point. We will see that at the end. * Moves logging where it should have been in previous commit. My bad. * Modifies the dictionary we use with the info we need to add a milestoned_with keyword * Test that the payload has the right criteria before processing it. This doesn't contain the actual posting to the public repo **yet** Because there will be probably related to #3141 too. --- .../webhooks/private_milestone_accepted.json | 263 +----------------- tests/unit/test_webhook.py | 58 +++- webcompat/webhooks/__init__.py | 1 - webcompat/webhooks/helpers.py | 30 +- 4 files changed, 97 insertions(+), 255 deletions(-) diff --git a/tests/fixtures/webhooks/private_milestone_accepted.json b/tests/fixtures/webhooks/private_milestone_accepted.json index 1539e0325..2b3ad3193 100644 --- a/tests/fixtures/webhooks/private_milestone_accepted.json +++ b/tests/fixtures/webhooks/private_milestone_accepted.json @@ -1,256 +1,23 @@ { "action": "milestoned", "issue": { - "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30", + "title": "www.netflix.com - test private issue accepted", "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-private", - "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/labels{/name}", - "comments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/comments", - "events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/30/events", - "html_url": "https://github.com/webcompat/webcompat-tests-private/issues/30", - "id": 550573102, - "node_id": "MDU6SXNzdWU1NTA1NzMxMDI=", - "number": 30, - "title": "In the moderation queue.", - "user": { - "login": "karlcow", - "id": 505230, - "node_id": "MDQ6VXNlcjUwNTIzMA==", - "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/karlcow", - "html_url": "https://github.com/karlcow", - "followers_url": "https://api.github.com/users/karlcow/followers", - "following_url": "https://api.github.com/users/karlcow/following{/other_user}", - "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", - "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", - "organizations_url": "https://api.github.com/users/karlcow/orgs", - "repos_url": "https://api.github.com/users/karlcow/repos", - "events_url": "https://api.github.com/users/karlcow/events{/privacy}", - "received_events_url": "https://api.github.com/users/karlcow/received_events", - "type": "User", - "site_admin": false - }, - "labels": [], - "state": "open", - "locked": false, - "assignee": null, - "assignees": [ - - ], - "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": 2744172, - "node_id": "MDk6TWlsZXN0b25lMjc0NDE3Mg==", - "number": 2, - "title": "accepted", - "description": "Issues which have been accepted", - "creator": { - "login": "karlcow", - "id": 505230, - "node_id": "MDQ6VXNlcjUwNTIzMA==", - "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/karlcow", - "html_url": "https://github.com/karlcow", - "followers_url": "https://api.github.com/users/karlcow/followers", - "following_url": "https://api.github.com/users/karlcow/following{/other_user}", - "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", - "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", - "organizations_url": "https://api.github.com/users/karlcow/orgs", - "repos_url": "https://api.github.com/users/karlcow/repos", - "events_url": "https://api.github.com/users/karlcow/events{/privacy}", - "received_events_url": "https://api.github.com/users/karlcow/received_events", - "type": "User", - "site_admin": false - }, - "open_issues": 634, - "closed_issues": 24, - "state": "open", - "created_at": "2017-09-05T03:39:06Z", - "updated_at": "2020-01-16T04:51:44Z", - "due_on": null, - "closed_at": null - }, - "comments": 0, - "created_at": "2020-01-16T04:51:43Z", - "updated_at": "2020-01-16T04:51:44Z", - "closed_at": null, - "author_association": "CONTRIBUTOR", - "body": "

This issue has been put in the moderation queue. A human\nwill review if the message meets our current\n\nacceptable use guidelines.\nIt will probably take a couple of days depending on the backlog.\nOnce it has been reviewed, the content will be either made public\nor deleted.

" + "number": 600, + "body": "\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" + } + ] }, "milestone": { - "url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2", - "html_url": "https://github.com/webcompat/webcompat-tests-private/milestone/1", - "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones/2/labels", - "id": 2744172, - "node_id": "MDk6TWlsZXN0b25lMjc0NDE3Mg==", - "number": 2, - "title": "accepted", - "description": "Issues which needs to be triaged", - "creator": { - "login": "karlcow", - "id": 505230, - "node_id": "MDQ6VXNlcjUwNTIzMA==", - "avatar_url": "https://avatars0.githubusercontent.com/u/505230?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/karlcow", - "html_url": "https://github.com/karlcow", - "followers_url": "https://api.github.com/users/karlcow/followers", - "following_url": "https://api.github.com/users/karlcow/following{/other_user}", - "gists_url": "https://api.github.com/users/karlcow/gists{/gist_id}", - "starred_url": "https://api.github.com/users/karlcow/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/karlcow/subscriptions", - "organizations_url": "https://api.github.com/users/karlcow/orgs", - "repos_url": "https://api.github.com/users/karlcow/repos", - "events_url": "https://api.github.com/users/karlcow/events{/privacy}", - "received_events_url": "https://api.github.com/users/karlcow/received_events", - "type": "User", - "site_admin": false - }, - "open_issues": 634, - "closed_issues": 24, - "state": "open", - "created_at": "2017-09-05T03:39:06Z", - "updated_at": "2020-01-16T04:51:44Z", - "due_on": null, - "closed_at": null - }, - "repository": { - "id": 17839063, - "node_id": "MDEwOlJlcG9zaXRvcnkxNzgzOTA2Mw==", - "name": "webcompat-tests-private", - "full_name": "webcompat/webcompat-tests-private", - "private": false, - "owner": { - "login": "webcompat", - "id": 6175168, - "node_id": "MDEyOk9yZ2FuaXphdGlvbjYxNzUxNjg=", - "avatar_url": "https://avatars3.githubusercontent.com/u/6175168?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/webcompat", - "html_url": "https://github.com/webcompat", - "followers_url": "https://api.github.com/users/webcompat/followers", - "following_url": "https://api.github.com/users/webcompat/following{/other_user}", - "gists_url": "https://api.github.com/users/webcompat/gists{/gist_id}", - "starred_url": "https://api.github.com/users/webcompat/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/webcompat/subscriptions", - "organizations_url": "https://api.github.com/users/webcompat/orgs", - "repos_url": "https://api.github.com/users/webcompat/repos", - "events_url": "https://api.github.com/users/webcompat/events{/privacy}", - "received_events_url": "https://api.github.com/users/webcompat/received_events", - "type": "Organization", - "site_admin": false - }, - "html_url": "https://github.com/webcompat/webcompat-tests-private", - "description": "Test issues for webcompat.com", - "fork": false, - "url": "https://api.github.com/repos/webcompat/webcompat-tests-private", - "forks_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/forks", - "keys_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/keys{/key_id}", - "collaborators_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/collaborators{/collaborator}", - "teams_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/teams", - "hooks_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/hooks", - "issue_events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/events{/number}", - "events_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/events", - "assignees_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/assignees{/user}", - "branches_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/branches{/branch}", - "tags_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/tags", - "blobs_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/blobs{/sha}", - "git_tags_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/tags{/sha}", - "git_refs_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/refs{/sha}", - "trees_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/trees{/sha}", - "statuses_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/statuses/{sha}", - "languages_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/languages", - "stargazers_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/stargazers", - "contributors_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/contributors", - "subscribers_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/subscribers", - "subscription_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/subscription", - "commits_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/commits{/sha}", - "git_commits_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/git/commits{/sha}", - "comments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/comments{/number}", - "issue_comment_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues/comments{/number}", - "contents_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/contents/{+path}", - "compare_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/compare/{base}...{head}", - "merges_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/merges", - "archive_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/{archive_format}{/ref}", - "downloads_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/downloads", - "issues_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/issues{/number}", - "pulls_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/pulls{/number}", - "milestones_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/milestones{/number}", - "notifications_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/notifications{?since,all,participating}", - "labels_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/labels{/name}", - "releases_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/releases{/id}", - "deployments_url": "https://api.github.com/repos/webcompat/webcompat-tests-private/deployments", - "created_at": "2014-03-17T18:39:50Z", - "updated_at": "2019-10-28T18:41:13Z", - "pushed_at": "2019-10-09T01:11:11Z", - "git_url": "git://github.com/webcompat/webcompat-tests-private.git", - "ssh_url": "git@github.com:webcompat/webcompat-tests-private.git", - "clone_url": "https://github.com/webcompat/webcompat-tests-private.git", - "svn_url": "https://github.com/webcompat/webcompat-tests-private", - "homepage": "", - "size": 7, - "stargazers_count": 7, - "watchers_count": 7, - "language": null, - "has_issues": true, - "has_projects": true, - "has_downloads": true, - "has_wiki": false, - "has_pages": false, - "forks_count": 2, - "mirror_url": null, - "archived": false, - "disabled": false, - "open_issues_count": 693, - "license": { - "key": "mpl-2.0", - "name": "Mozilla Public License 2.0", - "spdx_id": "MPL-2.0", - "url": "https://api.github.com/licenses/mpl-2.0", - "node_id": "MDc6TGljZW5zZTE0" - }, - "forks": 2, - "open_issues": 693, - "watchers": 7, - "default_branch": "master" - }, - "organization": { - "login": "webcompat", - "id": 6175168, - "node_id": "MDEyOk9yZ2FuaXphdGlvbjYxNzUxNjg=", - "url": "https://api.github.com/orgs/webcompat", - "repos_url": "https://api.github.com/orgs/webcompat/repos", - "events_url": "https://api.github.com/orgs/webcompat/events", - "hooks_url": "https://api.github.com/orgs/webcompat/hooks", - "issues_url": "https://api.github.com/orgs/webcompat/issues", - "members_url": "https://api.github.com/orgs/webcompat/members{/member}", - "public_members_url": "https://api.github.com/orgs/webcompat/public_members{/member}", - "avatar_url": "https://avatars3.githubusercontent.com/u/6175168?v=4", - "description": "" - }, - "sender": { - "login": "anonymousbot", - "id": 8493563, - "node_id": "MDQ6VXNlcjg0OTM1NjM=", - "avatar_url": "https://avatars2.githubusercontent.com/u/8493563?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/anonymousbot", - "html_url": "https://github.com/anonymousbot", - "followers_url": "https://api.github.com/users/anonymousbot/followers", - "following_url": "https://api.github.com/users/anonymousbot/following{/other_user}", - "gists_url": "https://api.github.com/users/anonymousbot/gists{/gist_id}", - "starred_url": "https://api.github.com/users/anonymousbot/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/anonymousbot/subscriptions", - "organizations_url": "https://api.github.com/users/anonymousbot/orgs", - "repos_url": "https://api.github.com/users/anonymousbot/repos", - "events_url": "https://api.github.com/users/anonymousbot/events{/privacy}", - "received_events_url": "https://api.github.com/users/anonymousbot/received_events", - "type": "User", - "site_admin": false + "title": "accepted" } } diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index f084f425e..ef0568e53 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -292,6 +292,14 @@ def test_get_issue_info(self): actual = helpers.get_issue_info(payload) self.assertDictEqual(expected, actual) + def test_get_milestoned_issue_info(self): + """Extract the right information for a milestoned issue.""" + json_event, signature = event_data('private_milestone_accepted.json') + payload = json.loads(json_event) + expected = {'number': 600, 'repository_url': 'https://api.github.com/repos/webcompat/webcompat-tests-private', 'action': 'milestoned', 'domain': 'www.netflix.com', 'milestoned_with': 'accepted'} # noqa + actual = helpers.get_issue_info(payload) + self.assertDictEqual(expected, actual) + def test_signature_check(self): """Test the signature check function for WebHooks.""" payload = 'A body' @@ -392,14 +400,53 @@ def test_repo_scope_unknown(self): actual = helpers.repo_scope(url) self.assertEqual(expected, actual) - def test_patch_acceptable_issue(self): + @patch('webcompat.webhooks.helpers.private_issue_moderation') + def test_patch_acceptable_issue(self, mock_proxy): """Test for acceptable issues comes from private repo. payload: 'Moderated issue accepted' status: 200 content-type: text/plain """ - raise unittest.SkipTest('TODO') + json_event, signature = event_data('private_milestone_accepted.json') + headers = { + 'X-GitHub-Event': 'issues', + 'X-Hub-Signature': signature, + } + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 200 + rv = c.post( + '/webhooks/labeler', + data=json_event, + headers=headers + ) + self.assertEqual(rv.data, b'Moderated issue accepted') + self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.content_type, 'text/plain') + + @patch('webcompat.webhooks.helpers.private_issue_moderation') + def test_patch_acceptable_issue_problem(self, mock_proxy): + """Test for accepted issues failed. + + payload: 'ooops' + status: 400 + content-type: text/plain + """ + json_event, signature = event_data('private_milestone_accepted.json') + headers = { + 'X-GitHub-Event': 'issues', + 'X-Hub-Signature': signature, + } + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 400 + rv = c.post( + '/webhooks/labeler', + data=json_event, + headers=headers + ) + self.assertEqual(rv.data, b'ooops') + self.assertEqual(rv.status_code, 400) + self.assertEqual(rv.content_type, 'text/plain') def test_patch_not_acceptable_issue(self): """Test for rejected issues from private repo. @@ -419,6 +466,13 @@ def test_patch_wrong_repo_for_moderation(self): """ raise unittest.SkipTest('TODO') + def test_private_issue_moderated_ok(self): + """Test for private issue successfully moderated. + + it returns a 200 code. + """ + raise unittest.SkipTest('TODO') + if __name__ == '__main__': unittest.main() diff --git a/webcompat/webhooks/__init__.py b/webcompat/webhooks/__init__.py index 435b8c58f..b2aacc1a4 100644 --- a/webcompat/webhooks/__init__.py +++ b/webcompat/webhooks/__init__.py @@ -10,7 +10,6 @@ """ import json -import logging from flask import request diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 2696dcea7..f1d125feb 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -8,6 +8,7 @@ import hashlib import hmac import json +import logging import re from webcompat import app @@ -146,10 +147,14 @@ def get_issue_info(payload): # Extract the title and the body title = payload.get('issue')['title'] # Create the issue dictionary - return {'action': payload.get('action'), - 'number': payload.get('issue')['number'], - 'domain': title.partition(' ')[0], - 'repository_url': payload.get('issue')['repository_url']} + issue = {'action': payload.get('action'), + 'number': payload.get('issue')['number'], + 'domain': title.partition(' ')[0], + 'repository_url': payload.get('issue')['repository_url']} + # webhook with a milestoned action + if payload.get('milestone'): + issue['milestoned_with'] = payload.get('milestone')['title'] + return issue def get_issue_labels(issue_body): @@ -239,6 +244,18 @@ def process_issue_action(issue, payload): else: msg_log('public:opened labels failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) + # TODO probably do a function. Too many conditions. + elif (issue['action'] == 'milestoned' and + scope == 'private' and + issue['milestoned_with'] == 'accepted'): + # private issue have been moderated and we will make it public + response = private_issue_moderation(payload) + if response.status_code == 200: + return ('Moderated issue accepted', + 200, {'Content-Type': 'text/plain'}) + else: + msg_log('private:moving to public failed', issue_number) + return ('ooops', 400, {'Content-Type': 'text/plain'}) else: return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) @@ -265,3 +282,8 @@ def msg_log(msg, issue_number): log.setLevel(logging.INFO) msg = 'issue {issue} {msg}'.format(issue=issue_number, msg=msg) log.info(msg) + + +def private_issue_moderation(payload): + """Publish a private issue in public after moderation.""" + return 'boom' From c553d92d440d8e1ddca9979d7089f4360f0a1123 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Fri, 24 Jan 2020 16:47:31 +0900 Subject: [PATCH 10/22] Issue #3140 - Adjusts fixture with public_url metadata --- tests/fixtures/webhooks/private_milestone_accepted.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/webhooks/private_milestone_accepted.json b/tests/fixtures/webhooks/private_milestone_accepted.json index 2b3ad3193..dbb7e0a21 100644 --- a/tests/fixtures/webhooks/private_milestone_accepted.json +++ b/tests/fixtures/webhooks/private_milestone_accepted.json @@ -4,7 +4,7 @@ "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**URL**: https://www.netflix.com/", + "body": "\n\n\n\n\n**URL**: https://www.netflix.com/", "labels": [ { "id": 1788251357, From ee11706d6fa3bfab403d767f68c76d1c5117b985 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Fri, 24 Jan 2020 16:49:48 +0900 Subject: [PATCH 11/22] Issue #3140 - Passes only one type of structured data This will make it a bit clearer in the code. Instead of manipulating two types of data, we focus only on one structured dictionary --- webcompat/webhooks/__init__.py | 4 ++-- webcompat/webhooks/helpers.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/webcompat/webhooks/__init__.py b/webcompat/webhooks/__init__.py index b2aacc1a4..80a8257f9 100644 --- a/webcompat/webhooks/__init__.py +++ b/webcompat/webhooks/__init__.py @@ -33,9 +33,9 @@ def hooklistener(): event_type = request.headers.get('X-GitHub-Event') # Treating events related to issues if event_type == 'issues': - issue = get_issue_info(payload) + issue_info = get_issue_info(payload) # we process the action - response = process_issue_action(issue, payload) + response = process_issue_action(issue_info) return response elif event_type == 'ping': return ('pong', 200, {'Content-Type': 'text/plain'}) diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index f1d125feb..63de9b119 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -215,7 +215,7 @@ def tag_new_public_issue(payload): return proxy_response -def process_issue_action(issue, payload): +def process_issue_action(issue_info): """Route the actions and provide different responses. There are two possible known scopes: @@ -229,27 +229,27 @@ def process_issue_action(issue, payload): * milestoned (private repo only) When the issue is being moderated with a milestone: accepted """ - source_repo = issue['repository_url'] + source_repo = issue_info['repository_url'] scope = repo_scope(source_repo) - issue_number = issue['number'] + issue_number = issue_info['number'] # We do not process further in case # we don't know what we are dealing with if scope == 'unknown': return ('Wrong repository', 403, {'Content-Type': 'text/plain'}) - if issue['action'] == 'opened' and scope == 'public': + if issue_info['action'] == 'opened' and scope == 'public': # we are setting labels on each new open issues - response = tag_new_public_issue(payload) + response = tag_new_public_issue(issue_info) if response.status_code == 200: return ('gracias, amigo.', 200, {'Content-Type': 'text/plain'}) else: msg_log('public:opened labels failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) # TODO probably do a function. Too many conditions. - elif (issue['action'] == 'milestoned' and + elif (issue_info['action'] == 'milestoned' and scope == 'private' and - issue['milestoned_with'] == 'accepted'): + issue_info['milestoned_with'] == 'accepted'): # private issue have been moderated and we will make it public - response = private_issue_moderation(payload) + response = private_issue_moderation(issue_info) if response.status_code == 200: return ('Moderated issue accepted', 200, {'Content-Type': 'text/plain'}) From a1b4c1dfca557bab72a60fa5908c93e383cbf94b Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Fri, 24 Jan 2020 16:53:01 +0900 Subject: [PATCH 12/22] Issue #3140 - Adjusts the model for sending information Basically we have a structured dictionary which contains all the info we need. This will be sent to the repo for updating the public issue. There's still a bit of data massaging to make it proper in `private_issue_moderation` (Probably once this is all done, we would need to rebase all of this or create a new branch with a proper set of commits.) --- tests/unit/test_webhook.py | 75 +++++++++++++++++++++++++++---- webcompat/webhooks/helpers.py | 85 ++++++++++++++++++++++++++++------- 2 files changed, 136 insertions(+), 24 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index ef0568e53..c3e29143a 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -44,6 +44,7 @@ def setUp(self): """Set up tests.""" # sets a more detailed message when testing. self.longMessage = True + self.maxDiff = None webcompat.app.config['TESTING'] = True self.app = webcompat.app.test_client() self.headers = {'content-type': 'application/json'} @@ -53,6 +54,7 @@ def setUp(self): + **URL**: https://www.example.com/ **Browser / Version**: Firefox 55.0 @@ -85,11 +87,51 @@ def setUp(self): **Tested Another Browser**: Yes """ # noqa - self.issue_body6 = u""" + self.issue_body6 = """ **URL**: https://not-gecko.example.com/ """ + self.issue_body7 = """ + **URL**: https://not-gecko.example.com/ + + """ + + self.issue_info1 = { + 'action': 'foobar', + 'body': '\n' + '\n' + '\n' + '\n' + '**URL**: https://www.netflix.com/', + 'domain': 'www.netflix.com', + 'number': 600, + 'original_labels': [], + 'repository_url': + 'https://api.github.com/repos/webcompat/webcompat-tests', + 'title': 'www.netflix.com - test invalid event'} + + self.issue_info2 = { + 'action': 'milestoned', + 'milestoned_with': 'accepted', + 'body': '\n' + '\n' + '\n' + '\n' + '\n' + '**URL**: https://www.netflix.com/', + 'domain': 'www.netflix.com', + 'number': 600, + 'original_labels': ['action-needsmoderation'], + 'public_url': + 'https://github.com/webcompat/webcompat-tests/issues/1', + 'repository_url': + 'https://api.github.com/repos/webcompat/webcompat-tests-private', # noqa + 'title': 'www.netflix.com - test private issue accepted'} + def tearDown(self): """Tear down tests.""" pass @@ -160,7 +202,8 @@ def test_extract_metadata(self): 'extra_labels': 'type-media, type-stylo', 'ua_header': ('Mozilla/5.0 (what) Gecko/20100101 ' 'Firefox/55.0'), - 'browser': 'Firefox 55.0'} + 'browser': 'Firefox 55.0', + 'public_url': 'https://foo.example.org/issues/1'} actual = helpers.extract_metadata(self.issue_body) self.assertEqual(expected, actual) @@ -288,7 +331,7 @@ def test_get_issue_info(self): """Extract the right information from an issue.""" json_event, signature = event_data('new_event_invalid.json') payload = json.loads(json_event) - expected = {'number': 600, 'repository_url': 'https://api.github.com/repos/webcompat/webcompat-tests', 'action': 'foobar', 'domain': 'www.netflix.com'} # noqa + expected = self.issue_info1 actual = helpers.get_issue_info(payload) self.assertDictEqual(expected, actual) @@ -296,7 +339,7 @@ def test_get_milestoned_issue_info(self): """Extract the right information for a milestoned issue.""" json_event, signature = event_data('private_milestone_accepted.json') payload = json.loads(json_event) - expected = {'number': 600, 'repository_url': 'https://api.github.com/repos/webcompat/webcompat-tests-private', 'action': 'milestoned', 'domain': 'www.netflix.com', 'milestoned_with': 'accepted'} # noqa + expected = self.issue_info2 actual = helpers.get_issue_info(payload) self.assertDictEqual(expected, actual) @@ -318,16 +361,17 @@ def test_tag_new_public_issue(self): # A 200 response json_event, signature = event_data('new_event_valid.json') payload = json.loads(json_event) + issue_info = helpers.get_issue_info(payload) with patch('webcompat.webhooks.helpers.proxy_request') as proxy: proxy.return_value.status_code = 200 - response = helpers.tag_new_public_issue(payload) + response = helpers.tag_new_public_issue(issue_info) self.assertEqual(response.status_code, 200) # A 401 response proxy.return_value.status_code = 401 proxy.return_value.content = '{"message":"Bad credentials","documentation_url":"https://developer.github.com/v3"}' # noqa with patch.dict('webcompat.webhooks.helpers.app.config', {'OAUTH_TOKEN': ''}): - response = helpers.tag_new_public_issue(payload) + response = helpers.tag_new_public_issue(issue_info) self.assertEqual(response.status_code, 401) self.assertTrue('Bad credentials' in response.content) @@ -466,12 +510,27 @@ def test_patch_wrong_repo_for_moderation(self): """ raise unittest.SkipTest('TODO') - def test_private_issue_moderated_ok(self): + @patch('webcompat.webhooks.helpers.private_issue_moderation') + def test_private_issue_moderated_ok(self, mock_proxy): """Test for private issue successfully moderated. it returns a 200 code. """ - raise unittest.SkipTest('TODO') + json_event, signature = event_data('private_milestone_accepted.json') + headers = { + 'X-GitHub-Event': 'issues', + 'X-Hub-Signature': signature, + } + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 200 + rv = c.post( + '/webhooks/labeler', + data=json_event, + headers=headers + ) + self.assertEqual(rv.data, b'Moderated issue accepted') + self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.content_type, 'text/plain') if __name__ == '__main__': diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 63de9b119..aafbb1c6c 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -145,16 +145,28 @@ def is_github_hook(request): def get_issue_info(payload): """Extract all information we need when handling webhooks for issues.""" # Extract the title and the body - title = payload.get('issue')['title'] + issue = payload.get('issue') + full_title = issue.get('title', 'Weird_Title - Inspect') + labels = issue.get('labels', []) # Create the issue dictionary - issue = {'action': payload.get('action'), - 'number': payload.get('issue')['number'], - 'domain': title.partition(' ')[0], - 'repository_url': payload.get('issue')['repository_url']} + issue_info = { + 'title': full_title, + 'action': payload.get('action'), + 'number': issue.get('number'), + 'domain': full_title.partition(' ')[0], + 'repository_url': issue.get('repository_url'), + 'body': issue.get('body'), + } + # labels on the original issue? + original_labels = [label['name'] for label in labels] + issue_info['original_labels'] = original_labels # webhook with a milestoned action if payload.get('milestone'): - issue['milestoned_with'] = payload.get('milestone')['title'] - return issue + issue_info['milestoned_with'] = payload.get('milestone')['title'] + issue_body = issue.get('body') + public_url = extract_metadata(issue_body)['public_url'] + issue_info['public_url'] = public_url.strip() + return issue_info def get_issue_labels(issue_body): @@ -176,7 +188,7 @@ def get_issue_labels(issue_body): return labelslist -def tag_new_public_issue(payload): +def tag_new_public_issue(issue_info): """Set the core actions on new opened issues. When a new issue is opened, we set a couple of things. @@ -194,14 +206,12 @@ def tag_new_public_issue(payload): "labels": ['Label1', 'Label2'] } """ - issue_body = payload.get('issue')['body'] - issue_number = payload.get('issue')['number'] + issue_body = issue_info['body'] + issue_number = issue_info['number'] # Grabs the labels already set so they will not be erased - original_labels = [label['name'] - for label in payload.get('issue')['labels']] # Gets the labels from the body labels = get_issue_labels(issue_body) - labels.extend(original_labels) + labels.extend(issue_info['original_labels']) milestone = app.config['STATUSES']['needstriage']['id'] # Preparing the proxy request headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} @@ -284,6 +294,49 @@ def msg_log(msg, issue_number): log.info(msg) -def private_issue_moderation(payload): - """Publish a private issue in public after moderation.""" - return 'boom' +def private_issue_moderation(issue_info): + """Write the private issue in public. + + When the issue has been moderated, + we need to change a couple of things in the public space + + - Title + - body + - Any labels from the private issue + + Then Send a GitHub PATCH to set labels and milestone for the issue. + + PATCH /repos/:owner/:repo/issues/:number + { + "title": "a string for the title", + "body": "the full body", + "labels": ['Label1', 'Label2'], + } + + Milestone should be already set on needstriage + + we get the destination through the public_url + """ + # Extract the relevant information + public_url = issue_info['public_url'] + body = issue_info['body'] + title = issue_info['title'] + issue_number = issue_info['number'] + # Gets the labels from the body + labels = get_issue_labels(body) + labels.extend(issue_info['original_labels']) + # Prepares the payload + payload_request = { + 'labels': labels, + 'title': title, + 'body': body} + # Preparing the proxy request + # TODO: CREATE the right destination for the URL + headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} + path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], issue_number) + proxy_response = proxy_request( + 'patch', + path, + headers=headers, + data=json.dumps(payload_request)) + return proxy_response From 8c4ea9de81f97f7688cb65e0d5d875cd5b10a5d1 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Fri, 24 Jan 2020 16:58:25 +0900 Subject: [PATCH 13/22] Issue #3140 - fix style --- webcompat/webhooks/helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index aafbb1c6c..8dab3e9e0 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -155,8 +155,7 @@ def get_issue_info(payload): 'number': issue.get('number'), 'domain': full_title.partition(' ')[0], 'repository_url': issue.get('repository_url'), - 'body': issue.get('body'), - } + 'body': issue.get('body')} # labels on the original issue? original_labels = [label['name'] for label in labels] issue_info['original_labels'] = original_labels From c4c3915cb9a06c70ed5a01e7a63fda4f6e7d0722 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 11:39:11 +0900 Subject: [PATCH 14/22] Issue #3140 - Adds a payload preparation for accepted issues This creates a separate function in charge of preparing the payload. - It removes the action-needsmoderation label - It grabs the required body - It grabs the title of the issue This will make it easier to test if/when we change what we want to send as a payload. --- tests/unit/test_webhook.py | 16 +++++++++++++ webcompat/webhooks/helpers.py | 42 ++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index c3e29143a..8751c4d6d 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -510,6 +510,22 @@ def test_patch_wrong_repo_for_moderation(self): """ raise unittest.SkipTest('TODO') + def test_prepare_accepted_issue(self): + """Test the payload preparation for accepted moderated issues.""" + actual = helpers.prepare_accepted_issue(self.issue_info2) + expected = { + 'body': '\n' + '\n' + '\n' + '\n' + '\n' + '**URL**: https://www.netflix.com/', + 'labels': ['browser-firefox', 'priority-critical', 'engine-gecko'], + 'title': 'www.netflix.com - test private issue accepted'} + self.assertEqual(expected, actual) + @patch('webcompat.webhooks.helpers.private_issue_moderation') def test_private_issue_moderated_ok(self, mock_proxy): """Test for private issue successfully moderated. diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 8dab3e9e0..eb1a81a48 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -293,28 +293,15 @@ def msg_log(msg, issue_number): log.info(msg) -def private_issue_moderation(issue_info): - """Write the private issue in public. +def prepare_accepted_issue(issue_info): + """Create the payload for the accepted moderated issue. - When the issue has been moderated, + When the issue has been moderated as accepted, we need to change a couple of things in the public space - Title - body - Any labels from the private issue - - Then Send a GitHub PATCH to set labels and milestone for the issue. - - PATCH /repos/:owner/:repo/issues/:number - { - "title": "a string for the title", - "body": "the full body", - "labels": ['Label1', 'Label2'], - } - - Milestone should be already set on needstriage - - we get the destination through the public_url """ # Extract the relevant information public_url = issue_info['public_url'] @@ -324,11 +311,34 @@ def private_issue_moderation(issue_info): # Gets the labels from the body labels = get_issue_labels(body) labels.extend(issue_info['original_labels']) + # Let's remove action-needsmoderation in case it's here + if 'action-needsmoderation' in labels: + labels.remove('action-needsmoderation') # Prepares the payload payload_request = { 'labels': labels, 'title': title, 'body': body} + return payload_request + + +def private_issue_moderation(issue_info): + """Write the private issue in public. + + Send a GitHub PATCH to set labels and milestone for the issue. + + PATCH /repos/:owner/:repo/issues/:number + { + "title": "a string for the title", + "body": "the full body", + "labels": ['Label1', 'Label2'], + } + + Milestone should be already set on needstriage + + we get the destination through the public_url + """ + payload_request = prepare_accepted_issue(issue_info) # Preparing the proxy request # TODO: CREATE the right destination for the URL headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} From f5ad195fc293fdf0504b0fd635ecf32f14aa7927 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 13:30:59 +0900 Subject: [PATCH 15/22] Issue #3140 - Isolates the test from sites.db interaction The tests call indirectly extract_priority_label which triggers a different result if the db is populated or not. So instead of having a variation, we force the return value of the priority for this specific case. --- tests/unit/test_webhook.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 8751c4d6d..95df97a71 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -510,8 +510,15 @@ def test_patch_wrong_repo_for_moderation(self): """ raise unittest.SkipTest('TODO') - def test_prepare_accepted_issue(self): - """Test the payload preparation for accepted moderated issues.""" + @patch('webcompat.webhooks.helpers.extract_priority_label') + def test_prepare_accepted_issue(self, mock_priority): + """Test the payload preparation for accepted moderated issues. + + Labels extraction will create a call to the topsites db + and return a value. If the db has not been populated, the result + will be different. So we return a value 'priority-critical' here. + """ + mock_priority.return_value = 'priority-critical' actual = helpers.prepare_accepted_issue(self.issue_info2) expected = { 'body': '\n' From d946cd0c02b149680836fd94952b07779564e173 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 15:49:17 +0900 Subject: [PATCH 16/22] Issue #3140 - Adds a method to extract the public issue number We need the number to patch the issue at the right place. --- tests/unit/test_webhook.py | 5 +++++ webcompat/webhooks/helpers.py | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 95df97a71..e758b09ca 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -555,6 +555,11 @@ def test_private_issue_moderated_ok(self, mock_proxy): self.assertEqual(rv.status_code, 200) self.assertEqual(rv.content_type, 'text/plain') + def test_get_public_issue_number(self): + """Test the extraction of the issue number from the public_url.""" + public_url = 'https://github.com/webcompat/webcompat-tests/issues/1' + self.assertEqual(helpers.get_public_issue_number(public_url), '1') + if __name__ == '__main__': unittest.main() diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index eb1a81a48..b1aa3db1c 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -168,6 +168,12 @@ def get_issue_info(payload): return issue_info +def get_public_issue_number(public_url): + """Extract the issue number from the public url.""" + public_number = public_url.strip().rsplit('/', 1)[1] + return public_number + + def get_issue_labels(issue_body): """Extract the list of labels from an issue body to be sent to GitHub.""" labelslist = [] From 4eddc0e5775366a2e4ba3c911f8cb92613f3e9dd Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 15:52:36 +0900 Subject: [PATCH 17/22] Issue #3140 - Adds tests on arguments for the PATCH public issue We make sure that we send the HTTP PATCH with the right arguments. So basically that we are sending the right information at the right place. A question though: do we want to send the public_url in the body? --- tests/unit/test_webhook.py | 16 ++++++++++++++++ webcompat/webhooks/helpers.py | 7 ++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index e758b09ca..d2e10d78c 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -11,6 +11,7 @@ import unittest import flask +from mock import ANY from mock import patch import webcompat @@ -555,6 +556,21 @@ def test_private_issue_moderated_ok(self, mock_proxy): self.assertEqual(rv.status_code, 200) self.assertEqual(rv.content_type, 'text/plain') + @patch('webcompat.webhooks.helpers.proxy_request') + def test_arguments_private_issue_moderation(self, mock_proxy): + """Test that we are sending the request with the right arguments.""" + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 200 + rv = helpers.private_issue_moderation(self.issue_info2) + mock_proxy.assert_called_with( + path='repos/webcompat/webcompat-tests/issues/1', + method='patch', + data=ANY, + headers=ANY) + self.assertIn( + 'www.netflix.com - test private issue accepted', + mock_proxy.call_args.kwargs['data']) + def test_get_public_issue_number(self): """Test the extraction of the issue number from the public_url.""" public_url = 'https://github.com/webcompat/webcompat-tests/issues/1' diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index b1aa3db1c..574d996c3 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -345,13 +345,14 @@ def private_issue_moderation(issue_info): we get the destination through the public_url """ payload_request = prepare_accepted_issue(issue_info) + public_number = get_public_issue_number(issue_info['public_url']) # Preparing the proxy request # TODO: CREATE the right destination for the URL headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} - path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], issue_number) + path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], public_number) proxy_response = proxy_request( - 'patch', - path, + method='patch', + path=path, headers=headers, data=json.dumps(payload_request)) return proxy_response From 67a955360a9b4a294e419ca67ea8ecfa4433f02a Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 16:11:25 +0900 Subject: [PATCH 18/22] Issue #3140 - Adds test for unkwown scope in issue moderation This double checks that the processing for issue moderation is coming from the right scope. Often probably a wrong repo. That would prevent us from a copy pasta error. --- ...private_milestone_accepted_wrong_repo.json | 23 +++++++++++++++++++ tests/unit/test_webhook.py | 19 +++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json diff --git a/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json b/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json new file mode 100644 index 000000000..019443827 --- /dev/null +++ b/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json @@ -0,0 +1,23 @@ +{ + "action": "milestoned", + "issue": { + "title": "www.netflix.com - test private issue accepted", + "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests-foobar", + "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" + } + ] + }, + "milestone": { + "title": "accepted" + } +} diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index d2e10d78c..2fabfdf16 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -502,14 +502,29 @@ def test_patch_not_acceptable_issue(self): """ raise unittest.SkipTest('TODO') - def test_patch_wrong_repo_for_moderation(self): + @patch('webcompat.webhooks.helpers.private_issue_moderation') + def test_patch_wrong_repo_for_moderation(self, mock_proxy): """Test for issues in the wrong repo. payload: 'Wrong repository' status: 403 content-type: text/plain """ - raise unittest.SkipTest('TODO') + json_event, signature = event_data( + 'private_milestone_accepted_wrong_repo.json') + headers = { + 'X-GitHub-Event': 'issues', + 'X-Hub-Signature': signature, + } + with webcompat.app.test_client() as c: + rv = c.post( + '/webhooks/labeler', + data=json_event, + headers=headers + ) + self.assertEqual(rv.data, b'Wrong repository') + self.assertEqual(rv.status_code, 403) + self.assertEqual(rv.content_type, 'text/plain') @patch('webcompat.webhooks.helpers.extract_priority_label') def test_prepare_accepted_issue(self, mock_priority): From e4aa9d22eae8ab99541c659b7e5c0fccbba0c0b1 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Mon, 3 Feb 2020 16:31:13 +0900 Subject: [PATCH 19/22] Issues #3140 - Adds the code skeleton for rejected Issues This just put in place references to the code. The rest of this could be addressed in this PR or in a specific PR with commits in #3141 --- tests/unit/test_webhook.py | 4 +++- webcompat/webhooks/helpers.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 2fabfdf16..e23e7855b 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -499,8 +499,10 @@ def test_patch_not_acceptable_issue(self): payload: 'Moderated issue rejected' status: 200 content-type: text/plain + + A rejected issue is a private issue which has been closed. """ - raise unittest.SkipTest('TODO') + raise unittest.SkipTest('TODO: private_issue_rejected. See #3141') @patch('webcompat.webhooks.helpers.private_issue_moderation') def test_patch_wrong_repo_for_moderation(self, mock_proxy): diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 574d996c3..6b8d6eddc 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -271,6 +271,16 @@ def process_issue_action(issue_info): else: msg_log('private:moving to public failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) + elif (scope == 'private' and issue_info['status'] == 'closed'): + # private issue has been closed. It is rejected + # We need to patch with a template. + response = private_issue_rejected(issue_info) + if response.status_code == 200: + return ('Moderated issue rejected', + 200, {'Content-Type': 'text/plain'}) + else: + msg_log('private:moving to public failed', issue_number) + return ('ooops', 400, {'Content-Type': 'text/plain'}) else: return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) @@ -356,3 +366,10 @@ def private_issue_moderation(issue_info): headers=headers, data=json.dumps(payload_request)) return proxy_response + + +def private_issue_rejected(issue_info): + """Send a rejected moderattion PATCH on the public issue.""" + # TODO: reuse a modified version (with parameters) + # of webcompat.issues.moderation_template + pass From dbd4d7393ef1d3693f178a91ec66860d0d9f1773 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Tue, 4 Feb 2020 12:49:12 +0900 Subject: [PATCH 20/22] Issue #3141 - Refactor the moderation template function continuation of #3140 for handling issue moderation. The moderation template now handles two keywords ongoing and rejected. This will help to adjust the code sent to patch the issue. --- tests/unit/test_issues.py | 23 ++++++++++++++++++++++- webcompat/issues.py | 33 +++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_issues.py b/tests/unit/test_issues.py index 9adf74057..b041f9b95 100644 --- a/tests/unit/test_issues.py +++ b/tests/unit/test_issues.py @@ -82,7 +82,28 @@ def test_moderation_template(self): - must be a dictionary - must contain the keys: title, body """ - actual = moderation_template() + actual = moderation_template('ongoing') self.assertIs(type(actual), dict) self.assertIn('title', actual.keys()) self.assertIn('body', actual.keys()) + + def test_moderation_template_rejected(self): + """Check the return values are for the rejected case.""" + actual = moderation_template('rejected') + self.assertEqual(actual['title'], 'Issue rejected.') + self.assertIn('Its original content has been deleted', actual['body']) + + def test_moderation_template_ongoing(self): + """Check the return values are for the needsmoderation case.""" + # test the default + actual = moderation_template() + self.assertEqual(actual['title'], 'In the moderation queue.') + self.assertIn('put in the moderation queue.', actual['body']) + # test the keyword + actual = moderation_template('ongoing') + self.assertEqual(actual['title'], 'In the moderation queue.') + self.assertIn('put in the moderation queue.', actual['body']) + # bad keyword, we go back to the default. + actual = moderation_template('punkcat') + self.assertEqual(actual['title'], 'In the moderation queue.') + self.assertIn('put in the moderation queue.', actual['body']) diff --git a/webcompat/issues.py b/webcompat/issues.py index 1be82074a..800ac910f 100644 --- a/webcompat/issues.py +++ b/webcompat/issues.py @@ -23,19 +23,36 @@ PRIVATE_REPO_URI = app.config['PRIVATE_REPO_URI'] PRIVATE_REPO_MILESTONE = app.config['PRIVATE_REPO_MILESTONE'] - -def moderation_template(): - """Gets the placeholder data to send for unmoderated issues.""" - - summary = 'In the moderation queue.' - body = '''

This issue has been put in the moderation queue. A human +REJECTED_TITLE = 'Issue rejected.' +REJECTED_BODY = '''

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

''' +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 acceptable use guidelines. 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.

''' - return {'title': summary, 'body': body} + + +def moderation_template(choice='ongoing'): + """Gets the placeholder data to send for unmoderated issues. + + The moderation is for now two types: + - ongoing: the issue is in the moderation queue. + - rejected: the issue has been rejected. + + The default is 'ongoing' even with unknown keywords. + """ + if choice == 'rejected': + title = REJECTED_TITLE + body = REJECTED_BODY + else: + title = ONGOING_TITLE + body = ONGOING_BODY + return {'title': title, 'body': body} def report_private_issue(form, public_url): @@ -62,7 +79,7 @@ def report_public_issue(): Returns a requests.Response object. """ path = 'repos/{0}'.format(REPO_URI) - public_data = moderation_template() + public_data = moderation_template('ongoing') # We add action-needsmoderation label, so reviewers can filter out public_data['labels'] = ['action-needsmoderation'] return proxy_request('post', path, data=json.dumps(public_data)) From fab54be8a38e2179fcccb3704fab1c1695ba7b40 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Tue, 4 Feb 2020 15:58:51 +0900 Subject: [PATCH 21/22] Issue #3141 - Adds test and methods for rejected issues This should be the final patch for the new anonymous workflow. When the issue has been moderated as rejected, we need to change a couple of things in the public space - change Title - change body - close the issue - remove the action-needsmoderation label - change the milestone to invalid This is the companion for #3140 Once this is merged on staging, we can activate #3155 on the private test repo, then on the public repo. --- .../fixtures/webhooks/new_event_invalid.json | 3 +- tests/fixtures/webhooks/new_event_valid.json | 3 +- .../webhooks/private_milestone_accepted.json | 3 +- ...private_milestone_accepted_wrong_repo.json | 3 +- .../webhooks/private_milestone_closed.json | 24 ++++++ tests/fixtures/webhooks/wrong_repo.json | 3 +- tests/unit/test_webhook.py | 75 ++++++++++++++++++- webcompat/webhooks/helpers.py | 45 +++++++++-- 8 files changed, 144 insertions(+), 15 deletions(-) create mode 100644 tests/fixtures/webhooks/private_milestone_closed.json diff --git a/tests/fixtures/webhooks/new_event_invalid.json b/tests/fixtures/webhooks/new_event_invalid.json index eeca4bd4b..c25e65890 100644 --- a/tests/fixtures/webhooks/new_event_invalid.json +++ b/tests/fixtures/webhooks/new_event_invalid.json @@ -4,6 +4,7 @@ "title": "www.netflix.com - test invalid event", "repository_url": "https://api.github.com/repos/webcompat/webcompat-tests", "number": 600, - "body": "\n\n\n\n**URL**: https://www.netflix.com/" + "body": "\n\n\n\n**URL**: https://www.netflix.com/", + "state": "open" } } diff --git a/tests/fixtures/webhooks/new_event_valid.json b/tests/fixtures/webhooks/new_event_valid.json index 69c230491..8dcef64af 100644 --- a/tests/fixtures/webhooks/new_event_valid.json +++ b/tests/fixtures/webhooks/new_event_valid.json @@ -15,6 +15,7 @@ "default": false, "description": "issue in the process of being moderated" } - ] + ], + "state": "open" } } diff --git a/tests/fixtures/webhooks/private_milestone_accepted.json b/tests/fixtures/webhooks/private_milestone_accepted.json index dbb7e0a21..496fc4b4c 100644 --- a/tests/fixtures/webhooks/private_milestone_accepted.json +++ b/tests/fixtures/webhooks/private_milestone_accepted.json @@ -15,7 +15,8 @@ "default": false, "description": "issue in the process of being moderated" } - ] + ], + "state": "open" }, "milestone": { "title": "accepted" diff --git a/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json b/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json index 019443827..7101509a8 100644 --- a/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json +++ b/tests/fixtures/webhooks/private_milestone_accepted_wrong_repo.json @@ -15,7 +15,8 @@ "default": false, "description": "issue in the process of being moderated" } - ] + ], + "state": "open" }, "milestone": { "title": "accepted" diff --git a/tests/fixtures/webhooks/private_milestone_closed.json b/tests/fixtures/webhooks/private_milestone_closed.json new file mode 100644 index 000000000..7ca3e8eb7 --- /dev/null +++ b/tests/fixtures/webhooks/private_milestone_closed.json @@ -0,0 +1,24 @@ +{ + "action": "closed", + "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": "closed" +}, + "milestone": { + "title": "needstriage" + } +} diff --git a/tests/fixtures/webhooks/wrong_repo.json b/tests/fixtures/webhooks/wrong_repo.json index 5ba3d2b46..b2d449df6 100644 --- a/tests/fixtures/webhooks/wrong_repo.json +++ b/tests/fixtures/webhooks/wrong_repo.json @@ -15,6 +15,7 @@ "default": false, "description": "issue in the process of being moderated" } - ] + ], + "state": "open" } } diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index e23e7855b..0a890ae21 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -100,6 +100,7 @@ def setUp(self): self.issue_info1 = { 'action': 'foobar', + 'state': 'open', 'body': '\n' '\n' @@ -115,6 +116,7 @@ def setUp(self): self.issue_info2 = { 'action': 'milestoned', + 'state': 'open', 'milestoned_with': 'accepted', 'body': '\n' '\n' + '\n' + '\n' + '\n' + '\n' + '**URL**: https://www.netflix.com/', + 'domain': 'www.netflix.com', + 'number': 600, + 'original_labels': ['action-needsmoderation'], + 'public_url': + 'https://github.com/webcompat/webcompat-tests/issues/1', + 'repository_url': + 'https://api.github.com/repos/webcompat/webcompat-tests-private', # noqa + 'title': 'www.netflix.com - test private issue accepted'} + def tearDown(self): """Tear down tests.""" pass @@ -493,7 +515,8 @@ def test_patch_acceptable_issue_problem(self, mock_proxy): self.assertEqual(rv.status_code, 400) self.assertEqual(rv.content_type, 'text/plain') - def test_patch_not_acceptable_issue(self): + @patch('webcompat.webhooks.helpers.private_issue_rejected') + def test_patch_not_acceptable_issue(self, mock_proxy): """Test for rejected issues from private repo. payload: 'Moderated issue rejected' @@ -502,7 +525,21 @@ def test_patch_not_acceptable_issue(self): A rejected issue is a private issue which has been closed. """ - raise unittest.SkipTest('TODO: private_issue_rejected. See #3141') + json_event, signature = event_data('private_milestone_closed.json') + headers = { + 'X-GitHub-Event': 'issues', + 'X-Hub-Signature': signature, + } + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 200 + rv = c.post( + '/webhooks/labeler', + data=json_event, + headers=headers + ) + self.assertEqual(rv.data, b'Moderated issue rejected') + self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.content_type, 'text/plain') @patch('webcompat.webhooks.helpers.private_issue_moderation') def test_patch_wrong_repo_for_moderation(self, mock_proxy): @@ -575,7 +612,7 @@ def test_private_issue_moderated_ok(self, mock_proxy): @patch('webcompat.webhooks.helpers.proxy_request') def test_arguments_private_issue_moderation(self, mock_proxy): - """Test that we are sending the request with the right arguments.""" + """Test accepted issue request with the right arguments.""" with webcompat.app.test_client() as c: mock_proxy.return_value.status_code = 200 rv = helpers.private_issue_moderation(self.issue_info2) @@ -593,6 +630,38 @@ def test_get_public_issue_number(self): public_url = 'https://github.com/webcompat/webcompat-tests/issues/1' self.assertEqual(helpers.get_public_issue_number(public_url), '1') + @patch('webcompat.webhooks.helpers.proxy_request') + def test_arguments_private_issue_rejected(self, mock_proxy): + """Test rejected issue request with the right arguments.""" + with webcompat.app.test_client() as c: + mock_proxy.return_value.status_code = 200 + rv = helpers.private_issue_rejected(self.issue_info3) + post_data = json.loads(mock_proxy.call_args.kwargs['data']) + self.assertEqual('Issue rejected.', post_data['title']) + self.assertIn('Its original content has been deleted', + post_data['body']) + self.assertEqual([], post_data['labels']) + self.assertEqual('closed', post_data['state']) + self.assertIn('milestone', post_data) + mock_proxy.assert_called_with( + path='repos/webcompat/webcompat-tests/issues/1', + method='patch', + data=ANY, + headers=ANY) + + 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" + 'acceptable use\n' + 'guidelines. Its original content has been deleted.

', + 'labels': [], + 'milestone': 8, + 'state': 'closed', + 'title': 'Issue rejected.'} + actual = helpers.prepare_rejected_issue() + self.assertEqual(type(actual), dict) + self.assertEqual(actual, expected) + if __name__ == '__main__': unittest.main() diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 6b8d6eddc..993274591 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -18,6 +18,7 @@ from webcompat.helpers import extract_url from webcompat.helpers import proxy_request from webcompat.helpers import to_bytes +from webcompat.issues import moderation_template BROWSERS = ['blackberry', 'brave', 'chrome', 'edge', 'firefox', 'iceweasel', 'ie', 'lynx', 'myie', 'opera', 'puffin', 'qq', 'safari', 'samsung', 'seamonkey', 'uc', 'vivaldi'] # noqa MOZILLA_BROWSERS = ['browser-fenix', @@ -151,6 +152,7 @@ def get_issue_info(payload): # Create the issue dictionary issue_info = { 'title': full_title, + 'state': issue.get('state'), 'action': payload.get('action'), 'number': issue.get('number'), 'domain': full_title.partition(' ')[0], @@ -271,7 +273,7 @@ def process_issue_action(issue_info): else: msg_log('private:moving to public failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) - elif (scope == 'private' and issue_info['status'] == 'closed'): + elif (scope == 'private' and issue_info['state'] == 'closed'): # private issue has been closed. It is rejected # We need to patch with a template. response = private_issue_rejected(issue_info) @@ -279,7 +281,7 @@ def process_issue_action(issue_info): return ('Moderated issue rejected', 200, {'Content-Type': 'text/plain'}) else: - msg_log('private:moving to public failed', issue_number) + msg_log('public rejection failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) else: return ('Not an interesting hook', 403, {'Content-Type': 'text/plain'}) @@ -357,7 +359,6 @@ def private_issue_moderation(issue_info): payload_request = prepare_accepted_issue(issue_info) public_number = get_public_issue_number(issue_info['public_url']) # Preparing the proxy request - # TODO: CREATE the right destination for the URL headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], public_number) proxy_response = proxy_request( @@ -369,7 +370,37 @@ def private_issue_moderation(issue_info): def private_issue_rejected(issue_info): - """Send a rejected moderattion PATCH on the public issue.""" - # TODO: reuse a modified version (with parameters) - # of webcompat.issues.moderation_template - pass + """Send a rejected moderation PATCH on the public issue.""" + payload_request = prepare_rejected_issue() + public_number = get_public_issue_number(issue_info['public_url']) + # Preparing the proxy request + # TODO: CREATE the right destination for the URL + headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} + path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], public_number) + proxy_response = proxy_request( + method='patch', + path=path, + headers=headers, + data=json.dumps(payload_request)) + return proxy_response + + +def prepare_rejected_issue(): + """Create the payload for the rejected moderated issue. + + When the issue has been moderated as rejected, + we need to change a couple of things in the public space + + - change Title + - 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('rejected') + payload_request['labels'] = [] + payload_request['state'] = 'closed' + payload_request['milestone'] = invalid_id + return payload_request From 8ec394bf8c5ec0a118b33c20fa6b68c243bd3598 Mon Sep 17 00:00:00 2001 From: Karl Dubost Date: Wed, 5 Feb 2020 11:39:37 +0900 Subject: [PATCH 22/22] Issue #3140 - Adds a status-notacceptable label Part of #3141 This also addresses comments on the PR review --- tests/unit/test_webhook.py | 4 ++-- webcompat/webhooks/helpers.py | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_webhook.py b/tests/unit/test_webhook.py index 0a890ae21..f5023827f 100644 --- a/tests/unit/test_webhook.py +++ b/tests/unit/test_webhook.py @@ -640,7 +640,7 @@ def test_arguments_private_issue_rejected(self, mock_proxy): self.assertEqual('Issue rejected.', post_data['title']) self.assertIn('Its original content has been deleted', post_data['body']) - self.assertEqual([], post_data['labels']) + self.assertEqual(['status-notacceptable'], post_data['labels']) self.assertEqual('closed', post_data['state']) self.assertIn('milestone', post_data) mock_proxy.assert_called_with( @@ -654,7 +654,7 @@ def test_prepare_rejected_issue(self): expected = {'body': "

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

', - 'labels': [], + 'labels': ['status-notacceptable'], 'milestone': 8, 'state': 'closed', 'title': 'Issue rejected.'} diff --git a/webcompat/webhooks/helpers.py b/webcompat/webhooks/helpers.py index 993274591..99e15d43a 100644 --- a/webcompat/webhooks/helpers.py +++ b/webcompat/webhooks/helpers.py @@ -158,7 +158,7 @@ def get_issue_info(payload): 'domain': full_title.partition(' ')[0], 'repository_url': issue.get('repository_url'), 'body': issue.get('body')} - # labels on the original issue? + # If labels on the original issue, we need them. original_labels = [label['name'] for label in labels] issue_info['original_labels'] = original_labels # webhook with a milestoned action @@ -261,7 +261,6 @@ def process_issue_action(issue_info): else: msg_log('public:opened labels failed', issue_number) return ('ooops', 400, {'Content-Type': 'text/plain'}) - # TODO probably do a function. Too many conditions. elif (issue_info['action'] == 'milestoned' and scope == 'private' and issue_info['milestoned_with'] == 'accepted'): @@ -318,7 +317,7 @@ def prepare_accepted_issue(issue_info): we need to change a couple of things in the public space - Title - - body + - Body - Any labels from the private issue """ # Extract the relevant information @@ -374,7 +373,6 @@ def private_issue_rejected(issue_info): payload_request = prepare_rejected_issue() public_number = get_public_issue_number(issue_info['public_url']) # Preparing the proxy request - # TODO: CREATE the right destination for the URL headers = {'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])} path = 'repos/{0}/{1}'.format(app.config['ISSUES_REPO_URI'], public_number) proxy_response = proxy_request( @@ -400,7 +398,7 @@ def prepare_rejected_issue(): # Extract the relevant information invalid_id = app.config['STATUSES']['invalid']['id'] payload_request = moderation_template('rejected') - payload_request['labels'] = [] + payload_request['labels'] = ['status-notacceptable'] payload_request['state'] = 'closed' payload_request['milestone'] = invalid_id return payload_request