From 39a45be387ca70676081987041f3de8fe18bad6a Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 15 Mar 2024 09:41:28 +0100 Subject: [PATCH] :ok_hand: [#2041] PR feedback --- src/open_inwoner/accounts/models.py | 8 +- .../accounts/tests/test_profile_views.py | 117 +++++++++++------- .../accounts/tests/test_user_constraints.py | 12 +- .../cms/cases/tests/test_contactform.py | 14 +-- src/open_inwoner/openklant/tests/data.py | 9 +- .../openklant/tests/test_contactform.py | 8 +- .../openklant/tests/test_views.py | 8 +- 7 files changed, 94 insertions(+), 82 deletions(-) diff --git a/src/open_inwoner/accounts/models.py b/src/open_inwoner/accounts/models.py index bb8d62528a..cdd1ca920b 100644 --- a/src/open_inwoner/accounts/models.py +++ b/src/open_inwoner/accounts/models.py @@ -278,11 +278,9 @@ class Meta: | Q(login_type=LoginTypeChoices.eherkenning), name="check_kvk_or_rsin_only_set_when_login_eherkenning", ), - # CheckConstraint( - # check=((Q(kvk="") & ~Q(rsin="")) | (~Q(kvk="") & Q(rsin=""))) - # | ~Q(login_type=LoginTypeChoices.eherkenning), - # name="check_kvk_or_rsin_exclusive_when_login_eherkenning", - # ), + # NOTE: we do not need a constraint that enforces exclusivity between + # `KVK` and `RSIN`, because companies have both of these attributes and we + # need both of them to fetch cases for these companies ] def __init__(self, *args, **kwargs): diff --git a/src/open_inwoner/accounts/tests/test_profile_views.py b/src/open_inwoner/accounts/tests/test_profile_views.py index ecb218bebc..bfa21197e9 100644 --- a/src/open_inwoner/accounts/tests/test_profile_views.py +++ b/src/open_inwoner/accounts/tests/test_profile_views.py @@ -516,7 +516,6 @@ def test_modify_phone_and_email_updates_klant_api(self, m): response = self.app.get(self.url, user=data.user) # reset noise from signals - m.reset_mock() self.clearTimelineLogs() form = response.forms["profile-edit"] @@ -541,52 +540,76 @@ def test_modify_phone_and_email_updates_klant_api(self, m): ) @requests_mock.Mocker() - def test_eherkenning_user_updates_klant_api(self, m): + def test_eherkenning_user_updates_klant_api_with_rsin(self, m): MockAPIReadPatchData.setUpServices() + data = MockAPIReadPatchData() + + matchers = data.install_mocks_eherkenning(m, use_rsin=True).matchers + + config = OpenKlantConfig.get_solo() + config.use_rsin_for_innNnpId_query_parameter = True + config.save() + + response = self.app.get(self.url, user=data.eherkenning_user) + + # reset noise from signals + self.clearTimelineLogs() - for use_rsin_for_innNnpId_query_parameter in [True, False]: - with self.subTest( - use_rsin_for_innNnpId_query_parameter=use_rsin_for_innNnpId_query_parameter - ): - # NOTE Explicitly creating a new Mocker object here, because for some reason - # `m` is overridden somewhere, which causes issues when `MockAPIReadPatchData.install_mocks` - # is run for the second time - with requests_mock.Mocker() as m: - data = MockAPIReadPatchData().install_mocks_eherkenning( - m, use_rsin=use_rsin_for_innNnpId_query_parameter - ) - - config = OpenKlantConfig.get_solo() - config.use_rsin_for_innNnpId_query_parameter = ( - use_rsin_for_innNnpId_query_parameter - ) - config.save() - - response = self.app.get(self.url, user=data.eherkenning_user) - - # reset noise from signals - m.reset_mock() - self.clearTimelineLogs() - - form = response.forms["profile-edit"] - form["email"] = "new@example.com" - form["phonenumber"] = "0612345678" - form.submit() - - # user data tested in other cases - self.assertTrue(data.matchers[0].called) - klant_patch_data = data.matchers[1].request_history[0].json() - self.assertEqual( - klant_patch_data, - { - "emailadres": "new@example.com", - "telefoonnummer": "0612345678", - }, - ) - self.assertTimelineLog("retrieved klant for user") - self.assertTimelineLog( - "patched klant from user profile edit with fields: emailadres, telefoonnummer" - ) + form = response.forms["profile-edit"] + form["email"] = "new@example.com" + form["phonenumber"] = "0612345678" + form.submit() + + # user data tested in other cases + self.assertTrue(matchers[0].called) + klant_patch_data = matchers[1].request_history[0].json() + self.assertEqual( + klant_patch_data, + { + "emailadres": "new@example.com", + "telefoonnummer": "0612345678", + }, + ) + self.assertTimelineLog("retrieved klant for user") + self.assertTimelineLog( + "patched klant from user profile edit with fields: emailadres, telefoonnummer" + ) + + @requests_mock.Mocker() + def test_eherkenning_user_updates_klant_api_with_kvk(self, m): + MockAPIReadPatchData.setUpServices() + data = MockAPIReadPatchData() + + matchers = data.install_mocks_eherkenning(m, use_rsin=False).matchers + + config = OpenKlantConfig.get_solo() + config.use_rsin_for_innNnpId_query_parameter = False + config.save() + + response = self.app.get(self.url, user=data.eherkenning_user) + + # reset noise from signals + self.clearTimelineLogs() + + form = response.forms["profile-edit"] + form["email"] = "new@example.com" + form["phonenumber"] = "0612345678" + form.submit() + + # user data tested in other cases + self.assertTrue(matchers[0].called) + klant_patch_data = matchers[1].request_history[0].json() + self.assertEqual( + klant_patch_data, + { + "emailadres": "new@example.com", + "telefoonnummer": "0612345678", + }, + ) + self.assertTimelineLog("retrieved klant for user") + self.assertTimelineLog( + "patched klant from user profile edit with fields: emailadres, telefoonnummer" + ) @requests_mock.Mocker() def test_modify_phone_updates_klant_api_but_skips_unchanged(self, m): @@ -615,7 +638,7 @@ def test_modify_phone_updates_klant_api_but_skip_unchanged_email(self, m): response = self.app.get(self.url, user=data.user) # reset noise from signals - m.reset_mock() + # m.reset_mock() self.clearTimelineLogs() form = response.forms["profile-edit"] @@ -645,7 +668,7 @@ def test_modify_phone_updates_klant_api_but_skip_unchanged_phone(self, m): response = self.app.get(self.url, user=data.user) # reset noise from signals - m.reset_mock() + # m.reset_mock() self.clearTimelineLogs() form = response.forms["profile-edit"] diff --git a/src/open_inwoner/accounts/tests/test_user_constraints.py b/src/open_inwoner/accounts/tests/test_user_constraints.py index cb88f77092..c4b849c90d 100644 --- a/src/open_inwoner/accounts/tests/test_user_constraints.py +++ b/src/open_inwoner/accounts/tests/test_user_constraints.py @@ -1,5 +1,3 @@ -from unittest import skip - from django.db import IntegrityError, transaction from django.test import TestCase @@ -50,12 +48,10 @@ def test_unique_rsin_when_login_herkenning(self): with self.assertRaises(IntegrityError): UserFactory(rsin="12345678", login_type=LoginTypeChoices.eherkenning) - @skip("Not yet implemented") - def test_not_both_kvk_and_rsin_when_login_eherkenning(self): - with self.assertRaises(IntegrityError): - UserFactory( - kvk="12345678", rsin="12345678", login_type=LoginTypeChoices.eherkenning - ) + def test_both_kvk_and_rsin_when_login_eherkenning(self): + UserFactory( + kvk="12345678", rsin="12345678", login_type=LoginTypeChoices.eherkenning + ) def test_not_bsn_when_login_not_digid(self): for login_type in [ diff --git a/src/open_inwoner/cms/cases/tests/test_contactform.py b/src/open_inwoner/cms/cases/tests/test_contactform.py index 368918f485..739b3bb6da 100644 --- a/src/open_inwoner/cms/cases/tests/test_contactform.py +++ b/src/open_inwoner/cms/cases/tests/test_contactform.py @@ -15,7 +15,6 @@ ) from zgw_consumers.constants import APITypes -from open_inwoner.accounts.models import User from open_inwoner.accounts.tests.factories import ( DigidUserFactory, eHerkenningUserFactory, @@ -458,18 +457,15 @@ def test_form_success_with_api(self, m, contactmoment_mock): def test_form_success_with_api_eherkenning_user(self, m, contactmoment_mock): self._setUpMocks(m) self._setUpExtraMocks(m) - + eherkenning_user = eHerkenningUserFactory.create( + kvk="12345678", + rsin="000000000", + email=self.klant["emailadres"], + ) for use_rsin_for_innNnpId_query_parameter in [True, False]: with self.subTest( use_rsin_for_innNnpId_query_parameter=use_rsin_for_innNnpId_query_parameter ): - # FIXME for some reason creating the user outside of the loop - # makes the second iteration fail? - User.objects.filter(kvk="12345678").delete() - eherkenning_user = eHerkenningUserFactory( - kvk="12345678", rsin="000000000" - ) - config = OpenKlantConfig.get_solo() config.use_rsin_for_innNnpId_query_parameter = ( use_rsin_for_innNnpId_query_parameter diff --git a/src/open_inwoner/openklant/tests/data.py b/src/open_inwoner/openklant/tests/data.py index cab0268b04..28c02dc77d 100644 --- a/src/open_inwoner/openklant/tests/data.py +++ b/src/open_inwoner/openklant/tests/data.py @@ -1,7 +1,6 @@ from requests.exceptions import RequestException from zgw_consumers.constants import APITypes -from open_inwoner.accounts.models import User from open_inwoner.accounts.tests.factories import ( DigidUserFactory, eHerkenningUserFactory, @@ -39,13 +38,11 @@ def setUpServices(cls): class MockAPIReadPatchData(MockAPIData): def __init__(self): - User.objects.filter(email="old@example.com").delete() self.user = DigidUserFactory( email="old@example.com", phonenumber="0100000000", ) - User.objects.filter(kvk="12345678").delete() self.eherkenning_user = eHerkenningUserFactory( email="old2@example.com", kvk="12345678", @@ -107,14 +104,14 @@ def install_mocks_eherkenning(self, m, use_rsin=True) -> "MockAPIReadPatchData": class MockAPIReadData(MockAPIData): def __init__(self): - User.objects.filter(bsn="100000001").delete() self.user = DigidUserFactory( bsn="100000001", ) - User.objects.filter(kvk="12345678").delete() + self.eherkenning_user = eHerkenningUserFactory( kvk="12345678", rsin="000000000", + email="foo@bar.com", ) self.klant = generate_oas_component_cached( @@ -311,11 +308,9 @@ def install_mocks(self, m, link_objectcontactmomenten=False) -> "MockAPIReadData class MockAPICreateData(MockAPIData): def __init__(self): - User.objects.filter(bsn="100000001").delete() self.user = DigidUserFactory( bsn="100000001", ) - User.objects.filter(kvk="12345678").delete() self.eherkenning_user = eHerkenningUserFactory( kvk="12345678", rsin="000000000", diff --git a/src/open_inwoner/openklant/tests/test_contactform.py b/src/open_inwoner/openklant/tests/test_contactform.py index 432aa274a8..967a7a08db 100644 --- a/src/open_inwoner/openklant/tests/test_contactform.py +++ b/src/open_inwoner/openklant/tests/test_contactform.py @@ -435,6 +435,8 @@ def test_submit_and_register_bsn_user_via_api(self, m): def test_submit_and_register_kvk_or_rsin_user_via_api(self, _m): MockAPICreateData.setUpServices() + data = MockAPICreateData() + original_phonenumber = data.eherkenning_user.phonenumber config = OpenKlantConfig.get_solo() config.register_contact_moment = True @@ -456,7 +458,9 @@ def test_submit_and_register_kvk_or_rsin_user_via_api(self, _m): ) config.save() - data = MockAPICreateData() + data.eherkenning_user.phonenumber = original_phonenumber + data.eherkenning_user.save() + data.install_mocks_eherkenning( m, use_rsin=use_rsin_for_innNnpId_query_parameter ) @@ -604,6 +608,7 @@ def test_submit_and_register_bsn_user_via_api_and_update_klant(self, m): def test_submit_and_register_kvk_or_rsin_user_via_api_and_update_klant(self, _m): MockAPICreateData.setUpServices() + data = MockAPICreateData() config = OpenKlantConfig.get_solo() config.register_contact_moment = True @@ -625,7 +630,6 @@ def test_submit_and_register_kvk_or_rsin_user_via_api_and_update_klant(self, _m) ) config.save() - data = MockAPICreateData() data.install_mocks_eherkenning_missing_contact_info( m, use_rsin=use_rsin_for_innNnpId_query_parameter ) diff --git a/src/open_inwoner/openklant/tests/test_views.py b/src/open_inwoner/openklant/tests/test_views.py index 52eba20223..3c44e084a4 100644 --- a/src/open_inwoner/openklant/tests/test_views.py +++ b/src/open_inwoner/openklant/tests/test_views.py @@ -149,6 +149,8 @@ def test_list_for_bsn_new_answer_available(self, m, mock_get_kcm_answer_mapping) self.assertIn(_("Nieuw antwoord beschikbaar"), response.text) def test_list_for_kvk_or_rsin(self, m, mock_get_kcm_answer_mapping): + data = MockAPIReadData().install_mocks(m) + for use_rsin_for_innNnpId_query_parameter in [True, False]: with self.subTest( use_rsin_for_innNnpId_query_parameter=use_rsin_for_innNnpId_query_parameter @@ -159,8 +161,6 @@ def test_list_for_kvk_or_rsin(self, m, mock_get_kcm_answer_mapping): ) config.save() - data = MockAPIReadData().install_mocks(m) - detail_url = reverse( "cases:contactmoment_detail", kwargs={"kcm_uuid": data.klant_contactmoment2["uuid"]}, @@ -364,6 +364,8 @@ def test_show_detail_for_bsn_with_zaak_reformat_esuite_id( ) def test_show_detail_for_kvk_or_rsin(self, m, mock_get_kcm_answer_mapping): + data = MockAPIReadData().install_mocks(m) + for use_rsin_for_innNnpId_query_parameter in [True, False]: with self.subTest( use_rsin_for_innNnpId_query_parameter=use_rsin_for_innNnpId_query_parameter @@ -377,8 +379,6 @@ def test_show_detail_for_kvk_or_rsin(self, m, mock_get_kcm_answer_mapping): ) config.save() - data = MockAPIReadData().install_mocks(m) - detail_url = reverse( "cases:contactmoment_detail", kwargs={"kcm_uuid": data.klant_contactmoment2["uuid"]},