Skip to content

Commit

Permalink
Addressed PR comments.
Browse files Browse the repository at this point in the history
Added ApplyChangeSetRequestSerializer to validate the request.
Changed "post" method in ApplyChangeSetView to use the serializer to validate the request.
  • Loading branch information
Julio-Oliveira-Encora committed Mar 5, 2024
1 parent aa6097e commit 2ab090e
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 49 deletions.
20 changes: 20 additions & 0 deletions diode-netbox-plugin/netbox_diode_plugin/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,23 @@ def get_object(self, instance):

context = {"request": self.context.get("request")}
return serializer(object_data, context=context, many=True).data[0]


class ChangeSetSerializer(serializers.Serializer):
"""ChangeSet Serializer."""

change_id = serializers.UUIDField(required=True)
change_type = serializers.CharField(required=True)
object_version = serializers.IntegerField(required=False, allow_null=True)
object_type = serializers.CharField(required=True)
object_id = serializers.IntegerField(required=False, allow_null=True)
data = serializers.DictField(required=True)


class ApplyChangeSetRequestSerializer(serializers.Serializer):
"""ApplyChangeSet request Serializer."""

change_set_id = serializers.UUIDField(required=True)
change_set = serializers.ListField(
child=ChangeSetSerializer(), required=True, allow_empty=False
)
61 changes: 29 additions & 32 deletions diode-netbox-plugin/netbox_diode_plugin/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
from utilities.api import get_serializer_for_model

from netbox_diode_plugin.api.permissions import IsDiodeReader, IsDiodeWriter
from netbox_diode_plugin.api.serializers import ObjectStateSerializer
from netbox_diode_plugin.api.serializers import (
ApplyChangeSetRequestSerializer,
ObjectStateSerializer,
)


