Fixes #2006: Add support for multiple `label` and `details` params. by miketaylr · Pull Request #2007 · webcompat/webcompat.com

miketaylr · 2018-01-09T04:42:14Z

Our test situation seems FUBAR in Firefox 57+ right now, but these new functional tests pass in Chrome, and nosetests are good.

r? @karlcow
r? @zoepage

Sign in to view

-    return extra_label
+def extract_extra_labels(metadata_dict):
+    """Return the 'extra_labels' labels from metadata_dict, as a list."""
+    # we have to handle the ', ' foo biz here.


Sign in to view

+    extra_labels = metadata_dict.get('extra_labels', None)
+    if extra_labels:
+        extra_labels = extra_labels.split(', ')
+        print(extra_labels)


Sign in to view

+    extra_labels = extract_extra_labels(metadata_dict)
+    priority_label = extract_priority_label(issue_body)
+    labelslist = []
+    print(browser_label, extra_labels, priority_label)


Sign in to view

+    priority_label = extract_priority_label(issue_body)
+    labelslist = []
+    print(browser_label, extra_labels, priority_label)
+    for label in (browser_label, extra_labels, priority_label):


karlcow · 2018-01-09T05:14:20Z

ok let's review this.

git checkout master
git fetch upstream
git merge upstream/master
git checkout -b issues/2006/1 upstream/issues/2006/1

Let's play with the code now.

karlcow

@miketaylr First round of comments.

Sign in to view

+    priority_label = extract_priority_label(issue_body)
+    labelslist = []
+    print(browser_label, extra_labels, priority_label)
+    for label in (browser_label, extra_labels, priority_label):


Sign in to view

@@ -134,7 +135,8 @@ def test_fails_on_not_known_action(self):
        self.assertEqual(rv.data, 'Not an interesting hook')

    def test_extract_metadata(self):


Sign in to view

+        ]
+        for issue_body, expected in labels_tests:
+            actual = helpers.get_issue_labels(issue_body)
+            self.assertEqual(expected, actual)


Sign in to view

+    priority_label = extract_priority_label(issue_body)
+    labelslist = []
+    print(browser_label, extra_labels, priority_label)
+    for label in (browser_label, extra_labels, priority_label):


Sign in to view

 def normalize_metadata(metadata_value):
    """Normalize the metadata received from the form."""
+    # If it's a list, we are dealing with @extra_labels,
+    # so convert into a comma separated string.


Sign in to view

+        if request.args.get('src'):
+            session['src'] = request.args.get('src')
+        if request.args.get('label'):
+            session['src'] = request.args.getlist('label')


Sign in to view

+    extra_labels = metadata_dict.get('extra_labels', None)
+    if extra_labels:
+        extra_labels = extra_labels.split(', ')
+        print(extra_labels)


Sign in to view

+        print(extra_labels)
+        for idx, label in enumerate(extra_labels):
+            extra_labels[idx] = label.lower()
+            extra_labels[idx] = label.encode('utf-8')


Sign in to view

+        for idx, label in enumerate(extra_labels):
+            extra_labels[idx] = label.lower()
+            extra_labels[idx] = label.encode('utf-8')
+    return extra_labels


Sign in to view

        metadata_tests = [
-            ({'extra_label': 'type-media'}, 'type-media'),
+            ({'extra_labels': 'type-media'}, ['type-media']),
+            ({'extra_labels': 'cool, dude'}, ['cool', 'dude']),


karlcow · 2018-01-09T07:00:47Z

I have not reviewed the JavaScript. @zoepage do you want to do it?

zoepage · 2018-01-09T13:12:26Z

The JS looks 👍 to me.

Sign in to view

+        ]
+        for issue_body, expected in labels_tests:
+            actual = helpers.get_issue_labels(issue_body)
+            self.assertEqual(sorted(expected), sorted(actual))


miketaylr · 2018-01-09T19:55:39Z

Ready for re-review, Karl.

r? @karlcow

