5 files changed
@@ -216,32 +216,24 @@ class Meta: | |||
| 216 | 216 | read_only_fields = ["id", "name"] | |
| 217 | 217 | ||
| 218 | 218 | ||
| 219 | - class EditableRfcSerializer(serializers.ModelSerializer): | ||
| 220 | - # Would be nice to reconcile this with ietf.doc.serializers.RfcSerializer. | ||
| 221 | - # The purposes of that serializer (representing data for Red) and this one | ||
| 222 | - # (accepting updates from Purple) are different enough that separate formats | ||
| 223 | - # may be needed, but if not it'd be nice to have a single RfcSerializer that | ||
| 224 | - # can serve both. | ||
| 225 | - # | ||
| 226 | - # For now, only handles authors | ||
| 227 | - authors = RfcAuthorSerializer(many=True, min_length=1, source="rfcauthor_set") | ||
| 219 | + def _update_authors(rfc, authors_data): | ||
| 220 | + # Construct unsaved instances from validated author data | ||
| 221 | + new_authors = [RfcAuthor(**authdata) for authdata in authors_data] | ||
| 222 | + # Update the RFC with the new author set | ||
| 223 | + with transaction.atomic(): | ||
| 224 | + change_events = update_rfcauthors(rfc, new_authors) | ||
| 225 | + for event in change_events: | ||
| 226 | + event.save() | ||
| 227 | + return change_events | ||
| 228 | 228 | ||
| 229 | - class Meta: | ||
| 230 | - model = Document | ||
| 231 | - fields = ["id", "authors"] | ||
| 232 | 229 | ||
| 233 | - def update(self, instance, validated_data): | ||
| 234 | - assert isinstance(instance, Document) | ||
| 235 | - authors_data = validated_data.pop("rfcauthor_set", None) | ||
| 236 | - if authors_data is not None: | ||
| 237 | - # Construct unsaved instances from validated author data | ||
| 238 | - new_authors = [RfcAuthor(**ad) for ad in authors_data] | ||
| 239 | - # Update the RFC with the new author set | ||
| 240 | - with transaction.atomic(): | ||
| 241 | - change_events = update_rfcauthors(instance, new_authors) | ||
| 242 | - for event in change_events: | ||
| 243 | - event.save() | ||
| 244 | - return instance | ||
| 230 | + class SubseriesNameField(serializers.RegexField): | ||
| 231 | + | ||
| 232 | + def __init__(self, **kwargs): | ||
| 233 | + # pattern: no leading 0, finite length (arbitrarily set to 5 digits) | ||
| 234 | + regex = r"^(bcp|std|fyi)[1-9][0-9]{0,4}$" | ||
| 235 | + super().__init__(regex, **kwargs) | ||
| 236 | + | ||
| 245 | 237 | ||
| 246 | 238 | ||
| 247 | 239 | class RfcPubSerializer(serializers.ModelSerializer): | |
@@ -283,13 +275,7 @@ class RfcPubSerializer(serializers.ModelSerializer): | |||
| 283 | 275 | slug_field="rfc_number", | |
| 284 | 276 | queryset=Document.objects.filter(type_id="rfc"), | |
| 285 | 277 | ) | |
| 286 | - subseries = serializers.ListField( | ||
| 287 | - child=serializers.RegexField( | ||
| 288 | - required=False, | ||
| 289 | - # pattern: no leading 0, finite length (arbitrarily set to 5 digits) | ||
| 290 | - regex=r"^(bcp|std|fyi)[1-9][0-9]{0,4}$", | ||
| 291 | - ) | ||
| 292 | - ) | ||
| 278 | + subseries = serializers.ListField(child=SubseriesNameField(required=False)) | ||
| 293 | 279 | # N.b., authors is _not_ a field on Document! | |
| 294 | 280 | authors = RfcAuthorSerializer(many=True) | |
| 295 | 281 | ||
@@ -327,6 +313,9 @@ def validate(self, data): | |||
| 327 | 313 | ) | |
| 328 | 314 | return data | |
| 329 | 315 | ||
| 316 | + def update(self, instance, validated_data): | ||
| 317 | + raise RuntimeError("Cannot update with this serializer") | ||
| 318 | + | ||
| 330 | 319 | def create(self, validated_data): | |
| 331 | 320 | """Publish an RFC""" | |
| 332 | 321 | published = validated_data.pop("published") | |
@@ -515,6 +504,182 @@ def _create_rfc(self, validated_data): | |||
| 515 | 504 | return rfc | |
| 516 | 505 | ||
| 517 | 506 | ||
| 507 | + class EditableRfcSerializer(serializers.ModelSerializer): | ||
| 508 | + # Would be nice to reconcile this with ietf.doc.serializers.RfcSerializer. | ||
| 509 | + # The purposes of that serializer (representing data for Red) and this one | ||
| 510 | + # (accepting updates from Purple) are different enough that separate formats | ||
| 511 | + # may be needed, but if not it'd be nice to have a single RfcSerializer that | ||
| 512 | + # can serve both. | ||
| 513 | + # | ||
| 514 | + # Should also consider whether this and RfcPubSerializer should merge. | ||
| 515 | + # | ||
| 516 | + # Treats published and subseries fields as write-only. This isn't quite correct, | ||
| 517 | + # but makes it easier and we don't currently use the serialized value except for | ||
| 518 | + # debugging. | ||
| 519 | + published = serializers.DateTimeField( | ||
| 520 | + default_timezone=datetime.timezone.utc, | ||
| 521 | + write_only=True, | ||
| 522 | + ) | ||
| 523 | + authors = RfcAuthorSerializer(many=True, min_length=1, source="rfcauthor_set") | ||
| 524 | + subseries = serializers.ListField( | ||
| 525 | + child=SubseriesNameField(required=False), | ||
| 526 | + write_only=True, | ||
| 527 | + ) | ||
| 528 | + | ||
| 529 | + class Meta: | ||
| 530 | + model = Document | ||
| 531 | + fields = [ | ||
| 532 | + "published", | ||
| 533 | + "title", | ||
| 534 | + "authors", | ||
| 535 | + "stream", | ||
| 536 | + "abstract", | ||
| 537 | + "pages", | ||
| 538 | + "std_level", | ||
| 539 | + "subseries", | ||
| 540 | + ] | ||
| 541 | + | ||
| 542 | + def create(self, validated_data): | ||
| 543 | + raise RuntimeError("Cannot create with this serializer") | ||
| 544 | + | ||
| 545 | + def update(self, instance, validated_data): | ||
| 546 | + assert isinstance(instance, Document) | ||
| 547 | + assert instance.type_id == "rfc" | ||
| 548 | + rfc = instance # get better name | ||
| 549 | + | ||
| 550 | + system_person = Person.objects.get(name="(System)") | ||
| 551 | + | ||
| 552 | + # Remove data that needs special handling. Use a singleton object to detect | ||
| 553 | + # missing values in case we ever support a value that needs None as an option. | ||
| 554 | + omitted = object() | ||
| 555 | + published = validated_data.pop("published", omitted) | ||
| 556 | + subseries = validated_data.pop("subseries", omitted) | ||
| 557 | + authors_data = validated_data.pop("rfcauthor_set", omitted) | ||
| 558 | + | ||
| 559 | + # Transaction to clean up if something fails | ||
| 560 | + with transaction.atomic(): | ||
| 561 | + # update the rfc Document itself | ||
| 562 | + rfc_changes = [] | ||
| 563 | + rfc_events = [] | ||
| 564 | + | ||
| 565 | + for attr, new_value in validated_data.items(): | ||
| 566 | + old_value = getattr(rfc, attr) | ||
| 567 | + if new_value != old_value: | ||
| 568 | + rfc_changes.append( | ||
| 569 | + f"changed {attr} to '{new_value}' from '{old_value}'" | ||
| 570 | + ) | ||
| 571 | + setattr(rfc, attr, new_value) | ||
| 572 | + if len(rfc_changes) > 0: | ||
| 573 | + rfc_change_summary = f"{', '.join(rfc_changes)}" | ||
| 574 | + rfc_events.append( | ||
| 575 | + DocEvent.objects.create( | ||
| 576 | + doc=rfc, | ||
| 577 | + rev=rfc.rev, | ||
| 578 | + by=system_person, | ||
| 579 | + type="sync_from_rfc_editor", | ||
| 580 | + desc=f"Changed metadata: {rfc_change_summary}", | ||
| 581 | + ) | ||
| 582 | + ) | ||
| 583 | + if authors_data is not omitted: | ||
| 584 | + rfc_events.extend(_update_authors(instance, authors_data)) | ||
| 585 | + | ||
| 586 | + if published is not omitted: | ||
| 587 | + published_event = rfc.latest_event(type="published_rfc") | ||
| 588 | + if published_event is None: | ||
| 589 | + # unexpected, but possible in theory | ||
| 590 | + rfc_events.append( | ||
| 591 | + DocEvent.objects.create( | ||
| 592 | + doc=rfc, | ||
| 593 | + rev=rfc.rev, | ||
| 594 | + type="published_rfc", | ||
| 595 | + time=published, | ||
| 596 | + by=system_person, | ||
| 597 | + desc="RFC published", | ||
| 598 | + ) | ||
| 599 | + ) | ||
| 600 | + rfc_events.append( | ||
| 601 | + DocEvent.objects.create( | ||
| 602 | + doc=rfc, | ||
| 603 | + rev=rfc.rev, | ||
| 604 | + type="sync_from_rfc_editor", | ||
| 605 | + by=system_person, | ||
| 606 | + desc=( | ||
| 607 | + f"Set publication timestamp to {published.isoformat()}" | ||
| 608 | + ), | ||
| 609 | + ) | ||
| 610 | + ) | ||
| 611 | + else: | ||
| 612 | + original_pub_time = published_event.time | ||
| 613 | + if published != original_pub_time: | ||
| 614 | + published_event.time = published | ||
| 615 | + published_event.save() | ||
| 616 | + rfc_events.append( | ||
| 617 | + DocEvent.objects.create( | ||
| 618 | + doc=rfc, | ||
| 619 | + rev=rfc.rev, | ||
| 620 | + type="sync_from_rfc_editor", | ||
| 621 | + by=system_person, | ||
| 622 | + desc=( | ||
| 623 | + f"Changed publication time to " | ||
| 624 | + f"{published.isoformat()} from " | ||
| 625 | + f"{original_pub_time.isoformat()}" | ||
| 626 | + ) | ||
| 627 | + ) | ||
| 628 | + ) | ||
| 629 | + | ||
| 630 | + # update subseries relations | ||
| 631 | + if subseries is not omitted: | ||
| 632 | + for subseries_doc_name in subseries: | ||
| 633 | + ss_slug = subseries_doc_name[:3] | ||
| 634 | + subseries_doc, ss_doc_created = Document.objects.get_or_create( | ||
| 635 | + type_id=ss_slug, name=subseries_doc_name | ||
| 636 | + ) | ||
| 637 | + if ss_doc_created: | ||
| 638 | + subseries_doc.docevent_set.create( | ||
| 639 | + type=f"{ss_slug}_doc_created", | ||
| 640 | + by=system_person, | ||
| 641 | + desc=f"Created {subseries_doc_name} via update of {rfc.name}", | ||
| 642 | + ) | ||
| 643 | + _, ss_rel_created = subseries_doc.relateddocument_set.get_or_create( | ||
| 644 | + relationship_id="contains", target=rfc | ||
| 645 | + ) | ||
| 646 | + if ss_rel_created: | ||
| 647 | + subseries_doc.docevent_set.create( | ||
| 648 | + type="sync_from_rfc_editor", | ||
| 649 | + by=system_person, | ||
| 650 | + desc=f"Added {rfc.name} to {subseries_doc.name}", | ||
| 651 | + ) | ||
| 652 | + rfc_events.append( | ||
| 653 | + rfc.docevent_set.create( | ||
| 654 | + type="sync_from_rfc_editor", | ||
| 655 | + by=system_person, | ||
| 656 | + desc=f"Added {rfc.name} to {subseries_doc.name}", | ||
| 657 | + ) | ||
| 658 | + ) | ||
| 659 | + # Delete subseries relations that are no longer current | ||
| 660 | + stale_subseries_relations = rfc.relations_that("contains").exclude( | ||
| 661 | + source__name__in=subseries | ||
| 662 | + ) | ||
| 663 | + for stale_relation in stale_subseries_relations: | ||
| 664 | + stale_subseries_doc = stale_relation.source | ||
| 665 | + rfc_events.append( | ||
| 666 | + rfc.docevent_set.create( | ||
| 667 | + type="sync_from_rfc_editor", | ||
| 668 | + by=system_person, | ||
| 669 | + desc=f"Removed {rfc.name} from {stale_subseries_doc.name}", | ||
| 670 | + ) | ||
| 671 | + ) | ||
| 672 | + stale_subseries_doc.docevent_set.create( | ||
| 673 | + type="sync_from_rfc_editor", | ||
| 674 | + by=system_person, | ||
| 675 | + desc=f"Removed {rfc.name} from {stale_subseries_doc.name}", | ||
| 676 | + ) | ||
| 677 | + stale_subseries_relations.delete() | ||
| 678 | + if len(rfc_events) > 0: | ||
| 679 | + rfc.save_with_history(rfc_events) | ||
| 680 | + return rfc | ||
| 681 | + | ||
| 682 | + | ||
| 518 | 683 | class RfcFileSerializer(serializers.Serializer): | |
| 519 | 684 | # The structure of this serializer is constrained by what openapi-generator-cli's | |
| 520 | 685 | # python generator can correctly serialize as multipart/form-data. It does not | |
@@ -0,0 +1,139 @@ | |||
| 1 | + # Copyright The IETF Trust 2026, All Rights Reserved | ||
| 2 | + from django.utils import timezone | ||
| 3 | + | ||
| 4 | + from ietf.utils.test_utils import TestCase | ||
| 5 | + from ietf.doc.models import Document | ||
| 6 | + from ietf.doc.factories import WgRfcFactory | ||
| 7 | + from .serializers_rpc import EditableRfcSerializer | ||
| 8 | + | ||
| 9 | + | ||
| 10 | + class EditableRfcSerializerTests(TestCase): | ||
| 11 | + def test_create(self): | ||
| 12 | + serializer = EditableRfcSerializer( | ||
| 13 | + data={ | ||
| 14 | + "published": timezone.now(), | ||
| 15 | + "title": "Yadda yadda yadda", | ||
| 16 | + "authors": [ | ||
| 17 | + { | ||
| 18 | + "titlepage_name": "B. Fett", | ||
| 19 | + "is_editor": False, | ||
| 20 | + "affiliation": "DBA Galactic Empire", | ||
| 21 | + "country": "", | ||
| 22 | + }, | ||
| 23 | + ], | ||
| 24 | + "stream": "ietf", | ||
| 25 | + "abstract": "A long time ago in a galaxy far, far away...", | ||
| 26 | + "pages": 3, | ||
| 27 | + "std_level": "inf", | ||
| 28 | + "subseries": ["fyi999"], | ||
| 29 | + } | ||
| 30 | + ) | ||
| 31 | + self.assertTrue(serializer.is_valid()) | ||
| 32 | + with self.assertRaises(RuntimeError, msg="serializer does not allow create()"): | ||
| 33 | + serializer.save() | ||
| 34 | + | ||
| 35 | + def test_update(self): | ||
| 36 | + rfc = WgRfcFactory(pages=10) | ||
| 37 | + serializer = EditableRfcSerializer( | ||
| 38 | + instance=rfc, | ||
| 39 | + data={ | ||
| 40 | + "published": timezone.now(), | ||
| 41 | + "title": "Yadda yadda yadda", | ||
| 42 | + "authors": [ | ||
| 43 | + { | ||
| 44 | + "titlepage_name": "B. Fett", | ||
| 45 | + "is_editor": False, | ||
| 46 | + "affiliation": "DBA Galactic Empire", | ||
| 47 | + "country": "", | ||
| 48 | + }, | ||
| 49 | + ], | ||
| 50 | + "stream": "ise", | ||
| 51 | + "abstract": "A long time ago in a galaxy far, far away...", | ||
| 52 | + "pages": 3, | ||
| 53 | + "std_level": "inf", | ||
| 54 | + "subseries": ["fyi999"], | ||
| 55 | + }, | ||
| 56 | + ) | ||
| 57 | + self.assertTrue(serializer.is_valid()) | ||
| 58 | + result = serializer.save() | ||
| 59 | + result.refresh_from_db() | ||
| 60 | + self.assertEqual(result.title, "Yadda yadda yadda") | ||
| 61 | + self.assertEqual( | ||
| 62 | + list( | ||
| 63 | + result.rfcauthor_set.values( | ||
| 64 | + "titlepage_name", "is_editor", "affiliation", "country" | ||
| 65 | + ) | ||
| 66 | + ), | ||
| 67 | + [ | ||
| 68 | + { | ||
| 69 | + "titlepage_name": "B. Fett", | ||
| 70 | + "is_editor": False, | ||
| 71 | + "affiliation": "DBA Galactic Empire", | ||
| 72 | + "country": "", | ||
| 73 | + }, | ||
| 74 | + ], | ||
| 75 | + ) | ||
| 76 | + self.assertEqual(result.stream_id, "ise") | ||
| 77 | + self.assertEqual( | ||
| 78 | + result.abstract, "A long time ago in a galaxy far, far away..." | ||
| 79 | + ) | ||
| 80 | + self.assertEqual(result.pages, 3) | ||
| 81 | + self.assertEqual(result.std_level_id, "inf") | ||
| 82 | + self.assertEqual( | ||
| 83 | + result.part_of(), | ||
| 84 | + [Document.objects.get(name="fyi999")], | ||
| 85 | + ) | ||
| 86 | + | ||
| 87 | + def test_partial_update(self): | ||
| 88 | + # We could test other permutations of fields, but authors is a partial update | ||
| 89 | + # we know we are going to use, so verifying that one in particular. | ||
| 90 | + rfc = WgRfcFactory(pages=10, abstract="do or do not", title="padawan") | ||
| 91 | + serializer = EditableRfcSerializer( | ||
| 92 | + partial=True, | ||
| 93 | + instance=rfc, | ||
| 94 | + data={ | ||
| 95 | + "authors": [ | ||
| 96 | + { | ||
| 97 | + "titlepage_name": "B. Fett", | ||
| 98 | + "is_editor": False, | ||
| 99 | + "affiliation": "DBA Galactic Empire", | ||
| 100 | + "country": "", | ||
| 101 | + }, | ||
| 102 | + ], | ||
| 103 | + }, | ||
| 104 | + ) | ||
| 105 | + self.assertTrue(serializer.is_valid()) | ||
| 106 | + result = serializer.save() | ||
| 107 | + result.refresh_from_db() | ||
| 108 | + self.assertEqual(rfc.title, "padawan") | ||
| 109 | + self.assertEqual( | ||
| 110 | + list( | ||
| 111 | + result.rfcauthor_set.values( | ||
| 112 | + "titlepage_name", "is_editor", "affiliation", "country" | ||
| 113 | + ) | ||
| 114 | + ), | ||
| 115 | + [ | ||
| 116 | + { | ||
| 117 | + "titlepage_name": "B. Fett", | ||
| 118 | + "is_editor": False, | ||
| 119 | + "affiliation": "DBA Galactic Empire", | ||
| 120 | + "country": "", | ||
| 121 | + }, | ||
| 122 | + ], | ||
| 123 | + ) | ||
| 124 | + self.assertEqual(result.stream_id, "ietf") | ||
| 125 | + self.assertEqual(result.abstract, "do or do not") | ||
| 126 | + self.assertEqual(result.pages, 10) | ||
| 127 | + self.assertEqual(result.std_level_id, "ps") | ||
| 128 | + self.assertEqual(result.part_of(), []) | ||
| 129 | + | ||
| 130 | + # Test only a field on the Document itself to be sure that it works | ||
| 131 | + serializer = EditableRfcSerializer( | ||
| 132 | + partial=True, | ||
| 133 | + instance=rfc, | ||
| 134 | + data={"title": "jedi master"}, | ||
| 135 | + ) | ||
| 136 | + self.assertTrue(serializer.is_valid()) | ||
| 137 | + result = serializer.save() | ||
| 138 | + result.refresh_from_db() | ||
| 139 | + self.assertEqual(rfc.title, "jedi master") | ||
0 commit comments