Refactor the CodebaseRelationListView #858

Signed-off-by: Thomas Druez <tdruez@nexb.com>
aboutcode-org · Aug 14, 2023 · 0677f97 · 0677f97
1 parent b5e22ff
commit 0677f97
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 96 deletions.
diff --git a/scanpipe/filters.py b/scanpipe/filters.py
@@ -334,20 +334,25 @@ def filter(self, qs, value):
         return qs
 
 
+MAP_TYPE_CHOICES = (
+    ("about_file", "about file"),
+    ("java_to_class", "java to class"),
+    ("jar_to_source", "jar to source"),
+    ("js_compiled", "js compiled"),
+    ("js_colocation", "js colocation"),
+    ("js_path", "js path"),
+    ("path", "path"),
+    ("sha1", "sha1"),
+)
+
+
 class RelationMapTypeFilter(django_filters.ChoiceFilter):
     def __init__(self, *args, **kwargs):
         kwargs["choices"] = (
             ("none", "No map"),
             ("any", "Any map"),
             ("many", "Many map"),
-            ("about_file", "about file"),
-            ("java_to_class", "java to class"),
-            ("jar_to_source", "jar to source"),
-            ("js_compiled", "js compiled"),
-            ("js_colocation", "js colocation"),
-            ("js_path", "js path"),
-            ("path", "path"),
-            ("sha1", "sha1"),
+            *MAP_TYPE_CHOICES,
         )
         super().__init__(*args, **kwargs)
 
@@ -632,3 +637,35 @@ class Meta:
             "model",
             "message",
         ]
+
+
+class RelationFilterSet(FilterSetUtilsMixin, django_filters.FilterSet):
+    dropdown_widget_fields = [
+        "status",
+        "map_type",
+    ]
+
+    search = django_filters.CharFilter(
+        label="Search",
+        field_name="to_resource__path",
+        lookup_expr="icontains",
+    )
+    sort = django_filters.OrderingFilter(
+        label="Sort",
+        fields=[
+            "from_resource",
+            "to_resource",
+            "map_type",
+        ],
+    )
+    map_type = django_filters.ChoiceFilter(choices=MAP_TYPE_CHOICES)
+    # TODO: Choices
+    status = StatusFilter(field_name="from_resource__status")
+
+    class Meta:
+        model = ProjectError
+        fields = [
+            "search",
+            "map_type",
+            "status",
+        ]
diff --git a/scanpipe/templates/scanpipe/base.html b/scanpipe/templates/scanpipe/base.html
@@ -74,7 +74,7 @@
       #project-extra-data pre {background-color: initial; color: initial; padding: initial; white-space: pre-wrap; word-break: break-all;}
       #codebase-relation-list table {table-layout: fixed;}
       #codebase-relation-list th#column-status {width: 110px;}
-      #codebase-relation-list th#column-related_from__map_type {width: 145px;}
+      #codebase-relation-list th#column-map_type {width: 145px;}
     </style>
     {% block extrahead %}{% endblock %}
   </head>

diff --git a/scanpipe/templates/scanpipe/includes/filter_dropdown_choices_field.html b/scanpipe/templates/scanpipe/includes/filter_dropdown_choices_field.html
@@ -1,7 +1,7 @@
 <div class="dropdown is-hoverable {% if is_right %}is-right{% endif %}">
   <div class="dropdown-trigger">
     <a class="{% if filter.data %}has-text-link{% else %}is-grey-link{% endif %}" aria-haspopup="true" aria-controls="{{ filter.id_for_label }}">
-      <span class="icon width-1"><i class="fa-solid fa-filter"></i></span>
+      <span class="icon width-1 height-1"><i class="fa-solid fa-filter"></i></span>
     </a>
   </div>
   <div class="dropdown-menu" id="{{ filter.id_for_label }}" role="menu">

diff --git a/scanpipe/templates/scanpipe/relation_list.html b/scanpipe/templates/scanpipe/relation_list.html
@@ -20,51 +20,36 @@
     <table class="table is-bordered is-narrow is-hoverable is-fullwidth">
       {% include 'scanpipe/includes/list_view_thead.html' %}
       <tbody>
