Merge pull request #2094 from cisagov/rjm/2057-save-phone-number

Issue #2057: Manage user phone number - (no sandbox)

src/djangooidc/backends.py

@@ -65,13 +65,31 @@ class OpenIdConnectBackend(ModelBackend):
         return user
 
     def update_existing_user(self, user, kwargs):
-        """Update other fields without overwriting first_name and last_name.
-        Overwrite first_name and last_name if not empty string"""
+        """
+        Update user fields without overwriting certain fields.
 
+        Args:
+            user: User object to be updated.
+            kwargs: Dictionary containing fields to update and their new values.
+
+        Note:
+            This method updates user fields while preserving the values of 'first_name',
+            'last_name', and 'phone' fields, unless specific conditions are met.
+
+            - 'first_name', 'last_name' or 'phone' will be updated if the provided value is not empty.
+        """
+
+        fields_to_check = ["first_name", "last_name", "phone"]
+
+        # Iterate over fields to update
         for key, value in kwargs.items():
-            # Check if the key is not first_name or last_name or value is not empty string
-            if key not in ["first_name", "last_name"] or value:
+            # Check if the field is not 'first_name', 'last_name', or 'phone',
+            # or if it's 'first_name' or 'last_name' or 'phone' and the provided value is not empty
+            if key not in fields_to_check or (key in fields_to_check and value):
+                # Update the corresponding attribute of the user object
                 setattr(user, key, value)
+
+        # Save the user object with the updated fields
         user.save()
 
     def clean_username(self, username):

src/djangooidc/tests/test_backends.py

@@ -50,15 +50,21 @@ class OpenIdConnectBackendTestCase(TestCase):
         self.assertEqual(user.email, "john.doe@example.com")
         self.assertEqual(user.phone, "123456789")
 
-    def test_authenticate_with_existing_user_no_name(self):
+    def test_authenticate_with_existing_user_with_existing_first_last_phone(self):
         """Test that authenticate updates an existing user if it finds one.
-        For this test, given_name and family_name are not supplied"""
+        For this test, given_name and family_name are not supplied.
+
+        The existing user's first and last name are not overwritten.
+        The existing user's phone number is not overwritten"""
         # Create an existing user with the same username and with first and last names
-        existing_user = User.objects.create_user(username="test_user", first_name="John", last_name="Doe")
+        existing_user = User.objects.create_user(
+            username="test_user", first_name="WillNotBe", last_name="Replaced", phone="9999999999"
+        )
 
         # Remove given_name and family_name from the input, self.kwargs
         self.kwargs.pop("given_name", None)
         self.kwargs.pop("family_name", None)
+        self.kwargs.pop("phone", None)
 
         # Ensure that the authenticate method updates the existing user
         # and preserves existing first and last names
@@ -68,16 +74,18 @@ class OpenIdConnectBackendTestCase(TestCase):
         self.assertEqual(user, existing_user)  # The same user instance should be returned
 
         # Verify that user fields are correctly updated
-        self.assertEqual(user.first_name, "John")
-        self.assertEqual(user.last_name, "Doe")
+        self.assertEqual(user.first_name, "WillNotBe")
+        self.assertEqual(user.last_name, "Replaced")
         self.assertEqual(user.email, "john.doe@example.com")
-        self.assertEqual(user.phone, "123456789")
+        self.assertEqual(user.phone, "9999999999")
 
-    def test_authenticate_with_existing_user_different_name(self):
+    def test_authenticate_with_existing_user_different_name_phone(self):
         """Test that authenticate updates an existing user if it finds one.
         For this test, given_name and family_name are supplied and overwrite"""
         # Create an existing user with the same username and with first and last names
-        existing_user = User.objects.create_user(username="test_user", first_name="WillBe", last_name="Replaced")
+        existing_user = User.objects.create_user(
+            username="test_user", first_name="WillBe", last_name="Replaced", phone="987654321"
+        )
 
         # Ensure that the authenticate method updates the existing user
         # and preserves existing first and last names

src/registrar/models/contact.py

@@ -101,11 +101,23 @@ class Contact(TimeStampedModel):
         # Call the parent class's save method to perform the actual save
         super().save(*args, **kwargs)
 
