#1264 Approve a deleted domain name - [RH] (#3718)

* Biz logic attempt

* Adding documentation and some notes

* Testing migration fixes for testing

* Holding fix so we dont trigger migrations

* Testing onto sandbox

* Fix linter issues

* Statement check to see if it errors

* Add the exclude deleted check

* Checking for not deleted

* Retry updating the constraint

* Try a different check

* Lets try not using the constraint and just have a check instead

* try this transitional fix

* just do the clean method in save for deleted

* Add print statements

* Add in UniqueConstraint check back

* Add in tests

* Add in not deleted check

* Add unit tests for isnotdeleted remove comment clean up and more

* Add code I accidentally removed

* Accidentally removed a check

* Fix test naming

src/registrar/admin.py

@@ -3229,7 +3229,11 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             and obj.status == models.DomainRequest.DomainRequestStatus.APPROVED
             and original_obj.requested_domain is not None
             and Domain.objects.filter(name=original_obj.requested_domain.name).exists()
+            and Domain.is_not_deleted(domain_name)
         ):
+            # NOTE: We want to allow it to be approved again if it's already deleted
+            # So we want to exclude deleted
+
             # REDUNDANT CHECK:
             # This action (approving a request when the domain exists)
             # would still not go through even without this check as the rules are

src/registrar/migrations/0148_alter_domain_name_domain_unique_name_except_deleted.py

@@ -0,0 +1,29 @@
+# Generated by Django 4.2.20 on 2025-05-14 16:40
+
+from django.db import migrations, models
+import registrar.models.utility.domain_field
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("registrar", "0147_alter_hostip_options"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="domain",
+            name="name",
+            field=registrar.models.utility.domain_field.DomainField(
+                default=None, help_text="Fully qualified domain name", max_length=253, verbose_name="domain"
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="domain",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("state", "deleted"), _negated=True),
+                fields=("name",),
+                name="unique_name_except_deleted",
+            ),
+        ),
+    ]

src/registrar/models/domain.py

@@ -5,9 +5,8 @@ import re
 import time
 from datetime import date, timedelta
 from typing import Optional
-from django.db import transaction
+from django.db import transaction, models, IntegrityError
 from django_fsm import FSMField, transition, TransitionNotAllowed  # type: ignore
-from django.db import models, IntegrityError
 from django.utils import timezone
 from typing import Any
 from registrar.models.domain_invitation import DomainInvitation
@@ -35,6 +34,7 @@ from epplibwrapper import (
 from registrar.models.utility.contact_error import ContactError, ContactErrorCodes
 
 from django.db.models import DateField, TextField
+
 from .utility.domain_field import DomainField
 from .utility.domain_helper import DomainHelper
 from .utility.time_stamped_model import TimeStampedModel
@@ -76,6 +76,14 @@ class Domain(TimeStampedModel, DomainHelper):
             models.Index(fields=["state"]),
         ]
 
+        # Domain name must be unique across all non-deletd domains
+        # If domain is in deleted state, its name can be reused - submitted/approved
+        constraints = [
+            models.UniqueConstraint(
+                fields=["name"], condition=~models.Q(state="deleted"), name="unique_name_except_deleted"
+            )
+        ]
+
     def __init__(self, *args, **kwargs):
         self._cache = {}
         super(Domain, self).__init__(*args, **kwargs)
@@ -236,6 +244,7 @@ class Domain(TimeStampedModel, DomainHelper):
         # If the domain is deleted we don't want the expiration date to be set
         if self.state == self.State.DELETED and self.expiration_date:
             self.expiration_date = None
+
         super().save(force_insert, force_update, using, update_fields)
 
     @classmethod
