Replace `NestedFooSerializer` and `?brief=True` parameters using DRF's built-in `self.Meta.depth` #3500

HanlinMiao · 2023-03-28T03:26:22Z

Closes: #3042

What's Changed

Remaining Items

Location Browsable API
?depth=0 / default depth

?depth = 1

?depth = 2 too much info to fit on one screen but notice that parent's parent is expanded as well

TODO

Explanation of Change(s)
Added change log fragment(s) (for more information see the documentation)
Attached Screenshots, Payload Example
Unit, Integration Tests
Documentation Updates (when adding/changing features)
Example Plugin Updates (when adding/changing features)
Outline Remaining Work, Constraints from Design

nautobot/extras/api/serializers.py

nautobot/dcim/api/serializers.py

nautobot/core/api/serializers.py

nautobot/core/api/views.py

… I can

…ch as possible

nautobot/circuits/api/serializers.py

nautobot/core/api/serializers.py

timizuoebideri1 · 2023-04-03T11:06:22Z

nautobot/core/api/serializers.py

+    def build_nested_field(self, field_name, relation_info, nested_depth):
+        field = get_serializer_for_model(relation_info.related_model)
+
+        class NautobotNestedSerializer(field):


This class logic looks really similar to NautobotPrimaryKeyRelatedField above, I guess finding a way to utitlize that class here might be a better approach

…tedTagSerailizer and TagSerializerField

…penAPISchema Serializer depth default to 1

jvanderaa · 2023-04-06T12:36:03Z

What is going to be the overall change on this. What is the goal, what should be tested in SDKs?

…nto hanlin-#3042

jvanderaa · 2023-04-10T17:00:14Z

Some discussion points that will be coming up.

From my viewpoint, whether there is a default depth of 0 or 1 isn't really a big deal. I like the brevity that comes with 0, just need to make sure that the novel pynautobot and Nautobot API user know how to work with it.

From my standpoint, if the default is 0, pynautobot would remain having the default be passed in, and provide a mechanism to set the depth on the calls within the instance create time. The Nautobot Ansible modules would create the instance with a depth of 1 to get at the same nested serializer data that is coming along.

So if there is a vote, I'm a vote for a depth of 0 as default, and that the SDKs take care of providing sane default (Ansible) and access to the system.

glennmatthews

🚤 🐎 ⏩

changes/3042.changed

nautobot/core/api/mixins.py

nautobot/ipam/api/serializers.py

nautobot/virtualization/api/serializers.py

glennmatthews · 2023-04-20T16:06:34Z

nautobot/virtualization/forms.py

@@ -453,7 +453,7 @@ class VMInterfaceForm(NautobotModelForm, InterfaceCommonForm):
        queryset=VLAN.objects.all(),
        required=False,
        label="Untagged VLAN",
-        brief_mode=False,
+        depth=1,


See my other comments, I don't think this is in fact correct/necessary in the vast majority of cases.

glennmatthews · 2023-04-21T13:13:37Z

nautobot/core/api/serializers.py

+            field_name == "tags"
+            and isinstance(model_class.tags, (_NautobotTaggableManager, _TaggableManager))


Probably just the isinstance check is sufficient here - I think even if we had a _TaggableManager field named something other than tags we'd still need this case?

glennmatthews · 2023-04-21T13:15:40Z

nautobot/core/api/serializers.py

@@ -315,3 +425,194 @@ class BulkOperationIntegerIDSerializer(serializers.Serializer):
 class GraphQLAPISerializer(serializers.Serializer):
    query = serializers.CharField(required=True, help_text="GraphQL query")
    variables = serializers.JSONField(required=False, help_text="Variables in JSON Format")
+
+
+class CustomFieldModelSerializerMixin(ValidatedModelSerializer):


nit: should this be in nautobot.core.api.mixins instead?

glennmatthews · 2023-04-21T13:16:04Z

nautobot/core/api/serializers.py

+    pass
+
+
+class RelationshipModelSerializerMixin(ValidatedModelSerializer):


nit: move to nautobot.core.api.mixins?

I did try that before, however ValidatedModelSerializer and BaseModelSerializer base classes for these mixins seem to create another circular import problem🫠

glennmatthews · 2023-04-21T13:16:16Z

nautobot/core/api/serializers.py

+        return fields
+
+
+class NotesSerializerMixin(BaseModelSerializer):


nit: move to nautobot.core.api.mixins?

Same comment above

glennmatthews · 2023-04-21T13:18:47Z

nautobot/core/api/utils.py

