squash assorted bugs (#871)

* update

* add jinja2 requirements

* fix form validation where fields are not submitted

* fix url errors for invalid responses

* fix issues with bad autocomplete requests

* fix audit urls

* fix missing pubmed initials (PMID 31695588)

* fix empty dataframe

* use built-in django exception instead of custom

* raise permission denied if we cannot determine assessment

* use correct jinja2 version

* catch comma ordering - carltongibson/django-filter#1598

* check methods on admin htmx views

* stricter absolute URLs -> trailing slash required and no extensions

* ruff: don't auto-delete unused variables (but fail linting)

* add tests

* skip buggy test

compose/example.env

@@ -31,6 +31,8 @@ HAWC_SESSION_DURATION=604800
 HAWC_ANYONE_CAN_CREATE_ASSESSMENTS=True
 HAWC_PM_CAN_MAKE_PUBLIC=True
 HAWC_LOAD_TEST_DB=0
+HAWC_FEATURE_FLAGS='{}'
+HAWC_CSRF_TRUSTED_ORIGINS=https://hawc.com
 
 # google tag manager
 GTM_ID=GTM-0000000

hawc/apps/assessment/actions/audit.py

@@ -239,10 +239,11 @@ def get_riskofbias_queryset(self):
         return domain_qs | metric_qs | score_qs
 
     def get_queryset(self):
-        audit_type = self.type
-        if audit_type == AuditType.EPI:
+        audit_type = self.type.value
+        if audit_type == "epi":
             audit_type = "epiv1" if self.assessment.epi_version == EpiVersion.V1 else "epiv2"
-        qs = getattr(self, f"get_{audit_type.value}_queryset")()
+        method = f"get_{audit_type}_queryset"
+        qs = getattr(self, method)()
         return qs.select_related("content_type", "revision")
 
     def export(self) -> FlatExport:

hawc/apps/assessment/api/viewsets.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 
 from django.apps import apps
+from django.core.exceptions import ImproperlyConfigured
 from django.db import transaction
 from django.db.models import Count, Model
 from django.http import Http404
@@ -14,7 +15,6 @@
 
 from ....services.epa.dsstox import RE_DTXSID
 from ...common.api import CleanupBulkIdFilter, DisabledPagination, ListUpdateModelMixin
-from ...common.exceptions import ClassConfigurationException
 from ...common.helper import FlatExport, re_digits
 from ...common.renderers import PandasRenderers
 from ...common.views import bulk_create_object_log, create_object_log
@@ -113,7 +113,7 @@ def get_object_checks(self, serializer):
 
         # ensure we have at least one object to check
         if len(objects) == 0:
-            raise ClassConfigurationException("Permission check required; nothing to check")
+            raise ImproperlyConfigured("Permission check required; nothing to check")
 
         return objects
 

hawc/apps/assessment/forms.py

@@ -152,7 +152,7 @@ def __init__(self, *args, **kwargs):
     def clean(self) -> dict:
         cleaned_data = super().clean()
         # set None to an empty dict; see https://code.djangoproject.com/ticket/27697
-        if cleaned_data["extra"] is None:
+        if cleaned_data.get("extra") is None:
             cleaned_data["extra"] = dict()
         return cleaned_data
 
@@ -227,7 +227,7 @@ def clean(self):
         cleaned_data = super().clean()
 
         # set None to an empty dict; see https://code.djangoproject.com/ticket/27697
-        if cleaned_data["extra"] is None:
+        if cleaned_data.get("extra") is None:
             cleaned_data["extra"] = dict()
 
         evaluation_type = cleaned_data.get("evaluation_type")

hawc/apps/assessment/models.py

@@ -22,6 +22,7 @@
 
 from hawc.services.epa.dsstox import DssSubstance
 
+from ..common.exceptions import AssessmentNotFound
 from ..common.helper import HAWCDjangoJSONEncoder, SerializerHelper, new_window_a
 from ..common.models import get_private_data_storage
 from ..common.validators import FlatJSON, validate_hyperlink
@@ -746,6 +747,8 @@ def get_dict(self):
         }
 
     def get_assessment(self):
+        if self.content_object is None:
+            raise AssessmentNotFound()
         return self.content_object.get_assessment()
 
 

hawc/apps/common/autocomplete/views.py

@@ -1,4 +1,5 @@
 from dal import autocomplete
