Skip to content

Fixes #3464 - Only reject an issue for action==closed #3465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 24, 2020
Merged

Fixes #3464 - Only reject an issue for action==closed #3465

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b8e371c
Issue #3464 - Part 1. Only reject an issue for action==closed
Aug 21, 2020
7579506
Issue #3464 - Part 2. Rename fixture for clarity.
Aug 21, 2020
c7b1fa5
Issue #3464 - Part 3. Improve test coverage.
Aug 21, 2020
13836bb
Issue #3464 - Part 4. Update comments in process_issue_action
Aug 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions tests/fixtures/webhooks/private_milestone_closed_invalid.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
{
"action": "closed",
"issue": {
"url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/2567",
"repository_url": "https://api.github.com/repos/webcompat/webcompat-tests",
"labels_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/2567/labels{/name}",
"comments_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/2567/comments",
"events_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/2567/events",
"html_url": "https://github.com/webcompat/webcompat-tests/issues/2567",
"id": 683733083,
"node_id": "MDU6SXNzdWU2ODM3MzMwODM=",
"number": 2567,
"title": "example.com - site is not usable",
"user": {
"login": "webcompat-bot",
"id": 8862693,
"node_id": "MDQ6VXNlcjg4NjI2OTM=",
"avatar_url": "https://avatars3.githubusercontent.com/u/8862693?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/webcompat-bot",
"html_url": "https://github.com/webcompat-bot",
"followers_url": "https://api.github.com/users/webcompat-bot/followers",
"following_url": "https://api.github.com/users/webcompat-bot/following{/other_user}",
"gists_url": "https://api.github.com/users/webcompat-bot/gists{/gist_id}",
"starred_url": "https://api.github.com/users/webcompat-bot/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/webcompat-bot/subscriptions",
"organizations_url": "https://api.github.com/users/webcompat-bot/orgs",
"repos_url": "https://api.github.com/users/webcompat-bot/repos",
"events_url": "https://api.github.com/users/webcompat-bot/events{/privacy}",
"received_events_url": "https://api.github.com/users/webcompat-bot/received_events",
"type": "User",
"site_admin": false
},
"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"
},
{
"id": 964159807,
"node_id": "MDU6TGFiZWw5NjQxNTk4MDc=",
"url": "https://api.github.com/repos/webcompat/webcompat-tests/labels/browser-fixme",
"name": "browser-fixme",
"color": "ededed",
"default": false,
"description": null
}
],
"state": "closed",
"locked": false,
"assignee": null,
"assignees": [

],
"milestone": {
"url": "https://api.github.com/repos/webcompat/webcompat-tests/milestones/7",
"html_url": "https://github.com/webcompat/webcompat-tests/milestone/7",
"labels_url": "https://api.github.com/repos/webcompat/webcompat-tests/milestones/7/labels",
"id": 2744181,
"node_id": "MDk6TWlsZXN0b25lMjc0NDE4MQ==",
"number": 7,
"title": "incomplete",
"description": "Issues are closed.\r\nWe just do not have enough information for handling them.",
"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": 0,
"closed_issues": 127,
"state": "open",
"created_at": "2017-09-05T03:43:31Z",
"updated_at": "2020-08-21T18:18:15Z",
"due_on": null,
"closed_at": null
},
"comments": 0,
"created_at": "2020-08-21T18:17:04Z",
"updated_at": "2020-08-21T18:18:15Z",
"closed_at": "2020-08-21T18:18:15Z",
"author_association": "NONE",
"active_lock_reason": null,
"body": "<p>Thanks for the report. Unfortunately without any\ndetail about the issue you experienced, we cannot help with this bug.\nPlease leave a comment with more detail, or file a new report and we will\ngladly investigate this further.</p>",
"performed_via_github_app": null
},
"repository": {
"id": 17839063,
"node_id": "MDEwOlJlcG9zaXRvcnkxNzgzOTA2Mw==",
"name": "webcompat-tests",
"full_name": "webcompat/webcompat-tests",
"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",
"description": "Test issues for webcompat.com",
"fork": false,
"url": "https://api.github.com/repos/webcompat/webcompat-tests",
"forks_url": "https://api.github.com/repos/webcompat/webcompat-tests/forks",
"keys_url": "https://api.github.com/repos/webcompat/webcompat-tests/keys{/key_id}",
"collaborators_url": "https://api.github.com/repos/webcompat/webcompat-tests/collaborators{/collaborator}",
"teams_url": "https://api.github.com/repos/webcompat/webcompat-tests/teams",
"hooks_url": "https://api.github.com/repos/webcompat/webcompat-tests/hooks",
"issue_events_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/events{/number}",
"events_url": "https://api.github.com/repos/webcompat/webcompat-tests/events",
"assignees_url": "https://api.github.com/repos/webcompat/webcompat-tests/assignees{/user}",
"branches_url": "https://api.github.com/repos/webcompat/webcompat-tests/branches{/branch}",
"tags_url": "https://api.github.com/repos/webcompat/webcompat-tests/tags",
"blobs_url": "https://api.github.com/repos/webcompat/webcompat-tests/git/blobs{/sha}",
"git_tags_url": "https://api.github.com/repos/webcompat/webcompat-tests/git/tags{/sha}",
"git_refs_url": "https://api.github.com/repos/webcompat/webcompat-tests/git/refs{/sha}",
"trees_url": "https://api.github.com/repos/webcompat/webcompat-tests/git/trees{/sha}",
"statuses_url": "https://api.github.com/repos/webcompat/webcompat-tests/statuses/{sha}",
"languages_url": "https://api.github.com/repos/webcompat/webcompat-tests/languages",
"stargazers_url": "https://api.github.com/repos/webcompat/webcompat-tests/stargazers",
"contributors_url": "https://api.github.com/repos/webcompat/webcompat-tests/contributors",
"subscribers_url": "https://api.github.com/repos/webcompat/webcompat-tests/subscribers",
"subscription_url": "https://api.github.com/repos/webcompat/webcompat-tests/subscription",
"commits_url": "https://api.github.com/repos/webcompat/webcompat-tests/commits{/sha}",
"git_commits_url": "https://api.github.com/repos/webcompat/webcompat-tests/git/commits{/sha}",
"comments_url": "https://api.github.com/repos/webcompat/webcompat-tests/comments{/number}",
"issue_comment_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues/comments{/number}",
"contents_url": "https://api.github.com/repos/webcompat/webcompat-tests/contents/{+path}",
"compare_url": "https://api.github.com/repos/webcompat/webcompat-tests/compare/{base}...{head}",
"merges_url": "https://api.github.com/repos/webcompat/webcompat-tests/merges",
"archive_url": "https://api.github.com/repos/webcompat/webcompat-tests/{archive_format}{/ref}",
"downloads_url": "https://api.github.com/repos/webcompat/webcompat-tests/downloads",
"issues_url": "https://api.github.com/repos/webcompat/webcompat-tests/issues{/number}",
"pulls_url": "https://api.github.com/repos/webcompat/webcompat-tests/pulls{/number}",
"milestones_url": "https://api.github.com/repos/webcompat/webcompat-tests/milestones{/number}",
"notifications_url": "https://api.github.com/repos/webcompat/webcompat-tests/notifications{?since,all,participating}",
"labels_url": "https://api.github.com/repos/webcompat/webcompat-tests/labels{/name}",
"releases_url": "https://api.github.com/repos/webcompat/webcompat-tests/releases{/id}",
"deployments_url": "https://api.github.com/repos/webcompat/webcompat-tests/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.git",
"ssh_url": "[email protected]:webcompat/webcompat-tests.git",
"clone_url": "https://github.com/webcompat/webcompat-tests.git",
"svn_url": "https://github.com/webcompat/webcompat-tests",
"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": 1196,
"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": 1196,
"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": "Helping make the Web work in any browsers"
},
"sender": {
"login": "webcompat-bot",
"id": 8862693,
"node_id": "MDQ6VXNlcjg4NjI2OTM=",
"avatar_url": "https://avatars3.githubusercontent.com/u/8862693?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/webcompat-bot",
"html_url": "https://github.com/webcompat-bot",
"followers_url": "https://api.github.com/users/webcompat-bot/followers",
"following_url": "https://api.github.com/users/webcompat-bot/following{/other_user}",
"gists_url": "https://api.github.com/users/webcompat-bot/gists{/gist_id}",
"starred_url": "https://api.github.com/users/webcompat-bot/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/webcompat-bot/subscriptions",
"organizations_url": "https://api.github.com/users/webcompat-bot/orgs",
"repos_url": "https://api.github.com/users/webcompat-bot/repos",
"events_url": "https://api.github.com/users/webcompat-bot/events{/privacy}",
"received_events_url": "https://api.github.com/users/webcompat-bot/received_events",
"type": "User",
"site_admin": false
}
}
0 ...es/webhooks/private_milestone_closed.json → ...private_milestone_closed_unmoderated.json
View file
Open in desktop
File renamed without changes.
48 changes: 46 additions & 2 deletions tests/unit/test_webhook_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def test_process_issue_action_scenarios(mock_mr):
('wrong_repo.json', wrong_repo),
('private_milestone_accepted_wrong_repo.json', wrong_repo),
('private_milestone_accepted.json', accepted),
('private_milestone_closed.json', rejected),
('private_milestone_closed_unmoderated.json', rejected),
('private_milestone_accepted_incomplete.json', incomplete),
('private_milestone_accepted_invalid.json', invalid),
('private_milestone_accepted_closed.json', boring),
Expand Down Expand Up @@ -254,7 +254,7 @@ def test_process_issue_action_github_api_exception(mock_mr, caplog):
'comment_public_uri'),
('new_event_valid.json', 'public:opened labels failed',
'tag_as_public'),
('private_milestone_closed.json',
('private_milestone_closed_unmoderated.json',
'public rejection failed', 'close_public_issue'),
('private_milestone_accepted_invalid.json',
'private:closing public issue as invalid failed',
Expand All @@ -272,3 +272,47 @@ def test_process_issue_action_github_api_exception(mock_mr, caplog):
rv = issue.process_issue_action()
assert rv == oops
assert expected_log in caplog.text


@patch('webcompat.webhooks.model.make_request')
@patch('webcompat.webhooks.model.WebHookIssue.close_public_issue')
def test_process_issue_action_close_scenarios(mock_close, mock_mr):
"""Test 3 scenarios that will result in a closed issue

1. milestoned w/ accepted: incomplete
2. milestoned w/ accepted: invalid
3. closed as unmoderated
And ensure it gets called with the right argument.
"""
called = [
('private_milestone_closed_unmoderated.json', 'rejected'),
('private_milestone_accepted_incomplete.json', 'incomplete'),
('private_milestone_accepted_invalid.json', 'invalid'),
]
for scenario in called:
issue_payload, arg = scenario
json_event, signature = event_data(issue_payload)
payload = json.loads(json_event)
issue = WebHookIssue.from_dict(payload)
with webcompat.app.test_request_context():
issue.process_issue_action()
mock_close.assert_called_with(reason=arg)


@patch('webcompat.webhooks.model.make_request')
@patch('webcompat.webhooks.model.WebHookIssue.close_public_issue')
def test_process_issue_action_not_closed_scenarios(mock_close, mock_mr):
"""Test scenarios where close_public_issue is never called."""
not_called = [
'private_milestone_closed_invalid.json',
'new_event_valid.json',
'private_milestone_accepted_wrong_repo.json',
'private_issue_opened.json'
]
for scenario in not_called:
json_event, signature = event_data(scenario)
payload = json.loads(json_event)
issue = WebHookIssue.from_dict(payload)
with webcompat.app.test_request_context():
issue.process_issue_action()
mock_close.assert_not_called()
4 changes: 2 additions & 2 deletions webcompat/webhooks/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ def process_issue_action(self):
# we didn't get exceptions, so it's safe to close it
self.close_private_issue()
return make_response('Moderated issue closed as invalid', 200)
elif (scope == 'private' and self.state == 'closed' and
not self.milestone == 'accepted'):
elif (scope == 'private' and self.action == 'closed' and
self.milestone == 'unmoderated'):
# private issue has been closed. It is rejected
# We need to patch with a template.
try:
Expand Down