diff --git a/profiles/schema.py b/profiles/schema.py index d7c78de4..127accd2 100644 --- a/profiles/schema.py +++ b/profiles/schema.py @@ -1260,7 +1260,7 @@ def mutate_and_get_payload(cls, root, info, **input): if sensitive_data: update_sensitivedata(profile, sensitive_data) - profile_updated.send(sender=profile.__class__, instance=profile) + profile_updated.send(sender=profile.__class__, instance=profile) return UpdateMyProfileMutation(profile=profile) @@ -1336,7 +1336,7 @@ def mutate_and_get_payload(cls, root, info, **input): if sensitive_data: update_sensitivedata(profile, sensitive_data) - profile_updated.send(sender=profile.__class__, instance=profile) + profile_updated.send(sender=profile.__class__, instance=profile) return UpdateProfileMutation(profile=profile) @@ -1369,9 +1369,9 @@ def mutate_and_get_payload(cls, root, info, **input): profile_to_claim.save() profile_to_claim.claim_tokens.all().delete() - profile_updated.send( - sender=profile_to_claim.__class__, instance=profile_to_claim - ) + profile_updated.send( + sender=profile_to_claim.__class__, instance=profile_to_claim + ) return ClaimProfileMutation(profile=profile_to_claim) diff --git a/profiles/tests/test_gql_update_my_profile_mutation.py b/profiles/tests/test_gql_update_my_profile_mutation.py index b919b570..1458449c 100644 --- a/profiles/tests/test_gql_update_my_profile_mutation.py +++ b/profiles/tests/test_gql_update_my_profile_mutation.py @@ -589,11 +589,14 @@ def test_remove_all_emails_if_they_are_not_primary(user_gql_client): assert executed["data"] == expected_data -def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_produced( +def test_when_keycloak_returns_conflict_on_update_changes_are_reverted( user_gql_client, keycloak_setup, mocker ): + """Correct error code is produced and local changes are reverted.""" profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) - email = profile.emails.first() + primary_email = profile.get_primary_email() + original_email = primary_email.email + new_email = f"new-{primary_email.email}" mocker.patch.object( keycloak.KeycloakAdminClient, @@ -601,7 +604,7 @@ def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_pro return_value={ "firstName": profile.first_name, "lastName": profile.last_name, - "email": "old@email.example", + "email": primary_email.email, }, ) @@ -612,13 +615,14 @@ def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_pro ) email_updates = [ - {"id": to_global_id("EmailNode", email.id), "email": "new@email.example"} + {"id": to_global_id("EmailNode", primary_email.id), "email": new_email} ] executed = user_gql_client.execute( EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} ) assert_match_error_code(executed, "DATA_CONFLICT_ERROR") + assert profile.get_primary_email().email == original_email PHONES_MUTATION = """ diff --git a/profiles/tests/test_gql_update_profile_mutation.py b/profiles/tests/test_gql_update_profile_mutation.py index d058573b..c32854a9 100644 --- a/profiles/tests/test_gql_update_profile_mutation.py +++ b/profiles/tests/test_gql_update_profile_mutation.py @@ -272,11 +272,14 @@ def test_changing_an_email_address_marks_it_unverified(user_gql_client, service) assert executed["data"] == expected_data -def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_produced( +def test_when_keycloak_returns_conflict_on_update_changes_are_reverted( user_gql_client, service, keycloak_setup, mocker ): + """Correct error code is produced and local changes are reverted.""" profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) - email = profile.emails.first() + primary_email = profile.get_primary_email() + original_email = primary_email.email + new_email = f"new-{primary_email.email}" setup_profile_and_staff_user_to_service(profile, user_gql_client.user, service) @@ -286,7 +289,7 @@ def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_pro return_value={ "firstName": profile.first_name, "lastName": profile.last_name, - "email": "old@email.example", + "email": primary_email.email, }, ) @@ -301,8 +304,8 @@ def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_pro "id": to_global_id("ProfileNode", profile.id), "updateEmails": [ { - "id": to_global_id("EmailNode", email.id), - "email": "new@email.example", + "id": to_global_id("EmailNode", primary_email.id), + "email": new_email, } ], } @@ -312,6 +315,7 @@ def test_when_keycloak_returns_conflict_on_update_then_correct_error_code_is_pro ) assert_match_error_code(executed, "DATA_CONFLICT_ERROR") + assert profile.get_primary_email().email == original_email class TestProfileInputValidation(ExistingProfileInputValidationBase): diff --git a/profiles/tests/test_profile_changes_to_keycloak.py b/profiles/tests/test_profile_changes_to_keycloak.py index 2b1b1855..0b0acea9 100644 --- a/profiles/tests/test_profile_changes_to_keycloak.py +++ b/profiles/tests/test_profile_changes_to_keycloak.py @@ -34,7 +34,6 @@ def test_do_nothing_if_user_is_not_found_in_keycloak(mocker): mocked_update_user = mocker.patch.object( keycloak.KeycloakAdminClient, "update_user" ) - profile = ProfileFactory() profile_updated.send(sender=profile.__class__, instance=profile) @@ -48,7 +47,6 @@ def test_changed_names_are_sent_to_keycloak(mocker): "lastName": "New last name", "email": None, } - mocker.patch.object( keycloak.KeycloakAdminClient, "get_user", @@ -57,11 +55,11 @@ def test_changed_names_are_sent_to_keycloak(mocker): mocked_update_user = mocker.patch.object( keycloak.KeycloakAdminClient, "update_user" ) - profile = ProfileFactory( first_name=new_values["firstName"], last_name=new_values["lastName"] ) user_id = profile.user.uuid + profile_updated.send(sender=profile.__class__, instance=profile) mocked_update_user.assert_called_once_with(user_id, new_values) @@ -77,7 +75,6 @@ def test_changing_email_causes_it_to_be_marked_unverified( "email": "new@email.example", "emailVerified": False, } - mocker.patch.object( keycloak.KeycloakAdminClient, "get_user", @@ -99,12 +96,12 @@ def test_changing_email_causes_it_to_be_marked_unverified( else Exception("send_verify_email failed") ), ) - profile = ProfileFactory( first_name=new_values["firstName"], last_name=new_values["lastName"] ) profile.emails.create(email=new_values["email"], primary=True) user_id = profile.user.uuid + profile_updated.send(sender=profile.__class__, instance=profile) mocked_update_user.assert_called_once_with(user_id, new_values) @@ -113,15 +110,14 @@ def test_changing_email_causes_it_to_be_marked_unverified( def test_if_there_are_no_changes_then_nothing_is_sent_to_keycloak(mocker): values = {"firstName": "First name", "lastName": "Last name"} - mocker.patch.object(keycloak.KeycloakAdminClient, "get_user", return_value=values) mocked_update_user = mocker.patch.object( keycloak.KeycloakAdminClient, "update_user" ) - profile = ProfileFactory( first_name=values["firstName"], last_name=values["lastName"] ) + profile_updated.send(sender=profile.__class__, instance=profile) mocked_update_user.assert_not_called() @@ -129,7 +125,6 @@ def test_if_there_are_no_changes_then_nothing_is_sent_to_keycloak(mocker): def test_when_update_causes_a_conflict_then_data_conflict_error_is_raised(mocker): profile = ProfileWithPrimaryEmailFactory() - mocker.patch.object( keycloak.KeycloakAdminClient, "get_user", @@ -139,7 +134,6 @@ def test_when_update_causes_a_conflict_then_data_conflict_error_is_raised(mocker "email": "old@email.example", }, ) - mocker.patch.object( keycloak.KeycloakAdminClient, "update_user",