refactor: generate 1wg-charters files via celery (#7428)

* refactor: move helpers to utils.py

* feat: task to generate 1wg-charters files

* refactor: use 1wg-charter files in views

* chore: create periodic task + slight renaming

* chore: remove wgets from bin/hourly

* test: refactor tests for new task/views

* fix: fix bug uncovered by tests

* chore: remove unused imports

* fix: clean whitespace in draft titles

* fix: return verbatim bytes for charter views

* chore: remove now-empty /bin/hourly 🎉

bin/daily

@@ -17,9 +17,6 @@ cd $DTDIR/
 
 logger -p user.info -t cron "Running $DTDIR/bin/daily"
 
-# Run the hourly jobs first
-$DTDIR/bin/hourly
-
 # Set up the virtual environment
 source $DTDIR/env/bin/activate
 

bin/hourly

@@ -1,19 +0,0 @@
-#!/bin/bash
-
-# Hourly datatracker jobs
-# 
-# This script is expected to be triggered by cron from
-# /etc/cron.d/datatracker
-export LANG=en_US.UTF-8
-
-# Make sure we stop if something goes wrong:
-program=${0##*/}
-trap 'echo "$program($LINENO): Command failed with error code $? ([$$] $0 $*)"; exit 1' ERR
-
-logger -p user.info -t cron "Running $DTDIR/bin/hourly"
-
-CHARTER=/a/www/ietf-ftp/charter
-wget -q https://datatracker.ietf.org/wg/1wg-charters-by-acronym.txt -O $CHARTER/1wg-charters-by-acronym.txt
-wget -q https://datatracker.ietf.org/wg/1wg-charters.txt            -O $CHARTER/1wg-charters.txt
-
-# exit 0

ietf/doc/views_charter.py

@@ -37,8 +37,8 @@ from ietf.doc.utils_charter import ( historic_milestones_for_charter,
 from ietf.doc.mails import email_state_changed, email_charter_internal_review
 from ietf.group.mails import email_admin_re_charter
 from ietf.group.models import Group, ChangeStateGroupEvent, MilestoneGroupEvent
-from ietf.group.utils import save_group_in_history, save_milestone_in_history, can_manage_all_groups_of_type
+from ietf.group.utils import save_group_in_history, save_milestone_in_history, can_manage_all_groups_of_type, \
-from ietf.group.views import fill_in_charter_info
+    fill_in_charter_info
 from ietf.ietfauth.utils import has_role, role_required
 from ietf.name.models import GroupStateName
 from ietf.person.models import Person

ietf/group/tasks.py

@@ -0,0 +1,33 @@
+# Copyright The IETF Trust 2024, All Rights Reserved
+#
+# Celery task definitions
+#
+from celery import shared_task
+from pathlib import Path
+
+from django.conf import settings
+from django.template.loader import render_to_string
+
+from .models import Group
+from .utils import fill_in_charter_info, fill_in_wg_drafts, fill_in_wg_roles
+
+
+@shared_task
+def generate_wg_charters_files_task():
+    areas = Group.objects.filter(type="area", state="active").order_by("name")
+    groups = Group.objects.filter(type="wg", state="active").exclude(parent=None).order_by("acronym")
+    for group in groups:
+        fill_in_charter_info(group)
+        fill_in_wg_roles(group)
+        fill_in_wg_drafts(group)
+    for area in areas:
+        area.groups = [g for g in groups if g.parent_id == area.pk]
+    charter_path = Path(settings.CHARTER_PATH)
+    (charter_path / "1wg-charters.txt").write_text(
+        render_to_string("group/1wg-charters.txt", {"areas": areas}),
+        encoding="utf8",
+    )
+    (charter_path / "1wg-charters-by-acronym.txt").write_text(
+        render_to_string("group/1wg-charters-by-acronym.txt", {"groups": groups}),
+        encoding="utf8",
+    )

ietf/group/tests_info.py

@@ -34,6 +34,7 @@ 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.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
@@ -117,10 +118,6 @@ class GroupPagesTests(TestCase):
 
         chair = Email.objects.filter(role__group=group, role__name="chair")[0]
 
-        (
-            Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt"
-         ).write_text("This is a charter.")
-
         url = urlreverse('ietf.group.views.wg_summary_area', kwargs=dict(group_type="wg"))
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
@@ -136,23 +133,65 @@ class GroupPagesTests(TestCase):
         self.assertContains(r, group.name)
         self.assertContains(r, chair.address)
         
-        url = urlreverse('ietf.group.views.wg_charters', kwargs=dict(group_type="wg"))
+    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, 200)
+        self.assertEqual(r.status_code, 404)
-        self.assertContains(r, group.acronym)
-        self.assertContains(r, group.name)
-        self.assertContains(r, group.ad_role().person.plain_name())
-        self.assertContains(r, chair.address)
-        self.assertContains(r, "This is a charter.")
 
-        url = urlreverse('ietf.group.views.wg_charters_by_acronym', kwargs=dict(group_type="wg"))
+        # 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.assertContains(r, group.acronym)
+        self.assertEqual(r.charset, "UTF-8")
-        self.assertContains(r, group.name)
+        self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é")
-        self.assertContains(r, group.ad_role().person.plain_name())
+
-        self.assertContains(r, chair.address)
+        # non-wg request = 404 even if the file exists
-        self.assertContains(r, "This is a charter.")
+        url = urlreverse("ietf.group.views.wg_charters", kwargs=dict(group_type="rg"))
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 404)
+
+    def test_wg_charters_by_acronym(self):
+        url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="wg"))
+        r = self.client.get(url)
+        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("utf8"), "This is a charters file with an é")
+
+        # non-wg request = 404 even if the file exists
+        url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="rg"))
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 404)
+
+    def test_generate_wg_charters_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]
+        (
+            Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt"
+         ).write_text("This is a charter.")
+
+        generate_wg_charters_files_task()
+        wg_charters_contents = (Path(settings.CHARTER_PATH) / "1wg-charters.txt").read_text(encoding="utf8")
+        self.assertIn(group.acronym, wg_charters_contents)
+        self.assertIn(group.name, wg_charters_contents)
+        self.assertIn(group.ad_role().person.plain_name(), wg_charters_contents)
+        self.assertIn(chair.address, wg_charters_contents)
+        self.assertIn("This is a charter.", wg_charters_contents)
+
+        wg_charters_by_acronym_contents = (Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt").read_text(encoding="utf8")
+        self.assertIn(group.acronym, wg_charters_by_acronym_contents)
+        self.assertIn(group.name, wg_charters_by_acronym_contents)
+        self.assertIn(group.ad_role().person.plain_name(), wg_charters_by_acronym_contents)
+        self.assertIn(chair.address, wg_charters_by_acronym_contents)
+        self.assertIn("This is a charter.", wg_charters_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/utils.py

@@ -15,13 +15,13 @@ import debug                            # pyflakes:ignore
 
 from ietf.community.models import CommunityList, SearchRule
 from ietf.community.utils import reset_name_contains_index_for_rule, can_manage_community_list
-from ietf.doc.models import Document, State
+from ietf.doc.models import Document, State, RelatedDocument
 from ietf.group.models import Group, RoleHistory, Role, GroupFeatures, GroupEvent
 from ietf.ietfauth.utils import has_role
 from ietf.name.models import GroupTypeName, RoleName
 from ietf.person.models import Email
 from ietf.review.utils import can_manage_review_requests_for_team
-from ietf.utils import log
+from ietf.utils import log, markdown
 from ietf.utils.history import get_history_object_for, copy_many_to_many_for_history
 from ietf.doc.templatetags.ietf_filters import is_valid_url
 from functools import reduce
@@ -450,3 +450,68 @@ def role_holder_emails():
         address__startswith="unknown-email-"
     )
     return emails.filter(person__role__in=roles).distinct()
+
+
+def fill_in_charter_info(group, include_drafts=False):
+    group.areadirector = getattr(group.ad_role(),'email',None)
+
+    personnel = {}
+    for r in Role.objects.filter(group=group).order_by('person__name').select_related("email", "person", "name"):
+        if r.name_id not in personnel:
+            personnel[r.name_id] = []
+        personnel[r.name_id].append(r)
+
+    if group.parent and group.parent.type_id == "area" and group.ad_role() and "ad" not in personnel:
+        ad_roles = list(Role.objects.filter(group=group.parent, name="ad", person=group.ad_role().person))
+        if ad_roles:
+            personnel["ad"] = ad_roles
+
+    group.personnel = []
+    for role_name_slug, roles in personnel.items():
+        label = roles[0].name.name
+        if len(roles) > 1:
+            if label.endswith("y"):
+                label = label[:-1] + "ies"
+            else:
+                label += "s"
+
+        group.personnel.append((role_name_slug, label, roles))
+
+    group.personnel.sort(key=lambda t: t[2][0].name.order)
+
+    milestone_state = "charter" if group.state_id == "proposed" else "active"
+    group.milestones = group.groupmilestone_set.filter(state=milestone_state)
+    if group.uses_milestone_dates:
+        group.milestones = group.milestones.order_by('resolved', 'due')
+    else:
+        group.milestones = group.milestones.order_by('resolved', 'order')
+
+    if group.charter:
+        group.charter_text = get_charter_text(group)
+    else:
+        group.charter_text = "Not chartered yet."
+    group.charter_html = markdown.markdown(group.charter_text)
+
+
+def fill_in_wg_roles(group):
+    def get_roles(slug, default):
+        for role_slug, label, roles in group.personnel:
+            if slug == role_slug:
+                return roles
+        return default
+
+    group.chairs = get_roles("chair", [])
+    ads = get_roles("ad", [])
+    group.areadirector = ads[0] if ads else None
+    group.techadvisors = get_roles("techadv", [])
+    group.editors = get_roles("editor", [])
+    group.secretaries = get_roles("secr", [])
+
+
+def fill_in_wg_drafts(group):
+    group.drafts = Document.objects.filter(type_id="draft", group=group).order_by("name")
+    group.rfcs = Document.objects.filter(type_id="rfc", group=group).order_by("rfc_number")
+    for rfc in group.rfcs:
+        # TODO: remote_field?
+        rfc.remote_field = RelatedDocument.objects.filter(source=rfc,relationship_id__in=['obs','updates']).distinct()
+        rfc.invrel = RelatedDocument.objects.filter(target=rfc,relationship_id__in=['obs','updates']).distinct()

ietf/group/views.py

@@ -41,9 +41,10 @@ import io
 import math
 import re
 import json
+import types
 
 from collections import OrderedDict, defaultdict
-import types
+from pathlib import Path
 from simple_history.utils import update_change_reason
 
 from django import forms
@@ -75,12 +76,12 @@ from ietf.group.forms import (GroupForm, StatusUpdateForm, ConcludeGroupForm, St
 from ietf.group.mails import email_admin_re_charter, email_personnel_change, email_comment
 from ietf.group.models import ( Group, Role, GroupEvent, GroupStateTransitions,
                               ChangeStateGroupEvent, GroupFeatures, AppealArtifact )
-from ietf.group.utils import (get_charter_text, can_manage_all_groups_of_type, 
+from ietf.group.utils import (can_manage_all_groups_of_type,
                               milestone_reviewer_for_group_type, can_provide_status_update,
                               can_manage_materials, group_attribute_change_desc,
                               construct_group_menu_context, get_group_materials,
                               save_group_in_history, can_manage_group, update_role_set,
-                              get_group_or_404, setup_default_community_list_for_group, )                              
+                              get_group_or_404, setup_default_community_list_for_group, fill_in_charter_info)                              
 #
 from ietf.ietfauth.utils import has_role, is_authorized_in_group
 from ietf.mailtrigger.utils import gather_relevant_expansions
@@ -132,70 +133,9 @@ def roles(group, role_name):
     return Role.objects.filter(group=group, name=role_name).select_related("email", "person")
 
 
-def fill_in_charter_info(group, include_drafts=False):
-    group.areadirector = getattr(group.ad_role(),'email',None)
-
-    personnel = {}
-    for r in Role.objects.filter(group=group).order_by('person__name').select_related("email", "person", "name"):
-        if r.name_id not in personnel:
-            personnel[r.name_id] = []
-        personnel[r.name_id].append(r)
-
-    if group.parent and group.parent.type_id == "area" and group.ad_role() and "ad" not in personnel:
-        ad_roles = list(Role.objects.filter(group=group.parent, name="ad", person=group.ad_role().person))
-        if ad_roles:
-            personnel["ad"] = ad_roles
-
-    group.personnel = []
-    for role_name_slug, roles in personnel.items():
-        label = roles[0].name.name
-        if len(roles) > 1:
-            if label.endswith("y"):
-                label = label[:-1] + "ies"
-            else:
-                label += "s"
-
-        group.personnel.append((role_name_slug, label, roles))
-
-    group.personnel.sort(key=lambda t: t[2][0].name.order)
-
-    milestone_state = "charter" if group.state_id == "proposed" else "active"
-    group.milestones = group.groupmilestone_set.filter(state=milestone_state)
-    if group.uses_milestone_dates:
-        group.milestones = group.milestones.order_by('resolved', 'due')
-    else:
-        group.milestones = group.milestones.order_by('resolved', 'order')
-
-    if group.charter:
-        group.charter_text = get_charter_text(group)
-    else:
-        group.charter_text = "Not chartered yet."
-    group.charter_html = markdown.markdown(group.charter_text)
-
 def extract_last_name(role):
     return role.person.name_parts()[3]
 
-def fill_in_wg_roles(group):
-    def get_roles(slug, default):
-        for role_slug, label, roles in group.personnel:
-            if slug == role_slug:
-                return roles
-        return default
-
-    group.chairs = get_roles("chair", [])
-    ads = get_roles("ad", [])
-    group.areadirector = ads[0] if ads else None
-    group.techadvisors = get_roles("techadv", [])
-    group.editors = get_roles("editor", [])
-    group.secretaries = get_roles("secr", [])
-
-def fill_in_wg_drafts(group):
-    group.drafts = Document.objects.filter(type_id="draft", group=group).order_by("name")
-    group.rfcs = Document.objects.filter(type_id="rfc", group=group).order_by("rfc_number")
-    for rfc in group.rfcs:
-        # TODO: remote_field?
-        rfc.remote_field = RelatedDocument.objects.filter(source=rfc,relationship_id__in=['obs','updates']).distinct()
-        rfc.invrel = RelatedDocument.objects.filter(target=rfc,relationship_id__in=['obs','updates']).distinct()
 
 def check_group_email_aliases():
     pattern = re.compile(r'expand-(.*?)(-\w+)@.*? +(.*)$')
@@ -241,34 +181,28 @@ def wg_summary_acronym(request, group_type):
                     'groups': groups },
                   content_type='text/plain; charset=UTF-8')
 