@@ -245,7 +254,6 @@ class Domain(TimeStampedModel, DomainHelper):
         is called in the validate function on the request/domain page
 
         throws- RegistryError or InvalidDomainError"""
-
         if not cls.string_could_be_domain(domain):
             logger.warning("Not a valid domain: %s" % str(domain))
             # throw invalid domain error so that it can be caught in
@@ -279,6 +287,28 @@ class Domain(TimeStampedModel, DomainHelper):
                 raise err
         return False
 
+    @classmethod
+    def is_not_deleted(cls, domain: str) -> bool:
+        """Check if the domain is NOT DELETED."""
+        domain_name = domain.lower()
+
+        try:
+            info_req = commands.InfoDomain(domain_name)
+            info_response = registry.send(info_req, cleaned=True)
+            if info_response and info_response.res_data:
+                return True
+            # No res_data implies likely deleted
+            return False
+        except RegistryError as err:
+            if not err.is_connection_error():
+                # 2303 = Object does not exist --> Domain is deleted
+                if err.code == 2303:
+                    return False
+                logger.info(f"Unexpected registry error while checking domain -- {err}")
+                return True
+            else:
+                raise err
+
     @classmethod
     def registered(cls, domain: str) -> bool:
         """Check if a domain is _not_ available."""
@@ -1211,7 +1241,7 @@ class Domain(TimeStampedModel, DomainHelper):
         max_length=253,
         blank=False,
         default=None,  # prevent saving without a value
-        unique=True,
+        unique=False,
         help_text="Fully qualified domain name",
         verbose_name="domain",
     )
@@ -2051,7 +2081,6 @@ class Domain(TimeStampedModel, DomainHelper):
         """extract data from response from registry"""
 
         data = data_response.res_data[0]
-
         return {
             "auth_info": getattr(data, "auth_info", ...),
             "_contacts": getattr(data, "contacts", ...),

src/registrar/models/domain_request.py

@@ -1216,8 +1216,13 @@ class DomainRequest(TimeStampedModel):
         # create the domain
         Domain = apps.get_model("registrar.Domain")
 
-        # == Check that the domain_request is valid == #
+        """
-        if Domain.objects.filter(name=self.requested_domain.name).exists():
+        Checks that the domain_request:
+        1. Filters by specific domain name
+        2. Excludes any domain in the DELETED state
+        3. Check if there are any non DELETED state domains with same name
+        """
+        if Domain.objects.filter(name=self.requested_domain.name).exclude(state=Domain.State.DELETED).exists():
             raise FSMDomainRequestError(code=FSMErrorCodes.APPROVE_DOMAIN_IN_USE)
 
         # == Create the domain and related components == #

src/registrar/tests/test_models_domain.py

@@ -498,6 +498,51 @@ class TestDomainCreation(MockEppLib):
         with self.assertRaisesRegex(IntegrityError, "name"):
             Domain.objects.create(name="igorville.gov")
 
+    def test_duplicate_domain_name_not_allowed_if_not_deleted(self):
+        """Can't create domain if name is not unique AND not deleted."""
+
+        # 1. Mocking that it's in active state
+        mock_first_domain = MagicMock(name="meoward-is-cool.gov", state="active")
+
+        with patch.object(Domain.objects, "create") as mock_create:
+            # 2. Mock the outcomes of like we are from a "real DB":
+            # A. Simulate a domain in ACTIVE state (from #1)
+            # B. Simulate a Integrity Error due to the UniqueConstraint we added
+            mock_create.side_effect = [mock_first_domain, IntegrityError("mocked constraint")]
+
+            # 3. "Create" but actually mocking it and make sure that it's in correct (ACTIVE) state
+            domain_1 = Domain.objects.create(name="meoward-is-cool.gov", state="active")
+            self.assertEqual(domain_1.state, "active")
+            mock_create.assert_called_once_with(name="meoward-is-cool.gov", state="active")
+
+            # 4. Asserting that when we do create it again we get the mocked IntegrityError
+            with self.assertRaises(IntegrityError):
+                Domain.objects.create(name="meoward-is-cool.gov", state="active")
+
+    def test_duplicate_domain_name_allowed_if_one_is_deleted(self):
+        """Can create domain with same name if one is deleted."""
+        with patch.object(Domain.objects, "create") as mock_create:
+            # 1. Simulate the states for it to be:
+            # A. First call to be in DELETED state
+            # B. Second call for it to be in ACTIVE
+            mock_create.side_effect = [
+                MagicMock(name="meoward-is-cool.gov", state="deleted"),
+                MagicMock(name="meoward-is-cool.gov", state="active"),
+            ]
+
+            # 2. 1A in action (above comment), and verification for correct state (DELETED) below
+            domain_1 = Domain.objects.create(name="meoward-is-cool.gov", state="deleted")
+            self.assertEqual(domain_1.state, "deleted")
+            mock_create.assert_called_once_with(name="meoward-is-cool.gov", state="deleted")
+
+            # 3. 1B in action, and verification for correc state (ACTIVE) below)
+            try:
+                domain_2 = Domain.objects.create(name="meoward-is-cool.gov", state="active")
+                self.assertEqual(domain_2.state, "active")
+                mock_create.assert_any_call(name="meoward-is-cool.gov", state="active")
+            except IntegrityError:
+                self.fail("Should allow same name if one is deleted")
+
     def tearDown(self) -> None:
         DomainInformation.objects.all().delete()
         DomainRequest.objects.all().delete()
@@ -726,6 +771,47 @@ class TestDomainAvailable(MockEppLib):
 
             self.assertFalse(result)
 
+    def test_is_not_deleted_returns_true_when_domain_exists(self):
+        """
+        TLDR: Domain is NOT DELETED
+
+        Scenario: Domain exists in the registry
+        Should return True
+
+        * Mock InfoDomain command to return valid res_data
+        * Validate send is called with correct domain
+        * Validate response is True
+        """
+        with patch("registrar.models.domain.registry.send") as mocked_send:
+            mock_response = MagicMock()
+            mock_response.res_data = [MagicMock()]  # non-empty res_data
+            mocked_send.return_value = mock_response
+
+            result = Domain.is_not_deleted("not-deleted.gov")
+
+            mocked_send.assert_called_once_with(commands.InfoDomain("not-deleted.gov"), cleaned=True)
+            self.assertTrue(result)
+
+    def test_is_not_deleted_returns_false_when_domain_does_not_exist(self):
+        """
+        TLDR: Domain IS DELETED
+
+        Scenario: Domain does not exist in the registry
+        Should return False
+
+        * Mock registry.send to raise RegistryError with code 2303
+        * Validate response is False
+        """
+        with patch("registrar.models.domain.registry.send") as mocked_send:
+            error = RegistryError("Object does not exist")
+            error.code = 2303
+            error.is_connection_error = MagicMock(return_value=False)
+            mocked_send.side_effect = error
+
+            result = Domain.is_not_deleted("deleted.gov")
+
+            self.assertFalse(result)
+
     def test_domain_available_with_invalid_error(self):
         """
         Scenario: Testing whether an invalid domain is available