From 9565731a933b915fae19abc9c76fbb14433941f5 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 15 Nov 2024 16:56:15 -0500 Subject: [PATCH 1/7] trigger PR --- src/registrar/fixtures/fixtures_users.py | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index e60be9872..343686028 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -23,6 +23,13 @@ class UserFixture: """ ADMINS = [ + # { + # "username": "a6b5dd22-f54e-4d55-83ce-ca7b65b2dc1a", + # "first_name": "Rach test admin", + # "last_name": "Mrad test admin", + # "email": "rachid.mrad+2@gmail.com", + # "title": "Super admin tester", + # }, { "username": "aad084c3-66cc-4632-80eb-41cdf5c5bcbf", "first_name": "Aditi", @@ -154,6 +161,14 @@ class UserFixture: ] STAFF = [ + + # { + # "username": "994b7a90-f1d1-4140-a3d2-ff34183c7ee2", + # "first_name": "Rach test staff", + # "last_name": "Mrad test staff", + # "email": "rachid.mrad+3@gmail.com", + # "title": "Super staff tester", + # }, { "username": "ffec5987-aa84-411b-a05a-a7ee5cbcde54", "first_name": "Aditi-Analyst", @@ -309,6 +324,18 @@ class UserFixture: # Get all users to be updated (both new and existing) created_or_existing_users = User.objects.filter(username__in=[user.get("username") for user in users]) + # Update `is_staff` for existing users if necessary + users_to_update = [] + for user in created_or_existing_users: + if not user.is_staff: + user.is_staff = True + users_to_update.append(user) + + # Save any users that were updated + if users_to_update: + User.objects.bulk_update(users_to_update, ["is_staff"]) + logger.info(f"Updated {len(users_to_update)} existing users to have is_staff=True.") + # Filter out users who are already in the group users_not_in_group = created_or_existing_users.exclude(groups__id=group.id) From d032d76c7507a4c1be13ecdb03ff9c025593b3aa Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 18 Nov 2024 12:19:47 -0500 Subject: [PATCH 2/7] unique requested domains, portfolio appropriate suborg --- src/registrar/fixtures/fixtures_requests.py | 32 +++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index f5b57491e..e869cda44 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -6,8 +6,10 @@ from faker import Faker from django.db import transaction from registrar.fixtures.fixtures_portfolios import PortfolioFixture +from registrar.fixtures.fixtures_suborganizations import SuborganizationFixture from registrar.fixtures.fixtures_users import UserFixture from registrar.models import User, DomainRequest, DraftDomain, Contact, Website, FederalAgency +from registrar.models.domain import Domain from registrar.models.portfolio import Portfolio from registrar.models.suborganization import Suborganization @@ -189,7 +191,13 @@ class DomainRequestFixture: if not request.requested_domain: if "requested_domain" in request_dict and request_dict["requested_domain"] is not None: return DraftDomain.objects.get_or_create(name=request_dict["requested_domain"])[0] - return DraftDomain.objects.create(name=cls.fake_dot_gov()) + + # Generate a unique fake domain + # This will help us avoid domain already approved log warnings + while True: + fake_name = cls.fake_dot_gov() + if not Domain.objects.filter(name=fake_name).exists(): + return DraftDomain.objects.create(name=fake_name) return request.requested_domain @classmethod @@ -213,7 +221,7 @@ class DomainRequestFixture: if not request.sub_organization: if "sub_organization" in request_dict and request_dict["sub_organization"] is not None: return Suborganization.objects.get_or_create(name=request_dict["sub_organization"])[0] - return cls._get_random_sub_organization() + return cls._get_random_sub_organization(request) return request.sub_organization @classmethod @@ -226,12 +234,21 @@ class DomainRequestFixture: except Exception as e: logger.warning(f"Expected fixture portfolio, did not find it: {e}") return None - + @classmethod - def _get_random_sub_organization(cls): + def _get_random_sub_organization(cls, request): try: - suborg_options = [Suborganization.objects.first(), Suborganization.objects.last()] - return random.choice(suborg_options) # nosec + # Filter Suborganizations by the request's portfolio + portfolio_suborganizations = Suborganization.objects.filter(portfolio=request.portfolio) + + # Assuming SuborganizationFixture.SUBORGS is a list of dictionaries with a "name" key + suborganization_names = [suborg["name"] for suborg in SuborganizationFixture.SUBORGS] + + # Further filter by names in suborganization_names + suborganization_options = portfolio_suborganizations.filter(name__in=suborganization_names) + + # Randomly choose one if any exist + return random.choice(suborganization_options) if suborganization_options.exists() else None # nosec except Exception as e: logger.warning(f"Expected fixture sub_organization, did not find it: {e}") return None @@ -273,6 +290,9 @@ class DomainRequestFixture: # 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. + # 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: # Get the usernames of users created in the UserFixture From 2b3ce1cdfbb16ef01f7302d37619b284302ce6e9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 18 Nov 2024 15:05:25 -0500 Subject: [PATCH 3/7] Move unique logic from _get_requested_domain into fake_dot_gov, handle cases of missing phone or title --- .github/workflows/load-fixtures.yaml | 50 +++++++++++++++++++++ src/registrar/fixtures/fixtures_requests.py | 13 +++--- src/registrar/fixtures/fixtures_users.py | 14 ++++++ 3 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/load-fixtures.yaml diff --git a/.github/workflows/load-fixtures.yaml b/.github/workflows/load-fixtures.yaml new file mode 100644 index 000000000..108a54564 --- /dev/null +++ b/.github/workflows/load-fixtures.yaml @@ -0,0 +1,50 @@ +# Manually load fixtures to an environment of choice. + +name: Load fixtures +run-name: Manually load fixtures to sandbox of choice + +on: + workflow_dispatch: + inputs: + environment: + description: Which environment should we load data for? + type: 'choice' + options: + - ab + - backup + - el + - cb + - dk + - es + - gd + - ko + - ky + - nl + - rb + - rh + - rjm + - meoward + - bob + - hotgov + - litterbox + - ms + - ad + - ag + +jobs: + load-fixtures: + runs-on: ubuntu-latest + env: + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + steps: + - uses: GitHubSecurityLab/actions-permissions/monitor@v1 + - name: Load fake data for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py load' --name loaddata" + diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index e869cda44..9f736d0d1 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -104,7 +104,10 @@ class DomainRequestFixture: @classmethod def fake_dot_gov(cls): - return f"{fake.slug()}.gov" + while True: + fake_name = f"{fake.slug()}.gov" + if not Domain.objects.filter(name=fake_name).exists(): + return DraftDomain.objects.create(name=fake_name) @classmethod def fake_expiration_date(cls): @@ -192,12 +195,8 @@ class DomainRequestFixture: if "requested_domain" in request_dict and request_dict["requested_domain"] is not None: return DraftDomain.objects.get_or_create(name=request_dict["requested_domain"])[0] - # Generate a unique fake domain - # This will help us avoid domain already approved log warnings - while True: - fake_name = cls.fake_dot_gov() - if not Domain.objects.filter(name=fake_name).exists(): - return DraftDomain.objects.create(name=fake_name) + # Generate a unique fake domain + return cls.fake_dot_gov() return request.requested_domain @classmethod diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index 343686028..0c4da9bac 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -327,8 +327,22 @@ class UserFixture: # Update `is_staff` for existing users if necessary users_to_update = [] for user in created_or_existing_users: + updated = False + + if not user.title: + user.title = "Peon" + updated = True + + if not user.phone: + user.phone = "2022222222" + updated = True + if not user.is_staff: user.is_staff = True + updated = True + + # Only append the user if any of the fields were updated + if updated: users_to_update.append(user) # Save any users that were updated From ec8e8b1f65093d90d018744cf175bfc56320cbc8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Nov 2024 13:15:27 -0500 Subject: [PATCH 4/7] lint --- src/registrar/fixtures/fixtures_requests.py | 10 +++++----- src/registrar/fixtures/fixtures_users.py | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index 9f736d0d1..c413f4b62 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -194,8 +194,8 @@ class DomainRequestFixture: if not request.requested_domain: if "requested_domain" in request_dict and request_dict["requested_domain"] is not None: return DraftDomain.objects.get_or_create(name=request_dict["requested_domain"])[0] - - # Generate a unique fake domain + + # Generate a unique fake domain return cls.fake_dot_gov() return request.requested_domain @@ -233,19 +233,19 @@ class DomainRequestFixture: except Exception as e: logger.warning(f"Expected fixture portfolio, did not find it: {e}") return None - + @classmethod def _get_random_sub_organization(cls, request): try: # Filter Suborganizations by the request's portfolio portfolio_suborganizations = Suborganization.objects.filter(portfolio=request.portfolio) - + # Assuming SuborganizationFixture.SUBORGS is a list of dictionaries with a "name" key suborganization_names = [suborg["name"] for suborg in SuborganizationFixture.SUBORGS] # Further filter by names in suborganization_names suborganization_options = portfolio_suborganizations.filter(name__in=suborganization_names) - + # Randomly choose one if any exist return random.choice(suborganization_options) if suborganization_options.exists() else None # nosec except Exception as e: diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index 0c4da9bac..b3ab530c3 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -161,7 +161,6 @@ class UserFixture: ] STAFF = [ - # { # "username": "994b7a90-f1d1-4140-a3d2-ff34183c7ee2", # "first_name": "Rach test staff", @@ -344,7 +343,7 @@ class UserFixture: # Only append the user if any of the fields were updated if updated: users_to_update.append(user) - + # Save any users that were updated if users_to_update: User.objects.bulk_update(users_to_update, ["is_staff"]) From 0958eda7cc454b124997e27695a70692c9573bc2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Nov 2024 13:31:13 -0500 Subject: [PATCH 5/7] refactor for readability --- src/registrar/fixtures/fixtures_users.py | 148 +++++++++++++---------- 1 file changed, 81 insertions(+), 67 deletions(-) diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index b3ab530c3..be4afa311 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -281,80 +281,24 @@ class UserFixture: """Loads the users into the database and assigns them to the specified group.""" logger.info(f"Going to load {len(users)} users for group {group_name}") + # Step 1: Fetch the group group = UserGroup.objects.get(name=group_name) - # Prepare sets of existing usernames and IDs in one query - user_identifiers = [(user.get("username"), user.get("id")) for user in users] - existing_users = User.objects.filter( - username__in=[user[0] for user in user_identifiers] + [user[1] for user in user_identifiers] - ).values_list("username", "id") + # Step 2: Identify new and existing users + existing_usernames, existing_user_ids = cls._get_existing_users(users) + new_users = cls._prepare_new_users(users, existing_usernames, existing_user_ids, are_superusers) - existing_usernames = set(user[0] for user in existing_users) - existing_user_ids = set(user[1] for user in existing_users) - - # Filter out users with existing IDs or usernames - new_users = [ - User( - id=user_data.get("id"), - first_name=user_data.get("first_name"), - last_name=user_data.get("last_name"), - username=user_data.get("username"), - email=user_data.get("email", ""), - title=user_data.get("title", "Peon"), - phone=user_data.get("phone", "2022222222"), - is_active=user_data.get("is_active", True), - is_staff=True, - is_superuser=are_superusers, - ) - for user_data in users - if user_data.get("username") not in existing_usernames and user_data.get("id") not in existing_user_ids - ] - - # Perform bulk creation for new users - if new_users: - try: - User.objects.bulk_create(new_users) - logger.info(f"Created {len(new_users)} new users.") - except Exception as e: - logger.error(f"Unexpected error during user bulk creation: {e}") - else: - logger.info("No new users to create.") + # Step 3: Create new users + cls._create_new_users(new_users) + # Step 4: Update existing users # Get all users to be updated (both new and existing) created_or_existing_users = User.objects.filter(username__in=[user.get("username") for user in users]) + users_to_update = cls._get_users_to_update(created_or_existing_users) + cls._update_existing_users(users_to_update) - # Update `is_staff` for existing users if necessary - users_to_update = [] - for user in created_or_existing_users: - updated = False - - if not user.title: - user.title = "Peon" - updated = True - - if not user.phone: - user.phone = "2022222222" - updated = True - - if not user.is_staff: - user.is_staff = True - updated = True - - # Only append the user if any of the fields were updated - if updated: - users_to_update.append(user) - - # Save any users that were updated - if users_to_update: - User.objects.bulk_update(users_to_update, ["is_staff"]) - logger.info(f"Updated {len(users_to_update)} existing users to have is_staff=True.") - - # Filter out users who are already in the group - users_not_in_group = created_or_existing_users.exclude(groups__id=group.id) - - # Add only users who are not already in the group - if users_not_in_group.exists(): - group.user_set.add(*users_not_in_group) + # Step 5: Assign users to the group + cls._assign_users_to_group(group, created_or_existing_users) logger.info(f"Users loaded for group {group_name}.") @@ -386,6 +330,76 @@ class UserFixture: else: logger.info("No allowed emails to load") + @staticmethod + def _get_existing_users(users): + user_identifiers = [(user.get("username"), user.get("id")) for user in users] + existing_users = User.objects.filter( + username__in=[user[0] for user in user_identifiers] + [user[1] for user in user_identifiers] + ).values_list("username", "id") + existing_usernames = set(user[0] for user in existing_users) + existing_user_ids = set(user[1] for user in existing_users) + return existing_usernames, existing_user_ids + + @staticmethod + def _prepare_new_users(users, existing_usernames, existing_user_ids, are_superusers): + return [ + User( + id=user_data.get("id"), + first_name=user_data.get("first_name"), + last_name=user_data.get("last_name"), + username=user_data.get("username"), + email=user_data.get("email", ""), + title=user_data.get("title", "Peon"), + phone=user_data.get("phone", "2022222222"), + is_active=user_data.get("is_active", True), + is_staff=True, + is_superuser=are_superusers, + ) + for user_data in users + if user_data.get("username") not in existing_usernames and user_data.get("id") not in existing_user_ids + ] + + @staticmethod + def _create_new_users(new_users): + if new_users: + try: + User.objects.bulk_create(new_users) + logger.info(f"Created {len(new_users)} new users.") + except Exception as e: + logger.error(f"Unexpected error during user bulk creation: {e}") + else: + logger.info("No new users to create.") + + @staticmethod + def _get_users_to_update(users): + users_to_update = [] + for user in users: + updated = False + if not user.title: + user.title = "Peon" + updated = True + if not user.phone: + user.phone = "2022222222" + updated = True + if not user.is_staff: + user.is_staff = True + updated = True + if updated: + users_to_update.append(user) + return users_to_update + + @staticmethod + def _update_existing_users(users_to_update): + if users_to_update: + User.objects.bulk_update(users_to_update, ["is_staff", "title", "phone"]) + logger.info(f"Updated {len(users_to_update)} existing users.") + + @staticmethod + def _assign_users_to_group(group, users): + users_not_in_group = users.exclude(groups__id=group.id) + if users_not_in_group.exists(): + group.user_set.add(*users_not_in_group) + @classmethod def load(cls): with transaction.atomic(): From ff8a4a3fa193a84f58276d790710effc633a708e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Nov 2024 21:42:40 -0500 Subject: [PATCH 6/7] clean up test users --- src/registrar/fixtures/fixtures_users.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index be4afa311..a8cdb5b9a 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -23,13 +23,6 @@ class UserFixture: """ ADMINS = [ - # { - # "username": "a6b5dd22-f54e-4d55-83ce-ca7b65b2dc1a", - # "first_name": "Rach test admin", - # "last_name": "Mrad test admin", - # "email": "rachid.mrad+2@gmail.com", - # "title": "Super admin tester", - # }, { "username": "aad084c3-66cc-4632-80eb-41cdf5c5bcbf", "first_name": "Aditi", @@ -161,13 +154,6 @@ class UserFixture: ] STAFF = [ - # { - # "username": "994b7a90-f1d1-4140-a3d2-ff34183c7ee2", - # "first_name": "Rach test staff", - # "last_name": "Mrad test staff", - # "email": "rachid.mrad+3@gmail.com", - # "title": "Super staff tester", - # }, { "username": "ffec5987-aa84-411b-a05a-a7ee5cbcde54", "first_name": "Aditi-Analyst", From e1ff54d2643bdd9e9e8be6da5eb17c7415de4aae Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 26 Nov 2024 11:54:11 -0500 Subject: [PATCH 7/7] revise while True --- src/registrar/fixtures/fixtures_requests.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index c413f4b62..93167ec61 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -103,11 +103,13 @@ class DomainRequestFixture: } @classmethod - def fake_dot_gov(cls): - while True: + def fake_dot_gov(cls, max_attempts=100): + """Generate a unique .gov domain name without using an infinite loop.""" + for _ in range(max_attempts): fake_name = f"{fake.slug()}.gov" if not Domain.objects.filter(name=fake_name).exists(): return DraftDomain.objects.create(name=fake_name) + raise RuntimeError(f"Failed to generate a unique .gov domain after {max_attempts} attempts") @classmethod def fake_expiration_date(cls): @@ -240,7 +242,7 @@ class DomainRequestFixture: # Filter Suborganizations by the request's portfolio portfolio_suborganizations = Suborganization.objects.filter(portfolio=request.portfolio) - # Assuming SuborganizationFixture.SUBORGS is a list of dictionaries with a "name" key + # Select a suborg that's defined in the fixtures suborganization_names = [suborg["name"] for suborg in SuborganizationFixture.SUBORGS] # Further filter by names in suborganization_names