diff --git a/src/open_inwoner/accounts/signals.py b/src/open_inwoner/accounts/signals.py index 19579fb100..6970aaf471 100644 --- a/src/open_inwoner/accounts/signals.py +++ b/src/open_inwoner/accounts/signals.py @@ -8,6 +8,7 @@ from open_inwoner.accounts.models import User from open_inwoner.haalcentraal.models import HaalCentraalConfig from open_inwoner.haalcentraal.utils import update_brp_data_in_db +from open_inwoner.kvk.client import KvKClient from open_inwoner.openklant.services import OpenKlant2Service, eSuiteKlantenService from open_inwoner.utils.logentry import user_action @@ -26,7 +27,7 @@ @receiver(user_logged_in) -def update_user_from_klant_on_login(sender, user, request, *args, **kwargs): +def update_user_on_login(sender, user, request, *args, **kwargs): # This additional guard is mainly to facilitate easier testing, where not # all request factories return a full-fledged request object. if not hasattr(request, "user"): @@ -35,6 +36,9 @@ def update_user_from_klant_on_login(sender, user, request, *args, **kwargs): if user.login_type not in [LoginTypeChoices.digid, LoginTypeChoices.eherkenning]: return + if user.login_type is LoginTypeChoices.eherkenning: + _update_eherkenning_user_from_kvk_api(user=user) + # OpenKlant2 try: service = OpenKlant2Service() @@ -74,6 +78,16 @@ def _update_user_from_esuite( service.update_user_from_klant(klant, user) +def _update_eherkenning_user_from_kvk_api(user: User): + kvk_client = KvKClient() + + vestiging = kvk_client.get_company_headquarters(kvk=user.kvk) + + if company_name := vestiging.get("naam"): + user.company_name = company_name + user.save() + + @receiver(user_logged_in) def log_user_login(sender, user, request, *args, **kwargs): current_path = request.path diff --git a/src/open_inwoner/accounts/tests/test_auth.py b/src/open_inwoner/accounts/tests/test_auth.py index 5d4926b97c..ff933c7ef7 100644 --- a/src/open_inwoner/accounts/tests/test_auth.py +++ b/src/open_inwoner/accounts/tests/test_auth.py @@ -14,7 +14,7 @@ from pyquery import PyQuery as PQ from open_inwoner.accounts.choices import NotificationChannelChoice -from open_inwoner.accounts.signals import update_user_from_klant_on_login +from open_inwoner.accounts.signals import KvKClient, update_user_on_login from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.haalcentraal.tests.mixins import HaalCentraalMixin from open_inwoner.kvk.branches import get_kvk_branch_number @@ -1896,7 +1896,7 @@ def test_anonymous_user_is_redirected_to_login_page_if_password_change_is_access @requests_mock.Mocker() -class TestUpdateUserFromKlantUponLoginTests(TestCase): +class UpdateUserOnLoginTest(TestCase): @classmethod def setUpTestData(cls): MockAPIReadPatchData.setUpServices() @@ -1906,31 +1906,86 @@ def setUpTestData(cls): def test_update_hook_is_registered_on_login(self, m): connected_functions = [receiver[1]() for receiver in user_logged_in.receivers] - self.assertIn(update_user_from_klant_on_login, connected_functions) + self.assertIn(update_user_on_login, connected_functions) - def test_update_user_from_klant_hook_only_called_for_digid_and_eherkenning(self, m): + def test_update_hook_not_called(self, m): self.data = MockAPIReadPatchData().install_mocks(m) request = RequestFactory().get("/foo") request.user = self.data.user - for login_type in LoginTypeChoices: - with self.subTest( - f"Test update klant hook is called for login type {login_type}" - ): + for login_type in [LoginTypeChoices.default, LoginTypeChoices.oidc]: + with self.subTest(f"{login_type}"): self.data.user.login_type = login_type self.data.user.save() + + update_user_on_login( + self.__class__, + request.user, + request, + ) + with patch( "open_inwoner.openklant.services.eSuiteKlantenService.update_user_from_klant" - ) as update_user_from_klant_mock: - update_user_from_klant_on_login( - self.__class__, - request.user, - request, - ) - if login_type in [ - LoginTypeChoices.digid, - LoginTypeChoices.eherkenning, - ]: - update_user_from_klant_mock.assert_called_once() - else: - update_user_from_klant_mock.assert_not_called() + ) as update_user_mock: + update_user_mock.assert_not_called() + + def test_digid_user_update_hook_called(self, m): + self.data = MockAPIReadPatchData().install_mocks(m) + request = RequestFactory().get("/foo") + request.user = self.data.user + + self.data.user.login_type = LoginTypeChoices.digid + self.data.user.save() + with patch( + "open_inwoner.openklant.services.eSuiteKlantenService.update_user_from_klant" + ) as update_user_mock: + update_user_on_login( + self.__class__, + request.user, + request, + ) + update_user_mock.assert_called_once() + + def test_update_eherkenning_user(self, m): + self.data = MockAPIReadPatchData().install_mocks(m) + request = RequestFactory().get("/foo") + request.user = self.data.user + + self.data.user.login_type = LoginTypeChoices.eherkenning + self.data.user.save() + + self.assertEqual(request.user.company_name, "") + + vestiging = { + "kvkNummer": "68750110", + "vestigingsnummer": "000037178598", + "naam": "Test BV Donald", + "adres": { + "binnenlandsAdres": { + "type": "bezoekadres", + "straatnaam": "Hizzaarderlaan", + "plaats": "Lollum", + } + }, + "type": "hoofdvestiging", + "_links": { + "basisprofiel": { + "href": "https://api.kvk.nl/test/api/v1/basisprofielen/68750110" + }, + "vestigingsprofiel": { + "href": "https://api.kvk.nl/test/api/v1/vestigingsprofielen/000037178598" + }, + }, + } + + with patch.object(KvKClient, "get_company_headquarters") as mock_kvk: + mock_kvk.return_value = vestiging + update_user_on_login( + self.__class__, + request.user, + request, + ) + + mock_kvk.assert_called_once() + + self.assertEqual(request.user.company_name, "Test BV Donald") diff --git a/src/open_inwoner/kvk/tests/test_views.py b/src/open_inwoner/kvk/tests/test_views.py index 832559af2f..da1c9d954c 100644 --- a/src/open_inwoner/kvk/tests/test_views.py +++ b/src/open_inwoner/kvk/tests/test_views.py @@ -152,14 +152,23 @@ def test_get_branches_page_no_branches_found_sets_branch_check_done( @patch( "open_inwoner.kvk.models.KvKConfig.get_solo", ) - def test_get_branches_page_one_branch_found_sets_branch_check_done( - self, mock_solo, mock_kvk - ): + def test_get_branches_page_one_branch_found(self, mock_solo, mock_kvk): """ - Regression test for endless redirect: https://taiga.maykinmedia.nl/project/open-inwoner/task/2000 + The branch selection page should be displayed, and the vestigingsnummer stored in the + session, even if only one branch is found. + + Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/task/2946 + + We previously skipped the branch selection for single-branch companies because of problems + with our redirect middleware: https://taiga.maykinmedia.nl/project/open-inwoner/task/2000 """ mock_kvk.return_value = [ - {"kvkNummer": "12345678", "vestigingsnummer": "1234"}, + { + "kvkNummer": "12345678", + "vestigingsnummer": "1234", + "naam": "Makers and Shakers", + "type": "hoofdvestiging", + }, ] mock_solo.return_value.api_key = "123" @@ -171,17 +180,24 @@ def test_get_branches_page_one_branch_found_sets_branch_check_done( response = self.client.get(self.url) - self.assertEqual(response.status_code, 302) - self.assertEqual(response.url, reverse("pages-root")) - # Because no branches were found, the branch check should be skipped in the future - # and no branch number should be set - self.assertEqual(kvk_branch_selected_done(self.client.session), True) - self.assertEqual(get_kvk_branch_number(self.client.session), None) + self.assertEqual(response.status_code, 200) - response = self.client.get(response.url) + doc = PyQuery(response.content) - # Following redirect should not result in endless redirect - self.assertEqual(response.status_code, 200) + branch_inputs = doc.find("[name='branch_number']") + + # check that pseudo-branch representing company as a whole has been added + self.assertEqual(len(branch_inputs), 2) + + self.assertEqual(branch_inputs[0], doc.find("[id='entire-company']")[0]) + self.assertEqual(branch_inputs[1], doc.find("[id='branch-1234']")[0]) + + # chack that company name is displayed for every branch + company_name_displays = doc("h2:Contains('Makers and Shakers')") + self.assertEqual(len(company_name_displays), 2) + + main_branch_display = doc("p:Contains('Hoofdvestiging')") + self.assertEqual(len(main_branch_display), 1) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") @patch( diff --git a/src/open_inwoner/kvk/views.py b/src/open_inwoner/kvk/views.py index 846a3f7589..53f09fd9cc 100644 --- a/src/open_inwoner/kvk/views.py +++ b/src/open_inwoner/kvk/views.py @@ -32,7 +32,9 @@ def get_form_kwargs(self): company_branches = kvk_client.get_all_company_branches( kvk=self.request.user.kvk ) - # create pseudo-branch representing the company as a whole + # create pseudo-branch representing the company as a whole. + # technically, the compnay as a legal entity is represented as "rechtspersoon", + # but this is not always included in query results master_branch = { "vestigingsnummer": "", "naam": company_branches[0].get("naam", "") if company_branches else "", @@ -63,11 +65,9 @@ def get(self, request, *args, **kwargs): form = context["form"] - company_branches_with_master = form.company_branches - # Exclude the "master" branch from these checks, since we always insert this - company_branches = company_branches_with_master[1:] - - if not company_branches or len(company_branches) == 1: + # check that there are company branches besides our artifical "master_branch" + vestigingen = form.company_branches[1:] + if not vestigingen or not any(v.get("vestigingsnummer") for v in vestigingen): request.session[KVK_BRANCH_SESSION_VARIABLE] = None request.session.save() return HttpResponseRedirect(redirect) diff --git a/src/open_inwoner/openklant/tests/test_views.py b/src/open_inwoner/openklant/tests/test_views.py index fac5534f64..26c34e3e04 100644 --- a/src/open_inwoner/openklant/tests/test_views.py +++ b/src/open_inwoner/openklant/tests/test_views.py @@ -14,7 +14,7 @@ from pyquery import PyQuery from zgw_consumers.api_models.base import factory -from open_inwoner.accounts.signals import update_user_from_klant_on_login +from open_inwoner.accounts.signals import update_user_on_login from open_inwoner.accounts.tests.factories import DigidUserFactory, UserFactory from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.openklant.api_models import ContactMoment, Klant, KlantContactMoment @@ -60,7 +60,7 @@ def setUp(self): ) super().setUp() - signals.user_logged_in.disconnect(receiver=update_user_from_klant_on_login) + signals.user_logged_in.disconnect(receiver=update_user_on_login) MockAPIReadData.setUpServices() self.api_group = ZGWApiGroupConfig.objects.get()