Skip to content

Commit

Permalink
Issue #3360 - Adds control for reported_with
Browse files Browse the repository at this point in the history
This makes a couple of modifications

1. It uses a list of controled values coming from the config file.
2. it makes sure the extracted value is part of the controled list
3. It also records when the value is not known aka not part of the controled list. This is important to not screw the stats for the Web values AND to understand when there is a new client.

It creates the tests associated with the function.

Probably we need to convert test_form.py fully to pytest, but that should be done in another PR to not muddy this one.
  • Loading branch information
karlcow committed Jul 2, 2020
1 parent 037fd68 commit 8a2e8c9
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
14 changes: 14 additions & 0 deletions 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 @@ -321,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 8a2e8c9

Please sign in to comment.