Improve the presentation of "multi-values" in DejaCode Reports #10 (#52)

Signed-off-by: tdruez <tdruez@nexb.com>

reporting/models.py

@@ -68,7 +68,7 @@
 
 LICENSE_TAG_PREFIX = "tag: "
 
-MULTIVALUE_SEPARATOR = ", "
+MULTIVALUE_SEPARATOR = "\n"
 
 ERROR_STR = "Error"
 
@@ -662,10 +662,12 @@ def _get_objects_for_field_name(self, objects, field_name, user):
             result.extend(value if isinstance(value, list) else [value])
         return result
 
-    def get_value_for_instance(self, instance, user=None):
+    def get_value_for_instance(self, instance, user=None, multi_as_list=False):
         """
         Return the value to be display in the row, given an instance
         and the current self.field_name value of this assigned field.
+        When `multi_as_list` is enabled, return the results as a list instead of
+        joining on `MULTIVALUE_SEPARATOR`.
         """
         if self.get_model_class() != instance.__class__:
             raise AssertionError("content types do not match")
@@ -674,8 +676,17 @@ def get_value_for_instance(self, instance, user=None):
         for field_name in self.field_name.split("__"):
             objects = self._get_objects_for_field_name(objects, field_name, user)
 
-        results = [str(val) for val in objects if not (len(objects) < 2 and val is None)]
-        return MULTIVALUE_SEPARATOR.join(results)
+        results = [
+            # The .strip() ensure the SafeString types are casted to regular str
+            str(val).strip()
+            for val in objects
+            if not (len(objects) < 2 and val is None)
+        ]
+
+        if multi_as_list:
+            return results
+        else:
+            return MULTIVALUE_SEPARATOR.join(results)
 
 
 class ReportQuerySet(DataspacedQuerySet):
@@ -747,23 +758,37 @@ def save(self, *args, **kwargs):
     def get_absolute_url(self):
         return reverse("reporting:report_details", args=[self.uuid])
 
-    def get_output(self, queryset=None, user=None, include_view_link=False):
+    def get_output(self, queryset=None, user=None, include_view_link=False, multi_as_list=False):
         # Checking if the parameter is given rather than the boolean value of the QuerySet
         if queryset is None:
             queryset = self.query.get_qs(user=user)
 
         icon = format_html('<i class="fas fa-external-link-alt"></i>')
         rows = []
+
         for instance in queryset:
-            cells = [
-                field.get_value_for_instance(instance, user=user)
-                for field in self.column_template.fields.all()
-            ]
+            cells = []
             if include_view_link:
-                cells.insert(
-                    0, instance.get_absolute_link(value=icon, title="View", target="_blank")
-                )
+                view_link = instance.get_absolute_link(value=icon, title="View", target="_blank")
+                cells.append(view_link)
+
+            for field in self.column_template.fields.all():
+                value = field.get_value_for_instance(instance, user, multi_as_list)
+
+                if type(value) is list:
+                    if value == []:
+                        cell_value = ""
+                    elif len(value) > 1:
+                        cell_value = value
+                    else:
+                        cell_value = value[0]
+                else:
+                    cell_value = value
+
+                cells.append(cell_value)
+
             rows.append(cells)
+
         return rows
 
 

reporting/templates/reporting/includes/report_results_table.html

@@ -1,4 +1,4 @@
-<table class="table table-striped table-bordered table-hover table-md text-break">
+<table class="table table-striped table-bordered table-hover table-md">
   <thead>
     <tr>
       {% for header in headers %}
@@ -12,9 +12,8 @@
         {% for item in list %}
           {% if format == 'xls' %}
             <td class="text">{{ item|linebreaksbr }}</td>
-          {% elif format == 'html' or not format %}
-            <td>{{ item|default:'&nbsp;'|urlize|linebreaksbr }}</td>
           {% else %}
+            {# WARNING: Using |urlize breaks the "view_link" #}
             <td>{{ item|default:'&nbsp;'|linebreaksbr }}</td>
           {% endif %}
         {% endfor %}

reporting/templates/reporting/report_run.html

@@ -86,7 +86,7 @@ <h1 class="header-title">
       </div>
     {% endif %}
     <form class="mb-4">
-      <table class="table table-striped table-bordered table-hover table-md text-break mb-2">
+      <table class="table table-striped table-bordered table-hover table-md mb-2">
         <thead>
           <tr>
             <th>Field</th>

reporting/tests/test_models.py

@@ -1196,12 +1196,12 @@ def test_get_value_for_instance_on_license_on_various_fields(self):
             ("owner__children__children", ""),
             ("owner__children__children__name", ""),
             # Many2Many
-            ("tags", "{}, {}".format(self.license_tag, license_tag2)),
-            ("tags__id", "{}, {}".format(self.license_tag.id, license_tag2.id)),
-            ("tags__show_in_license_list_view", "False, False"),
-            ("tags__default_value", "None, True"),
-            ("tags__label", "Network Redistribution, Tag2"),
-            ("tags__uuid", "{}, {}".format(self.license_tag.uuid, license_tag2.uuid)),
+            ("tags", "{}\n{}".format(self.license_tag, license_tag2)),
+            ("tags__id", "{}\n{}".format(self.license_tag.id, license_tag2.id)),
+            ("tags__show_in_license_list_view", "False\nFalse"),
+            ("tags__default_value", "None\nTrue"),
+            ("tags__label", "Network Redistribution\nTag2"),
+            ("tags__uuid", "{}\n{}".format(self.license_tag.uuid, license_tag2.uuid)),
             # Special tag property
             ("{}{}".format(LICENSE_TAG_PREFIX, self.license_tag.label), "True"),
             # Related
@@ -1342,7 +1342,7 @@ def test_get_value_for_instance_on_component_on_various_fields(self):
             ("code_view_url", ""),
             ("bug_tracking_url", ""),
             ("primary_language", ""),
-            ("keywords", "Keyword1, Keyword2"),
+            ("keywords", "Keyword1\nKeyword2"),
             ("guidance", ""),
             ("admin_notes", ""),
             ("is_active", "True"),
@@ -1694,9 +1694,9 @@ def test_get_value_for_instance_on_component_for_licenses_tags(self):
             component=self.component, license=license2, dataspace=self.dataspace
         )
 
