From 38a49b4a105ea8b6d0399883adfebf4582d0d808 Mon Sep 17 00:00:00 2001 From: Matt Griswold Date: Tue, 18 Aug 2020 09:28:18 -0500 Subject: [PATCH] Gh 761 (#819) * Make in_dfz a readonly field * add migrations for in_dfz noop, add validation error when trying to set in_dfz to false over api fix template layout * remove in_dfz from ixlanprefixinline admin * fix error message * black formatted Co-authored-by: egfrank Co-authored-by: Stefan Pratter --- peeringdb_server/admin.py | 3 +- .../management/commands/pdb_api_test.py | 68 +++++++++++-------- .../migrations/0052_deactivate_in_dfz.py | 27 ++++++++ peeringdb_server/models.py | 4 ++ peeringdb_server/serializers.py | 20 +++++- .../templates/site/view_exchange_bottom.html | 20 ++---- 6 files changed, 97 insertions(+), 45 deletions(-) create mode 100644 peeringdb_server/migrations/0052_deactivate_in_dfz.py diff --git a/peeringdb_server/admin.py b/peeringdb_server/admin.py index 4cef9889..bf6a6322 100644 --- a/peeringdb_server/admin.py +++ b/peeringdb_server/admin.py @@ -432,6 +432,7 @@ class IXLanPrefixInline(SanitizedAdmin, admin.TabularInline): model = IXLanPrefix extra = 0 form = IXLanPrefixForm + fields = ["status", "protocol", "prefix"] class IXLanInline(SanitizedAdmin, admin.StackedInline): @@ -988,7 +989,7 @@ class InternetExchangeFacilityAdmin(SoftDeleteAdmin): class IXLanPrefixAdmin(SoftDeleteAdmin): list_display = ("id", "prefix", "ixlan", "ix", "status", "created", "updated") - readonly_fields = ("ix", "id") + readonly_fields = ("ix", "id", "in_dfz") search_fields = ("ixlan__name", "ixlan__ix__name", "prefix") list_filter = (StatusFilter,) form = IXLanPrefixForm diff --git a/peeringdb_server/management/commands/pdb_api_test.py b/peeringdb_server/management/commands/pdb_api_test.py index a5108b81..4118c592 100644 --- a/peeringdb_server/management/commands/pdb_api_test.py +++ b/peeringdb_server/management/commands/pdb_api_test.py @@ -384,7 +384,7 @@ class TestJSON(unittest.TestCase): "ixlan_id": SHARED["ixlan_r_ok"].id, "protocol": "IPv4", "prefix": "10.%d.10.0/23" % (self.PREFIX_COUNT + 1), - "in_dfz": False, + "in_dfz": True, } if "prefix" not in kwargs: self.PREFIX_COUNT += 1 @@ -559,19 +559,19 @@ class TestJSON(unittest.TestCase): # we test fail because of invalid data if "invalid" in test_failures: - data_invalid = copy.copy(data) - for k, v in list(test_failures["invalid"].items()): - data_invalid[k] = v - with pytest.raises(InvalidRequestException) as excinfo: - r = db.create(typ, data_invalid, return_response=True) - # FIXME - # The following lines will not be called since - # the InvalidRequestException is raised - # in the previous line - for k, v in list(test_failures["invalid"].items()): - self.assertIn(k, list(r.keys())) - assert "400 Bad Request" in str(excinfo.value) + tests = test_failures["invalid"] + if not isinstance(tests, list): + tests = [tests] + + for test in tests: + data_invalid = copy.copy(data) + for k, v in list(test.items()): + data_invalid[k] = v + + with pytest.raises(InvalidRequestException) as excinfo: + r = db.create(typ, data_invalid, return_response=True) + assert "400 Bad Request" in str(excinfo.value) # we test fail because of parent entity status if "status" in test_failures: @@ -636,13 +636,26 @@ class TestJSON(unittest.TestCase): # we test fail because of invalid data if "invalid" in test_failures: - data_invalid = copy.copy(orig) - for k, v in list(test_failures["invalid"].items()): - data_invalid[k] = v - with pytest.raises(InvalidRequestException) as excinfo: - db.update(typ, **data_invalid) - assert "400 Bad Request" in str(excinfo.value) + tests = test_failures["invalid"] + + # `invalid` test_failures can be a list to + # test multiple instances of invalid values + # however we also support passing a single + # dict of fields, in which case we wrap + # it in a list here. + + if not isinstance(tests, list): + tests = [tests] + + for test in tests: + data_invalid = copy.copy(orig) + for k, v in list(test.items()): + data_invalid[k] = v + + with pytest.raises(InvalidRequestException) as excinfo: + db.update(typ, **data_invalid) + assert "400 Bad Request" in str(excinfo.value) # we test fail because of permissions if "perms" in test_failures: @@ -1513,7 +1526,10 @@ class TestJSON(unittest.TestCase): "ixpfx", data, test_failures={ - "invalid": {"prefix": "127.0.0.0/8"}, + "invalid": [ + {"prefix": "127.0.0.0/8"}, + {"in_dfz": False} + ], "perms": { "prefix": "205.127.237.0/24", "ixlan_id": SHARED["ixlan_r_ok"].id, @@ -1527,19 +1543,16 @@ class TestJSON(unittest.TestCase): SHARED["ixpfx_id"] = r_data["id"] - # self.assert_create(self.db_org_admin, "ixpfx", data, test_failures={ - # "invalid": { - # "prefix": "206.126.236.0/25" - # }, - # }, test_success=False) - self.assert_update( self.db_org_admin, "ixpfx", SHARED["ixpfx_id"], {"prefix": "206.127.236.0/26"}, test_failures={ - "invalid": {"prefix": "NEEDS TO BE VALID PREFIX"}, + "invalid": [ + {"prefix": "NEEDS TO BE VALID PREFIX"}, + {"in_dfz": False} + ], "perms": {"ixlan_id": SHARED["ixlan_r_ok"].id}, }, ) @@ -1592,6 +1605,7 @@ class TestJSON(unittest.TestCase): self.db_org_admin, "ixpfx", test_protected=SHARED["ixpfx_id"] ) + ########################################################################## def test_org_admin_002_POST_PUT_DELETE_netixlan(self): diff --git a/peeringdb_server/migrations/0052_deactivate_in_dfz.py b/peeringdb_server/migrations/0052_deactivate_in_dfz.py new file mode 100644 index 00000000..2746baee --- /dev/null +++ b/peeringdb_server/migrations/0052_deactivate_in_dfz.py @@ -0,0 +1,27 @@ +# Generated by Django 2.2.14 on 2020-08-18 07:09 + +from django.db import migrations, models + +def noop_in_dfz(apps, schema_editor): + + IXLanPrefix = apps.get_model("peeringdb_server", "IXLanPrefix") + + for pfx in IXLanPrefix.handleref.filter(in_dfz=False): + pfx.in_dfz = True + pfx.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('peeringdb_server', '0051_auto_20200730_1622'), + ] + + operations = [ + migrations.AlterField( + model_name='ixlanprefix', + name='in_dfz', + field=models.BooleanField(default=True), + ), + migrations.RunPython(noop_in_dfz), + ] diff --git a/peeringdb_server/models.py b/peeringdb_server/models.py index 7041fe50..fe589184 100644 --- a/peeringdb_server/models.py +++ b/peeringdb_server/models.py @@ -3409,6 +3409,10 @@ class IXLanPrefix(ProtectedMixin, pdb_models.IXLanPrefixBase): IXLan, on_delete=models.CASCADE, default=0, related_name="ixpfx_set" ) + # override in_dfz to default it to True on the schema level (#761) + + in_dfz = models.BooleanField(default=True) + @property def descriptive_name(self): """ diff --git a/peeringdb_server/serializers.py b/peeringdb_server/serializers.py index 38b69bad..e352d429 100644 --- a/peeringdb_server/serializers.py +++ b/peeringdb_server/serializers.py @@ -1904,6 +1904,7 @@ class IXLanPrefixSerializer(ModelSerializer): validate_prefix_overlap, ] ) + in_dfz = serializers.SerializerMethodField(read_only=False) class Meta: model = IXLanPrefix @@ -1920,6 +1921,10 @@ class IXLanPrefixSerializer(ModelSerializer): list_exclude = ["ixlan"] + @staticmethod + def get_in_dfz(obj): + return True + @classmethod def prepare_query(cls, qset, **kwargs): filters = get_relation_filters(["ix_id", "ix", "whereis"], cls, **kwargs) @@ -1974,13 +1979,26 @@ class IXLanPrefixSerializer(ModelSerializer): "Prefix netmask invalid, needs to be valid according to the selected protocol" ) + # The implementation of #761 has deprecated the in_dfz + # property as a writeable setting, if someone tries + # to actively set it to `False` let them know it is no + # longer supported + + if self.initial_data.get("in_dfz", True) == False: + raise serializers.ValidationError( + _( + "The `in_dfz` property has been deprecated " + "and setting it to `False` is no " + "longer supported" + ) + ) + if self.instance: prefix = data["prefix"] if prefix != self.instance.prefix and not self.instance.deletable: raise serializers.ValidationError( {"prefix": self.instance.not_deletable_reason} ) - return data diff --git a/peeringdb_server/templates/site/view_exchange_bottom.html b/peeringdb_server/templates/site/view_exchange_bottom.html index 591f0eab..d357d918 100644 --- a/peeringdb_server/templates/site/view_exchange_bottom.html +++ b/peeringdb_server/templates/site/view_exchange_bottom.html @@ -14,12 +14,9 @@
{% trans "Protocol" %}
-
+
{% trans "Prefix" %}
-
-
{% trans "In DFZ" %}
-
{% with x=instance.ixlan %} @@ -40,21 +37,14 @@ {% endif %} {{ prefix.protocol }} -
{{ prefix.prefix }}
-
- -
- + {% endfor %} {% if permissions.can_create %} @@ -236,6 +226,4 @@ -{% endif %} - - +{% endif %} \ No newline at end of file