diff --git a/apps/accounts/admin.py b/apps/accounts/admin.py index 9a5a5357..8c299748 100644 --- a/apps/accounts/admin.py +++ b/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 diff --git a/apps/accounts/models/hosting.py b/apps/accounts/models/hosting.py index 954d2a8f..13604674 100644 --- a/apps/accounts/models/hosting.py +++ b/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 diff --git a/apps/accounts/models/test_models.py b/apps/accounts/models/test_models.py index e2858aae..f632faf5 100644 --- a/apps/accounts/models/test_models.py +++ b/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: """ diff --git a/apps/accounts/tests/test_admin.py b/apps/accounts/tests/test_admin.py index 309bd0d6..3f5ffb7d 100644 --- a/apps/accounts/tests/test_admin.py +++ b/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: diff --git a/conftest.py b/conftest.py index 6606c2c3..9bec848a 100644 --- a/conftest.py +++ b/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)