diff --git a/ietf/api/serializers_rpc.py b/ietf/api/serializers_rpc.py index 98f8693514..0c6ff4c23a 100644 --- a/ietf/api/serializers_rpc.py +++ b/ietf/api/serializers_rpc.py @@ -10,9 +10,21 @@ from rest_framework import serializers from ietf.doc.expire import move_draft_files_to_archive -from ietf.doc.models import DocumentAuthor, Document, RfcAuthor, RelatedDocument, State, \ - DocEvent -from ietf.doc.utils import default_consensus, prettify_std_name, update_action_holders +from ietf.doc.models import ( + DocumentAuthor, + Document, + RelatedDocument, + State, + DocEvent, + RfcAuthor, +) +from ietf.doc.serializers import RfcAuthorSerializer +from ietf.doc.utils import ( + default_consensus, + prettify_std_name, + update_action_holders, + update_rfcauthors, +) from ietf.group.models import Group from ietf.name.models import StreamName, StdLevelName, FormalLanguageName from ietf.person.models import Person @@ -201,22 +213,32 @@ class Meta: read_only_fields = ["id", "name"] -class AuthorSerializer(serializers.ModelSerializer): - """Serialize an RfcAuthor record +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") - todo fix naming confusion with ietf.doc.serializers.RfcAuthorSerializer - """ class Meta: - model = RfcAuthor - fields = [ - "id", - "titlepage_name", - "is_editor", - "person", - "email", - "affiliation", - "country", - ] + 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 RfcPubSerializer(serializers.ModelSerializer): @@ -273,7 +295,7 @@ class RfcPubSerializer(serializers.ModelSerializer): regex=r"^(bcp|std|fyi)[1-9][0-9]{0,4}$", ) ) - authors = AuthorSerializer(many=True) + authors = RfcAuthorSerializer(many=True) class Meta: model = Document diff --git a/ietf/api/views_rpc.py b/ietf/api/views_rpc.py index 7e5feed6e6..bec2f22768 100644 --- a/ietf/api/views_rpc.py +++ b/ietf/api/views_rpc.py @@ -22,7 +22,6 @@ from rest_framework.pagination import LimitOffsetPagination from ietf.api.serializers_rpc import ( - AuthorSerializer, PersonSerializer, FullDraftSerializer, DraftSerializer, @@ -33,8 +32,10 @@ RfcWithAuthorsSerializer, DraftWithAuthorsSerializer, NotificationAckSerializer, RfcPubSerializer, RfcFileSerializer, + EditableRfcSerializer, ) -from ietf.doc.models import Document, DocHistory, RfcAuthor +from ietf.doc.models import Document, DocHistory, RfcAuthor, EditedRfcAuthorsDocEvent +from ietf.doc.serializers import RfcAuthorSerializer from ietf.person.models import Email, Person @@ -269,9 +270,11 @@ def bulk_authors(self, request): responses=OriginalStreamSerializer(many=True), ) ) -class RfcViewSet(viewsets.GenericViewSet): +class RfcViewSet(mixins.UpdateModelMixin, viewsets.GenericViewSet): queryset = Document.objects.filter(type_id="rfc") api_key_endpoint = "ietf.api.views_rpc" + lookup_field = "rfc_number" + serializer_class = EditableRfcSerializer @action(detail=False, serializer_class=OriginalStreamSerializer) def rfc_original_stream(self, request): @@ -320,7 +323,7 @@ def post(self, request): return Response(DraftSerializer(docs, many=True).data) -class RfcAuthorViewSet(viewsets.ModelViewSet): +class RfcAuthorViewSet(viewsets.ReadOnlyModelViewSet): """ViewSet for RfcAuthor model Router needs to provide rfc_number as a kwarg @@ -328,7 +331,7 @@ class RfcAuthorViewSet(viewsets.ModelViewSet): api_key_endpoint = "ietf.api.views_rpc" queryset = RfcAuthor.objects.all() - serializer_class = AuthorSerializer + serializer_class = RfcAuthorSerializer lookup_url_kwarg = "author_id" rfc_number_param = "rfc_number" @@ -342,22 +345,6 @@ def get_queryset(self): ) ) - def perform_create(self, serializer): - rfc = Document.objects.filter( - type_id="rfc", - rfc_number=self.kwargs[self.rfc_number_param] - ).first() - if rfc is None: - raise NotFound("RFC not found") - # Find the current highest order for this document - from django.db.models import Max - max_order = ( - RfcAuthor.objects.filter(document=rfc) - .aggregate(max_order=Max("order", default=0)) - .get("max_order") - ) - serializer.save(document=rfc, order=max_order + 1) - class RfcPubNotificationView(APIView): api_key_endpoint = "ietf.api.views_rpc" diff --git a/ietf/doc/admin.py b/ietf/doc/admin.py index 03558a6693..d36f1e55c7 100644 --- a/ietf/doc/admin.py +++ b/ietf/doc/admin.py @@ -13,7 +13,8 @@ TelechatDocEvent, BallotPositionDocEvent, ReviewRequestDocEvent, InitialReviewDocEvent, AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL, ReviewAssignmentDocEvent, IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder, - BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor ) + BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor, + EditedRfcAuthorsDocEvent) from ietf.utils.validators import validate_external_resource_value @@ -173,6 +174,7 @@ def short_desc(self, obj): admin.site.register(TelechatDocEvent, DocEventAdmin) admin.site.register(InitialReviewDocEvent, DocEventAdmin) admin.site.register(EditedAuthorsDocEvent, DocEventAdmin) +admin.site.register(EditedRfcAuthorsDocEvent, DocEventAdmin) admin.site.register(IanaExpertDocEvent, DocEventAdmin) class BallotPositionDocEventAdmin(DocEventAdmin): diff --git a/ietf/doc/migrations/0029_editedrfcauthorsdocevent.py b/ietf/doc/migrations/0029_editedrfcauthorsdocevent.py new file mode 100644 index 0000000000..60837c5cb2 --- /dev/null +++ b/ietf/doc/migrations/0029_editedrfcauthorsdocevent.py @@ -0,0 +1,30 @@ +# Copyright The IETF Trust 2025, All Rights Reserved + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("doc", "0028_rfcauthor"), + ] + + operations = [ + migrations.CreateModel( + name="EditedRfcAuthorsDocEvent", + fields=[ + ( + "docevent_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="doc.docevent", + ), + ), + ], + bases=("doc.docevent",), + ), + ] diff --git a/ietf/doc/models.py b/ietf/doc/models.py index a9530f134d..8064ca477e 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -1693,6 +1693,11 @@ class EditedAuthorsDocEvent(DocEvent): """ basis = models.CharField(help_text="What is the source or reasoning for the changes to the author list",max_length=255) + +class EditedRfcAuthorsDocEvent(DocEvent): + """Change to the RfcAuthor list for a document""" + + class BofreqEditorDocEvent(DocEvent): """ Capture the proponents of a BOF Request.""" editors = models.ManyToManyField('person.Person', blank=True) diff --git a/ietf/doc/resources.py b/ietf/doc/resources.py index 03faf1a84b..556465a522 100644 --- a/ietf/doc/resources.py +++ b/ietf/doc/resources.py @@ -17,8 +17,9 @@ InitialReviewDocEvent, DocHistoryAuthor, BallotDocEvent, RelatedDocument, RelatedDocHistory, BallotPositionDocEvent, AddedMessageEvent, SubmissionDocEvent, ReviewRequestDocEvent, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL, - IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder, - BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor) + IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder, + BofreqEditorDocEvent, BofreqResponsibleDocEvent, StoredObject, RfcAuthor, + EditedRfcAuthorsDocEvent) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -650,6 +651,31 @@ class Meta: api.doc.register(EditedAuthorsDocEventResource()) + +from ietf.person.resources import PersonResource +class EditedRfcAuthorsDocEventResource(ModelResource): + by = ToOneField(PersonResource, 'by') + doc = ToOneField(DocumentResource, 'doc') + docevent_ptr = ToOneField(DocEventResource, 'docevent_ptr') + class Meta: + queryset = EditedRfcAuthorsDocEvent.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'editedrfcauthorsdocevent' + ordering = ['id', ] + filtering = { + "id": ALL, + "time": ALL, + "type": ALL, + "rev": ALL, + "desc": ALL, + "by": ALL_WITH_RELATIONS, + "doc": ALL_WITH_RELATIONS, + "docevent_ptr": ALL_WITH_RELATIONS, + } +api.doc.register(EditedRfcAuthorsDocEventResource()) + + from ietf.name.resources import DocUrlTagNameResource class DocumentURLResource(ModelResource): doc = ToOneField(DocumentResource, 'doc') diff --git a/ietf/doc/serializers.py b/ietf/doc/serializers.py index ef47a43511..6c431a46f3 100644 --- a/ietf/doc/serializers.py +++ b/ietf/doc/serializers.py @@ -15,7 +15,6 @@ class RfcAuthorSerializer(serializers.ModelSerializer): """Serializer for a DocumentAuthor in a response""" - email = serializers.EmailField(source="email.address", required=False) datatracker_person_path = serializers.URLField( source="person.get_absolute_url", required=False, @@ -28,7 +27,7 @@ class Meta: "titlepage_name", "is_editor", "person", - "email", + "email", # relies on email.pk being email.address "affiliation", "country", "datatracker_person_path", @@ -54,6 +53,26 @@ def to_representation(self, instance): ) return super().to_representation(instance) + def validate(self, data): + email = data.get("email") + if email is not None: + person = data.get("person") + if person is None: + raise serializers.ValidationError( + { + "email": "cannot have an email without a person", + }, + code="email-without-person", + ) + if email.person_id != person.pk: + raise serializers.ValidationError( + { + "email": "email must belong to person", + }, + code="email-person-mismatch", + ) + return data + @dataclass class DocIdentifier: diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 37ba4adb78..d9524c246b 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -13,12 +13,13 @@ from dataclasses import dataclass from hashlib import sha384 from pathlib import Path -from typing import Iterator, Optional, Union +from typing import Iterator, Optional, Union, Iterable from zoneinfo import ZoneInfo from django.conf import settings from django.contrib import messages from django.core.cache import caches +from django.db import transaction from django.db.models import OuterRef from django.forms import ValidationError from django.http import Http404 @@ -33,7 +34,14 @@ from ietf.community.models import CommunityList from ietf.community.utils import docs_tracked_by_community_list -from ietf.doc.models import Document, DocHistory, State, DocumentAuthor, DocHistoryAuthor +from ietf.doc.models import ( + DocHistory, + DocHistoryAuthor, + Document, + DocumentAuthor, + RfcAuthor, + State, EditedRfcAuthorsDocEvent, +) from ietf.doc.models import RelatedDocument, RelatedDocHistory, BallotType, DocReminder from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, IRSGBallotDocEvent, NewRevisionDocEvent, StateDocEvent from ietf.doc.models import TelechatDocEvent, DocumentActionHolder, EditedAuthorsDocEvent, BallotPositionDocEvent @@ -562,6 +570,41 @@ def update_action_holders(doc, prev_state=None, new_state=None, prev_tags=None, # TODO : do we need an analog for rfcauthors here? Likely we do. + +def _change_field_and_describe( + author: DocumentAuthor | RfcAuthor, + field: str, + newval, + field_display_name: str | None = None, +): + # make the change + oldval = getattr(author, field) + setattr(author, field, newval) + + was_empty = oldval is None or len(str(oldval)) == 0 + now_empty = newval is None or len(str(newval)) == 0 + + # describe the change + if oldval == newval: + return None + else: + if field_display_name is None: + field_display_name = field + + if was_empty and not now_empty: + return 'set {field} to "{new}"'.format( + field=field_display_name, new=newval + ) + elif now_empty and not was_empty: + return 'cleared {field} (was "{old}")'.format( + field=field_display_name, old=oldval + ) + else: + return 'changed {field} from "{old}" to "{new}"'.format( + field=field_display_name, old=oldval, new=newval + ) + + def update_documentauthors(doc, new_docauthors, by=None, basis=None): """Update the list of authors for a document @@ -574,27 +617,6 @@ def update_documentauthors(doc, new_docauthors, by=None, basis=None): used. These objects will not be saved, their attributes will be used to create new DocumentAuthor instances. (The document and order fields will be ignored.) """ - def _change_field_and_describe(auth, field, newval): - # make the change - oldval = getattr(auth, field) - setattr(auth, field, newval) - - was_empty = oldval is None or len(str(oldval)) == 0 - now_empty = newval is None or len(str(newval)) == 0 - - # describe the change - if oldval == newval: - return None - else: - if was_empty and not now_empty: - return 'set {field} to "{new}"'.format(field=field, new=newval) - elif now_empty and not was_empty: - return 'cleared {field} (was "{old}")'.format(field=field, old=oldval) - else: - return 'changed {field} from "{old}" to "{new}"'.format( - field=field, old=oldval, new=newval - ) - persons = [] changes = [] # list of change descriptions @@ -638,6 +660,111 @@ def _change_field_and_describe(auth, field, newval): ) for change in changes ] + +def update_rfcauthors( + rfc: Document, new_rfcauthors: Iterable[RfcAuthor], by: Person | None = None +) -> Iterable[EditedRfcAuthorsDocEvent]: + def _find_matching_author( + author_to_match: RfcAuthor, existing_authors: Iterable[RfcAuthor] + ) -> RfcAuthor | None: + """Helper to find a matching existing author""" + if author_to_match.person_id is not None: + for candidate in existing_authors: + if candidate.person_id == author_to_match.person_id: + return candidate + return None # no match + # author does not have a person, match on titlepage name + for candidate in existing_authors: + if candidate.titlepage_name == author_to_match.titlepage_name: + return candidate + return None # no match + + def _rfcauthor_from_documentauthor(docauthor: DocumentAuthor) -> RfcAuthor: + """Helper to create an equivalent RfcAuthor from a DocumentAuthor""" + return RfcAuthor( + document_id=docauthor.document_id, + titlepage_name=docauthor.person.plain_name(), # closest thing we have + is_editor=False, + person_id=docauthor.person_id, + affiliation=docauthor.affiliation, + country=docauthor.country, + order=docauthor.order, + ) + + # Is this the first time this document is getting an RfcAuthor? If so, the + # updates will need to account for the model change. + converting_from_docauthors = not rfc.rfcauthor_set.exists() + + if converting_from_docauthors: + original_authors = [ + _rfcauthor_from_documentauthor(da) for da in rfc.documentauthor_set.all() + ] + else: + original_authors = list(rfc.rfcauthor_set.all()) + + authors_to_commit = [] + changes = [] + for order, new_author in enumerate(new_rfcauthors): + matching_author = _find_matching_author(new_author, original_authors) + if matching_author is not None: + # Update existing matching author using new_author data + authors_to_commit.append(matching_author) + original_authors.remove(matching_author) # avoid reuse + # Describe changes to this author + author_changes = [] + # Update fields other than order + for field in ["titlepage_name", "is_editor", "affiliation", "country"]: + author_changes.append( + _change_field_and_describe( + matching_author, + field, + getattr(new_author, field), + # List titlepage_name as "name" in logs + "name" if field == "titlepage_name" else field, + ) + ) + # Update order + author_changes.append( + _change_field_and_describe(matching_author, "order", order + 1) + ) + matching_author.save() + author_change_summary = ", ".join( + [ch for ch in author_changes if ch is not None] + ) + if len(author_change_summary) > 0: + changes.append( + 'Changed author "{name}": {summary}'.format( + name=matching_author.titlepage_name, + summary=author_change_summary, + ) + ) + else: + # No author matched, so update the new_author and use that + new_author.document = rfc + new_author.order = order + 1 + new_author.save() + changes.append(f'Added "{new_author.titlepage_name}" 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') + # Create DocEvents, but leave it up to caller to save + if by is None: + by = Person.objects.get(name="(System)") + return [ + EditedRfcAuthorsDocEvent( + type="edited_authors", + by=by, + doc=rfc, + desc=change, + ) + for change in changes + ] + + def update_reminder(doc, reminder_type_slug, event, due_date): reminder_type = DocReminderTypeName.objects.get(slug=reminder_type_slug)