First pass

src/registrar/fixtures/fixtures_domains.py

@@ -29,19 +29,19 @@ class DomainFixture(DomainRequestFixture):
     def load(cls):
         # Lumped under .atomic to ensure we don't make redundant DB calls.
         # This bundles them all together, and then saves it in a single call.
-        with transaction.atomic():
+        try:
-            try:
+            with transaction.atomic():
                 # Get the usernames of users created in the UserFixture
                 created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
 
                 # Filter users to only include those created by the fixture
                 users = list(User.objects.filter(username__in=created_usernames))
-            except Exception as e:
+        except Exception as e:
-                logger.warning(e)
+            logger.warning(e)
-                return
+            return
 
-            # Approve each user associated with `in review` status domains
+        # Approve each user associated with `in review` status domains
-            cls._approve_domain_requests(users)
+        cls._approve_domain_requests(users)
 
     @staticmethod
     def _generate_fake_expiration_date(days_in_future=365):

src/registrar/fixtures/fixtures_portfolios.py

@@ -87,39 +87,40 @@ class PortfolioFixture:
 
         # Lumped under .atomic to ensure we don't make redundant DB calls.
         # This bundles them all together, and then saves it in a single call.
-        with transaction.atomic():
+        try:
-            try:
+            with transaction.atomic():
                 user = User.objects.all().last()
+        except Exception as e:
+            logger.warning(e)
+            return
+
+        portfolios_to_create = []
+        for portfolio_data in cls.PORTFOLIOS:
+            organization_name = portfolio_data["organization_name"]
+
+            # Check if portfolio with the organization name already exists
+            if Portfolio.objects.filter(organization_name=organization_name).exists():
+                logger.info(
+                    f"Portfolio with organization name '{organization_name}' already exists, skipping creation."
+                )
+                continue
+
+        try:
+            with transaction.atomic():
+                portfolio = Portfolio(
+                    creator=user,
+                    organization_name=portfolio_data["organization_name"],
+                )
+                cls._set_non_foreign_key_fields(portfolio, portfolio_data)
+                cls._set_foreign_key_fields(portfolio, portfolio_data, user)
+                portfolios_to_create.append(portfolio)
+        except Exception as e:
+            logger.warning(e)
+
+        # Bulk create domain requests
+        if len(portfolios_to_create) > 0:
+            try:
+                Portfolio.objects.bulk_create(portfolios_to_create)
+                logger.info(f"Successfully created {len(portfolios_to_create)} portfolios")
             except Exception as e:
-                logger.warning(e)
+                logger.warning(f"Error bulk creating portfolios: {e}")
-                return
-
-            portfolios_to_create = []
-            for portfolio_data in cls.PORTFOLIOS:
-                organization_name = portfolio_data["organization_name"]
-
-                # Check if portfolio with the organization name already exists
-                if Portfolio.objects.filter(organization_name=organization_name).exists():
-                    logger.info(
-                        f"Portfolio with organization name '{organization_name}' already exists, skipping creation."
-                    )
-                    continue
-
-                try:
-                    portfolio = Portfolio(
-                        creator=user,
-                        organization_name=portfolio_data["organization_name"],
-                    )
-                    cls._set_non_foreign_key_fields(portfolio, portfolio_data)
-                    cls._set_foreign_key_fields(portfolio, portfolio_data, user)
-                    portfolios_to_create.append(portfolio)
-                except Exception as e:
-                    logger.warning(e)
-
-            # Bulk create domain requests
-            if len(portfolios_to_create) > 0:
-                try:
-                    Portfolio.objects.bulk_create(portfolios_to_create)
-                    logger.info(f"Successfully created {len(portfolios_to_create)} portfolios")
-                except Exception as e:
-                    logger.warning(f"Error bulk creating portfolios: {e}")

src/registrar/fixtures/fixtures_requests.py

@@ -309,18 +309,18 @@ class DomainRequestFixture:
         # The atomic block will cause the code to stop executing if one instance in the
         # nested iteration fails, which will cause an early exit and make it hard to debug.
         # Comment out with transaction.atomic() when debugging.
-        with transaction.atomic():
+        try:
-            try:
+            with transaction.atomic():
                 # Get the usernames of users created in the UserFixture
                 created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
 
                 # Filter users to only include those created by the fixture
                 users = list(User.objects.filter(username__in=created_usernames))
-            except Exception as e:
+        except Exception as e:
-                logger.warning(e)
+            logger.warning(e)
-                return
+            return
 
-            cls._create_domain_requests(users)
+        cls._create_domain_requests(users)
 
     @classmethod
     def _create_domain_requests(cls, users):  # noqa: C901

src/registrar/fixtures/fixtures_user_portfolio_permissions.py

@@ -26,56 +26,55 @@ class UserPortfolioPermissionFixture:
 
         # Lumped under .atomic to ensure we don't make redundant DB calls.
         # This bundles them all together, and then saves it in a single call.