-        expected = "True, False"
+        expected = "True\nFalse"
         self.assertEqual(expected, self.field1.get_value_for_instance(self.component))
-        expected = "License1, License2"
+        expected = "License1\nLicense2"
         self.assertEqual(expected, field2.get_value_for_instance(self.component))
 
     def test_get_value_for_instance_multi_value_ordering(self):
@@ -1735,9 +1735,9 @@ def test_get_value_for_instance_multi_value_ordering(self):
 
         expected = [license_a.key, license_b.name]
         self.assertEqual(expected, list(self.component.licenses.values_list("key", flat=True)))
-        expected = ", ".join([license_a.key, license_b.name])
+        expected = "\n".join([license_a.key, license_b.name])
         self.assertEqual(expected, field2.get_value_for_instance(self.component))
-        expected = ", ".join([str(assigned_tag_a.value), str(assigned_tag_b.value)])
+        expected = "\n".join([str(assigned_tag_a.value), str(assigned_tag_b.value)])
         self.assertEqual(expected, self.field1.get_value_for_instance(self.component))
 
     def test_column_template_with_fields_copy(self):

reporting/views.py

@@ -9,7 +9,6 @@
 import datetime
 import io
 import json
-from collections import OrderedDict
 
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.core.exceptions import FieldDoesNotExist
@@ -25,7 +24,8 @@
 from django.views.generic.detail import SingleObjectMixin
 from django.views.generic.list import MultipleObjectMixin
 
-import pyaml
+import saneyaml
+import xlsxwriter
 
 from dje.utils import get_preserved_filters
 from dje.views import AdminLinksDropDownMixin
@@ -191,38 +191,35 @@ def get_context_data(self, **kwargs):
         report = self.object
         model_class = report.column_template.get_model_class()
         # Only available in the UI since the link is relative to the current URL
-        include_view_link = hasattr(model_class, "get_absolute_url") and not self.format
+        include_view_link = not self.format and hasattr(model_class, "get_absolute_url")
+        interpolated_report_context = self.get_interpolated_report_context(request, report)
+        multi_as_list = True if self.format in ["json", "yaml"] else False
+        output = report.get_output(
+            queryset=context["object_list"],
+            user=request.user,
+            include_view_link=include_view_link,
+            multi_as_list=multi_as_list,
+        )
 
         context.update(
             {
                 "opts": self.model._meta,
                 "preserved_filters": get_preserved_filters(request, self.model),
-                "headers": report.column_template.as_headers(include_view_link=include_view_link),
+                "headers": report.column_template.as_headers(include_view_link),
                 "runtime_filter_formset": self.runtime_filter_formset,
                 "query_string_params": self.get_query_string_params(),
-                "interpolated_report_context": self.get_interpolated_report_context(
-                    request, report
-                ),
+                "interpolated_report_context": interpolated_report_context,
                 "errors": getattr(self, "errors", []),
-                "include_view_link": include_view_link,
-                "output": report.get_output(
-                    queryset=context["object_list"],
-                    user=request.user,
-                    include_view_link=include_view_link,
-                ),
+                "output": output,
             }
         )
 
         return context
 
     def get_dump(self, dumper, **dumper_kwargs):
-        """
-        Return the data dump using provided kwargs.
-        The columns order form the ColumnTemplateAssignedField sequence is respected
-        using an OrderedDict.
-        """
+        """Return the data dump using provided kwargs."""
         context = self.get_context_data(**self.kwargs)
-        data = [OrderedDict(zip(context["headers"], values)) for values in context["output"]]
+        data = [dict(zip(context["headers"], values)) for values in context["output"]]
         return dumper(data, **dumper_kwargs)
 
     def get_json_response(self, **response_kwargs):
@@ -231,17 +228,12 @@ def get_json_response(self, **response_kwargs):
         return HttpResponse(dump, **response_kwargs)
 
     def get_yaml_response(self, **response_kwargs):
-        """
-        Return serialized results as yaml content response.
-        Using pretty-yaml (pyaml) on top of PyYAML for the OrderDict support with safe_dump.
-        """
-        dump = self.get_dump(pyaml.dump, safe=True)
+        """Return serialized results as yaml content response."""
+        dump = self.get_dump(saneyaml.dump)
         return HttpResponse(dump, **response_kwargs)
 
     def get_xlsx_response(self, **response_kwargs):
         """Return the results as `xlsx` format."""
-        import xlsxwriter
-
         context = self.get_context_data(**self.kwargs)
         report_data = [context["headers"]] + context["output"]
 

setup.cfg

@@ -116,7 +116,6 @@ install_requires =
     charset-normalizer==3.1.0
     PyYAML==6.0
     Cython==0.29.30
-    pyaml==21.10.1
     importlib_metadata==4.11.4
     zipp==3.8.0
     XlsxWriter==3.1.9