Add tests for deactivating networks for archived providers

apps/accounts/admin.py

@@ -649,6 +649,23 @@ def send_email(self, request, *args, **kwargs):
 
     # Mutators
 
+    def save_model(self, request, obj, form, change):
+        """
+        Extra actions in response to specific options being set on the main model go here.
+        Note: this will not catch changes made to inlines. For that see the overrides in
+        the save_formset()
+
+        https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.save_model
+        """
+
+        # if the provider is being archived, deactivate all networks to avoid having to manually
+        # do this for each network
+        breakpoint()
+        if "archived" in form.changed_data and not obj.archived:
+            obj.deactivate_networks()
+
+        super().save_model(request, obj, form, change)
+
     def save_formset(self, request, form, formset, change):
         """
         Save the child objects in this form, and account for the special cases
@@ -946,17 +963,6 @@ def _changeform_view(self, request, object_id, form_url, extra_context):
         except self.model.DoesNotExist:
             logger.warning(f"Could not find provider with the id {object_id}")
 
-        if request.method == "POST":
-            # "archived" only appears in the payload if the box is checked
-            # in an admin form submission
-            archive_in_payload = "archived" in request.POST
-
-            # we only trigger this if a provider isn't already archived
-            # to avoid making needless queries
-            if archive_in_payload and not instance.archived:
-                # deactivate IP ranges and ASNs
-                # instance.deactivate_networks()
-
         return super()._changeform_view(request, object_id, form_url, extra_context)
 
     @mark_safe
@@ -1196,9 +1202,9 @@ def change_view(self, request, object_id, form_url="", extra_context=None):
 
             if associated_providers_count:
                 extra_context["associated_providers_count"] = associated_providers_count
-                extra_context[
-                    "associated_providers"
-                ] = datacentre.hostingproviders.all()
+                extra_context["associated_providers"] = (
+                    datacentre.hostingproviders.all()
+                )
                 extra_context["dc_has_providers"] = True
 
         return super().change_view(
@@ -1408,9 +1414,9 @@ def send_approval_email(
 
         # For a new provider request, use this subject line
         subject = (
-                f"Verification request to the Green Web Dataset is approved: "
-                f"{mark_safe(provider_request.name)}"
-            )
+            f"Verification request to the Green Web Dataset is approved: "
+            f"{mark_safe(provider_request.name)}"
+        )
 
         if existing_provider:
             context["provider"] = provider_request.provider

apps/accounts/models/hosting.py

@@ -474,7 +474,7 @@ def deactivate_networks(self):
         active_green_ips = self.greencheckip_set.filter(active=True)
         active_green_asns = self.greencheckasn_set.filter(active=True)
         active_green_ips.update(active=False)
-        active_green_asns(active=False)
+        active_green_asns.update(active=False)
 
     # Queries
 

apps/accounts/models/test_models.py

@@ -62,6 +62,28 @@ def test_accepted_ways_to_model(
 
         assert datacenter.model == accounting_model
 
+    def test_deactivate_networks(
+        self, db, hosting_provider_factory, green_ip_factory, green_asn_factory
+    ):
+
+        provider = hosting_provider_factory.create()
+        # make a green ip range
+        ip_range = green_ip_factory.create(hostingprovider=provider)
+        # make a green asn range
+        as_network = green_asn_factory.create(hostingprovider=provider)
+
+        assert ip_range.active is True
+        assert as_network.active is True
+
+        provider.deactivate_networks()
+        ip_range.refresh_from_db()
+        as_network.refresh_from_db()
+
+        assert provider.active_ip_ranges().count() == 0
+        assert provider.active_asns().count() == 0
+        assert ip_range.active is False
+        assert as_network.active is False
+
 
 class TestHostingProviderEvidence:
     """

apps/accounts/tests/test_admin.py

