diff --git a/ietf/api/serializers_rpc.py b/ietf/api/serializers_rpc.py index 34e2c791c0..d5f5363990 100644 --- a/ietf/api/serializers_rpc.py +++ b/ietf/api/serializers_rpc.py @@ -216,32 +216,24 @@ class Meta: read_only_fields = ["id", "name"] -class EditableRfcSerializer(serializers.ModelSerializer): - # Would be nice to reconcile this with ietf.doc.serializers.RfcSerializer. - # The purposes of that serializer (representing data for Red) and this one - # (accepting updates from Purple) are different enough that separate formats - # may be needed, but if not it'd be nice to have a single RfcSerializer that - # can serve both. - # - # For now, only handles authors - authors = RfcAuthorSerializer(many=True, min_length=1, source="rfcauthor_set") +def _update_authors(rfc, authors_data): + # Construct unsaved instances from validated author data + new_authors = [RfcAuthor(**authdata) for authdata in authors_data] + # Update the RFC with the new author set + with transaction.atomic(): + change_events = update_rfcauthors(rfc, new_authors) + for event in change_events: + event.save() + return change_events - class Meta: - model = Document - fields = ["id", "authors"] - def update(self, instance, validated_data): - assert isinstance(instance, Document) - authors_data = validated_data.pop("rfcauthor_set", None) - if authors_data is not None: - # Construct unsaved instances from validated author data - new_authors = [RfcAuthor(**ad) for ad in authors_data] - # Update the RFC with the new author set - with transaction.atomic(): - change_events = update_rfcauthors(instance, new_authors) - for event in change_events: - event.save() - return instance +class SubseriesNameField(serializers.RegexField): + + def __init__(self, **kwargs): + # pattern: no leading 0, finite length (arbitrarily set to 5 digits) + regex = r"^(bcp|std|fyi)[1-9][0-9]{0,4}$" + super().__init__(regex, **kwargs) + class RfcPubSerializer(serializers.ModelSerializer): @@ -283,13 +275,7 @@ class RfcPubSerializer(serializers.ModelSerializer): slug_field="rfc_number", queryset=Document.objects.filter(type_id="rfc"), ) - subseries = serializers.ListField( - child=serializers.RegexField( - required=False, - # pattern: no leading 0, finite length (arbitrarily set to 5 digits) - regex=r"^(bcp|std|fyi)[1-9][0-9]{0,4}$", - ) - ) + subseries = serializers.ListField(child=SubseriesNameField(required=False)) # N.b., authors is _not_ a field on Document! authors = RfcAuthorSerializer(many=True) @@ -327,6 +313,9 @@ def validate(self, data): ) return data + def update(self, instance, validated_data): + raise RuntimeError("Cannot update with this serializer") + def create(self, validated_data): """Publish an RFC""" published = validated_data.pop("published") @@ -515,6 +504,182 @@ def _create_rfc(self, validated_data): return rfc +class EditableRfcSerializer(serializers.ModelSerializer): + # Would be nice to reconcile this with ietf.doc.serializers.RfcSerializer. + # The purposes of that serializer (representing data for Red) and this one + # (accepting updates from Purple) are different enough that separate formats + # may be needed, but if not it'd be nice to have a single RfcSerializer that + # can serve both. + # + # Should also consider whether this and RfcPubSerializer should merge. + # + # Treats published and subseries fields as write-only. This isn't quite correct, + # but makes it easier and we don't currently use the serialized value except for + # debugging. + published = serializers.DateTimeField( + default_timezone=datetime.timezone.utc, + write_only=True, + ) + authors = RfcAuthorSerializer(many=True, min_length=1, source="rfcauthor_set") + subseries = serializers.ListField( + child=SubseriesNameField(required=False), + write_only=True, + ) + + class Meta: + model = Document + fields = [ + "published", + "title", + "authors", + "stream", + "abstract", + "pages", + "std_level", + "subseries", + ] + + def create(self, validated_data): + raise RuntimeError("Cannot create with this serializer") + + def update(self, instance, validated_data): + assert isinstance(instance, Document) + assert instance.type_id == "rfc" + rfc = instance # get better name + + system_person = Person.objects.get(name="(System)") + + # Remove data that needs special handling. Use a singleton object to detect + # missing values in case we ever support a value that needs None as an option. + omitted = object() + published = validated_data.pop("published", omitted) + subseries = validated_data.pop("subseries", omitted) + authors_data = validated_data.pop("rfcauthor_set", omitted) + + # Transaction to clean up if something fails + with transaction.atomic(): + # update the rfc Document itself + rfc_changes = [] + rfc_events = [] + + for attr, new_value in validated_data.items(): + old_value = getattr(rfc, attr) + if new_value != old_value: + rfc_changes.append( + f"changed {attr} to '{new_value}' from '{old_value}'" + ) + setattr(rfc, attr, new_value) + if len(rfc_changes) > 0: + rfc_change_summary = f"{', '.join(rfc_changes)}" + rfc_events.append( + DocEvent.objects.create( + doc=rfc, + rev=rfc.rev, + by=system_person, + type="sync_from_rfc_editor", + desc=f"Changed metadata: {rfc_change_summary}", + ) + ) + if authors_data is not omitted: + rfc_events.extend(_update_authors(instance, authors_data)) + + if published is not omitted: + published_event = rfc.latest_event(type="published_rfc") + if published_event is None: + # unexpected, but possible in theory + rfc_events.append( + DocEvent.objects.create( + doc=rfc, + rev=rfc.rev, + type="published_rfc", + time=published, + by=system_person, + desc="RFC published", + ) + ) + rfc_events.append( + DocEvent.objects.create( + doc=rfc, + rev=rfc.rev, + type="sync_from_rfc_editor", + by=system_person, + desc=( + f"Set publication timestamp to {published.isoformat()}" + ), + ) + ) + else: + original_pub_time = published_event.time + if published != original_pub_time: + published_event.time = published + published_event.save() + rfc_events.append( + DocEvent.objects.create( + doc=rfc, + rev=rfc.rev, + type="sync_from_rfc_editor", + by=system_person, + desc=( + f"Changed publication time to " + f"{published.isoformat()} from " + f"{original_pub_time.isoformat()}" + ) + ) + ) + + # update subseries relations + if subseries is not omitted: + for subseries_doc_name in subseries: + ss_slug = subseries_doc_name[:3] + subseries_doc, ss_doc_created = Document.objects.get_or_create( + type_id=ss_slug, name=subseries_doc_name + ) + if ss_doc_created: + subseries_doc.docevent_set.create( + type=f"{ss_slug}_doc_created", + by=system_person, + desc=f"Created {subseries_doc_name} via update of {rfc.name}", + ) + _, ss_rel_created = subseries_doc.relateddocument_set.get_or_create( + relationship_id="contains", target=rfc + ) + if ss_rel_created: + subseries_doc.docevent_set.create( + type="sync_from_rfc_editor", + by=system_person, + desc=f"Added {rfc.name} to {subseries_doc.name}", + ) + rfc_events.append( + rfc.docevent_set.create( + type="sync_from_rfc_editor", + by=system_person, + desc=f"Added {rfc.name} to {subseries_doc.name}", + ) + ) + # Delete subseries relations that are no longer current + stale_subseries_relations = rfc.relations_that("contains").exclude( + source__name__in=subseries + ) + for stale_relation in stale_subseries_relations: + stale_subseries_doc = stale_relation.source + rfc_events.append( + rfc.docevent_set.create( + type="sync_from_rfc_editor", + by=system_person, + desc=f"Removed {rfc.name} from {stale_subseries_doc.name}", + ) + ) + stale_subseries_doc.docevent_set.create( + type="sync_from_rfc_editor", + by=system_person, + desc=f"Removed {rfc.name} from {stale_subseries_doc.name}", + ) + stale_subseries_relations.delete() + if len(rfc_events) > 0: + rfc.save_with_history(rfc_events) + return rfc + + class RfcFileSerializer(serializers.Serializer): # The structure of this serializer is constrained by what openapi-generator-cli's # python generator can correctly serialize as multipart/form-data. It does not diff --git a/ietf/api/tests_serializers_rpc.py b/ietf/api/tests_serializers_rpc.py new file mode 100644 index 0000000000..1babb4c30f --- /dev/null +++ b/ietf/api/tests_serializers_rpc.py @@ -0,0 +1,139 @@ +# Copyright The IETF Trust 2026, All Rights Reserved +from django.utils import timezone + +from ietf.utils.test_utils import TestCase +from ietf.doc.models import Document +from ietf.doc.factories import WgRfcFactory +from .serializers_rpc import EditableRfcSerializer + + +class EditableRfcSerializerTests(TestCase): + def test_create(self): + serializer = EditableRfcSerializer( + data={ + "published": timezone.now(), + "title": "Yadda yadda yadda", + "authors": [ + { + "titlepage_name": "B. Fett", + "is_editor": False, + "affiliation": "DBA Galactic Empire", + "country": "", + }, + ], + "stream": "ietf", + "abstract": "A long time ago in a galaxy far, far away...", + "pages": 3, + "std_level": "inf", + "subseries": ["fyi999"], + } + ) + self.assertTrue(serializer.is_valid()) + with self.assertRaises(RuntimeError, msg="serializer does not allow create()"): + serializer.save() + + def test_update(self): + rfc = WgRfcFactory(pages=10) + serializer = EditableRfcSerializer( + instance=rfc, + data={ + "published": timezone.now(), + "title": "Yadda yadda yadda", + "authors": [ + { + "titlepage_name": "B. Fett", + "is_editor": False, + "affiliation": "DBA Galactic Empire", + "country": "", + }, + ], + "stream": "ise", + "abstract": "A long time ago in a galaxy far, far away...", + "pages": 3, + "std_level": "inf", + "subseries": ["fyi999"], + }, + ) + self.assertTrue(serializer.is_valid()) + result = serializer.save() + result.refresh_from_db() + self.assertEqual(result.title, "Yadda yadda yadda") + self.assertEqual( + list( + result.rfcauthor_set.values( + "titlepage_name", "is_editor", "affiliation", "country" + ) + ), + [ + { + "titlepage_name": "B. Fett", + "is_editor": False, + "affiliation": "DBA Galactic Empire", + "country": "", + }, + ], + ) + self.assertEqual(result.stream_id, "ise") + self.assertEqual( + result.abstract, "A long time ago in a galaxy far, far away..." + ) + self.assertEqual(result.pages, 3) + self.assertEqual(result.std_level_id, "inf") + self.assertEqual( + result.part_of(), + [Document.objects.get(name="fyi999")], + ) + + def test_partial_update(self): + # We could test other permutations of fields, but authors is a partial update + # we know we are going to use, so verifying that one in particular. + rfc = WgRfcFactory(pages=10, abstract="do or do not", title="padawan") + serializer = EditableRfcSerializer( + partial=True, + instance=rfc, + data={ + "authors": [ + { + "titlepage_name": "B. Fett", + "is_editor": False, + "affiliation": "DBA Galactic Empire", + "country": "", + }, + ], + }, + ) + self.assertTrue(serializer.is_valid()) + result = serializer.save() + result.refresh_from_db() + self.assertEqual(rfc.title, "padawan") + self.assertEqual( + list( + result.rfcauthor_set.values( + "titlepage_name", "is_editor", "affiliation", "country" + ) + ), + [ + { + "titlepage_name": "B. Fett", + "is_editor": False, + "affiliation": "DBA Galactic Empire", + "country": "", + }, + ], + ) + self.assertEqual(result.stream_id, "ietf") + self.assertEqual(result.abstract, "do or do not") + self.assertEqual(result.pages, 10) + self.assertEqual(result.std_level_id, "ps") + self.assertEqual(result.part_of(), []) + + # Test only a field on the Document itself to be sure that it works + serializer = EditableRfcSerializer( + partial=True, + instance=rfc, + data={"title": "jedi master"}, + ) + self.assertTrue(serializer.is_valid()) + result = serializer.save() + result.refresh_from_db() + self.assertEqual(rfc.title, "jedi master") diff --git a/ietf/api/views_rpc.py b/ietf/api/views_rpc.py index 2bf16480f2..0819c04215 100644 --- a/ietf/api/views_rpc.py +++ b/ietf/api/views_rpc.py @@ -35,7 +35,7 @@ NotificationAckSerializer, RfcPubSerializer, RfcFileSerializer, EditableRfcSerializer, ) -from ietf.doc.models import Document, DocHistory, RfcAuthor +from ietf.doc.models import Document, DocHistory, RfcAuthor, DocEvent from ietf.doc.serializers import RfcAuthorSerializer from ietf.doc.storage_utils import remove_from_storage, store_file, exists_in_storage from ietf.person.models import Email, Person @@ -278,6 +278,16 @@ class RfcViewSet(mixins.UpdateModelMixin, viewsets.GenericViewSet): lookup_field = "rfc_number" serializer_class = EditableRfcSerializer + def perform_update(self, serializer): + DocEvent.objects.create( + doc=serializer.instance, + rev=serializer.instance.rev, + by=Person.objects.get(name="(System)"), + type="sync_from_rfc_editor", + desc="Metadata update from RFC Editor", + ) + super().perform_update(serializer) + @action(detail=False, serializer_class=OriginalStreamSerializer) def rfc_original_stream(self, request): rfcs = self.get_queryset().annotate( diff --git a/ietf/doc/serializers.py b/ietf/doc/serializers.py index 36076c30be..a7ea640be8 100644 --- a/ietf/doc/serializers.py +++ b/ietf/doc/serializers.py @@ -27,6 +27,7 @@ class RfcAuthorSerializer(serializers.ModelSerializer): source="person.get_absolute_url", required=False, help_text="URL for person link (relative to datatracker base URL)", + read_only=True, ) class Meta: diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 42fab7d472..396b3fcfa4 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -740,14 +740,26 @@ def _rfcauthor_from_documentauthor(docauthor: DocumentAuthor) -> RfcAuthor: new_author.document = rfc new_author.order = order + 1 new_author.save() - changes.append(f'Added "{new_author.titlepage_name}" as author') + if new_author.person_id is not None: + person_desc = f"Person {new_author.person_id}" + else: + person_desc = "no Person linked" + changes.append( + f'Added "{new_author.titlepage_name}" ({person_desc}) as author' + ) # Any authors left in original_authors are no longer in the list, so remove them for removed_author in original_authors: # Skip actual removal of old authors if we are converting from the # DocumentAuthor models - the original_authors were just stand-ins anyway. if not converting_from_docauthors: removed_author.delete() - changes.append(f'Removed "{removed_author.titlepage_name}" as author') + if removed_author.person_id is not None: + person_desc = f"Person {removed_author.person_id}" + else: + person_desc = "no Person linked" + changes.append( + f'Removed "{removed_author.titlepage_name}" ({person_desc}) as author' + ) # Create DocEvents, but leave it up to caller to save if by is None: by = Person.objects.get(name="(System)")