karlcow

@miketaylr After removing the typo, this is good to go. Congratulations.

→ nosetests -v --nocapture
Statuses Initialization…
Fetching milestones from Github…
Milestones saved in data/
Milestones in memory
Pagination in comments Link response header. ... ok
API issue for a non existent number returns JSON 404. ... ok
API access to labels without auth returns JSON 200. ... ok
Patching the issue is working only with certain circumstances. ... ok
API with wrong parameter returns JSON 404. ... ok
API setting labels without auth returns JSON 403 error code. ... ok
API access to user activity without auth returns JSON 401. ... ok
API with wrong category returns JSON 404. ... ok
API with wrong route returns JSON 404. ... ok
Check we receive the right list of browsers. ... ok
Assess the filtering is correct. ... ok
Check if a list of labels contains the 'status-needinfo'. ... ok
Check if an issue is older than a certain time gap. ... ok
The data body sent to GitHub API. ... ok
Check that domain name is extracted. ... ok
Checks we return the right form with the appropriate data. ... ok
HTML comments need the right values depending on the keys. ... ok
Check that metadata is processed and wrapped. ... ok
Avoid some type of strings. ... ok
Check that URL is normalized. ... ok
Check that appropriate radio button label is returned. ... ok
Test HTTP Links formating. ... ok
Test browser parsing via get_browser helper method. ... ok
Test browser name parsing via get_browser_name helper method. ... ok
Test name extraction from Dict via get_name helper method. ... ok
Test OS parsing via get_os helper method. ... ok
Test version string composition from Dict ... ok
Test that API params are correctly converted to Search API. ... ok
normalize_api_params shouldn't transform unknown params. ... ok
Test HTTP Links parsing for GitHub only. ... ok
test_rewrite_and_sanitize_link (tests.test_helpers.TestHelpers) ... ok
Test we're correctly rewriting the passed in link. ... ok
Test that we're removing access_token parameters. ... ok
Check Cache-Control for issues. ... ok
Check ETAG for issues. ... ok
Checks if we receive a 304 Not Modified. ... ok
Page title format for different URIs. ... ok
Testing user activity page title. ... ok
test_build_query_string (tests.test_topsites.TestTopsites) ... ok
test_build_uri (tests.test_topsites.TestTopsites) ... ok
test_node_text (tests.test_topsites.TestTopsites) ... ok
Test that Base64 screenshots return the expected status codes. ... ok
Test that /upload/ doesn't let you GET. ... ok
Test that uploaded files return the expected status code. ... ok
Test that /about exists. ... ok
Test that asks user to log in before displaying activity. ... ok
Test POST to /csp-report w/ correct content-type returns 204. ... ok
Test POST w/ wrong content-type to /csp-report returns 400. ... ok
Request to /dashboard should be 404. ... ok
Request to /dashboard/triage should be 200. ... ok
Test that the home page exists. ... ok
Test if issues are really integer. ... ok
Test that the /issues/<number> exists, and does not redirect. ... ok
Test that the /issues route gets 200 and does not redirect. ... ok
Sends 400 to POST on /issues/new with missing parameters. ... ok
Test that /issues/new exists. ... ok
/issues/new POST exit with 400 if missing parameters. ... ok
Test that /privacy exists. ... ok
Rate Limit URI sends 410 Gone. ... ok
Test that the /tools/cssfixme route gets 200. ... ok
Extract browser label name. ... ok
Extract 'extra' label. ... ok
Extract dictionary of metadata for an issue body. ... ok
Extract priority label. ... ok
POST without bogus signature on labeler webhook is forbidden. ... ok
POST with event not being 'issues' or 'ping' fails. ... ok
POST without signature on labeler webhook is forbidden. ... ok
POST with an unknown action fails. ... ok
GET is forbidden on labeler webhook. ... ok
Extract the right information from an issue. ... ok
Extract list of labels from an issue body. ... ok
Validation tests for GitHub Webhooks. ... ok
POST with PING events just return a 200 and contains pong. ... ok

