diff --git a/apps/accounts/migrations/0063_provider_request_related_provider.py b/apps/accounts/migrations/0063_provider_request_related_provider.py new file mode 100644 index 00000000..f03dfe65 --- /dev/null +++ b/apps/accounts/migrations/0063_provider_request_related_provider.py @@ -0,0 +1,22 @@ +# Generated by Django 3.2.21 on 2023-09-24 21:00 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("accounts", "0062_merge_20230921_1230"), + ] + + operations = [ + migrations.AddField( + model_name="providerrequest", + name="provider", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="accounts.hostingprovider", + ), + ), + ] diff --git a/apps/accounts/models/hosting.py b/apps/accounts/models/hosting.py index fd76811d..1a8ac8cd 100644 --- a/apps/accounts/models/hosting.py +++ b/apps/accounts/models/hosting.py @@ -294,9 +294,12 @@ class Hostingprovider(models.Model): through_fields=("hostingprovider", "datacenter"), related_name="hostingproviders", ) - # prevent circular imports by lazy-loading the ProviderRequest model + # link to a provider request that was recently used to create or update the data request = models.ForeignKey( - "accounts.ProviderRequest", null=True, on_delete=models.SET_NULL + # prevent circular imports by lazy-loading the ProviderRequest model + "accounts.ProviderRequest", + null=True, + on_delete=models.SET_NULL, ) def __str__(self): @@ -396,7 +399,6 @@ def website_link(self) -> str: else: return f"https://{self.website}" - # Mutators def refresh_shared_secret(self) -> str: try: diff --git a/apps/accounts/models/provider_request.py b/apps/accounts/models/provider_request.py index 44eee626..d76ba1c1 100644 --- a/apps/accounts/models/provider_request.py +++ b/apps/accounts/models/provider_request.py @@ -107,6 +107,10 @@ class ProviderRequest(TimeStampedModel): newsletter_opt_in = models.BooleanField( default=False, verbose_name="Newsletter signup" ) + # if this field is set, approving a request will update the provider instead of creating a new one + provider = models.ForeignKey( + to=Hostingprovider, on_delete=models.SET_NULL, null=True + ) def __str__(self) -> str: return f"{self.name}" @@ -151,7 +155,8 @@ def get_service_choices(cls) -> List[Tuple[int, str]]: @transaction.atomic def approve(self) -> Hostingprovider: """ - Create a new Hostingprovider and underlying objects and set appropriate permissions. + Create a new Hostingprovider and underlying objects or update an existing one + and set appropriate permissions. This method is defined as an atomic transaction: in case any exception occurs, all changes will be rolled back, @@ -173,8 +178,8 @@ def approve(self) -> Hostingprovider: existing_hp = Hostingprovider.objects.filter(request=self) if existing_hp.exists(): raise ValueError( - f"{failed_msg} because a related hosting provider '{existing_hp.get()}'" - "already exists in the database" + f"{failed_msg} an existing hosting provider '{existing_hp.get()}'" + "was already updated using the data from this verification request" ) # Temporarily use only the first location @@ -183,17 +188,35 @@ def approve(self) -> Hostingprovider: if not first_location: raise ValueError(f"{failed_msg} because there are no locations provided") - # create a Hostingprovider and assign it to the user who created ProviderRequest - hp = Hostingprovider.objects.create( - name=self.name, - description=self.description, - # set the first location from the list - country=first_location.country, - city=first_location.city, - website=self.website, - request=self, - created_by=self.created_by, - ) + # decide whether to update an existing provider or create a new one + if self.provider: + hp = Hostingprovider.objects.get(pk=self.provider.id) + + # delete related objects, they will be recreated with recent data + hp.services.clear() + + for asn in hp.greencheckasn_set.all(): + asn.delete() + + for ip_range in hp.greencheckip_set.all(): + ip_range.delete() + + for doc in hp.supporting_documents.all(): + doc.delete() + + else: + hp = Hostingprovider.objects.crete() + self.provider = hp + + # fill in data from this request + hp.name = self.name + hp.description = self.description + # set the first location from the list + hp.country = first_location.country + hp.city = first_location.city + hp.website = self.website + hp.request = self + hp.created_by = self.created_by # set services (https://django-taggit.readthedocs.io/en/latest/api.html) hp.services.set(list(self.services.all())) diff --git a/apps/accounts/tests/test_provider_request.py b/apps/accounts/tests/test_provider_request.py index b4637d9a..7cd183d0 100644 --- a/apps/accounts/tests/test_provider_request.py +++ b/apps/accounts/tests/test_provider_request.py @@ -541,6 +541,42 @@ def test_approve_creates_hosting_provider(): assert "other-none" not in hp.services.slugs() +@pytest.mark.django_db +def test_approve_updates_existing_provider(hosting_provider_with_sample_user): + # given: a provider request linked to an existing hosting provider + pr = ProviderRequestFactory.create( + services=faker.words(nb=4), provider=hosting_provider_with_sample_user + ) + ProviderRequestLocationFactory.create(request=pr) + ProviderRequestEvidenceFactory.create(request=pr) + + # when: the request is approved + result = pr.approve() + hp = models.Hostingprovider.objects.get(id=result.id) + + # then: resulting Hostingprovider is the one linked to the original request + assert hp.id == pr.provider.id + + # then: resulting Hostingprovider is configured properly + assert hp.name == pr.name + assert hp.description == pr.description + assert list(hp.services.all()) == list(pr.services.all()) + assert hp.website == pr.website + assert hp.request == pr + assert hp.created_by == pr.created_by + # then: user who created the request has permissions to manage the new hosting provider + assert hp in pr.created_by.hosting_providers + + # provider is visible by default + # appropriate tag is added + assert hp.showonwebsite is True + assert "up-to-date" in hp.staff_labels.slugs() + # "other-none" is the label condition check for + # when someone is just trying to get a site marked + # as green when they don't offer hosted services + assert "other-none" not in hp.services.slugs() + + @pytest.mark.django_db def test_approve_supports_orgs_not_offering_hosted_services(): # given: a verification request for an organisation that does