refactor: make WG summary view into a task (#7529)

* feat: generate_wg_summary_files_task()

* refactor: wg summaries from filesys for view

* refactor: use new helper for charter views

* refactor: use FileResponse

* refactor: don't use FileResponse

FileResponse generates a StreamingHttpResponse
which brings with it differences I don't fully
understand, so let's stay with HttpResponse

* test: update view tests

* test: test_generate_wg_summary_files_task()

* chore: create PeriodicTask

N.B. that this makes it hourly instead of daily

ietf/group/tasks.py

@@ -14,6 +14,7 @@ from ietf.utils import log
 
 from .models import Group
 from .utils import fill_in_charter_info, fill_in_wg_drafts, fill_in_wg_roles
+from .views import extract_last_name, roles
 
 
 @shared_task
@@ -59,3 +60,40 @@ def generate_wg_charters_files_task():
                 log.log(
                     f"Error copying {charters_by_acronym_file} to {charter_copy_dest}: {err}"
                 )
+
+
+@shared_task
+def generate_wg_summary_files_task():
+    # Active WGs (all should have a parent, but filter to be sure)
+    groups = (
+        Group.objects.filter(type="wg", state="active")
+        .exclude(parent=None)
+        .order_by("acronym")
+    )
+    # Augment groups with chairs list
+    for group in groups:
+        group.chairs = sorted(roles(group, "chair"), key=extract_last_name)
+
+    # Active areas with one or more active groups in them 
+    areas = Group.objects.filter(
+        type="area",
+        state="active",
+        group__in=groups,
+    ).distinct().order_by("name")
+    # Augment areas with their groups
+    for area in areas:
+        area.groups = [g for g in groups if g.parent_id == area.pk]
+    summary_path = Path(settings.GROUP_SUMMARY_PATH)
+    summary_file = summary_path / "1wg-summary.txt"
+    summary_file.write_text(
+        render_to_string("group/1wg-summary.txt", {"areas": areas}),
+        encoding="utf8",
+    )
+    summary_by_acronym_file = summary_path / "1wg-summary-by-acronym.txt"
+    summary_by_acronym_file.write_text(
+        render_to_string(
+            "group/1wg-summary-by-acronym.txt",
+            {"areas": areas, "groups": groups},
+        ),
+        encoding="utf8",
+    )

ietf/group/tests_info.py

@@ -8,7 +8,7 @@ import datetime
 import io
 import bleach
 
-from unittest.mock import patch
+from unittest.mock import call, patch
 from pathlib import Path
 from pyquery import PyQuery
 from tempfile import NamedTemporaryFile
@@ -16,6 +16,7 @@ from tempfile import NamedTemporaryFile
 import debug                            # pyflakes:ignore
 
 from django.conf import settings
+from django.http import Http404, HttpResponse
 from django.test import RequestFactory
 from django.test.utils import override_settings
 from django.urls import reverse as urlreverse
@@ -35,7 +36,8 @@ from ietf.group.factories import (GroupFactory, RoleFactory, GroupEventFactory,
     DatedGroupMilestoneFactory, DatelessGroupMilestoneFactory)
 from ietf.group.forms import GroupForm
 from ietf.group.models import Group, GroupEvent, GroupMilestone, GroupStateTransitions, Role
-from ietf.group.tasks import generate_wg_charters_files_task
+from ietf.group.tasks import generate_wg_charters_files_task, generate_wg_summary_files_task
+from ietf.group.views import response_from_file
 from ietf.group.utils import save_group_in_history, setup_default_community_list_for_group
 from ietf.meeting.factories import SessionFactory
 from ietf.name.models import DocTagName, GroupStateName, GroupTypeName, ExtResourceName, RoleName
@@ -58,7 +60,11 @@ def pklist(docs):
     return [ str(doc.pk) for doc in docs.all() ]
 
 class GroupPagesTests(TestCase):
-    settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['CHARTER_PATH', 'CHARTER_COPY_PATH']
+    settings_temp_path_overrides = TestCase.settings_temp_path_overrides + [
+        "CHARTER_PATH",
+        "CHARTER_COPY_PATH",
+        "GROUP_SUMMARY_PATH",
+    ]
 
     def test_active_groups(self):
         area = GroupFactory.create(type_id='area')
@@ -112,63 +118,90 @@ class GroupPagesTests(TestCase):
             self.assertContains(r, draft.name)
             self.assertContains(r, draft.title)
 
-    def test_wg_summaries(self):
+    def test_response_from_file(self):
-        group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area')).group
+        # n.b., GROUP_SUMMARY_PATH is a temp dir that will be cleaned up automatically
-        RoleFactory(group=group,name_id='chair',person=PersonFactory())
+        fp = Path(settings.GROUP_SUMMARY_PATH) / "some-file.txt"
-        RoleFactory(group=group,name_id='ad',person=PersonFactory())
+        fp.write_text("This is a charters file with an é")
-
+        r = response_from_file(fp)
-        chair = Email.objects.filter(role__group=group, role__name="chair")[0]
-
-        url = urlreverse('ietf.group.views.wg_summary_area', kwargs=dict(group_type="wg"))
-        r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
-        self.assertContains(r, group.parent.name)
+        self.assertEqual(r.headers["Content-Type"], "text/plain; charset=utf-8")
-        self.assertContains(r, group.acronym)
-        self.assertContains(r, group.name)
-        self.assertContains(r, chair.address)
-
-        url = urlreverse('ietf.group.views.wg_summary_acronym', kwargs=dict(group_type="wg"))
-        r = self.client.get(url)
-        self.assertEqual(r.status_code, 200)
-        self.assertContains(r, group.acronym)
-        self.assertContains(r, group.name)
-        self.assertContains(r, chair.address)
-        
-    def test_wg_charters(self):
-        # file does not exist = 404
-        url = urlreverse("ietf.group.views.wg_charters", kwargs=dict(group_type="wg"))
-        r = self.client.get(url)
-        self.assertEqual(r.status_code, 404)
-
-        # should return expected file with expected encoding
-        wg_path = Path(settings.CHARTER_PATH) / "1wg-charters.txt"
-        wg_path.write_text("This is a charters file with an é")
-        r = self.client.get(url)
-        self.assertEqual(r.status_code, 200)
-        self.assertEqual(r.charset, "UTF-8")
         self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é")
+        # now try with a nonexistent file
+        fp.unlink()
+        with self.assertRaises(Http404):
+            response_from_file(fp)
 
-        # non-wg request = 404 even if the file exists
+    @patch("ietf.group.views.response_from_file")
-        url = urlreverse("ietf.group.views.wg_charters", kwargs=dict(group_type="rg"))
+    def test_wg_summary_area(self, mock):
-        r = self.client.get(url)
+        r = self.client.get(
+            urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "rg"})
+        )  # not wg
         self.assertEqual(r.status_code, 404)
