diff --git a/docker/configs/settings_local.py b/docker/configs/settings_local.py index 94adc516a47..227da0a0ace 100644 --- a/docker/configs/settings_local.py +++ b/docker/configs/settings_local.py @@ -115,6 +115,8 @@ APP_API_TOKENS = { "ietf.api.red_api" : ["devtoken", "redtoken"], # Not a real secret "ietf.api.views_rpc" : ["devtoken"], # Not a real secret + "ietf.person.api_uuid" : ["devtoken"], # Not a real secret + "ietf.person.api_uuid_by_pk" : ["devtoken"], # Not a real secret } # Errata system api configuration diff --git a/ietf/api/tests.py b/ietf/api/tests.py index f7fa1aff02c..75147d679a4 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -1072,10 +1072,10 @@ def test_role_holder_addresses(self): ) @override_settings( - APP_API_TOKENS={"ietf.api.views.rfc_authors": ["valid-token"]} + APP_API_TOKENS={"ietf.api.views.rfc_author_survey_recipients": ["valid-token"]} ) - def test_rfc_authors(self): - url = urlreverse("ietf.api.views.rfc_authors") + def test_rfc_author_survey_recipients(self): + url = urlreverse("ietf.api.views.rfc_author_survey_recipients") # auth and method checks self.assertEqual( self.client.get(url, headers={}).status_code, 403, "No api token, no access" @@ -1179,18 +1179,30 @@ def test_rfc_authors(self): self.assertEqual(rows[0]["name"], "Jane Q. Author") # If in test mode and testaddr parameters are present, records for those - # addresses should be returned with a fake RFC. + # addresses should be returned with a fake RFC. Also exercise specifying + # recipient type, including that author is the default. r = self.client.get( - url + "?testing&testaddr=fake@a.example.com&testaddr=phony@b.example.com", + url + + ( + "?testing" + "&testaddr=fake@a.example.com" + "&testaddr=phony@b.example.com;shepherd" + "&testaddr=ersatz@c.example.com;shepherd,author" + ), headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) rows = json.loads(r.content) - self.assertEqual(len(rows), 3) + self.assertEqual(len(rows), 4) fake_author_addr = author.email().address.split("@", 1)[0] + "@fake.example.com" self.assertCountEqual( - [fake_author_addr, "fake@a.example.com", "phony@b.example.com"], - [r["email"] for r in rows], + [ + (fake_author_addr, "author"), + ("fake@a.example.com", "author"), + ("phony@b.example.com", "shepherd"), + ("ersatz@c.example.com", "author/shepherd"), + ], + [(r["email"], r["type"]) for r in rows], ) # Can only use testaddr when testing is also present @@ -1210,6 +1222,90 @@ def test_rfc_authors(self): ) self.assertEqual(r.status_code, 400, "out-of-order from/to parameters") + @override_settings( + APP_API_TOKENS={"ietf.api.views.rfc_author_survey_recipients": ["valid-token"]} + ) + def test_rfc_author_survey_recipients_with_shepherd(self): + url = urlreverse("ietf.api.views.rfc_author_survey_recipients") + now = timezone.now() + one_day_ago = now - datetime.timedelta(days=1) + # A recently published RFC with a known author and shepherd + author = PersonFactory(name="Jane Q. Author") + recent_rfc = WgRfcFactory(title="A Recently Published RFC") + DocEventFactory(doc=recent_rfc, type="published_rfc", time=one_day_ago) + RfcAuthorFactory(document=recent_rfc, person=author) + shepherd_email = EmailFactory(person__name="Alicia Shepherd") + shepherd = shepherd_email.person + recent_rfc_draft = WgDraftFactory(shepherd_id=shepherd_email.pk) + recent_rfc_draft.relateddocument_set.create( + relationship_id="became_rfc", target=recent_rfc + ) + r = self.client.get(url, headers={"X-Api-Key": "valid-token"}) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + rows = json.loads(r.content) + expected_rows = [ + { + "name": shepherd.name, + "email": shepherd.email_address(), + "type": "shepherd", + "rfc_number": str(recent_rfc.rfc_number), + "rfc_name": recent_rfc.name, + "rfc_number_and_title": ( + f"RFC {recent_rfc.rfc_number}: {recent_rfc.title}" + ), + "rfc_title": recent_rfc.title, + "published_date": str(recent_rfc.pub_date()), + }, + { + "name": author.name, + "email": author.email_address(), + "type": "author", + "rfc_number": str(recent_rfc.rfc_number), + "rfc_name": recent_rfc.name, + "rfc_title": recent_rfc.title, + "rfc_number_and_title": ( + f"RFC {recent_rfc.rfc_number}: {recent_rfc.title}" + ), + "published_date": str(recent_rfc.pub_date()), + }, + ] + self.assertCountEqual( + rows, + expected_rows, + ) + + # Now give the shepherd an RFC as author + other_rfc = WgRfcFactory(title="Other Published RFC") + DocEventFactory(doc=other_rfc, type="published_rfc", time=one_day_ago) + RfcAuthorFactory(document=other_rfc, person=shepherd) + + r = self.client.get(url, headers={"X-Api-Key": "valid-token"}) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + rows = json.loads(r.content) + # update expected rows + expected_rows[0]["type"] = "author/shepherd" + expected_rows[0]["rfc_number"] = ( + f"{recent_rfc.rfc_number}, {other_rfc.rfc_number}" + ) + expected_rows[0]["rfc_name"] = f"{recent_rfc.name}, {other_rfc.name}" + expected_rows[0]["rfc_title"] = f"{recent_rfc.title}, {other_rfc.title}" + expected_rows[0]["rfc_number_and_title"] = ( + f"RFC {recent_rfc.rfc_number}: {recent_rfc.title}, " + f"RFC {other_rfc.rfc_number}: {other_rfc.title}" + ) + expected_rows[0]["published_date"] = ( + f"{recent_rfc.pub_date()}, {other_rfc.pub_date()}" + ) + # and compare + self.assertCountEqual( + rows, + expected_rows, + ) + + + @override_settings( APP_API_TOKENS={"ietf.api.views.ingest_email": "valid-token", "ietf.api.views.ingest_email_test": "test-token"} ) diff --git a/ietf/api/urls.py b/ietf/api/urls.py index dc1ddb6d0ca..8e843d720ff 100644 --- a/ietf/api/urls.py +++ b/ietf/api/urls.py @@ -9,6 +9,7 @@ from ietf import api from ietf.doc import views_ballot, api as doc_api from ietf.meeting import views as meeting_views +from ietf.person import api_uuid as person_uuid_api from ietf.submit import views as submit_views from ietf.utils.urls import url @@ -21,6 +22,14 @@ # core_router.register("email", person_api.EmailViewSet) # core_router.register("person", person_api.PersonViewSet) +# Person identity API router +person_router = PrefixedSimpleRouter( + use_regex_path=False, name_prefix="ietf.api.person_api" +) +person_router.register( + "uuid", person_uuid_api.PersonUUIDViewSet, basename="person-uuid" +) + # todo more general name for this API? red_router = PrefixedSimpleRouter(name_prefix="ietf.api.red_api") # red api router red_router.register("doc", doc_api.RfcViewSet) @@ -44,8 +53,10 @@ # --- Custom API endpoints, sorted alphabetically --- # Email alias information for drafts url(r'^doc/draft-aliases/$', api_views.draft_aliases), - # Authors of recently published RFCs, as CSV - url(r'^doc/rfc-authors/$', api_views.rfc_authors), + # Recipients for author survey for recently published RFCs + url( + r'^doc/rfc-author-survey-recipients/$', api_views.rfc_author_survey_recipients + ), # email ingestor url(r'email/$', api_views.ingest_email), # email ingestor @@ -86,6 +97,16 @@ url(r'^person/email/$', api_views.active_email_list), # Related Email listing url(r'^person/email/(?P[^/\x00]+)/related/$', api_views.related_email_list), + # Transitional pk-to-UUID conversion. Before the router include below so it wins + # over the router's uuid/ routes. + path( + "person/uuid/by-person-pk/", + person_uuid_api.PersonUUIDByPersonPkView.as_view(), + name="ietf.api.person_api.person-uuid-by-pk", + ), + # Person UUID resolution API. After the ^person/email/ routes above so those keep + # matching first. + path("person/", include(person_router.urls)), # Draft submission API url(r'^submit/?$', submit_views.api_submit_tombstone), # Draft upload API diff --git a/ietf/api/views.py b/ietf/api/views.py index 9c4743c152a..0b766d22d07 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -90,8 +90,6 @@ def top_level(request): def api_help(request): key = JWK() - # import just public part here, for display in info page - key.import_from_pem(settings.API_PUBLIC_KEY_PEM) return render(request, "api/index.html", {'key': key, 'settings':settings, }) @@ -566,27 +564,39 @@ def role_holder_addresses(request): @requires_api_token @csrf_exempt -def rfc_authors(request): - """Return authors of published RFCs as a JSON list of objects. +def rfc_author_survey_recipients(request): + """Return recipients of the RFC Author Survey for RFCs in a time range - Finds authors by date, though this could be extended to other selection + Returns the authors and (if present) document shepherd for RFCs as a JSON + structure. This is intended for generation of the post-publication Author + Survey and is not likely to be useful elsewhere. + + Finds RFCs by date, though this could be extended to other selection criteria in the future. Specify the range as ?from=&to=. Each is an ISO-8601 timestamp, treated as UTC if it does not include time zone information. Defaults to `to`=now, `from`=14 days before `to` Each author appears once, with their RFC numbers, names, titles, and publication dates accumulated across every RFC they published in the window. + Authors have `"type": "author"` in their records. + + If an RFC has a shepherd assigned to its originating draft, that shepherd is + included in the list of recipients alongside the authors. Shepherds have + `"type": "shepherd"` in their records. If a shepherd is also an author, then + `"type": "author/shepherd"`. When the ?testing query parameter is supplied, each author's real email address is replaced with a fake one that keeps the original mailbox but uses the "fake.example.com" domain, so the output can be shared without exposing - real addresses. + real addresses. When the ?testing query parameter is supplied, one or more testaddr=ADDRESS query parameters can also be specified. The value of each parameter is an email address. When these parameters are present, the response data will include an - entry for each address as though it belonged to the author of a recently published - RFC. + entry for each address as though it belonged to an author or shepherd of a recently + published RFC. To specify the recipient type, append ";author", ";shepherd", + or ";author,shepherd" to the end of the email address. E.g., + "?testing&testaddr=somebody@example.com;author,shepherd" """ if request.method != "GET": return HttpResponse(status=405) @@ -634,56 +644,99 @@ def rfc_authors(request): docevent__type="published_rfc", docevent__time__gte=time_range_start, docevent__time__lt=time_range_end, - ).distinct() + ).order_by("rfc_number").distinct() # Collect per-author data keyed by email so each author gets one row. # Values accumulate RFC numbers, names, titles, and dates across all RFCs. - author_data = {} + recipient_data = {} for rfc in rfcs: # RfcAuthor is the authoritative source for RFC authors. Documents of # type "rfc" always have an rfcauthor_set, so no fallback is needed. - authors = [ + recipients = [ { "name": a.person.name if a.person else a.titlepage_name, "email": a.person.email().address if (a.person and a.person.email()) else None, + "type": "author", # distinguishes authors from the shepherd entry added below } for a in rfc.rfcauthor_set.select_related("person").order_by("order") ] - for author in authors: - if not author["email"]: + # As of Aug 2026, the rfc Document doesn't carry its own shepherd - it's only + # set on the draft it came from. If the shepherd changes, the value on the draft + # will be kept up to date. + # + # came_from_draft() can return None (e.g. very old RFCs with no tracked + # originating draft, April 1 RFCs, and other special cases), so guard for that. + # Also, shepherd.person should always be set (see assertion in views_doc.py) + # but skip rather than crash if that invariant is ever violated. + originating_draft = rfc.came_from_draft() + if ( + originating_draft + and originating_draft.shepherd is not None + and originating_draft.shepherd.person is not None + ): + # Use email_address() to find an active address if this one is stale + shepherd_email = originating_draft.shepherd.email_address() + shepherd = originating_draft.shepherd.person + if shepherd_email is not None: + recipients.append({ + "name": shepherd.name, + "email": shepherd_email, + "type": "shepherd", + }) + else: + log.log( + f"rfc_authors(): shepherd for {rfc.name}, has no active email " + f"address, omitting shepherd ({shepherd.name})" + ) + + for recipient in recipients: + if not recipient["email"]: continue if testing: - mailbox = author["email"].split("@", 1)[0] + mailbox = recipient["email"].split("@", 1)[0] email = f"{mailbox}@fake.example.com" else: - email = author["email"] + email = recipient["email"] - if email not in author_data: - author_data[email] = { - "name": author["name"], + if email not in recipient_data: + recipient_data[email] = { + "name": recipient["name"], "email": email, + "types": set(), "rfc_numbers": [], "rfc_names": [], "rfc_titles": [], "published_dates": [], } - author_data[email]["rfc_numbers"].append(str(rfc.rfc_number)) - author_data[email]["rfc_names"].append(rfc.name) - author_data[email]["rfc_titles"].append(rfc.title) - author_data[email]["published_dates"].append(str(rfc.pub_date())) + recipient_data[email]["rfc_numbers"].append(str(rfc.rfc_number)) + recipient_data[email]["rfc_names"].append(rfc.name) + recipient_data[email]["rfc_titles"].append(rfc.title) + recipient_data[email]["published_dates"].append(str(rfc.pub_date())) + recipient_data[email]["types"].add(recipient["type"]) if testing: for n, email in enumerate(test_addresses): - if email in author_data: - continue # author will already be included - author_data[email] = { + if email in recipient_data: + continue # recipient will already be included + # see if a type list was included + if ";" in email: + email, types = email.rsplit(";", 1) + types = {type_.strip() for type_ in types.split(",")} + if len(types.difference({"author", "shepherd"})) != 0: + return HttpResponseBadRequest( + f"Invalid types for testaddr={email}" + ) + else: + types = {"author"} + recipient_data[email] = { "name": f"Test Author {n + 1}", "email": email, + "types": types, "rfc_numbers": ["99999"], "rfc_names": ["rfc99999"], "rfc_titles": ["A Fake RFC for Testing"], @@ -691,11 +744,13 @@ def rfc_authors(request): } rows = [] - for entry in author_data.values(): + for entry in recipient_data.values(): + entry_type = "/".join(sorted(entry["types"])) rows.append( { "name": entry["name"], "email": entry["email"], + "type": entry_type, # "author", "shepherd", or "author/shepherd" "rfc_number": ", ".join(entry["rfc_numbers"]), "rfc_name": ", ".join(entry["rfc_names"]), "rfc_title": ", ".join(entry["rfc_titles"]), diff --git a/ietf/checks.py b/ietf/checks.py index f911d081f0b..3853e49f04e 100644 --- a/ietf/checks.py +++ b/ietf/checks.py @@ -4,7 +4,6 @@ import os import time -from textwrap import dedent from typing import List, Tuple # pyflakes:ignore import debug # pyflakes:ignore @@ -270,40 +269,3 @@ def maybe_patch_library(app_configs, **kwargs): ) pass return errors - -@checks.register('security') -def check_api_key_in_local_settings(app_configs, **kwargs): - errors = [] - import ietf.settings_local - if settings.SERVER_MODE == 'production': - if not ( hasattr(ietf.settings_local, 'API_PUBLIC_KEY_PEM') - and hasattr(ietf.settings_local, 'API_PRIVATE_KEY_PEM')): - errors.append(checks.Critical( - "There are no API key settings in your settings_local.py", - hint = dedent(""" - You are running in production mode, and need API key settings that are - different than the default settings. Please add settings for - API_PUBLIC_KEY_PEM and API_PRIVATE_KEY_PEM to your settings local. The - content should be matching public and private keys in PEM format. You - can generate a suitable keypair with 'ssh-keygen -f apikey.pem', and then - extract the public key with 'openssl rsa -in apikey.pem -pubout > apikey.pub'. - - """).replace('\n', '\n ').rstrip(), - id = "datatracker.E0020", - )) - elif not ( ietf.settings_local.API_PUBLIC_KEY_PEM == settings.API_PUBLIC_KEY_PEM - and ietf.settings_local.API_PRIVATE_KEY_PEM == settings.API_PRIVATE_KEY_PEM ): - errors.append(checks.Critical( - "Your API key settings in your settings_local.py are not picked up in settings.", - hint = dedent(""" - You are running in production mode, and need API key settings which are - different than the default settings. You seem to have API key settings - in settings_local.py, but they don't seem to propagate to django.conf.settings. - Please check if you have multiple settings_local.py files. - - """).replace('\n', '\n ').rstrip(), - id = "datatracker.E0021", - )) - - return errors - diff --git a/ietf/doc/admin.py b/ietf/doc/admin.py index 86f5ac5fda1..78e55a0add7 100644 --- a/ietf/doc/admin.py +++ b/ietf/doc/admin.py @@ -15,7 +15,7 @@ AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL, ReviewAssignmentDocEvent, IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder, BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor, - EditedRfcAuthorsDocEvent, RpcAssignmentDocEvent) + EditedRfcAuthorsDocEvent, RpcAssignmentDocEvent, RpcActionHolderOpenEntry) from ietf.utils.admin import SaferTabularInline from ietf.utils.validators import validate_external_resource_value @@ -233,6 +233,26 @@ class RpcAssignmentDocEventAdmin(DocEventAdmin): search_fields = DocEventAdmin.search_fields + ["assignments"] admin.site.register(RpcAssignmentDocEvent, RpcAssignmentDocEventAdmin) +class RpcActionHolderOpenEntryAdmin(admin.ModelAdmin): + """Read-only view of the open action holder entries from the RPC tool + + The RPC tool owns these - every queue push replaces them - so editing them + here would accomplish nothing. + """ + list_display = ['id', 'purple_id', 'document', 'name', 'since_when', 'deadline', ] + search_fields = ['document__name', 'person__name', 'body', ] + raw_id_fields = ['document', 'person', ] + + def has_add_permission(self, request): + return False + + def has_change_permission(self, request, obj=None): + return False + + def has_delete_permission(self, request, obj=None): + return False +admin.site.register(RpcActionHolderOpenEntry, RpcActionHolderOpenEntryAdmin) + class DocumentUrlAdmin(admin.ModelAdmin): list_display = ['id', 'doc', 'tag', 'url', 'desc', ] search_fields = ['doc__name', 'url', ] diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 0cf83e4525f..aadac27c6f9 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -14,7 +14,8 @@ from ietf.doc.models import ( Document, DocEvent, NewRevisionDocEvent, State, DocumentAuthor, StateDocEvent, BallotPositionDocEvent, BallotDocEvent, BallotType, IRSGBallotDocEvent, TelechatDocEvent, - DocumentActionHolder, BofreqEditorDocEvent, BofreqResponsibleDocEvent, DocExtResource, RfcAuthor ) + DocumentActionHolder, BofreqEditorDocEvent, BofreqResponsibleDocEvent, DocExtResource, RfcAuthor, + RpcActionHolderOpenEntry ) from ietf.group.models import Group from ietf.person.factories import PersonFactory from ietf.group.factories import RoleFactory @@ -384,6 +385,17 @@ class Meta: document = factory.SubFactory(WgDraftFactory) person = factory.SubFactory('ietf.person.factories.PersonFactory') +class RpcActionHolderOpenEntryFactory(factory.django.DjangoModelFactory): + class Meta: + model = RpcActionHolderOpenEntry + + purple_id = factory.Sequence(lambda n: n + 1) + document = factory.SubFactory(WgDraftFactory) + person = factory.SubFactory('ietf.person.factories.PersonFactory') + display_name = factory.LazyAttribute(lambda o: o.person.plain_name() if o.person else '') + comment = factory.Faker('sentence') + since_when = factory.LazyFunction(timezone.now) + class DocumentAuthorFactory(factory.django.DjangoModelFactory): class Meta: model = DocumentAuthor diff --git a/ietf/doc/forms.py b/ietf/doc/forms.py index 768d6f96af2..82076d31733 100644 --- a/ietf/doc/forms.py +++ b/ietf/doc/forms.py @@ -5,8 +5,10 @@ import datetime import debug #pyflakes:ignore from django import forms +from django.conf import settings from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import validate_email +from django.db.models.functions import Collate from ietf.doc.fields import SearchableDocumentField, SearchableDocumentsField from ietf.doc.models import RelatedDocument, DocExtResource, State @@ -63,8 +65,14 @@ class DocAuthorChangeBasisForm(forms.Form): help_text='What is the source or reasoning for the changes to the author list?') class AdForm(forms.Form): - ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active", role__group__type='area').order_by('name'), - label="Shepherding AD", empty_label="(None)", required=True) + ad = forms.ModelChoiceField( + Person.objects.filter( + role__name="ad", role__group__state="active", role__group__type="area" + ).order_by(Collate("name", settings.PREFERRED_COLLATION)), + label="Shepherding AD", + empty_label="(None)", + required=True, + ) def __init__(self, *args, **kwargs): super(self.__class__, self).__init__(*args, **kwargs) diff --git a/ietf/doc/migrations/0038_rpcactionholderopenentry.py b/ietf/doc/migrations/0038_rpcactionholderopenentry.py new file mode 100644 index 00000000000..365a7f5ba63 --- /dev/null +++ b/ietf/doc/migrations/0038_rpcactionholderopenentry.py @@ -0,0 +1,69 @@ +# Copyright The IETF Trust 2026, All Rights Reserved + +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("person", "0005_alter_historicalperson_pronouns_selectable_and_more"), + ("doc", "0037_rpcassignmentdocevent"), + ] + + operations = [ + migrations.CreateModel( + name="RpcActionHolderOpenEntry", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "purple_id", + models.PositiveIntegerField( + help_text="ID of the ActionHolder in the RPC tool", unique=True + ), + ), + ( + "body", + models.CharField( + blank=True, + default="", + help_text="Name of the body holding the action, if it is not a person", + max_length=64, + ), + ), + ( + "display_name", + models.CharField(blank=True, default="", max_length=255), + ), + ("comment", models.TextField(blank=True)), + ("rfc_number", models.PositiveIntegerField(blank=True, null=True)), + ("since_when", models.DateTimeField()), + ("deadline", models.DateTimeField(blank=True, null=True)), + ("time_captured", models.DateTimeField(auto_now=True)), + ( + "document", + ietf.utils.models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="doc.document" + ), + ), + ( + "person", + ietf.utils.models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="person.person", + ), + ), + ], + ), + ] diff --git a/ietf/doc/models.py b/ietf/doc/models.py index ff879dd71c6..8aed1726103 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -401,8 +401,33 @@ def get_state_slug(self, state_type=None): self._cached_state_slug[state_type] = s.slug if s else None return self._cached_state_slug[state_type] - def friendly_state(self): - """ Return a concise text description of the document's current state.""" + def rfc_editor_queue_status(self): + """Human-readable RPC publication queue "Status" for the document, or None. + + While a document is in the RFC Editor queue (draft-rfceditor state + "in_progress" or "blocked"), this is the status text pushed by the RFC + Production Center, matching what the queue website shows. Displays of the RFC + Editor state show it in place of the state name. It is None for a document whose + draft-rfceditor state predates the queue integration, which leaves those displays + showing the state name itself. + + Costs a query per call. Tables rendering many documents replace this with a + precomputed value; see ietf.doc.utils_search.fill_in_rfc_editor_queue_status. + """ + if self.get_state_slug("draft-rfceditor") not in ("in_progress", "blocked"): + return None + event = self.latest_event(RpcAssignmentDocEvent, type="changed_rpc_assignments") + return event.assignments if event else None + + def friendly_state(self, label_iesg_state=True): + """ Return a concise text description of the document's current state. + + For a draft in the RFC Editor queue that description is "IESG: RFC Ed Queue", + labeled because displays of it sit next to the RFC Editor's own status for the + document. Every other state stands on its own and is returned unlabeled. Callers + rendering the description somewhere already labeled as the IESG's pass + label_iesg_state=False. + """ state = self.get_state() if not state: return "Unknown state" @@ -440,7 +465,11 @@ def friendly_state(self): e = self.latest_event(LastCallDocEvent, type="sent_last_call") if e: return iesg_state_summary + " (ends %s)" % e.expires.astimezone(DEADLINE_TZINFO).date().isoformat() - + elif label_iesg_state and iesg_state.slug == "rfcqueue": + # The only state whose displays sit next to the RFC Editor's own + # status for the document, where a bare state name is ambiguous. + return "IESG: %s" % iesg_state_summary + return iesg_state_summary else: return "I-D Exists" @@ -608,12 +637,21 @@ def all_relations_that_doc(self, relationship, related=None): return related def related_that(self, relationship): + # _cached_related_that is populated in bulk by callers that render many + # documents at once (see ietf.doc.utils_search.fill_in_document_relations); + # without it each document costs a query per relationship it displays. + cached = getattr(self, "_cached_related_that", None) + if cached is not None and relationship in cached: + return cached[relationship] return list(set([x.source for x in self.relations_that(relationship)])) def all_related_that(self, relationship, related=None): return list(set([x.source for x in self.all_relations_that(relationship)])) def related_that_doc(self, relationship): + cached = getattr(self, "_cached_related_that_doc", None) + if cached is not None and relationship in cached: + return cached[relationship] return list(set([x.target for x in self.relations_that_doc(relationship)])) def all_related_that_doc(self, relationship, related=None): @@ -1617,6 +1655,77 @@ class ConsensusDocEvent(DocEvent): class RpcAssignmentDocEvent(DocEvent): assignments = models.TextField(blank=True) + +class RpcActionHolderOpenEntry(models.Model): + """Open action holder entry from the RFC Production Center + + A read-only capture of the RPC tool's open ActionHolder entries, held here + so the datatracker can query them efficiently - who is the RPC waiting on, + and for which documents. The RPC tool owns the entries. It sends the full + set for every document in the publication queue on each push to + /api/purple/queue/process/, and ietf.sync.tasks.process_rpc_queue_task + reconciles this table against that push. Nothing else writes it, there is no + editing UI, and any local change is discarded by the next push. + + Only open entries are kept: an entry the RPC has completed is dropped at + ingest, and rows disappear when their document leaves the publication queue. + Every row present is therefore one the RPC is still waiting on, so readers + do not need to filter. + + Not to be confused with DocumentActionHolder, which is the datatracker's own + action holder list for documents in IESG processing. + """ + + purple_id = models.PositiveIntegerField( + unique=True, help_text="ID of the ActionHolder in the RPC tool" + ) + document = ForeignKey(Document) + # Null when a body rather than a person holds the action, when the RPC tool + # sent its system person as a placeholder, or when it named a person the + # datatracker cannot resolve. Never the "(System)" person. + person = ForeignKey(Person, blank=True, null=True) + body = models.CharField( + max_length=64, + blank=True, + default="", + help_text="Name of the body holding the action, if it is not a person", + ) + display_name = models.CharField(max_length=255, blank=True, default="") + comment = models.TextField(blank=True) + # Belongs to the document rather than to the action holder, and is null + # until the RPC assigns it. Used to build the final review link. + rfc_number = models.PositiveIntegerField(blank=True, null=True) + # Only the date components of since_when / deadline are significant - the + # RPC tool customarily sets the time to 12:00 UTC. + since_when = models.DateTimeField() + deadline = models.DateTimeField(blank=True, null=True) + time_captured = models.DateTimeField(auto_now=True) + + def __str__(self): + return "%s: open action held by %s" % (self.document.name, self.name()) + + def name(self): + """Best available label for whoever holds this action""" + if self.body: + return self.body + if self.person: + return self.person.plain_name() + return self.display_name + + def final_review_url(self): + """Link to the queue site final review page, if there is one + + A document can have an action holder before the RPC assigns it an RFC + number, and there is no final review page until it does. + """ + if self.rfc_number is None: + return None + return "%s/final-review/rfc%d/" % ( + settings.RFC_EDITOR_QUEUE_SITE_BASE_URL, + self.rfc_number, + ) + + # IESG events class BallotType(models.Model): doc_type = ForeignKey(DocTypeName, blank=True, null=True) diff --git a/ietf/doc/resources.py b/ietf/doc/resources.py index 9da7cb57d80..f7db33925cd 100644 --- a/ietf/doc/resources.py +++ b/ietf/doc/resources.py @@ -19,7 +19,7 @@ ReviewRequestDocEvent, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL, IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder, BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor, - EditedRfcAuthorsDocEvent, RpcAssignmentDocEvent) + EditedRfcAuthorsDocEvent, RpcAssignmentDocEvent, RpcActionHolderOpenEntry) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -941,3 +941,28 @@ class Meta: "docevent_ptr": ALL_WITH_RELATIONS, } api.doc.register(RpcAssignmentDocEventResource()) + + +class RpcActionHolderOpenEntryResource(ModelResource): + document = ToOneField(DocumentResource, 'document') + person = ToOneField(PersonResource, 'person', null=True) + class Meta: + queryset = RpcActionHolderOpenEntry.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'rpcactionholderopenentry' + ordering = ['id', ] + filtering = { + "id": ALL, + "purple_id": ALL, + "body": ALL, + "display_name": ALL, + "comment": ALL, + "rfc_number": ALL, + "since_when": ALL, + "deadline": ALL, + "time_captured": ALL, + "document": ALL_WITH_RELATIONS, + "person": ALL_WITH_RELATIONS, + } +api.doc.register(RpcActionHolderOpenEntryResource()) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index e72cc04ff34..420fb0f7853 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -27,7 +27,7 @@ from ietf.doc.models import ConsensusDocEvent from ietf.ietfauth.utils import can_request_rfc_publication as utils_can_request_rfc_publication from ietf.utils import log -from ietf.doc.utils import prettify_std_name +from ietf.doc.utils import external_canonical_url, prettify_std_name from ietf.utils.html import clean_html from ietf.utils.text import wordwrap, fill, wrap_text_if_unwrapped, linkify from ietf.utils.validators import validate_url @@ -140,6 +140,19 @@ def rfceditor_info_url(rfcnum : str): """Link to the RFC editor info page for an RFC""" return urljoin(settings.RFC_EDITOR_INFO_BASE_URL, f'rfc{rfcnum}/') +@register.simple_tag(takes_context=True) +def canonical_url(context, doc): + """Absolute URL to declare canonical for doc on the current page + + Never returns None - an empty or "None" href would be a canonical pointing at a + URL that does not exist. + """ + if not context.get("snapshot"): + external = external_canonical_url(doc) + if external: + return external + return urljoin(settings.IDTRACKER_BASE_URL, context["request"].path) + def doc_name(name): """Check whether a given document exists, and return its canonical name""" diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 6f15003f92f..83a152c148b 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -5,14 +5,17 @@ import os import datetime import io +import re from hashlib import sha384 +from django.contrib.auth.models import AnonymousUser from django.http import HttpRequest import lxml import bibtexparser from unittest import mock import json import copy +import pickle import random from http.cookies import SimpleCookie @@ -24,9 +27,13 @@ from django.urls import reverse as urlreverse from django.conf import settings +from django.core.cache import cache +from django.db import connection from django.forms import Form +from django.http import QueryDict from django.utils.html import escape -from django.test import override_settings +from django.test import override_settings, RequestFactory +from django.test.utils import CaptureQueriesContext from django.utils import timezone from django.utils.text import slugify @@ -47,11 +54,16 @@ BallotDocEventFactory, DocumentAuthorFactory, NewRevisionDocEventFactory, StatusChangeFactory, DocExtResourceFactory, - RgDraftFactory, BcpFactory, RfcAuthorFactory) + RgDraftFactory, BcpFactory, StdFactory, + FyiFactory, RfcAuthorFactory, + RpcActionHolderOpenEntryFactory, + TelechatDocEventFactory) from ietf.doc.forms import NotifyForm from ietf.doc.fields import SearchableDocumentsField from ietf.doc.utils import ( create_ballot_if_not_open, + save_document_in_history, + external_canonical_url, investigate_fragment, uppercase_std_abbreviated_name, DraftAliasGenerator, @@ -60,6 +72,7 @@ get_doc_email_aliases, ) from ietf.doc.views_doc import get_diff_revisions +from ietf.doc.views_search import SearchForm, retrieve_search_results, _search_cache_key from ietf.group.models import Group, Role from ietf.group.factories import GroupFactory, RoleFactory from ietf.ipr.factories import HolderIprDisclosureFactory @@ -75,7 +88,8 @@ from ietf.utils.test_utils import TestCase from ietf.utils.text import normalize_text, texescape from ietf.utils.timezone import date_today, datetime_today, DEADLINE_TZINFO, RPC_TZINFO -from ietf.doc.utils_search import AD_WORKLOAD +from ietf.doc.utils_search import (AD_WORKLOAD, fill_in_rfc_editor_queue_status, + fill_in_telechat_date, prepare_document_table) class SearchTests(TestCase): @@ -189,6 +203,311 @@ def test_search_became_rfc(self): self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) + def test_search_by_author(self): + """The author search covers both DocumentAuthor and RfcAuthor""" + base_url = urlreverse('ietf.doc.views_search.search') + + person = PersonFactory(name="Ford Prefect") + draft = WgDraftFactory(authors=[person]) + rfc = WgRfcFactory() + RfcAuthorFactory(document=rfc, person=person, titlepage_name="F. Prefect") + # an RFC whose title page credits someone the datatracker has no Person for + anonymous_rfc = WgRfcFactory() + RfcAuthorFactory(document=anonymous_rfc, person=None, titlepage_name="Zaphod Beeblebrox") + + def search(author): + r = self.client.get(base_url + f"?activedrafts=on&rfcs=on&by=author&author={author}") + self.assertEqual(r.status_code, 200) + return r + + # by alias + r = search("Prefect") + self.assertContains(r, draft.title) + self.assertContains(r, rfc.title) + self.assertNotContains(r, anonymous_rfc.title) + + # by email address + r = search(person.email().address) + self.assertContains(r, draft.title) + self.assertContains(r, rfc.title) + + # by title page name only + r = search("Beeblebrox") + self.assertContains(r, anonymous_rfc.title) + self.assertNotContains(r, draft.title) + + def test_search_results_are_not_duplicated(self): + """retrieve_search_results must match each document at most once. + + It does not apply distinct(): doing so forces a sort over every selected column + (including abstract and, once prepare_document_table adds its select_related, + group description and person biography). Any filter that can match a document + twice has to be expressed as a subquery instead. + """ + person = PersonFactory() + rfc = WgRfcFactory() + # several ways for one document to match one author search + RfcAuthorFactory(document=rfc, person=person) + RfcAuthorFactory(document=rfc, person=person) + EmailFactory(person=person) + draft = WgDraftFactory(authors=[person, person]) + draft.set_state(State.objects.get(type="draft", slug="active")) + + for query in ( + f"activedrafts=on&olddrafts=on&rfcs=on&by=author&author={person.name}", + "activedrafts=on&olddrafts=on&rfcs=on", + f"activedrafts=on&rfcs=on&by=group&group={draft.group.acronym}", + ): + form = SearchForm(QueryDict(query)) + self.assertTrue(form.is_valid(), form.errors) + pks = list(retrieve_search_results(form).values_list("pk", flat=True)) + self.assertEqual(len(pks), len(set(pks)), f"duplicate rows for ?{query}") + + def test_search_does_not_join_multivalued_relations(self): + """The main search query must not join any multi-valued relation. + + ORing lookups across documentauthor, rfcauthor and targets_related into a single + filter() makes those paths cross-multiply. Against the production data set the + search this guards produced a 126M-row intermediate result to return 32 + documents; the joins have to stay out of the outer query. + """ + form = SearchForm(QueryDict("by=author&author=Beeblebrox&name=rfc&rfcs=on")) + self.assertTrue(form.is_valid(), form.errors) + query = retrieve_search_results(form).query + + # Only the outer query matters: the relations are reached through subqueries, + # which do contain joins of their own but are each evaluated once. + sql = str(query) + from_clause = sql[sql.index(" FROM ") : sql.index(" WHERE ")] + self.assertNotIn("JOIN", from_clause, f"main search query joins: {from_clause}") + self.assertFalse(query.distinct, "distinct() over the full column list is expensive") + + def test_search_cache_key(self): + def key(query): + form = SearchForm(QueryDict(query)) + self.assertTrue(form.is_valid(), form.errors) + return _search_cache_key(form) + + # A multi-valued field must not collapse to its last value -- these are + # different searches and must not share an entry. + self.assertNotEqual( + key("doctypes=charter&doctypes=statchg"), key("doctypes=statchg") + ) + # ...but the order the values arrive in does not change the search + self.assertEqual( + key("doctypes=statchg&doctypes=charter"), key("doctypes=charter&doctypes=statchg") + ) + # sort is applied on every request, so it must not split the cache + self.assertEqual(key("rfcs=on&sort=title"), key("rfcs=on&sort=-date")) + # equivalent spellings of a checkbox are one search + self.assertEqual(key("rfcs=on&name=foo"), key("rfcs=1&name=foo")) + # different searches stay apart + self.assertNotEqual(key("rfcs=on&name=foo"), key("rfcs=on&name=bar")) + + def test_search_query_count_does_not_grow_with_results(self): + """Rendering the document table must not cost queries per row. + + Every attribute the table shows is filled in for the whole result set at once, + so doubling the number of rows must not change the number of queries. A lookup + that slipped back into the per-row path shows up here as a count that grows. + + Mind the blind spots: these documents have no ballot, last call, action holders, + telechat, obsoleting RFCs, or IESG state under way, so the per-row work the + columns driven by those still do is not covered -- state_age_colored and the + action holder list each still cost a query for a document being processed by the + IESG. Widen the fixtures rather than reading a pass here as "the table does no + per-row queries". + """ + group = GroupFactory(type_id="wg") + url = urlreverse('ietf.doc.views_search.search') + ( + f"?activedrafts=on&olddrafts=on&rfcs=on&by=group&group={group.acronym}" + ) + system = Person.objects.get(name="(System)") + + def add_documents(count): + for _ in range(count): + WgDraftFactory(group=group, authors=[PersonFactory()], ad=PersonFactory(), + shepherd=EmailFactory()) + WgRfcFactory(group=group) + # A draft in the RFC Editor queue, whose row also shows the queue status. + # Not one the IESG is processing, to keep the blind spots above out of + # the count. + queued = WgDraftFactory( + group=group, + states=[("draft", "active"), ("draft-iesg", "idexists"), + ("draft-rfceditor", "in_progress")], + ) + RpcAssignmentDocEvent.objects.create( + doc=queued, rev=queued.rev, by=system, + type="changed_rpc_assignments", + assignments="In Progress (First Edit)", + desc="RPC status changed to In Progress (First Edit)", + ) + + def count_queries(): + with CaptureQueriesContext(connection) as context: + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + return len(context.captured_queries) + + add_documents(2) + baseline = count_queries() + add_documents(4) + doubled = count_queries() + + # A per-row lookup would add at least one query for each of the 12 new documents. + self.assertLessEqual( + doubled, baseline + 2, + f"query count grew from {baseline} to {doubled} when the result set tripled", + ) + + @override_settings(CACHES={"default": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "LOCATION": "test_search_cache_hit_preserves_row_order", + }}) + def test_search_cache_hit_preserves_row_order(self): + """A cached search must render its rows in the same order as an uncached one. + + The view caches document ids and re-prepares them on a hit, so the rows arrive + in whatever order the pk lookup returns. prepare_document_table sorts stably and + several sort keys tie heavily -- ipr, status and ad -- so without a total + ordering the same URL renders differently depending on whether it hit the cache. + Dev and test normally configure a dummy cache, hence the override. + """ + group = GroupFactory(type_id="wg") + # Documents sharing a timestamp, which is what makes the ordering ambiguous. + shared_time = timezone.now() - datetime.timedelta(days=30) + for _ in range(6): + draft = WgDraftFactory(group=group, authors=[PersonFactory()]) + Document.objects.filter(pk=draft.pk).update(time=shared_time) + base = urlreverse('ietf.doc.views_search.search') + + for sort in ("ipr", "status", "ad", ""): + with self.subTest(sort=sort): + cache.clear() + url = f"{base}?activedrafts=on&rfcs=on&by=group&group={group.acronym}&sort={sort}" + miss = self.client.get(url) + hit = self.client.get(url) + self.assertEqual(miss.status_code, 200) + self.assertEqual(hit.status_code, 200) + rows = lambda r: re.findall( # noqa: E731 + rb'href="(/doc/[^"]+)"', r.content + ) + self.assertEqual(rows(miss), rows(hit)) + + def test_prepared_documents_are_picklable(self): + """ietf.doc.views_search.recent_drafts pickles prepared documents into a cache. + + In production the slowpages cache is file-based, so everything attached to a + document by prepare_document_table has to survive pickling. Dev and test both + use a dummy cache, which accepts anything without serializing it, so nothing + else in the suite would notice a regression here. + """ + draft = WgDraftFactory(authors=[PersonFactory()], ad=PersonFactory(), + shepherd=EmailFactory()) + TelechatDocEventFactory(doc=draft) + WgRfcFactory() + queued = WgDraftFactory( + states=[("draft", "active"), ("draft-iesg", "rfcqueue"), + ("draft-rfceditor", "in_progress")], + ) + RpcAssignmentDocEvent.objects.create( + doc=queued, rev=queued.rev, by=Person.objects.get(name="(System)"), + type="changed_rpc_assignments", assignments="In Progress (First Edit)", + desc="RPC status changed to In Progress (First Edit)", + ) + + request = RequestFactory().get("/doc/recent/") + request.user = AnonymousUser() + results, meta = prepare_document_table(request, Document.objects.all()) + self.assertTrue(results) + + restored, _ = pickle.loads(pickle.dumps([results, meta])) + for before, after in zip(results, restored): + self.assertEqual(after.telechat_date(), before.telechat_date()) + self.assertEqual( + after.rfc_editor_queue_status(), before.rfc_editor_queue_status() + ) + + def test_fill_in_telechat_date_matches_the_method(self): + """The precomputed value has to equal what Document.telechat_date() returns. + + The IESG agenda views call fill_in_telechat_date() over a queryset and then + filter on doc.telechat_date(), so a mismatch silently drops documents off a + telechat agenda. + """ + future = WgDraftFactory() + TelechatDocEventFactory(doc=future, + telechat_date=timezone.now() + datetime.timedelta(days=14)) + past = WgDraftFactory() + TelechatDocEventFactory(doc=past, + telechat_date=timezone.now() - datetime.timedelta(days=14)) + rescheduled = WgDraftFactory() + TelechatDocEventFactory(doc=rescheduled, + telechat_date=timezone.now() - datetime.timedelta(days=7)) + TelechatDocEventFactory(doc=rescheduled, + telechat_date=timezone.now() + datetime.timedelta(days=7)) + none_scheduled = WgDraftFactory() + + expected = { + d.name: Document.objects.get(pk=d.pk).telechat_date() + for d in (future, past, rescheduled, none_scheduled) + } + + docs = list(Document.objects.filter( + name__in=[d.name for d in (future, past, rescheduled, none_scheduled)] + )) + fill_in_telechat_date(docs) + + for doc in docs: + self.assertEqual(doc.telechat_date(), expected[doc.name], doc.name) + # the two ends of the range, so the test would fail if everything came back None + self.assertIsNotNone(expected[future.name]) + self.assertIsNone(expected[past.name]) + + def test_fill_in_rfc_editor_queue_status_matches_the_method(self): + """The precomputed value has to equal what Document.rfc_editor_queue_status() returns. + + The document page renders the method and the document tables render the + precomputed value, so a mismatch shows the same document two different statuses. + """ + system = Person.objects.get(name="(System)") + + def queued(state_slug, *statuses): + doc = WgDraftFactory( + states=[("draft", "active"), ("draft-iesg", "rfcqueue"), + ("draft-rfceditor", state_slug)], + ) + for status in statuses: + RpcAssignmentDocEvent.objects.create( + doc=doc, rev=doc.rev, by=system, type="changed_rpc_assignments", + assignments=status, desc=f"RPC status changed to {status}", + ) + return doc + + in_progress = queued("in_progress", "In Progress (First Edit)") + # A document that has moved on has an event for every status it has held. + moved_on = queued("in_progress", "Awaiting First editor", "In Final Review") + blocked = queued("blocked", "blocked: Manual Hold") + # No queue status: a state from before the queue integration, and no state at all. + legacy = queued("rfc-edit", "In Progress (First Edit)") + not_queued = WgDraftFactory() + + fixtures = (in_progress, moved_on, blocked, legacy, not_queued) + expected = { + d.name: Document.objects.get(pk=d.pk).rfc_editor_queue_status() + for d in fixtures + } + self.assertEqual(expected[moved_on.name], "In Final Review") + self.assertIsNone(expected[legacy.name]) + + docs = list(Document.objects.filter(name__in=[d.name for d in fixtures])) + doc_dict = {d.pk: d for d in docs} + fill_in_rfc_editor_queue_status(docs, doc_dict, list(doc_dict)) + + for doc in docs: + self.assertEqual(doc.rfc_editor_queue_status(), expected[doc.name], doc.name) + def test_search_for_name(self): draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory()) draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="pub-req")) @@ -407,6 +726,80 @@ def test_docs_for_ad(self): self.assertContains(r, discuss_other.doc.name) self.assertContains(r, block_other.doc.name) + def test_ad_workload_shows_rpc_decisions_pending(self): + """The IESG dashboard shows who the RPC is currently waiting on""" + # ad_list.html builds element ids from AD names, so start from a known + # set of ADs whose names slugify predictably. + Role.objects.filter(name_id="ad").delete() + ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active', + person__name='Example Areadirector').person + idle_ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active', + person__name='Other Areadirector').person + RpcActionHolderOpenEntryFactory(person=ad) + RpcActionHolderOpenEntryFactory(person=ad) + + url = urlreverse('ietf.doc.views_search.ad_workload') + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'RPC decisions pending') + q = PyQuery(r.content) + rows = q('#rpc-pending').next('p').next('table').find('tbody tr') + self.assertEqual(len(rows), 1, 'only ADs with something pending get a row') + self.assertIn(ad.plain_name(), rows.text()) + self.assertNotIn(idle_ad.plain_name(), rows.text()) + self.assertIn('2', rows.text()) + + def test_ad_workload_without_rpc_decisions_pending(self): + """The section is absent when the RPC is waiting on nobody""" + Role.objects.filter(name_id="ad").delete() + RoleFactory(name_id='ad', group__type_id='area', group__state_id='active', + person__name='Example Areadirector') + r = self.client.get(urlreverse('ietf.doc.views_search.ad_workload')) + self.assertEqual(r.status_code, 200) + self.assertNotContains(r, 'RPC decisions pending') + + def test_docs_for_ad_shows_rpc_action_holders(self): + """An AD sees the decisions the RPC is waiting on them for""" + ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active').person + other_ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active').person + entry = RpcActionHolderOpenEntryFactory( + person=ad, comment='Confirm the change in section 4.2', rfc_number=9850 + ) + other_entry = RpcActionHolderOpenEntryFactory(person=other_ad) + + url = urlreverse('ietf.doc.views_search.docs_for_ad', + kwargs=dict(name=ad.full_name_as_key())) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'RPC decisions pending') + self.assertContains(r, entry.document.name) + self.assertNotContains(r, other_entry.document.name) + # the queue site final review page for this document + self.assertContains(r, f'{settings.RFC_EDITOR_QUEUE_SITE_BASE_URL}/final-review/rfc9850/') + # the request is shown here to everyone, including the anonymous user + self.assertContains(r, 'Confirm the change in section 4.2') + + self.client.login(username=ad.user.username, password=ad.user.username + '+password') + self.assertContains(self.client.get(url), 'Confirm the change in section 4.2') + + def test_docs_for_ad_without_rpc_action_holders(self): + """The section is absent when the RPC is waiting on nothing""" + ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active').person + r = self.client.get(urlreverse('ietf.doc.views_search.docs_for_ad', + kwargs=dict(name=ad.full_name_as_key()))) + self.assertEqual(r.status_code, 200) + self.assertNotContains(r, 'RPC decisions pending') + + def test_docs_for_ad_rpc_action_holder_without_rfc_number(self): + """A document with no rfc number yet has no final review page to link""" + ad = RoleFactory(name_id='ad', group__type_id='area', group__state_id='active').person + RpcActionHolderOpenEntryFactory(person=ad, rfc_number=None) + r = self.client.get(urlreverse('ietf.doc.views_search.docs_for_ad', + kwargs=dict(name=ad.full_name_as_key()))) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'RPC decisions pending') + self.assertNotContains(r, '/final-review/') + def test_docs_for_iesg(self): ad1 = RoleFactory(name_id='ad',group__type_id='area',group__state_id='active').person ad2 = RoleFactory(name_id='ad',group__type_id='area',group__state_id='active').person @@ -444,6 +837,90 @@ def test_auth48_doc_for_ad(self): self.assertContains(r, draft.name) self.assertContains(r, 'title="AUTH48"') # title attribute of AUTH48 badge in auth48_alert_badge filter + def _status_column_text(self, draft): + r = self.client.get( + urlreverse("ietf.doc.views_search.search") + + f"?activedrafts=on&olddrafts=on&rfcs=on&name={draft.name}" + ) + self.assertEqual(r.status_code, 200) + return PyQuery(r.content)("td.status").text() + + def test_search_labels_the_iesg_state_of_a_queued_draft(self): + """The RFC Ed Queue state is labeled as the IESG's, because the row also shows + the RFC Editor's own status. Other states are left to speak for themselves. + """ + queued = IndividualDraftFactory( + states=[("draft", "active"), ("draft-iesg", "rfcqueue"), + ("draft-rfceditor", "in_progress")] + ) + queue_state_name = queued.get_state("draft-iesg").name + self.assertIn(f"IESG: {queue_state_name}", self._status_column_text(queued)) + # The document page has a row labeled "IESG state" already, so the value there + # is the bare state name. + r = self.client.get( + urlreverse("ietf.doc.views_doc.document_main", + kwargs=dict(name=queued.name)) + ) + self.assertEqual(r.status_code, 200) + self.assertContains(r, queue_state_name) + self.assertNotContains(r, f"IESG: {queue_state_name}") + + for slug, states in ( + ("iesg-eva", [("draft", "active"), ("draft-iesg", "iesg-eva")]), + ("idexists", [("draft", "active"), ("draft-iesg", "idexists")]), + ("expired", [("draft", "expired"), ("draft-iesg", "idexists")]), + ): + with self.subTest(slug=slug): + draft = IndividualDraftFactory(states=states) + self.assertNotIn("IESG: ", self._status_column_text(draft)) + + # friendly_state labels this one itself; it must not be labeled twice. + dead = IndividualDraftFactory( + states=[("draft", "active"), ("draft-iesg", "dead")] + ) + self.assertIn("I-D Exists (IESG: Dead)", self._status_column_text(dead)) + + def test_search_shows_rfc_editor_queue_status(self): + """A queued draft's row shows the publication queue Status, as its document page does. + + The status column carries two state machines at once, so each state is labeled + with the body it belongs to. + """ + draft = IndividualDraftFactory( + states=[ + ("draft", "active"), + ("draft-iesg", "rfcqueue"), + ("draft-rfceditor", "in_progress"), + ] + ) + RpcAssignmentDocEvent.objects.create( + doc=draft, + rev=draft.rev, + by=Person.objects.get(name="(System)"), + type="changed_rpc_assignments", + assignments="In Progress (First Edit)", + desc="RPC status changed to In Progress (First Edit)", + ) + status = self._status_column_text(draft) + self.assertIn(f"IESG: {draft.get_state('draft-iesg').name}", status) + self.assertIn("RFC Editor: In Progress (First Edit)", status) + + def test_search_falls_back_to_rfc_editor_state_name(self): + """A document with no queue status shows its draft-rfceditor state name. + + Documents that went through the RFC Editor before the publication queue + integration have a draft-rfceditor state but no RpcAssignmentDocEvent. + """ + draft = IndividualDraftFactory( + states=[ + ("draft", "active"), + ("draft-iesg", "rfcqueue"), + ("draft-rfceditor", "rfc-edit"), + ] + ) + status = self._status_column_text(draft) + self.assertIn(f"RFC Editor: {draft.get_state('draft-rfceditor').name}", status) + def test_drafts_in_last_call(self): draft = IndividualDraftFactory(pages=1) draft.action_holders.set([PersonFactory()]) @@ -701,6 +1178,47 @@ def setUp(self): with (Path(dir) / 'draft-ietf-mars-test-01.txt').open('w') as f: f.write(self.draft_text) + def test_document_draft_rpc_action_holders(self): + """A draft in the RFC Editor queue shows who the RPC is waiting on""" + draft = WgDraftFactory(states=[('draft-iesg', 'rfcqueue'), + ('draft-rfceditor', 'in_progress')], + rev='00') + holder = PersonFactory() + RpcActionHolderOpenEntryFactory( + document=draft, person=holder, comment='Confirm the change in section 4.2' + ) + # An action held by a body is not shown here - the queue status covers it + RpcActionHolderOpenEntryFactory( + document=draft, person=None, body='Registry Of Xyzzy', + display_name='Registry Of Xyzzy' + ) + url = urlreverse('ietf.doc.views_doc.document_main', kwargs=dict(name=draft.name)) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, escape(holder.name)) + self.assertNotContains(r, 'Registry Of Xyzzy') + self.assertNotContains(r, 'Confirm the change in section 4.2') + + # the person being asked can see the request even though they are not an AD + self.client.login(username=holder.user.username, + password=holder.user.username + '+password') + self.assertContains(self.client.get(url), 'Confirm the change in section 4.2') + + # An earlier revision is a snapshot of the past and reports none of it. + # The snapshot is taken while the draft is in the queue, so its own + # RFC Editor state is set and that block of the page does render. + save_document_in_history(draft) + draft.rev = '01' + draft.save() + snapshot_url = urlreverse('ietf.doc.views_doc.document_main', + kwargs=dict(name=draft.name, rev='00')) + r = self.client.get(snapshot_url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'RFC Editor status') # the block is there ... + self.assertNotContains(r, escape(holder.name)) # ... without the holders + self.assertNotContains(r, 'Confirm the change in section 4.2') + def test_document_draft(self): draft = WgDraftFactory(name='draft-ietf-mars-test',rev='01', create_revisions=range(0,2)) @@ -2345,6 +2863,98 @@ def test_template_tags(self): failures, tests = doctest.testmod(ietf_filters) self.assertEqual(failures, 0) +@override_settings(RFC_EDITOR_INFO_BASE_URL="https://www.rfc-editor.example.org/info/") +class CanonicalUrlTests(TestCase): + """Tests of the rel=canonical link declared by document pages""" + + def canonical_href(self, r): + """Extract the canonical href from a response, asserting that it is usable + + An empty href resolves to the current URL and an href of "None" resolves to a + URL that does not exist - neither is visible when eyeballing a rendered page. + """ + self.assertEqual(r.status_code, 200) + links = PyQuery(r.content)("link[rel='canonical']") + self.assertEqual(len(links), 1) + href = links.attr("href") + self.assertNotIn(href, ["", "None", None]) + return href + + def test_external_canonical_url(self): + for doc in [WgRfcFactory(), BcpFactory(), StdFactory(), FyiFactory()]: + self.assertEqual( + external_canonical_url(doc), + f"https://www.rfc-editor.example.org/info/{doc.name}/", + f"{doc.type_id} belongs to the RFC Editor", + ) + for doc in [WgDraftFactory(), CharterFactory(), StatusChangeFactory()]: + self.assertIsNone(external_canonical_url(doc), f"{doc.type_id} is ours") + + def test_rfc_pages_canonicalize_to_rfc_editor(self): + rfc = WgRfcFactory() + rfc.save_with_history([DocEventFactory(doc=rfc)]) + (Path(settings.RFC_PATH) / rfc.get_base_name()).touch() + expected = f"https://www.rfc-editor.example.org/info/{rfc.name}/" + + for viewname in [ + "ietf.doc.views_doc.document_main", + "ietf.doc.views_doc.document_html", + ]: + url = urlreverse(viewname, kwargs=dict(name=rfc.name)) + r = self.client.get(url) + self.assertEqual(self.canonical_href(r), expected, f"{url} canonical") + + def test_draft_pages_canonicalize_to_datatracker(self): + draft = WgDraftFactory() + # an active draft's file is in both of these - see Document.get_file_path() + for dir in [settings.INTERNET_DRAFT_PATH, settings.INTERNET_ALL_DRAFTS_ARCHIVE_DIR]: + (Path(dir) / draft.get_base_name()).touch() + + for viewname in [ + "ietf.doc.views_doc.document_main", + "ietf.doc.views_doc.document_html", + ]: + url = urlreverse(viewname, kwargs=dict(name=draft.name)) + r = self.client.get(url) + self.assertEqual( + self.canonical_href(r), + f"{settings.IDTRACKER_BASE_URL}{url}", + f"{url} canonical", + ) + + def test_subseries_pages_canonicalize_to_rfc_editor(self): + for doc in [BcpFactory(), StdFactory(), FyiFactory()]: + url = urlreverse( + "ietf.doc.views_doc.document_main", kwargs=dict(name=doc.name) + ) + r = self.client.get(url) + self.assertEqual( + self.canonical_href(r), + f"https://www.rfc-editor.example.org/info/{doc.name}/", + f"{url} canonical", + ) + + +class SubseriesHtmlRedirectTests(TestCase): + """Tests of the /doc/html/ redirects for the bcp/std/fyi subseries + + These patterns interpolate RFC_EDITOR_INFO_BASE_URL when the URLconf is imported, + so override_settings cannot reach them - build the expectation from the setting. + """ + + def test_subseries_html_redirects_to_rfc_editor(self): + for name in ["bcp1", "std2", "fyi3"]: + for suffix in ["", "/", ".txt", ".html"]: + url = f"/doc/html/{name}{suffix}" + r = self.client.get(url) + self.assertEqual(r.status_code, 302, url) + self.assertEqual( + r["Location"], + f"{settings.RFC_EDITOR_INFO_BASE_URL}{name}/", + url, + ) + + class ReferencesTest(TestCase): def test_references(self): @@ -3038,8 +3648,23 @@ def test_obsoleted(self): def test_generate_idnits2_rfc_status(self): for slug in ('bcp', 'ds', 'exp', 'hist', 'inf', 'std', 'ps', 'unkn'): WgRfcFactory(std_level_id=slug) + WgRfcFactory(rfc_number=10001, std_level_id='ps') + WgRfcFactory(rfc_number=10002, std_level_id=None) + blob = generate_idnits2_rfc_status().replace("\n", "") + self.assertEqual(blob[6312-1], "O") + # idnits2 discards the whole file if this is not "O" - see generate_idnits2_rfc_status + self.assertEqual(blob[16-1], "O") + self.assertEqual(blob[10001-1], "P") + self.assertEqual(blob[10002-1], "U") + + def test_generate_idnits2_rfc_status_low_numbers_only(self): + # The workarounds write fixed offsets, so the blob has to reach 6312 even when + # no RFC does. + WgRfcFactory(rfc_number=1001, std_level_id="ps") blob = generate_idnits2_rfc_status().replace("\n", "") self.assertEqual(blob[6312-1], "O") + self.assertEqual(blob[200-1], "O") + self.assertEqual(blob[16-1], "O") def test_rfc_status(self): url = urlreverse('ietf.doc.views_doc.idnits2_rfc_status') diff --git a/ietf/doc/urls.py b/ietf/doc/urls.py index 0c13503b787..a73c340aac8 100644 --- a/ietf/doc/urls.py +++ b/ietf/doc/urls.py @@ -73,8 +73,9 @@ url(r'^stats/person/(?P[0-9]+)/drafts/data/?$', views_stats.chart_data_person_drafts), # This block should really all be at the idealized docs.ietf.org service - url(r'^html/(?Pbcp[0-9]+?)(\.txt|\.html)?/?$', RedirectView.as_view(url=settings.RFC_EDITOR_INFO_BASE_URL+"%(name)s", permanent=False)), - url(r'^html/(?Pstd[0-9]+?)(\.txt|\.html)?/?$', RedirectView.as_view(url=settings.RFC_EDITOR_INFO_BASE_URL+"%(name)s", permanent=False)), + url(r'^html/(?Pbcp[0-9]+?)(\.txt|\.html)?/?$', RedirectView.as_view(url=settings.RFC_EDITOR_INFO_BASE_URL+"%(name)s/", permanent=False)), + url(r'^html/(?Pstd[0-9]+?)(\.txt|\.html)?/?$', RedirectView.as_view(url=settings.RFC_EDITOR_INFO_BASE_URL+"%(name)s/", permanent=False)), + url(r'^html/(?Pfyi[0-9]+?)(\.txt|\.html)?/?$', RedirectView.as_view(url=settings.RFC_EDITOR_INFO_BASE_URL+"%(name)s/", permanent=False)), url(r'^html/%(name)s(?:-(?P[0-9]{2}(-[0-9]{2})?))?(\.txt|\.html)?/?$' % settings.URL_REGEXPS, views_doc.document_html), url(r'^id/%(name)s(?:-%(rev)s)?(?:\.(?P(txt|html|xml)))?/?$' % settings.URL_REGEXPS, views_doc.document_raw_id), diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 5f8f587c59f..301c352a0de 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -15,12 +15,13 @@ from hashlib import sha384 from pathlib import Path from typing import Iterator, Optional, Union, Iterable +from urllib.parse import urljoin from zoneinfo import ZoneInfo from django.conf import settings from django.contrib import messages from django.core.cache import caches -from django.db.models import OuterRef +from django.db.models import Max, OuterRef from django.forms import ValidationError from django.http import Http404 from django.template.loader import render_to_string @@ -455,6 +456,24 @@ def add_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_t return e +def show_rpc_action_holder_comments(user, entries): + """Should this user be shown the RPC's requests to these action holders? + + Controls whether a view renders the comment the RPC attached to an action + holder. Only the document main page views ask: they show it to the IESG, the + Secretariat and the RPC, and to whoever is being asked for the decision. + Views that do not call this show the comments to everyone, as the AD + document list does. The text is not confidential either way - the queue site + publishes it on its public final-review pages - so this is a choice about + what belongs on a given page. + """ + if has_role(user, ["Area Director", "Secretariat", "RFC Editor"]): + return True + if not (user.is_authenticated and hasattr(user, "person")): + return False + return any(entry.person_id == user.person.pk for entry in entries) + + def add_action_holder_change_event(doc, by, prev_set, reason=None): set_changed = False if doc.documentactionholder_set.exclude(person__in=prev_set).exists(): @@ -807,6 +826,18 @@ def prettify_std_name(n, spacing=" "): else: return n +def external_canonical_url(doc): + """Authoritative external URL for doc, or None if the datatracker is authoritative + + The authoritative home of an RFC, and of a bcp/std/fyi subseries document, is the + RFC Editor's info page, so we point search engines there rather than at our own + rendering of the same thing. Documents of other types are ours. + """ + if doc.type_id in ["rfc", "bcp", "std", "fyi"]: + # trailing slash matches the form the RFC Editor serves + return urljoin(settings.RFC_EDITOR_INFO_BASE_URL, f"{doc.name}/") + return None + def default_consensus(doc): # if someone edits the consensus return that, otherwise # ietf stream => true and irtf stream => false @@ -1355,8 +1386,6 @@ def update_doc_extresources(doc, new_resources, by): def generate_idnits2_rfc_status(): - blob=['N']*10000 - symbols={ 'ps': 'P', 'inf': 'I', @@ -1368,10 +1397,17 @@ def generate_idnits2_rfc_status(): 'unkn': 'U', } - rfcs = Document.objects.filter(type_id='rfc') + rfcs = Document.objects.filter(type_id='rfc').exclude(rfc_number=None) + + # One character per RFC number, indexed by that number, so the array has to reach the + # highest RFC published. The floor keeps the fixed offsets in the workarounds below in + # range when only a few RFCs exist, as is the case under test. + highest = rfcs.aggregate(Max('rfc_number'))['rfc_number__max'] or 0 + blob=['N']*max(highest, 6312) + for rfc in rfcs: offset = int(rfc.rfc_number)-1 - blob[offset] = symbols[rfc.std_level_id] + blob[offset] = symbols.get(rfc.std_level_id, 'U') if rfc.related_that('obs'): blob[offset] = 'O' @@ -1391,6 +1427,17 @@ def generate_idnits2_rfc_status(): # RFC200 is an old RFC List by Number blob[200 -1] = 'O' + # !! Do not remove: idnits2 rejects this entire file if RFC16 is not 'O' !! + # + # This deliberately contradicts both the datatracker and the RFC Editor, which + # record RFC16 as updated rather than obsoleted. idnits2 validates its download + # of this file by matching the first 64 characters against a literal pattern that + # asserts 'O' here, inherited from a tools.ietf.org curation that disagreed with + # the RFC Editor. A mismatch makes idnits2 discard the file as corrupt and fall + # back to whatever stale copy it has, silently performing no RFC status checks at + # all. Removing this line therefore breaks every idnits2 client, not just RFC16. + blob[16 - 1] = 'O' + # End Workarounds blob = re.sub('N*$','',''.join(blob)) diff --git a/ietf/doc/utils_search.py b/ietf/doc/utils_search.py index a5f461f9bb7..45108debb18 100644 --- a/ietf/doc/utils_search.py +++ b/ietf/doc/utils_search.py @@ -5,20 +5,241 @@ import datetime import debug # pyflakes:ignore +from collections import defaultdict from zoneinfo import ZoneInfo from django.conf import settings -from ietf.doc.models import Document, RelatedDocument, DocEvent, TelechatDocEvent, BallotDocEvent, DocTypeName +from ietf.doc.models import (Document, RelatedDocument, DocEvent, TelechatDocEvent, BallotDocEvent, + DocTypeName, RpcAssignmentDocEvent) from ietf.doc.expire import expirable_drafts from ietf.doc.utils import augment_docs_and_person_with_person_info from ietf.meeting.models import SessionPresentation, Meeting, Session +from ietf.person.models import Alias from ietf.review.utils import review_assignments_to_list_for_docs from ietf.utils.timezone import date_today -def wrap_value(v): - return lambda: v +class wrap_value: + """Callable stand-in for a no-argument method whose value was computed in bulk. + + Assigned over the method on the instance, so templates and code can keep calling + doc.telechat_date() without hitting the database. A class rather than a closure + because documents carrying one of these get pickled into the caches behind + ietf.doc.views_search.recent_drafts, and a lambda is not picklable. + """ + + def __init__(self, value): + self.value = value + + def __call__(self): + return self.value + + def __eq__(self, other): + return isinstance(other, wrap_value) and self.value == other.value + + def __hash__(self): + # Defining __eq__ without this would set __hash__ to None and make instances + # unhashable, which a method they stand in for is not. + return hash(self.value) + + def __repr__(self): + return f"wrap_value({self.value!r})" + + +# Relationships the document table renders for each row. RELATED_THAT holds the ones +# read in the "documents pointing at this one" direction (Document.related_that), +# RELATED_THAT_DOC the ones read in the "documents this one points at" direction +# (Document.related_that_doc). +RELATED_THAT = ("replaces", "contains") +RELATED_THAT_DOC = ("became_rfc", "replaces") + +# Followed transitively to find the documents whose IPR disclosures count as related. +IPR_RELATED = ("obs", "replaces") + + +def fill_in_document_relations(docs, doc_dict, doc_ids): + """Seed each document's relation caches from two queries. + + Document.related_that/related_that_doc otherwise run one query per document per + relationship, and the table reads several of them for every row (friendly_state, + part_of, replaces, became_rfc). + """ + for d in docs: + d._cached_related_that = {name: [] for name in RELATED_THAT} + d._cached_related_that_doc = {name: [] for name in RELATED_THAT_DOC} + + for rel in RelatedDocument.objects.filter( + target_id__in=doc_ids, relationship__in=RELATED_THAT + ).select_related("source"): + doc_dict[rel.target_id]._cached_related_that[rel.relationship_id].append(rel.source) + + for rel in RelatedDocument.objects.filter( + source_id__in=doc_ids, relationship__in=RELATED_THAT_DOC + ).select_related("target"): + doc_dict[rel.source_id]._cached_related_that_doc[rel.relationship_id].append(rel.target) + + for d in docs: + # related_that/related_that_doc deduplicate; match that. + for cache in (d._cached_related_that, d._cached_related_that_doc): + for name, related in cache.items(): + cache[name] = list({r.pk: r for r in related}.values()) + d._cached_became_rfc = next(iter(d._cached_related_that_doc["became_rfc"]), None) + + # For each subseries document a row is part of, the table also reads what that + # subseries contains. Those documents are not in `docs`, so seed them here rather + # than leaving a query per subseries membership. + subseries = defaultdict(list) + for d in docs: + for sub in d._cached_related_that["contains"]: + subseries[sub.pk].append(sub) + if subseries: + contains = defaultdict(list) + for rel in RelatedDocument.objects.filter( + source_id__in=subseries, relationship_id="contains" + ).select_related("target"): + contains[rel.source_id].append(rel.target) + for pk, instances in subseries.items(): + targets = list({r.pk: r for r in contains[pk]}.values()) + for sub in instances: + sub._cached_related_that_doc = {"contains": targets} + + +def fill_in_related_ipr(docs, doc_dict, doc_ids): + """Attach the related IPR disclosure ids to each document. + + Document.related_ipr walks the obs/replaces graph with Document.all_relations_that_doc, + which issues a query per node it visits, per document. Here the graph is walked once + for the whole result set -- one query per level of depth -- and the disclosures are + fetched in a single query. + """ + from ietf.ipr.models import IprDocRel + + edges = defaultdict(set) + seen = set(doc_ids) + front = set(doc_ids) + while front: + next_front = set() + for source_id, target_id in RelatedDocument.objects.filter( + source_id__in=front, relationship__in=IPR_RELATED + ).values_list("source_id", "target_id"): + edges[source_id].add(target_id) + if target_id not in seen: + seen.add(target_id) + next_front.add(target_id) + front = next_front + + def reachable_from(start): + """start plus every document it directly or indirectly obsoletes or replaces.""" + found = {start} + stack = [start] + while stack: + for target_id in edges[stack.pop()]: + if target_id not in found: + found.add(target_id) + stack.append(target_id) + return found + + reachable = {pk: reachable_from(pk) for pk in doc_ids} + + disclosures = defaultdict(set) + involved = set().union(*reachable.values()) if reachable else set() + for document_id, disclosure_id in IprDocRel.objects.filter( + document_id__in=involved, disclosure__state__in=settings.PUBLISH_IPR_STATES + ).values_list("document_id", "disclosure_id"): + disclosures[document_id].add(disclosure_id) + + for d in docs: + related = set() + for pk in reachable[d.pk]: + related |= disclosures[pk] + # Wrapped rather than assigned bare so that the attribute stays callable, like + # the Document.related_ipr method it shadows. Templates auto-call either way, + # but a bare list would turn doc.related_ipr() into a TypeError for any Python + # caller handed a prepared document. + d.related_ipr = wrap_value(sorted(related)) + + +def fill_in_rfc_editor_queue_status(docs, doc_dict, doc_ids): + """Attach each document's RFC Editor publication queue status. + + The status column shows it for every document sitting in the RFC Editor queue, and + Document.rfc_editor_queue_status() costs a query per such row to find the latest + RpcAssignmentDocEvent. Here they take one query between them. + + Finding which documents are queued reads each document's states, which this assumes + is free -- either prefetched by the caller, as prepare_document_table() does, or + already cached on the instances by an earlier get_state(). Absent both, that read + costs a query per document, which is the sort of per-row cost this exists to avoid. + """ + queued_ids = [ + d.pk + for d in docs + if d.get_state_slug("draft-rfceditor") in ("in_progress", "blocked") + ] + # Wrapped rather than assigned bare so the attribute stays callable, like the + # Document.rfc_editor_queue_status method it shadows. + for d in docs: + d.rfc_editor_queue_status = wrap_value(None) + if not queued_ids: + return + + # DISTINCT ON fetches only the newest event per document; a document that has moved + # through the queue has one for every status it has held. + for e in (RpcAssignmentDocEvent.objects + .filter(doc_id__in=queued_ids, type="changed_rpc_assignments") + .order_by("doc_id", "-time", "-id") + .distinct("doc_id")): + doc_dict[e.doc_id].rfc_editor_queue_status = wrap_value(e.assignments) + + +def fill_in_person_caches(docs): + """Seed the per-instance caches person_link and email_person_link read. + + select_related hands every row its own Person instance, so Person.email() and + Person.has_alias_for_name() each cost a query per person the table names -- the AD, + the shepherd, and any action holders. The emails come from the prefetches set up in + prepare_document_table; the aliases need one query between all of them. + """ + # People whose address the table renders via Person.email(). Their email_set is + # prefetched in prepare_document_table. Action holders are only read when the + # document has them enabled, which is the same condition the template applies -- so + # this costs nothing extra for callers that skipped the prefetch. + with_email = [d.ad for d in docs if d.ad_id] + for d in docs: + if d.action_holders_enabled(): + with_email.extend(holder.person for holder in d.documentactionholder_set.all()) + # The shepherd column renders the address it already holds, but still needs an alias. + shepherds = [d.shepherd.person for d in docs if d.shepherd_id and d.shepherd.person_id] + + people = with_email + shepherds + if not people: + return + + aliased = set( + Alias.objects.filter(person__in={p.pk for p in people}).values_list("person_id", "name") + ) + for person in people: + person._cached_has_alias_for_name = (person.pk, person.name) in aliased + + for person in with_email: + if hasattr(person, "_cached_email"): + continue + emails = list(person.email_set.all()) + # Mirror Person.email(): a primary address if there is one -- lowest by address, + # which is the pk an unordered first() would have ordered by -- and otherwise + # the most recent active one. Email.address is a CICharField, so the database + # orders it case-insensitively; casefold the key to match. + primary = sorted((e for e in emails if e.primary), key=lambda e: e.address.lower()) + if primary: + person._cached_email = primary[0] + else: + active = sorted( + (e for e in emails if e.active), + key=lambda e: (e.time, e.address.lower()), + reverse=True, + ) + person._cached_email = active[0] if active else None def fill_in_telechat_date(docs, doc_dict=None, doc_ids=None): @@ -29,12 +250,19 @@ def fill_in_telechat_date(docs, doc_dict=None, doc_ids=None): doc_ids = list(doc_dict.keys()) seen = set() - for e in TelechatDocEvent.objects.filter(doc__id__in=doc_ids, type="scheduled_for_telechat").order_by('-time'): + for e in TelechatDocEvent.objects.filter(doc__id__in=doc_ids, type="scheduled_for_telechat").order_by('-time', '-id'): if e.doc_id not in seen: - #d = doc_dict[e.doc_id] - #d.telechat_date = wrap_value(d.telechat_date(e)) + d = doc_dict[e.doc_id] + # Shadow Document.telechat_date with a callable returning the precomputed + # value, so templates can keep calling doc.telechat_date without each row + # issuing its own latest_event() query. + d.telechat_date = wrap_value(d.telechat_date(e)) seen.add(e.doc_id) + for pk, d in doc_dict.items(): + if pk not in seen: + d.telechat_date = wrap_value(None) + def fill_in_document_sessions(docs, doc_dict, doc_ids): today = date_today() beg_date = today-datetime.timedelta(days=7) @@ -72,15 +300,31 @@ def fill_in_document_table_attributes(docs, have_telechat_date=False): for e in event_types: d.latest_event_cache[e] = None - for e in DocEvent.objects.filter(doc__id__in=doc_ids, type__in=event_types).order_by('time'): + # DISTINCT ON fetches only the newest event of each (doc, type) pair. Ordering + # ascending and letting later rows overwrite earlier ones pulls back every matching + # event, and a draft routinely has dozens of new_revision events. + for e in (DocEvent.objects + .filter(doc__id__in=doc_ids, type__in=event_types) + .order_by('doc_id', 'type', '-time', '-id') + .distinct('doc_id', 'type')): doc_dict[e.doc_id].latest_event_cache[e.type] = e + # Default to None so that ballot_icon finds the attribute for documents with no + # ballot event at all. Otherwise it falls back to doc.active_ballot(), which costs a + # query per such row. + for d in docs: + d.ballot = None seen = set() for e in BallotDocEvent.objects.filter(doc__id__in=doc_ids, type__in=('created_ballot', 'closed_ballot')).order_by('-time','-id'): if not e.doc_id in seen: doc_dict[e.doc_id].ballot = e if e.type == 'created_ballot' else None seen.add(e.doc_id) + fill_in_document_relations(docs, doc_dict, doc_ids) + fill_in_related_ipr(docs, doc_dict, doc_ids) + fill_in_rfc_editor_queue_status(docs, doc_dict, doc_ids) + fill_in_person_caches(docs) + if not have_telechat_date: fill_in_telechat_date(docs, doc_dict, doc_ids) @@ -90,6 +334,13 @@ def fill_in_document_table_attributes(docs, have_telechat_date=False): # misc expirable_pks = expirable_drafts(Document.objects.filter(pk__in=doc_ids)).values_list('pk', flat=True) + + # Look up review assignments for every draft at once. Calling this per document, as + # the loop below used to, repeats a breadth-first walk of the replaces graph and an + # assignment query for each row. + review_docs = [d for d in docs if d.type_id == "draft" and d.get_state_slug() != "rfc"] + review_assignments = review_assignments_to_list_for_docs(review_docs) if review_docs else {} + for d in docs: if d.type_id == "rfc" and d.latest_event_cache["published_rfc"]: @@ -121,8 +372,10 @@ def fill_in_document_table_attributes(docs, have_telechat_date=False): d.expirable = False if d.type_id == "draft" and d.get_state_slug() != "rfc": - d.milestones = [ m for (t, s, v, m) in sorted(((m.time, m.state.slug, m.desc, m) for m in d.groupmilestone_set.all() if m.state_id == "active")) ] - d.review_assignments = review_assignments_to_list_for_docs([d]).get(d.name, []) + # m.state_id is the state slug; reading m.state.slug instead costs a query + # per milestone because the prefetch does not cover it. + d.milestones = [ m for (t, s, v, m) in sorted(((m.time, m.state_id, m.desc, m) for m in d.groupmilestone_set.all() if m.state_id == "active")) ] + d.review_assignments = review_assignments.get(d.name, []) e = d.latest_event_cache.get('started_iesg_process', None) d.balloting_started = e.time if e else datetime.datetime.min @@ -147,7 +400,7 @@ def fill_in_document_table_attributes(docs, have_telechat_date=False): RelatedDocument.objects.filter( target__name__in=list(rfcs.values()), relationship__in=("obs", "updates"), - ).select_related("target") + ).select_related("target", "source") ) # TODO - this likely reduces to something even simpler rel_rfcs = { @@ -193,9 +446,14 @@ def prepare_document_table(request, docs, query=None, max_results=200, show_ad_a if not isinstance(docs, list): # evaluate and fill in attribute results immediately to decrease # the number of queries - docs = docs.select_related("ad", "std_level", "intended_std_level", "group", "stream", "shepherd", ) + # "type" is here because fill_in_document_table_attributes renders it into + # search_heading for every non-draft row. "iprdocrel_set" is not: the table shows + # doc.related_ipr, which is precomputed in fill_in_document_table_attributes and + # never touches that relation. + docs = docs.select_related("ad", "std_level", "intended_std_level", "group", "stream", + "shepherd__person", "type", ) docs = docs.prefetch_related("states__type", "tags", "groupmilestone_set__group", "reviewrequest_set__team", - "ad__email_set", "iprdocrel_set") + "ad__email_set", "documentactionholder_set__person__email_set") docs = docs[:max_results] # <- that is still a queryset, but with a LIMIT now docs = list(docs) else: diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index e899f592271..b2f400d6d6f 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -20,6 +20,7 @@ from django.utils import timezone from django.utils.encoding import force_str from django.utils.html import escape +from django.db.models.functions import Collate import debug # pyflakes:ignore @@ -305,8 +306,14 @@ def change_title(request, name, option=None): )) class AdForm(forms.Form): - ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active", role__group__type="area").order_by('name'), - label="Responsible AD", empty_label="(None)", required=True) + ad = forms.ModelChoiceField( + Person.objects.filter( + role__name="ad", role__group__state="active", role__group__type="area" + ).order_by(Collate("name", settings.PREFERRED_COLLATION)), + label="Responsible AD", + empty_label="(None)", + required=True, + ) def __init__(self, *args, **kwargs): super(self.__class__, self).__init__(*args, **kwargs) diff --git a/ietf/doc/views_conflict_review.py b/ietf/doc/views_conflict_review.py index 159f1340a49..495958133ad 100644 --- a/ietf/doc/views_conflict_review.py +++ b/ietf/doc/views_conflict_review.py @@ -14,6 +14,7 @@ from django.template.loader import render_to_string from django.conf import settings from django.utils.html import escape +from django.db.models.functions import Collate import debug # pyflakes:ignore @@ -404,8 +405,14 @@ class SimpleStartReviewForm(forms.Form): ) class StartReviewForm(forms.Form): - ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active",role__group__type='area').order_by('name'), - label="Shepherding AD", empty_label="(None)", required=True) + ad = forms.ModelChoiceField( + Person.objects.filter( + role__name="ad", role__group__state="active", role__group__type="area" + ).order_by(Collate("name", settings.PREFERRED_COLLATION)), + label="Shepherding AD", + empty_label="(None)", + required=True, + ) create_in_state = forms.ModelChoiceField(State.objects.filter(used=True, type="conflrev", slug__in=("needshep", "adrev")), empty_label=None, required=False) notify = forms.CharField( widget=forms.Textarea, diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 1472c808eb0..6ae100e6cb0 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -60,9 +60,9 @@ from ietf.doc.models import ( Document, DocHistory, DocEvent, BallotDocEvent, BallotType, ConsensusDocEvent, NewRevisionDocEvent, StoredObject, TelechatDocEvent, WriteupDocEvent, IanaExpertDocEvent, IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder, DocumentAuthor, - RelatedDocument, RelatedDocHistory, RpcAssignmentDocEvent) + RelatedDocument, RelatedDocHistory, RpcActionHolderOpenEntry, RpcAssignmentDocEvent) from ietf.doc.tasks import investigate_fragment_task -from ietf.doc.utils import (augment_events_with_revision, +from ietf.doc.utils import (augment_events_with_revision, show_rpc_action_holder_comments, can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, @@ -213,6 +213,20 @@ def rfc_editor_queue_status(doc): event = doc.latest_event(RpcAssignmentDocEvent, type="changed_rpc_assignments") return event.assignments if event else None +def rpc_action_holders(doc): + """Open action holder entries the RPC has for this document + + Only entries naming a person the datatracker knows: an action held by a + body, or by the RPC tool's own placeholder person, is not something to show + here - the queue status already reports that the RPC is waiting on someone. + """ + return list( + RpcActionHolderOpenEntry.objects.filter( + document=doc, person__isnull=False + ).select_related("person").order_by("since_when") + ) + + def document_main(request, name, rev=None, document_html=False): doc = get_object_or_404(Document.objects.select_related(), name=name) @@ -380,7 +394,7 @@ def document_main(request, name, rev=None, document_html=False): has_errata=doc.pk and doc.tags.filter(slug="errata"), # doc.pk == None if using a fake_history_obj file_urls=file_urls, rfc_editor_state=doc.get_state("draft-rfceditor"), - rfc_editor_queue_status=rfc_editor_queue_status(doc), + rfc_editor_queue_status=doc.rfc_editor_queue_status(), iana_review_state=doc.get_state("draft-iana-review"), iana_action_state=doc.get_state("draft-iana-action"), iana_experts_state=doc.get_state("draft-iana-experts"), @@ -406,7 +420,8 @@ def document_main(request, name, rev=None, document_html=False): if isinstance(doc, Document): log.assertion('iesg_state', note="A document's 'draft-iesg' state should never be unset'. Failed for %s"%doc.name) iesg_state_slug = iesg_state.slug if iesg_state else None - iesg_state_summary = doc.friendly_state() + # Rendered in a row of its own labeled "IESG state", so not labeled again here. + iesg_state_summary = doc.friendly_state(label_iesg_state=False) irsg_state = doc.get_state("draft-stream-irtf") can_edit = has_role(request.user, ("Area Director", "Secretariat")) @@ -664,6 +679,11 @@ def document_main(request, name, rev=None, document_html=False): if html: css += Path(finders.find("ietf/css/document_html_txt.css")).read_text() + # Actions the RPC is waiting on, while the document is in its queue. + # The entries describe the document as it stands now, so a view of an + # earlier revision has none. + doc_rpc_action_holders = [] if snapshot else rpc_action_holders(doc) + return render(request, "doc/document_draft.html" if document_html is False else "doc/document_html.html", dict(doc=doc, document_html=document_html, @@ -724,8 +744,11 @@ def document_main(request, name, rev=None, document_html=False): iesg_state=iesg_state, iesg_state_summary=iesg_state_summary, rfc_editor_state=doc.get_state("draft-rfceditor"), - rfc_editor_queue_status=rfc_editor_queue_status(doc), + rfc_editor_queue_status=doc.rfc_editor_queue_status(), rfc_editor_auth48_url=auth48_url, + rpc_action_holders=doc_rpc_action_holders, + show_rpc_action_holder_comments=show_rpc_action_holder_comments( + request.user, doc_rpc_action_holders), iana_review_state=doc.get_state("draft-iana-review"), iana_action_state=doc.get_state("draft-iana-action"), iana_experts_state=doc.get_state("draft-iana-experts"), diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index a64d0a53fe9..a0904afa0bc 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -20,6 +20,8 @@ from django.forms.utils import ErrorList from django.template.defaultfilters import pluralize from django.utils import timezone +from django.db.models.functions import Collate + import debug # pyflakes:ignore @@ -1130,7 +1132,7 @@ class AdForm(forms.Form): role__name__in=("ad", "pre-ad"), role__group__state="active", role__group__type="area", - ).order_by('name'), + ).order_by(Collate('name', settings.PREFERRED_COLLATION)), label="Shepherding AD", empty_label="(None)", required=False, diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 1968b133ce1..17eb4d40b47 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -249,7 +249,8 @@ def review_request(request, name, request_id): if review_req.doc.group: wg_chairs = [role.person for role in review_req.doc.group.role_set.filter(name__slug='chair')] - iesg_state_summary = review_req.doc.friendly_state() + # Rendered in a row of its own labeled "IESG document state", so not labeled again here. + iesg_state_summary = review_req.doc.friendly_state(label_iesg_state=False) history = list(review_req.history.all()) history += itertools.chain(*[list(r.history.all()) for r in review_req.reviewassignment_set.all()]) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 5d59adb349f..9da1aa04529 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -41,13 +41,14 @@ import operator from collections import defaultdict +from django_stubs_ext import QuerySetAny from functools import reduce from django import forms from django.conf import settings from django.core.cache import cache, caches from django.urls import reverse as urlreverse -from django.db.models import Q +from django.db.models import Count, Model, Q from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect, QueryDict from django.shortcuts import render from django.utils import timezone @@ -55,15 +56,17 @@ from django.utils.cache import _generate_cache_key # type: ignore from django.utils.text import slugify - import debug # pyflakes:ignore -from ietf.doc.models import ( Document, DocHistory, State, - NewRevisionDocEvent, IESG_SUBSTATE_TAGS, +from ietf.doc.models import ( Document, DocHistory, DocumentAuthor, RelatedDocument, + RfcAuthor, RpcActionHolderOpenEntry, State, NewRevisionDocEvent, IESG_SUBSTATE_TAGS, IESG_BALLOT_ACTIVE_STATES, IESG_STATCHG_CONFLREV_ACTIVE_STATES, IESG_CHARTER_ACTIVE_STATES ) from ietf.doc.fields import select2_id_doc_name_json -from ietf.doc.utils import augment_events_with_revision, needed_ballot_positions +from ietf.doc.utils import ( + augment_events_with_revision, + needed_ballot_positions, +) from ietf.group.models import Group from ietf.idindex.index import active_drafts_index_by_group from ietf.name.models import DocTagName, DocTypeName, StreamName @@ -196,27 +199,38 @@ def retrieve_search_results(form, all_types=False): Q(title__icontains=singlespace) ]) + # Matches against a related document are expressed as a subquery on pk rather + # than by following the multi-valued `targets_related` relation. A join here + # would cross-multiply with any other multi-valued relation ORed into the same + # filter (notably the author search below), and the intermediate result explodes. + def related_source_matches(relationship, **source_lookup): + return Q( + pk__in=RelatedDocument.objects.filter( + relationship_id=relationship, **source_lookup + ).values("target_id") + ) + # Do a similar thing if the search is just for a subseries doc, like a bcp. if look_for.lower()[:3] in ["bcp", "fyi", "std"] and look_for[3:].strip().isdigit() and query["rfcs"]: # Also look for rfcs contained in the subseries. queries.extend([ - Q(targets_related__source__name__icontains=look_for, targets_related__relationship_id="contains"), - Q(targets_related__source__title__icontains=look_for, targets_related__relationship_id="contains"), + related_source_matches("contains", source__name__icontains=look_for), + related_source_matches("contains", source__title__icontains=look_for), ]) spaceless = look_for.lower()[:3]+look_for[3:].strip() if spaceless != look_for: queries.extend([ - Q(targets_related__source__name__icontains=spaceless, targets_related__relationship_id="contains"), - Q(targets_related__source__title__icontains=spaceless, targets_related__relationship_id="contains"), + related_source_matches("contains", source__name__icontains=spaceless), + related_source_matches("contains", source__title__icontains=spaceless), ]) singlespace = look_for.lower()[:3]+" "+look_for[3:].strip() if singlespace != look_for: queries.extend([ - Q(targets_related__source__name__icontains=singlespace, targets_related__relationship_id="contains"), - Q(targets_related__source__title__icontains=singlespace, targets_related__relationship_id="contains"), + related_source_matches("contains", source__name__icontains=singlespace), + related_source_matches("contains", source__title__icontains=singlespace), ]) if query["rfcs"]: - queries.extend([Q(targets_related__source__name__icontains=look_for, targets_related__relationship_id="became_rfc")]) + queries.append(related_source_matches("became_rfc", source__name__icontains=look_for)) combined_query = reduce(operator.or_, queries) docs = docs.filter(combined_query) @@ -228,18 +242,41 @@ def retrieve_search_results(form, all_types=False): if query["olddrafts"]: allowed_draft_states.extend(['repl', 'expired', 'auth-rm', 'ietf-rm']) - docs = docs.filter(Q(states__slug__in=allowed_draft_states) | - ~Q(type__slug='draft')) + if allowed_draft_states: + # Subquery rather than a join on `states`: a document has several state rows, so + # joining here duplicates every document that passes the ~Q(type__slug='draft') + # half of the OR, which is what forced the distinct() this function used to end + # with. See the comment before the return. + docs = docs.filter( + ~Q(type__slug='draft') + | Q(pk__in=Document.states.through.objects.filter( + state__slug__in=allowed_draft_states + ).values("document_id")) + ) + elif all_types: + # No draft state is allowed, so no draft can match. Only the all_types path can + # still be holding drafts at this point -- when types drives the queryset above, + # "draft" is in it only if at least one draft state is allowed. + docs = docs.exclude(type__slug='draft') # radio choices by = query["by"] if by == "author": + # Resolve the name or address to people first and match documents against those + # people by primary key. Expressed as ORed joins, the documentauthor and + # rfcauthor paths (each reaching person -> alias and person -> email) cross- + # multiply into an enormous intermediate result. + author = query["author"] + person_ids = Person.objects.filter( + Q(alias__name__icontains=author) | Q(email__address__icontains=author) + ).values("pk") docs = docs.filter( - Q(documentauthor__person__alias__name__icontains=query["author"]) | - Q(documentauthor__person__email__address__icontains=query["author"]) | - Q(rfcauthor__person__alias__name__icontains=query["author"]) | - Q(rfcauthor__person__email__address__icontains=query["author"]) | - Q(rfcauthor__titlepage_name__icontains=query["author"]) + Q(pk__in=DocumentAuthor.objects.filter( + person__in=person_ids + ).values("document_id")) + | Q(pk__in=RfcAuthor.objects.filter( + Q(person__in=person_ids) | Q(titlepage_name__icontains=author) + ).values("document_id")) ) elif by == "group": docs = docs.filter(group__acronym__iexact=query["group"]) @@ -258,22 +295,53 @@ def retrieve_search_results(form, all_types=False): elif by == "stream": docs = docs.filter(stream=query["stream"]) - docs=docs.distinct() + # No distinct() here: every filter above matches a document at most once. The + # multi-valued relations (documentauthor, rfcauthor, targets_related, states) are all + # reached through pk subqueries, and the remaining `states`/`tags` filters match a + # single specific row. A distinct() would be applied to the full column list -- which + # prepare_document_table then widens further with select_related() -- and force a sort + # over every selected column, including abstract, biography and group description. + # If a multi-valued join is ever added back above, restore the distinct() with it. # order by time here to retain the most recent documents in case we - # find too many and have to chop the results list in prepare_document_table - docs = docs.order_by('-time') + # find too many and have to chop the results list in prepare_document_table. + # `time` alone is not a total order -- documents sharing a timestamp to the second + # are common -- so tie-break on pk. Without it the set of documents kept by that + # truncation, and the order prepare_document_table's stable sort falls back to for + # equal sort keys, both vary from one execution to the next. + docs = docs.order_by('-time', 'pk') return docs -def search(request): - def _get_cache_key(params): - fields = set(SearchForm.base_fields) - {'sort'} - kwargs = dict([(k, v) for (k, v) in list(params.items()) if k in fields]) - key = "doc:document:search:" + hashlib.sha512(json.dumps(kwargs, sort_keys=True).encode('utf-8')).hexdigest() - return key +def _search_cache_key(form): + """Cache key for a validated SearchForm. + Derived from cleaned_data rather than the raw query string. The raw parameters vary + in ways that do not change the search ("on" vs "1" for a checkbox), and + QueryDict.items() returns only the last value of a multi-valued field, so + ?doctypes=charter&doctypes=statchg and ?doctypes=statchg used to share a key and + serve each other's results. + + 'sort' is excluded deliberately: the cached value is the list of matching document + ids, and prepare_document_table applies the requested sort on every request, so one + entry serves every sort order. + """ + def normalize(value): + if isinstance(value, QuerySetAny): # ModelMultipleChoiceField, e.g. doctypes + return sorted(str(obj.pk) for obj in value) + if isinstance(value, Model): # ModelChoiceField, e.g. area, state + return str(value.pk) + return value + + kwargs = {k: normalize(v) for k, v in form.cleaned_data.items() if k != "sort"} + digest = hashlib.sha512( + json.dumps(kwargs, sort_keys=True, default=str).encode("utf-8") + ).hexdigest() + return "doc:document:search:" + digest + + +def search(request): if request.GET: # backwards compatibility get_params = request.GET.copy() @@ -288,15 +356,31 @@ def _get_cache_key(params): if not form.is_valid(): return HttpResponseBadRequest("form not valid: %s" % form.errors) - cache_key = _get_cache_key(get_params) - cached_val = cache.get(cache_key) - if cached_val: - [results, meta] = cached_val - else: + # Cache the ids of the matching documents, not the prepared Document objects. + # Pickling the prepared objects produced payloads over memcached's 1MB item + # limit for large result sets (measured at 1.28MB for 200 rows), which + # LenientMemcacheCache silently discards -- so the most expensive searches were + # never cached at all. It also meant a cached result carried the sort order it + # was first computed with, since 'sort' is not part of the key. + cache_key = _search_cache_key(form) + cached_pks = cache.get(cache_key) + if cached_pks is None: results = retrieve_search_results(form) results, meta = prepare_document_table(request, results, get_params) - cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS + cache.set( # for settings.CACHE_MIDDLEWARE_SECONDS + cache_key, [doc.pk for doc in results] + ) log(f"Search results computed for {get_params}") + else: + # Same ordering as retrieve_search_results, so a hit hands + # prepare_document_table the rows in the order the miss did. Its sort is + # stable and several sort keys tie heavily (ipr, status, ad), so an + # unordered pk__in here would reorder those pages between requests. + results, meta = prepare_document_table( + request, + Document.objects.filter(pk__in=cached_pks).order_by('-time', 'pk'), + get_params, + ) meta['searching'] = True else: form = SearchForm() @@ -622,10 +706,28 @@ def _state_to_doc_type(state): for dt in AD_WORKLOAD } + # Current state, not a trend: the open entries the RPC pushes are a + # snapshot, so there is no history to bucket the way the tables above do. + rpc_pending_counts = dict( + RpcActionHolderOpenEntry.objects.filter(person__in=ads) + .values_list("person") + .annotate(Count("id")) + ) + rpc_pending = [ + {"ad": ad, "count": rpc_pending_counts[ad.pk]} + for ad in ads + if rpc_pending_counts.get(ad.pk) + ] + return render( request, "doc/ad_list.html", - {"metadata": metadata, "data": data, "delta": days}, + { + "metadata": metadata, + "data": data, + "delta": days, + "rpc_pending": rpc_pending, + }, ) @@ -750,6 +852,13 @@ def sort_key(doc): if re.search(r"\bNeeds\s+\d+", iesg_ballot_summary): not_balloted_docs.append(doc) + # Actions the RPC is waiting on this AD for, from the publication queue. + rpc_action_holders = list( + RpcActionHolderOpenEntry.objects.filter(person=ad) + .select_related("document") + .order_by("since_when") + ) + return render( request, "doc/drafts_for_ad.html", @@ -759,6 +868,7 @@ def sort_key(doc): "ad": ad, "blocked_docs": blocked_docs, "not_balloted_docs": not_balloted_docs, + "rpc_action_holders": rpc_action_holders, }, ) diff --git a/ietf/doc/views_status_change.py b/ietf/doc/views_status_change.py index 2bccc213c40..a5db6e4bdd0 100644 --- a/ietf/doc/views_status_change.py +++ b/ietf/doc/views_status_change.py @@ -18,6 +18,7 @@ from django.conf import settings from django.utils.encoding import force_str from django.utils.html import escape +from django.db.models.functions import Collate import debug # pyflakes:ignore from ietf.doc.mails import email_ad_approved_status_change @@ -484,8 +485,14 @@ def clean(self): class StartStatusChangeForm(forms.Form): document_name = forms.CharField(max_length=255, label="Document name", help_text="A descriptive name such as status-change-md2-to-historic is better than status-change-rfc1319.", required=True) title = forms.CharField(max_length=255, label="Title", required=True) - ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active",role__group__type='area').order_by('name'), - label="Shepherding AD", empty_label="(None)", required=False) + ad = forms.ModelChoiceField( + Person.objects.filter( + role__name="ad", role__group__state="active", role__group__type="area" + ).order_by(Collate("name", settings.PREFERRED_COLLATION)), + label="Shepherding AD", + empty_label="(None)", + required=False, + ) create_in_state = forms.ModelChoiceField(State.objects.filter(type="statchg", slug__in=("needshep", "adrev")), empty_label=None, required=False) notify = forms.CharField( widget=forms.Textarea, diff --git a/ietf/group/models.py b/ietf/group/models.py index a7e3c6616e7..dfa710627e7 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -511,35 +511,3 @@ def notify_rfceditor_of_group_name_change(sender, instance=None, **kwargs): """ % (current.acronym, current.name, instance.name, ) send_mail_text(None, to=addr, frm=None, subject="Group '%s' name change"%instance.acronym, txt=msg) log.log("Sent notification email: %s: '%s' --> '%s' to %s" % (current.acronym, current.name, instance.name, addr)) - - -## Keep this code as a worked and tested example of sending signed notifies -## by HTTP POST. (superseded for this use case by email notification) -# url = settings.RFC_EDITOR_GROUP_NOTIFICATION_URL -# if url and instance.name != current.name: -# data = { -# 'acronym': current.acronym, -# 'old_name': current.name, -# 'name': instance.name, -# } -# # Build signed data -# key = jwk.JWK() -# key.import_from_pem(settings.API_PRIVATE_KEY_PEM) -# payload = json.dumps(data) -# jwstoken = jws.JWS(payload.encode('utf-8')) -# jwstoken.add_signature(key, None, -# json_encode({"alg": settings.API_KEY_TYPE}), -# json_encode({"kid": key.thumbprint()})) -# sig = jwstoken.serialize() -# # Send signed data -# response = requests.post(url, data = { 'jws': sig, }) -# log.log("Sent notify: %s: '%s' --> '%s' to %s, result code %s" % -# (current.acronym, current.name, instance.name, url, response.status_code)) -# # Verify locally, to make sure we've got things right -# key = jwk.JWK() -# key.import_from_pem(settings.API_PUBLIC_KEY_PEM) -# jwstoken = jws.JWS() -# jwstoken.deserialize(sig) -# jwstoken.verify(key) -# log.assertion('payload == jwstoken.payload') - diff --git a/ietf/iesg/views.py b/ietf/iesg/views.py index af04ad06fd5..d65d923ad9c 100644 --- a/ietf/iesg/views.py +++ b/ietf/iesg/views.py @@ -383,7 +383,9 @@ def agenda_documents(request): docs_by_date = dict((d, []) for d in dates) docs = Document.objects.filter(docevent__telechatdocevent__telechat_date__in=dates).distinct() docs = docs.select_related("ad", "std_level", "intended_std_level", "group", "stream", "shepherd", ) - # No prefetch-related -- turns out not to be worth it + # The states and tags the status column of every row reads, each of which otherwise + # costs a query per document. + docs = docs.prefetch_related("states__type", "tags") fill_in_telechat_date(docs) for doc in docs: diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index a77e5bd5d58..9b793bf6ccb 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -34,7 +34,13 @@ from ietf.ietfauth.utils import has_role from ietf.meeting.factories import MeetingFactory, RegistrationFactory, RegistrationTicketFactory from ietf.nomcom.factories import NomComFactory -from ietf.person.factories import PersonFactory, EmailFactory, UserFactory, PersonalApiKeyFactory +from ietf.person.factories import ( + PersonFactory, + EmailFactory, + UserFactory, + PersonalApiKeyFactory, + PersonUUIDFactory, +) from ietf.person.models import Person, Email from ietf.person.tasks import send_apikey_usage_emails_task from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory @@ -657,7 +663,9 @@ def test_change_password(self): ) user.set_password(VALID_PASSWORD) user.save() - p = Person.objects.create(name="Some One", ascii="Some One", user=user) + p = PersonFactory( + user=user, name="Some One", ascii="Some One", default_emails=False + ) Email.objects.create(address=user.username, person=p, origin=user.username) # log in @@ -758,7 +766,9 @@ def test_change_username(self): ) user.set_password(VALID_PASSWORD) user.save() - p = Person.objects.create(name="Some One", ascii="Some One", user=user) + p = PersonFactory( + user=user, name="Some One", ascii="Some One", default_emails=False + ) Email.objects.create(address=user.username, person=p, origin=user.username) Email.objects.create( address="othername@example.org", person=p, origin=user.username @@ -1162,7 +1172,16 @@ def test_oidc_code_auth(self): session["nonce"] = rndstr() args = { "response_type": "code", - "scope": ['openid', 'profile', 'email', 'roles', 'registration', 'dots', 'pronouns' ], + "scope": [ + "openid", + "profile", + "email", + "roles", + "registration", + "dots", + "pronouns", + "datatracker_uuid", + ], "nonce": session["nonce"], "redirect_uri": redirect_uris[0], "state": session["state"] @@ -1207,6 +1226,10 @@ def test_oidc_code_auth(self): self.assertIn(key, access_token_info) for key in ['iss', 'sub', 'aud', 'exp', 'iat', 'auth_time', 'nonce', 'at_hash']: self.assertIn(key, access_token_info['id_token']) + # Custom claims are served from userinfo, not the id_token. This guards + # against an accidental OIDC_IDTOKEN_INCLUDE_CLAIMS flip. + for key in ["datatracker_uuid", "datatracker_prior_uuids"]: + self.assertNotIn(key, access_token_info["id_token"]) # Get userinfo, check keys present, most common scenario userinfo = client.do_user_info_request(state=params["state"], scope=args['scope']) @@ -1218,6 +1241,18 @@ def test_oidc_code_auth(self): self.assertNotIn('hackathon_onsite', set(userinfo['reg_type'].split())) self.assertIn(active_group.acronym, [i[1] for i in userinfo['roles']]) self.assertNotIn(closed_group.acronym, [i[1] for i in userinfo['roles']]) + self.assertEqual(userinfo['datatracker_uuid'], str(person.primary_uuid)) + # Present and empty, not absent, for a Person that has never been merged + self.assertIn("datatracker_prior_uuids", userinfo) + self.assertEqual(userinfo["datatracker_prior_uuids"], []) + + # A UUID absorbed by a merge shows up in the prior list + absorbed = PersonUUIDFactory(person=person) + userinfo = client.do_user_info_request( + state=params["state"], scope=args["scope"] + ) + self.assertEqual(userinfo["datatracker_uuid"], str(person.primary_uuid)) + self.assertEqual(userinfo["datatracker_prior_uuids"], [str(absorbed.uuid)]) # Create a registration, with only email, no person (rare if at all) reg_person.delete() diff --git a/ietf/ietfauth/utils.py b/ietf/ietfauth/utils.py index 30d51cddd09..fcce43dc2dc 100644 --- a/ietf/ietfauth/utils.py +++ b/ietf/ietfauth/utils.py @@ -341,6 +341,29 @@ def scope_dots(self): dots = get_dots(self.user.person) return { 'dots': dots } + info_datatracker_uuid = ( + "Datatracker person identifiers", + ( + "Access to the stable identifier the datatracker uses for you when " + "telling other systems who you are, and to any identifiers it used for " + "you before they were superseded." + ), + ) + + def scope_datatracker_uuid(self): + # One scope for both claims: there is no case for granting the current + # identifier without the superseded ones that resolve to it. + person = self.user.person + return { + # An empty string is dropped by ScopeClaims._clean_dic, so an inconsistent + # Person yields an absent claim rather than a bogus identifier. + "datatracker_uuid": str(person.primary_uuid or ""), + # An empty list survives _clean_dic, so this claim is present-and-empty + # rather than absent for a Person that has never been merged. It holds only + # superseded identifiers - the current one is datatracker_uuid. + "datatracker_prior_uuids": [str(u) for u in person.prior_uuids], + } + def scope_pronouns(self): return { 'pronouns': self.user.person.pronouns() } diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index b5256b14f8b..15a37968a52 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -53,7 +53,7 @@ from django.contrib.auth.views import LoginView from django.contrib.sites.models import Site from django.core.exceptions import ObjectDoesNotExist, ValidationError -from django.db import IntegrityError +from django.db import IntegrityError, transaction from django.urls import reverse as urlreverse from django.http import Http404, HttpResponseRedirect, HttpResponseForbidden from django.shortcuts import render, redirect, get_object_or_404 @@ -69,6 +69,7 @@ from ietf.name.models import ExtResourceName from ietf.nomcom.models import NomCom from ietf.person.models import Person, Email, Alias, PersonalApiKey, PERSON_API_KEY_VALUES +from ietf.person.utils import assign_primary_uuid from ietf.review.models import ReviewerSettings, ReviewWish, ReviewAssignment from ietf.review.utils import unavailable_periods_to_list, get_default_filter_re from ietf.doc.fields import SearchableDocumentField @@ -232,12 +233,15 @@ def confirm_account(request, auth): if not person: name = form.cleaned_data["name"] ascii = form.cleaned_data["ascii"] - person = Person.objects.create(user=user, - name=name, - ascii=ascii) - for name in set([ person.name, person.ascii, person.plain_name(), person.plain_ascii(), ]): - Alias.objects.create(person=person, name=name) + # Atomic so a Person is never left without the primary UUID that + # external systems need to name them by. + with transaction.atomic(): + person = Person.objects.create(user=user, name=name, ascii=ascii) + assign_primary_uuid(person) + + for name in set([ person.name, person.ascii, person.plain_name(), person.plain_ascii(), ]): + Alias.objects.create(person=person, name=name) if not email_obj: email_obj = Email.objects.create(address=email, person=person, origin=user.username) diff --git a/ietf/meeting/models.py b/ietf/meeting/models.py index 8cf386abf29..bf0c4dabd12 100644 --- a/ietf/meeting/models.py +++ b/ietf/meeting/models.py @@ -1243,6 +1243,12 @@ def can_manage_materials(self, user): def is_material_submission_cutoff(self): return date_today(datetime.UTC) > self.meeting.get_submission_correction_date() + def is_past(self): + timeslotassignment = self.official_timeslotassignment() + if timeslotassignment is None: + return False # if it's not scheduled, it's not past + return timezone.now() > timeslotassignment.timeslot.end_time() + def joint_with_groups_acronyms(self): return [group.acronym for group in self.joint_with_groups.all()] diff --git a/ietf/meeting/tests_session_requests.py b/ietf/meeting/tests_session_requests.py index 42dbee5f230..1ea7d42f22a 100644 --- a/ietf/meeting/tests_session_requests.py +++ b/ietf/meeting/tests_session_requests.py @@ -12,6 +12,7 @@ from ietf.group.factories import GroupFactory, RoleFactory from ietf.meeting.models import Session, ResourceAssociation, SchedulingEvent, Constraint from ietf.meeting.factories import MeetingFactory, SessionFactory +from ietf.meeting.views_session_request import get_requester_text from ietf.name.models import ConstraintName, TimerangeName from ietf.person.factories import PersonFactory from ietf.person.models import Person @@ -724,6 +725,70 @@ def test_request_notification_msg(self): get_payload_text(msg), ) + def test_wg_request_notification_msg(self): + to = "" + subject = "Dummy subject" + template = "meeting/session_request_notification.txt" + header = "A new" + meeting = MeetingFactory(type_id="ietf", date=date_today()) + area = GroupFactory(type_id='area') + mars = GroupFactory(parent=area, acronym='mars') + secretariat_role = RoleFactory(group__acronym='secretariat', name_id='secr') + requester = get_requester_text(secretariat_role.person, mars) + context = {"header": header, "meeting": meeting, "requester": requester} + cc = "cc.a@example.com, cc.b@example.com" + bcc = "bcc@example.com" + + msg = send_mail( + None, + to, + None, + subject, + template, + context, + cc=cc, + bcc=bcc, + ) + # Undo the text wrapping for simple checking + payload = get_payload_text(msg).replace("\n"," ") + self.assertIn( + f"{header} meeting session request has just been submitted by {requester}", + payload, + ) + self.assertIn("MARS Working Group", payload) + + def test_bof_request_notification_msg(self): + to = "" + subject = "Dummy subject" + template = "meeting/session_request_notification.txt" + header = "A new" + meeting = MeetingFactory(type_id="ietf", date=date_today()) + bof = RoleFactory(group__type_id="wg", group__state_id="bof", name_id="chair").group + secretariat_role = RoleFactory(group__acronym='secretariat', name_id='secr') + requester = get_requester_text(secretariat_role.person, bof) + context = {"header": header, "meeting": meeting, "requester": requester} + cc = "cc.a@example.com, cc.b@example.com" + bcc = "bcc@example.com" + + msg = send_mail( + None, + to, + None, + subject, + template, + context, + cc=cc, + bcc=bcc, + ) + # Undo the text wrapping for simple checking + payload = get_payload_text(msg).replace("\n"," ") + self.assertIn( + f"{header} meeting session request has just been submitted by {requester}", + payload, + ) + self.assertIn("BOF", payload) + self.assertNotIn("Working Group", payload) + def test_request_notification_third_session(self): meeting = MeetingFactory(type_id='ietf', date=date_today()) ad = Person.objects.get(user__username='ad') diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index beaaf8da8a7..9daf3c19157 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -7090,7 +7090,8 @@ def test_remove_sessionpresentation(self, mock_slides_manager_cls): def test_propose_session_slides(self): for type_id in ['ietf','interim']: - session = SessionFactory(meeting__type_id=type_id) + # create session in a meeting in the near future, not the past + session = SessionFactory(meeting__type_id=type_id, meeting__date=date_today() + datetime.timedelta(days=3)) chair = RoleFactory(group=session.group,name_id='chair').person session.meeting.importantdate_set.create(name_id='revsub',date=date_today() + datetime.timedelta(days=20)) newperson = PersonFactory() @@ -7169,6 +7170,33 @@ def test_propose_session_slides(self): self.assertEqual(len(q('.uploadslidelist p')), 0) self.client.logout() + # once the session is past, participants can no longer propose slides + timeslot = session.official_timeslotassignment().timeslot + timeslot.time = ( + timezone.now() - timeslot.duration - datetime.timedelta(seconds=1) + ) + timeslot.save() + + self.client.login( + username=newperson.user.username, + password=newperson.user.username + "+password", + ) + r = self.client.get(session_overview_url) + self.assertEqual(r.status_code,200) + q = PyQuery(r.content) + self.assertFalse(q('.proposeslides')) + r = self.client.get(upload_url) + self.assertEqual(r.status_code,403) + self.client.logout() + + # but a chair still can + self.client.login( + username=chair.user.username, password=chair.user.username + "+password" + ) + r = self.client.get(upload_url) + self.assertEqual(r.status_code,200) + self.client.logout() + def test_disapprove_proposed_slides(self): submission = SlideSubmissionFactory() submission.session.meeting.importantdate_set.create(name_id='revsub',date=date_today() + datetime.timedelta(days=20)) @@ -7291,7 +7319,8 @@ def test_approve_proposed_slides_multisession_apply_all(self, mock_slides_manage @override_settings(MEETECHO_API_CONFIG="fake settings") # enough to trigger API calls @patch("ietf.meeting.views.SlidesManager") def test_submit_and_approve_multiple_versions(self, mock_slides_manager_cls): - session = SessionFactory(meeting__type_id='ietf') + # create session in a meeting in the near future, not the past + session = SessionFactory(meeting__type_id='ietf', meeting__date=date_today() + datetime.timedelta(days=3)) chair = RoleFactory(group=session.group,name_id='chair').person session.meeting.importantdate_set.create(name_id='revsub',date=date_today()+datetime.timedelta(days=20)) newperson = PersonFactory() @@ -7624,6 +7653,41 @@ def test_handles_notes_server_failure(self): class SessionTests(TestCase): + def test_is_past(self): + now = timezone.now() + delta_t = datetime.timedelta(minutes=1) # long compared to test duration + duration = datetime.timedelta(minutes=30) + + for type_id in ["ietf", "interim"]: + # Create an ongoing meeting. The date of the meeting is not really important, + # and it's not realistic for an interim, but it gets the job done. + meeting = MeetingFactory( + type_id=type_id, date=date_today() - datetime.timedelta(days=1), days=7 + ) + # and schedule past and future sessions + past_session = SessionFactory(meeting=meeting, add_to_schedule=False) + # significant moment is the _end_ of the session + past_timeslot = TimeSlotFactory( + meeting=meeting, time=now - duration - delta_t, duration=duration + ) + SchedTimeSessAssignment.objects.create( + timeslot=past_timeslot, session=past_session, schedule=meeting.schedule + ) + future_session = SessionFactory(meeting=meeting, add_to_schedule=False) + future_timeslot = TimeSlotFactory( + meeting=meeting, time=now - duration + delta_t, duration=duration + ) + SchedTimeSessAssignment.objects.create( + timeslot=future_timeslot, session=future_session, schedule=meeting.schedule + ) + # Unscheduled sessions are arbitrarily declared not to be past + unscheduled_session = SessionFactory(meeting=meeting, add_to_schedule=False) + # and, finally, assert the expected behavior + self.assertTrue(past_session.is_past()) + self.assertFalse(future_session.is_past()) + self.assertFalse(unscheduled_session.is_past()) + + def test_get_summary_by_area(self): meeting = make_meeting_test_data(meeting=MeetingFactory(type_id='ietf', number='100')) diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index e2a15d3e8aa..3e5002a466f 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -1931,7 +1931,11 @@ def api_get_session_materials(request, session_id=None): minutes = session.minutes() slides_actions = [] - if can_manage_session_materials(request.user, session.group, session) or not session.is_material_submission_cutoff(): + if ( + has_role(request.user, "Secretariat") + or (not session.is_material_submission_cutoff() and session.can_manage_materials(request.user)) + or not session.is_past() + ): slides_actions.append( { "label": "Upload slides", @@ -3550,6 +3554,12 @@ def upload_session_slides(request, session_id, num, name=None): "The materials cutoff for this session has passed. Contact the secretariat for further action.", ) + if session.is_past() and not can_manage: + permission_denied( + request, + "This meeting has already occurred. Contact a chair or the secretariat for further action.", + ) + session_number = None sessions = get_sessions(session.meeting.number, session.group.acronym) show_apply_to_all_checkbox = ( diff --git a/ietf/meeting/views_session_request.py b/ietf/meeting/views_session_request.py index a1ef74f1b88..924249e9d4c 100644 --- a/ietf/meeting/views_session_request.py +++ b/ietf/meeting/views_session_request.py @@ -200,6 +200,9 @@ def get_requester_text(person, group): in the session request notification email, ie. Joe Smith, a Chair of the ancp working group """ + group_type = group.type.verbose_name + if group_type == "Working Group" and group.state_id == "bof": + group_type = group.state_id.upper() roles = group.role_set.filter(name__in=("chair", "secr", "ad"), person=person) if roles: rolename = str(roles[0].name) @@ -207,13 +210,13 @@ def get_requester_text(person, group): person.name, inflect.engine().a(rolename), group.acronym.upper(), - group.type.verbose_name, + group_type, ) if person.role_set.filter(name="secr", group__acronym="secretariat"): return "%s, on behalf of the %s %s" % ( person.name, group.acronym.upper(), - group.type.verbose_name, + group_type, ) diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 46418714880..0edf4498fc0 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -18,6 +18,7 @@ from email.utils import parseaddr from textwrap import dedent +from django.db import transaction from django.db.models import Q, Count, F, QuerySet from django.conf import settings from django.contrib.sites.models import Site @@ -36,6 +37,7 @@ from ietf.utils.mail import send_mail_text, send_mail, get_payload_text from ietf.utils.log import log from ietf.person.name import unidecode_name +from ietf.person.utils import assign_primary_uuid from ietf.utils.timezone import date_today, datetime_from_date, DEADLINE_TZINFO import debug # pyflakes:ignore @@ -416,13 +418,17 @@ def make_nomineeposition(nomcom, candidate, position, author): def make_nomineeposition_for_newperson(nomcom, candidate_name, candidate_email, position, author): - # This is expected to fail if called with an existing email address - email = Email.objects.create(address=candidate_email, origin="nominee: %s" % nomcom.group.acronym) - person = Person.objects.create(name=candidate_name, - ascii=unidecode_name(candidate_name), - ) - email.person = person - email.save() + # This is expected to fail if called with an existing email address. + # Atomic so a Person is never left without the primary UUID that external systems + # need to name them by, and so a failure part way leaves no half-built nominee. + with transaction.atomic(): + email = Email.objects.create(address=candidate_email, origin="nominee: %s" % nomcom.group.acronym) + person = Person.objects.create(name=candidate_name, + ascii=unidecode_name(candidate_name), + ) + assign_primary_uuid(person) + email.person = person + email.save() # send email to secretariat and nomcomchair to warn about the new person subject = 'New person is created' diff --git a/ietf/person/admin.py b/ietf/person/admin.py index f46edcf8aeb..4d3ad745e63 100644 --- a/ietf/person/admin.py +++ b/ietf/person/admin.py @@ -3,9 +3,13 @@ import simple_history from django import forms +from django.contrib import messages +from django.db import transaction -from ietf.person.models import Email, Alias, Person, PersonalApiKey, PersonEvent, PersonApiKeyEvent, PersonExtResource +from ietf.person.models import Email, Alias, Person, PersonalApiKey, PersonEvent, \ + PersonApiKeyEvent, PersonExtResource, PersonUUID from ietf.person.name import name_parts +from ietf.person.utils import queue_person_uuid_push from ietf.utils.admin import SaferStackedInline, SaferTabularInline from ietf.utils.validators import validate_external_resource_value @@ -29,6 +33,53 @@ class AliasAdmin(admin.ModelAdmin): class AliasInline(SaferStackedInline): model = Alias + +@admin.action(description="Make this the person's primary UUID") +def set_primary(modeladmin, request, queryset): + """Re-designate a Person's primary UUID + + Acts on exactly one UUID at a time: promoting two at once would either violate the + one-primary-per-person constraint or silently ignore one of them. + """ + if queryset.count() != 1: + modeladmin.message_user( + request, "Select exactly one UUID.", level=messages.ERROR + ) + return + new_primary = queryset.first() + if new_primary.primary: + modeladmin.message_user(request, "That UUID is already primary.") + return + person = new_primary.person + with transaction.atomic(): + person.uuids.filter(primary=True).update(primary=False) + new_primary.primary = True + new_primary.save(update_fields=["primary"]) + queue_person_uuid_push(person) + modeladmin.message_user( + request, f"{new_primary.uuid} is now the primary UUID for {person}." + ) + + +class PersonUUIDAdmin(admin.ModelAdmin): + list_display = ["uuid", "person", "primary", "time"] # noqa: RUF012 + list_filter = ["primary"] # noqa: RUF012 + search_fields = ["uuid", "person__name"] # noqa: RUF012 + raw_id_fields = ["person"] # noqa: RUF012 + readonly_fields = ["uuid", "primary", "time"] # noqa: RUF012 + actions = [set_primary] # noqa: RUF012 +admin.site.register(PersonUUID, PersonUUIDAdmin) + + +class PersonUUIDInline(SaferStackedInline): + model = PersonUUID + extra = 0 + # primary is changed through the PersonUUID admin's set_primary action, which demotes + # the old primary first. Editing it here would trip the uniqueness constraint. + readonly_fields = ["uuid", "primary", "time"] # noqa: RUF012 + can_delete = False + + class PersonAdmin(simple_history.admin.SimpleHistoryAdmin): def plain_name(self, obj): if obj.plain: @@ -41,7 +92,7 @@ def plain_name(self, obj): readonly_fields = ("name_from_draft", ) search_fields = ["name", "ascii"] raw_id_fields = ["user"] - inlines = [ EmailInline, AliasInline, ] + inlines = [ EmailInline, AliasInline, PersonUUIDInline] # actions = None admin.site.register(Person, PersonAdmin) diff --git a/ietf/person/api_uuid.py b/ietf/person/api_uuid.py new file mode 100644 index 00000000000..0fbf452566e --- /dev/null +++ b/ietf/person/api_uuid.py @@ -0,0 +1,290 @@ +# Copyright The IETF Trust 2026, All Rights Reserved +"""Person UUID resolution API + +Lets an authorized application ask about a Person UUID it holds and learn the Person's +current identifier set. Responses carry identifiers only - no name, address or database +key. +""" + +from drf_spectacular.utils import ( + OpenApiExample, + extend_schema, + extend_schema_view, +) +from rest_framework import mixins, serializers, viewsets +from rest_framework.decorators import action +from rest_framework.response import Response +from rest_framework.views import APIView + +from ietf.person.models import Person, PersonUUID + +MAX_BATCH = 500 + +# A batch entry either resolved to a Person or it did not. Both outcomes use the same +# entry shape, discriminated by this field, so a consumer switches on it instead of +# inspecting which fields came back. +ENTRY_STATUSES = ("resolved", "unknown") + + +def uuid_sets_for(person_ids): + """Map each person_id to its (primary_uuid, [prior_uuids]) in a single query + + Keeps the batch endpoints' query count independent of the batch size: reading + Person.primary_uuid and Person.prior_uuids per row would be two queries per Person. + Ordering matches Person.prior_uuids. + """ + sets = {pid: (None, []) for pid in person_ids} + rows = ( + PersonUUID.objects.filter(person_id__in=person_ids) + .order_by("time", "uuid") + .values_list("person_id", "uuid", "primary") + ) + for pid, value, is_primary in rows: + _, priors = sets[pid] + if is_primary: + sets[pid] = (value, priors) + else: + priors.append(value) + return sets + + +class PersonUUIDResolutionSerializer(serializers.Serializer): + """A PersonUUID together with its Person's whole identifier set""" + + uuid = serializers.UUIDField(read_only=True) + is_primary = serializers.BooleanField(source="primary", read_only=True) + primary_uuid = serializers.UUIDField(source="person.primary_uuid", read_only=True) + prior_uuids = serializers.ListField( + source="person.prior_uuids", child=serializers.UUIDField(), read_only=True + ) + + +class PersonUUIDBatchRequestSerializer(serializers.Serializer): + uuids = serializers.ListField( + child=serializers.UUIDField(), allow_empty=False, max_length=MAX_BATCH + ) + + +class PersonUUIDBatchEntrySerializer(serializers.Serializer): + """One requested UUID and, when it resolved, its Person's identifier set + + Every field is always present. The identifier fields are null - prior_uuids empty - + when status is unknown. + + Output only, and deliberately not read_only=True field by field: read_only implies + required=False, which would leave a generated client treating even status as optional. + """ + + uuid = serializers.UUIDField() + status = serializers.ChoiceField(choices=ENTRY_STATUSES) + is_primary = serializers.BooleanField(allow_null=True) + primary_uuid = serializers.UUIDField(allow_null=True) + prior_uuids = serializers.ListField(child=serializers.UUIDField()) + + +class PersonUUIDBatchResponseSerializer(serializers.Serializer): + results = PersonUUIDBatchEntrySerializer(many=True) + + +class PersonPkBatchRequestSerializer(serializers.Serializer): + person_pks = serializers.ListField( + child=serializers.IntegerField(), allow_empty=False, max_length=MAX_BATCH + ) + + +class PersonPkBatchEntrySerializer(serializers.Serializer): + """One requested Person.pk and, when it resolved, that Person's identifier set + + Same one-shape-for-both-outcomes and output-only rules as + PersonUUIDBatchEntrySerializer. + """ + + person_pk = serializers.IntegerField() + status = serializers.ChoiceField(choices=ENTRY_STATUSES) + primary_uuid = serializers.UUIDField(allow_null=True) + prior_uuids = serializers.ListField(child=serializers.UUIDField()) + + +class PersonPkBatchResponseSerializer(serializers.Serializer): + results = PersonPkBatchEntrySerializer(many=True) + + +@extend_schema(tags=["person"]) +@extend_schema_view( + retrieve=extend_schema( + operation_id="person_uuid_retrieve", + summary="Resolve a Person UUID", + description=( + "Resolve any UUID the datatracker has issued for a Person to that Person's " + "current identifier set. A UUID that stopped being primary because of a " + 'merge still resolves, and the response carries the current primary. A 200 ' + 'whose primary_uuid differs from the requested uuid means "same person, new ' + 'identifier" - it is not an error.\n\n' + "A 404 means no Person has this UUID. It does not distinguish a UUID the " + "datatracker never issued from one it issued to a Person that has since been " + "deleted, because deleting a Person deletes its UUIDs.\n\n" + "primary_uuid is the person's current primary. Re-read it; do not assume it " + "is unchanged from a previous response, and do not assume a UUID you hold " + "remains primary.\n\n" + "Responses contain identifiers only. No name, address or database key is " + "returned." + ), + responses=PersonUUIDResolutionSerializer, + ) +) +class PersonUUIDViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet): + """Resolve Person UUIDs to their Person's current identifier set""" + + api_key_endpoint = "ietf.person.api_uuid" + queryset = PersonUUID.objects.select_related("person") + serializer_class = PersonUUIDResolutionSerializer + lookup_field = "uuid" + lookup_url_kwarg = "uuid" + lookup_value_converter = "anycase_uuid" + + @extend_schema( + operation_id="person_uuid_lookup", + summary="Resolve a batch of Person UUIDs", + description=( + f"Resolve up to {MAX_BATCH} UUIDs in one call. Always returns 200 with one " + "results entry per distinct requested UUID - no entry is ever omitted and no " + "unresolvable UUID fails the request.\n\n" + "Each entry carries a status of resolved or unknown, corresponding to the " + "200 and 404 outcomes of the single-UUID endpoint. Every entry has the same " + "fields either way, with the identifier fields null when status is unknown, " + "so switch on status rather than on which fields are present. Duplicate " + "inputs produce one entry. Entry order is not significant - match on uuid." + ), + request=PersonUUIDBatchRequestSerializer, + responses=PersonUUIDBatchResponseSerializer, + ) + @action(detail=False, methods=["post"]) + def lookup(self, request): + requested_serializer = PersonUUIDBatchRequestSerializer(data=request.data) + requested_serializer.is_valid(raise_exception=True) + requested = list(dict.fromkeys(requested_serializer.validated_data["uuids"])) + + found = {row.uuid: row for row in PersonUUID.objects.filter(uuid__in=requested)} + sets = uuid_sets_for({row.person_id for row in found.values()}) + + results = [] + for value in requested: + row = found.get(value) + if row is None: + results.append( + { + "uuid": value, + "status": "unknown", + "is_primary": None, + "primary_uuid": None, + "prior_uuids": [], + } + ) + continue + primary, priors = sets[row.person_id] + results.append( + { + "uuid": value, + "status": "resolved", + "is_primary": row.primary, + "primary_uuid": primary, + "prior_uuids": priors, + } + ) + return Response( + PersonUUIDBatchResponseSerializer({"results": results}).data + ) + + +@extend_schema(tags=["person"]) +class PersonUUIDByPersonPkView(APIView): + """Transitional pk-to-UUID conversion, for consumers migrating off Person.pk + + Batch-only on purpose: the one legitimate use is a single bulk conversion, and a + convenient per-request lookup would become a permanent pk-to-UUID service. Its own + api_key_endpoint so its tokens can be withdrawn without touching the resolution API. + + A plain APIView rather than a ViewSet: nothing here is a resource, and routing a + lookup through a ViewSet meant calling it "create", which made the schema claim a 201 + for a request that creates nothing and returns 200. + """ + + api_key_endpoint = "ietf.person.api_uuid_by_pk" + + @extend_schema( + deprecated=True, + operation_id="person_uuid_by_person_pk", + summary="TRANSITIONAL: resolve Person database keys to UUIDs", + description=( + "DEPRECATED FROM FIRST RELEASE, AND WILL BE WITHDRAWN. Resolves up to " + f"{MAX_BATCH} Person.pk values to their UUID sets, so an application that " + "keyed its records on the datatracker database key can convert them to " + "UUIDs once and stop using the key. Person.pk is the identifier this whole " + "feature exists to stop exporting: it does not survive a Person merge, " + "which is why holding it is unsafe.\n\n" + "Intended to be called once per consuming application, to migrate a table. " + "Do not call it at request time and do not build anything that keeps " + "needing it.\n\n" + "One results entry per distinct requested pk, with the same fields either " + "way and the identifier fields null when status is unknown. Switch on " + "status." + ), + request=PersonPkBatchRequestSerializer, + responses={200: PersonPkBatchResponseSerializer}, + examples=[ + OpenApiExample( + "Two pks, one of them unknown", + value={ + "results": [ + { + "person_pk": 12345, + "status": "resolved", + "primary_uuid": "6f9a1c30-6c7e-4f0a-9a3f-2f1d0b8a4e11", + "prior_uuids": ["0b21f8d4-1a55-4c9e-8f77-9c2b4a6e3d02"], + }, + { + "person_pk": 999999, + "status": "unknown", + "primary_uuid": None, + "prior_uuids": [], + }, + ] + }, + response_only=True, + ) + ], + ) + def post(self, request): + requested_serializer = PersonPkBatchRequestSerializer(data=request.data) + requested_serializer.is_valid(raise_exception=True) + requested = list( + dict.fromkeys(requested_serializer.validated_data["person_pks"]) + ) + + existing = set( + Person.objects.filter(pk__in=requested).values_list("pk", flat=True) + ) + sets = uuid_sets_for(existing) + + results = [] + for pk in requested: + if pk not in existing: + results.append( + { + "person_pk": pk, + "status": "unknown", + "primary_uuid": None, + "prior_uuids": [], + } + ) + continue + primary, priors = sets[pk] + results.append( + { + "person_pk": pk, + "status": "resolved", + "primary_uuid": primary, + "prior_uuids": priors, + } + ) + return Response(PersonPkBatchResponseSerializer({"results": results}).data) diff --git a/ietf/person/factories.py b/ietf/person/factories.py index 655f25994b3..110542e0796 100644 --- a/ietf/person/factories.py +++ b/ietf/person/factories.py @@ -20,7 +20,8 @@ import debug # pyflakes:ignore -from ietf.person.models import Person, Alias, Email, PersonalApiKey, PersonApiKeyEvent, PERSON_API_KEY_ENDPOINTS +from ietf.person.models import Person, Alias, Email, PersonalApiKey, PersonApiKeyEvent, \ + PERSON_API_KEY_ENDPOINTS, PersonUUID from ietf.person.name import normalize_name, unidecode_name @@ -64,6 +65,21 @@ def set_password(obj, create, extracted, **kwargs): # pylint: disable=no-self-ar obj.set_password( '%s+password' % obj.username ) # pylint: disable=no-value-for-parameter obj.save() + +class PersonUUIDFactory(factory.django.DjangoModelFactory): + """A UUID for a Person + + Defaults to a superseded, non-primary UUID, which is what a test asking for an extra + UUID wants. PersonFactory uses this with primary=True to make each Person's primary; + creating a second primary for the same Person violates a uniqueness constraint. + """ + person = factory.SubFactory("ietf.person.factories.PersonFactory") + primary = False + + class Meta: + model = PersonUUID + + class PersonFactory(factory.django.DjangoModelFactory): class Meta: model = Person @@ -79,6 +95,16 @@ class Meta: class Params: with_bio = factory.Trait(biography = "\n\n".join(fake.paragraphs())) # type: ignore + @factory.post_generation + def primary_uuid(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument + """Give the Person the primary UUID every Person is supposed to have + + Pass primary_uuid=False for a Person with no UUIDs at all, which is otherwise + not reachable through any production path. + """ + if create and extracted is not False: + PersonUUIDFactory(person=obj, primary=True) + @factory.post_generation def default_aliases(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument make_alias = getattr(AliasFactory, 'create' if create else 'build') diff --git a/ietf/person/migrations/0006_personuuid.py b/ietf/person/migrations/0006_personuuid.py new file mode 100644 index 00000000000..17a56808c6d --- /dev/null +++ b/ietf/person/migrations/0006_personuuid.py @@ -0,0 +1,67 @@ +# Copyright The IETF Trust 2026, All Rights Reserved + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import ietf.person.models + + +def forward(apps, schema_editor): + Person = apps.get_model("person", "Person") + PersonUUID = apps.get_model("person", "PersonUUID") + # uuid and time come from the field defaults + PersonUUID.objects.bulk_create( + [PersonUUID(person=person, primary=True) for person in Person.objects.all()], + batch_size=1000, + ) + + +def reverse(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + dependencies = [ + ("person", "0005_alter_historicalperson_pronouns_selectable_and_more"), + ] + + operations = [ + migrations.CreateModel( + name="PersonUUID", + fields=[ + ( + "uuid", + models.UUIDField( + default=ietf.person.models.unused_person_uuid, + editable=False, + primary_key=True, + serialize=False, + ), + ), + ("primary", models.BooleanField(default=False)), + ( + "time", + models.DateTimeField( + default=django.utils.timezone.now, editable=False + ), + ), + ( + "person", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="uuids", + to="person.person", + ), + ), + ], + ), + migrations.AddConstraint( + model_name="personuuid", + constraint=models.UniqueConstraint( + condition=models.Q(("primary", True)), + fields=("person",), + name="unique_primary_uuid_per_person", + ), + ), + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/person/models.py b/ietf/person/models.py index 3ab89289a65..8b05c5f8a61 100644 --- a/ietf/person/models.py +++ b/ietf/person/models.py @@ -44,6 +44,48 @@ def name_character_validator(value): ) +def unused_person_uuid(): + MAX_ATTEMPTS = 50 # ludicrously large + for _ in range(MAX_ATTEMPTS): + candidate = uuid.uuid4() + if not PersonUUID.objects.filter(uuid=candidate).exists(): + return candidate + raise RuntimeError("Unable to generate unused UUID") + + +class PersonUUID(models.Model): + """Surrogate key for a Person + + A Person has one or more UUIDs. Exactly one is primary: it is the identifier handed + to external systems. Merging Persons unions the sets and keeps a single primary, so a + UUID an external system holds keeps resolving to the surviving Person even after it + stops being primary. + + Deleting a Person deletes its UUIDs. Their values return to the pool + unused_person_uuid() draws from, so a deleted UUID could in principle be issued again + to a different Person. That requires uuid4() to reproduce a specific 122-bit value, + so the risk is accepted. + """ + uuid = models.UUIDField(primary_key=True, editable=False, default=unused_person_uuid) + person = models.ForeignKey( + "person.Person", related_name="uuids", on_delete=models.CASCADE + ) + primary = models.BooleanField(default=False) + time = models.DateTimeField(default=timezone.now, editable=False) + + class Meta: + constraints = [ # noqa: RUF012 + models.UniqueConstraint( + fields=["person"], + condition=models.Q(primary=True), + name="unique_primary_uuid_per_person", + ), + ] + + def __str__(self): + return str(self.uuid) + + class Person(models.Model): history = HistoricalRecords() user = OneToOneField(User, blank=True, null=True, on_delete=models.SET_NULL) @@ -158,6 +200,15 @@ def email(self): e = self.email_set.filter(active=True).order_by("-time").first() self._cached_email = e return self._cached_email + def has_alias_for_name(self): + """Is self.name recorded as one of this person's aliases? + + Cached per instance so that callers rendering many people at once can seed it + in bulk instead of paying a query apiece. + """ + if not hasattr(self, '_cached_has_alias_for_name'): + self._cached_has_alias_for_name = self.alias_set.filter(name=self.name).exists() + return self._cached_has_alias_for_name def email_allowing_inactive(self): if not hasattr(self, "_cached_email_allowing_inactive"): e = self.email() @@ -192,6 +243,27 @@ def full_name_as_key(self): # this is mostly a remnant from the old views, needed in the menu return self.plain_name().lower().replace(" ", ".") + @property + def primary_uuid(self): + """The UUID to hand to external systems, or None if data is inconsistent""" + row = self.uuids.filter(primary=True).first() + return row.uuid if row else None + + @property + def prior_uuids(self): + """UUIDs that still resolve to this Person but are no longer primary + + Oldest first. Ties are broken on the UUID itself so the order is total rather + than left to the database, and so it matches uuid_sets_for() in + ietf.person.api_uuid. Nothing in the current code produces a tie: the timestamp + defaults per row and a merge moves UUIDs without rewriting it. + """ + return list( + self.uuids.filter(primary=False) + .order_by("time", "uuid") + .values_list("uuid", flat=True) + ) + def photo_name(self,thumb=False): hasher = Hashids(salt='Person photo name salt',min_length=5) _, first, _, last, _ = name_parts(self.ascii) @@ -254,6 +326,8 @@ def save(self, *args, **kwargs): if self.ascii and self.name != self.ascii: if not self.ascii in [ a.name for a in self.alias_set.filter(name=self.ascii) ]: self.alias_set.create(name=self.ascii) + # The aliases just changed; drop what has_alias_for_name() memoized about them. + self.__dict__.pop('_cached_has_alias_for_name', None) #this variable, if not None, may be used by url() to keep the sitefqdn. default_hostscheme = None diff --git a/ietf/person/resources.py b/ietf/person/resources.py index 42bb9f7732c..c8a8bf8af35 100644 --- a/ietf/person/resources.py +++ b/ietf/person/resources.py @@ -10,7 +10,18 @@ from ietf import api -from ietf.person.models import (Person, Email, Alias, PersonalApiKey, PersonEvent, PersonApiKeyEvent, HistoricalPerson, HistoricalEmail, PersonExtResource) # type: ignore +from ietf.person.models import ( # type: ignore + Person, + Email, + Alias, + PersonalApiKey, + PersonEvent, + PersonApiKeyEvent, + HistoricalPerson, + HistoricalEmail, + PersonExtResource, + PersonUUID, +) from ietf.utils.resources import UserResource @@ -35,6 +46,26 @@ class Meta: } api.person.register(PersonResource()) + +class PersonUUIDResource(ModelResource): + person = ToOneField(PersonResource, "person") + + class Meta: + cache = SimpleCache() + queryset = PersonUUID.objects.all() + serializer = api.Serializer() + # resource_name = 'personuuid' + ordering = ['uuid', ] # noqa: RUF012 + filtering = { # noqa: RUF012 + "uuid": ALL, + "primary": ALL, + "time": ALL, + "person": ALL_WITH_RELATIONS, + } + + +api.person.register(PersonUUIDResource()) + class EmailResource(ModelResource): person = ToOneField(PersonResource, 'person', null=True) class Meta: diff --git a/ietf/person/tasks.py b/ietf/person/tasks.py index f0c979fa267..4e0a34942df 100644 --- a/ietf/person/tasks.py +++ b/ietf/person/tasks.py @@ -7,11 +7,13 @@ from celery import shared_task from django.conf import settings +from django.db.models import Count, Q from django.utils import timezone from ietf.utils import log from ietf.utils.mail import send_mail -from .models import PersonalApiKey, PersonApiKeyEvent +from .models import Person, PersonalApiKey, PersonApiKeyEvent +from .utils import ensure_primary_uuid @shared_task @@ -57,3 +59,72 @@ def purge_personal_api_key_events_task(keep_days): count = len(old_events) old_events.delete() log.log(f"Deleted {count} PersonApiKeyEvents older than {keep_since}") + + +@shared_task +def check_person_uuids_task(fix=False): + """Report - and optionally repair - Persons whose UUID set is inconsistent + + Every Person is supposed to have exactly one primary UUID. Assignment is an explicit + assign_primary_uuid() call at each site that creates a Person, so a site added + without one leaves Persons that cannot be named to any external system. This finds + them. + + Checks for exactly one rather than at least one. A partial unique constraint should + make more than one impossible, so finding one means something is grossly wrong with + the data and worth saying out loud - and this is the job that claims the condition + holds, so it may as well test it. + """ + broken = ( + Person.objects.annotate( + uuid_count=Count("uuids", distinct=True), + primary_count=Count("uuids", filter=Q(uuids__primary=True), distinct=True), + ) + .exclude(primary_count=1) + .order_by("pk") + ) + + count = 0 + for person in broken: + count += 1 + if person.primary_count > 1: + # ensure_primary_uuid() cannot resolve this - it would pick one of the + # primaries arbitrarily. Needs a human to decide which one survives. + log.log( + f"Person {person.pk} ({person.name}): {person.primary_count} primary " + f"UUIDs, which the unique constraint should have prevented - not " + f"repairing automatically" + ) + continue + problem = "no UUIDs at all" if person.uuid_count == 0 else "no primary UUID" + log.log(f"Person {person.pk} ({person.name}): {problem}") + if fix: + row = ensure_primary_uuid(person) + log.log(f"Person {person.pk}: primary UUID is now {row.uuid}") + + if count == 0: + log.log("check_person_uuids: every Person has exactly one primary UUID") + else: + log.log( + f"check_person_uuids: {count} Person(s) " + f"{'repaired' if fix else 'need attention'}" + ) + return count + + +@shared_task +def push_person_uuids_task(person_pk): + """Push a Person's UUID set to Authentik + + Enqueued by ietf.person.utils.queue_person_uuid_push whenever the set changes. The + Authentik client does not exist yet, so for now this records the desired state that + will be pushed. + """ + person = Person.objects.filter(pk=person_pk).first() + if person is None: + log.log(f"Not pushing UUIDs for Person {person_pk}: no such Person") + return + log.log( + f"Person {person_pk} UUIDs: primary={person.primary_uuid} " + f"prior={[str(u) for u in person.prior_uuids]}" + ) diff --git a/ietf/person/templatetags/person_filters.py b/ietf/person/templatetags/person_filters.py index a7a6e8193a0..715d154a949 100644 --- a/ietf/person/templatetags/person_filters.py +++ b/ietf/person/templatetags/person_filters.py @@ -53,11 +53,7 @@ def person_link(person, **kwargs): titlepage_name = kwargs.get("titlepage_name", None) if person is not None: plain_name = person.plain_name() - name = ( - person.name - if person.alias_set.filter(name=person.name).exists() - else plain_name - ) + name = person.name if person.has_alias_for_name() else plain_name email = person.email_address() return { "name": name, @@ -78,11 +74,7 @@ def email_person_link(email, **kwargs): cls = kwargs.get("class", "") with_email = kwargs.get("with_email", True) plain_name = email.person.plain_name() - name = ( - email.person.name - if email.person.alias_set.filter(name=email.person.name).exists() - else plain_name - ) + name = email.person.name if email.person.has_alias_for_name() else plain_name email = email.address return { "name": name, diff --git a/ietf/person/tests.py b/ietf/person/tests.py index 42c2c1547cb..0af8271594f 100644 --- a/ietf/person/tests.py +++ b/ietf/person/tests.py @@ -4,19 +4,26 @@ import datetime import json +import uuid from unittest import mock from io import StringIO, BytesIO from PIL import Image from pyquery import PyQuery +import django.core.signing from django.core.exceptions import ValidationError +from django.db import connection, transaction +from django.db.utils import IntegrityError from django.http import HttpRequest from django.test import override_settings +from django.test.utils import CaptureQueriesContext from django.urls import reverse as urlreverse from django.utils import timezone from django.utils.encoding import iri_to_uri +import yaml + import debug # pyflakes:ignore from ietf.community.models import CommunityList @@ -26,12 +33,22 @@ from ietf.nomcom.models import NomCom from ietf.nomcom.test_data import nomcom_test_data from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory -from ietf.person.factories import EmailFactory, PersonFactory, PersonApiKeyEventFactory -from ietf.person.models import Person, Alias, PersonApiKeyEvent -from ietf.person.tasks import purge_personal_api_key_events_task +from ietf.nomcom.utils import make_nomineeposition_for_newperson +from ietf.person.factories import ( + EmailFactory, + PersonFactory, + PersonApiKeyEventFactory, + PersonUUIDFactory, +) +from ietf.person.models import Person, Alias, PersonApiKeyEvent, PersonUUID +from ietf.person.tasks import (purge_personal_api_key_events_task, push_person_uuids_task, + check_person_uuids_task) from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification, handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees, - handle_reviewer_settings, get_dots) + handle_reviewer_settings, get_dots, assign_primary_uuid, ensure_primary_uuid, + get_person_uuid_object) +from ietf.submit.utils import ensure_person_email_info_exists +from kombu.exceptions import OperationalError as KombuOperationalError from ietf.review.models import ReviewerSettings from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox, empty_outbox @@ -116,6 +133,54 @@ def test_person_profile_without_email(self): r = self.client.get(url) self.assertContains(r, person.name, status_code=200) + def test_person_profile_by_uuid(self): + person_a = PersonFactory(name="A Fine Person") + url_a = urlreverse( + "ietf.person.views.profile_by_uuid", + kwargs={"uuid": person_a.primary_uuid}, + ) + + person_b = PersonFactory(name="Brilliant Person") + url_b = urlreverse( + "ietf.person.views.profile_by_uuid", + kwargs={"uuid": person_b.primary_uuid}, + ) + + r = self.client.get(url_a) + self.assertContains(r, person_a.name) + self.assertNotContains(r, person_b.name) + + r = self.client.get(url_b) + self.assertNotContains(r, person_a.name) + self.assertContains(r, person_b.name) + + # Move b's UUID to a as a prior UUID, as a merge would... + person_b.uuids.update(person=person_a, primary=False) + # ... and the old address redirects to a's canonical one + r = self.client.get(url_b) + self.assertRedirects(r, url_a) + r = self.client.get(url_b, follow=True) + self.assertContains(r, person_a.name) + self.assertNotContains(r, person_b.name) + + def test_person_profile_by_uuid_upper_case(self): + person = PersonFactory() + uuid_value = person.primary_uuid + url = urlreverse( + "ietf.person.views.profile_by_uuid", kwargs={"uuid": uuid_value} + ) + upper = url.replace(str(uuid_value), str(uuid_value).upper()) + self.assertNotEqual(url, upper) + r = self.client.get(upper) + self.assertContains(r, person.name, status_code=200) + + def test_person_profile_by_uuid_unknown(self): + url = urlreverse( + "ietf.person.views.profile_by_uuid", kwargs={"uuid": uuid.uuid4()} + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 404) + def test_case_insensitive(self): # Case insensitive seach person = PersonFactory(name="Test Person") @@ -394,9 +459,12 @@ def test_merge_persons(self): request = HttpRequest() request.user = user source = PersonFactory() + PersonUUIDFactory(person=source) # give them an extra target = PersonFactory() mars = RoleFactory(name_id='chair',group__acronym='mars').group source_id = source.pk + source_uuids = {source.primary_uuid, *source.prior_uuids} + target_uuids = {target.primary_uuid, *target.prior_uuids} source_email = source.email_set.first() source_alias = source.alias_set.first() source_user = source.user @@ -415,6 +483,14 @@ def test_merge_persons(self): self.assertIn(nomination, target.nomination_set.all()) self.assertFalse(Person.objects.filter(id=source_id)) self.assertFalse(source_user.is_active) + self.assertEqual( + {target.primary_uuid, *target.prior_uuids}, + source_uuids | target_uuids, + ) + # The survivor keeps its own primary; the source's UUIDs become priors + self.assertIsNotNone(target.primary_uuid) + self.assertIn(target.primary_uuid, target_uuids) + self.assertEqual(set(target.prior_uuids), source_uuids) def test_merge_persons_reviewer_settings(self): secretariat_role = RoleFactory(group__acronym='secretariat', name_id='secr') @@ -478,6 +554,506 @@ def test_send_merge_request(self): self.assertIn(source.user.username, message.to) +class PersonUUIDTests(TestCase): + def test_every_creation_path_assigns_a_primary(self): + """Each production route that creates a Person gives it one primary UUID""" + # ietf.ietfauth.views.confirm_account + confirm_url = urlreverse( + "ietf.ietfauth.views.confirm_account", + kwargs={ + "auth": django.core.signing.dumps( + "uuidtest@example.com", salt="create_account" + ) + }, + ) + self.client.post( + confirm_url, + { + "name": "UUID Test", + "ascii": "UUID Test", + "password": "secret+password", + "password_confirmation": "secret+password", + }, + ) + created = Person.objects.get(name="UUID Test") + self.assertIsNotNone(created.primary_uuid) + self.assertEqual(created.prior_uuids, []) + + # ietf.nomcom.utils.make_nomineeposition_for_newperson + nomcom = NomComFactory(group__acronym="nomcom2021") + position = PositionFactory(nomcom=nomcom) + make_nomineeposition_for_newperson( + nomcom, + "New Nominee", + "newnominee@example.com", + position, + PersonFactory().email(), + ) + nominee_person = Person.objects.get(name="New Nominee") + self.assertIsNotNone(nominee_person.primary_uuid) + self.assertEqual(nominee_person.prior_uuids, []) + + # ietf.submit.utils.ensure_person_email_info_exists + ensure_person_email_info_exists( + "Draft Author", "draftauthor@example.com", "draft-uuid-test" + ) + author = Person.objects.get(name="Draft Author") + self.assertIsNotNone(author.primary_uuid) + self.assertEqual(author.prior_uuids, []) + + def test_factory_assigns_a_primary(self): + person = PersonFactory() + # Reads the rows directly: this is what proves the accessors below agree with + # the model, so it must not go through them + self.assertEqual(person.uuids.filter(primary=True).count(), 1) + self.assertIsNotNone(person.primary_uuid) + self.assertEqual(person.prior_uuids, []) + + def test_factory_can_skip_the_primary(self): + person = PersonFactory(primary_uuid=False) + self.assertEqual(person.uuids.count(), 0) + self.assertIsNone(person.primary_uuid) + self.assertEqual(person.prior_uuids, []) + + def test_prior_uuids_holds_the_superseded_ones_in_order(self): + person = PersonFactory() + primary = person.primary_uuid + older = PersonUUIDFactory( + person=person, time=timezone.now() - datetime.timedelta(days=2) + ) + newer = PersonUUIDFactory( + person=person, time=timezone.now() - datetime.timedelta(days=1) + ) + # The primary is not in the prior list, and the priors are oldest-first + self.assertEqual(person.primary_uuid, primary) + self.assertEqual(person.prior_uuids, [older.uuid, newer.uuid]) + + def test_assign_primary_uuid_is_idempotent(self): + person = PersonFactory() + first = person.primary_uuid + assign_primary_uuid(person) + self.assertEqual(person.uuids.count(), 1) + self.assertEqual(person.primary_uuid, first) + + def test_only_one_primary_per_person(self): + person = PersonFactory() + with self.assertRaises(IntegrityError), transaction.atomic(): + PersonUUID.objects.create(person=person, primary=True) + + def test_ensure_primary_uuid_promotes_earliest(self): + person = PersonFactory() + oldest = person.uuids.get() + PersonUUIDFactory(person=person) + person.uuids.update(primary=False) # deliberately inconsistent state + promoted = ensure_primary_uuid(person) + self.assertEqual(promoted.uuid, oldest.uuid) + self.assertEqual(person.uuids.filter(primary=True).count(), 1) + + def test_ensure_primary_uuid_creates_when_none(self): + person = PersonFactory(primary_uuid=False) + created = ensure_primary_uuid(person) + self.assertTrue(created.primary) + self.assertEqual(person.uuids.count(), 1) + + def test_get_person_uuid_object(self): + person = PersonFactory() + prior = PersonUUIDFactory(person=person) + self.assertIsNone(get_person_uuid_object(uuid.uuid4())) + primary_obj = get_person_uuid_object(person.primary_uuid) + self.assertEqual(primary_obj.person, person) + self.assertTrue(primary_obj.primary) + prior_obj = get_person_uuid_object(prior.uuid) + self.assertEqual(prior_obj.person, person) + self.assertFalse(prior_obj.primary) + + def test_deleting_a_person_deletes_its_uuids(self): + person = PersonFactory() + values = [person.primary_uuid, *person.prior_uuids] + person.delete() + self.assertEqual(PersonUUID.objects.filter(uuid__in=values).count(), 0) + for value in values: + self.assertIsNone(get_person_uuid_object(value)) + + def test_merge_chain_keeps_one_primary(self): + secretariat_role = RoleFactory(group__acronym="secretariat", name_id="secr") + request = HttpRequest() + request.user = secretariat_role.person.user + a, b, c = PersonFactory.create_batch(3) + a_uuids = {a.primary_uuid, *a.prior_uuids} + b_uuids = {b.primary_uuid, *b.prior_uuids} + c_primary = c.primary_uuid + merge_persons(request, a, b, file=StringIO()) + merge_persons(request, b, c, file=StringIO()) + c.refresh_from_db() + self.assertIsNotNone(c.primary_uuid) + self.assertEqual(c.primary_uuid, c_primary) + self.assertEqual(set(c.prior_uuids), a_uuids | b_uuids) + for value in a_uuids | b_uuids: + self.assertEqual(get_person_uuid_object(value).person, c) + + def test_merge_promotes_a_primary_for_a_target_without_one(self): + secretariat_role = RoleFactory(group__acronym="secretariat", name_id="secr") + request = HttpRequest() + request.user = secretariat_role.person.user + source = PersonFactory() + target = PersonFactory() + target.uuids.update(primary=False) # deliberately inconsistent state + merge_persons(request, source, target, file=StringIO()) + target.refresh_from_db() + self.assertIsNotNone(target.primary_uuid) + + @mock.patch("ietf.person.utils.transaction.on_commit", side_effect=lambda f: f()) + @mock.patch("ietf.person.tasks.push_person_uuids_task.apply_async") + def test_creating_a_person_does_not_push(self, mock_apply, mock_on_commit): + # A brand-new Person has no Authentik account, so there is nothing to push to. + person = PersonFactory() + self.assertFalse(mock_apply.called) + + person.name = person.name + " Jr" + person.save() + self.assertFalse(mock_apply.called) # nor does an unrelated save + + @mock.patch("ietf.person.utils.transaction.on_commit", side_effect=lambda f: f()) + @mock.patch("ietf.person.tasks.push_person_uuids_task.apply_async") + def test_push_is_dispatched_when_the_set_changes(self, mock_apply, mock_on_commit): + secretariat_role = RoleFactory(group__acronym="secretariat", name_id="secr") + request = HttpRequest() + request.user = secretariat_role.person.user + source = PersonFactory() + target = PersonFactory() + mock_apply.reset_mock() + + merge_persons(request, source, target, file=StringIO()) + self.assertTrue(mock_apply.called) + self.assertEqual( + mock_apply.call_args.kwargs["kwargs"], {"person_pk": target.pk} + ) + # Celery's default retry policy applies - short enough for the request path, + # and enough to ride out a broker blip. See queue_person_uuid_push(). + self.assertNotIn("retry", mock_apply.call_args.kwargs) + + # Promoting a primary for a Person that already existed does push + mock_apply.reset_mock() + other = PersonFactory() + other.uuids.update(primary=False) # deliberately inconsistent state + ensure_primary_uuid(other) + self.assertEqual(mock_apply.call_args.kwargs["kwargs"], {"person_pk": other.pk}) + + @mock.patch("ietf.person.utils.transaction.on_commit", side_effect=lambda f: f()) + @mock.patch("ietf.person.tasks.push_person_uuids_task.apply_async") + @mock.patch("ietf.person.utils.log.log") + def test_unreachable_broker_does_not_break_the_caller( + self, mock_log, mock_apply, mock_on_commit + ): + mock_apply.side_effect = KombuOperationalError("no broker here") + person = PersonFactory() + person.uuids.update(primary=False) # deliberately inconsistent state + ensure_primary_uuid(person) # must not raise + self.assertIsNotNone(person.primary_uuid) + self.assertIn("Could not queue UUID push", mock_log.call_args[0][0]) + + @mock.patch("ietf.person.tasks.log.log") + def test_push_person_uuids_task(self, mock_log): + person = PersonFactory() + prior = PersonUUIDFactory(person=person) + push_person_uuids_task(person_pk=person.pk) + message = mock_log.call_args[0][0] + self.assertIn(str(person.primary_uuid), message) + self.assertIn(str(prior.uuid), message) + + mock_log.reset_mock() + push_person_uuids_task(person_pk=person.pk + 10000) + self.assertIn("no such Person", mock_log.call_args[0][0]) + + @mock.patch("ietf.person.tasks.log.log") + def test_check_person_uuids_task(self, mock_log): + good = PersonFactory() + broken = PersonFactory(primary_uuid=False) + demoted = PersonFactory() + demoted.uuids.update(primary=False) # deliberately inconsistent state + + def logged(): + return "\n".join(call[0][0] for call in mock_log.call_args_list) + + self.assertEqual(check_person_uuids_task(), 2) + report = logged() + self.assertIn(f"Person {broken.pk}", report) + self.assertIn("no UUIDs at all", report) + self.assertIn(f"Person {demoted.pk}", report) + self.assertIn("no primary UUID", report) + self.assertNotIn(f"Person {good.pk} ", report) + self.assertIn("2 Person(s) need attention", report) + + mock_log.reset_mock() + self.assertEqual(check_person_uuids_task(fix=True), 2) + self.assertIn("2 Person(s) repaired", logged()) + for person in (broken, demoted): + self.assertIsNotNone(person.primary_uuid) + + mock_log.reset_mock() + self.assertEqual(check_person_uuids_task(), 0) + self.assertIn("every Person has exactly one primary UUID", logged()) + + +@override_settings( + APP_API_TOKENS={ + "ietf.person.api_uuid": ["uuid-api-token"], + "ietf.person.api_uuid_by_pk": ["by-pk-token"], + } +) +class PersonUUIDApiTests(TestCase): + def retrieve_url(self, uuid_value): + return urlreverse( + "ietf.api.person_api.person-uuid-detail", kwargs={"uuid": uuid_value} + ) + + @property + def lookup_url(self): + return urlreverse("ietf.api.person_api.person-uuid-lookup") + + @property + def by_pk_url(self): + return urlreverse("ietf.api.person_api.person-uuid-by-pk") + + def test_requires_a_valid_api_key(self): + person = PersonFactory() + url = self.retrieve_url(person.primary_uuid) + self.assertEqual(self.client.get(url).status_code, 403) + self.assertEqual( + self.client.get(url, headers={"X-Api-Key": "nope"}).status_code, 403 + ) + self.assertEqual( + self.client.get(url, headers={"X-Api-Key": "by-pk-token"}).status_code, 403 + ) + self.assertEqual( + self.client.get(url, headers={"X-Api-Key": "uuid-api-token"}).status_code, + 200, + ) + + def test_retrieve_primary(self): + person = PersonFactory() + r = self.client.get( + self.retrieve_url(person.primary_uuid), + headers={"X-Api-Key": "uuid-api-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertEqual( + r.json(), + { + "uuid": str(person.primary_uuid), + "is_primary": True, + "primary_uuid": str(person.primary_uuid), + "prior_uuids": [], + }, + ) + + def test_retrieve_superseded(self): + person = PersonFactory() + prior = PersonUUIDFactory(person=person) + r = self.client.get( + self.retrieve_url(prior.uuid), headers={"X-Api-Key": "uuid-api-token"} + ) + self.assertEqual(r.status_code, 200) + self.assertEqual( + r.json(), + { + "uuid": str(prior.uuid), + "is_primary": False, + "primary_uuid": str(person.primary_uuid), + "prior_uuids": [str(prior.uuid)], + }, + ) + + def test_response_carries_identifiers_only(self): + person = PersonFactory() + r = self.client.get( + self.retrieve_url(person.primary_uuid), + headers={"X-Api-Key": "uuid-api-token"}, + ) + self.assertEqual( + set(r.json().keys()), + {"uuid", "is_primary", "primary_uuid", "prior_uuids"}, + ) + + def test_retrieve_unknown(self): + r = self.client.get( + self.retrieve_url(uuid.uuid4()), headers={"X-Api-Key": "uuid-api-token"} + ) + self.assertEqual(r.status_code, 404) + + def test_retrieve_upper_case(self): + person = PersonFactory() + url = self.retrieve_url(person.primary_uuid) + upper = url.replace(str(person.primary_uuid), str(person.primary_uuid).upper()) + self.assertNotEqual(url, upper) + r = self.client.get(upper, headers={"X-Api-Key": "uuid-api-token"}) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.json()["uuid"], str(person.primary_uuid)) + + def test_retrieve_malformed(self): + r = self.client.get( + "/api/person/uuid/not-a-uuid/", headers={"X-Api-Key": "uuid-api-token"} + ) + self.assertEqual(r.status_code, 404) + + def test_batch(self): + person = PersonFactory() + prior = PersonUUIDFactory(person=person) + missing = uuid.uuid4() + r = self.client.post( + self.lookup_url, + { + "uuids": [ + str(prior.uuid), + str(person.primary_uuid), + str(missing), + str(prior.uuid), + ] + }, + content_type="application/json", + headers={"X-Api-Key": "uuid-api-token"}, + ) + self.assertEqual(r.status_code, 200) + results = r.json()["results"] + # One entry per distinct requested UUID, duplicates collapsed + self.assertEqual(len(results), 3) + by_uuid = {entry["uuid"]: entry for entry in results} + # Same shape as a resolved entry, with the identifiers nulled out + self.assertEqual( + by_uuid[str(missing)], + { + "uuid": str(missing), + "status": "unknown", + "is_primary": None, + "primary_uuid": None, + "prior_uuids": [], + }, + ) + self.assertEqual(by_uuid[str(prior.uuid)]["status"], "resolved") + self.assertFalse(by_uuid[str(prior.uuid)]["is_primary"]) + self.assertEqual( + by_uuid[str(prior.uuid)]["primary_uuid"], str(person.primary_uuid) + ) + self.assertTrue(by_uuid[str(person.primary_uuid)]["is_primary"]) + + def test_batch_query_count_is_independent_of_size(self): + people = PersonFactory.create_batch(6) + # Resolve the UUIDs up front so the capture below sees only the API's queries + values = [str(p.primary_uuid) for p in people] + + def post(subset): + return self.client.post( + self.lookup_url, + {"uuids": subset}, + content_type="application/json", + headers={"X-Api-Key": "uuid-api-token"}, + ) + + with CaptureQueriesContext(connection) as small: + self.assertEqual(post(values[:2]).status_code, 200) + with CaptureQueriesContext(connection) as large: + self.assertEqual(post(values).status_code, 200) + self.assertEqual(len(small.captured_queries), len(large.captured_queries)) + + def test_batch_rejects_bad_input(self): + for payload in ( + {"uuids": []}, + {"uuids": ["not-a-uuid"]}, + {"uuids": [str(uuid.uuid4()) for _ in range(501)]}, + {}, + ): + r = self.client.post( + self.lookup_url, + payload, + content_type="application/json", + headers={"X-Api-Key": "uuid-api-token"}, + ) + self.assertEqual(r.status_code, 400, payload) + + def test_by_person_pk(self): + person = PersonFactory() + prior = PersonUUIDFactory(person=person) + r = self.client.post( + self.by_pk_url, + {"person_pks": [person.pk, person.pk + 10000]}, + content_type="application/json", + headers={"X-Api-Key": "by-pk-token"}, + ) + self.assertEqual(r.status_code, 200) + results = r.json()["results"] + self.assertEqual(len(results), 2) + self.assertEqual( + results[0], + { + "person_pk": person.pk, + "status": "resolved", + "primary_uuid": str(person.primary_uuid), + "prior_uuids": [str(prior.uuid)], + }, + ) + self.assertEqual( + results[1], + { + "person_pk": person.pk + 10000, + "status": "unknown", + "primary_uuid": None, + "prior_uuids": [], + }, + ) + + def test_by_person_pk_has_its_own_token(self): + person = PersonFactory() + r = self.client.post( + self.by_pk_url, + {"person_pks": [person.pk]}, + content_type="application/json", + headers={"X-Api-Key": "uuid-api-token"}, + ) + self.assertEqual(r.status_code, 403) + + def test_by_person_pk_rejects_over_cap(self): + r = self.client.post( + self.by_pk_url, + {"person_pks": list(range(501))}, + content_type="application/json", + headers={"X-Api-Key": "by-pk-token"}, + ) + self.assertEqual(r.status_code, 400) + + def test_schema(self): + r = self.client.get("/api/schema/") + self.assertEqual(r.status_code, 200) + schema = yaml.safe_load(r.content) + paths = schema["paths"] + self.assertIn("/api/person/uuid/{uuid}/", paths) + self.assertIn("/api/person/uuid/lookup/", paths) + self.assertIn("/api/person/uuid/by-person-pk/", paths) + self.assertEqual( + paths["/api/person/uuid/{uuid}/"]["get"]["operationId"], + "person_uuid_retrieve", + ) + self.assertEqual( + paths["/api/person/uuid/lookup/"]["post"]["operationId"], + "person_uuid_lookup", + ) + by_pk = paths["/api/person/uuid/by-person-pk/"]["post"] + self.assertEqual(by_pk["operationId"], "person_uuid_by_person_pk") + self.assertTrue(by_pk["deprecated"]) + self.assertIn("PersonUUIDResolution", schema["components"]["schemas"]) + for path, method in ( + ("/api/person/uuid/{uuid}/", "get"), + ("/api/person/uuid/lookup/", "post"), + ("/api/person/uuid/by-person-pk/", "post"), + ): + responses = paths[path][method]["responses"] + self.assertIn("200", responses, path) + # Consumers switch on status, so it has to be a declared, required field + for component in ("PersonUUIDBatchEntry", "PersonPkBatchEntry"): + entry = schema["components"]["schemas"][component] + self.assertIn("status", entry["required"], component) + self.assertTrue(entry["properties"]["primary_uuid"]["nullable"], component) + + class TaskTests(TestCase): @mock.patch("ietf.person.tasks.log.log") def test_purge_personal_api_key_events_task(self, mock_log): diff --git a/ietf/person/urls.py b/ietf/person/urls.py index f3eccd04b73..15338d48595 100644 --- a/ietf/person/urls.py +++ b/ietf/person/urls.py @@ -1,6 +1,8 @@ # Copyright The IETF Trust 2009-2025, All Rights Reserved # -*- coding: utf-8 -*- -from ietf.person import views, ajax +from django.urls import path + +from ietf.person import ajax, views from ietf.utils.urls import url urlpatterns = [ @@ -9,6 +11,8 @@ url(r'^merge/send_request/?$', views.send_merge_request), url(r'^search/(?P(person|email))/$', views.ajax_select2_search), url(r'^(?P[0-9]+)/email.json$', ajax.person_email_json), + path('', views.profile_by_uuid, + name='ietf.person.views.profile_by_uuid'), url(r'^(?P[^/]+)$', views.profile), url(r'^(?P[^/]+)/photo/?$', views.photo), ] diff --git a/ietf/person/utils.py b/ietf/person/utils.py index 5ed90591f9a..3f00a04cd0d 100755 --- a/ietf/person/utils.py +++ b/ietf/person/utils.py @@ -9,15 +9,92 @@ from django.contrib import admin from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction from django.db.models import Q from django.http import Http404 +from kombu.exceptions import OperationalError as KombuOperationalError import debug # pyflakes:ignore -from ietf.person.models import Person, Alias, Email +from ietf.person.models import Person, Alias, Email, PersonUUID from ietf.utils import log from ietf.utils.mail import send_mail + +def get_person_uuid_object(uuid_value): + """Look up a UUID currently issued for a Person + + Returns the PersonUUID object, whose `person` is the Person it identifies and whose + `primary` says whether it is that Person's current identifier, or None if no such + UUID exists. + """ + return PersonUUID.objects.select_related("person").filter(uuid=uuid_value).first() + + +def queue_person_uuid_push(person): + """Enqueue an Authentik attribute push for this Person's UUID set + + Called explicitly by every site that changes the set for a Person that may already + have an Authentik account. Deferred to commit so a rolled-back transaction never + pushes state that does not exist. + + Queueing is best-effort: an unreachable broker is logged and ignored rather than + failing the datatracker operation that changed the UUID set. Celery's default retry + policy applies - three attempts over well under a second, enough to ride out a blip + or a broker failover without meaningfully delaying the caller. An outright outage + still cannot fail the operation, and the reconcile job is the backstop for a push + that never got queued. + """ + from ietf.person.tasks import push_person_uuids_task # avoid a circular import + + person_pk = person.pk + + def enqueue(): + try: + push_person_uuids_task.apply_async(kwargs={"person_pk": person_pk}) + except (KombuOperationalError, OSError) as err: + log.log(f"Could not queue UUID push for Person {person_pk}: {err}") + + transaction.on_commit(enqueue) + + +def assign_primary_uuid(person): + """Give a newly created Person its primary UUID + + Idempotent: returns the existing primary if the Person already has one, so a caller + that is unsure whether an earlier step already ran can call it safely. + + Deliberately does not queue an Authentik push. A Person that has just been created + has no Authentik account, so there would be nothing to push to; its UUID reaches + Authentik when the account is linked. + """ + existing = person.uuids.filter(primary=True).first() + if existing is not None: + return existing + return PersonUUID.objects.create(person=person, primary=True) + + +def ensure_primary_uuid(person): + """Make sure a Person has exactly one primary UUID + + Promotes the earliest existing UUID if there are any but none is primary, otherwise + creates one. Returns the primary PersonUUID. + """ + existing = person.uuids.filter(primary=True).first() + if existing is not None: + return existing + oldest = person.uuids.order_by("time", "uuid").first() + if oldest is None: + row = assign_primary_uuid(person) + else: + oldest.primary = True + oldest.save(update_fields=["primary"]) + row = oldest + # Unlike a brand-new Person, this one already existed and may be linked. + queue_person_uuid_push(person) + return row + + def merge_persons(request, source, target, file=sys.stdout, verbose=False): changes = [] @@ -49,6 +126,15 @@ def merge_persons(request, source, target, file=sys.stdout, verbose=False): if reviewer_changes: changes.extend(reviewer_changes) merge_nominees(source, target) + + # Move the source's UUIDs to the target, demoting the source's primary. The target's + # primary survives; the source's identifiers keep resolving, to the target. This runs + # before move_related_objects(), which would otherwise carry the UUIDs across still + # flagged primary and trip the one-primary-per-person constraint. + ensure_primary_uuid(target) + source.uuids.update(person=target, primary=False) + queue_person_uuid_push(target) + move_related_objects(source, target, file=file, verbose=verbose) dedupe_aliases(target) @@ -131,6 +217,10 @@ def move_related_objects(source, target, file, verbose=False): and f.auto_created and not f.concrete ] for related_object in related_objects: accessor = related_object.get_accessor_name() + if accessor == "uuids": + # PersonUUIDs move by their own rule - the source's primary has to be + # demoted on the way, or the target ends up with two. See merge_persons(). + continue field_name = related_object.field.name queryset = getattr(source, accessor).all() if verbose: diff --git a/ietf/person/views.py b/ietf/person/views.py index d0b5912431e..52137f0fb91 100644 --- a/ietf/person/views.py +++ b/ietf/person/views.py @@ -19,7 +19,12 @@ from ietf.person.models import Email, Person from ietf.person.fields import select2_id_name_json from ietf.person.forms import MergeForm, MergeRequestForm -from ietf.person.utils import handle_users, merge_persons, lookup_persons +from ietf.person.utils import ( + get_person_uuid_object, + handle_users, + lookup_persons, + merge_persons, +) from ietf.utils.mail import send_mail_text @@ -77,6 +82,23 @@ def profile(request, email_or_name): return render(request, 'person/profile.html', {'persons': persons, 'today': timezone.now()}) +def profile_by_uuid(request, uuid): + person_uuid = get_person_uuid_object(uuid) + if person_uuid is None: + raise Http404("No such person identifier") + if not person_uuid.primary: + # Self-heal a link that predates a merge by sending it to the canonical address. + return redirect( + "ietf.person.views.profile_by_uuid", + uuid=person_uuid.person.primary_uuid, + ) + return render( + request, + "person/profile.html", + {"persons": [person_uuid.person], "today": timezone.now()}, + ) + + def photo(request, email_or_name): persons = lookup_persons(email_or_name) if len(persons) > 1: diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 61494738d35..cf5482a0c1a 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -73,8 +73,13 @@ def can_access_review_stats_for_team(user, team): or has_role(user, ["Secretariat", "Area Director"])) def review_assignments_to_list_for_docs(docs): + # The document table renders the request's doc, team and type, and links to the + # review itself, for every assignment it shows -- fetch them with the assignment + # rather than one query per attribute per row. assignment_qs = ReviewAssignment.objects.filter( state__in=["assigned", "accepted", "part-completed", "completed"], + ).select_related( + "review_request__doc", "review_request__team", "review_request__type", "review" ).prefetch_related("result") doc_names = [d.name for d in docs] diff --git a/ietf/secr/announcement/forms.py b/ietf/secr/announcement/forms.py index 91004ea2705..e00b5af4100 100644 --- a/ietf/secr/announcement/forms.py +++ b/ietf/secr/announcement/forms.py @@ -9,6 +9,7 @@ from ietf.ietfauth.utils import has_role from ietf.message.models import Message, AnnouncementFrom from ietf.utils.fields import MultiEmailField +from ietf.utils.mail import is_valid_email # --------------------------------------------- # Globals @@ -145,13 +146,31 @@ def __init__(self, *args, **kwargs): for key in list(self.fields.keys()): self.fields[key].widget = forms.HiddenInput() + def clean_cc(self): + cc_data = self.cleaned_data["cc"] + cc = [addr.strip() for addr in cc_data.split(",") if addr.strip()] + errors = [ + forms.ValidationError( + "Invalid address: %(addr)s", "invalid_cc", {"addr": addr} + ) + for addr in cc + if not is_valid_email(addr) + ] + if errors: + raise forms.ValidationError(errors) + return cc_data + def clean(self): super(AnnounceForm, self).clean() data = self.cleaned_data if self.errors: return self.cleaned_data - if data["to"] == "Other..." and not data["to_custom"]: - raise forms.ValidationError('You must enter a "To" email address') + + if data.get("to") == "Other..." and data.get("to_custom", []) == []: + self.add_error( + None, forms.ValidationError('Must specify a "To" address', "empty_to") + ) + for k in [ "to", "frm", diff --git a/ietf/secr/announcement/tests.py b/ietf/secr/announcement/tests.py index f08e824397e..ebd3a6a69aa 100644 --- a/ietf/secr/announcement/tests.py +++ b/ietf/secr/announcement/tests.py @@ -100,6 +100,7 @@ def test_valid_submit(self): "nomcom": nomcom.pk, "to": "Other...", "to_custom": "phil@example.com", + "cc": "lizz@example.com, no-brackets@example.com", "frm": "IETF Secretariat <ietf-secretariat@ietf.org>", "reply_to": "secretariat@ietf.org", "subject": "Test Subject", @@ -113,6 +114,77 @@ def test_valid_submit(self): self.assertEqual(len(outbox), 1) self.assertEqual(outbox[0]["subject"], "Test Subject") self.assertEqual(outbox[0]["to"], "") + self.assertEqual(outbox[0]["cc"], "lizz@example.com, no-brackets@example.com") message = Message.objects.filter(by__user__username="secretary").last() self.assertEqual(message.subject, "Test Subject") self.assertTrue(nomcom in message.related_groups.all()) + + def test_empty_submit(self): + "Submit Empty Email Fields" + nomcom_test_data() + empty_outbox() + url = reverse("ietf.secr.announcement.views.main") + nomcom = Group.objects.get(type="nomcom") + post_data = { + "nomcom": nomcom.pk, + "to": "Other...", + "to_custom": "", + "cc": "", + "frm": "IETF Secretariat <ietf-secretariat@ietf.org>", + "reply_to": "secretariat@ietf.org", + "subject": "Test Subject", + "body": "This is a test.", + } + self.client.login(username="secretary", password="secretary+password") + response = self.client.post(url, post_data) + self.assertNotContains(response, "Confirm Announcement") + self.assertEqual(len(outbox), 0) + + def test_invalid_submit(self): + "Invalid Submit" + nomcom_test_data() + empty_outbox() + url = reverse("ietf.secr.announcement.views.main") + nomcom = Group.objects.get(type="nomcom") + post_data = { + "nomcom": nomcom.pk, + "to": "Other...", + "to_custom": "phil@example.com", + "cc": "lizz@example.com, invalid_email@example", + "frm": "IETF Secretariat <ietf-secretariat@ietf.org>", + "reply_to": "secretariat@ietf.org", + "subject": "Test Subject", + "body": "This is a test.", + } + post_data_badlist_to = { + "nomcom": nomcom.pk, + "to": "Other...", + "to_custom": "phil@example.com; test@example.com", + "cc": "lizz@example.com, person@example.com", + "frm": "IETF Secretariat <ietf-secretariat@ietf.org>", + "reply_to": "secretariat@ietf.org", + "subject": "Test Subject", + "body": "This is a test.", + } + post_data_badlist_cc = { + "nomcom": nomcom.pk, + "to": "Other...", + "to_custom": "phil@example.com, test@example.com", + "cc": "lizz@example.com; person@example.com", + "frm": "IETF Secretariat <ietf-secretariat@ietf.org>", + "reply_to": "secretariat@ietf.org", + "subject": "Test Subject", + "body": "This is a test.", + } + self.client.login(username="secretary", password="secretary+password") + response = self.client.post(url, post_data) + self.assertNotContains(response, "Confirm Announcement") + self.assertEqual(len(outbox), 0) + response = self.client.post(url, post_data_badlist_to) + self.assertNotContains(response, "Confirm Announcement") + self.assertEqual(len(outbox), 0) + response = self.client.post(url, post_data_badlist_cc) + self.assertNotContains(response, "Confirm Announcement") + self.assertEqual(len(outbox), 0) + + diff --git a/ietf/settings.py b/ietf/settings.py index d2a622d63e6..4b3bbb2a153 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -121,6 +121,14 @@ } +# Collation that we wish we were using. The production database is currently using +# the C collation, which does not handle accented characters. Do not change this +# without confirming that the production and dev databases support the new collation. +# This setting and places we use it can go away if we switch the production database +# collation. That requires creating and populating a new database, it cannot be done +# on an existing one. +PREFERRED_COLLATION = "en-US-x-icu" + # Local time zone for this installation. Choices can be found here: # http://www.postgresql.org/docs/8.1/static/datetime-keywords.html#DATETIME-TIMEZONE-SET-TABLE # although not all variations may be possible on all operating systems. @@ -1283,22 +1291,6 @@ def skip_unreadable_post(record): UTILS_APIKEY_GUI_LOGIN_LIMIT_DAYS = 30 -API_KEY_TYPE="ES256" # EC / P=256 -API_PUBLIC_KEY_PEM = b""" ------BEGIN PUBLIC KEY----- -MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEqVojsaofDJScuMJN+tshumyNM5ME -garzVPqkVovmF6yE7IJ/dv4FcV+QKCtJ/rOS8e36Y8ZAEVYuukhes0yZ1w== ------END PUBLIC KEY----- -""" -API_PRIVATE_KEY_PEM = b""" ------BEGIN PRIVATE KEY----- -MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgoI6LJkopKq8XrHi9 -QqGQvE4A83TFYjqLz+8gULYecsqhRANCAASpWiOxqh8MlJy4wk362yG6bI0zkwSB -qvNU+qRWi+YXrITsgn92/gVxX5AoK0n+s5Lx7fpjxkARVi66SF6zTJnX ------END PRIVATE KEY----- -""" - - # Default timeout for HTTP requests via the requests library DEFAULT_REQUESTS_TIMEOUT = 20 # seconds diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 457462e4f2c..b331d71631f 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -60,6 +60,7 @@ from ietf.utils.timezone import date_today from ietf.utils.xmldraft import InvalidMetadataError, XMLDraft, capture_xml2rfc_output from ietf.person.name import unidecode_name +from ietf.person.utils import assign_primary_uuid def validate_submission(submission): @@ -589,7 +590,11 @@ def ensure_person_email_info_exists(name, email, docname): person.name_from_draft = name log.assertion('isinstance(person.name, str)') person.ascii = unidecode_name(person.name) - person.save() + # Atomic so a Person is never left without the primary UUID that external + # systems need to name them by. + with transaction.atomic(): + person.save() + assign_primary_uuid(person) else: person.name_from_draft = name diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py index 24d3c77b3be..3a037667e28 100644 --- a/ietf/sync/tasks.py +++ b/ietf/sync/tasks.py @@ -11,8 +11,17 @@ from django.conf import settings from django.utils import timezone - -from ietf.doc.models import DocEvent, DocTagName, Document, RelatedDocument, RpcAssignmentDocEvent, State +from django.utils.dateparse import parse_datetime + +from ietf.doc.models import ( + DocEvent, + DocTagName, + Document, + RelatedDocument, + RpcActionHolderOpenEntry, + RpcAssignmentDocEvent, + State, +) from ietf.doc.tasks import rebuild_reference_relations_task from ietf.doc.utils import add_state_change_event, new_state_change_event, update_action_holders from ietf.person.models import Person @@ -382,6 +391,71 @@ def format_rpc_queue_status(obj): return ", ".join(parts) +# The datatracker's "(System)" person. The RPC tool substitutes a placeholder +# person of its own whenever no real person was named, and sends that person's +# datatracker id - so an action holder must never be associated with this pk. +# Which pk the RPC tool uses is configurable at its end, hence the separate +# check against the "(System)" person the datatracker actually has. +SYSTEM_PERSON_ID = 1 + + +def _rpc_action_holder_person(holder, system_person): + """Resolve the datatracker Person for one action holder, if there is one + + Returns None whenever the entry does not name a person the datatracker can + act on. The RPC tool substitutes its system person when no real person was + named, and that placeholder must never become an action holder here. Do not + use "body" to make this decision - the RPC tool's edit path can set or clear + "body" without touching the person, so it is unreliable in both directions. + """ + person_id = (holder.get("person") or {}).get("person_id") + if person_id in (SYSTEM_PERSON_ID, system_person.pk): + return None + if holder.get("body"): + return None # a body holds the action, not a person + person = Person.objects.filter(pk=person_id).first() + if person is None: + log.log( + f"process_rpc_queue_task: unknown action holder person {person_id}" + ) + return None + return person + + +def _sync_rpc_action_holders(doc, holders, rfc_number, system_person): + """Reconcile the open action holder entries for one document""" + open_purple_ids = [] + for holder in holders: + if holder.get("completed") is not None: + # The RPC tool reports completed action holders as well as open + # ones. Only open ones are kept here, so this line is the only + # place the datatracker sees a completion happen. + continue + RpcActionHolderOpenEntry.objects.update_or_create( + purple_id=holder["id"], + defaults=dict( + document=doc, + person=_rpc_action_holder_person(holder, system_person), + body=holder.get("body") or "", + display_name=holder.get("display_name") or "", + comment=holder.get("comment") or "", + rfc_number=rfc_number, + since_when=parse_datetime(holder["since_when"]), + deadline=( + parse_datetime(holder["deadline"]) + if holder.get("deadline") + else None + ), + ), + ) + open_purple_ids.append(holder["id"]) + # Anything else we were holding for this document has been completed or + # removed at the RPC tool. + RpcActionHolderOpenEntry.objects.filter(document=doc).exclude( + purple_id__in=open_purple_ids + ).delete() + + @shared_task def process_rpc_queue_task(data: list): in_progress_state = State.objects.get( @@ -485,6 +559,10 @@ def process_rpc_queue_task(data: list): d.tags.remove(*iana_ref_tags) + _sync_rpc_action_holders( + d, obj.get("actionholder_set") or [], rfc_number, system + ) + if events: d.save_with_history(events) @@ -495,3 +573,4 @@ def process_rpc_queue_task(data: list): ): d.tags.remove(*iana_ref_tags) d.unset_state("draft-rfceditor") + RpcActionHolderOpenEntry.objects.filter(document=d).delete() diff --git a/ietf/sync/tests_tasks.py b/ietf/sync/tests_tasks.py index 264a5f46bb3..7f0c5aee960 100644 --- a/ietf/sync/tests_tasks.py +++ b/ietf/sync/tests_tasks.py @@ -1,5 +1,6 @@ # Copyright The IETF Trust 2026, All Rights Reserved +import datetime import mock from django.test.utils import override_settings @@ -9,9 +10,11 @@ DocTagName, Document, DocumentURL, + RpcActionHolderOpenEntry, RpcAssignmentDocEvent, State, ) +from ietf.person.factories import PersonFactory from ietf.person.models import Person from ietf.sync import tasks from ietf.utils.mail import outbox @@ -19,7 +22,12 @@ def _make_entry( - doc_name, roles=None, blocking_reasons=None, rfc_number=None, final_approval=None + doc_name, + roles=None, + blocking_reasons=None, + rfc_number=None, + final_approval=None, + action_holders=None, ): return { "name": doc_name, @@ -27,6 +35,23 @@ def _make_entry( "blocking_reasons": blocking_reasons or [], "rfc_number": rfc_number, "final_approval": final_approval or [], + "actionholder_set": action_holders or [], + } + + +def _make_action_holder( + purple_id=1, person_id=None, body="", completed=None, deadline=None, comment="" +): + """Build one actionholder_set element as the RPC tool sends it""" + return { + "id": purple_id, + "person": {"person_id": person_id, "name": "Anybody"}, + "display_name": body or "Anybody", + "deadline": deadline, + "since_when": "2026-08-11T12:00:00Z", + "completed": completed, + "comment": comment, + "body": body, } @@ -481,6 +506,136 @@ def test_docs_in_queue_retain_rfceditor_state(self): self.assertIsNotNone(draft.get_state("draft-rfceditor")) +class RpcActionHolderSyncTests(TestCase): + def setUp(self): + super().setUp() + self.draft = WgDraftFactory(states=[("draft-iesg", "rfcqueue")]) + self.person = PersonFactory() + + def _sync(self, *holders, rfc_number=None): + tasks.process_rpc_queue_task( + [ + _make_entry( + self.draft.name, + roles=["first_editor"], + rfc_number=rfc_number, + action_holders=list(holders), + ) + ] + ) + return RpcActionHolderOpenEntry.objects.filter(document=self.draft) + + def test_person_holder_is_stored(self): + """An action holder naming a person is kept, with that person.""" + entries = self._sync( + _make_action_holder( + purple_id=17, + person_id=self.person.pk, + comment="Please confirm the change in section 4.2.", + deadline="2026-09-01T12:00:00Z", + ) + ) + self.assertEqual(entries.count(), 1) + entry = entries.first() + self.assertEqual(entry.purple_id, 17) + self.assertEqual(entry.person, self.person) + self.assertEqual(entry.body, "") + self.assertEqual(entry.comment, "Please confirm the change in section 4.2.") + self.assertEqual(entry.since_when.date(), datetime.date(2026, 8, 11)) + self.assertEqual(entry.deadline.date(), datetime.date(2026, 9, 1)) + + def test_body_holder_is_stored_without_a_person(self): + """An action held by a body is kept, but belongs to nobody.""" + entries = self._sync(_make_action_holder(person_id=self.person.pk, body="IANA")) + self.assertEqual(entries.count(), 1) + self.assertIsNone(entries.first().person) + self.assertEqual(entries.first().body, "IANA") + + def test_system_person_id_is_never_an_action_holder(self): + """The RPC tool's placeholder person never becomes an action holder. + + It arrives both with and without a body set - the RPC tool's edit path + can clear the body without changing the person - so the person id alone + has to be enough to reject it. + """ + for body in ("IANA", ""): + RpcActionHolderOpenEntry.objects.all().delete() + entries = self._sync( + _make_action_holder(person_id=tasks.SYSTEM_PERSON_ID, body=body) + ) + self.assertEqual(entries.count(), 1, f"body={body!r}") + self.assertIsNone(entries.first().person, f"body={body!r}") + + def test_person_resolving_to_system_is_never_an_action_holder(self): + """A person id that resolves to (System) is rejected on its own merits.""" + system = Person.objects.get(name="(System)") + entries = self._sync(_make_action_holder(person_id=system.pk)) + self.assertEqual(entries.count(), 1) + self.assertIsNone(entries.first().person) + + def test_unknown_person_is_stored_without_a_person(self): + """An unresolvable person is logged, and the entry kept and legible.""" + entries = self._sync(_make_action_holder(person_id=99999999)) + self.assertEqual(entries.count(), 1) + self.assertIsNone(entries.first().person) + self.assertEqual(entries.first().display_name, "Anybody") + + def test_completed_holder_is_not_stored(self): + """A completed action holder is dropped rather than kept.""" + entries = self._sync( + _make_action_holder( + person_id=self.person.pk, completed="2026-08-14T12:00:00Z" + ) + ) + self.assertEqual(entries.count(), 0) + + def test_completion_removes_a_stored_entry(self): + """An entry the RPC tool completes stops being held.""" + self._sync(_make_action_holder(purple_id=17, person_id=self.person.pk)) + entries = self._sync( + _make_action_holder( + purple_id=17, + person_id=self.person.pk, + completed="2026-08-14T12:00:00Z", + ) + ) + self.assertEqual(entries.count(), 0) + + def test_entry_dropped_from_the_payload_is_removed(self): + """An entry the RPC tool deletes stops being held.""" + self._sync(_make_action_holder(purple_id=17, person_id=self.person.pk)) + self.assertEqual(self._sync().count(), 0) + + def test_entries_are_removed_when_the_document_leaves_the_queue(self): + """Nothing is held for a document the RPC tool no longer reports.""" + self._sync(_make_action_holder(person_id=self.person.pk)) + tasks.process_rpc_queue_task([]) + self.assertFalse(RpcActionHolderOpenEntry.objects.exists()) + + def test_rfc_number_comes_from_the_queue_entry(self): + """The rfc number is captured from the entry, not the action holder.""" + entries = self._sync( + _make_action_holder(person_id=self.person.pk), rfc_number=9850 + ) + self.assertEqual(entries.first().rfc_number, 9850) + + def test_rfc_number_is_null_before_one_is_assigned(self): + """A document can hold an action before it has an rfc number.""" + entries = self._sync(_make_action_holder(person_id=self.person.pk)) + self.assertIsNone(entries.first().rfc_number) + + def test_existing_entry_is_updated_in_place(self): + """A second push updates the entry it already holds.""" + self._sync(_make_action_holder(purple_id=17, person_id=self.person.pk)) + entries = self._sync( + _make_action_holder( + purple_id=17, person_id=self.person.pk, comment="Now with a comment" + ) + ) + self.assertEqual(entries.count(), 1) + self.assertEqual(entries.first().comment, "Now with a comment") + + class FormatRpcQueueStatusTests(TestCase): """Unit tests for the queue "Status" renderer, mirroring the ietf-tools/queue site.""" diff --git a/ietf/templates/api/index.html b/ietf/templates/api/index.html index e21a50101ad..86bfcc46011 100644 --- a/ietf/templates/api/index.html +++ b/ietf/templates/api/index.html @@ -278,45 +278,5 @@

a regular login in order to activate access. -

- Signing Keys -

-

- When sending notifications to other APIs, the datatracker may sign - information with a - - RFC - 7515: JSON Web Signature (JWS) - , - using a public/private keypair with - this public key: -

-

- {{ key.export_public }} -

-

- or alternatively: -

- 
{{ key.export_to_pem }}
-

- To verify a signature and get the verified data using Python with the - - jwcrypto - - module, - you could do: -

- 
-from jwcrypto import jwk, jws
-
-# ... receive json web signed data as 'data', used below ...
-
-key = jwk.JWK()
-key.import_from_pem(API_PUBLIC_KEY_PEM)   # the key above
-jwstoken = jws.JWS()
-jwstoken.deserialize(data)
-jwstoken.verify(key)
-payload = jwstoken.payload
-
{% endblock %} \ No newline at end of file diff --git a/ietf/templates/doc/ad_list.html b/ietf/templates/doc/ad_list.html index cac709021ea..3db2ecd24f8 100644 --- a/ietf/templates/doc/ad_list.html +++ b/ietf/templates/doc/ad_list.html @@ -8,8 +8,8 @@ {% endblock %} {% block morecss %} table .border-bottom { border-bottom-color: var(--highcharts-neutral-color-80) !important; } - .highcharts-container .highcharts-axis-labels { - font-size: .7rem; + .highcharts-container .highcharts-axis-labels { + font-size: .7rem; fill: var(--bs-body-color) } .highcharts-container .highcharts-graph { stroke-width: 2.5; } @@ -37,6 +37,29 @@

IESG Dashboard

Documents in IESG Processing IESG view of Working Groups

+ {% if rpc_pending %} +

RPC decisions pending

+

+ Documents in the RFC Editor queue for which the RPC is currently waiting on a + decision from an Area Director. +

+ + + + + + + + + {% for row in rpc_pending %} + + + + + {% endfor %} + +
Area DirectorDecisions pending
{{ row.ad.plain_name }}{{ row.count }}
+ {% endif %} {% for dt in metadata %}

{{ dt.type.1 }} State Counts

@@ -45,7 +68,7 @@

{{ dt.type.1 }} State Counts

{% if dt.type.1 == "Internet-Draft" %} - {% endif %} + {% endif %} {% for state, state_name in dt.states %} + {% if rpc_action_holders %} + + + + + + + {% endif %}
Area DirectorPre pubreq @@ -72,7 +95,7 @@

{{ dt.type.1 }} State Counts

> {{ ad.pre_pubreq }} - {% endif %} + {% endif %} {% for state, state_name in dt.states %} 		@@ -201,7 +224,7 @@

{{ dt.type.1 }} State Counts

const sums = Array.from(table.querySelectorAll("[data-sum]")).reduce(( sumsAccumulator, cell - ) => { + ) => { const key = cell.dataset.sum const value = safeParseFloat(cell.textContent) if(key && !isNaN(value)) { diff --git a/ietf/templates/doc/canonical_link.html b/ietf/templates/doc/canonical_link.html new file mode 100644 index 00000000000..8732aa5ee48 --- /dev/null +++ b/ietf/templates/doc/canonical_link.html @@ -0,0 +1,3 @@ +{# Copyright The IETF Trust 2026, All Rights Reserved #} +{% load ietf_filters %} + diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index a39fdf6eb73..543b5606430 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -592,6 +592,27 @@ {{ rfc_editor_queue_status|default:rfc_editor_state }}
+ Action Holders + + + {% for entry in rpc_action_holders %} +
+ {% person_link entry.person %} + since {{ entry.since_when|date:"Y-m-d" }} + {% if show_rpc_action_holder_comments and entry.comment %} +
{{ entry.comment }}
+ {% endif %} +
+ {% endfor %} +
diff --git a/ietf/templates/doc/document_subseries.html b/ietf/templates/doc/document_subseries.html index f0273c9093d..8d233ed1fb0 100644 --- a/ietf/templates/doc/document_subseries.html +++ b/ietf/templates/doc/document_subseries.html @@ -4,6 +4,9 @@ {% load static %} {% load ietf_filters %} {% block title %}{{ doc.name|prettystdname }}{% endblock %} +{% block pagehead %} + {% include "doc/canonical_link.html" %} +{% endblock %} {% block content %} {% origin %} {{ top|safe }} diff --git a/ietf/templates/doc/drafts_for_ad.html b/ietf/templates/doc/drafts_for_ad.html index 18e5960e813..fcaf748b2b9 100644 --- a/ietf/templates/doc/drafts_for_ad.html +++ b/ietf/templates/doc/drafts_for_ad.html @@ -80,6 +80,39 @@

Missing ballot positions for {{ ad.name }}
{% endif %} + {% if rpc_action_holders %} +

RPC decisions pending from {{ ad.name }}

+ + + + + + + + + + + + {% for entry in rpc_action_holders %} + + + + + + + + {% endfor %} + +
DocumentQueue entryWaiting sinceDeadlineRequest
{{ entry.document.displayname_with_link }} + {% if entry.final_review_url %} + Final review + {% endif %} + {{ entry.since_when|date:"Y-m-d" }} + {% if entry.deadline %} + {{ entry.deadline|date:"Y-m-d" }} + {% endif %} + {{ entry.comment }}
+ {% endif %}

Documents for {{ ad.name }}

{% include "doc/search/search_results.html" with start_table=True end_table=True %} {% endblock %} diff --git a/ietf/templates/doc/opengraph.html b/ietf/templates/doc/opengraph.html index 1c8c5abe912..ef80fcbcee9 100644 --- a/ietf/templates/doc/opengraph.html +++ b/ietf/templates/doc/opengraph.html @@ -5,7 +5,7 @@ {% origin %} - +{% include "doc/canonical_link.html" %} diff --git a/ietf/templates/doc/search/status_columns.html b/ietf/templates/doc/search/status_columns.html index 50d70fcb379..353abf917d1 100644 --- a/ietf/templates/doc/search/status_columns.html +++ b/ietf/templates/doc/search/status_columns.html @@ -5,16 +5,17 @@
{% ballot_icon doc %}
{% if not doc.type_id == "rfc" %} - {% if '::' in doc.friendly_state %} - {{ doc.friendly_state|safe }} - {% else %} - {{ doc.friendly_state|safe }} - {% endif %} - {% if doc|state:"draft-rfceditor" %} - : {{ doc|state:"draft-rfceditor" }} - {% endif %} + {{ doc.friendly_state|safe }} {{ doc|auth48_alert_badge }} {{ doc|state_age_colored }} + {# The RFC Editor runs a state machine of its own, so its status is labeled rather than read as a substate of the state above. #} + {% with rfc_editor_state=doc|state:"draft-rfceditor" %} + {% if rfc_editor_state %} +
+ RFC Editor: + {{ doc.rfc_editor_queue_status|default:rfc_editor_state }} + {% endif %} + {% endwith %} {% if doc.telechat_date %}
IESG telechat: {{ doc.telechat_date }} diff --git a/ietf/templates/group/review_requests_history.html b/ietf/templates/group/review_requests_history.html index 1b1fb4d2636..34e12ad0fa1 100644 --- a/ietf/templates/group/review_requests_history.html +++ b/ietf/templates/group/review_requests_history.html @@ -18,7 +18,7 @@

Review requests history

-
diff --git a/ietf/templates/meeting/session_details_panel.html b/ietf/templates/meeting/session_details_panel.html index 7c52ac0b4ab..62fda934705 100644 --- a/ietf/templates/meeting/session_details_panel.html +++ b/ietf/templates/meeting/session_details_panel.html @@ -185,7 +185,7 @@

Slides

href="{% url 'ietf.meeting.views.upload_session_slides' session_id=session.pk num=session.meeting.number %}"> Upload new slides - {% elif request.user.is_authenticated and not session.is_material_submission_cutoff %} + {% elif request.user.is_authenticated and not session.is_past %} Propose slides diff --git a/ietf/templates/meeting/session_request_info.txt b/ietf/templates/meeting/session_request_info.txt index 2e96efb31f1..99502a5c573 100644 --- a/ietf/templates/meeting/session_request_info.txt +++ b/ietf/templates/meeting/session_request_info.txt @@ -1,7 +1,7 @@ {# Copyright The IETF Trust 2025, All Rights Reserved #} {% load ams_filters %} --------------------------------------------------------- -Working Group Name: {{ group.name }} +Group Name: {{ group.name }} Area Name: {{ group.parent }} Session Requester: {{ login }} {% if session.joint_with_groups %}{{ session.joint_for_session_display }} joint with: {{ session.joint_with_groups }}{% endif %} diff --git a/ietf/templates/meeting/session_request_view_table.html b/ietf/templates/meeting/session_request_view_table.html index a5cb85c2520..a5939f4755b 100644 --- a/ietf/templates/meeting/session_request_view_table.html +++ b/ietf/templates/meeting/session_request_view_table.html @@ -3,7 +3,7 @@
- Working Group Name + Group Name
{{ group.name }} ({{ group.acronym }}) diff --git a/ietf/urls.py b/ietf/urls.py index e822b2042ef..61ab4b2de88 100644 --- a/ietf/urls.py +++ b/ietf/urls.py @@ -6,7 +6,7 @@ from django.contrib.sitemaps import views as sitemap_views from django.contrib.staticfiles.urls import staticfiles_urlpatterns from django.http import HttpResponse -from django.urls import include, path +from django.urls import include, path, register_converter from django.views import static as static_view from django.views.generic import TemplateView from django.views.defaults import server_error @@ -17,9 +17,16 @@ from ietf.group.urls import group_urls, grouptype_urls, stream_urls from ietf.ipr.sitemaps import IPRMap from ietf.liaisons.sitemaps import LiaisonMap +from ietf.utils.converters import AnyCaseUUIDConverter from ietf.utils.urls import url +# Register path converters here, in the root URLconf, before urlpatterns names any of +# them. Django refuses to register a converter twice, so registering at the point of +# definition would make importing that module from two URLconfs an error. +register_converter(AnyCaseUUIDConverter, "anycase_uuid") + + # sometimes, this code gets called more than once, which is an # that seems impossible to work around. try: diff --git a/ietf/utils/converters.py b/ietf/utils/converters.py new file mode 100644 index 00000000000..d4ebbbb42b3 --- /dev/null +++ b/ietf/utils/converters.py @@ -0,0 +1,21 @@ +# Copyright The IETF Trust 2026, All Rights Reserved +"""URL path converters + +Registered in the root URLconf, not here - Django does not allow registering a converter +twice, so a module-level register_converter() would break as soon as two URLconfs +imported this module. +""" + +from django.urls.converters import UUIDConverter + + +class AnyCaseUUIDConverter(UUIDConverter): + """UUIDConverter that also accepts upper-case hex + + Django's built-in "uuid" converter matches lower case only, so an upper-cased + identifier would 404 rather than resolve. + """ + + regex = ( + "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + ) diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index c878ba49f10..9ba3c79edd9 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -245,6 +245,17 @@ def create_default_tasks(self): ), ) + PeriodicTask.objects.get_or_create( + name="Check Person UUIDs", + task="ietf.person.tasks.check_person_uuids_task", + kwargs=json.dumps({"fix": False}), + defaults={ + "enabled": False, + "crontab": self.crontabs["daily"], + "description": "Report Persons with no primary UUID", + }, + ) + PeriodicTask.objects.get_or_create( name="Run Yang model checks", task="ietf.submit.tasks.run_yang_model_checks_task", diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index c5d34727511..f7c4bb3efd4 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -22,6 +22,7 @@ from ietf.group.utils import setup_default_community_list_for_group from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, ReviewTypeName, ReviewTeamSettings ) from ietf.person.name import unidecode_name +from ietf.person.utils import assign_primary_uuid from ietf.utils.timezone import date_today @@ -40,6 +41,7 @@ def create_person(group, role_name, name=None, username=None, email_address=None user.set_password(password) user.save() person = Person.objects.create(name=name, ascii=unidecode_name(smart_str(name)), user=user) + assign_primary_uuid(person) email = Email.objects.create(address=email_address, person=person, origin=user.username) Role.objects.create(group=group, name_id=role_name, person=person, email=email) return person @@ -63,6 +65,7 @@ def make_immutable_base_data(): # system system_person = Person.objects.create(name="(System)", ascii="(System)") + assign_primary_uuid(system_person) Email.objects.create(address="", person=system_person, origin='test') # high-level groups @@ -120,6 +123,7 @@ def make_immutable_base_data(): for i in range(1, 10): u = User.objects.create(username="ad%s" % i) p = Person.objects.create(name="Ad No%s" % i, ascii="Ad No%s" % i, user=u) + assign_primary_uuid(p) email = Email.objects.create(address="ad%s@example.org" % i, person=p, origin=u.username) if i < 6: # active @@ -148,6 +152,7 @@ def make_immutable_base_data(): for i in range(1, 5): u = User.objects.create(username="irsgmember%s" % i) p = Person.objects.create(name="IRSG Member No%s" % i, ascii="IRSG Member No%s" % i, user=u) + assign_primary_uuid(p) email = Email.objects.create(address="irsgmember%s@example.org" % i, person=p, origin=u.username) Role.objects.create(name_id="member", group=irsg, person=p, email=email) @@ -250,6 +255,7 @@ def make_test_data(): u.set_password("plain+password") u.save() plainman = Person.objects.create(name="Plain Man", ascii="Plain Man", user=u) + assign_primary_uuid(plainman) email = Email.objects.create(address="plain@example.com", person=plainman, origin=u.username) # group personnel @@ -436,6 +442,7 @@ def make_review_data(doc): u.set_password("reviewer+password") u.save() reviewer = Person.objects.create(name="Some Réviewer", ascii="Some Reviewer", user=u) + assign_primary_uuid(reviewer) email = Email.objects.create(address="reviewer@example.com", person=reviewer, origin=u.username) for team in (team1, team2, team3): @@ -459,6 +466,7 @@ def make_review_data(doc): u.set_password("reviewsecretary+password") u.save() reviewsecretary = Person.objects.create(name="Réview Secretary", ascii="Review Secretary", user=u) + assign_primary_uuid(reviewsecretary) reviewsecretary_email = Email.objects.create(address="reviewsecretary@example.com", person=reviewsecretary, origin=u.username) Role.objects.create(name_id="secr", person=reviewsecretary, email=reviewsecretary_email, group=team1) @@ -466,6 +474,7 @@ def make_review_data(doc): u.set_password("reviewsecretary3+password") u.save() reviewsecretary3 = Person.objects.create(name="Réview Secretary3", ascii="Review Secretary3", user=u) + assign_primary_uuid(reviewsecretary3) reviewsecretary3_email = Email.objects.create(address="reviewsecretary3@example.com", person=reviewsecretary, origin=u.username) Role.objects.create(name_id="secr", person=reviewsecretary3, email=reviewsecretary3_email, group=team3) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index 0a46fdf807a..f0245237670 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -251,6 +251,34 @@ def load_and_run_fixtures(verbosity): fn = getattr(module, components[-1]) fn() + check_base_data_person_uuids() + + +def check_base_data_person_uuids(): + """Every Person in the base test data must have exactly one primary UUID + + UUIDs are assigned by an explicit assign_primary_uuid() call at each site that + creates a Person (see ietf.person.utils), so a new site added without one would + otherwise go unnoticed until something asked for the Person's identifier. + """ + from django.db.models import Count, Q + + from ietf.person.models import Person + + broken = list( + Person.objects.annotate( + primary_count=Count("uuids", filter=Q(uuids__primary=True), distinct=True) + ) + .exclude(primary_count=1) + .values_list("pk", "name", "primary_count")[:10] + ) + if broken: + raise RuntimeError( + "Base test data has Persons without exactly one primary UUID - a Person " + "with none needs an assign_primary_uuid() call where it is created: " + + ", ".join(f"{pk} ({name}): {n} primary" for pk, name, n in broken) + ) + def safe_create_test_db(self, verbosity, *args, **kwargs): if old_create is None: raise RuntimeError("old_create has not been set, cannot proceed") diff --git a/k8s/README.md b/k8s/README.md index 3966101ab80..649e4058dd9 100644 --- a/k8s/README.md +++ b/k8s/README.md @@ -2,4 +2,5 @@ ## Run locally -The `secrets.yaml` file is provided as a reference only and must be referenced manually in the `kustomization.yaml` file. \ No newline at end of file +The `secrets.yaml.example` file is provided as a reference. To use it, rename it to `secrets.yaml`, update its contents, and reference it in the `kustomization.yaml` file. + diff --git a/k8s/datatracker.yaml b/k8s/datatracker.yaml index 2a96ab63bce..5183893bc8c 100644 --- a/k8s/datatracker.yaml +++ b/k8s/datatracker.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: name: datatracker spec: - replicas: 2 + replicas: 1 revisionHistoryLimit: 2 selector: matchLabels: diff --git a/k8s/secrets.yaml b/k8s/secrets.yaml.example similarity index 96% rename from k8s/secrets.yaml rename to k8s/secrets.yaml.example index 4d326521467..38fb06b3670 100644 --- a/k8s/secrets.yaml +++ b/k8s/secrets.yaml.example @@ -13,7 +13,7 @@ stringData: Nicolas Giard DATATRACKER_ALLOWED_HOSTS: ".ietf.org" # newline-separated list also allowed # DATATRACKER_DATATRACKER_DEBUG: "false" - + # DB access details - needs to be filled in # DATATRACKER_DB_HOST: "db" # DATATRACKER_DB_PORT: "5432" @@ -22,15 +22,15 @@ stringData: # DATATRACKER_DB_PASS: "RkTkDPFnKpko" # secret # DATATRACKER_DB_CONN_MAX_AGE: "0" # connection per request if not set, no limit if set to "None" # DATATRACKER_DB_CONN_HEALTH_CHECKS: "false" - + DATATRACKER_DJANGO_SECRET_KEY: "PDwXboUq!=hPjnrtG2=ge#N$Dwy+wn@uivrugwpic8mxyPfHk" # secret # Set this to point testing / staging at the production statics server until we # sort that out # DATATRACKER_STATIC_URL: "https://static.ietf.org/dt/12.10.0/" - + # DATATRACKER_EMAIL_DEBUG: "true" - + # Outgoing email details # DATATRACKER_EMAIL_HOST: "localhost" # defaults to localhost # DATATRACKER_EMAIL_PORT: "2025" # defaults to 2025 @@ -51,7 +51,7 @@ stringData: ZQcWtWb3ZtRjZ5RTdJSi9kdjRGY1YrUUtDdEovck9TOGUzNlk4WkFFVll1dWtoZXM weVoxdz09Ci0tLS0tRU5EIFBVQkxJQyBLRVktLS0tLQo= # secret - value here is the default from settings.py (i.e., not actually secret) - DATATRACKER_API_PRIVATE_KEY_PEM_B64: |- + DATATRACKER_API_PRIVATE_KEY_PEM_B64: |- Ci0tLS0tQkVHSU4gUFJJVkFURSBLRVktLS0tLQpNSUdIQWdFQU1CTUdCeXFHU000O UFnRUdDQ3FHU000OUF3RUhCRzB3YXdJQkFRUWdvSTZMSmtvcEtxOFhySGk5ClFxR1 F2RTRBODNURllqcUx6KzhnVUxZZWNzcWhSQU5DQUFTcFdpT3hxaDhNbEp5NHdrMzY @@ -71,7 +71,7 @@ stringData: # Only one of these may be set # DATATRACKER_APP_API_TOKENS_JSON_B64: "e30K" # secret - # DATATRACKER_APP_API_TOKENS_JSON: "{}" # secret + # DATATRACKER_APP_API_TOKENS_JSON: "{}" # secret # use this to override default - one entry per line # DATATRACKER_CSRF_TRUSTED_ORIGINS: |- diff --git a/k8s/settings_local.py b/k8s/settings_local.py index e5f0a9af0f3..e12599e18e9 100644 --- a/k8s/settings_local.py +++ b/k8s/settings_local.py @@ -50,30 +50,6 @@ def _multiline_to_list(s): else: raise RuntimeError("DATATRACKER_YOUTUBE_API_KEY must be set") -_GITHUB_BACKUP_API_KEY = os.environ.get("DATATRACKER_GITHUB_BACKUP_API_KEY", None) -if _GITHUB_BACKUP_API_KEY is not None: - GITHUB_BACKUP_API_KEY = _GITHUB_BACKUP_API_KEY -else: - raise RuntimeError("DATATRACKER_GITHUB_BACKUP_API_KEY must be set") - -_API_KEY_TYPE = os.environ.get("DATATRACKER_API_KEY_TYPE", None) -if _API_KEY_TYPE is not None: - API_KEY_TYPE = _API_KEY_TYPE -else: - raise RuntimeError("DATATRACKER_API_KEY_TYPE must be set") - -_API_PUBLIC_KEY_PEM_B64 = os.environ.get("DATATRACKER_API_PUBLIC_KEY_PEM_B64", None) -if _API_PUBLIC_KEY_PEM_B64 is not None: - API_PUBLIC_KEY_PEM = b64decode(_API_PUBLIC_KEY_PEM_B64) -else: - raise RuntimeError("DATATRACKER_API_PUBLIC_KEY_PEM_B64 must be set") - -_API_PRIVATE_KEY_PEM_B64 = os.environ.get("DATATRACKER_API_PRIVATE_KEY_PEM_B64", None) -if _API_PRIVATE_KEY_PEM_B64 is not None: - API_PRIVATE_KEY_PEM = b64decode(_API_PRIVATE_KEY_PEM_B64) -else: - raise RuntimeError("DATATRACKER_API_PRIVATE_KEY_PEM_B64 must be set") - _RED_PRECOMPUTER_TRIGGER_RETRY_DELAY = os.environ.get( "DATATRACKER_RED_PRECOMPUTER_TRIGGER_RETRY_DELAY", None ) diff --git a/playwright/package-lock.json b/playwright/package-lock.json index 3f354187450..143838b6c40 100644 --- a/playwright/package-lock.json +++ b/playwright/package-lock.json @@ -15,7 +15,7 @@ }, "devDependencies": { "@playwright/test": "1.62.1", - "npm-check-updates": "23.0.1" + "npm-check-updates": "23.0.2" } }, "node_modules/@faker-js/faker": { @@ -84,9 +84,9 @@ "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" }, "node_modules/npm-check-updates": { - "version": "23.0.1", - "resolved": "https://registry.npmjs.org/npm-check-updates/-/npm-check-updates-23.0.1.tgz", - "integrity": "sha512-e4hu3Rq4waj7SnhzvqAc5UG557mfP5e3JIFFQd7Fg3RiRkJl8yR90NFoZ1GSWUT3S/QacJndJb+7dLQBPeH+iA==", + "version": "23.0.2", + "resolved": "https://registry.npmjs.org/npm-check-updates/-/npm-check-updates-23.0.2.tgz", + "integrity": "sha512-t5tv+d4sP+WbSwcDAFBwDxSUJRomc+TJoURPu7q5B/19toUsR/7eshRxBdWdE7iYw80iE4O4D/7OvCvIcaG8ug==", "dev": true, "bin": { "ncu": "build/cli.js", @@ -180,9 +180,9 @@ "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" }, "npm-check-updates": { - "version": "23.0.1", - "resolved": "https://registry.npmjs.org/npm-check-updates/-/npm-check-updates-23.0.1.tgz", - "integrity": "sha512-e4hu3Rq4waj7SnhzvqAc5UG557mfP5e3JIFFQd7Fg3RiRkJl8yR90NFoZ1GSWUT3S/QacJndJb+7dLQBPeH+iA==", + "version": "23.0.2", + "resolved": "https://registry.npmjs.org/npm-check-updates/-/npm-check-updates-23.0.2.tgz", + "integrity": "sha512-t5tv+d4sP+WbSwcDAFBwDxSUJRomc+TJoURPu7q5B/19toUsR/7eshRxBdWdE7iYw80iE4O4D/7OvCvIcaG8ug==", "dev": true }, "playwright": { diff --git a/playwright/package.json b/playwright/package.json index d33dca96930..3c9db331095 100644 --- a/playwright/package.json +++ b/playwright/package.json @@ -17,6 +17,6 @@ }, "devDependencies": { "@playwright/test": "1.62.1", - "npm-check-updates": "23.0.1" + "npm-check-updates": "23.0.2" } }