class ObjectStateView(views.APIView):
Expand Down Expand Up @@ -127,7 +130,7 @@ def _get_serializer(
return serializer

@staticmethod
def _get_error_response(change_set_id: str = None, error: list = None):
def _get_error_response(change_set_id, error):
return Response(
{
"change_set_id": change_set_id,
Expand All @@ -143,44 +146,38 @@ def post(self, request, *args, **kwargs):
The request body should contain a list of changes to be applied.
"""
errors = []
serializer_list = []
validation_errors = []
serializer_errors = []

serializer = ApplyChangeSetRequestSerializer(data=request.data)

change_set_id = self.request.data.get("change_set_id", None)
if not change_set_id:
errors.append("change_set_id parameter is required")

change_set = self.request.data.get("change_set", None)
if not change_set:
errors.append("change_set parameter is required")
if not serializer.is_valid():
for error in serializer.errors:
if isinstance(serializer.errors[error], dict):
for _, error_values in serializer.errors[error].items():
for field_name, field_errors in error_values.items():
serializer_errors.append(
{field_name: f"{str(field_errors[0])}"}
)
else:
errors_dict = {
error: f"{str(field_errors)}"
for field_errors in serializer.errors[error]
}

# Return error response if any errors are found
if errors:
return self._get_error_response(change_set_id, errors)
serializer_errors.append(errors_dict)

for change in change_set:
return self._get_error_response(change_set_id, serializer_errors)

change_id = change.get("change_id", None)
if not change_id:
errors.append("change_id parameter is required")
change_set = self.request.data.get("change_set", None)

for change in change_set:
change_id = change.get("change_id", None)
change_type = change.get("change_type", None)
if not change_type:
errors.append("change_type parameter is required")

object_type = change.get("object_type", None)
if not object_type:
errors.append("object_type parameter is required")

object_data = change.get("data", None)
if not object_data:
errors.append("data parameter is required")

# Return error response if any errors are found
if errors:
return self._get_error_response(change_set_id, errors)

object_id = change.get("object_id", None)

serializer = self._get_serializer(
Expand All @@ -195,10 +192,10 @@ def post(self, request, *args, **kwargs):
for field_name, field_errors in serializer.errors.items()
}

validation_errors.append({"change_id": change_id, **errors_dict})
serializer_errors.append({"change_id": change_id, **errors_dict})

if validation_errors:
return self._get_error_response(change_set_id, validation_errors)
if serializer_errors:
return self._get_error_response(change_set_id, serializer_errors)

with transaction.atomic():
[serializer.save() for serializer in serializer_list]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ def test_change_type_create_and_update_with_error_in_one_object_return_400(self)
self.assertFalse(device_created.exists())

def test_multiples_change_type_create_with_error_in_two_objects_return_400(self):
"""Test create change_type with error in one object."""
"""Test create change_type with error in two object."""
payload = {
"change_set_id": str(uuid.uuid4()),
"change_set": [
Expand Down Expand Up @@ -485,7 +485,6 @@ def test_multiples_change_type_create_with_error_in_two_objects_return_400(self)
response = self.client.post(
self.url, payload, format="json", **self.user_header
)

site_created = Site.objects.filter(name="Site Z")
device_created = Device.objects.filter(name="Test Device 4")

Expand Down Expand Up @@ -537,7 +536,7 @@ def test_change_type_update_with_object_id_not_exist_return_400(self):
self.assertEqual(site_updated.name, "Site 2")

def test_change_set_id_field_not_provided_return_400(self):
"""Test update object with nonexistent object_id."""
"""Test update object with change_set_id incorrect."""
payload = {
"change_set_id": None,
"change_set": [
Expand All @@ -546,7 +545,7 @@ def test_change_set_id_field_not_provided_return_400(self):
"change_type": "update",
"object_version": None,
"object_type": "dcim.site",
"object_id": 30,
"object_id": 20,
"data": {
"name": "Site A",
"slug": "site-a",
Expand All @@ -566,20 +565,23 @@ def test_change_set_id_field_not_provided_return_400(self):
)
self.assertEqual(response.status_code, 400)
self.assertEqual(
response.json().get("errors")[0], "change_set_id parameter is required"
response.json().get("errors")[0].get("change_set_id"),
"This field may not be null.",
)

def test_change_type_field_not_provided_return_400(self):
"""Test update object with nonexistent object_id."""
def test_change_set_id_change_id_and_change_type_field_not_provided_return_400(
self,
):
"""Test update object with change_set_id, change_id, and change_type incorrect."""
payload = {
"change_set_id": str(uuid.uuid4()),
"change_set_id": "",
"change_set": [
{
"change_id": str(uuid.uuid4()),
"change_type": None,
"change_id": "",
"change_type": "",
"object_version": None,
"object_type": "dcim.site",
"object_id": 30,
"object_id": 20,
"data": {
"name": "Site A",
"slug": "site-a",
Expand All @@ -599,24 +601,34 @@ def test_change_type_field_not_provided_return_400(self):
)
self.assertEqual(response.status_code, 400)
self.assertEqual(
response.json().get("errors")[0], "change_type parameter is required"
response.json().get("errors")[0].get("change_set_id"),
"Must be a valid UUID.",
)
self.assertEqual(
response.json().get("errors")[1].get("change_id"),
"Must be a valid UUID.",
)
self.assertEqual(
response.json().get("errors")[2].get("change_type"),
"This field may not be blank.",
)

def test_change_set_id_field_and_change_set_not_provided_return_400(self):
"""Test update object with nonexistent object_id."""
"""Test update object with change_set_id and change_set incorrect."""
payload = {
"change_set_id": None,
"change_set_id": "",
"change_set": [],
}

response = self.client.post(
self.url, payload, format="json", **self.user_header
)

self.assertEqual(response.status_code, 400)
self.assertEqual(
response.json().get("errors")[0], "change_set_id parameter is required"
response.json().get("errors")[0].get("change_set_id"),
"Must be a valid UUID.",
)
self.assertEqual(
response.json().get("errors")[1], "change_set parameter is required"
response.json().get("errors")[1].get("change_set"),
"This list may not be empty.",
)

0 comments on commit 2ab090e

Please sign in to comment.