New field: PR.provider, decide whether to update an existing provider…

… or create a new one

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",
+            ),
+        ),
+    ]

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:

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()))

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