Convert report views to functional views

src/nomnom/canonicalize/admin.py

@@ -1,6 +1,5 @@
 import contextvars
 import csv
-import functools
 import io
 from collections import Counter
 from collections.abc import Iterable
@@ -19,7 +18,6 @@
 from django.http import HttpRequest, HttpResponse, JsonResponse
 from django.shortcuts import get_object_or_404, redirect, render
 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_admin_action_forms import (
@@ -36,7 +34,12 @@
 )
 from nomnom.canonicalize.sankey import transform_eph_to_sankey
 from nomnom.nominate import models as nominate
-from nomnom.reporting import Report, ReportView
+from nomnom.nominate.decorators import compose_decorators
+from nomnom.reporting import (
+    Report,
+    get_raw_report_response,
+    render_report_in_page,
+)
 from nomnom.wsfs.rules import eph
 from nomnom.wsfs.rules.constitution_2023 import CountData
 
@@ -473,10 +476,11 @@ def get_admin_url_with_filters(self):
 admin.site.register(models.Work, WorkAdmin)
 admin.site.register(models.CanonicalizedNomination, NominationGroupingView)
 
-report_decorators = [
+report_decorators_list = [
     user_passes_test(lambda u: u.is_staff, login_url="/admin/login/"),
     permission_required("nominate.report"),
 ]
+report_decorators = compose_decorators(*report_decorators_list)
 
 
 class BallotReport(Report):
@@ -529,17 +533,18 @@ def get_report_row(self, field_names: list[str], row: Any) -> list[Any]:
 
 
 # custom views for EPH and Finalists
-@method_decorator(report_decorators, name="get")
-class BallotReportView(ReportView):
-    report_class = BallotReport
-    html_template_name = "canonicalize/ballots.html"
+@report_decorators
+def ballot_report_view(request: HttpRequest, category_id: int) -> HttpResponse:
+    if request.method != "GET":
+        return HttpResponse(status=405)
 
-    @functools.lru_cache
-    def category(self) -> nominate.Category:
-        return get_object_or_404(nominate.Category, pk=self.kwargs.get("category_id"))
+    category = get_object_or_404(nominate.Category, pk=category_id)
+    report = BallotReport(category)
 
-    def prepare_report(self):
-        return self.get_report_class()(self.category())
+    if request.GET.get("html") == "true":
+        return render_report_in_page(request, report, "canonicalize/ballots.html")
+    else:
+        return get_raw_report_response(request, report, is_attachment=True)
 
 
 def nominating_ballots_from_category(category: nominate.Category) -> list[Counter[str]]:

src/nomnom/canonicalize/tests/test_reports.py

@@ -0,0 +1,116 @@
+import pytest
+from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Permission
+from django.urls import reverse
+
+from nomnom.canonicalize import models
+from nomnom.canonicalize.admin import BallotReport
+from nomnom.nominate import factories
+from nomnom.nominate import models as nominate
+
+
+@pytest.fixture(name="user")
+def make_user(db):
+    UserModel = get_user_model()
+    user = UserModel.objects.create_user(
+        username="testuser", password="password123", email="reports@example.com"
+    )
+    user.is_staff = True
+    user.user_permissions.add(Permission.objects.get(codename="report"))
+    user.user_permissions.add(Permission.objects.get(codename="view_raw_results"))
+    user.save()
+    return user
+
+
+@pytest.fixture(name="nomination")
+def make_nomination(db, user, category):
+    nominator = nominate.NominatingMemberProfile.objects.create(
+        user=user, preferred_name="Test User", member_number="123"
+    )
+    return factories.NominationFactory(nominator=nominator, category=category)
+
+
+def test_report_is_empty_when_no_nominations(client, user, category):
+    report = BallotReport(category)
+    (header, *lines) = report.build_report()
+    assert header.strip() == "nominator,work 1,work 2,work 3,work 4,work 5"
+    assert lines == []
+
+
+def test_report_is_empty_when_no_canonicalizations(client, user, category, nomination):
+    report = BallotReport(category)
+    (header, *lines) = report.build_report()
+    assert header.strip() == "nominator,work 1,work 2,work 3,work 4,work 5"
+    assert lines == []
+
+
+def test_report_includes_canonicalized_nominations(client, user, category, nomination):
+    # Canonicalize the nomination
+    canonical_work = models.Work.objects.create(
+        name="Canonical Work Name",
+        category=nomination.category,
+    )
+    canonical_work.nominations.add(nomination)
+    canonical_work.save()
+
+    ic(models.CanonicalizedNomination.objects.all())
+
+    report = BallotReport(category)
+    (header, *lines) = report.build_report()
+    assert header.strip() == "nominator,work 1,work 2,work 3,work 4,work 5"
+    assert len(lines) == 1
+    assert lines[0].startswith(
+        f"{nomination.nominator.preferred_name},{canonical_work.name}"
+    )
+
+
+@pytest.fixture(name="canonicalized_ballot_report_url")
+def make_canonicalized_ballot_report_url(category):
+    return reverse("canonicalize:ballots", kwargs={"category_id": category.id})
+
+
+def test_report_view_get_request_unauthenticated(
+    client, canonicalized_ballot_report_url
+):
+    response = client.get(canonicalized_ballot_report_url)
+    assert response.status_code == 302
+    assert response.url.startswith("/admin/login/")  # Redirect to login
+
+
+def test_report_view_get_request_without_permission_redirects_to_login(
+    client, user, canonicalized_ballot_report_url
+):
+    client.force_login(user)
+    user.user_permissions.clear()  # Remove all permissions
+    response = client.get(canonicalized_ballot_report_url)
+    assert response.status_code == 302
+    assert response.url.startswith("/accounts/login/")  # Redirect to login
+
+
+def test_report_view_get_request_with_permission(
+    client, user, canonicalized_ballot_report_url
+):
+    client.force_login(user)
+    response = client.get(canonicalized_ballot_report_url)
+    assert response.status_code == 200  # OK
+    assert "text/csv" in response["Content-Type"]
+    assert "attachment" in response["Content-Disposition"]
+
+
+def test_report_view_get_request_with_permission_contents(
+    client, user, canonicalized_ballot_report_url, nomination
+):
+    # Canonicalize the nomination
+    canonical_work = models.Work.objects.create(
+        name="Canonical Work Name",
+        category=nomination.category,
+    )
+    canonical_work.nominations.add(nomination)
+    canonical_work.save()
+
+    client.force_login(user)
+    response = client.get(canonicalized_ballot_report_url)
+    assert response.status_code == 200  # OK
+    content = response.content.decode("utf-8")
+    assert "nominator,work 1,work 2,work 3,work 4,work 5" in content
+    assert f"{nomination.nominator.preferred_name},{canonical_work.name}" in content

src/nomnom/canonicalize/urls.py

@@ -7,7 +7,7 @@
 urlpatterns = [
     path(
         "<int:category_id>/ballots/",
-        admin.BallotReportView.as_view(),
+        admin.ballot_report_view,
         name="ballots",
     ),
     path(

src/nomnom/nominate/decorators.py

@@ -1,4 +1,5 @@
 from functools import wraps
+import functools
 
 from django.core.exceptions import PermissionDenied
 
@@ -35,3 +36,12 @@ def _wrapper_view(request, *args, **kwargs):
         return _wrapper_view
 
     return decorator
+
+
+def compose_decorators(*decorators):
+    """Compose multiple decorators into a single decorator."""
+    return lambda view: functools.reduce(
+        lambda wrapped, decorator: decorator(wrapped),
+        reversed(decorators),
+        view,
+    )

src/nomnom/nominate/reports.py

@@ -1,5 +1,4 @@
 import csv
-import functools
 from collections.abc import Generator, Iterable
 from io import StringIO
 from itertools import groupby
@@ -18,27 +17,30 @@
     HttpResponse,
 )
 from django.shortcuts import get_object_or_404
-from django.utils.decorators import method_decorator
 from markdown import markdown
 
 from nomnom.nominate import models, tasks
-from nomnom.nominate.decorators import user_passes_test_or_forbidden
+from nomnom.nominate.decorators import compose_decorators, user_passes_test_or_forbidden
 from nomnom.nominate.templatetags.nomnom_filters import html_text
-from nomnom.reporting import Report, ReportView
+from nomnom.reporting import Report
 
-report_decorators = [
+report_decorators_list = [
     user_passes_test(lambda u: u.is_staff, login_url="/admin/login/"),
     permission_required("nominate.report"),
 ]
 
-raw_report_decorators = [
+report_decorators = compose_decorators(*report_decorators_list)
+
+raw_report_decorators_list = [
     login_required,
     user_passes_test_or_forbidden(lambda u: u.is_staff),
     permission_required(
         ("nominate.view_raw_results", "nominate.report"), raise_exception=True
     ),
 ]
 
+raw_report_decorators = compose_decorators(*raw_report_decorators_list)
+
 
 class NominationsReportBase(Report):
     extra_fields = ["email", "member_number", "canonical_work", "canonical_category"]
@@ -152,29 +154,36 @@ def build_report(self) -> Generator[str, None, None]:
             yield out.getvalue()
 
 
-@method_decorator(report_decorators, name="get")
-class ElectionReportView(ReportView):
-    is_attachment: bool = True
-    content_type: str = "text/csv"
-    html_template_name: str | None = None
+@report_decorators
+def nominations_report_view(request: HttpRequest, election_id: str) -> HttpResponse:
+    election = get_object_or_404(models.Election, slug=election_id)
+    report = NominationsReport(election)
+
+    response = HttpResponse(report.build_report(), content_type="text/csv")
 
-    def get_report_class(self):
-        return getattr(self, "report_class", NominationsReport)
+    response["Content-Disposition"] = f'attachment; filename="{report.get_filename()}"'
+    return response
 
-    @functools.lru_cache
-    def election(self) -> models.Election:
-        return get_object_or_404(models.Election, slug=self.kwargs.get("election_id"))
 
-    def prepare_report(self):
-        return self.get_report_class()(self.election())
+@report_decorators
+def invalidated_nominations_report_view(
+    request: HttpRequest, election_id: str
+) -> HttpResponse:
+    election = get_object_or_404(models.Election, slug=election_id)
+    report = InvalidatedNominationsReport(election)
 
+    response = HttpResponse(report.build_report(), content_type="text/csv")
 
-class Nominations(ElectionReportView):
-    report_class = NominationsReport
+    response["Content-Disposition"] = f'attachment; filename="{report.get_filename()}"'
+    return response
 
 
-class InvalidatedNominations(ElectionReportView):
-    report_class = InvalidatedNominationsReport
+class BoomReport(Report):
+    def __init__(self, *args, **kwargs):
+        super().__init__()
+
+    def build_report(self, header=True) -> Generator[str, None, None]:
+        raise Exception("Boom! This is a test exception for error handling.")
 
 
 class RanksReport(Report):
@@ -252,53 +261,31 @@ def build_report(self) -> Generator[str, None, None]:
             yield out.getvalue()
 
 
-@method_decorator(raw_report_decorators, name="dispatch")
-class AllVotes(ElectionReportView):
-    content_type = "text/plain"
-    is_attachment = True
-    report_class = RanksReport
-    html_template_name = "nominate/reports/voting_report.html"
-
-    def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
-        # get the user's email address
-        recipient = request.user.email
-        if not recipient:
-            messages.error(
-                request,
-                "You do not have an email address configured; we cannot send you the report",
-            )
-            return HttpResponse("")
+@raw_report_decorators
+def send_all_votes_report(request: HttpRequest, election_id: str) -> HttpResponse:
+    if request.method != "POST":
+        return HttpResponse(status=405)
 
-        tasks.send_rank_report.delay(
-            election_id=self.election().slug,
-            recipients=recipient,
-            exclude_configured_recipients=True,
-        )
+    recipient = request.user.email
 
-        messages.success(
+    if not recipient:
+        messages.error(
             request,
-            f"The full election report will be sent to {recipient}; this may take up to a half hour.",
+            "You do not have an email address configured; we cannot send you the report",
         )
-
         return HttpResponse("")
 
+    election = get_object_or_404(models.Election, slug=election_id)
 
-@method_decorator(raw_report_decorators, name="get")
-class CategoryVotes(ElectionReportView):
-    content_type = "text/csv"
-    is_attachment = False
-
-    report_class = CategoryVotingReport
-
-    @functools.lru_cache
-    def category(self) -> models.Category:
-        return get_object_or_404(models.Category, id=self.kwargs.get("category_id"))
-
-    @functools.lru_cache
-    def report(self) -> Report:
-        return CategoryVotingReport(category=self.category())
+    tasks.send_rank_report.delay(
+        election_id=election.slug,
+        recipients=recipient,
+        exclude_configured_recipients=True,
+    )
 
+    messages.success(
+        request,
+        f"The full election report will be sent to {recipient}; this may take up to a half hour.",
+    )
 
-@method_decorator(raw_report_decorators, name="get")
-class ElectionResults(ElectionReportView):
-    report_class = InvalidatedNominationsReport
+    return HttpResponse("")