Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions ietf/api/serializers_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,18 @@ class EditableRfcSerializer(serializers.ModelSerializer):
child=SubseriesNameField(required=False),
write_only=True,
)
updates = serializers.ListField(
child=serializers.IntegerField(),
required=False,
write_only=True,
help_text="List of RFC numbers this document updates."
)
obsoletes = serializers.ListField(
child=serializers.IntegerField(),
required=False,
write_only=True,
help_text="List of RFC numbers this document obsoletes."
)

class Meta:
model = Document
Expand All @@ -584,8 +596,28 @@ class Meta:
"std_level",
"subseries",
"keywords",
"updates",
"obsoletes",
]

def _validate_rfc_number_list(self, field_name, rfc_numbers):
"""Raise ValidationError if any RFC numbers in the list don't exist."""
unknown = [
n for n in rfc_numbers
if not Document.objects.filter(rfc_number=n, type_id="rfc").exists()
]
if unknown:
raise serializers.ValidationError(
{field_name: [f"Unknown RFC number: {n}" for n in unknown]}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is creating a ValidationError that maps the field name to a list rather than a string. That might work, but I think it should be more like

{field_name: f"Unknown RFC number(s): {", ".join(n for n in unknown)}"}

)
return rfc_numbers

def validate_updates(self, value):
return self._validate_rfc_number_list("updates", value)

def validate_obsoletes(self, value):
return self._validate_rfc_number_list("obsoletes", value)

def create(self, validated_data):
raise RuntimeError("Cannot create with this serializer")

Expand All @@ -602,6 +634,8 @@ def update(self, instance, validated_data):
published = validated_data.pop("published", omitted)
subseries = validated_data.pop("subseries", omitted)
authors_data = validated_data.pop("rfcauthor_set", omitted)
updates = validated_data.pop("updates", omitted)
obsoletes = validated_data.pop("obsoletes", omitted)

# Transaction to clean up if something fails
with transaction.atomic():
Expand Down Expand Up @@ -673,6 +707,24 @@ def update(self, instance, validated_data):
)
)
)
if updates is not omitted:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For both updates and obsoletes, we sometimes add DocEvents when they change. That's perhaps not needed for RFCs, where the document itself is an indication of when the older document became updated or obsoleted. @rjsparks ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the long run I think DocEvents on the older RFC when a newer one updates or obsoletes it would be a nice thing to have going forward just to make the history easier to read, but to keep things consistent we'd have to back-cast these events for the whole series, and that's not something I think we should bite off right now.

So, lets add making them a feature request and move on without adding them now.

RelatedDocument.objects.filter(
source=rfc, relationship_id="updates"
).exclude(target__rfc_number__in=updates).delete()
for rfc_num in updates:
target = Document.objects.get(rfc_number=rfc_num, type_id="rfc")
RelatedDocument.objects.get_or_create(
source=rfc, relationship_id="updates", target=target
)
if obsoletes is not omitted:
RelatedDocument.objects.filter(
source=rfc, relationship_id="obs"
).exclude(target__rfc_number__in=obsoletes).delete()
for rfc_num in obsoletes:
target = Document.objects.get(rfc_number=rfc_num, type_id="rfc")
RelatedDocument.objects.get_or_create(
source=rfc, relationship_id="obs", target=target
)

# update subseries relations
if subseries is not omitted:
Expand Down
24 changes: 24 additions & 0 deletions ietf/api/tests_serializers_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,27 @@ def test_partial_update(self, mock_trigger_red_task, mock_update_searchindex_tas
mock_update_searchindex_task.delay.call_args,
mock.call(rfc.rfc_number),
)

def test_unknown_rfc_number_rejected(self):
"""Unknown RFC numbers in updates/obsoletes should cause validation failure."""
from django.db.models import Max

rfc = WgRfcFactory()
unknown_rfc_number = (
Document.objects.filter(rfc_number__isnull=False).aggregate(
m=Max("rfc_number") + 1
)["m"]
or 10000
)

for field in ("updates", "obsoletes"):
serializer = EditableRfcSerializer(
partial=True,
instance=rfc,
data={field: [unknown_rfc_number]},
)
self.assertFalse(
serializer.is_valid(),
msg=f"{field} with unknown RFC number should be invalid",
)
self.assertIn(field, serializer.errors)
Loading