Skip to content

Commit

Permalink
Merge pull request #3371 from karlcow/3360/1
Browse files Browse the repository at this point in the history
fix #3360 - creates controled values list for reported_with
  • Loading branch information
Mike Taylor authored Jul 6, 2020
2 parents 1dcb16f + 8a2e8c9 commit fbaba42
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
11 changes: 11 additions & 0 deletions config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,17 @@ def get_variation(variation_key, variations_dict, defaults_dict):
'type-webvr',
]

# List of accepted values for browser sources.
REPORTED_WITH = [
"addon-reporter-chrome",
"addon-reporter-firefox-mobile",
"addon-reporter-firefox",
"addon-reporter-opera",
"desktop-reporter",
"mobile-reporter",
"web-fxr",
]

# Get AB experiment variation values from the environement.
AB_VARIATIONS = {
'V1_VARIATION': os.environ.get('V1_VARIATION', '0 100'),
Expand Down
18 changes: 17 additions & 1 deletion tests/unit/test_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import unittest

import pytest
from werkzeug.datastructures import MultiDict

import webcompat
Expand Down Expand Up @@ -120,13 +121,15 @@ def test_get_form(self):
"""Check we return the right form with the appropriate data."""
with webcompat.app.test_request_context('/issues/new'):
form_data = {'user_agent': FIREFOX_UA,
'url': 'http://example.net/'}
'url': 'http://example.net/',
'src': 'desktop-reporter'}
actual = form.get_form(form_data)
expected_browser = 'Firefox 48.0'
expected_os = 'Mac OS X 10.11'
self.assertIsInstance(actual, form.IssueForm)
self.assertEqual(actual.browser.data, expected_browser)
self.assertEqual(actual.os.data, expected_os)
self.assertEqual(actual.reported_with.data, 'desktop-reporter')

def test_get_metadata(self):
"""HTML comments need the right values depending on the keys."""
Expand Down Expand Up @@ -319,5 +322,18 @@ def test_report_issue_anonymous_fails_with_wrong_credentials(self):
self.assertEqual(rv.status_code, 400)


@pytest.mark.parametrize(
'test_input,expected',
[({}, 'web'),
({'src': ''}, 'unknown'),
({'src': 'punkCat'}, 'unknown'),
({'src': 'mobile-reporter'}, 'mobile-reporter'),
]
)
def test_report_source(test_input, expected):
"""Extract the reporting source when valid and invalid."""
assert form.extract_report_source(test_input) == expected


if __name__ == '__main__':
unittest.main()
24 changes: 23 additions & 1 deletion webcompat/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def get_form(form_data, form=IssueForm):
bug_form.details.data = json.dumps(form_data.get('details'), indent=2)
bug_form.extra_labels = form_data.get('extra_labels', None)
bug_form.os.data = get_os(ua_header)
bug_form.reported_with.data = form_data.get('src', 'web')
bug_form.reported_with.data = extract_report_source(form_data)
bug_form.ua_header.data = ua_header
bug_form.url.data = form_data.get('url', None)
return bug_form
Expand Down Expand Up @@ -443,6 +443,28 @@ def add_metadata(form, metadata_dict):
return form


def extract_report_source(form_data):
"""Extract the source of report.
This also controls if the src argument is part of a known list.
"""
reported_with = 'unknown'
try:
reported_with = form_data['src']
if not is_valid_source(reported_with):
reported_with = 'unknown'
except KeyError:
reported_with = 'web'
return reported_with


def is_valid_source(reported_with):
"""Only a known subset of known values are accepted."""
if reported_with in app.config['REPORTED_WITH']:
return True
return False


def build_formdata(form_object):
"""Convert HTML form data to GitHub API data.
Expand Down

0 comments on commit fbaba42

Please sign in to comment.