+from django.core.exceptions import BadRequest, FieldError
 from django.http import HttpResponseForbidden
 
 from ..helper import reverse_with_query_lazy
@@ -54,7 +55,10 @@ def get_base_queryset(cls, filters: dict | None = None):
 
     def get_queryset(self):
         # get base queryset
-        qs = self.get_base_queryset(self.request.GET)
+        try:
+            qs = self.get_base_queryset(self.request.GET)
+        except ValueError:
+            raise BadRequest("Invalid filter parameters")
 
         # check forwarded values for search_fields
         self.search_fields = self.forwarded.get("search_fields") or self.search_fields
@@ -64,7 +68,10 @@ def get_queryset(self):
 
         if self.field:
             # order by field and get distinct
-            qs = qs.order_by(self.field).distinct(self.field)
+            try:
+                qs = qs.order_by(self.field).distinct(self.field)
+            except FieldError:
+                raise BadRequest("Invalid field parameters")
         else:
             # check forwarded values for ordering
             self.order_by = self.forwarded.get("order_by") or self.order_by

hawc/apps/common/exceptions.py

@@ -1,6 +1,4 @@
-class ClassConfigurationException(Exception):
-    """
-    Raised when a programmer has misconfigured something; should not occur in normal application lifecycle.
-    """
+class AssessmentNotFound(Exception):
+    """Raise when an assessment cannot be determined"""
 
     pass

hawc/apps/common/htmx.py

@@ -17,6 +17,7 @@
 
 from ..assessment.models import Assessment
 from ..assessment.permissions import AssessmentPermissions
+from .exceptions import AssessmentNotFound
 from .views import create_object_log
 
 
@@ -58,39 +59,49 @@ def is_htmx(request) -> bool:
 
 
 # Permissions function checks for @action decorator
-def allow_any(user, item: Item) -> bool:
+def allow_any(user, item: Item | None) -> bool:
     """Allow any request"""
     return True
 
 
-def can_view(user, item: Item) -> bool:
+def deny_all(user, item: Item | None) -> bool:
+    """Deny all requests"""
+    return False
+
+
+def staff_only(user, item: Item | None) -> bool:
+    """Staff only requests"""
+    return user.is_staff
+
+
+def can_view(user, item: Item | None) -> bool:
     """Can a user view this hawc item? Equivalent to a reviewer on non-public assessments"""
-    return item.permissions(user)["view"]
+    return item.permissions(user)["view"] if item else False
 
 
-def can_edit(user, item: Item) -> bool:
+def can_edit(user, item: Item | None) -> bool:
     """Can a user edit this item? Equivalent to assessment team-member or higher"""
-    return item.permissions(user)["edit"]
+    return item.permissions(user)["edit"] if item else False
 
 
-def can_edit_assessment(user, item: Item) -> bool:
+def can_edit_assessment(user, item: Item | None) -> bool:
     """Can a user edit this item? Equivalent to project-manager or higher"""
-    return item.permissions(user)["edit_assessment"]
+    return item.permissions(user)["edit_assessment"] if item else False
 
 
-def action(permission: Callable, htmx_only: bool = True, methods: Iterable[str] | None = None):
+def action(
+    permission: Callable = deny_all, htmx_only: bool = True, methods: Iterable[str] = ("get",)
+):
     """Decorator for an HtmxViewSet action method
 
-    Influenced by django-rest framework's action decorator on viewsets; permissions checking that
+    Influenced by django-rest framework's ViewSet action decorator; permissions checking that
     the user making the request can make this request, and the request is valid.
 
     Args:
-        permission (Callable): A permssion function that returns True/False
+        permission (Callable, optional, default deny all): A permission function; returns True/False
         htmx_only (bool, optional, default True): Accept only htmx requests
-        methods (Optional[Iterable[str]]): Accepted http methods; defaults to {"get"} if undefined.
+        methods (Iterable[str]): Accepted http methods; defaults to ("get",)
     """
-    if methods is None:
-        methods = {"get"}
 
     def actual_decorator(func):
         @wraps(func)
@@ -102,7 +113,7 @@ def wrapper(view, request, *args, **kwargs):
             if request.method.lower() not in methods:
                 return HttpResponseNotAllowed("Invalid HTTP method")
             # check permissions
-            if not permission(request.user, request.item):
+            if not permission(request.user, getattr(request, "item", None)):
                 raise PermissionDenied()
             return func(view, request, *args, **kwargs)
 
