[#2946] Fix login for eHerkenning user with single vestiging

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(

src/open_inwoner/kvk/views.py

@@ -7,13 +7,14 @@
 from furl import furl
 
 from open_inwoner.kvk.branches import KVK_BRANCH_SESSION_VARIABLE
+from open_inwoner.utils.views import LogMixin
 
 from ..utils.url import get_next_url_from
 from .client import KvKClient
 from .forms import CompanyBranchChoiceForm
 
 
-class CompanyBranchChoiceView(FormView):
+class CompanyBranchChoiceView(LogMixin, FormView):
     """Choose the branch ("vestiging") of a company"""
 
     template_name = "pages/kvk/branches.html"
@@ -32,12 +33,14 @@ 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
-        master_branch = {
+        # create pseudo-branch representing the company as a whole (the "rechtspersoon").
+        # technically, the compnay as a legal entity is represented as "rechtspersoon",
+        # but this is not always included in query results
+        rechtspersoon_entry = {
             "vestigingsnummer": "",
             "naam": company_branches[0].get("naam", "") if company_branches else "",
         }
-        company_branches.insert(0, master_branch)
+        company_branches.insert(0, rechtspersoon_entry)
 
         kwargs["company_branches"] = company_branches
 
@@ -63,11 +66,13 @@ 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 "rechtspersoon_entry"
+        vestigingen = form.company_branches[1:]
+        if not vestigingen or not any(v.get("vestigingsnummer") for v in vestigingen):
+            self.log_system_action(
+                f"List of company branches for KVK number {request.user.kvk} contains "
+                "no branch with vestigingsnummer"
+            )
             request.session[KVK_BRANCH_SESSION_VARIABLE] = None
             request.session.save()
             return HttpResponseRedirect(redirect)
@@ -90,6 +95,8 @@ def post(self, request):
             # Directly calling `super().form_invalid(form)` would override the error
             return self.render_to_response(context)
 
+        # empty string for KVK_BRANCH_SESSION_VARIABLE is interpreted as
+        # "interact as the rechtspersoon, not as any specific branch"
         request.session[KVK_BRANCH_SESSION_VARIABLE] = request.POST["branch_number"]
 
         return HttpResponseRedirect(redirect)

src/open_inwoner/templates/pages/kvk/branches.html

@@ -13,7 +13,7 @@ <h1 class="utrecht-heading-1" id="title">{% trans "Log in" %}</h1>
 
                 <ul class="choice-list choice-list--single">
                     {# first only show option to select entire company in separate list-item #}
-                    <p><strong>{% trans "Log in namens alle vestigingen" %}</strong></p>
+                    <p><strong>{% trans "Log in namens rechtspersoon" %}</strong></p>
                     {% with entire_company=company_branches.0 %}
                         <li class="choice-list__item">
                             <label class="choice-list__label">