+    in which relation_info and nested_depth are already given.
+    """
+    nested_serializer_name = (
+        "Nested" + str(nested_depth) + f"{relation_info.related_model._meta.model_name.capitalize()}"


Possibly?

Suggested change

"Nested" + str(nested_depth) + f"{relation_info.related_model._meta.model_name.capitalize()}"

f"Nested{nested_depth}{relation_info.related_model.__name__}"

glennmatthews · 2023-04-21T13:23:15Z

nautobot/dcim/api/serializers.py

-    @extend_schema_field(NestedDeviceSerializer)
-    def get_parent_device(self, obj):
+    @extend_schema_field(DeviceBaySerializer)
+    def get_parent_bay(self, obj):


Need to document this as an API change.

Added as an entry in v2-api-renamed-fields.yaml

glennmatthews · 2023-04-21T13:28:48Z

nautobot/dcim/api/serializers.py

-        context = {"request": self.context["request"]}
-        data = NestedDeviceSerializer(instance=device_bay.device, context=context).data
-        data["device_bay"] = NestedDeviceBaySerializer(instance=device_bay, context=context).data
+        depth = int(self.context.get("depth", 0))


I'll be honest, I still don't fully understand this. :-) I see where Meta.depth is decremented when we create a nested serializer, but I don't see where context["depth"] does this. So it seems like with this code as-is, a query with depth=1 would result in the Device serializer rendering with depth 1, the nested device-bay serializer rendering with depth 1, its nested device serializer rendering with depth 1, its own nested device-bay serializer rendering with depth 1, etc. to infinite depth. But obviously that isn't happening... so what am I missing? :-)

glennmatthews

🚢 🇮🇹, I say !

jathanism · 2023-04-21T16:47:59Z

nautobot/core/api/mixins.py

+
+    def get_queryset_filter_params(self, data, queryset):
+        """
+        Data could be a dictionary and an int (for the User model) or a str that represents the primary key.


Dictionary and an int? It's not clear to me what that means.

It could be {"location": location.pk} or a nested dictionary {"location": {"name": "Campus-01"}}

jathanism · 2023-04-21T16:50:01Z

nautobot/core/api/mixins.py

+            params = dict_to_filter_params(data)
+            return self.remove_non_filter_fields(params)
+        try:
+            pk = int(data) if isinstance(queryset.model._meta.pk, AutoField) else uuid.UUID(str(data))


Why casting data to a UUID in the latter case? UUIDs can be strings. Are you just trying to force the error to assert that it must be either an int or UUID? If so can you please add a brief comment as to the intent here for posterity?

I believe the int case is for the User model mentioned in the docstring above. But yeah I can add a comment here as well.

jathanism · 2023-04-21T17:01:23Z

nautobot/docs/installation/upgrading-from-nautobot-v1.md

@@ -128,6 +128,10 @@ These endpoints `/ipam/roles/`, `/dcim/rack-roles/` and `/dcim/device-roles/` ar
 | `/dcim/rack-roles/`   | `/extras/roles/` |
 | `/ipam/roles/`        | `/extras/roles/` |