-        with transaction.atomic():
+        try:
-            try:
+            # Get the usernames of users created in the UserFixture
-                # Get the usernames of users created in the UserFixture
+            created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
-                created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
 
-                # Filter users to only include those created by the fixture
+            # Filter users to only include those created by the fixture
-                users = list(User.objects.filter(username__in=created_usernames))
+            users = list(User.objects.filter(username__in=created_usernames))
 
-                organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS]
+            organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS]
 
-                portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names))
+            portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names))
 
-                if not users:
+            if not users:
-                    logger.warning("User fixtures missing.")
+                logger.warning("User fixtures missing.")
-                    return
-
-                if not portfolios:
-                    logger.warning("Portfolio fixtures missing.")
-                    return
-
-            except Exception as e:
-                logger.warning(e)
                 return
 
-            user_portfolio_permissions_to_create = []
+            if not portfolios:
-            for user in users:
+                logger.warning("Portfolio fixtures missing.")
-                # Assign a random portfolio to a user
+                return
-                portfolio = random.choice(portfolios)  # nosec
-                try:
-                    if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists():
-                        user_portfolio_permission = UserPortfolioPermission(
-                            user=user,
-                            portfolio=portfolio,
-                            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
-                            additional_permissions=[
-                                UserPortfolioPermissionChoices.EDIT_MEMBERS,
-                                UserPortfolioPermissionChoices.EDIT_REQUESTS,
-                            ],
-                        )
-                        user_portfolio_permissions_to_create.append(user_portfolio_permission)
-                    else:
-                        logger.info(
-                            f"Permission exists for user '{user.username}' "
-                            f"on portfolio '{portfolio.organization_name}'."
-                        )
-                except Exception as e:
-                    logger.warning(e)
 
-            # Bulk create permissions
+        except Exception as e:
-            cls._bulk_create_permissions(user_portfolio_permissions_to_create)
+            logger.warning(e)
+            return
+
+        user_portfolio_permissions_to_create = []
+        for user in users:
+            # Assign a random portfolio to a user
+            portfolio = random.choice(portfolios)  # nosec
+            try:
+                if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists():
+                    user_portfolio_permission = UserPortfolioPermission(
+                        user=user,
+                        portfolio=portfolio,
+                        roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+                        additional_permissions=[
+                            UserPortfolioPermissionChoices.EDIT_MEMBERS,
+                            UserPortfolioPermissionChoices.EDIT_REQUESTS,
+                        ],
+                    )
+                    user_portfolio_permissions_to_create.append(user_portfolio_permission)
+                else:
+                    logger.info(
+                        f"Permission exists for user '{user.username}' "
+                        f"on portfolio '{portfolio.organization_name}'."
+                    )
+            except Exception as e:
+                logger.warning(e)
+
+        # Bulk create permissions
+        cls._bulk_create_permissions(user_portfolio_permissions_to_create)
 
     @classmethod
     def _bulk_create_permissions(cls, user_portfolio_permissions_to_create):

src/registrar/management/commands/remove_unused_portfolios.py

@@ -149,9 +149,9 @@ class Command(BaseCommand):
                 )
                 return
 
-        with transaction.atomic():
+        # Try to delete the portfolios
-            # Try to delete the portfolios
+        try:
-            try:
+            with transaction.atomic():
                 summary = []
                 for portfolio in portfolios_to_delete:
                     portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"]
@@ -222,14 +222,14 @@ class Command(BaseCommand):
                     """
                 )
 
-            except IntegrityError as e:
+        except IntegrityError as e:
-                logger.info(
+            logger.info(
-                    f"""{TerminalColors.FAIL}
+                f"""{TerminalColors.FAIL}
-                    Could not delete some portfolios due to integrity constraints:
+                Could not delete some portfolios due to integrity constraints:
-                    {e}
+                {e}
-                    {TerminalColors.ENDC}
+                {TerminalColors.ENDC}
-                    """
+                """
-                )
+            )
 
     def handle(self, *args, **options):
         # Get all Portfolio entries not in the allowed portfolios list

src/registrar/utility/db_helpers.py

@@ -9,12 +9,13 @@ def ignore_unique_violation():
     Execute within an atomic transaction so that if a unique constraint violation occurs,
     the individual transaction is rolled back without invalidating any larger transaction.
     """
-    with transaction.atomic():
+    try:
-        try:
+        # NOTE - is transaction doing anything here??
+        with transaction.atomic():
             yield
-        except IntegrityError as e:
+    except IntegrityError as e:
-            if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION:
+        if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION:
-                # roll back to the savepoint, effectively ignoring this transaction
+            # roll back to the savepoint, effectively ignoring this transaction
-                pass
+            pass
-            else:
+        else:
-                raise e
+            raise e