#9045 #9046 - remove legacy fields from Provider (#10377)

* #9045 - remove legacy fields from Provider

* Add safegaurd for legacy data to migration

* 9045 remove fields from forms and tables

* Update unrelated tests to use ASN model instead of Provider

* Fix migrations collision

Co-authored-by: jeremystretch <jstretch@ns1.com>

netbox/circuits/api/serializers.py

@@ -31,7 +31,7 @@ class ProviderSerializer(NetBoxModelSerializer):
     class Meta:
         model = Provider
         fields = [
-            'id', 'url', 'display', 'name', 'slug', 'asn', 'account', 'portal_url', 'noc_contact', 'admin_contact',
+            'id', 'url', 'display', 'name', 'slug', 'account',
             'comments', 'asns', 'tags', 'custom_fields', 'created', 'last_updated', 'circuit_count',
         ]
 

netbox/circuits/filtersets.py

@@ -65,7 +65,7 @@ class ProviderFilterSet(NetBoxModelFilterSet, ContactModelFilterSet):
 
     class Meta:
         model = Provider
-        fields = ['id', 'name', 'slug', 'asn', 'account']
+        fields = ['id', 'name', 'slug', 'account']
 
     def search(self, queryset, name, value):
         if not value.strip():
@@ -73,8 +73,6 @@ class ProviderFilterSet(NetBoxModelFilterSet, ContactModelFilterSet):
         return queryset.filter(
             Q(name__icontains=value) |
             Q(account__icontains=value) |
-            Q(noc_contact__icontains=value) |
-            Q(admin_contact__icontains=value) |
             Q(comments__icontains=value)
         )
 

netbox/circuits/forms/bulk_edit.py