@@ -333,54 +333,6 @@ def test_log_created_email_for_user_with_provider_as_note(
         assert len(labels) == 1
         assert labels[0].name == "welcome-email sent"
 
-    @pytest.mark.parametrize(
-        "archived",
-        (
-            (True, 0),
-            (False, 1),
-        ),
-    )
-    def test_archived_providers_are_hidden_by_default(
-        self, db, client, hosting_provider_with_sample_user, archived
-    ):
-        """Test that by default, archived users to not show up in our listings."""
-
-        hosting_provider_with_sample_user.archived = archived[0]
-        hosting_provider_with_sample_user.save()
-        client.force_login(hosting_provider_with_sample_user.users.first())
-
-        admin_url = urls.reverse("greenweb_admin:accounts_hostingprovider_changelist")
-        resp = client.get(admin_url, follow=True)
-
-        assert len(resp.context["results"]) == archived[1]
-        assert resp.status_code == 200
-
-    @pytest.mark.skip(reason="we handle this with a list filter option now")
-    @pytest.mark.parametrize(
-        "archived",
-        (
-            (True, 1),
-            (False, 0),
-        ),
-    )
-    def test_archived_providers_hidden_by_seen_with_override_params(
-        self, db, client, hosting_provider_with_sample_user, archived
-    ):
-        """
-        If we really need to see archived users, we can with a
-        special GET param, to override our view
-        """
-
-        hosting_provider_with_sample_user.archived = archived[0]
-        hosting_provider_with_sample_user.save()
-        client.force_login(hosting_provider_with_sample_user.users.first())
-
-        admin_url = urls.reverse("greenweb_admin:accounts_hostingprovider_changelist")
-        resp = client.get(admin_url, {"archived": True}, follow=True)
-
-        assert len(resp.context["results"]) == archived[1]
-        assert resp.status_code == 200
-
     def test_list_of_users_who_can_manage_provider_is_displayed(
         self, db, sample_hoster_user, greenweb_staff_user, hosting_provider
     ):
@@ -463,23 +415,56 @@ def test_regular_users_cannot_access_permissions_management(
                 str(sample_hoster_user.groups.first().id),
             )
 
-    def test_archiving_a_provider_also_deactivates_their_networks(
-        self, db, hosting_provider, sample_hoster_user, greenweb_staff_user
+
+class TestArchivingHostingProviderAdmin:
+
+    @pytest.mark.parametrize(
+        "archived",
+        (
+            (True, 0),
+            (False, 1),
+        ),
+    )
+    def test_archived_providers_are_hidden_by_default(
+        self, db, client, hosting_provider_with_sample_user, archived
+    ):
+        """Test that by default, archived users do not show up in our listings."""
+
+        hosting_provider_with_sample_user.archived = archived[0]
+        hosting_provider_with_sample_user.save()
+        client.force_login(hosting_provider_with_sample_user.users.first())
+
+        admin_url = urls.reverse("greenweb_admin:accounts_hostingprovider_changelist")
+        resp = client.get(admin_url, follow=True)
+
+        assert len(resp.context["results"]) == archived[1]
+        assert resp.status_code == 200
+
+    @pytest.mark.skip(reason="we handle this with a list filter option now")
+    @pytest.mark.parametrize(
+        "archived",
+        (
+            (True, 1),
+            (False, 0),
+        ),
+    )
+    def test_archived_providers_hidden_by_seen_with_override_params(
+        self, db, client, hosting_provider_with_sample_user, archived
     ):
         """
-        Archiving a provider should also deactivate their networks
-        without staff needing to do this manually.
+        If we really need to see archived users, we can with a
+        special GET param, to override our view
         """
-        hosting_provider.save()
 
-        # given: a provider has been created, with ASNs and IP ranges
-        # allocated to them
-
-        # when: I update the provider marking them as archived
+        hosting_provider_with_sample_user.archived = archived[0]
+        hosting_provider_with_sample_user.save()
+        client.force_login(hosting_provider_with_sample_user.users.first())
 
-        # then: the ASNs and IP ranges are also deactivated
+        admin_url = urls.reverse("greenweb_admin:accounts_hostingprovider_changelist")
+        resp = client.get(admin_url, {"archived": True}, follow=True)
 
-        pass
+        assert len(resp.context["results"]) == archived[1]
+        assert resp.status_code == 200
 
 
 class TestUserAdmin:

conftest.py

@@ -25,6 +25,7 @@
 register(gc_factories.ServiceFactory)
 register(gc_factories.HostingProviderFactory)
 register(gc_factories.GreenIpFactory)
+register(gc_factories.GreenASNFactory)
 register(gc_factories.GreenDomainFactory)
 register(gc_factories.DailyStatFactory)
 register(gc_factories.SiteCheckFactory)