From a53ae67faa2d618e2b89a8c326058d44c9880052 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 12 Jun 2024 17:45:10 -0300 Subject: [PATCH 1/8] feat: generate_wg_summary_files_task() --- ietf/group/tasks.py | 38 ++++++++++++++++++++++++++++++++++++++ ietf/settings.py | 1 + 2 files changed, 39 insertions(+) diff --git a/ietf/group/tasks.py b/ietf/group/tasks.py index f7717616d1..f19246fb55 100644 --- a/ietf/group/tasks.py +++ b/ietf/group/tasks.py @@ -14,6 +14,7 @@ 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", + ) diff --git a/ietf/settings.py b/ietf/settings.py index 4f0bba20a9..f04dd15bca 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -671,6 +671,7 @@ def skip_unreadable_post(record): 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' From 9643dfaffc075f188bf23ae5a4b9436a084730a2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 12 Jun 2024 18:55:16 -0300 Subject: [PATCH 2/8] refactor: wg summaries from filesys for view --- ietf/group/views.py | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/ietf/group/views.py b/ietf/group/views.py index 0c37cca3a0..6764648441 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -152,34 +152,27 @@ def check_group_email_aliases(): return False +def http_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") - 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) + return http_response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt") - 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") - 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') + return http_response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt") def wg_charters(request, group_type): From 2b64ba18135f1f336214be47e5c9ccfe2be00b6c Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 12 Jun 2024 19:03:27 -0300 Subject: [PATCH 3/8] refactor: use new helper for charter views --- ietf/group/views.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/ietf/group/views.py b/ietf/group/views.py index 6764648441..d1e7717352 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -178,23 +178,13 @@ def wg_summary_acronym(request, group_type): def wg_charters(request, group_type): if group_type != "wg": raise Http404 - fpath = 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") + return http_response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters.txt") 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") + return http_response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") def active_groups(request, group_type=None): From f39599fcd3ed599099b0a8fae1fb68841870ccf0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 10:21:32 -0300 Subject: [PATCH 4/8] refactor: use FileResponse --- ietf/group/views.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ietf/group/views.py b/ietf/group/views.py index d1e7717352..55f62dde89 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -52,7 +52,7 @@ from django.contrib.auth.decorators import login_required from django.db.models import Count, F, OuterRef, Prefetch, Q, Subquery, TextField, Value from django.db.models.functions import Coalesce -from django.http import HttpResponse, HttpResponseRedirect, Http404, JsonResponse +from django.http import HttpResponse, HttpResponseRedirect, Http404, JsonResponse, FileResponse from django.shortcuts import render, redirect, get_object_or_404 from django.template.loader import render_to_string from django.urls import reverse as urlreverse @@ -152,13 +152,12 @@ def check_group_email_aliases(): return False -def http_response_from_file(fpath: Path) -> HttpResponse: +def response_from_file(fpath: Path) -> FileResponse: """Helper to shovel a file back in an HttpResponse""" try: - content = fpath.read_bytes() + return FileResponse(fpath.open("rb"), content_type="text/plain; charset=utf-8") except IOError: raise Http404 - return HttpResponse(content, content_type="text/plain; charset=UTF-8") # --- View functions --------------------------------------------------- @@ -166,25 +165,25 @@ def http_response_from_file(fpath: Path) -> HttpResponse: def wg_summary_area(request, group_type): if group_type != "wg": raise Http404 - return http_response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt") + return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt") def wg_summary_acronym(request, group_type): if group_type != "wg": raise Http404 - return http_response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt") + return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt") def wg_charters(request, group_type): if group_type != "wg": raise Http404 - return http_response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters.txt") + return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters.txt") def wg_charters_by_acronym(request, group_type): if group_type != "wg": raise Http404 - return http_response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") + return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") def active_groups(request, group_type=None): From 0564ec24bbc86de856241a5f0f150fd80f40e3c3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 10:36:58 -0300 Subject: [PATCH 5/8] 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 --- ietf/group/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ietf/group/views.py b/ietf/group/views.py index 55f62dde89..09b1ac933f 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -52,7 +52,7 @@ from django.contrib.auth.decorators import login_required from django.db.models import Count, F, OuterRef, Prefetch, Q, Subquery, TextField, Value from django.db.models.functions import Coalesce -from django.http import HttpResponse, HttpResponseRedirect, Http404, JsonResponse, FileResponse +from django.http import HttpResponse, HttpResponseRedirect, Http404, JsonResponse from django.shortcuts import render, redirect, get_object_or_404 from django.template.loader import render_to_string from django.urls import reverse as urlreverse @@ -152,12 +152,13 @@ def check_group_email_aliases(): return False -def response_from_file(fpath: Path) -> FileResponse: +def response_from_file(fpath: Path) -> HttpResponse: """Helper to shovel a file back in an HttpResponse""" try: - return FileResponse(fpath.open("rb"), content_type="text/plain; charset=utf-8") + content = fpath.read_bytes() except IOError: raise Http404 + return HttpResponse(content, content_type="text/plain; charset=utf-8") # --- View functions --------------------------------------------------- From 4fdf045284ee04e4c9e31f950e5cce5f711ed7eb Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 10:53:07 -0300 Subject: [PATCH 6/8] test: update view tests --- ietf/group/tests_info.py | 137 ++++++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 52 deletions(-) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 777671db9d..7412fde12c 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -8,7 +8,7 @@ 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 @@ 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 @@ 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 @@ def test_group_home(self): self.assertContains(r, draft.name) self.assertContains(r, draft.title) - def test_wg_summaries(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] - - 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.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) + def test_response_from_file(self): + # n.b., GROUP_SUMMARY_PATH is a temp dir that will be cleaned up automatically + fp = Path(settings.GROUP_SUMMARY_PATH) / "some-file.txt" + fp.write_text("This is a charters file with an é") + r = response_from_file(fp) 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.headers["Content-Type"], "text/plain; 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) + + @patch("ietf.group.views.response_from_file") + def test_wg_summary_area(self, mock): + r = self.client.get( + urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "rg"}) + ) # not wg 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.assertFalse(mock.called) + mock.return_value = HttpResponse("yay") + r = self.client.get( + urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "wg"}) + ) 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", kwargs=dict(group_type="rg")) - r = self.client.get(url) + self.assertEqual(r.content.decode(), "yay") + self.assertEqual(mock.call_args, call(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt")) + + @patch("ietf.group.views.response_from_file") + def test_wg_summary_acronym(self, mock): + 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") + ) - 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) + @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) - - 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.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.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.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( From 9c0d8cd713cfa2df467f841671093ce30076198d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 11:15:38 -0300 Subject: [PATCH 7/8] test: test_generate_wg_summary_files_task() --- ietf/group/tests_info.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 7412fde12c..29a45a32eb 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -287,6 +287,30 @@ def test_generate_wg_charters_files_task_without_copy(self): ) 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 From e3d5e848d7fe14b4d94578cd5da2c6b81994201b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 11:17:18 -0300 Subject: [PATCH 8/8] chore: create PeriodicTask N.B. that this makes it hourly instead of daily --- ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index 7f0c988dcd..76d362bb24 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -231,6 +231,16 @@ def create_default_tasks(self): ), ) + 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",