@@ -20,10 +20,6 @@ __all__ = (
 
 
 class ProviderBulkEditForm(NetBoxModelBulkEditForm):
-    asn = forms.IntegerField(
-        required=False,
-        label='ASN (legacy)'
-    )
     asns = DynamicModelMultipleChoiceField(
         queryset=ASN.objects.all(),
         label=_('ASNs'),
@@ -34,20 +30,6 @@ class ProviderBulkEditForm(NetBoxModelBulkEditForm):
         required=False,
         label='Account number'
     )
-    portal_url = forms.URLField(
-        required=False,
-        label='Portal'
-    )
-    noc_contact = forms.CharField(
-        required=False,
-        widget=SmallTextarea,
-        label='NOC contact'
-    )
-    admin_contact = forms.CharField(
-        required=False,
-        widget=SmallTextarea,
-        label='Admin contact'
-    )
     comments = CommentField(
         widget=SmallTextarea,
         label='Comments'
@@ -55,10 +37,10 @@ class ProviderBulkEditForm(NetBoxModelBulkEditForm):
 
     model = Provider
     fieldsets = (
-        (None, ('asn', 'asns', 'account', 'portal_url', 'noc_contact', 'admin_contact')),
+        (None, ('asns', 'account', )),
     )
     nullable_fields = (
-        'asn', 'asns', 'account', 'portal_url', 'noc_contact', 'admin_contact', 'comments',
+        'asns', 'account', 'comments',
     )
 
 

netbox/circuits/forms/bulk_import.py

@@ -18,7 +18,7 @@ class ProviderCSVForm(NetBoxModelCSVForm):
     class Meta:
         model = Provider
         fields = (
-            'name', 'slug', 'asn', 'account', 'portal_url', 'noc_contact', 'admin_contact', 'comments',
+            'name', 'slug', 'account', 'comments',
         )
 
 

netbox/circuits/forms/models.py

@@ -30,29 +30,17 @@ class ProviderForm(NetBoxModelForm):
     comments = CommentField()
 
     fieldsets = (
-        ('Provider', ('name', 'slug', 'asn', 'asns', 'tags')),
+        ('Provider', ('name', 'slug', 'asns', 'tags')),
-        ('Support Info', ('account', 'portal_url', 'noc_contact', 'admin_contact')),
+        ('Support Info', ('account',)),
     )
 
     class Meta:
         model = Provider
         fields = [
-            'name', 'slug', 'asn', 'account', 'portal_url', 'noc_contact', 'admin_contact', 'asns', 'comments', 'tags',
+            'name', 'slug', 'account', 'asns', 'comments', 'tags',
         ]
-        widgets = {
-            'noc_contact': SmallTextarea(
-                attrs={'rows': 5}
-            ),
-            'admin_contact': SmallTextarea(
-                attrs={'rows': 5}
-            ),
-        }
         help_texts = {
             'name': "Full name of the provider",
-            'asn': "BGP autonomous system number (if applicable)",
-            'portal_url': "URL of the provider's customer support portal",
-            'noc_contact': "NOC email address and phone number",
-            'admin_contact': "Administrative contact email address and phone number",
         }
 
 

netbox/circuits/migrations/0040_provider_remove_deprecated_fields.py

@@ -0,0 +1,59 @@
+import os
+
+from django.db import migrations
+from django.db.utils import DataError
+
+
+def check_legacy_data(apps, schema_editor):
+    """
+    Abort the migration if any legacy provider fields still contain data.
+    """
+    Provider = apps.get_model('circuits', 'Provider')
+
+    provider_count = Provider.objects.exclude(asn__isnull=True).count()
+    if provider_count and 'NETBOX_DELETE_LEGACY_DATA' not in os.environ:
+        raise DataError(
+            f"Unable to proceed with deleting asn field from Provider model: Found {provider_count} "
+            f"providers with legacy ASN data. Please ensure all legacy provider ASN data has been "
+            f"migrated to ASN objects before proceeding. Or, set the NETBOX_DELETE_LEGACY_DATA "
+            f"environment variable to bypass this safeguard and delete all legacy provider ASN data."
+        )
+
+    provider_count = Provider.objects.exclude(admin_contact='', noc_contact='', portal_url='').count()
+    if provider_count and 'NETBOX_DELETE_LEGACY_DATA' not in os.environ:
+        raise DataError(
+            f"Unable to proceed with deleting contact fields from Provider model: Found {provider_count} "
+            f"providers with legacy contact data. Please ensure all legacy provider contact data has been "
+            f"migrated to contact objects before proceeding. Or, set the NETBOX_DELETE_LEGACY_DATA "
+            f"environment variable to bypass this safeguard and delete all legacy provider contact data."
+        )
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('circuits', '0039_unique_constraints'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=check_legacy_data,
+            reverse_code=migrations.RunPython.noop
+        ),
+        migrations.RemoveField(
+            model_name='provider',
+            name='admin_contact',
+        ),
+        migrations.RemoveField(
+            model_name='provider',
+            name='asn',
+        ),
+        migrations.RemoveField(
+            model_name='provider',
+            name='noc_contact',
+        ),
+        migrations.RemoveField(
+            model_name='provider',
+            name='portal_url',
+        ),
+    ]

netbox/circuits/models/providers.py

@@ -24,12 +24,6 @@ class Provider(NetBoxModel):
         max_length=100,
         unique=True
     )
-    asn = ASNField(
-        blank=True,
-        null=True,
-        verbose_name='ASN',
-        help_text='32-bit autonomous system number'
-    )
     asns = models.ManyToManyField(
         to='ipam.ASN',
         related_name='providers',
@@ -40,18 +34,6 @@ class Provider(NetBoxModel):
         blank=True,
         verbose_name='Account number'
     )
-    portal_url = models.URLField(
-        blank=True,
-        verbose_name='Portal URL'
-    )
-    noc_contact = models.TextField(
-        blank=True,
-        verbose_name='NOC contact'
-    )
-    admin_contact = models.TextField(
-        blank=True,
-        verbose_name='Admin contact'
-    )
     comments = models.TextField(
         blank=True
     )
@@ -62,7 +44,7 @@ class Provider(NetBoxModel):
     )
 
     clone_fields = (
-        'asn', 'account', 'portal_url', 'noc_contact', 'admin_contact',
+        'account',
     )
 
     class Meta:

