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)