-        {% for resource in object_list %}
-          {% for relation in resource.related_from.all %}
-            <tr>
-              {% if forloop.first %}
-                <td class="break-all" rowspan="{{ resource.related_from.all|length }}">
-                  {# CAUTION: Avoid relying on get_absolute_url to prevent unnecessary query triggers #}
-                  <a href="{% url 'resource_detail' project.slug resource.path %}#viewer">{{ resource.path }}</a>
-                </td>
+        {% for relation in object_list %}
+          <tr>
+            <td class="break-all">
+              {# CAUTION: Avoid relying on get_absolute_url to prevent unnecessary query triggers #}
+              <a href="{% url 'resource_detail' project.slug relation.to_resource.path %}#viewer">{{ relation.to_resource.path }}</a>
+            </td>
+            <td class="break-all">
+              <a href="?status={{ relation.to_resource.status }}" class="is-black-link">{{ relation.to_resource.status }}</a>
+            </td>
+            <td class="break-all">
+              <a href="?map_type={{ relation.map_type }}" class="is-black-link">{{ relation.map_type }}</a>
+              {% if relation.extra_data.path_score %}
+                {{ relation.extra_data.path_score }}
+              {% endif %}
+              {% if relation.map_type == "path" and resource.is_text and relation.from_resource.is_text %}
+                <div>
+                  <a href="{% url 'resource_diff' project.slug %}?from_path={{ resource.path }}&to_path={{ relation.from_resource.path }}" target="_blank">diff</a>
+                  {% if relation.extra_data.diff_ratio %}
+                    ratio: {{ relation.extra_data.diff_ratio }}
+                  {% endif %}
+                </div>
               {% endif %}
-              <td>
-                <a href="?status={{ resource.status }}" class="is-black-link">{{ resource.status }}</a>
-              </td>
-              <td>
-                <a href="?relation_map_type={{ relation.map_type }}" class="is-black-link">{{ relation.map_type }}</a>
-                {% if relation.extra_data.path_score %}
-                  {{ relation.extra_data.path_score }}
-                {% endif %}
-                {% if relation.map_type == "path" and resource.is_text and relation.from_resource.is_text %}
-                  <div>
-                    <a href="{% url 'resource_diff' project.slug %}?from_path={{ resource.path }}&to_path={{ relation.from_resource.path }}" target="_blank">diff</a>
-                    {% if relation.extra_data.diff_ratio %}
-                      ratio: {{ relation.extra_data.diff_ratio }}
-                    {% endif %}
-                  </div>
-                {% endif %}
-              </td>
-              <td class="break-all">
-                <a href="{% url 'resource_detail' project.slug resource.path %}#viewer">{{ relation.from_resource.path }}</a>
-              </td>
-            </tr>
-          {% empty %}
-            <tr>
-              <td class="break-all">
-                <a class="has-text-danger" href="{% url 'resource_detail' project.slug resource.path %}#viewer">{{ resource.path }}</a>
-              </td>
-              <td>
-                <a href="?status={{ resource.status }}" class="is-black-link">{{ resource.status }}</a>
-              </td>
-              <td></td>
-              <td></td>
-            </tr>
-          {% endfor %}
+            </td>
+            <td class="break-all">
+              <a href="{% url 'resource_detail' project.slug relation.from_resource.path %}#viewer">{{ relation.from_resource.path }}</a>
+            </td>
+          </tr>
         {% empty %}
           <tr>
-            <td colspan="42" class="has-text-centered p-3">
+            <td colspan="4" class="has-text-centered p-3">
               No Relations found. <a href="?">Clear search and filters</a>
             </td>
           </tr>

diff --git a/scanpipe/views.py b/scanpipe/views.py
@@ -64,12 +64,14 @@
 from scanpipe.filters import ErrorFilterSet
 from scanpipe.filters import PackageFilterSet
 from scanpipe.filters import ProjectFilterSet
+from scanpipe.filters import RelationFilterSet
 from scanpipe.filters import ResourceFilterSet
 from scanpipe.forms import AddInputsForm
 from scanpipe.forms import AddPipelineForm
 from scanpipe.forms import ArchiveProjectForm
 from scanpipe.forms import ProjectForm
 from scanpipe.forms import ProjectSettingsForm
+from scanpipe.models import CodebaseRelation
 from scanpipe.models import CodebaseResource
 from scanpipe.models import DiscoveredDependency
 from scanpipe.models import DiscoveredPackage
@@ -1226,70 +1228,44 @@ class ProjectErrorListView(
 )
 
 
+# TODO: Prefetch related and QS optimization
 class CodebaseRelationListView(
     ConditionalLoginRequired,
     ProjectRelatedViewMixin,
     TableColumnsMixin,
     ExportXLSXMixin,
     PaginatedFilterView,
 ):
-    model = CodebaseResource
-    filterset_class = ResourceFilterSet
+    model = CodebaseRelation
+    filterset_class = RelationFilterSet
     template_name = "scanpipe/relation_list.html"
     paginate_by = settings.SCANCODEIO_PAGINATE_BY.get("relation", 100)
     table_columns = [
-        {
-            "field_name": "path",
-            "label": "To resource",
-        },
+        "to_resource",
         {
             "field_name": "status",
             "filter_fieldname": "status",
         },
         {
-            "field_name": "related_from__map_type",
-            "label": "Map type",
-            "filter_fieldname": "relation_map_type",
-        },
-        {
-            "field_name": "related_from__from_resource__path",
-            "label": "From resource",
+            "field_name": "map_type",
+            "filter_fieldname": "map_type",
         },
+        "from_resource",
     ]
 
-    def get_queryset(self):
-        return (
-            super()
-            .get_queryset()
-            .files()
-            .to_codebase()
-            .has_relation()
-            .prefetch_related("related_from__from_resource__project")
-        )
-
-    def get_context_data(self, **kwargs):
-        context = super().get_context_data(**kwargs)
-        context["relation_count"] = context["filter"].qs.has_relation().count()
-        return context
-
     @staticmethod
     def get_rows(qs):
-        for resource in qs:
-            relations = resource.related_from.all()
-            if not relations:
-                yield RelationRow(resource.path, resource.status, "", "", "")
-            else:
-                for relation in resource.related_from.all():
-                    score = relation.extra_data.get("path_score", "")
-                    if diff_ratio := relation.extra_data.get("diff_ratio", ""):
-                        score += f" diff_ratio: {diff_ratio}"
-                    yield RelationRow(
-                        resource.path,
-                        resource.status,
-                        relation.map_type,
-                        score,
-                        relation.from_resource.path,
-                    )
+        for relation in qs:
+            score = relation.extra_data.get("path_score", "")
+            if diff_ratio := relation.extra_data.get("diff_ratio", ""):
+                score += f" diff_ratio: {diff_ratio}"
+            yield RelationRow(
+                relation.to_resource.path,
+                relation.to_resource.status,
+                relation.map_type,
+                score,
+                relation.from_resource.path,
+            )
 
     def export_xlsx_file_response(self):
         filtered_qs = self.filterset.qs