Merge pull request #2658 from /issues/2653/1

Fixes #2653. Encode details as JSON before filling input

Author: karlcow

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

@@ -103,6 +103,12 @@ def get_form(form_data):
     ua_header = form_data['user_agent']
     # Populate the form
     bug_form.browser.data = get_browser(ua_header)
+    # Note: The details JSON that was POSTed to the new issue endpoint is at
+    # this point a Python dict. We need to re-serialize to JSON when we store
+    # its value in the hidden details form element, otherwise when we attempt
+    # to decode it as JSON on form submission, it will throw (because Python
+    # dicts are not valid JSON)
+    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')
@@ -111,32 +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):