From 3cac4693fae8adb72a2c9807d9697acc9544b485 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 21 Jan 2025 18:13:28 -0700 Subject: [PATCH 01/14] draft script (still needs check for dependencies) --- .../commands/remove_unused_portfolios.py | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 src/registrar/management/commands/remove_unused_portfolios.py diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py new file mode 100644 index 000000000..bd75ca7b0 --- /dev/null +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -0,0 +1,129 @@ +import argparse +import logging + +from django.core.management.base import BaseCommand +from django.db import IntegrityError +from registrar.models import Portfolio +from registrar.management.commands.utility.terminal_helper import ( + TerminalColors, + TerminalHelper, +) +logger = logging.getLogger(__name__) + +ALLOWED_PORTFOLIOS = [ + "Department of Veterans Affairs", + "Department of the Treasury", + "National Archives and Records Administration", + "Department of Defense", + "Department of Defense", + "Office of Personnel Management", + "National Aeronautics and Space Administration", + "City and County of San Francisco", + "State of Arizona, Executive Branch", + "State of Arizona, Executive Branch", + "Department of the Interior", + "Department of State", + "Department of Justice", + "Department of Veterans Affairs", + "Capitol Police", + "Administrative Office of the Courts", + "Supreme Court of the United States", +] + +class Command(BaseCommand): + help = 'Remove all Portfolio entries with names not in the allowed list.' + + + def add_arguments(self, parser): + """ + OPTIONAL ARGUMENTS: + --debug + A boolean (default to true), which activates additional print statements + """ + parser.add_argument("--debug", action=argparse.BooleanOptionalAction) + + + def prompt_delete_entries(self, portfolios_to_delete, debug_on): + """Brings up a prompt in the terminal asking + if the user wishes to delete data in the + Portfolio table. If the user confirms, + deletes the data in the Portfolio table""" + + entries_to_remove_by_name = list(portfolios_to_delete.values_list("organization_name", flat=True)) + formatted_entries = "\n\t\t".join(entries_to_remove_by_name) + confirm_delete = TerminalHelper.query_yes_no( + f""" + {TerminalColors.FAIL} + WARNING: You are about to delete the following portfolios: + + {formatted_entries} + + Are you sure you want to continue?{TerminalColors.ENDC}""" + ) + if confirm_delete: + logger.info( + f"""{TerminalColors.YELLOW} + ----------Deleting entries---------- + (please wait) + {TerminalColors.ENDC}""" + ) + self.delete_entries(portfolios_to_delete, debug_on) + else: + logger.info( + f"""{TerminalColors.OKCYAN} + ----------No entries deleted---------- + (exiting script) + {TerminalColors.ENDC}""" + ) + + + + def delete_entries(self, portfolios_to_delete, debug_on): + # Log the number of entries being removed + count = portfolios_to_delete.count() + if count == 0: + logger.info( + f"""{TerminalColors.OKCYAN} + No entries to remove. + {TerminalColors.ENDC} + """ + ) + return + + # If debug mode is on, print out entries being removed + if debug_on: + entries_to_remove_by_name = list(portfolios_to_delete.values_list("organization_name", flat=True)) + formatted_entries = ", ".join(entries_to_remove_by_name) + logger.info( + f"""{TerminalColors.YELLOW} + Entries to be removed: {formatted_entries} + {TerminalColors.ENDC} + """ + ) + + # Delete the entries + try: + portfolios_to_delete.delete() + # Output a success message + logger.info( + f"""{TerminalColors.OKGREEN} + Successfully removed {count} entries. + {TerminalColors.ENDC} + """ + ) + except IntegrityError as e: + logger.info( + f"""{TerminalColors.FAIL} + Could not delete some entries due to protected relationships + {TerminalColors.ENDC} + """ + ) + + + + + def handle(self, *args, **options): + # Get all Portfolio entries not in the allowed portfolios list + portfolios_to_delete = Portfolio.objects.exclude(organization_name__in=ALLOWED_PORTFOLIOS) + + self.prompt_delete_entries(portfolios_to_delete, options.get("debug")) From ccc67c3c889698f93c3eb407fee053eaeea83db7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 21 Jan 2025 22:41:09 -0700 Subject: [PATCH 02/14] completed script (might updated formatting of print statements, but script works) --- .../commands/remove_unused_portfolios.py | 122 ++++++++++++++++-- src/registrar/models/portfolio.py | 2 +- 2 files changed, 113 insertions(+), 11 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index bd75ca7b0..9f8beb3a4 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -8,6 +8,14 @@ TerminalColors, TerminalHelper, ) +from registrar.models import ( + DomainGroup, + DomainInformation, + DomainRequest, + PortfolioInvitation, + Suborganization, + UserPortfolioPermission +) logger = logging.getLogger(__name__) ALLOWED_PORTFOLIOS = [ @@ -28,6 +36,8 @@ "Capitol Police", "Administrative Office of the Courts", "Supreme Court of the United States", + # "Hotel California", # for testing + # "Wish You Were Here" # for testing ] class Command(BaseCommand): @@ -100,28 +110,120 @@ def delete_entries(self, portfolios_to_delete, debug_on): {TerminalColors.ENDC} """ ) + + + # Check for portfolios with non-empty related objects + # (These will throw integrity errors if they are not updated) + portfolios_with_assignments = [] + for portfolio in portfolios_to_delete: + has_assignments = any([ + portfolio.information_portfolio.exists(), + DomainGroup.objects.filter(portfolio=portfolio).exists(), + DomainInformation.objects.filter(portfolio=portfolio).exists(), + DomainRequest.objects.filter(portfolio=portfolio).exists(), + PortfolioInvitation.objects.filter(portfolio=portfolio).exists(), + Suborganization.objects.filter(portfolio=portfolio).exists(), + UserPortfolioPermission.objects.filter(portfolio=portfolio).exists() + ]) + if has_assignments: + portfolios_with_assignments.append(portfolio) + + if portfolios_with_assignments: + formatted_entries = "\n\t\t".join( + f"{portfolio.organization_name}" for portfolio in portfolios_with_assignments + ) + confirm_integrity_error = TerminalHelper.query_yes_no( + f""" + {TerminalColors.FAIL} + WARNING: these entries have related objects. + + {formatted_entries} + + Deleting them will update any associated domains / domain requests to have no portfolio + and will cascade delete any associated portfolio invitations, portfolio permissions, + and suborganizations. Any suborganizations that get deleted will also orphan (not delete) their + associated domains / domain requests. - # Delete the entries + Are you sure you want to continue?{TerminalColors.ENDC}""" + ) + if not confirm_integrity_error: + logger.info( + f"""{TerminalColors.OKCYAN} + Operation canceled by the user. + {TerminalColors.ENDC} + """ + ) + return + + # Try to delete the portfolios try: - portfolios_to_delete.delete() - # Output a success message + summary = [] + for portfolio in portfolios_to_delete: + portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"] + if portfolio in portfolios_with_assignments: + domain_groups = DomainGroup.objects.filter(portfolio=portfolio) + domain_informations = DomainInformation.objects.filter(portfolio=portfolio) + domain_requests = DomainRequest.objects.filter(portfolio=portfolio) + portfolio_invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) + suborganizations = Suborganization.objects.filter(portfolio=portfolio) + user_permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) + + if domain_groups.exists(): + domain_groups.update(portfolio=None) + portfolio_summary.append(f"Orphaned DomainGroups: {[group.name for group in domain_groups]}") + + if domain_informations.exists(): + domain_informations.update(portfolio=None) + portfolio_summary.append(f"Orphaned DomainInformations: {[info.id for info in domain_informations]}") + + if domain_requests.exists(): + domain_requests.update(portfolio=None) + portfolio_summary.append(f"Orphaned DomainRequests: {[req.requested_domain for req in domain_requests]}") + + if portfolio_invitations.exists(): + portfolio_summary.append(f"Deleted PortfolioInvitations: {[inv.id for inv in portfolio_invitations]}") + portfolio_invitations.delete() + + if user_permissions.exists(): + portfolio_summary.append(f"Deleted UserPortfolioPermissions for the following users: {[perm.user.get_formatted_name() for perm in user_permissions]}") + formatted_user_list = "\n".join([perm.user.get_formatted_name() for perm in user_permissions]) + portfolio_summary.append(f"{formatted_user_list}") + user_permissions.delete() + + if suborganizations.exists(): + for suborg in suborganizations: + DomainInformation.objects.filter(sub_organization=suborg).update(sub_organization=None) + DomainRequest.objects.filter(sub_organization=suborg).update(sub_organization=None) + portfolio_summary.append(f"...Cascade Deleted Suborganization: {suborg.name}") + suborg.delete() + + portfolio.delete() + summary.append("\n\n".join(portfolio_summary)) + summary_string = "\n\n".join(summary) + + # Output a success message with detailed summary logger.info( - f"""{TerminalColors.OKGREEN} - Successfully removed {count} entries. + f"""{TerminalColors.OKCYAN} + Successfully removed {count} portfolios. + + The following portfolio deletions had cascading effects; + + {summary_string} {TerminalColors.ENDC} - """ - ) + """) + except IntegrityError as e: logger.info( f"""{TerminalColors.FAIL} - Could not delete some entries due to protected relationships + Could not delete some portfolios due to integrity constraints: + + {e} + {TerminalColors.ENDC} """ ) - - def handle(self, *args, **options): # Get all Portfolio entries not in the allowed portfolios list portfolios_to_delete = Portfolio.objects.exclude(organization_name__in=ALLOWED_PORTFOLIOS) diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 82afcd4d6..a502278e7 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -162,4 +162,4 @@ def get_domain_requests(self, order_by=None): # == Getters for suborganization == # def get_suborganizations(self): """Returns all suborganizations associated with this portfolio""" - return self.portfolio_suborganizations.all() + return self.d.all() From ff011137a5c53bba1775f67cbd9c167260e07833 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 21 Jan 2025 22:43:48 -0700 Subject: [PATCH 03/14] revert mis-type --- src/registrar/models/portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index a502278e7..82afcd4d6 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -162,4 +162,4 @@ def get_domain_requests(self, order_by=None): # == Getters for suborganization == # def get_suborganizations(self): """Returns all suborganizations associated with this portfolio""" - return self.d.all() + return self.portfolio_suborganizations.all() From 41b6e9bed320aac55336a2c49396b13500617053 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 21 Jan 2025 22:45:00 -0700 Subject: [PATCH 04/14] rename --- src/registrar/management/commands/remove_unused_portfolios.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 9f8beb3a4..650824d7d 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -132,7 +132,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): formatted_entries = "\n\t\t".join( f"{portfolio.organization_name}" for portfolio in portfolios_with_assignments ) - confirm_integrity_error = TerminalHelper.query_yes_no( + confirm_cascade_delete = TerminalHelper.query_yes_no( f""" {TerminalColors.FAIL} WARNING: these entries have related objects. @@ -146,7 +146,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): Are you sure you want to continue?{TerminalColors.ENDC}""" ) - if not confirm_integrity_error: + if not confirm_cascade_delete: logger.info( f"""{TerminalColors.OKCYAN} Operation canceled by the user. From d445a406f30040bb609894a6f949f701b029afab Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 11:30:11 -0700 Subject: [PATCH 05/14] linted --- .../commands/remove_unused_portfolios.py | 68 ++++++++++--------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 650824d7d..0253e12ab 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -3,7 +3,7 @@ from django.core.management.base import BaseCommand from django.db import IntegrityError -from registrar.models import Portfolio +from registrar.models import Portfolio from registrar.management.commands.utility.terminal_helper import ( TerminalColors, TerminalHelper, @@ -14,8 +14,9 @@ DomainRequest, PortfolioInvitation, Suborganization, - UserPortfolioPermission + UserPortfolioPermission, ) + logger = logging.getLogger(__name__) ALLOWED_PORTFOLIOS = [ @@ -40,9 +41,9 @@ # "Wish You Were Here" # for testing ] -class Command(BaseCommand): - help = 'Remove all Portfolio entries with names not in the allowed list.' +class Command(BaseCommand): + help = "Remove all Portfolio entries with names not in the allowed list." def add_arguments(self, parser): """ @@ -52,7 +53,6 @@ def add_arguments(self, parser): """ parser.add_argument("--debug", action=argparse.BooleanOptionalAction) - def prompt_delete_entries(self, portfolios_to_delete, debug_on): """Brings up a prompt in the terminal asking if the user wishes to delete data in the @@ -80,15 +80,13 @@ def prompt_delete_entries(self, portfolios_to_delete, debug_on): self.delete_entries(portfolios_to_delete, debug_on) else: logger.info( - f"""{TerminalColors.OKCYAN} + f"""{TerminalColors.OKCYAN} ----------No entries deleted---------- (exiting script) {TerminalColors.ENDC}""" ) - - - def delete_entries(self, portfolios_to_delete, debug_on): + def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 # Log the number of entries being removed count = portfolios_to_delete.count() if count == 0: @@ -110,21 +108,22 @@ def delete_entries(self, portfolios_to_delete, debug_on): {TerminalColors.ENDC} """ ) - - + # Check for portfolios with non-empty related objects # (These will throw integrity errors if they are not updated) portfolios_with_assignments = [] for portfolio in portfolios_to_delete: - has_assignments = any([ - portfolio.information_portfolio.exists(), - DomainGroup.objects.filter(portfolio=portfolio).exists(), - DomainInformation.objects.filter(portfolio=portfolio).exists(), - DomainRequest.objects.filter(portfolio=portfolio).exists(), - PortfolioInvitation.objects.filter(portfolio=portfolio).exists(), - Suborganization.objects.filter(portfolio=portfolio).exists(), - UserPortfolioPermission.objects.filter(portfolio=portfolio).exists() - ]) + has_assignments = any( + [ + portfolio.information_portfolio.exists(), + DomainGroup.objects.filter(portfolio=portfolio).exists(), + DomainInformation.objects.filter(portfolio=portfolio).exists(), + DomainRequest.objects.filter(portfolio=portfolio).exists(), + PortfolioInvitation.objects.filter(portfolio=portfolio).exists(), + Suborganization.objects.filter(portfolio=portfolio).exists(), + UserPortfolioPermission.objects.filter(portfolio=portfolio).exists(), + ] + ) if has_assignments: portfolios_with_assignments.append(portfolio) @@ -135,7 +134,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): confirm_cascade_delete = TerminalHelper.query_yes_no( f""" {TerminalColors.FAIL} - WARNING: these entries have related objects. + WARNING: these entries have related objects. {formatted_entries} @@ -155,7 +154,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): ) return - # Try to delete the portfolios + # Try to delete the portfolios try: summary = [] for portfolio in portfolios_to_delete: @@ -174,18 +173,27 @@ def delete_entries(self, portfolios_to_delete, debug_on): if domain_informations.exists(): domain_informations.update(portfolio=None) - portfolio_summary.append(f"Orphaned DomainInformations: {[info.id for info in domain_informations]}") + portfolio_summary.append( + f"Orphaned DomainInformations: {[info.id for info in domain_informations]}" + ) if domain_requests.exists(): domain_requests.update(portfolio=None) - portfolio_summary.append(f"Orphaned DomainRequests: {[req.requested_domain for req in domain_requests]}") + portfolio_summary.append( + f"Orphaned DomainRequests: {[req.requested_domain for req in domain_requests]}" + ) if portfolio_invitations.exists(): - portfolio_summary.append(f"Deleted PortfolioInvitations: {[inv.id for inv in portfolio_invitations]}") + portfolio_summary.append( + f"Deleted PortfolioInvitations: {[inv.id for inv in portfolio_invitations]}" + ) portfolio_invitations.delete() if user_permissions.exists(): - portfolio_summary.append(f"Deleted UserPortfolioPermissions for the following users: {[perm.user.get_formatted_name() for perm in user_permissions]}") + portfolio_summary.append( + f"""Deleted UserPortfolioPermissions for the following users: + {[perm.user.get_formatted_name() for perm in user_permissions]}""" + ) formatted_user_list = "\n".join([perm.user.get_formatted_name() for perm in user_permissions]) portfolio_summary.append(f"{formatted_user_list}") user_permissions.delete() @@ -210,20 +218,18 @@ def delete_entries(self, portfolios_to_delete, debug_on): {summary_string} {TerminalColors.ENDC} - """) + """ + ) except IntegrityError as e: logger.info( f"""{TerminalColors.FAIL} - Could not delete some portfolios due to integrity constraints: - + Could not delete some portfolios due to integrity constraints: {e} - {TerminalColors.ENDC} """ ) - def handle(self, *args, **options): # Get all Portfolio entries not in the allowed portfolios list portfolios_to_delete = Portfolio.objects.exclude(organization_name__in=ALLOWED_PORTFOLIOS) From 24c41b4815ec2e736f2a0a7f819ea91f74392ebe Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 16:16:25 -0700 Subject: [PATCH 06/14] added unit test --- .../tests/test_management_scripts.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 536d1e760..5c41c9d0d 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -3,6 +3,7 @@ from datetime import date, datetime, time from django.core.management import call_command from django.test import TestCase, override_settings +from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models.senior_official import SeniorOfficial from registrar.utility.constants import BranchChoices from django.utils import timezone @@ -27,6 +28,9 @@ FederalAgency, Portfolio, Suborganization, + DomainGroup, + PortfolioInvitation, + UserPortfolioPermission ) import tablib from unittest.mock import patch, call, MagicMock, mock_open @@ -2167,3 +2171,89 @@ def test_reference_updates(self): self.assertEqual(self.domain_information_1.sub_organization, keep_org) self.assertEqual(self.domain_request_2.sub_organization, unrelated_org) self.assertEqual(self.domain_information_2.sub_organization, unrelated_org) + + +class TestRemovePortfolios(TestCase): + """Test the remove_unused_portfolios command""" + + def setUp(self): + self.user = User.objects.create(username="testuser") + + self.logger_patcher = patch("registrar.management.commands.export_tables.logger") + self.logger_mock = self.logger_patcher.start() + + # Create mock database objects + self.portfolio_ok = Portfolio.objects.create(organization_name="Department of Veterans Affairs", creator=self.user) + self.unused_portfolio_with_related_objects = Portfolio.objects.create(organization_name="Test with orphaned objects", creator=self.user) + self.unused_portfolio_with_suborgs = Portfolio.objects.create(organization_name="Test with suborg", creator=self.user) + + # Create related objects for unused_portfolio_with_related_objects + self.domain_information = DomainInformation.objects.create(portfolio=self.unused_portfolio_with_related_objects, creator=self.user) + self.domain_request = DomainRequest.objects.create(portfolio=self.unused_portfolio_with_related_objects, creator=self.user) + + # Create a suborganization and suborg related objects for unused_portfolio_with_suborgs + self.suborganization = Suborganization.objects.create(portfolio=self.unused_portfolio_with_suborgs, name="Test Suborg") + self.suborg_domain_information = DomainInformation.objects.create(sub_organization=self.suborganization, creator=self.user) + + def tearDown(self): + self.logger_patcher.stop() + + @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no") + def test_delete_unlisted_portfolios(self, mock_query_yes_no): + """Test that portfolios not on the allowed list are deleted.""" + mock_query_yes_no.return_value = True + + # Ensure all portfolios exist before running the command + self.assertEqual(Portfolio.objects.count(), 3) + + # Run the command + call_command("remove_unused_portfolios", debug=False) + + # Check that the unlisted portfolio was removed + self.assertEqual(Portfolio.objects.count(), 1) + self.assertFalse(Portfolio.objects.filter(organization_name="Test with orphaned objects").exists()) + self.assertFalse(Portfolio.objects.filter(organization_name="Test with orphaned objects").exists()) + self.assertTrue(Portfolio.objects.filter(organization_name="Department of Veterans Affairs").exists()) + + @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no") + def test_delete_entries_with_related_objects(self, mock_query_yes_no): + """Test deletion with related objects being handled properly.""" + mock_query_yes_no.return_value = True + + # Ensure related objects exist before running the command + self.assertEqual(DomainInformation.objects.count(), 2) + self.assertEqual(DomainRequest.objects.count(), 1) + + # Run the command + call_command("remove_unused_portfolios", debug=False) + + # Check that related objects were updated + self.assertEqual(DomainInformation.objects.filter(portfolio=self.unused_portfolio_with_related_objects).count(), 0) + self.assertEqual(DomainRequest.objects.filter(portfolio=self.unused_portfolio_with_related_objects).count(), 0) + self.assertEqual(DomainInformation.objects.filter(portfolio=None).count(), 2) + self.assertEqual(DomainRequest.objects.filter(portfolio=None).count(), 1) + + # Check that the portfolio was deleted + self.assertFalse(Portfolio.objects.filter(organization_name="Test with orphaned objects").exists()) + + @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no") + def test_delete_entries_with_suborganizations(self, mock_query_yes_no): + """Test that suborganizations and their related objects are deleted along with the portfolio.""" + mock_query_yes_no.return_value = True + + # Ensure suborganization and related objects exist before running the command + self.assertEqual(Suborganization.objects.count(), 1) + self.assertEqual(DomainInformation.objects.filter(sub_organization=self.suborganization).count(), 1) + + # Run the command + call_command("remove_unused_portfolios", debug=False) + + # Check that the suborganization was deleted + self.assertEqual(Suborganization.objects.filter(portfolio=self.unused_portfolio_with_suborgs).count(), 0) + + # Check that deletion of suborganization had cascading effects (orphaned DomainInformation) + self.assertEqual(DomainInformation.objects.filter(sub_organization=self.suborganization).count(), 0) + + # Check that the portfolio was deleted + self.assertFalse(Portfolio.objects.filter(organization_name="Test with suborg").exists()) + From bf6748106f0f56e19b219ecba561c429af525860 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 16:50:53 -0700 Subject: [PATCH 07/14] fixed print statements --- .../commands/remove_unused_portfolios.py | 153 +++++++++--------- 1 file changed, 80 insertions(+), 73 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 0253e12ab..96c10ad49 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -3,12 +3,13 @@ from django.core.management.base import BaseCommand from django.db import IntegrityError -from registrar.models import Portfolio +from django.db import transaction from registrar.management.commands.utility.terminal_helper import ( TerminalColors, TerminalHelper, ) from registrar.models import ( + Portfolio, DomainGroup, DomainInformation, DomainRequest, @@ -154,81 +155,87 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 ) return - # Try to delete the portfolios - try: - summary = [] - for portfolio in portfolios_to_delete: - portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"] - if portfolio in portfolios_with_assignments: - domain_groups = DomainGroup.objects.filter(portfolio=portfolio) - domain_informations = DomainInformation.objects.filter(portfolio=portfolio) - domain_requests = DomainRequest.objects.filter(portfolio=portfolio) - portfolio_invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) - suborganizations = Suborganization.objects.filter(portfolio=portfolio) - user_permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) - - if domain_groups.exists(): - domain_groups.update(portfolio=None) - portfolio_summary.append(f"Orphaned DomainGroups: {[group.name for group in domain_groups]}") - - if domain_informations.exists(): - domain_informations.update(portfolio=None) - portfolio_summary.append( - f"Orphaned DomainInformations: {[info.id for info in domain_informations]}" - ) - - if domain_requests.exists(): - domain_requests.update(portfolio=None) - portfolio_summary.append( - f"Orphaned DomainRequests: {[req.requested_domain for req in domain_requests]}" - ) - - if portfolio_invitations.exists(): - portfolio_summary.append( - f"Deleted PortfolioInvitations: {[inv.id for inv in portfolio_invitations]}" - ) - portfolio_invitations.delete() - - if user_permissions.exists(): - portfolio_summary.append( - f"""Deleted UserPortfolioPermissions for the following users: - {[perm.user.get_formatted_name() for perm in user_permissions]}""" - ) - formatted_user_list = "\n".join([perm.user.get_formatted_name() for perm in user_permissions]) - portfolio_summary.append(f"{formatted_user_list}") - user_permissions.delete() - - if suborganizations.exists(): - for suborg in suborganizations: - DomainInformation.objects.filter(sub_organization=suborg).update(sub_organization=None) - DomainRequest.objects.filter(sub_organization=suborg).update(sub_organization=None) - portfolio_summary.append(f"...Cascade Deleted Suborganization: {suborg.name}") - suborg.delete() - - portfolio.delete() - summary.append("\n\n".join(portfolio_summary)) - summary_string = "\n\n".join(summary) - - # Output a success message with detailed summary - logger.info( - f"""{TerminalColors.OKCYAN} - Successfully removed {count} portfolios. + with transaction.atomic(): + # Try to delete the portfolios + try: + summary = [] + for portfolio in portfolios_to_delete: + portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"] + if portfolio in portfolios_with_assignments: + domain_groups = DomainGroup.objects.filter(portfolio=portfolio) + domain_informations = DomainInformation.objects.filter(portfolio=portfolio) + domain_requests = DomainRequest.objects.filter(portfolio=portfolio) + portfolio_invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) + suborganizations = Suborganization.objects.filter(portfolio=portfolio) + user_permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) + + if domain_groups.exists(): + formatted_groups = "\n".join([group.name for group in domain_groups]) + portfolio_summary.append( + f"{len(domain_groups)} Orphaned DomainGroups:\n{formatted_groups}" + ) + domain_groups.update(portfolio=None) + + if domain_informations.exists(): + formatted_domain_infos = "\n".join([str(info) for info in domain_informations]) + portfolio_summary.append( + f"{len(domain_informations)} Orphaned DomainInformations:\n{formatted_domain_infos}" + ) + domain_informations.update(portfolio=None) + + if domain_requests.exists(): + formatted_domain_reqs = "\n".join([str(req) for req in domain_requests]) + portfolio_summary.append( + f"{len(domain_requests)} Orphaned DomainRequests:\n{formatted_domain_reqs}" + ) + domain_requests.update(portfolio=None) + + if portfolio_invitations.exists(): + formatted_portfolio_invitations = "\n".join([str(inv) for inv in portfolio_invitations]) + portfolio_summary.append( + f"{len(portfolio_invitations)} Deleted PortfolioInvitations:\n{formatted_portfolio_invitations}" + ) + portfolio_invitations.delete() + + if user_permissions.exists(): + formatted_user_list = "\n".join([perm.user.get_formatted_name() for perm in user_permissions]) + portfolio_summary.append( + f"Deleted UserPortfolioPermissions for the following users:\n{formatted_user_list}" + ) + user_permissions.delete() + + if suborganizations.exists(): + portfolio_summary.append(f"Cascade Deleted Suborganizations:") + for suborg in suborganizations: + DomainInformation.objects.filter(sub_organization=suborg).update(sub_organization=None) + DomainRequest.objects.filter(sub_organization=suborg).update(sub_organization=None) + portfolio_summary.append(f"{suborg.name}\n") + suborg.delete() + + portfolio.delete() + summary.append("\n\n".join(portfolio_summary)) + summary_string = "\n\n".join(summary) + + # Output a success message with detailed summary + logger.info( + f"""{TerminalColors.OKCYAN} + Successfully removed {count} portfolios. - The following portfolio deletions had cascading effects; + The following portfolio deletions had cascading effects; - {summary_string} - {TerminalColors.ENDC} - """ - ) + {summary_string} + {TerminalColors.ENDC} + """ + ) - except IntegrityError as e: - logger.info( - f"""{TerminalColors.FAIL} - Could not delete some portfolios due to integrity constraints: - {e} - {TerminalColors.ENDC} - """ - ) + except IntegrityError as e: + logger.info( + f"""{TerminalColors.FAIL} + Could not delete some portfolios due to integrity constraints: + {e} + {TerminalColors.ENDC} + """ + ) def handle(self, *args, **options): # Get all Portfolio entries not in the allowed portfolios list From 8c8b1759cb345c756ecc9f5468cba5a2f01b85e0 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 16:52:12 -0700 Subject: [PATCH 08/14] remove redundant portfolio names --- src/registrar/management/commands/remove_unused_portfolios.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 96c10ad49..acb717ca5 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -34,12 +34,9 @@ "Department of the Interior", "Department of State", "Department of Justice", - "Department of Veterans Affairs", "Capitol Police", "Administrative Office of the Courts", "Supreme Court of the United States", - # "Hotel California", # for testing - # "Wish You Were Here" # for testing ] From bd20a0ce6883629a294ade3ead1c0787f8daf3f7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 17:00:32 -0700 Subject: [PATCH 09/14] merged with main and linted --- .../commands/remove_unused_portfolios.py | 12 +++--- .../tests/test_management_scripts.py | 37 ++++++++++++------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index acb717ca5..45651e826 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -168,9 +168,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 if domain_groups.exists(): formatted_groups = "\n".join([group.name for group in domain_groups]) - portfolio_summary.append( - f"{len(domain_groups)} Orphaned DomainGroups:\n{formatted_groups}" - ) + portfolio_summary.append(f"{len(domain_groups)} Orphaned DomainGroups:\n{formatted_groups}") domain_groups.update(portfolio=None) if domain_informations.exists(): @@ -190,19 +188,21 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 if portfolio_invitations.exists(): formatted_portfolio_invitations = "\n".join([str(inv) for inv in portfolio_invitations]) portfolio_summary.append( - f"{len(portfolio_invitations)} Deleted PortfolioInvitations:\n{formatted_portfolio_invitations}" + f"{len(portfolio_invitations)} Deleted PortfolioInvitations:\n{formatted_portfolio_invitations}" # noqa ) portfolio_invitations.delete() if user_permissions.exists(): - formatted_user_list = "\n".join([perm.user.get_formatted_name() for perm in user_permissions]) + formatted_user_list = "\n".join( + [perm.user.get_formatted_name() for perm in user_permissions] + ) portfolio_summary.append( f"Deleted UserPortfolioPermissions for the following users:\n{formatted_user_list}" ) user_permissions.delete() if suborganizations.exists(): - portfolio_summary.append(f"Cascade Deleted Suborganizations:") + portfolio_summary.append("Cascade Deleted Suborganizations:") for suborg in suborganizations: DomainInformation.objects.filter(sub_organization=suborg).update(sub_organization=None) DomainRequest.objects.filter(sub_organization=suborg).update(sub_organization=None) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 5c41c9d0d..3cd4ec961 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -3,7 +3,6 @@ from datetime import date, datetime, time from django.core.management import call_command from django.test import TestCase, override_settings -from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models.senior_official import SeniorOfficial from registrar.utility.constants import BranchChoices from django.utils import timezone @@ -28,9 +27,6 @@ FederalAgency, Portfolio, Suborganization, - DomainGroup, - PortfolioInvitation, - UserPortfolioPermission ) import tablib from unittest.mock import patch, call, MagicMock, mock_open @@ -2183,17 +2179,31 @@ def setUp(self): self.logger_mock = self.logger_patcher.start() # Create mock database objects - self.portfolio_ok = Portfolio.objects.create(organization_name="Department of Veterans Affairs", creator=self.user) - self.unused_portfolio_with_related_objects = Portfolio.objects.create(organization_name="Test with orphaned objects", creator=self.user) - self.unused_portfolio_with_suborgs = Portfolio.objects.create(organization_name="Test with suborg", creator=self.user) + self.portfolio_ok = Portfolio.objects.create( + organization_name="Department of Veterans Affairs", creator=self.user + ) + self.unused_portfolio_with_related_objects = Portfolio.objects.create( + organization_name="Test with orphaned objects", creator=self.user + ) + self.unused_portfolio_with_suborgs = Portfolio.objects.create( + organization_name="Test with suborg", creator=self.user + ) # Create related objects for unused_portfolio_with_related_objects - self.domain_information = DomainInformation.objects.create(portfolio=self.unused_portfolio_with_related_objects, creator=self.user) - self.domain_request = DomainRequest.objects.create(portfolio=self.unused_portfolio_with_related_objects, creator=self.user) + self.domain_information = DomainInformation.objects.create( + portfolio=self.unused_portfolio_with_related_objects, creator=self.user + ) + self.domain_request = DomainRequest.objects.create( + portfolio=self.unused_portfolio_with_related_objects, creator=self.user + ) # Create a suborganization and suborg related objects for unused_portfolio_with_suborgs - self.suborganization = Suborganization.objects.create(portfolio=self.unused_portfolio_with_suborgs, name="Test Suborg") - self.suborg_domain_information = DomainInformation.objects.create(sub_organization=self.suborganization, creator=self.user) + self.suborganization = Suborganization.objects.create( + portfolio=self.unused_portfolio_with_suborgs, name="Test Suborg" + ) + self.suborg_domain_information = DomainInformation.objects.create( + sub_organization=self.suborganization, creator=self.user + ) def tearDown(self): self.logger_patcher.stop() @@ -2228,7 +2238,9 @@ def test_delete_entries_with_related_objects(self, mock_query_yes_no): call_command("remove_unused_portfolios", debug=False) # Check that related objects were updated - self.assertEqual(DomainInformation.objects.filter(portfolio=self.unused_portfolio_with_related_objects).count(), 0) + self.assertEqual( + DomainInformation.objects.filter(portfolio=self.unused_portfolio_with_related_objects).count(), 0 + ) self.assertEqual(DomainRequest.objects.filter(portfolio=self.unused_portfolio_with_related_objects).count(), 0) self.assertEqual(DomainInformation.objects.filter(portfolio=None).count(), 2) self.assertEqual(DomainRequest.objects.filter(portfolio=None).count(), 1) @@ -2256,4 +2268,3 @@ def test_delete_entries_with_suborganizations(self, mock_query_yes_no): # Check that the portfolio was deleted self.assertFalse(Portfolio.objects.filter(organization_name="Test with suborg").exists()) - From f56ccf95da50a0552b9d1c86c3be833139e399c4 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 17:28:46 -0700 Subject: [PATCH 10/14] updated docs --- docs/operations/data_migration.md | 37 +++++++++++++++++++ .../commands/remove_unused_portfolios.py | 1 - 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 499c0840f..b64b5ea76 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -953,3 +953,40 @@ To create a specific portfolio: #### Step 1: Running the script ```docker-compose exec app ./manage.py patch_suborganizations``` + + +## Remove Non-whitelisted Portfolios +This script removes Portfolio entries from the database that are not part of a predefined list of allowed portfolios (`ALLOWED_PORTFOLIOS`). +It performs the following actions: +1. Prompts the user for confirmation before proceeding with deletions. +2. Updates related objects such as `DomainInformation`, `Domain`, and `DomainRequest` to set their `portfolio` field to `None` to prevent integrity errors. +3. Deletes associated objects such as `PortfolioInvitation`, `UserPortfolioPermission`, and `Suborganization`. +4. Logs a detailed summary of all cascading deletions and orphaned objects. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-nl` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Running the script +To remove portfolios: +```./manage.py remove_unused_portfolios``` + +If you wish to enable debug mode for additional logging: +```./manage.py remove_unused_portfolios --debug``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py remove_unused_portfolios``` + +To enable debug mode locally: +```docker-compose exec app ./manage.py remove_unused_portfolios --debug``` \ No newline at end of file diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 45651e826..acbfa805b 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -113,7 +113,6 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 for portfolio in portfolios_to_delete: has_assignments = any( [ - portfolio.information_portfolio.exists(), DomainGroup.objects.filter(portfolio=portfolio).exists(), DomainInformation.objects.filter(portfolio=portfolio).exists(), DomainRequest.objects.filter(portfolio=portfolio).exists(), From af1ccd625f0efeb5fba1cb1409e23ef6a8d6ee1f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 22 Jan 2025 17:30:19 -0700 Subject: [PATCH 11/14] unit test fix --- src/registrar/tests/test_management_scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 3cd4ec961..0c9ef0164 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -2222,7 +2222,7 @@ def test_delete_unlisted_portfolios(self, mock_query_yes_no): # Check that the unlisted portfolio was removed self.assertEqual(Portfolio.objects.count(), 1) self.assertFalse(Portfolio.objects.filter(organization_name="Test with orphaned objects").exists()) - self.assertFalse(Portfolio.objects.filter(organization_name="Test with orphaned objects").exists()) + self.assertFalse(Portfolio.objects.filter(organization_name="Test with suborg").exists()) self.assertTrue(Portfolio.objects.filter(organization_name="Department of Veterans Affairs").exists()) @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no") From 898406831283c40bf4c2631e0c5a28636463aac3 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 23 Jan 2025 13:26:37 -0700 Subject: [PATCH 12/14] Fixed DomainGroup handling --- .../management/commands/remove_unused_portfolios.py | 8 ++++---- src/registrar/tests/test_management_scripts.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index acbfa805b..2cc615084 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -136,7 +136,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 {formatted_entries} Deleting them will update any associated domains / domain requests to have no portfolio - and will cascade delete any associated portfolio invitations, portfolio permissions, + and will cascade delete any associated portfolio invitations, portfolio permissions, domain groups, and suborganizations. Any suborganizations that get deleted will also orphan (not delete) their associated domains / domain requests. @@ -166,9 +166,9 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 user_permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) if domain_groups.exists(): - formatted_groups = "\n".join([group.name for group in domain_groups]) - portfolio_summary.append(f"{len(domain_groups)} Orphaned DomainGroups:\n{formatted_groups}") - domain_groups.update(portfolio=None) + formatted_groups = "\n".join([str(group) for group in domain_groups]) + portfolio_summary.append(f"{len(domain_groups)} Deleted DomainGroups:\n{formatted_groups}") + domain_groups.delete() if domain_informations.exists(): formatted_domain_infos = "\n".join([str(info) for info in domain_informations]) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 0c9ef0164..f1afc0e7f 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -3,7 +3,10 @@ from datetime import date, datetime, time from django.core.management import call_command from django.test import TestCase, override_settings +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 from registrar.utility.constants import BranchChoices from django.utils import timezone from django.utils.module_loading import import_string @@ -2196,6 +2199,16 @@ def setUp(self): self.domain_request = DomainRequest.objects.create( portfolio=self.unused_portfolio_with_related_objects, creator=self.user ) + self.inv = PortfolioInvitation.objects.create( + portfolio=self.unused_portfolio_with_related_objects + ) + self.group = DomainGroup.objects.create( + portfolio=self.unused_portfolio_with_related_objects, + name="Test Domain Group" + ) + self.perm = UserPortfolioPermission.objects.create( + portfolio=self.unused_portfolio_with_related_objects, user=self.user + ) # Create a suborganization and suborg related objects for unused_portfolio_with_suborgs self.suborganization = Suborganization.objects.create( From 85c6aa348e4001f72c8b3ac392de7493fdea43dd Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 23 Jan 2025 15:14:03 -0700 Subject: [PATCH 13/14] linted --- src/registrar/tests/test_management_scripts.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index f1afc0e7f..334d7d83c 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -2199,12 +2199,9 @@ def setUp(self): self.domain_request = DomainRequest.objects.create( portfolio=self.unused_portfolio_with_related_objects, creator=self.user ) - self.inv = PortfolioInvitation.objects.create( - portfolio=self.unused_portfolio_with_related_objects - ) + self.inv = PortfolioInvitation.objects.create(portfolio=self.unused_portfolio_with_related_objects) self.group = DomainGroup.objects.create( - portfolio=self.unused_portfolio_with_related_objects, - name="Test Domain Group" + portfolio=self.unused_portfolio_with_related_objects, name="Test Domain Group" ) self.perm = UserPortfolioPermission.objects.create( portfolio=self.unused_portfolio_with_related_objects, user=self.user From 9cd9b97e874c5636616874e989818a4bc461717b Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 24 Jan 2025 12:25:46 -0700 Subject: [PATCH 14/14] removed duplicate names and slight formatting for print statements --- src/registrar/management/commands/remove_unused_portfolios.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 2cc615084..4940cc16f 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -25,12 +25,10 @@ "Department of the Treasury", "National Archives and Records Administration", "Department of Defense", - "Department of Defense", "Office of Personnel Management", "National Aeronautics and Space Administration", "City and County of San Francisco", "State of Arizona, Executive Branch", - "State of Arizona, Executive Branch", "Department of the Interior", "Department of State", "Department of Justice", @@ -205,7 +203,7 @@ def delete_entries(self, portfolios_to_delete, debug_on): # noqa: C901 for suborg in suborganizations: DomainInformation.objects.filter(sub_organization=suborg).update(sub_organization=None) DomainRequest.objects.filter(sub_organization=suborg).update(sub_organization=None) - portfolio_summary.append(f"{suborg.name}\n") + portfolio_summary.append(f"{suborg.name}") suborg.delete() portfolio.delete()