Issue #2653. Add console logs to details section, and update tests

This changes the way we build details, and updates tests accordingly.

In build_details, we first attempt to parse the details argument as JSON.
If successful, we pull out the `consoleLog` key and store it to be used
to build the console logs section. If JSON parsing failed, we assume we
received a (legacy) string, and pass that into get_details.

Inside of get_details, if we're able to access the items() member of the
details dict, we return the formatted string as a group of <li>s, otherwise
if we get an AttributeError (which happens for a string, or other non-dict
type), we just return the string as-is.

When building the console section of details, if there are not any logs,
it will just render as an empty string. But if there are logs, we return a
new section with the content inside a <pre> tag. This is handled by the new
get_console_section method.

Author: miketaylr

tests/unit/test_form.py

@@ -196,24 +196,46 @@ def test_build_formdata(self):
         self.assertEqual(actual, expected)
 
     def test_get_details(self):
-        """Assert we handle valid JSON and other values."""
+        """Assert we handle valid dict and other values."""
         actual_string_arg = form.get_details('cool')
         expected_string_arg = 'cool'
         self.assertEqual(actual_string_arg, expected_string_arg)
-        actual_json_arg = form.get_details(json.dumps({'a': 'b', 'c': False}))
-        expected_json_arg = '<li>a: b</li><li>c: false</li>'
-        self.assertEqual(actual_json_arg, expected_json_arg)
+        actual_dict_arg = form.get_details({'a': 'b', 'c': False})
+        expected_dict_arg = '<li>a: b</li><li>c: false</li>'
+        self.assertEqual(actual_dict_arg, expected_dict_arg)
 
     def test_build_details(self):
         """Expected HTML is returned for a json object or a string."""
         actual_json_arg = form.build_details(json.dumps(
             {'a': 'b', 'c': False}))
-        expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  <li>a: b</li><li>c: false</li>\n</ul>\n</details>'  # nopep8
+        expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>'  # nopep8
         self.assertEqual(actual_json_arg, expected_json_arg)
-        actual_string_arg = form.build_details("cool")
-        expected_string_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  cool\n</ul>\n</details>'  # nopep8
+        actual_string_arg = form.build_details('cool')
+        expected_string_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  cool\n</ul>\n\n</details>'  # nopep8
         self.assertEqual(actual_string_arg, expected_string_arg)
 
+    def test_build_details_with_console_logs(self):
+        """Expected HTML is returned for a json object with console logs."""
+        actual_json_arg = form.build_details(json.dumps(
+            {'a': 'b', 'c': False, 'consoleLog': ['console.log(hi)']}))
+        expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  <li>a: b</li><li>c: false</li>\n</ul>\n<p>Console Messages:</p>\n<pre>\n[u\'console.log(hi)\']\n</pre>\n</details>'  # nopep8
+        self.assertEqual(actual_json_arg, expected_json_arg)
+        actual_empty_log_arg = form.build_details(json.dumps(
+            {'a': 'b', 'c': False, 'consoleLog': ''}))
+        expected_empty_log_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n  <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>'  # nopep8
+        self.assertEqual(actual_empty_log_arg, expected_empty_log_arg)
+
+    def test_get_console_section(self):
+        """Assert we return an empty string, or a pre with console messages."""
+        actual_empty_arg = form.get_console_section('')
+        expected_empty_arg = ''
+        self.assertEqual(actual_empty_arg, expected_empty_arg)
+        actual_none_arg = form.get_console_section(None)
+        self.assertEqual(actual_none_arg, expected_empty_arg)
+        actual_stringy_arg = form.get_console_section('sup')
+        expected_stringy_arg = '<p>Console Messages:</p>\n<pre>\nsup\n</pre>'
+        self.assertEqual(actual_stringy_arg, expected_stringy_arg)
+
     def test_is_valid_issue_form(self):
         """Assert that we get the form parameters we want."""
         incomplete_form = MultiDict([('problem_category', u'unknown_bug')])

webcompat/form.py

@@ -117,31 +117,61 @@ def get_form(form_data):
     return bug_form
 
 
-def get_details(details_string):
+def get_details(details):
     """Return details content.
 
-    * If JSON as a formatted string
+    * If a dict, as a formatted string
     * Otherwise as a string as-is.
     """
-    content = details_string
+    content = details
     rv = ''
     try:
-        details = json.loads(content)
         rv = ''.join(['<li>{k}: {v}</li>'.format(k=k, v=get_str_value(v))
                       for k, v in details.items()])
-    except ValueError:
+    except AttributeError:
         return content
     return rv
 
 
+def get_console_section(console_logs):
+    """Return a section for console logs, or the empty string.
+
+    This populates the named argument `{console_section}`
+    inside the formatted string that `build_details` returns.
+    """
+    if not console_logs:
+        return ''
+    return """<p>Console Messages:</p>
+<pre>
+{console_logs}
+</pre>""".format(console_logs=console_logs)
+
+
 def build_details(details):
-    """Populate and return the Browser Configuration section template."""
+    """Populate and return the Browser Configuration section template.
+
+    If we get JSON, we try to pull out the console logs before building the
+    rest of the details.
+    """
+
+    console_logs = None
+    content = details
+    try:
+        content = json.loads(details)
+        console_logs = content.pop('consoleLog', None)
+    except ValueError:
+        # if we got a ValueError, details was a string, so just pass it
+        # into get_details below
+        pass
+
     return """<details>
 <summary>Browser Configuration</summary>
 <ul>
   {details_list_items}
 </ul>
-</details>""".format(details_list_items=get_details(details))
+{console_section}
+</details>""".format(details_list_items=get_details(content),
+                     console_section=get_console_section(console_logs))
 
 
 def get_radio_button_label(field_value, label_list):