+### API Query Parameters Changes
+
+Nautobot 2.0 removes the `?brief` query parameter and adds support for the `?depth` query parameter. As a result, the ability to specify `brief_mode` in `DynamicModelChoiceField` , `DynamicModelMultipleChoiceField` or `MultiMatchModelMultipleChoiceField` is also removed. For every occurrence of the aforementioned fields where you have `brief_mode` set to `True/False` e.g. `brief_mode=True`, please remove the statement. And leave other occurrences of the fields where you do not have `brief_mode` specified as they are. See more details about the `?depth` query parameter [here](../rest-api/overview.md/#depth-query-parameter)


We should never link anything in documentation using "here". Better to always link the contextual text.

Suggested change

Nautobot 2.0 removes the `?brief` query parameter and adds support for the `?depth` query parameter. As a result, the ability to specify `brief_mode` in `DynamicModelChoiceField` , `DynamicModelMultipleChoiceField` or `MultiMatchModelMultipleChoiceField` is also removed. For every occurrence of the aforementioned fields where you have `brief_mode` set to `True/False` e.g. `brief_mode=True`, please remove the statement. And leave other occurrences of the fields where you do not have `brief_mode` specified as they are. See more details about the `?depth` query parameter [here](../rest-api/overview.md/#depth-query-parameter)

Nautobot 2.0 removes the `?brief` query parameter and adds support for the `?depth` query parameter. As a result, the ability to specify `brief_mode` in `DynamicModelChoiceField`, `DynamicModelMultipleChoiceField`, and `MultiMatchModelMultipleChoiceField` has also been removed. For every occurrence of the aforementioned fields where you have `brief_mode` set to `True/False` (e.g. `brief_mode=True`), please remove the statement, leaving other occurrences of the fields where you do not have `brief_mode` specified as they are.

Please see the [documentation on the `?depth` query parameter](../rest-api/overview.md/#depth-query-parameter) for more information.

jathanism · 2023-04-21T17:01:53Z

nautobot/docs/models/extras/status.md

@@ -62,9 +62,9 @@ Any model form that is intended to have a `status` field must inherit from one o

 - FIXME: CSV import forms

-### `StatusModelSerializerMixin` serializer mixin
+### `` serializer mixin


This seems like a typo? Also everything below appears to be in bold now?

jathanism · 2023-04-21T17:02:29Z

nautobot/docs/models/extras/status.md

@@ -98,7 +98,7 @@ To fully integrate a model to include a `status` field, assert the following:

 ### Serializers

- Serializers for your model must inherit from `nautobot.extras.api.serializers.StatusModelSerializerMixin`
+- Serializers for your model must inherit from `nautobot.extras.api.serializers.`


Bad search/replace?

jathanism · 2023-04-21T17:08:30Z

nautobot/docs/release-notes/version-2.0.md

@@ -18,6 +18,10 @@ DeviceRole, RackRole, IPAM Role, and IPAddressRoleChoices have all been merged i

 Added Site Model Fields to Location. Location Model now has `asn`, `comments`, `contact_email`, `contact_name`, `contact_phone`, `facility`, `latitude`, `longitude`, `physical_address`, `shipping_address` and `time_zone` fields.

+#### Added Depth REST API Query Parameter
+
+Added the `?depth` query parameter in Nautobot v2.X to replace the `?brief` parameter in the REST API. It enables [nested serialization](https://www.django-rest-framework.org/api-guide/serializers/#specifying-nested-serialization) functionality and offers a more dynamic and comprehensive browsable API. It allows users greater control of the API response data and it is available for both retrieving a single object and a list of objects. This parameter is an positive integer value that can range from 0 to 10. Check out more about the `?depth` query parameter in our [REST API Documentation](../rest-api/overview.md/#depth-query-parameter)


Suggested change

Added the `?depth` query parameter in Nautobot v2.X to replace the `?brief` parameter in the REST API. It enables [nested serialization](https://www.django-rest-framework.org/api-guide/serializers/#specifying-nested-serialization) functionality and offers a more dynamic and comprehensive browsable API. It allows users greater control of the API response data and it is available for both retrieving a single object and a list of objects. This parameter is an positive integer value that can range from 0 to 10. Check out more about the `?depth` query parameter in our [REST API Documentation](../rest-api/overview.md/#depth-query-parameter)

Added the `?depth` query parameter in Nautobot v2.X to replace the `?brief` parameter in the REST API. It enables [nested serialization](https://www.django-rest-framework.org/api-guide/serializers/#specifying-nested-serialization) functionality and offers a more dynamic and comprehensive browsable API. It allows users greater control of the API response data and it is available for both retrieving a single object and a list of objects. This parameter is a positive integer value that can range from 0 to 10. To learn more more, check out the [documentation on the `?depth` query parameter](../rest-api/overview.md/#depth-query-parameter).

jathanism · 2023-04-21T17:09:39Z

nautobot/docs/release-notes/version-2.0.md

@@ -236,6 +240,10 @@ The "Container" status will be removed and all prefixes will be migrated to the

 ### Removed

+#### Removed Brief REST API Query Parameter
+
+Support for `?brief` REST API query parameter and `Nested*Serializers` are being removed in Nautobot v2.X. They are replaced by the new `?depth` query parameter. Check out the specific changes in [REST API documentation](../rest-api/overview.md/#depth-query-parameter)


Suggested change

Support for `?brief` REST API query parameter and `Nested*Serializers` are being removed in Nautobot v2.X. They are replaced by the new `?depth` query parameter. Check out the specific changes in [REST API documentation](../rest-api/overview.md/#depth-query-parameter)

Support for `?brief` REST API query parameter and `Nested*Serializers` have been removed in Nautobot v2.X. They are replaced by the new [`?depth` query parameter](../rest-api/overview.md/#depth-query-parameter).

jathanism · 2023-04-21T17:15:36Z

nautobot/project-static/js/forms.js

+                        // DynamicGroupSerializer has a `children` field which fits an inappropriate if condition
+                        // in select2.min.js, which will result in the incorrect rendering of DynamicGroup DynamicChoiceField.
+                        // So we nullify the field here since we do not need this field.
+                        if (record.url.includes("dynamic-groups")){


Can't wait to yeet this in v2 UI.

first commit

72eb145

HanlinMiao commented Mar 28, 2023

View reviewed changes

nautobot/extras/api/serializers.py Show resolved Hide resolved

HanlinMiao commented Mar 28, 2023

View reviewed changes

nautobot/dcim/api/serializers.py Show resolved Hide resolved

Hanlin Miao added 2 commits March 27, 2023 23:33

delete ?brief related method overrides

74e35fb

black

96396d1

HanlinMiao linked an issue Mar 28, 2023 that may be closed by this pull request

Replace NestedFooSerializer and brief=True pattern with DRF built-in Meta.depth #3042

Closed

glennmatthews reviewed Mar 28, 2023

View reviewed changes

nautobot/dcim/api/serializers.py Show resolved Hide resolved

nautobot/core/api/serializers.py Outdated Show resolved Hide resolved

nautobot/core/api/views.py Outdated Show resolved Hide resolved

Hanlin Miao added 5 commits March 28, 2023 21:09

good stopping point for now

77ef170

changelog and flake8

245aee3

push commits

68ff21f

resolve circular import and elminiate NestedFooSerializer usage where…

45e9b14

… I can

eliminate NestedFooSerializer and SerializePKRelatedField usage as mu…

d16e941

…ch as possible

timizuoebideri1 reviewed Apr 3, 2023

View reviewed changes

bryanculver mentioned this pull request Apr 3, 2023

Merge OrganizationalModel and PrimaryModel #2899

Open

3 tasks

Hanlin Miao added 4 commits April 3, 2023 15:32

add to_internal_value() to WritableSerializerMixin

476b365

remove brief/brief_mode references and eliminate the needs to use Nes…

c05bce5

…tedTagSerailizer and TagSerializerField

eliminate prefix='Nested' from PolymorphicProxySerializers and make O…

d34fdd0

…penAPISchema Serializer depth default to 1

merge next

2189d3a

Hanlin Miao and others added 9 commits April 6, 2023 18:07

fix dcim and some extras tests

a0b89d6

merge next

d28097e

fix circuits, core, dcim and some extras unittests

cb0e1e2

unittests

f395620

Delete output.txt

8f65d23

somewhat fixed extras unittests

01b4b06

Merge branch 'hanlin-#3042' of https://github.com/nautobot/nautobot i…

7f91511

…nto hanlin-#3042

flake8

e0586fb

pylint

f44e417

jvanderaa mentioned this pull request Apr 10, 2023

Nautobot 2.0: Nautobot Nested Serializer Change - NEED TO ADAPT the modules nautobot/nautobot-ansible#201

Open

HanlinMiao requested a review from jathanism April 20, 2023 00:26

HanlinMiao mentioned this pull request Apr 20, 2023

Add depth support in CRUD operation (PUT/POST/PATCH) headers. #3624

Open

glennmatthews reviewed Apr 20, 2023

View reviewed changes

address some of the feedback

5450ae5

addressed everything besides documentations

f98a333

bryanculver mentioned this pull request Apr 21, 2023

Get tests passing for UI 2.0 #3632

Merged

6 tasks

Hanlin Miao added 4 commits April 20, 2023 22:58

addressed documentation changes needed

be6ff3a

fix misnamed file

532127b

correct get_parent_bay() method

7712180

core unittest

b70a7dc

glennmatthews reviewed Apr 21, 2023

View reviewed changes

glennmatthews approved these changes Apr 21, 2023

View reviewed changes

documentation, serializer naming and deleted CustomFieldModelSerializer

23b451b

jathanism approved these changes Apr 21, 2023

View reviewed changes

Hanlin Miao added 2 commits April 21, 2023 13:51

adjusted how we get depth in SerializerMethodField

98c2da4

docstring

ea9a077

HanlinMiao merged commit 29348b1 into next Apr 21, 2023
18 checks passed

HanlinMiao deleted the hanlin-#3042 branch April 21, 2023 19:17

bryanculver added a commit that referenced this pull request Apr 22, 2023

Fix handling of #3321 and #3624 after #3500

187a989

bryanculver mentioned this pull request Apr 22, 2023

Replace NestedFooSerializer and brief=True pattern with DRF built-in Meta.depth #3042

Closed

glennmatthews mentioned this pull request Apr 27, 2023

[REVIEW] Next UI Hackathon #3651

Merged

bryanculver mentioned this pull request May 3, 2023

IPAM Overhaul: Assign IP Address to Multiple Interfaces #2403

Closed

glennmatthews mentioned this pull request May 10, 2023

RESTful API self-consistent data types regardless of depth #3725

Closed

This was referenced May 19, 2023

2.0: Swagger UI broken for POST /dcim/cables/, possibly others #3778

Closed

CSV export and import via REST API #3722

Merged

Return is_superuser and last_login fields for User API endpoints #2208

Closed

bryanculver mentioned this pull request Jul 13, 2023

IPAddressToInterface: Expose in REST API #4101

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `NestedFooSerializer` and `?brief=True` parameters using DRF's built-in `self.Meta.depth` #3500

Replace `NestedFooSerializer` and `?brief=True` parameters using DRF's built-in `self.Meta.depth` #3500

HanlinMiao commented Mar 28, 2023 •

edited

timizuoebideri1 Apr 3, 2023

jvanderaa commented Apr 6, 2023

jvanderaa commented Apr 10, 2023

glennmatthews left a comment

glennmatthews Apr 20, 2023

glennmatthews Apr 21, 2023

glennmatthews Apr 21, 2023

glennmatthews Apr 21, 2023

HanlinMiao Apr 21, 2023

glennmatthews Apr 21, 2023

HanlinMiao Apr 21, 2023

glennmatthews Apr 21, 2023

glennmatthews Apr 21, 2023

HanlinMiao Apr 21, 2023

glennmatthews Apr 21, 2023

glennmatthews left a comment

jathanism Apr 21, 2023

HanlinMiao Apr 21, 2023

jathanism Apr 21, 2023

HanlinMiao Apr 21, 2023

jathanism Apr 21, 2023

jathanism Apr 21, 2023

jathanism Apr 21, 2023

jathanism Apr 21, 2023

jathanism Apr 21, 2023

jathanism Apr 21, 2023

		field_name == "tags"
		and isinstance(model_class.tags, (_NautobotTaggableManager, _TaggableManager))

		pass


		class RelationshipModelSerializerMixin(ValidatedModelSerializer):

		return fields


		class NotesSerializerMixin(BaseModelSerializer):

	"Nested" + str(nested_depth) + f"{relation_info.related_model._meta.model_name.capitalize()}"
	f"Nested{nested_depth}{relation_info.related_model.__name__}"

-Nautobot 2.0 removes the `?brief` query parameter and adds support for the `?depth` query parameter. As a result, the ability to specify `brief_mode` in `DynamicModelChoiceField` , `DynamicModelMultipleChoiceField` or `MultiMatchModelMultipleChoiceField` is also removed. For every occurrence of the aforementioned fields where you have `brief_mode` set to `True/False` e.g. `brief_mode=True`, please remove the statement. And leave other occurrences of the fields where you do not have `brief_mode` specified as they are. See more details about the `?depth` query parameter [here](../rest-api/overview.md/#depth-query-parameter)
+Nautobot 2.0 removes the `?brief` query parameter and adds support for the `?depth` query parameter. As a result, the ability to specify `brief_mode` in `DynamicModelChoiceField`, `DynamicModelMultipleChoiceField`, and `MultiMatchModelMultipleChoiceField` has also been removed. For every occurrence of the aforementioned fields where you have `brief_mode` set to `True/False` (e.g. `brief_mode=True`), please remove the statement, leaving other occurrences of the fields where you do not have `brief_mode` specified as they are.
+Please see the [documentation on the `?depth` query parameter](../rest-api/overview.md/#depth-query-parameter) for more information.

	Support for `?brief` REST API query parameter and `Nested*Serializers` are being removed in Nautobot v2.X. They are replaced by the new `?depth` query parameter. Check out the specific changes in [REST API documentation](../rest-api/overview.md/#depth-query-parameter)
	Support for `?brief` REST API query parameter and `Nested*Serializers` have been removed in Nautobot v2.X. They are replaced by the new [`?depth` query parameter](../rest-api/overview.md/#depth-query-parameter).

Replace NestedFooSerializer and ?brief=True parameters using DRF's built-in self.Meta.depth #3500

Replace NestedFooSerializer and ?brief=True parameters using DRF's built-in self.Meta.depth #3500

Conversation

HanlinMiao commented Mar 28, 2023 • edited

Closes: #3042

What's Changed

Remaining Items

TODO

Choose a reason for hiding this comment

jvanderaa commented Apr 6, 2023

jvanderaa commented Apr 10, 2023

glennmatthews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glennmatthews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Replace `NestedFooSerializer` and `?brief=True` parameters using DRF's built-in `self.Meta.depth` #3500

Replace `NestedFooSerializer` and `?brief=True` parameters using DRF's built-in `self.Meta.depth` #3500

HanlinMiao commented Mar 28, 2023 •

edited