----------------------------------------------------------------------
Ran 73 tests in 3.990s

OK

Sign in to view


    def test_extract_metadata(self):
-        expected = {'reported_with': 'web', 'extra_label': 'type-media',
+        """Extract dictionary of metadata for an issue body.""".


miketaylr · 2018-01-10T17:46:52Z

nose tests passing on Travis: https://travis-ci.org/webcompat/webcompat.com/builds/327334792#L639

Let's merge. My plan is to not deploy to prod until Travis is green on master though.

Issue #2006. Add support for multiple details params in bug textarea.

2e55856

miketaylr added the in progress label Jan 9, 2018

miketaylr requested review from karlcow and zoepage January 9, 2018 04:42

miketaylr commented Jan 9, 2018

View reviewed changes

Comment thread webcompat/webhooks/helpers.py Outdated

return extra_label

def extract_extra_labels(metadata_dict):

"""Return the 'extra_labels' labels from metadata_dict, as a list."""

# we have to handle the ', ' foo biz here.

This comment was marked as abuse.

Sign in to view

miketaylr commented Jan 9, 2018

View reviewed changes

Comment thread webcompat/webhooks/helpers.py Outdated

extra_labels = metadata_dict.get('extra_labels', None)

if extra_labels:

extra_labels = extra_labels.split(', ')

print(extra_labels)

This comment was marked as abuse.

Sign in to view

miketaylr commented Jan 9, 2018

View reviewed changes

Comment thread webcompat/webhooks/helpers.py Outdated

extra_labels = extract_extra_labels(metadata_dict)

priority_label = extract_priority_label(issue_body)

labelslist = []

print(browser_label, extra_labels, priority_label)

This comment was marked as abuse.

Sign in to view

miketaylr commented Jan 9, 2018

View reviewed changes

karlcow requested changes Jan 9, 2018

View reviewed changes

Issue #2006. Add functional test for multiple details params.

440bad7

miketaylr force-pushed the issues/2006/1 branch from 55036af to e8235cc Compare January 9, 2018 19:43

miketaylr added 2 commits January 9, 2018 13:50

Issue #2006. Add support for multiple label GET params.

14c2e77

Issue #2006. Add tests for extra_labels as a list.

e7ba7b6

miketaylr force-pushed the issues/2006/1 branch from f222419 to a101404 Compare January 9, 2018 19:50

miketaylr commented Jan 9, 2018

View reviewed changes

Comment thread tests/test_webhook.py

]

for issue_body, expected in labels_tests:

actual = helpers.get_issue_labels(issue_body)

self.assertEqual(sorted(expected), sorted(actual))

This comment was marked as abuse.

Sign in to view

karlcow requested changes Jan 10, 2018

View reviewed changes

Issue #2006. Add missing docstring for test method.

8f54003

miketaylr force-pushed the issues/2006/1 branch from a101404 to 8f54003 Compare January 10, 2018 17:39

zoepage approved these changes Jan 10, 2018

View reviewed changes

miketaylr merged commit 5b5e0be into master Jan 10, 2018

miketaylr deleted the issues/2006/1 branch January 10, 2018 17:46

miketaylr added status: add to changelog and removed status: add to changelog labels Jan 10, 2018

		@@ -134,7 +135,8 @@ def test_fails_on_not_known_action(self):
		self.assertEqual(rv.data, 'Not an interesting hook')

		def test_extract_metadata(self):

Conversation

miketaylr commented Jan 9, 2018

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

karlcow commented Jan 9, 2018

Uh oh!

karlcow left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

karlcow commented Jan 9, 2018

Uh oh!

zoepage commented Jan 9, 2018

Uh oh!

This comment was marked as abuse.

Uh oh!

miketaylr commented Jan 9, 2018

Uh oh!

karlcow left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

miketaylr commented Jan 10, 2018

Uh oh!

Reviewers

Assignees

Labels