Refactor ProductTabVulnerabilitiesView to remove duplication #95

Signed-off-by: tdruez <tdruez@nexb.com>
aboutcode-org · Aug 30, 2024 · 1f6ed29 · 1f6ed29
1 parent bb91c1b
commit 1f6ed29
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 76 deletions.
diff --git a/dejacode/static/css/dejacode_bootstrap.css b/dejacode/static/css/dejacode_bootstrap.css
@@ -376,6 +376,20 @@ table.vulnerabilities-table .column-summary {
   max-width: 300px;
 }
 
+/* -- Vulnerability tab -- */
+#tab_vulnerabilities .column-vulnerability_id {
+  width: 210px;
+}
+#tab_vulnerabilities .column-aliases {
+  width: 210px;
+}
+#tab_vulnerabilities .column-max_score {
+  width: 105px;
+}
+#tab_vulnerabilities .column-column-affected_packages {
+  width: 320px;
+}
+
 /* -- Package Details -- */
 textarea.licenseexpressionwidget {
   height: 62px;

diff --git a/dje/templates/tabs/pagination.html b/dje/templates/tabs/pagination.html
@@ -0,0 +1,30 @@
+{% load humanize %}
+<div class="row align-items-end">
+  <div class="col mb-3">
+    <ul class="nav nav-pills">
+      <li class="nav-item">
+        <form id="tab-{{ tab_id }}-search-form" class="mt-md-0 me-sm-2">
+          <input style="width: 250px;" type="text" class="form-control form-control-sm" id="tab-{{ tab_id }}-search-input" name="{{ tab_id }}-q" placeholder="Search {{ tab_id }}" aria-label="Search" autocomplete="off" value="{{ search_query|escape }}">
+        </form>
+      </li>
+      <li class="nav-item">
+        <div class="h6 mt-2 mb-0 smaller">
+          {% if page_obj.paginator.count != total_count %}
+            {{ page_obj.paginator.count|intcomma }} of
+            <a href="#" hx-get="{{ request.path }}?all=true#{{ tab_id }}" hx-target="{{ tab_id_html }}">
+              {{ total_count }} results
+            </a>
+          {% else %}
+            {{ page_obj.paginator.count|intcomma }} results
+          {% endif %}
+        </div>
+      </li>
+    </ul>
+    <div class="mt-1">
+      {% include 'includes/filters_breadcrumbs.html' with filterset=filterset fragment=tab_id only %}
+    </div>
+  </div>
+  <div class="col-auto">
+    {% include 'pagination/object_list_pagination.html' with hx_target=tab_id_html %}
+  </div>
+</div>
diff --git a/dje/views.py b/dje/views.py
@@ -294,10 +294,11 @@ def get_context_data(self, **kwargs):
 class TableHeaderMixin:
     table_headers = ()
     model = None
-    filterset = None
+    table_model = None
 
     def get_table_headers(self):
-        opts = self.model._meta
+        model = self.table_model or self.model
+        opts = model._meta
 
         sort_fields = []
         if self.filterset and "sort" in self.filterset.filters:

diff --git a/product_portfolio/templates/product_portfolio/tabs/tab_dependencies.html b/product_portfolio/templates/product_portfolio/tabs/tab_dependencies.html
@@ -3,54 +3,24 @@
 {% load humanize %}
 
 {% spaceless %}
-<div class="row align-items-end">
-  <div class="col mb-3">
-    <ul class="nav nav-pills">
-      <li class="nav-item">
-        <form id="tab-dependencies-search-form" class="mt-md-0 me-sm-2">
-          <input style="width: 250px;" type="text" class="form-control form-control-sm" id="tab-dependencies-search-input" name="dependencies-q" placeholder="Search dependencies" aria-label="Search" autocomplete="off" value="{{ search_query|escape }}">
-        </form>
-      </li>
-      <li class="nav-item">
-        <div class="h6 mt-2 mb-0 smaller">
-          {% if page_obj.paginator.count != total_count %}
-            {{ page_obj.paginator.count|intcomma }} of
-            <a href="#" hx-get="{{ request.path }}?all=true#dependencies" hx-target="{{ tab_id_html }}">
-              {{ total_count }} results
-            </a>
-          {% else %}
-            {{ page_obj.paginator.count|intcomma }} results
-          {% endif %}
-        </div>
-      </li>
-    </ul>
-    <div class="mt-1">
-      {% include 'includes/filters_breadcrumbs.html' with filterset=filter_dependency fragment=tab_id only %}
-    </div>
-  </div>
-  <div class="col-auto">
-    {% include 'pagination/object_list_pagination.html' with hx_target=tab_id_html %}
-  </div>
-</div>
-
+{% include 'tabs/pagination.html' %}
 <table class="table table-bordered table-hover table-md text-break">
-  {% include 'includes/object_list_table_header.html' %}
   <thead>
     <tr>
       <th style="min-width: 250px">
         <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="top" title="{{ help_texts.for_package }}">
           {% trans 'For package' %}
         </span>
         <div class="float-end">
-          {{ filter_dependency.form.for_package }}
+          {{ filterset.form.for_package }}
         </div>
       </th>
       <th style="min-width: 250px">
         <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="top" title="{{ help_texts.resolved_to_package }}">
           {% trans 'Resolved to package' %}
         </span>
         <div class="float-end">
-          {{ filter_dependency.form.resolved_to_package }}
+          {{ filterset.form.resolved_to_package }}
         </div>
       </th>
       <th>
@@ -73,23 +43,23 @@
           {% trans 'Runtime' %}
         </span>
         <div class="float-end">
-          {{ filter_dependency.form.is_runtime }}
+          {{ filterset.form.is_runtime }}
         </div>
       </th>
       <th style="min-width: 100px">
         <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="top" title="{{ help_texts.is_optional }}">
           {% trans 'Optional' %}
         </span>
         <div class="float-end">
-          {{ filter_dependency.form.is_optional }}
+          {{ filterset.form.is_optional }}
         </div>
       </th>
       <th style="min-width: 88px">
         <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="top" title="{{ help_texts.is_resolved }}">
           {% trans 'Pinned' %}
         </span>
         <div class="float-end">
-          {{ filter_dependency.form.is_resolved }}
+          {{ filterset.form.is_resolved }}
         </div>
       </th>
     </tr>
@@ -163,7 +133,7 @@
       <tr>
         <td colspan="10">
           No results.
-          {% if filter_dependency.is_active %}
+          {% if filterset.is_active %}
             <a href="#" hx-get="{{ request.path }}?all=true#dependencies" hx-target="{{ tab_id_html }}">
               Clear search and filters
             </a>

diff --git a/product_portfolio/templates/product_portfolio/tabs/tab_vulnerabilities.html b/product_portfolio/templates/product_portfolio/tabs/tab_vulnerabilities.html
@@ -1,34 +1,8 @@
 {% load i18n %}
+{% include 'tabs/pagination.html' %}
+{{ table_headers }}
 <table class="table table-bordered table-hover table-md text-break">
-  <thead>
-    <tr>
-      <th style="width: 210px;">
-        <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="bottom" data-bs-title="Link to the VulnerableCode app.">
-          {% trans 'Affected by' %}
-        </span>
-      </th>
-      <th style="width: 210px;">
-        <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="bottom" data-bs-title="A list of aliases for this vulnerability.">
-          {% trans 'Aliases' %}
-        </span>
-      </th>
-      <th style="width: 90px;">
-        <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="bottom" data-bs-title="Severity score range.">
-          {% trans 'Score' %}
-        </span>
-      </th>
-      <th>
-        <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="bottom" data-bs-title="Summary of the vulnerability.">
-          {% trans 'Summary' %}
-        </span>
-      </th>
-      <th style="min-width: 320px;">
-        <span class="help_text" data-bs-toggle="tooltip" data-bs-placement="bottom" data-bs-title="Affected packages.">
-          {% trans 'Affected packages' %}
-        </span>
-      </th>
-    </tr>
-  </thead>
+  {% include 'includes/object_list_table_header.html' %}
   <tbody>
     {% for vulnerability in page_obj.object_list %}
       <tr>

diff --git a/product_portfolio/views.py b/product_portfolio/views.py
@@ -39,6 +39,7 @@
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.utils.html import format_html
+from django.utils.translation import gettext_lazy as _
 from django.views.decorators.csrf import csrf_exempt
 from django.views.decorators.http import require_POST
 from django.views.generic import DetailView
@@ -60,6 +61,7 @@
 from component_catalog.license_expression_dje import parse_expression
 from component_catalog.models import Component
 from component_catalog.models import Package
+from component_catalog.models import Vulnerability
 from dejacode_toolkit.purldb import PurlDB
 from dejacode_toolkit.scancodeio import ScanCodeIO
 from dejacode_toolkit.scancodeio import get_hash_uid
@@ -94,6 +96,7 @@
 from dje.views import SendAboutFilesView
 from dje.views import TabContentView
 from dje.views import TabField
+from dje.views import TableHeaderMixin
 from dje.views_formset import FormSetView
 from license_library.filters import LicenseFilterSet
 from license_library.models import License
@@ -126,7 +129,7 @@
 from product_portfolio.models import ScanCodeProject
 
 
-class BaseProductView:
+class BaseProductView:  # TODO: Rename this, it is a mixin
     model = Product
     slug_url_kwarg = ("name", "version")
 
@@ -1044,7 +1047,7 @@ def get_context_data(self, **kwargs):
 
         context_data.update(
             {
-                "filter_dependency": filter_dependency,
+                "filterset": filter_dependency,
                 "page_obj": page_obj,
                 "total_count": product.dependencies.count(),
                 "search_query": self.request.GET.get("dependencies-q", ""),
@@ -1068,16 +1071,25 @@ class ProductTabVulnerabilitiesView(
     LoginRequiredMixin,
     BaseProductView,
     PreviousNextPaginationMixin,
+    TableHeaderMixin,
     TabContentView,
 ):
-    # TODO: Remove duplication
+    # TODO: Remove duplication + check queries: assertMax
     template_name = "product_portfolio/tabs/tab_vulnerabilities.html"
-    paginate_by = 50
+    paginate_by = 5
     query_dict_page_param = "vulnerabilities-page"
     tab_id = "vulnerabilities"
+    table_model = Vulnerability
+    filterset_class = VulnerabilityFilterSet
+    table_headers = (
+        Header("vulnerability_id", _("Vulnerability")),
+        Header("aliases", _("Aliases")),
+        Header("max_score", _("Score"), help_text="Severity score range", filter="max_score"),
+        Header("summary", _("Summary")),
+        Header("affected_packages", _("Affected packages"), help_text="Affected product packages"),
+    )
 
     def get_context_data(self, **kwargs):
-        context_data = super().get_context_data(**kwargs)
         product = self.object
         base_vulnerability_qs = product.get_vulnerability_qs()
         total_count = base_vulnerability_qs.count()
@@ -1090,20 +1102,24 @@ def get_context_data(self, **kwargs):
             "-min_score",
         )
 
-        filter_vulnerability = VulnerabilityFilterSet(
+        # TODO: Add missing anchor
+        self.filterset = self.filterset_class(
             self.request.GET,
             queryset=vulnerability_qs,
             dataspace=product.dataspace,
             prefix=self.tab_id,
         )
 
-        paginator = Paginator(filter_vulnerability.qs, self.paginate_by)
+        # The self.filterset needs to be set before calling super()
+        context_data = super().get_context_data(**kwargs)
+
+        paginator = Paginator(self.filterset.qs, self.paginate_by)
         page_number = self.request.GET.get(self.query_dict_page_param)
         page_obj = paginator.get_page(page_number)
 
         context_data.update(
             {
-                "filter_vulnerability": filter_vulnerability,
+                "filterset": self.filterset,
                 "page_obj": page_obj,
                 "total_count": total_count,
                 "search_query": self.request.GET.get("vulnerabilities-q", ""),