👌 [#2041] PR feedback

src/open_inwoner/accounts/models.py

@@ -287,11 +287,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):

src/open_inwoner/accounts/tests/test_auth.py

@@ -1134,7 +1134,7 @@ def test_eherkenning_user_success(self, mock_kvk, mock_retrieve_rsin_with_kvk):
         test_user = eHerkenningUserFactory.create(
             email="test@localhost",
             kvk="64819772",
-            rsin="123456789",
+            rsin="123456782",
         )
 
         url = reverse("eherkenning-mock:password")

src/open_inwoner/accounts/tests/test_oidc_views.py

@@ -1055,7 +1055,7 @@ def test_new_user_is_created_when_new_kvk(
         # set up a user with a non existing email address
         mock_get_userinfo.return_value = {"sub": "00000000"}
         eHerkenningUserFactory.create(
-            kvk="12345678", rsin="123456789", email="existing_user@example.com"
+            kvk="12345678", rsin="123456782", email="existing_user@example.com"
         )
         session = self.client.session
         session["oidc_states"] = {"mock": {"nonce": "nonce"}}

src/open_inwoner/accounts/tests/test_profile_views.py

@@ -512,7 +512,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"]
@@ -537,52 +536,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):
@@ -611,7 +634,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"]
@@ -641,7 +664,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"]

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 [

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,
@@ -455,18 +454,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

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",
@@ -134,14 +131,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_bsn = generate_oas_component_cached(
@@ -353,11 +350,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",

src/open_inwoner/openklant/tests/test_contactform.py

@@ -434,6 +434,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
@@ -455,7 +457,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
                     )
@@ -603,6 +607,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
@@ -624,7 +629,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
                     )

src/open_inwoner/openklant/tests/test_views.py

@@ -151,6 +151,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
@@ -161,8 +163,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={
@@ -448,6 +448,8 @@ def test_display_contactmoment_subject_no_mapping_fallback(
         )
 
     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
@@ -461,8 +463,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={