@@ -140,7 +151,10 @@ def get_item(self, request: HttpRequest) -> Item:
             parent = get_object_or_404(self.parent_model, pk=self.kwargs.get(self.pk_url_kwarg))
         else:
             object = get_object_or_404(self.model, pk=self.kwargs.get(self.pk_url_kwarg))
-        assessment: Assessment = parent.get_assessment() if parent else object.get_assessment()
+        try:
+            assessment: Assessment = parent.get_assessment() if parent else object.get_assessment()
+        except AssessmentNotFound:
+            raise PermissionDenied()
         return Item(object=object, parent=parent, assessment=assessment)
 
     def get_context_data(self, **kwargs):

hawc/apps/common/validators.py

@@ -113,6 +113,8 @@ def validate_hyperlink(value: str, raise_exception: bool = True) -> bool:
     valid = url.scheme in valid_scheme and (
         url.netloc == "" or any(url.netloc.endswith(ending) for ending in valid_netloc_endings)
     )
+    if url.netloc == "" and ("." in url.path or not url.path.endswith("/")):
+        valid = False
     if raise_exception and not valid:
         raise ValidationError(f"Invalid hyperlink: {value}")
     return valid

hawc/apps/common/views.py

@@ -7,7 +7,7 @@
 from django.contrib import messages
 from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.contrib.auth.decorators import login_required, user_passes_test
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import FieldError, PermissionDenied
 from django.db import models, transaction
 from django.forms.models import model_to_dict
 from django.http import HttpRequest, HttpResponseRedirect
@@ -719,7 +719,11 @@ def filterset(self):
         return self._filterset
 
     def get_queryset(self):
-        return self.filterset.qs
+        try:
+            return self.filterset.qs
+        except FieldError:
+            # TODO - remove try/except after https://github.com/carltongibson/django-filter/pull/1598
+            return super().get_queryset().none()
 
     def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
         context = super().get_context_data(**kwargs)

hawc/apps/hawc_admin/templates/admin/fragments/assessment_growth.html

@@ -1,6 +1,6 @@
 <h2>Top assessments</h2>
 <p>Top 20 assessments with the most changes within in the last 180 days.</p>
-{{matrix|safe}}
+{% if matrix %}{{matrix|safe}}{% else %}<p class="errornote">Matrix cannot be generated - an error occurred.</p>{% endif %}
 
 <div style="height: 200vh">
 {% include 'admin/fragments/assessment_profile.html' %}

hawc/apps/hawc_admin/views.py

@@ -7,10 +7,9 @@
 from django.http import HttpRequest
 from django.shortcuts import render
 from django.utils.decorators import method_decorator
-from django.views.decorators.cache import cache_page
 from django.views.generic import TemplateView, View
 
-from ..common.htmx import is_htmx
+from ..common.htmx import action, staff_only
 from . import methods
 
 logger = logging.getLogger(__name__)
@@ -24,14 +23,15 @@ class Swagger(TemplateView):
 @method_decorator(staff_member_required, name="dispatch")
 class Dashboard(View):
     def dispatch(self, request, *args, **kwargs):
-        request.is_htmx = is_htmx(request)
         request.action = self.kwargs["action"]
         handler = getattr(self, request.action, self.http_method_not_allowed)
         return handler(request, *args, **kwargs)
 
+    @action(permission=staff_only, htmx_only=False)
     def index(self, request: HttpRequest, *args, **kwargs):
         return render(request, "admin/dashboard.html", {})
 
+    @action(permission=staff_only, methods=("get", "post"))
     def growth(self, request, *args, **kwargs):
         form = methods.GrowthForm(data=request.POST) if request.POST else methods.GrowthForm()
         df = fig = None
@@ -40,7 +40,7 @@ def growth(self, request, *args, **kwargs):
         context = dict(form=form, fig=fig, df=df)
         return render(request, "admin/fragments/growth.html", context)
 
-    @method_decorator(cache_page(3600))
+    @action(permission=staff_only)
     def users(self, request: HttpRequest, *args, **kwargs):
         return render(
             request,
@@ -52,21 +52,25 @@ def users(self, request: HttpRequest, *args, **kwargs):
             },
         )
 
