diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index b0e6417db..41e442f2d 100644 --- a/src/djangooidc/backends.py +++ b/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): diff --git a/src/djangooidc/tests/test_backends.py b/src/djangooidc/tests/test_backends.py index ac7f74903..c15106fa9 100644 --- a/src/djangooidc/tests/test_backends.py +++ b/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 diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 9deb22641..3ebd8bc3e 100644 --- a/src/registrar/models/contact.py +++ b/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: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index c2f11b85d..f1c77fb8e 100644 --- a/src/registrar/tests/common.py +++ b/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, diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ca77d1ddf..1558ab310 100644 --- a/src/registrar/tests/test_models.py +++ b/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."""