-
+        self.assertFalse(mock.called)
-    def test_wg_charters_by_acronym(self):
+        mock.return_value = HttpResponse("yay")
-        url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="wg"))
+        r = self.client.get(
-        r = self.client.get(url)
+            urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "wg"})
-        self.assertEqual(r.status_code, 404)
+        )
-
-        wg_path = Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt"
-        wg_path.write_text("This is a charters file with an é")
-        r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
-        self.assertEqual(r.charset, "UTF-8")
+        self.assertEqual(r.content.decode(), "yay")
-        self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é")
+        self.assertEqual(mock.call_args, call(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt"))
 
-        # non-wg request = 404 even if the file exists
+    @patch("ietf.group.views.response_from_file")
-        url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="rg"))
+    def test_wg_summary_acronym(self, mock):
-        r = self.client.get(url)
+        r = self.client.get(
+            urlreverse(
+                "ietf.group.views.wg_summary_acronym", kwargs={"group_type": "rg"}
+            )
+        )  # not wg
         self.assertEqual(r.status_code, 404)
+        self.assertFalse(mock.called)
+        mock.return_value = HttpResponse("yay")
+        r = self.client.get(
+            urlreverse(
+                "ietf.group.views.wg_summary_acronym", kwargs={"group_type": "wg"}
+            )
+        )
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r.content.decode(), "yay")
+        self.assertEqual(
+            mock.call_args, call(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt")
+        )
+
+    @patch("ietf.group.views.response_from_file")
+    def test_wg_charters(self, mock):
+        r = self.client.get(
+            urlreverse("ietf.group.views.wg_charters", kwargs={"group_type": "rg"})
+        )  # not wg
+        self.assertEqual(r.status_code, 404)
+        self.assertFalse(mock.called)
+        mock.return_value = HttpResponse("yay")
+        r = self.client.get(
+            urlreverse("ietf.group.views.wg_charters", kwargs={"group_type": "wg"})
+        )
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r.content.decode(), "yay")
+        self.assertEqual(mock.call_args, call(Path(settings.CHARTER_PATH) / "1wg-charters.txt"))
+
+    @patch("ietf.group.views.response_from_file")
+    def test_wg_charters_by_acronym(self, mock):
+        r = self.client.get(
+            urlreverse(
+                "ietf.group.views.wg_charters_by_acronym", kwargs={"group_type": "rg"}
+            )
+        )  # not wg
+        self.assertEqual(r.status_code, 404)
+        self.assertFalse(mock.called)
+        mock.return_value = HttpResponse("yay")
+        r = self.client.get(
+            urlreverse(
+                "ietf.group.views.wg_charters_by_acronym", kwargs={"group_type": "wg"}
+            )
+        )
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r.content.decode(), "yay")
+        self.assertEqual(
+            mock.call_args, call(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt")
+        )
 
     def test_generate_wg_charters_files_task(self):
         group = CharterFactory(
@@ -254,6 +287,30 @@ class GroupPagesTests(TestCase):
         )
         self.assertEqual(not_a_dir.read_text(), "Not a dir")
 
+    def test_generate_wg_summary_files_task(self):
+        group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area')).group
+        RoleFactory(group=group,name_id='chair',person=PersonFactory())
+        RoleFactory(group=group,name_id='ad',person=PersonFactory())
+
+        chair = Email.objects.filter(role__group=group, role__name="chair")[0]
+
+        generate_wg_summary_files_task()
+
+        summary_by_area_contents = (
+            Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt"
+        ).read_text(encoding="utf8")
+        self.assertIn(group.parent.name, summary_by_area_contents)
+        self.assertIn(group.acronym, summary_by_area_contents)
+        self.assertIn(group.name, summary_by_area_contents)
+        self.assertIn(chair.address, summary_by_area_contents)
+
+        summary_by_acronym_contents = (
+            Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt"
+        ).read_text(encoding="utf8")
+        self.assertIn(group.acronym, summary_by_acronym_contents)
+        self.assertIn(group.name, summary_by_acronym_contents)
+        self.assertIn(chair.address, summary_by_acronym_contents)
+
     def test_chartering_groups(self):
         group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area'),states=[('charter','intrev')]).group
 

ietf/group/views.py

@@ -152,56 +152,39 @@ def check_group_email_aliases():
     return False
 
 
+def response_from_file(fpath: Path) -> HttpResponse:
+    """Helper to shovel a file back in an HttpResponse"""
+    try:
+        content = fpath.read_bytes()
+    except IOError:
+        raise Http404
+    return HttpResponse(content, content_type="text/plain; charset=utf-8")
+
+
 # --- View functions ---------------------------------------------------
 
 def wg_summary_area(request, group_type):
     if group_type != "wg":
         raise Http404
-    areas = Group.objects.filter(type="area", state="active").order_by("name")
+    return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt")
-    for area in areas:
-        area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("acronym")
-        for group in area.groups:
-            group.chairs = sorted(roles(group, "chair"), key=extract_last_name)
 
-    areas = [a for a in areas if a.groups]
-
-    return render(request, 'group/1wg-summary.txt',
-                  { 'areas': areas },
-                  content_type='text/plain; charset=UTF-8')
 
 def wg_summary_acronym(request, group_type):
     if group_type != "wg":
         raise Http404
-    areas = Group.objects.filter(type="area", state="active").order_by("name")
+    return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt")
-    groups = Group.objects.filter(type="wg", state="active").order_by("acronym").select_related("parent")
-    for group in groups:
-        group.chairs = sorted(roles(group, "chair"), key=extract_last_name)
-    return render(request, 'group/1wg-summary-by-acronym.txt',
-                  { 'areas': areas,
-                    'groups': groups },
-                  content_type='text/plain; charset=UTF-8')
 
 
 def wg_charters(request, group_type):
     if group_type != "wg":
         raise Http404
-    fpath = Path(settings.CHARTER_PATH) / "1wg-charters.txt" 
+    return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters.txt") 
-    try:
-        content = fpath.read_bytes()
-    except IOError:
-        raise Http404
-    return HttpResponse(content, content_type="text/plain; charset=UTF-8")
 
 
 def wg_charters_by_acronym(request, group_type):
     if group_type != "wg":
         raise Http404
-    fpath = Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt" 
+    return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") 
-    try:
-        content = fpath.read_bytes()
-    except IOError:
-        raise Http404
-    return HttpResponse(content, content_type="text/plain; charset=UTF-8")
 
 
 def active_groups(request, group_type=None):

ietf/settings.py

@@ -671,6 +671,7 @@ INTERNET_DRAFT_PDF_PATH = '/a/www/ietf-datatracker/pdf/'
 RFC_PATH = '/a/www/ietf-ftp/rfc/'
 CHARTER_PATH = '/a/ietfdata/doc/charter/'
 CHARTER_COPY_PATH = '/a/www/ietf-ftp/ietf'  # copy 1wg-charters files here if set
+GROUP_SUMMARY_PATH = '/a/www/ietf-ftp/ietf'
 BOFREQ_PATH = '/a/ietfdata/doc/bofreq/'
 CONFLICT_REVIEW_PATH = '/a/ietfdata/doc/conflict-review'
 STATUS_CHANGE_PATH = '/a/ietfdata/doc/status-change'

ietf/utils/management/commands/periodic_tasks.py

@@ -231,6 +231,16 @@ class Command(BaseCommand):
             ),
         )
 
+        PeriodicTask.objects.get_or_create(
+            name="Generate WG summary files",
+            task="ietf.group.tasks.generate_wg_summary_files_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["hourly"],
+                description="Update 1wg-summary.txt and 1wg-summary-by-acronym.txt",
+            ),
+        )
+
         PeriodicTask.objects.get_or_create(
             name="Generate I-D bibxml files",
             task="ietf.doc.tasks.generate_draft_bibxml_files_task",