-    @method_decorator(cache_page(3600))
+    @action(permission=staff_only)
     def assessment_size(self, request: HttpRequest, *args, **kwargs):
         df = methods.size_df()
         html = df.to_html(index=False, table_id="table", escape=False, border=0)
         return render(request, "admin/fragments/assessment_size.html", {"table": html})
 
-    @method_decorator(cache_page(3600))
+    @action(permission=staff_only)
     def assessment_growth(self, request: HttpRequest, *args, **kwargs):
-        matrix = methods.growth_matrix().to_html()
+        try:
+            matrix = methods.growth_matrix().to_html()
+        except ValueError:
+            matrix = None
         return render(
             request,
             "admin/fragments/assessment_growth.html",
             {"matrix": matrix, "form": methods.AssessmentGrowthSettings()},
         )
 
+    @action(permission=staff_only, methods=("get", "post"))
     def assessment_profile(self, request: HttpRequest, *args, **kwargs):
         form = (
             methods.AssessmentGrowthSettings(data=request.POST)
@@ -82,6 +86,7 @@ def assessment_profile(self, request: HttpRequest, *args, **kwargs):
             {"form": form, "assessment": assessment, "fig": fig},
         )
 
+    @action(permission=staff_only)
     def daily_changes(self, request: HttpRequest, *args, **kwargs):
         data = methods.daily_changes()
         return render(request, "admin/fragments/changes.html", data)

hawc/apps/lit/forms.py

@@ -189,15 +189,16 @@ def validate_import_search_string(cls, search_string) -> list[int]:
 
     def clean_search_string(self):
         search_string = self.cleaned_data["search_string"]
-
         ids = self.validate_import_search_string(search_string)
-
-        if self.cleaned_data["source"] == constants.ReferenceDatabase.HERO:
+        source = self.cleaned_data.get("source")
+        if source == constants.ReferenceDatabase.HERO:
             content = models.Identifiers.objects.validate_hero_ids(ids)
             self._import_data = dict(ids=ids, content=content)
-        elif self.cleaned_data["source"] == constants.ReferenceDatabase.PUBMED:
+        elif source == constants.ReferenceDatabase.PUBMED:
             content = models.Identifiers.objects.validate_pubmed_ids(ids)
             self._import_data = dict(ids=ids, content=content)
+        else:
+            raise forms.ValidationError("Invalid  source")
 
         return search_string
 

hawc/apps/summary/models.py

@@ -214,7 +214,7 @@ def get_api_word_url(self):
 
     def get_content_schema_class(self):
         if self.table_type not in self.TABLE_SCHEMA_MAP:
-            raise NotImplementedError(f"Table type not found: {self.table_type}")
+            raise ValueError(f"Table type not found: {self.table_type}")
         return self.TABLE_SCHEMA_MAP[self.table_type]
 
     def get_table(self) -> BaseTable:

hawc/apps/summary/views.py

@@ -163,7 +163,11 @@ class SummaryTableCreate(BaseCreate):
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
-        kwargs["table_type"] = self.kwargs["table_type"]
+        try:
+            table_type = constants.TableType(self.kwargs["table_type"])
+        except ValueError:
+            raise Http404()
+        kwargs["table_type"] = table_type
         return kwargs
 
     def get_context_data(self, **kwargs):

hawc/services/nih/pubmed.py

@@ -243,10 +243,10 @@ def _authors_info(cls, tree: ET.Element, dtype) -> dict:
 
         for auth in auths:
             last = auth.find("LastName")
-            initials = auth.find("Initials")
             collective = auth.find("CollectiveName")
-            if last is not None and initials is not None:
-                names.append(normalize_author(f"{last.text} {initials.text}"))
+            if last is not None:
+                initials = cls._try_single_find(auth, "Initials")
+                names.append(normalize_author(f"{last.text} {initials}".strip()))
             elif collective is not None:
                 names.append(collective.text)
             else:

hawc/templates/admin/base.html

@@ -5,7 +5,7 @@
 {% load render_bundle from webpack_loader %}
 
 {% block extrahead %}
-    <script type="text/javascript" src="{% static 'vendor/jquery/3.6.3/jquery.min.js' %}"></script>
+    <script type="text/javascript" src="{% static 'vendor/jquery/3.7.0/jquery.min.js' %}"></script>
     <script type="text/javascript" src="{% static 'vendor/htmx/1.8.5/htmx.min.js' %}"></script>
     {% render_bundle 'main' %}
 {% endblock %}