-@cache_page ( 60 * 60, cache="slowpages" )
+
 def wg_charters(request, group_type):
     if group_type != "wg":
         raise Http404
-    areas = Group.objects.filter(type="area", state="active").order_by("name")
+    fpath = Path(settings.CHARTER_PATH) / "1wg-charters.txt" 
-    for area in areas:
+    try:
-        area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("name")
+        content = fpath.read_bytes()
-        for group in area.groups:
+    except IOError:
-            fill_in_charter_info(group)
+        raise Http404
-            fill_in_wg_roles(group)
+    return HttpResponse(content, content_type="text/plain; charset=UTF-8")
-            fill_in_wg_drafts(group)
+
-    return render(request, 'group/1wg-charters.txt',
-                  { 'areas': areas },
-                  content_type='text/plain; charset=UTF-8')
 
-@cache_page ( 60 * 60, cache="slowpages" )
 def wg_charters_by_acronym(request, group_type):
     if group_type != "wg":
         raise Http404
+    fpath = 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")
 
-    groups = Group.objects.filter(type="wg", state="active").exclude(parent=None).order_by("acronym")
-    for group in groups:
-        fill_in_charter_info(group)
-        fill_in_wg_roles(group)
-        fill_in_wg_drafts(group)
-    return render(request, 'group/1wg-charters-by-acronym.txt',
-                  { 'groups': groups },
-                  content_type='text/plain; charset=UTF-8')
 
 def active_groups(request, group_type=None):
 

ietf/templates/group/group_entry_with_charter.txt

@@ -37,7 +37,7 @@ Goals and Milestones:
 {% for milestone in group.milestones %}  {% if milestone.resolved %}{{ milestone.resolved }}    {% else %}{{ milestone.due|date:"M Y" }}{% endif %} - {{ milestone.desc }}
 {% endfor %}
 Internet-Drafts:
-{% for document in group.drafts %}  - {{ document.title }} [{{ document.name }}-{{ document.rev }}] ({{ document.pages }} pages)
+{% for document in group.drafts %}  - {{ document.title|clean_whitespace }} [{{ document.name }}-{{ document.rev }}] ({{ document.pages }} pages)
 {% endfor %}
 {% if group.rfcs %}Requests for Comments:
 {% for document in group.rfcs %}  {{ document.name.upper }}: {{ document.title}} ({{ document.pages }} pages){% for r in document.rel %}

ietf/utils/management/commands/periodic_tasks.py

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