From 072738cf03113b3535894a171271334eb0b6630c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:04:37 -0800 Subject: [PATCH 01/98] Add alert to user domain role delete confirmation page --- src/registrar/admin.py | 4 ++++ .../admin/user_domain_role_delete_confirmation.html | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e89147b11..f0e435090 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1369,9 +1369,13 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/user_domain_role_change_form.html" + # Override for the delete confirmation page on the domain table (bulk delete action) + delete_selected_confirmation_template = "django/admin/user_domain_role_delete_confirmation.html" + # Fixes a bug where non-superusers are redirected to the main page def delete_view(self, request, object_id, extra_context=None): """Custom delete_view implementation that specifies redirect behaviour""" + self.delete_confirmation_template = "django/admin/user_domain_role_delete_confirmation.html" response = super().delete_view(request, object_id, extra_context) if isinstance(response, HttpResponseRedirect) and not request.user.has_perm("registrar.full_access_permission"): diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html new file mode 100644 index 000000000..807502a31 --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -0,0 +1,13 @@ +{% extends 'admin/delete_confirmation.html' %} +{% load i18n static %} + +{% block content %} + + {{ block.super }} +{% endblock %} From ea31249ef80a0bfd7db00e15578720a4a0598a0c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:06:15 -0800 Subject: [PATCH 02/98] Move alert to content subtitle --- .../django/admin/user_domain_role_delete_confirmation.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html index 807502a31..171f4c3ea 100644 --- a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -1,7 +1,7 @@ {% extends 'admin/delete_confirmation.html' %} {% load i18n static %} -{% block content %} +{% block content_subtitle %} From 117bfa9b169a07876354323f1b6b3ba1c5b0ee97 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:07:16 -0800 Subject: [PATCH 17/98] Add space to domain manager deleted email template --- .../templates/emails/domain_manager_deleted_notification.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification.txt b/src/registrar/templates/emails/domain_manager_deleted_notification.txt index b16b74dec..af0b92a8c 100644 --- a/src/registrar/templates/emails/domain_manager_deleted_notification.txt +++ b/src/registrar/templates/emails/domain_manager_deleted_notification.txt @@ -1,7 +1,7 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} -A domain manager was removed from {{ domain.name }}. +A domain manager was removed from {{ domain.name }}. REMOVED BY: {{ removed_by.email }} REMOVED ON: {{ date }} MANAGER REMOVED: {{ manager_removed.email }} From b28ddf296ecf6985e3145d8dfc529d12bd12a4e9 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Sun, 2 Feb 2025 17:54:53 -0600 Subject: [PATCH 18/98] fix tests --- .../commands/create_federal_portfolio.py | 54 +++++++++++++++++- .../tests/test_management_scripts.py | 55 +++++++++++++------ 2 files changed, 88 insertions(+), 21 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 095d6d9f5..7a90deed1 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -30,6 +30,8 @@ class Command(BaseCommand): self.failed_portfolios = set() self.added_managers = set() self.added_invitations = set() + self.failed_managers = set() + self.failed_invitations = set() def add_arguments(self, parser): """Add command line arguments to create federal portfolios. @@ -114,9 +116,15 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) - portfolios.append(portfolio) - + # if the portfolio is already created, we don't want to create it again + portfolio = Portfolio.objects.filter(organization_name=federal_agency.agency) + if portfolio.exists(): + portfolio = portfolio.first() + message = f"Portfolio '{federal_agency.agency}' already exists. Skipping create." + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + else: + portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + portfolios.append(portfolio) logger.debug(f"add_managers: {add_managers}") if add_managers: logger.debug("Adding managers to portfolio") @@ -141,6 +149,25 @@ class Command(BaseCommand): display_as_str=True, ) + if add_managers: + TerminalHelper.log_script_run_summary( + self.added_managers, + self.failed_managers, + [], # can't skip managers, can only add or fail + log_header="----- MANAGERS ADDED -----", + debug=False, + display_as_str=True, + ) + + TerminalHelper.log_script_run_summary( + self.added_invitations, + self.failed_invitations, + [], # can't skip invitations, can only add or fail + log_header="----- INVITATIONS ADDED -----", + debug=False, + display_as_str=True, + ) + # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests or both: @@ -190,11 +217,13 @@ class Command(BaseCommand): user=user, defaults={"roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]}, ) + self.added_managers.add(user) if created: logger.info(f"Added manager '{user}' to portfolio '{portfolio}'") else: logger.info(f"Manager '{user}' already exists in portfolio '{portfolio}'") except User.DoesNotExist: + self.failed_managers.add(user) logger.debug(f"User '{user}' does not exist") for manager in invited_managers: @@ -229,13 +258,20 @@ class Command(BaseCommand): logger.info(f"Created portfolio permission for '{user}' to portfolio '{portfolio}'") else: logger.info(f"Retrieved existing portfolio permission for '{user}' to portfolio '{portfolio}'") + + self.added_invitations.add(user) except User.DoesNotExist: PortfolioInvitation.objects.get_or_create( portfolio=portfolio, email=email, defaults={"status": PortfolioInvitation.PortfolioInvitationStatus.INVITED}, ) + self.added_invitations.add(email) logger.info(f"Created portfolio invitation for '{email}' to portfolio '{portfolio}'") + except Exception as exc: + self.failed_invitations.add(email) + logger.error(exc, exc_info=True) + logger.error(f"Failed to create portfolio invitation for '{email}' to portfolio '{portfolio}'") def post_process_started_domain_requests(self, agencies, portfolios): """ @@ -250,13 +286,20 @@ class Command(BaseCommand): # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. # 3. The domain request is in status "started". # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. + message = f"agencies: {agencies}" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + domain_requests_to_update = DomainRequest.objects.filter( federal_agency__in=agencies, federal_agency__agency__isnull=False, status=DomainRequest.DomainRequestStatus.STARTED, organization_name__isnull=False, ) + message = (f"domain_requests_to_update: {domain_requests_to_update}") + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} + message = f"portfolio_set: {portfolio_set}" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) # Update the request, assuming the given agency name matches the portfolio name updated_requests = [] @@ -265,6 +308,9 @@ class Command(BaseCommand): if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) + + message = f"updated_requests: {updated_requests}" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) # Execute the update and Log the results if TerminalHelper.prompt_for_execution( @@ -275,6 +321,8 @@ class Command(BaseCommand): ), prompt_title="Do you wish to commit this update to the database?", ): + message = f"prompted for execution" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index b2b217044..515c32782 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1460,14 +1460,11 @@ class TestCreateFederalPortfolio(TestCase): first_name="first", last_name="last", email="mango@igorville.gov", federal_agency=self.executive_agency_2 ) - self.portfolio = Portfolio.objects.create(organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user) - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): self.domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, - portfolio=self.portfolio, user=self.user, ) self.domain_request.approve() @@ -1479,26 +1476,22 @@ class TestCreateFederalPortfolio(TestCase): status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, - portfolio=self.portfolio, user=self.user, organization_name="Test Federal Agency", ) self.domain_request_2.approve() self.domain_info_2 = DomainInformation.objects.filter(domain_request=self.domain_request_2).get() - self.domain_2 = Domain.objects.get(name="icecreamforigorville.gov") self.domain_request_3 = completed_domain_request( name="exec_1.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.executive_agency_1, - portfolio=self.portfolio, user=self.user, organization_name="Executive Agency 1", ) self.domain_request_3.approve() self.domain_info_3 = self.domain_request_3.DomainRequest_info - self.domain = Domain.objects.get(name="exec_1.gov") self.domain_request_4 = completed_domain_request( name="exec_2.gov", @@ -1506,12 +1499,10 @@ class TestCreateFederalPortfolio(TestCase): generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.executive_agency_2, user=self.user, - portfolio=self.portfolio, organization_name="Executive Agency 2", ) self.domain_request_4.approve() self.domain_info_4 = self.domain_request_4.DomainRequest_info - self.domain_4 = Domain.objects.get(name="exec_2.gov") def tearDown(self): DomainInformation.objects.all().delete() @@ -1529,7 +1520,7 @@ class TestCreateFederalPortfolio(TestCase): ): call_command("create_federal_portfolio", **kwargs) - @less_console_noise_decorator + # @less_console_noise_decorator def test_post_process_started_domain_requests_existing_portfolio(self): """Ensures that federal agency is cleared when agency name matches portfolio name. As the name implies, this implicitly tests the "post_process_started_domain_requests" function. @@ -1869,7 +1860,10 @@ class TestCreateFederalPortfolio(TestCase): UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) # Run the management command - self.run_create_federal_portfolio(agency_name=self.portfolio.organization_name, parse_requests=True, add_managers=True) + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) + + # Check that the portfolio was created + self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) # Check that the users have been added as portfolio managers permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user__in=[manager1, manager2]) @@ -1890,13 +1884,13 @@ class TestCreateFederalPortfolio(TestCase): email="manager1@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED ) - - # Ensure no existing PortfolioInvitation for the invited email - self.assertFalse(PortfolioInvitation.objects.filter(email="manager1@example.com", portfolio=self.portfolio).exists()) - + # Run the management command self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) + # Check that the portfolio was created + self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) + # Check that a PortfolioInvitation has been created for the invited email invitation = PortfolioInvitation.objects.get(email="manager1@example.com", portfolio=self.portfolio) @@ -1909,9 +1903,9 @@ class TestCreateFederalPortfolio(TestCase): # Verify that no duplicate invitations are created self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) - duplicated_invitations = PortfolioInvitation.objects.filter(email="manager1@example.com", portfolio=self.portfolio) + invitations = PortfolioInvitation.objects.filter(email="manager1@example.com", portfolio=self.portfolio) self.assertEqual( - duplicated_invitations.count(), + invitations.count(), 1, "Duplicate PortfolioInvitation should not be created for the same email and portfolio." ) @@ -1922,6 +1916,9 @@ class TestCreateFederalPortfolio(TestCase): # Create a manager manager = User.objects.create(username="manager", email="manager@example.com") UserDomainRole.objects.create(user=manager, domain=self.domain, role=UserDomainRole.Roles.MANAGER) + + # Create a pre-existing portfolio + self.portfolio = Portfolio.objects.create(organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user) # Manually add the manager to the portfolio UserPortfolioPermission.objects.create(portfolio=self.portfolio, user=manager, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER]) @@ -1931,4 +1928,26 @@ class TestCreateFederalPortfolio(TestCase): # Ensure that the manager is not duplicated permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user=manager) - self.assertEqual(permissions.count(), 1) \ No newline at end of file + self.assertEqual(permissions.count(), 1) + + @less_console_noise_decorator + def test_add_managers_portfolio_already_exists(self): + """Test that managers are skipped when the portfolio already exists.""" + + # Create a pre-existing portfolio + self.portfolio = Portfolio.objects.create(organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user) + + # Create users and assign them as domain managers + manager1 = User.objects.create(username="manager1", email="manager1@example.com") + manager2 = User.objects.create(username="manager2", email="manager2@example.com") + UserDomainRole.objects.create(user=manager1, domain=self.domain, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) + + # Run the management command + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) + + # Check that managers were added to the portfolio + permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user__in=[manager1, manager2]) + self.assertEqual(permissions.count(), 2) + for perm in permissions: + self.assertIn(UserPortfolioRoleChoices.ORGANIZATION_MEMBER, perm.roles) From bdd57c8cfda0abbaa7f3e3a0e8e1b9b8460ceb93 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 3 Feb 2025 11:20:09 -0600 Subject: [PATCH 19/98] linter and final test fixes --- .../commands/create_federal_portfolio.py | 42 ++++--- .../tests/test_management_scripts.py | 118 +++++++++++------- 2 files changed, 94 insertions(+), 66 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 2f2cc2625..71004dc49 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -89,7 +89,7 @@ class Command(BaseCommand): help="Only add suborganizations to newly created portfolios, skip existing ones.", ) - def handle(self, **options): + def handle(self, **options): # noqa: C901 agency_name = options.get("agency_name") branch = options.get("branch") parse_requests = options.get("parse_requests") @@ -116,7 +116,6 @@ class Command(BaseCommand): ) else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." @@ -127,6 +126,8 @@ class Command(BaseCommand): federal_agency, parse_domains, parse_requests, both, skip_existing_portfolios ) portfolios.append(portfolio) + if add_managers: + self.add_managers_to_portfolio(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -192,31 +193,32 @@ class Command(BaseCommand): This includes adding them to the correct group and creating portfolio invitations. """ logger.info(f"Adding managers for portfolio {portfolio}") - + # Fetch all domains associated with the portfolio domains = Domain.objects.filter(domain_info__portfolio=portfolio) logger.debug(f"domains: {domains}") - domain_managers = set() + domain_managers: set[int] = set() # Fetch all users with manager roles for the domains - managers = UserDomainRole.objects.filter( - domain__in=domains, - role=UserDomainRole.Roles.MANAGER - ).values_list('user', flat=True) + managers = UserDomainRole.objects.filter(domain__in=domains, role=UserDomainRole.Roles.MANAGER).values_list( + "user", flat=True + ) domain_managers.update(managers) - invited_managers = set() + invited_managers: set[str] = set() # Get the emails of invited managers for domain in domains: - domain_invitations = DomainInvitation.objects.filter(domain=domain, status=DomainInvitation.DomainInvitationStatus.INVITED).values_list('email', flat=True) + domain_invitations = DomainInvitation.objects.filter( + domain=domain, status=DomainInvitation.DomainInvitationStatus.INVITED + ).values_list("email", flat=True) invited_managers.update(domain_invitations) logger.debug(f"invited_managers: {invited_managers}") - for manager in domain_managers: + for id in domain_managers: try: # manager is a user id - user = User.objects.get(id=manager) + user = User.objects.get(id=id) _, created = UserPortfolioPermission.objects.get_or_create( portfolio=portfolio, user=user, @@ -230,9 +232,9 @@ class Command(BaseCommand): except User.DoesNotExist: self.failed_managers.add(user) logger.debug(f"User '{user}' does not exist") - - for manager in invited_managers: - self.create_portfolio_invitation(portfolio, manager) + + for email in invited_managers: + self.create_portfolio_invitation(portfolio, email) def create_portfolio_invitation(self, portfolio: Portfolio, email: str): """ @@ -252,18 +254,18 @@ class Command(BaseCommand): logger.info(f"Created portfolio invitation for '{user}' to portfolio '{portfolio}'") else: logger.info(f"Retrieved existing portfolio invitation for '{user}' to portfolio '{portfolio}'") - + # Assign portfolio permissions _, created = UserPortfolioPermission.objects.get_or_create( portfolio=portfolio, user=user, - defaults={"role": UserPortfolioPermission.RoleChoices.MANAGER}, + defaults={"role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER}, ) if created: logger.info(f"Created portfolio permission for '{user}' to portfolio '{portfolio}'") else: logger.info(f"Retrieved existing portfolio permission for '{user}' to portfolio '{portfolio}'") - + self.added_invitations.add(user) except User.DoesNotExist: PortfolioInvitation.objects.get_or_create( @@ -316,7 +318,7 @@ class Command(BaseCommand): if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) - + message = f"updated_requests: {updated_requests}" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) @@ -329,7 +331,7 @@ class Command(BaseCommand): ), prompt_title="Do you wish to commit this update to the database?", ): - message = f"prompted for execution" + message = "prompted for execution" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 2ca4e4ffe..ac507cdf2 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -3,17 +3,11 @@ import boto3_mocking # type: ignore from datetime import date, datetime, time from django.core.management import call_command from django.test import TestCase, override_settings -<<<<<<< HEAD -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.senior_official import SeniorOfficial -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices -======= from registrar.models.domain_group import DomainGroup from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_portfolio_permission import UserPortfolioPermission ->>>>>>> origin/main +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.constants import BranchChoices from django.utils import timezone from django.utils.module_loading import import_string @@ -1472,7 +1466,7 @@ class TestCreateFederalPortfolio(TestCase): self.executive_so_2 = SeniorOfficial.objects.create( first_name="first", last_name="last", email="mango@igorville.gov", federal_agency=self.executive_agency_2 ) - + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): self.domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.IN_REVIEW, @@ -1533,7 +1527,7 @@ class TestCreateFederalPortfolio(TestCase): ): call_command("create_federal_portfolio", **kwargs) - # @less_console_noise_decorator + @less_console_noise_decorator def test_post_process_started_domain_requests_existing_portfolio(self): """Ensures that federal agency is cleared when agency name matches portfolio name. As the name implies, this implicitly tests the "post_process_started_domain_requests" function. @@ -1862,7 +1856,6 @@ class TestCreateFederalPortfolio(TestCase): self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) -<<<<<<< HEAD @less_console_noise_decorator def test_add_managers_from_domains(self): """Test that all domain managers are added as portfolio managers.""" @@ -1872,56 +1865,55 @@ class TestCreateFederalPortfolio(TestCase): manager2 = User.objects.create(username="manager2", email="manager2@example.com") UserDomainRole.objects.create(user=manager1, domain=self.domain, role=UserDomainRole.Roles.MANAGER) UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) - + # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, add_managers=True) # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) - + # Check that the users have been added as portfolio managers permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user__in=[manager1, manager2]) - print(UserPortfolioPermission.objects.all()) - # Check that the users have been added as portfolio managers + # Check that the users have been added as portfolio managers self.assertEqual(permissions.count(), 2) for perm in permissions: self.assertIn(UserPortfolioRoleChoices.ORGANIZATION_MEMBER, perm.roles) - + @less_console_noise_decorator def test_add_invited_managers(self): """Test that invited domain managers receive portfolio invitations.""" # create a domain invitation for the manager _ = DomainInvitation.objects.create( - domain=self.domain, - email="manager1@example.com", - status=DomainInvitation.DomainInvitationStatus.INVITED - ) - + domain=self.domain, email="manager1@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ) + # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) - + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, add_managers=True) + # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) # Check that a PortfolioInvitation has been created for the invited email invitation = PortfolioInvitation.objects.get(email="manager1@example.com", portfolio=self.portfolio) - + # Verify the status of the invitation remains INVITED self.assertEqual( invitation.status, PortfolioInvitation.PortfolioInvitationStatus.INVITED, - "PortfolioInvitation status should remain INVITED for non-existent users." + "PortfolioInvitation status should remain INVITED for non-existent users.", ) - + # Verify that no duplicate invitations are created - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) + self.run_create_federal_portfolio( + agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True + ) invitations = PortfolioInvitation.objects.filter(email="manager1@example.com", portfolio=self.portfolio) self.assertEqual( invitations.count(), 1, - "Duplicate PortfolioInvitation should not be created for the same email and portfolio." + "Duplicate PortfolioInvitation should not be created for the same email and portfolio.", ) @less_console_noise_decorator @@ -1931,41 +1923,76 @@ class TestCreateFederalPortfolio(TestCase): manager = User.objects.create(username="manager", email="manager@example.com") UserDomainRole.objects.create(user=manager, domain=self.domain, role=UserDomainRole.Roles.MANAGER) - # Create a pre-existing portfolio - self.portfolio = Portfolio.objects.create(organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user) - + # Create a pre-existing portfolio + self.portfolio = Portfolio.objects.create( + organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user + ) + # Manually add the manager to the portfolio - UserPortfolioPermission.objects.create(portfolio=self.portfolio, user=manager, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER]) - + UserPortfolioPermission.objects.create( + portfolio=self.portfolio, user=manager, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) - + self.run_create_federal_portfolio( + agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True + ) + # Ensure that the manager is not duplicated permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user=manager) self.assertEqual(permissions.count(), 1) @less_console_noise_decorator - def test_add_managers_portfolio_already_exists(self): + def test_add_managers_skip_existing_portfolios(self): """Test that managers are skipped when the portfolio already exists.""" - - # Create a pre-existing portfolio - self.portfolio = Portfolio.objects.create(organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user) - + + # Create a pre-existing portfolio + self.portfolio = Portfolio.objects.create( + organization_name=self.federal_agency.agency, federal_agency=self.federal_agency, creator=self.user + ) + + domain_request_1 = completed_domain_request( + name="domain1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.CITY, + federal_agency=self.federal_agency, + user=self.user, + portfolio=self.portfolio, + ) + domain_request_1.approve() + domain1 = Domain.objects.get(name="domain1.gov") + + domain_request_2 = completed_domain_request( + name="domain2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.CITY, + federal_agency=self.federal_agency, + user=self.user, + portfolio=self.portfolio, + ) + domain_request_2.approve() + domain2 = Domain.objects.get(name="domain2.gov") + # Create users and assign them as domain managers manager1 = User.objects.create(username="manager1", email="manager1@example.com") manager2 = User.objects.create(username="manager2", email="manager2@example.com") - UserDomainRole.objects.create(user=manager1, domain=self.domain, role=UserDomainRole.Roles.MANAGER) - UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=manager1, domain=domain1, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=manager2, domain=domain2, role=UserDomainRole.Roles.MANAGER) # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True) - + self.run_create_federal_portfolio( + agency_name=self.federal_agency.agency, + parse_requests=True, + add_managers=True, + skip_existing_portfolios=True, + ) + # Check that managers were added to the portfolio permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user__in=[manager1, manager2]) self.assertEqual(permissions.count(), 2) for perm in permissions: self.assertIn(UserPortfolioRoleChoices.ORGANIZATION_MEMBER, perm.roles) -======= + def test_skip_existing_portfolios(self): """Tests the skip_existing_portfolios to ensure that it doesn't add suborgs, domain requests, and domain info.""" @@ -2466,4 +2493,3 @@ class TestRemovePortfolios(TestCase): # Check that the portfolio was deleted self.assertFalse(Portfolio.objects.filter(organization_name="Test with suborg").exists()) ->>>>>>> origin/main From dca2f635a287fb845ea1becfdf9c263f0d2908e4 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 3 Feb 2025 13:07:36 -0700 Subject: [PATCH 20/98] Shrink max-width of tables in widescreen. Constrict Action column to 80px. Add Action column updates to Members table. --- src/docker-compose.yml | 2 ++ .../src/js/getgov/table-domain-requests.js | 4 +-- .../assets/src/js/getgov/table-domains.js | 2 +- .../assets/src/js/getgov/table-members.js | 30 +++++++------------ .../assets/src/sass/_theme/_base.scss | 6 +++- .../templates/includes/members_table.html | 2 +- 6 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/docker-compose.yml b/src/docker-compose.yml index 5ad6d0ce6..09bf8243e 100644 --- a/src/docker-compose.yml +++ b/src/docker-compose.yml @@ -79,6 +79,8 @@ services: - POSTGRES_DB=app - POSTGRES_USER=user - POSTGRES_PASSWORD=feedabee + ports: + - "5432:5432" node: build: diff --git a/src/registrar/assets/src/js/getgov/table-domain-requests.js b/src/registrar/assets/src/js/getgov/table-domain-requests.js index f667a96b5..9a78a4551 100644 --- a/src/registrar/assets/src/js/getgov/table-domain-requests.js +++ b/src/registrar/assets/src/js/getgov/table-domain-requests.js @@ -116,8 +116,8 @@ export class DomainRequestsTable extends BaseTable { ${request.status} - -
+ +
${markupForSuborganizationRow} - + Extra Actions`; - let tableHeaderRow = this.tableWrapper.querySelector('thead tr'); - tableHeaderRow.appendChild(extraActionsHeader); - } return { 'hasAdditionalActions': hasEditPermission, 'UserPortfolioPermissionChoices' : data.UserPortfolioPermissionChoices @@ -121,15 +109,17 @@ export class MembersTable extends BaseTable { ${last_active.display_value} - - - - ${member.action_label} ${member.name} - + +
+ + + ${member.action_label} ${member.name} + + ${customTableOptions.hasAdditionalActions ? kebabHTML : ''} +
- ${customTableOptions.hasAdditionalActions ? ''+kebabHTML+'' : ''} `; tbody.appendChild(row); if (domainsHTML || permissionsHTML) { diff --git a/src/registrar/assets/src/sass/_theme/_base.scss b/src/registrar/assets/src/sass/_theme/_base.scss index be3b89baf..8ab914e6c 100644 --- a/src/registrar/assets/src/sass/_theme/_base.scss +++ b/src/registrar/assets/src/sass/_theme/_base.scss @@ -1,7 +1,7 @@ @use "uswds-core" as *; @use "cisa_colors" as *; -$widescreen-max-width: 1920px; +$widescreen-max-width: 1536px; //1920px; $widescreen-x-padding: 4.5rem; $hot-pink: #FFC3F9; @@ -275,3 +275,7 @@ abbr[title] { .width-quarter { width: 25%; } + +.width--action-column { + max-width: 80px; +} diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index cc308619a..18f6b3546 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -59,7 +59,7 @@ role="columnheader" id="header-action" > - Action + Action From d89f324e6f42f8110e2030ad18bd3b9ebf5dac09 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 3 Feb 2025 14:45:47 -0600 Subject: [PATCH 21/98] remove log statements --- .../management/commands/create_federal_portfolio.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 71004dc49..7ed7316be 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -293,8 +293,6 @@ class Command(BaseCommand): # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. # 3. The domain request is in status "started". # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. - message = f"agencies: {agencies}" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) domain_requests_to_update = DomainRequest.objects.filter( federal_agency__in=agencies, @@ -308,8 +306,6 @@ class Command(BaseCommand): return portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} - message = f"portfolio_set: {portfolio_set}" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) # Update the request, assuming the given agency name matches the portfolio name updated_requests = [] @@ -319,9 +315,6 @@ class Command(BaseCommand): req.federal_agency = None updated_requests.append(req) - message = f"updated_requests: {updated_requests}" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - # Execute the update and Log the results if TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, @@ -331,8 +324,6 @@ class Command(BaseCommand): ), prompt_title="Do you wish to commit this update to the database?", ): - message = "prompted for execution" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") From 72da75773efb8ece3d54f3b8beddfc089b89a487 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 3 Feb 2025 13:55:00 -0700 Subject: [PATCH 22/98] Icon alignments --- src/registrar/assets/src/js/getgov/table-domain-requests.js | 2 +- src/registrar/assets/src/js/getgov/table-domains.js | 2 +- src/registrar/assets/src/js/getgov/table-members.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-domain-requests.js b/src/registrar/assets/src/js/getgov/table-domain-requests.js index 9a78a4551..8556b714f 100644 --- a/src/registrar/assets/src/js/getgov/table-domain-requests.js +++ b/src/registrar/assets/src/js/getgov/table-domain-requests.js @@ -119,7 +119,7 @@ export class DomainRequestsTable extends BaseTable {
- ${request.requested_domain ? request.requested_domain : 'New domain request'} diff --git a/src/registrar/assets/src/js/getgov/table-domains.js b/src/registrar/assets/src/js/getgov/table-domains.js index cb0372584..7eeaebab4 100644 --- a/src/registrar/assets/src/js/getgov/table-domains.js +++ b/src/registrar/assets/src/js/getgov/table-domains.js @@ -58,7 +58,7 @@ export class DomainsTable extends BaseTable { ${markupForSuborganizationRow} - ${domain.name} diff --git a/src/registrar/assets/src/js/getgov/table-members.js b/src/registrar/assets/src/js/getgov/table-members.js index d676e7801..29d140185 100644 --- a/src/registrar/assets/src/js/getgov/table-members.js +++ b/src/registrar/assets/src/js/getgov/table-members.js @@ -110,9 +110,9 @@ export class MembersTable extends BaseTable { ${last_active.display_value} -
+
- ${member.name} From 78f447fad09fa38296ac5391ee5b182b3396f70a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 3 Feb 2025 14:00:49 -0700 Subject: [PATCH 23/98] cleanup --- src/registrar/assets/src/js/getgov/table-domains.js | 2 +- src/registrar/assets/src/sass/_theme/_base.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-domains.js b/src/registrar/assets/src/js/getgov/table-domains.js index 7eeaebab4..6536e7f6e 100644 --- a/src/registrar/assets/src/js/getgov/table-domains.js +++ b/src/registrar/assets/src/js/getgov/table-domains.js @@ -56,7 +56,7 @@ export class DomainsTable extends BaseTable { ${markupForSuborganizationRow} - +
+ {{ field.value | capfirst }} + + + +
+ diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html index 6ce6ed0d1..699760fa8 100644 --- a/src/registrar/templates/django/admin/domain_invitation_change_form.html +++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html @@ -11,4 +11,4 @@
{{ block.super }} -{% endblock %} +{% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html index f959f8edf..4c0e63d66 100644 --- a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html +++ b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html @@ -7,7 +7,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "email" %} {% include "admin/input_with_clipboard.html" with field=field.field %} + {% elif field.field.name == "status" %} + {% include "admin/status_with_clipboard.html" with field=field.field %} {% else %} {{ block.super }} {% endif %} {% endblock field_other %} + From dc3351e8c0daec1b1a0fc8e7e629de44ad4ba3d4 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 5 Feb 2025 16:05:49 -0500 Subject: [PATCH 35/98] revise UI of search bars with labels, create component --- .../includes/member_domains_edit_table.html | 46 ++-------------- .../includes/member_domains_table.html | 52 +++++-------------- src/registrar/templates/includes/search.html | 34 ++++++++++++ 3 files changed, 50 insertions(+), 82 deletions(-) create mode 100644 src/registrar/templates/includes/search.html diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index 0b8ff005a..cefc30c15 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -36,47 +36,9 @@
- + {% with label_text="Search all domains" item_name="edit-member-domains" aria_label_text="Member domains search component" %} + {% include "includes/search.html" %} + {% endwith %}
@@ -85,7 +47,7 @@ member domains - Assigned domains + Assigned domains Domains diff --git a/src/registrar/templates/includes/member_domains_table.html b/src/registrar/templates/includes/member_domains_table.html index d7839e485..4e63fdbc3 100644 --- a/src/registrar/templates/includes/member_domains_table.html +++ b/src/registrar/templates/includes/member_domains_table.html @@ -1,5 +1,3 @@ -{% load static %} - {% if member %} -