Merge branch 'main' into za/2348-csv-export-org-member-domain-export

src/registrar/management/commands/clean_tables.py

@@ -56,14 +56,27 @@ class Command(BaseCommand):
             self.clean_table(table_name)
 
     def clean_table(self, table_name):
-        """Delete all rows in the given table"""
+        """Delete all rows in the given table.
+
+        Delete in batches to be able to handle large tables"""
         try:
             # Get the model class dynamically
             model = apps.get_model("registrar", table_name)
-            # Use a transaction to ensure database integrity
+            BATCH_SIZE = 1000
-            with transaction.atomic():
+            total_deleted = 0
-                model.objects.all().delete()
+
-            logger.info(f"Successfully cleaned table {table_name}")
+            # Get initial batch of primary keys
+            pks = list(model.objects.values_list("pk", flat=True)[:BATCH_SIZE])
+
+            while pks:
+                # Use a transaction to ensure database integrity
+                with transaction.atomic():
+                    deleted, _ = model.objects.filter(pk__in=pks).delete()
+                    total_deleted += deleted
+                logger.debug(f"Deleted {deleted} {table_name}s, total deleted: {total_deleted}")
+                # Get the next batch of primary keys
+                pks = list(model.objects.values_list("pk", flat=True)[:BATCH_SIZE])
+            logger.info(f"Successfully cleaned table {table_name}, deleted {total_deleted} rows")
         except LookupError:
             logger.error(f"Model for table {table_name} not found.")
         except Exception as e:

src/registrar/templates/includes/finish_profile_form.html

@@ -53,6 +53,10 @@
             {% endwith %}
         </div>
 
+        {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %}
+            {% input_with_errors form.title %}
+        {% endwith %}
+        
         {% public_site_url "help/account-management/#get-help-with-login.gov" as login_help_url %}
         {% with show_readonly=True add_class="display-none" group_classes="usa-form-editable usa-form-editable padding-top-2 bold-usa-label" %}
             {% with link_href=login_help_url %}
@@ -64,10 +68,6 @@
             {% endwith %}
         {% endwith %}
 
-        {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %}
-            {% input_with_errors form.title %}
-        {% endwith %}
-
         {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %}
             {% with add_class="usa-input--medium" %}
                 {% input_with_errors form.phone %}

src/registrar/tests/test_management_scripts.py

@@ -810,36 +810,69 @@ class TestCleanTables(TestCase):
     @override_settings(IS_PRODUCTION=False)
     def test_command_cleans_tables(self):
         """test that the handle method functions properly to clean tables"""
-        with less_console_noise():
-            with patch("django.apps.apps.get_model") as get_model_mock:
-                model_mock = MagicMock()
-                get_model_mock.return_value = model_mock
 
-                with patch(
+        with patch("django.apps.apps.get_model") as get_model_mock:
-                    "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",  # noqa
+            model_mock = MagicMock()
-                    return_value=True,
+            get_model_mock.return_value = model_mock
-                ):
-                    call_command("clean_tables")
 
-                    table_names = [
+            with patch(
-                        "DomainInformation",
+                "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",  # noqa
-                        "DomainRequest",
+                return_value=True,
-                        "PublicContact",
+            ):
-                        "Domain",
+
-                        "User",
+                # List of pks to be returned in batches, one list for each of 11 tables
-                        "Contact",
+                pk_batch = [1, 2, 3, 4, 5, 6]
-                        "Website",
+                # Create a list of batches with alternating non-empty and empty lists
-                        "DraftDomain",
+                pk_batches = [pk_batch, []] * 11
-                        "HostIp",
+
-                        "Host",
+                # Set the side effect of values_list to return different pk batches
-                    ]
+                # First time values_list is called it returns list of 6 objects to delete;
+                # Next time values_list is called it returns empty list
+                def values_list_side_effect(*args, **kwargs):
+                    if args == ("pk",) and kwargs.get("flat", False):
+                        return pk_batches.pop(0)
+                    return []
+
+                model_mock.objects.values_list.side_effect = values_list_side_effect
+                # Mock the return value of `delete()` to be (6, ...)
+                model_mock.objects.filter.return_value.delete.return_value = (6, None)
+
+                call_command("clean_tables")
+
+                table_names = [
+                    "DomainInformation",
+                    "DomainRequest",
+                    "FederalAgency",
+                    "PublicContact",
+                    "HostIp",
+                    "Host",
+                    "Domain",
+                    "User",
+                    "Contact",
+                    "Website",
+                    "DraftDomain",
+                ]
+
+                expected_filter_calls = [call(pk__in=[1, 2, 3, 4, 5, 6]) for _ in range(11)]
+
+                actual_filter_calls = [c for c in model_mock.objects.filter.call_args_list if "pk__in" in c[1]]
+
+                try:
+                    # Assert that filter(pk__in=...) was called with expected arguments
+                    self.assertEqual(actual_filter_calls, expected_filter_calls)
+
+                    # Check that delete() was called for each batch
+                    for batch in [[1, 2, 3, 4, 5, 6]]:
+                        model_mock.objects.filter(pk__in=batch).delete.assert_called()
 
-                    # Check that each model's delete method was called
                     for table_name in table_names:
                         get_model_mock.assert_any_call("registrar", table_name)
-                        model_mock.objects.all().delete.assert_called()
+                        self.logger_mock.info.assert_any_call(
-
+                            f"Successfully cleaned table {table_name}, deleted 6 rows"
-                    self.logger_mock.info.assert_any_call("Successfully cleaned table DomainInformation")
+                        )
+                except AssertionError as e:
+                    print(f"AssertionError: {e}")
+                    raise
 
     @override_settings(IS_PRODUCTION=False)
     def test_command_handles_nonexistent_model(self):
@@ -870,15 +903,33 @@ class TestCleanTables(TestCase):
             with patch("django.apps.apps.get_model") as get_model_mock:
                 model_mock = MagicMock()
                 get_model_mock.return_value = model_mock
-                model_mock.objects.all().delete.side_effect = Exception("Some error")
+
+                # Mock the values_list so that DomainInformation attempts a delete
+                pk_batches = [[1, 2, 3, 4, 5, 6], []]
+
+                def values_list_side_effect(*args, **kwargs):
+                    if args == ("pk",) and kwargs.get("flat", False):
+                        return pk_batches.pop(0)
+                    return []
+
+                model_mock.objects.values_list.side_effect = values_list_side_effect
+
+                # Mock delete to raise a generic exception
+                model_mock.objects.filter.return_value.delete.side_effect = Exception("Mocked delete exception")
 
                 with patch(
-                    "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",  # noqa
+                    "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",
                     return_value=True,
                 ):
-                    call_command("clean_tables")
+                    with self.assertRaises(Exception) as context:
+                        # Execute the command
+                        call_command("clean_tables")
 
-                    self.logger_mock.error.assert_any_call("Error cleaning table DomainInformation: Some error")
+                        # Check the exception message
+                        self.assertEqual(str(context.exception), "Custom delete error")
+
+                        # Assert that delete was called
+                        model_mock.objects.filter.return_value.delete.assert_called()
 
 
 class TestExportTables(MockEppLib):