From 31a0f886a766b1a39d0a8be129b4426d699c94ee Mon Sep 17 00:00:00 2001 From: Matt Griswold Date: Tue, 18 Aug 2020 09:30:40 -0500 Subject: [PATCH] Dotf fixes 3 (#820) * [beta] IX-F importer: [showstopper]: Seeing 'Update' hint instead of 'Add' hint #816 * [beta] IX-F importer: list of hints should be case-independently sorted alphabetically #817 * [beta] IX-F importer: IX "Last Updated" timestamp is based on IX-F import time - not sure that is intended #812 * Improve soft delete error handling re: protectedactions * Fix template bug * Modify template again * add a test to confirm ipaddr suggestions end up in template * rebase #817 into this and adapt to changes in #817 * [beta] IX-F importer: notice about dismissed hints remains after no longer relevant #809 * Server Error 500 when deleting organisations #798 * black formatting Co-authored-by: Stefan Pratter Co-authored-by: egfrank --- peeringdb_server/admin.py | 22 +- peeringdb_server/ixf.py | 21 +- peeringdb_server/models.py | 28 ++- .../site/view_network_ixf_suggestions.html | 15 +- peeringdb_server/views.py | 2 +- tests/data/ixf/views/import.json | 102 +++++++++ tests/test_admin.py | 29 +++ tests/test_ixf_member_import_protocol.py | 85 +++++++- tests/test_ixf_member_views.py | 206 ++++++++++++++++-- 9 files changed, 462 insertions(+), 48 deletions(-) create mode 100644 tests/data/ixf/views/import.json diff --git a/peeringdb_server/admin.py b/peeringdb_server/admin.py index bf6a6322..15e94936 100644 --- a/peeringdb_server/admin.py +++ b/peeringdb_server/admin.py @@ -71,6 +71,7 @@ from peeringdb_server.models import ( DeskProTicket, IXFImportEmail, EnvironmentSetting, + ProtectedAction, ) from peeringdb_server.mail import mail_users_entity_merge from peeringdb_server.inet import RdapLookup, RdapException @@ -254,6 +255,20 @@ class StatusForm(baseForms.ModelForm): elif inst.status == "deleted": self.fields["status"].choices = [("ok", "ok"), ("deleted", "deleted")] + def clean(self): + + """ + Catches and raises validation errors where an object + is to be soft-deleted but cannot be because it is currently + protected. + """ + + if self.cleaned_data.get("DELETE"): + if self.instance and hasattr(self.instance, "deletable"): + if not self.instance.deletable: + self.cleaned_data["DELETE"] = False + raise ValidationError(self.instance.not_deletable_reason) + class ModelAdminWithUrlActions(admin.ModelAdmin): def make_redirect(self, obj, action): @@ -323,7 +338,11 @@ def soft_delete(modeladmin, request, queryset): return for row in queryset: - row.delete() + try: + row.delete() + except ProtectedAction as err: + messages.error(request, _("Protected object '{}': {}").format(row, err)) + continue soft_delete.short_description = _("SOFT DELETE") @@ -424,6 +443,7 @@ class IXLanPrefixForm(StatusForm): return value def clean(self): + super().clean() if self.prefix_changed and not self.instance.deletable: raise ValidationError(self.instance.not_deletable_reason) diff --git a/peeringdb_server/ixf.py b/peeringdb_server/ixf.py index e7f87f36..748b4d94 100644 --- a/peeringdb_server/ixf.py +++ b/peeringdb_server/ixf.py @@ -431,7 +431,6 @@ class Importer: return True - @reversion.create_revision() def update_ix(self): """ @@ -454,17 +453,25 @@ class Importer: updated__gte=self.now, ixlan=self.ixlan ).exists() - if ixf_member_data_changed or netixlan_data_changed: - ix.ixf_last_import = self.now - save_ix = True + ix.ixf_last_import = self.now ixf_net_count = len(self.pending_save) if ixf_net_count != ix.ixf_net_count: ix.ixf_net_count = ixf_net_count - save_ix = True - if save_ix: - ix.save() + # we do not want these updates to affect the + # exchanges `updated` timestamp as per #812 + # so we temporarily disable auto_now + + ix._meta.get_field("updated").auto_now = False + try: + with reversion.create_revision(): + ix.save() + finally: + + # always turn auto_now back on afterwards + + ix._meta.get_field("updated").auto_now = True def fix_consolidated_modify(self, ixf_member_data): """ diff --git a/peeringdb_server/models.py b/peeringdb_server/models.py index fe589184..c62b7f54 100644 --- a/peeringdb_server/models.py +++ b/peeringdb_server/models.py @@ -2508,16 +2508,24 @@ class IXFMemberData(pdb_models.NetworkIXLanBase): """ qset = cls.get_for_network(net).select_related("ixlan", "ixlan__ix") qset = qset.filter(dismissed=True) + return qset - dismissed = {} + @classmethod + def network_has_dismissed_actionable(cls, net): + """ + Returns whether or not the specified network has + any dismissed IXFMemberData suggestions that are + actionable - for ixf_member_data in qset: + Argument(s): - ix = ixf_member_data.ix - if ix.id not in dismissed: - dismissed[ix.id] = ix + - net(Network) + """ - return dismissed + for ixf_member_data in cls.dismissed_for_network(net): + if ixf_member_data.action != "noop": + return True + return False @classmethod def proposals_for_network(cls, net): @@ -2577,7 +2585,7 @@ class IXFMemberData(pdb_models.NetworkIXLanBase): proposals[ix_id][action].append(ixf_member_data) - return proposals + return sorted(proposals.values(), key=lambda x: x["ix"].name.lower()) @property def previous_data(self): @@ -2927,7 +2935,11 @@ class IXFMemberData(pdb_models.NetworkIXLanBase): # # action re-classified to modify (#770) if action == "add" and self.has_requirements: - action = "modify" + if ( + self.primary_requirement.asn == self.asn + and self.primary_requirement.action == "delete" + ): + action = "modify" return action diff --git a/peeringdb_server/templates/site/view_network_ixf_suggestions.html b/peeringdb_server/templates/site/view_network_ixf_suggestions.html index 56be843f..e836a54f 100644 --- a/peeringdb_server/templates/site/view_network_ixf_suggestions.html +++ b/peeringdb_server/templates/site/view_network_ixf_suggestions.html @@ -49,7 +49,9 @@ {% endif %} -{% for ix_id, ixf_proposals in data.ixf.items %} +{% for ixf_proposals in data.ixf %} +{% with ix_id=ixf_proposals.ix.id %} +