-        # Update the related User object's first_name and last_name
-        if self.user and (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
-            self.user.save()
+        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 phone if necessary
+            if not self.user.phone:
+                self.user.phone = self.phone
+                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:

src/registrar/tests/common.py

@@ -837,7 +837,7 @@ def completed_domain_request(
     )
     if not investigator:
         investigator, _ = User.objects.get_or_create(
-            username="incrediblyfakeinvestigator",
+            username="incrediblyfakeinvestigator" + str(uuid.uuid4())[:8],
             first_name="Joe",
             last_name="Bob",
             is_staff=True,

src/registrar/tests/test_models.py

@@ -1152,12 +1152,18 @@ class TestContact(TestCase):
     def setUp(self):
         self.email_for_invalid = "intern@igorville.gov"
         self.invalid_user, _ = User.objects.get_or_create(
-            username=self.email_for_invalid, email=self.email_for_invalid, first_name="", last_name=""
+            username=self.email_for_invalid,
+            email=self.email_for_invalid,
+            first_name="",
+            last_name="",
+            phone="",
         )
         self.invalid_contact, _ = Contact.objects.get_or_create(user=self.invalid_user)
 
         self.email = "mayor@igorville.gov"
-        self.user, _ = User.objects.get_or_create(email=self.email, first_name="Jeff", last_name="Lebowski")
+        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_as_ao, _ = Contact.objects.get_or_create(email="newguy@igorville.gov")
@@ -1169,19 +1175,22 @@ class TestContact(TestCase):
         Contact.objects.all().delete()
         User.objects.all().delete()
 
-    def test_saving_contact_updates_user_first_last_names(self):
+    def test_saving_contact_updates_user_first_last_names_and_phone(self):
         """When a contact is updated, we propagate the changes to the linked user if it exists."""
 
         # User and Contact are created and linked as expected.
         # An empty User object should create an empty contact.
         self.assertEqual(self.invalid_contact.first_name, "")
         self.assertEqual(self.invalid_contact.last_name, "")
+        self.assertEqual(self.invalid_contact.phone, "")
         self.assertEqual(self.invalid_user.first_name, "")
         self.assertEqual(self.invalid_user.last_name, "")
+        self.assertEqual(self.invalid_user.phone, "")
 
         # Manually update the contact - mimicking production (pre-existing data)
         self.invalid_contact.first_name = "Joey"
         self.invalid_contact.last_name = "Baloney"
+        self.invalid_contact.phone = "123456789"
         self.invalid_contact.save()
 
         # Refresh the user object to reflect the changes made in the database
@@ -1190,20 +1199,25 @@ class TestContact(TestCase):
         # Updating the contact's first and last names propagate to the user
         self.assertEqual(self.invalid_contact.first_name, "Joey")
         self.assertEqual(self.invalid_contact.last_name, "Baloney")
+        self.assertEqual(self.invalid_contact.phone, "123456789")
         self.assertEqual(self.invalid_user.first_name, "Joey")
         self.assertEqual(self.invalid_user.last_name, "Baloney")
+        self.assertEqual(self.invalid_user.phone, "123456789")
 
-    def test_saving_contact_does_not_update_user_first_last_names(self):
+    def test_saving_contact_does_not_update_user_first_last_names_and_phone(self):
         """When a contact is updated, we avoid propagating the changes to the linked user if it already has a value"""
 
         # User and Contact are created and linked as expected
         self.assertEqual(self.contact.first_name, "Jeff")
         self.assertEqual(self.contact.last_name, "Lebowski")
+        self.assertEqual(self.contact.phone, "123456789")
         self.assertEqual(self.user.first_name, "Jeff")
         self.assertEqual(self.user.last_name, "Lebowski")
+        self.assertEqual(self.user.phone, "123456789")
 
         self.contact.first_name = "Joey"
         self.contact.last_name = "Baloney"
+        self.contact.phone = "987654321"
         self.contact.save()
 
         # Refresh the user object to reflect the changes made in the database
@@ -1212,8 +1226,10 @@ class TestContact(TestCase):
         # Updating the contact's first and last names propagate to the user
         self.assertEqual(self.contact.first_name, "Joey")
         self.assertEqual(self.contact.last_name, "Baloney")
+        self.assertEqual(self.contact.phone, "987654321")
         self.assertEqual(self.user.first_name, "Jeff")
         self.assertEqual(self.user.last_name, "Lebowski")
+        self.assertEqual(self.user.phone, "123456789")
 
     def test_saving_contact_does_not_update_user_email(self):
         """When a contact's email is updated, the change is not propagated to the user."""