netbox/circuits/tables/providers.py

@@ -41,10 +41,10 @@ class ProviderTable(NetBoxTable):
     class Meta(NetBoxTable.Meta):
         model = Provider
         fields = (
-            'pk', 'id', 'name', 'asn', 'asns', 'account', 'portal_url', 'noc_contact', 'admin_contact', 'asn_count',
+            'pk', 'id', 'name', 'asns', 'account', 'asn_count',
             'circuit_count', 'comments', 'contacts', 'tags', 'created', 'last_updated',
         )
-        default_columns = ('pk', 'name', 'asn', 'account', 'circuit_count')
+        default_columns = ('pk', 'name', 'account', 'circuit_count')
 
 
 class ProviderNetworkTable(NetBoxTable):

netbox/circuits/tests/test_api.py

@@ -20,7 +20,7 @@ class ProviderTest(APIViewTestCases.APIViewTestCase):
     model = Provider
     brief_fields = ['circuit_count', 'display', 'id', 'name', 'slug', 'url']
     bulk_update_data = {
-        'asn': 1234,
+        'account': '1234',
     }
 
     @classmethod

netbox/circuits/tests/test_filtersets.py

@@ -25,11 +25,11 @@ class ProviderTestCase(TestCase, ChangeLoggedFilterSetTests):
         ASN.objects.bulk_create(asns)
 
         providers = (
-            Provider(name='Provider 1', slug='provider-1', asn=65001, account='1234'),
+            Provider(name='Provider 1', slug='provider-1', account='1234'),
-            Provider(name='Provider 2', slug='provider-2', asn=65002, account='2345'),
+            Provider(name='Provider 2', slug='provider-2', account='2345'),
-            Provider(name='Provider 3', slug='provider-3', asn=65003, account='3456'),
+            Provider(name='Provider 3', slug='provider-3', account='3456'),
-            Provider(name='Provider 4', slug='provider-4', asn=65004, account='4567'),
+            Provider(name='Provider 4', slug='provider-4', account='4567'),
-            Provider(name='Provider 5', slug='provider-5', asn=65005, account='5678'),
+            Provider(name='Provider 5', slug='provider-5', account='5678'),
         )
         Provider.objects.bulk_create(providers)
         providers[0].asns.set([asns[0]])
