diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a92e4c695..6f455fc89 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -593,12 +593,6 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): return filters -class UserContactInline(admin.StackedInline): - """Edit a user's profile on the user page.""" - - model = models.Contact - - class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): """Custom user admin class to use our inlines.""" @@ -615,8 +609,6 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): _meta = Meta() - inlines = [UserContactInline] - list_display = ( "username", "overridden_email_field", @@ -894,30 +886,20 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): list_display = [ "name", "email", - "user_exists", ] # this ordering effects the ordering of results - # in autocomplete_fields for user + # in autocomplete_fields ordering = ["first_name", "last_name", "email"] fieldsets = [ ( None, - {"fields": ["user", "first_name", "middle_name", "last_name", "title", "email", "phone"]}, + {"fields": ["first_name", "middle_name", "last_name", "title", "email", "phone"]}, ) ] - autocomplete_fields = ["user"] - change_form_template = "django/admin/email_clipboard_change_form.html" - def user_exists(self, obj): - """Check if the Contact has a related User""" - return "Yes" if obj.user is not None else "No" - - user_exists.short_description = "Is user" # type: ignore - user_exists.admin_order_field = "user" # type: ignore - # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it # This gets around the linter limitation, for now. @@ -935,9 +917,7 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): name.admin_order_field = "first_name" # type: ignore # Read only that we'll leverage for CISA Analysts - analyst_readonly_fields = [ - "user", - ] + analyst_readonly_fields = [] def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. diff --git a/src/registrar/management/commands/copy_names_from_contacts_to_users.py b/src/registrar/management/commands/copy_names_from_contacts_to_users.py deleted file mode 100644 index 384029400..000000000 --- a/src/registrar/management/commands/copy_names_from_contacts_to_users.py +++ /dev/null @@ -1,242 +0,0 @@ -import logging -import argparse -import sys - -from django.core.management import BaseCommand - -from registrar.management.commands.utility.terminal_helper import ( - TerminalColors, - TerminalHelper, -) -from registrar.models.contact import Contact -from registrar.models.user import User -from registrar.models.utility.domain_helper import DomainHelper - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = """Copy first and last names from a contact to - a related user if it exists and if its first and last name - properties are null or blank strings.""" - - # ====================================================== - # ===================== ARGUMENTS ===================== - # ====================================================== - def add_arguments(self, parser): - parser.add_argument("--debug", action=argparse.BooleanOptionalAction) - - # ====================================================== - # ===================== PRINTING ====================== - # ====================================================== - def print_debug_mode_statements(self, debug_on: bool): - """Prints additional terminal statements to indicate if --debug - or --limitParse are in use""" - TerminalHelper.print_conditional( - debug_on, - f"""{TerminalColors.OKCYAN} - ----------DEBUG MODE ON---------- - Detailed print statements activated. - {TerminalColors.ENDC} - """, - ) - - def print_summary_of_findings( - self, - skipped_contacts, - eligible_users, - processed_users, - debug_on, - ): - """Prints to terminal a summary of findings from - copying first and last names from contacts to users""" - - total_eligible_users = len(eligible_users) - total_skipped_contacts = len(skipped_contacts) - total_processed_users = len(processed_users) - - logger.info( - f"""{TerminalColors.OKGREEN} - ============= FINISHED =============== - Skipped {total_skipped_contacts} contacts - Found {total_eligible_users} users linked to contacts - Processed {total_processed_users} users - {TerminalColors.ENDC} - """ # noqa - ) - - # DEBUG: - TerminalHelper.print_conditional( - debug_on, - f"""{TerminalColors.YELLOW} - ======= DEBUG OUTPUT ======= - Users who have a linked contact: - {eligible_users} - - Processed users (users who have a linked contact and a missing first or last name): - {processed_users} - - ===== SKIPPED CONTACTS ===== - {skipped_contacts} - - {TerminalColors.ENDC} - """, - ) - - # ====================================================== - # =================== USER ===================== - # ====================================================== - def update_user(self, contact: Contact, debug_on: bool): - """Given a contact with a first_name and last_name, find & update an existing - corresponding user if her first_name and last_name are null. - - Returns tuple of eligible (is linked to the contact) and processed - (first and last are blank) users. - """ - - user_exists = User.objects.filter(contact=contact).exists() - if user_exists: - try: - # ----------------------- UPDATE USER ----------------------- - # ---- GET THE USER - eligible_user = User.objects.get(contact=contact) - processed_user = None - # DEBUG: - TerminalHelper.print_conditional( - debug_on, - f"""{TerminalColors.YELLOW} - > Found linked user for contact: - {contact} {contact.email} {contact.first_name} {contact.last_name} - > The linked user is {eligible_user} {eligible_user.username} - {TerminalColors.ENDC}""", # noqa - ) - - # Get the fields that exist on both User and Contact. Excludes id. - common_fields = DomainHelper.get_common_fields(User, Contact) - if "email" in common_fields: - # Don't change the email field. - common_fields.remove("email") - - for field in common_fields: - # Grab the value that contact has stored for this field - new_value = getattr(contact, field) - - # Set it on the user field - setattr(eligible_user, field, new_value) - - eligible_user.save() - processed_user = eligible_user - - return ( - eligible_user, - processed_user, - ) - - except Exception as error: - logger.warning( - f""" - {TerminalColors.FAIL} - !!! ERROR: An exception occured in the - User table for the following user: - {contact.email} {contact.first_name} {contact.last_name} - - Exception is: {error} - ----------TERMINATING----------""" - ) - sys.exit() - else: - return None, None - - # ====================================================== - # ================= PROCESS CONTACTS ================== - # ====================================================== - - def process_contacts( - self, - debug_on, - skipped_contacts=[], - eligible_users=[], - processed_users=[], - ): - for contact in Contact.objects.all(): - TerminalHelper.print_conditional( - debug_on, - f"{TerminalColors.OKCYAN}" - "Processing Contact: " - f"{contact.email}," - f" {contact.first_name}," - f" {contact.last_name}" - f"{TerminalColors.ENDC}", - ) - - # ====================================================== - # ====================== USER ======================= - (eligible_user, processed_user) = self.update_user(contact, debug_on) - - debug_string = "" - if eligible_user: - # ---------------- UPDATED ---------------- - eligible_users.append(contact.email) - debug_string = f"eligible user: {eligible_user}" - if processed_user: - processed_users.append(contact.email) - debug_string = f"processed user: {processed_user}" - else: - skipped_contacts.append(contact.email) - debug_string = f"skipped user: {contact.email}" - - # DEBUG: - TerminalHelper.print_conditional( - debug_on, - (f"{TerminalColors.OKCYAN} {debug_string} {TerminalColors.ENDC}"), - ) - - return ( - skipped_contacts, - eligible_users, - processed_users, - ) - - # ====================================================== - # ===================== HANDLE ======================== - # ====================================================== - def handle( - self, - **options, - ): - """Parse entries in Contact table - and update valid corresponding entries in the - User table.""" - - # grab command line arguments and store locally... - debug_on = options.get("debug") - - self.print_debug_mode_statements(debug_on) - - logger.info( - f"""{TerminalColors.OKCYAN} - ========================== - Beginning Data Transfer - ========================== - {TerminalColors.ENDC}""" - ) - - logger.info( - f"""{TerminalColors.OKCYAN} - ========= Adding Domains and Domain Invitations ========= - {TerminalColors.ENDC}""" - ) - ( - skipped_contacts, - eligible_users, - processed_users, - ) = self.process_contacts( - debug_on, - ) - - self.print_summary_of_findings( - skipped_contacts, - eligible_users, - processed_users, - debug_on, - ) diff --git a/src/registrar/migrations/0110_remove_contact_registrar_c_user_id_4059c4_idx_and_more.py b/src/registrar/migrations/0110_remove_contact_registrar_c_user_id_4059c4_idx_and_more.py new file mode 100644 index 000000000..858210be7 --- /dev/null +++ b/src/registrar/migrations/0110_remove_contact_registrar_c_user_id_4059c4_idx_and_more.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.10 on 2024-07-02 19:52 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0109_domaininformation_sub_organization_and_more"), + ] + + operations = [ + migrations.RemoveIndex( + model_name="contact", + name="registrar_c_user_id_4059c4_idx", + ), + migrations.RemoveField( + model_name="contact", + name="user", + ), + ] diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index f7bae3491..903633749 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -14,17 +14,9 @@ class Contact(TimeStampedModel): """Contains meta information about this class""" indexes = [ - models.Index(fields=["user"]), models.Index(fields=["email"]), ] - user = models.OneToOneField( - "registrar.User", - null=True, - blank=True, - on_delete=models.SET_NULL, - ) - first_name = models.CharField( null=True, blank=True, @@ -103,38 +95,6 @@ class Contact(TimeStampedModel): def has_contact_info(self): return bool(self.title or self.email or self.phone) - def save(self, *args, **kwargs): - # Call the parent class's save method to perform the actual save - super().save(*args, **kwargs) - - if self.user: - updated = False - - # Update first name and last name if necessary - if not self.user.first_name or not self.user.last_name: - self.user.first_name = self.first_name - self.user.last_name = self.last_name - updated = True - - # Update middle_name if necessary - if not self.user.middle_name: - self.user.middle_name = self.middle_name - updated = True - - # Update phone if necessary - if not self.user.phone: - self.user.phone = self.phone - updated = True - - # Update title if necessary - if not self.user.title: - self.user.title = self.title - updated = True - - # Save user if any updates were made - if updated: - self.user.save() - def __str__(self): if self.first_name or self.last_name: return self.get_formatted_name() diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 65427bb65..a1532cd39 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -4036,9 +4036,7 @@ class TestContactAdmin(TestCase): readonly_fields = self.admin.get_readonly_fields(request) - expected_fields = [ - "user", - ] + expected_fields = [] self.assertEqual(readonly_fields, expected_fields) @@ -4054,15 +4052,18 @@ class TestContactAdmin(TestCase): self.assertEqual(readonly_fields, expected_fields) def test_change_view_for_joined_contact_five_or_less(self): - """Create a contact, join it to 4 domain requests. 5th join is user. + """Create a contact, join it to 4 domain requests. Assert that the warning on the contact form lists 4 joins.""" with less_console_noise(): self.client.force_login(self.superuser) # Create an instance of the model - contact, _ = Contact.objects.get_or_create(user=self.staffuser) + contact, _ = Contact.objects.get_or_create( + first_name="Henry", + last_name="McFakerson", + ) - # join it to 4 domain requests. The 5th join will be a user. + # join it to 4 domain requests. domain_request1 = completed_domain_request(submitter=contact, name="city1.gov") domain_request2 = completed_domain_request(submitter=contact, name="city2.gov") domain_request3 = completed_domain_request(submitter=contact, name="city3.gov") @@ -4085,24 +4086,26 @@ class TestContactAdmin(TestCase): f"domainrequest/{domain_request3.pk}/change/'>city3.gov" "
  • Joined to DomainRequest: city4.gov
  • " - "
  • Joined to User: first last staff@example.com
  • " "", ) def test_change_view_for_joined_contact_five_or_more(self): - """Create a contact, join it to 5 domain requests. 6th join is user. + """Create a contact, join it to 6 domain requests. Assert that the warning on the contact form lists 5 joins and a '1 more' ellispsis.""" with less_console_noise(): self.client.force_login(self.superuser) # Create an instance of the model # join it to 6 domain requests. - contact, _ = Contact.objects.get_or_create(user=self.staffuser) + contact, _ = Contact.objects.get_or_create( + first_name="Henry", + last_name="McFakerson", + ) domain_request1 = completed_domain_request(submitter=contact, name="city1.gov") domain_request2 = completed_domain_request(submitter=contact, name="city2.gov") domain_request3 = completed_domain_request(submitter=contact, name="city3.gov") domain_request4 = completed_domain_request(submitter=contact, name="city4.gov") domain_request5 = completed_domain_request(submitter=contact, name="city5.gov") + domain_request6 = completed_domain_request(submitter=contact, name="city6.gov") with patch("django.contrib.messages.warning") as mock_warning: # Use the test client to simulate the request response = self.client.get(reverse("admin:registrar_contact_change", args=[contact.pk])) diff --git a/src/registrar/tests/test_copy_names_from_contacts_to_users.py b/src/registrar/tests/test_copy_names_from_contacts_to_users.py deleted file mode 100644 index 7fcbede1e..000000000 --- a/src/registrar/tests/test_copy_names_from_contacts_to_users.py +++ /dev/null @@ -1,124 +0,0 @@ -from django.test import TestCase - -from registrar.models import ( - User, - Contact, -) - -from registrar.management.commands.copy_names_from_contacts_to_users import Command - - -class TestDataUpdates(TestCase): - def setUp(self): - """We cannot setup the user details because contacts will override the first and last names in its save method - so we will initiate the users, setup the contacts and link them, and leave the rest of the setup to the test(s). - """ - - self.user1 = User.objects.create(username="user1") - self.user2 = User.objects.create(username="user2") - self.user3 = User.objects.create(username="user3") - self.user4 = User.objects.create(username="user4") - # The last user created triggers the creation of a contact and attaches itself to it. See signals. - # This bs_user defuses that situation. - self.bs_user = User.objects.create() - - self.contact1 = Contact.objects.create( - user=self.user1, - email="email1@igorville.gov", - first_name="first1", - last_name="last1", - middle_name="middle1", - title="title1", - ) - self.contact2 = Contact.objects.create( - user=self.user2, - email="email2@igorville.gov", - first_name="first2", - last_name="last2", - middle_name="middle2", - title="title2", - ) - self.contact3 = Contact.objects.create( - user=self.user3, - email="email3@igorville.gov", - first_name="first3", - last_name="last3", - middle_name="middle3", - title="title3", - ) - self.contact4 = Contact.objects.create( - email="email4@igorville.gov", first_name="first4", last_name="last4", middle_name="middle4", title="title4" - ) - - self.command = Command() - - def tearDown(self): - """Clean up""" - # Delete users and contacts - User.objects.all().delete() - Contact.objects.all().delete() - - def test_script_updates_linked_users(self): - """Test the script that copies contact information to the user object""" - - # Set up the users' first and last names here so - # they that they don't get overwritten by Contact's save() - # User with no first or last names - self.user1.first_name = "" - self.user1.last_name = "" - self.user1.title = "dummytitle" - self.user1.middle_name = "dummymiddle" - self.user1.save() - - # User with a first name but no last name - self.user2.first_name = "First name but no last name" - self.user2.last_name = "" - self.user2.save() - - # User with a first and last name - self.user3.first_name = "An existing first name" - self.user3.last_name = "An existing last name" - self.user3.save() - - # Call the parent method the same way we do it in the script - skipped_contacts = [] - eligible_users = [] - processed_users = [] - ( - skipped_contacts, - eligible_users, - processed_users, - ) = self.command.process_contacts( - # Set debugging to False - False, - skipped_contacts, - eligible_users, - processed_users, - ) - - # Trigger DB refresh - self.user1.refresh_from_db() - self.user2.refresh_from_db() - self.user3.refresh_from_db() - - # Asserts - # The user that has no first and last names will get them from the contact - self.assertEqual(self.user1.first_name, "first1") - self.assertEqual(self.user1.last_name, "last1") - self.assertEqual(self.user1.middle_name, "middle1") - self.assertEqual(self.user1.title, "title1") - # The user that has a first but no last will be updated - self.assertEqual(self.user2.first_name, "first2") - self.assertEqual(self.user2.last_name, "last2") - self.assertEqual(self.user2.middle_name, "middle2") - self.assertEqual(self.user2.title, "title2") - # The user that has a first and a last will be updated - self.assertEqual(self.user3.first_name, "first3") - self.assertEqual(self.user3.last_name, "last3") - self.assertEqual(self.user3.middle_name, "middle3") - self.assertEqual(self.user3.title, "title3") - # The unlinked user will be left alone - self.assertEqual(self.user4.first_name, "") - self.assertEqual(self.user4.last_name, "") - self.assertEqual(self.user4.middle_name, None) - self.assertEqual(self.user4.title, None) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 9a29f5428..0c5ca9193 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1221,7 +1221,10 @@ class TestContact(TestCase): self.user, _ = User.objects.get_or_create( email=self.email, first_name="Jeff", last_name="Lebowski", phone="123456789" ) - self.contact, _ = Contact.objects.get_or_create(user=self.user) + self.contact, _ = Contact.objects.get_or_create( + first_name="Jeff", + last_name="Lebowski", + ) self.contact_as_so, _ = Contact.objects.get_or_create(email="newguy@igorville.gov") self.domain_request = DomainRequest.objects.create(creator=self.user, senior_official=self.contact_as_so) @@ -1234,9 +1237,6 @@ class TestContact(TestCase): def test_has_more_than_one_join(self): """Test the Contact model method, has_more_than_one_join""" - # test for a contact which has one user defined - self.assertFalse(self.contact.has_more_than_one_join("user")) - self.assertTrue(self.contact.has_more_than_one_join("senior_official")) # test for a contact which is assigned as a senior official on a domain request self.assertFalse(self.contact_as_so.has_more_than_one_join("senior_official")) self.assertTrue(self.contact_as_so.has_more_than_one_join("submitted_domain_requests")) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 42c7b2d38..b98a30e79 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -372,7 +372,10 @@ class HomeTests(TestWithUser): ) # Attach a user object to a contact (should not be deleted) - contact_user, _ = Contact.objects.get_or_create(user=self.user) + contact_user, _ = Contact.objects.get_or_create( + first_name="Hank", + last_name="McFakey", + ) site = DraftDomain.objects.create(name="igorville.gov") domain_request = DomainRequest.objects.create( @@ -405,17 +408,12 @@ class HomeTests(TestWithUser): igorville = DomainRequest.objects.filter(requested_domain__name="igorville.gov") self.assertFalse(igorville.exists()) - # Check if the orphaned contact was deleted + # Check if the orphaned contacts were deleted orphan = Contact.objects.filter(id=contact.id) self.assertFalse(orphan.exists()) + orphan = Contact.objects.filter(id=contact_user.id) + self.assertFalse(orphan.exists()) - # All non-orphan contacts should still exist and are unaltered - try: - current_user = Contact.objects.filter(id=contact_user.id).get() - except Contact.DoesNotExist: - self.fail("contact_user (a non-orphaned contact) was deleted") - - self.assertEqual(current_user, contact_user) try: edge_case = Contact.objects.filter(id=contact_2.id).get() except Contact.DoesNotExist: @@ -444,7 +442,10 @@ class HomeTests(TestWithUser): ) # Attach a user object to a contact (should not be deleted) - contact_user, _ = Contact.objects.get_or_create(user=self.user) + contact_user, _ = Contact.objects.get_or_create( + first_name="Hank", + last_name="McFakey", + ) site = DraftDomain.objects.create(name="igorville.gov") domain_request = DomainRequest.objects.create( @@ -863,7 +864,10 @@ class UserProfileTests(TestWithUser, WebTest): def test_request_when_profile_feature_on(self): """test that Your profile is in request page when profile feature is on""" - contact_user, _ = Contact.objects.get_or_create(user=self.user) + contact_user, _ = Contact.objects.get_or_create( + first_name="Hank", + last_name="McFakerson", + ) site = DraftDomain.objects.create(name="igorville.gov") domain_request = DomainRequest.objects.create( creator=self.user, @@ -882,7 +886,10 @@ class UserProfileTests(TestWithUser, WebTest): def test_request_when_profile_feature_off(self): """test that Your profile is not in request page when profile feature is off""" - contact_user, _ = Contact.objects.get_or_create(user=self.user) + contact_user, _ = Contact.objects.get_or_create( + first_name="Hank", + last_name="McFakerson", + ) site = DraftDomain.objects.create(name="igorville.gov") domain_request = DomainRequest.objects.create( creator=self.user, diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index e78dfe860..de924576b 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -2974,7 +2974,10 @@ class TestWizardUnlockingSteps(TestWithUser, WebTest): ) # Attach a user object to a contact (should not be deleted) - contact_user, _ = Contact.objects.get_or_create(user=self.user) + contact_user, _ = Contact.objects.get_or_create( + first_name="Hank", + last_name="McFakey", + ) site = DraftDomain.objects.create(name="igorville.gov") domain_request = DomainRequest.objects.create( diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index e8e82500e..a7d6aa6ae 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -812,15 +812,15 @@ class DomainRequestDeleteView(DomainRequestPermissionDeleteView): self.object = self.get_object() self.object.delete() - # Delete orphaned contacts - but only for if they are not associated with a user - Contact.objects.filter(id__in=contacts_to_delete, user=None).delete() + # Delete orphaned contacts + Contact.objects.filter(id__in=contacts_to_delete).delete() # After a delete occurs, do a second sweep on any returned duplicates. # This determines if any of these three fields share a contact, which is used for # the edge case where the same user may be an SO, and a submitter, for example. if len(duplicates) > 0: duplicates_to_delete, _ = self._get_orphaned_contacts(domain_request, check_db=True) - Contact.objects.filter(id__in=duplicates_to_delete, user=None).delete() + Contact.objects.filter(id__in=duplicates_to_delete).delete() # Return a 200 response with an empty body return HttpResponse(status=200)