Fix cables status choice being provided by OPTIONS

[FragmentedPacket]

Fixes: #452 452

Remove the metadata_class as StatusViewSetMixin uses it to properly set the metadata returned.

[FragmentedPacket]

I would like to add some tests to make sure any model that uses the Status model will be tested that it's rendered metadata works and prevent any regression. Not sure if this should happen in two separate places (model or viewset tests and API tests) as the Ansible modules rely on this data.

[glennmatthews]

Looks like the metadata_class was added in Netbox netbox-community/netbox#4695 to handle a different issue with the OPTIONS content on Cables - are we sure that removing this is the right thing to do, or do we need to instead extend with a custom metadata_class that supports both status and termination_a_type information?

[FragmentedPacket]

I thought I had responded, but I guess I didn't hit comment. I don't think I have an opinion on this so whichever way y'all think is beneficial and I will implement it.

[jathanism]

This should be fine see

Looks like the metadata_class was added in Netbox netbox-community/netbox#4695 to handle a different issue with the OPTIONS content on Cables - are we sure that removing this is the right thing to do, or do we need to instead extend with a custom metadata_class that supports both status and termination_a_type information?

If you look at the code, despite the filter conditions, the two metadata subclasses are doing nearly the same thing:

• ContentTypeMetadata is gating on field type ContentTypeField
• StatusFieldMetadata is gating on field type StatusSerializerField

I would reckon that with a little debugging we could probably adapt StatusFieldMetadata to do both in this case.

[FragmentedPacket]

I did try and get that working, but wasn't successful as I think that makes the most sense at this point. I will give it another go and hit you up if I have issues with it.

[FragmentedPacket]

I'm about to push up working changes as well as new tests, but wanted to get y'alls feedback on testing.

I think I also discovered another bug that the statuses that are returned are not properly filtered for the endpoint. Cables should only have three status choices and virtual machines should have 6. Numbers are arbitrary, but this is with a fresh install of Nautobot.

>>> nb.dcim.cables.choices()['status']
[{'value': 'active', 'display': 'Active'}, {'value': 'available', 'display': 'Available'}, {'value': 'connected', 'display': 'Connected'}, {'value': 'container', 'display': 'Container'}, {'value': 'decommissioned', 'display': 'Decommissioned'}, {'value': 'decommissioning', 'display': 'Decommissioning'}, {'value': 'deprecated', 'display': 'Deprecated'}, {'value': 'deprovisioning', 'display': 'Deprovisioning'}, {'value': 'dhcp', 'display': 'DHCP'}, {'value': 'failed', 'display': 'Failed'}, {'value': 'inventory', 'display': 'Inventory'}, {'value': 'offline', 'display': 'Offline'}, {'value': 'planned', 'display': 'Planned'}, {'value': 'provisioning', 'display': 'Provisioning'}, {'value': 'reserved', 'display': 'Reserved'}, {'value': 'retired', 'display': 'Retired'}, {'value': 'slaac', 'display': 'SLAAC'}, {'value': 'staged', 'display': 'Staged'}, {'value': 'staging', 'display': 'Staging'}]
>>> nb.virtualization.virtual_machines.choices()['status']
[{'value': 'active', 'display': 'Active'}, {'value': 'available', 'display': 'Available'}, {'value': 'connected', 'display': 'Connected'}, {'value': 'container', 'display': 'Container'}, {'value': 'decommissioned', 'display': 'Decommissioned'}, {'value': 'decommissioning', 'display': 'Decommissioning'}, {'value': 'deprecated', 'display': 'Deprecated'}, {'value': 'deprovisioning', 'display': 'Deprovisioning'}, {'value': 'dhcp', 'display': 'DHCP'}, {'value': 'failed', 'display': 'Failed'}, {'value': 'inventory', 'display': 'Inventory'}, {'value': 'offline', 'display': 'Offline'}, {'value': 'planned', 'display': 'Planned'}, {'value': 'provisioning', 'display': 'Provisioning'}, {'value': 'reserved', 'display': 'Reserved'}, {'value': 'retired', 'display': 'Retired'}, {'value': 'slaac', 'display': 'SLAAC'}, {'value': 'staged', 'display': 'Staged'}, {'value': 'staging', 'display': 'Staging'}]

I haven't been able to dig into it too much, but I believe this is the culprit.

https://github.com/nautobot/nautobot/blob/develop/nautobot/extras/api/serializers.py#L621

[glennmatthews]

I think the bug identification and your analysis are correct. It should be possible to do something similar on the serializer field to what the form field does with StatusField.get_limit_choices_to() - looks like creating a custom get_queryset() method on StatusSerializerField would be the equivalent approach.

[glennmatthews]

Thank you!

[FragmentedPacket]

This should be ready for review now with applicable tests and a few minor updates to properly return content_types that are serialized as ContentTypeField.

[glennmatthews]

Looks great!

[jathanism]

Elegant decision just making StatusFieldMetadata a subclass of ContentTypeMetadata!