@@ -82,10 +82,6 @@ class ProviderTestCase(TestCase, ChangeLoggedFilterSetTests):
         params = {'slug': ['provider-1', 'provider-2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
-    def test_asn(self):  # Legacy field
-        params = {'asn': ['65001', '65002']}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
-
     def test_asn_id(self):  # ASN object assignment
         asns = ASN.objects.all()[:2]
         params = {'asn_id': [asns[0].pk, asns[1].pk]}

netbox/circuits/tests/test_views.py

@@ -23,9 +23,9 @@ class ProviderTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         ASN.objects.bulk_create(asns)
 
         providers = (
-            Provider(name='Provider 1', slug='provider-1', asn=65001),
+            Provider(name='Provider 1', slug='provider-1'),
-            Provider(name='Provider 2', slug='provider-2', asn=65002),
+            Provider(name='Provider 2', slug='provider-2'),
-            Provider(name='Provider 3', slug='provider-3', asn=65003),
+            Provider(name='Provider 3', slug='provider-3'),
         )
         Provider.objects.bulk_create(providers)
         providers[0].asns.set([asns[0], asns[1]])
@@ -37,12 +37,8 @@ class ProviderTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         cls.form_data = {
             'name': 'Provider X',
             'slug': 'provider-x',
-            'asn': 65123,
             'asns': [asns[6].pk, asns[7].pk],
             'account': '1234',
-            'portal_url': 'http://example.com/portal',
-            'noc_contact': 'noc@example.com',
-            'admin_contact': 'admin@example.com',
             'comments': 'Another provider',
             'tags': [t.pk for t in tags],
         }
@@ -55,11 +51,7 @@ class ProviderTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         )
 
         cls.bulk_edit_data = {
-            'asn': 65009,
             'account': '5678',
-            'portal_url': 'http://example.com/portal2',
-            'noc_contact': 'noc2@example.com',
-            'admin_contact': 'admin2@example.com',
             'comments': 'New comments',
         }
 
@@ -104,8 +96,8 @@ class CircuitTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     def setUpTestData(cls):
 
         providers = (
-            Provider(name='Provider 1', slug='provider-1', asn=65001),
+            Provider(name='Provider 1', slug='provider-1'),
-            Provider(name='Provider 2', slug='provider-2', asn=65002),
+            Provider(name='Provider 2', slug='provider-2'),
         )
         Provider.objects.bulk_create(providers)
 

netbox/extras/tests/test_customvalidator.py

@@ -2,7 +2,7 @@ from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.test import TestCase, override_settings
 
-from circuits.models import Provider
+from ipam.models import ASN, RIR
 from dcim.models import Site
 from extras.validators import CustomValidator
 
@@ -67,21 +67,25 @@ custom_validator = MyValidator()
 
 class CustomValidatorTest(TestCase):
 
-    @override_settings(CUSTOM_VALIDATORS={'circuits.provider': [min_validator]})
+    @classmethod
+    def setUpTestData(cls):
+        RIR.objects.create(name='RIR 1', slug='rir-1')
+
+    @override_settings(CUSTOM_VALIDATORS={'ipam.asn': [min_validator]})
     def test_configuration(self):
-        self.assertIn('circuits.provider', settings.CUSTOM_VALIDATORS)
+        self.assertIn('ipam.asn', settings.CUSTOM_VALIDATORS)
-        validator = settings.CUSTOM_VALIDATORS['circuits.provider'][0]
+        validator = settings.CUSTOM_VALIDATORS['ipam.asn'][0]
         self.assertIsInstance(validator, CustomValidator)
 
-    @override_settings(CUSTOM_VALIDATORS={'circuits.provider': [min_validator]})
+    @override_settings(CUSTOM_VALIDATORS={'ipam.asn': [min_validator]})
     def test_min(self):
         with self.assertRaises(ValidationError):
-            Provider(name='Provider 1', slug='provider-1', asn=1).clean()
+            ASN(asn=1, rir=RIR.objects.first()).clean()
 
-    @override_settings(CUSTOM_VALIDATORS={'circuits.provider': [max_validator]})
+    @override_settings(CUSTOM_VALIDATORS={'ipam.asn': [max_validator]})
     def test_max(self):
         with self.assertRaises(ValidationError):
-            Provider(name='Provider 1', slug='provider-1', asn=65535).clean()
+            ASN(asn=65535, rir=RIR.objects.first()).clean()
 
     @override_settings(CUSTOM_VALIDATORS={'dcim.site': [min_length_validator]})
     def test_min_length(self):

netbox/templates/circuits/provider.html

@@ -19,17 +19,6 @@
             <h5 class="card-header">Provider</h5>
             <div class="card-body">
                 <table class="table table-hover attr-table">
-                    <tr>
-                      <th scope="row">ASN</th>
-                      <td>
-                        {% if object.asn %}
-                          <div class="float-end text-warning">
-                            <i class="mdi mdi-alert" title="This field will be removed in a future release. Please migrate this data to ASN objects."></i>
-                          </div>
-                        {% endif %}
-                        {{ object.asn|placeholder }}
-                      </td>
-                    </tr>
                     <tr>
                       <th scope="row">ASNs</th>
                       <td>
@@ -44,24 +33,6 @@
                         <th scope="row">Account</th>
                         <td>{{ object.account|placeholder }}</td>
                     </tr>
-                    <tr>
-                        <th scope="row">Customer Portal</th>
-                        <td>
-                            {% if object.portal_url %}
-                                <a href="{{ object.portal_url }}">{{ object.portal_url }}</a>
-                            {% else %}
-                                {{ ''|placeholder }}
-                            {% endif %}
-                        </td>
-                    </tr>
-                    <tr>
-                        <th scope="row">NOC Contact</th>
-                        <td>{{ object.noc_contact|markdown|placeholder }}</td>
-                    </tr>
-                    <tr>
-                        <th scope="row">Admin Contact</th>
-                        <td>{{ object.admin_contact|markdown|placeholder }}</td>
-                    </tr>
                     <tr>
                         <th scope="row">Circuits</th>
                         <td>

netbox/utilities/tests/test_filters.py

@@ -5,8 +5,6 @@ from django.test import TestCase
 from mptt.fields import TreeForeignKey
 from taggit.managers import TaggableManager
 
-from circuits.filtersets import CircuitFilterSet, ProviderFilterSet
-from circuits.models import Circuit, Provider
 from dcim.choices import *
 from dcim.fields import MACAddressField
 from dcim.filtersets import DeviceFilterSet, SiteFilterSet
@@ -15,6 +13,7 @@ from dcim.models import (
 )
 from extras.filters import TagFilter
 from extras.models import TaggedItem
+from ipam.filtersets import ASNFilterSet
 from ipam.models import RIR, ASN
 from netbox.filtersets import BaseFilterSet
 from utilities.filters import (
@@ -338,13 +337,14 @@ class DynamicFilterLookupExpressionTest(TestCase):
     """
     @classmethod
     def setUpTestData(cls):
+        rir = RIR.objects.create(name='RIR 1', slug='rir-1')
 
-        providers = (
+        asns = (
-            Provider(name='Provider 1', slug='provider-1', asn=65001),
+            ASN(asn=65001, rir=rir),
-            Provider(name='Provider 2', slug='provider-2', asn=65101),
+            ASN(asn=65101, rir=rir),
-            Provider(name='Provider 3', slug='provider-3', asn=65201),
+            ASN(asn=65201, rir=rir),
         )
-        Provider.objects.bulk_create(providers)
+        ASN.objects.bulk_create(asns)
 
         manufacturers = (
             Manufacturer(name='Manufacturer 1', slug='manufacturer-1'),
@@ -389,15 +389,6 @@ class DynamicFilterLookupExpressionTest(TestCase):
         )
         Site.objects.bulk_create(sites)
 
-        rir = RIR.objects.create(name='RFC 6996', is_private=True)
-
-        asns = [
-            ASN(asn=65001, rir=rir),
-            ASN(asn=65101, rir=rir),
-            ASN(asn=65201, rir=rir)
-        ]
-        ASN.objects.bulk_create(asns)
-
         asns[0].sites.add(sites[0])
         asns[1].sites.add(sites[1])
         asns[2].sites.add(sites[2])
@@ -456,19 +447,19 @@ class DynamicFilterLookupExpressionTest(TestCase):
 
     def test_provider_asn_lt(self):
         params = {'asn__lt': [65101]}
-        self.assertEqual(ProviderFilterSet(params, Provider.objects.all()).qs.count(), 1)
+        self.assertEqual(ASNFilterSet(params, ASN.objects.all()).qs.count(), 1)
 
     def test_provider_asn_lte(self):
         params = {'asn__lte': [65101]}
-        self.assertEqual(ProviderFilterSet(params, Provider.objects.all()).qs.count(), 2)
+        self.assertEqual(ASNFilterSet(params, ASN.objects.all()).qs.count(), 2)
 
     def test_provider_asn_gt(self):
         params = {'asn__lt': [65101]}
-        self.assertEqual(ProviderFilterSet(params, Provider.objects.all()).qs.count(), 1)
+        self.assertEqual(ASNFilterSet(params, ASN.objects.all()).qs.count(), 1)
 
     def test_provider_asn_gte(self):
         params = {'asn__gte': [65101]}
-        self.assertEqual(ProviderFilterSet(params, Provider.objects.all()).qs.count(), 2)
+        self.assertEqual(ASNFilterSet(params, ASN.objects.all()).qs.count(), 2)
 
     def test_site_region_negation(self):
         params = {'region__n': ['region-1']}