[#2096] Change 'handelsnaam' to 'naam' and refactor api root validation

maykinmedia · Feb 19, 2024 · 9dcc55a · 9dcc55a
1 parent b9d96aa
commit 9dcc55a
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 78 deletions.
diff --git a/requirements/base.txt b/requirements/base.txt
@@ -62,7 +62,9 @@ cryptography==41.0.7
     #   mozilla-django-oidc
     #   pyopenssl
 css-inline==0.13.0
-    # via -r requirements/base.in
+    # via
+    #   -r requirements/base.in
+    #   mail-editor
 cssselect2==0.4.1
     # via
     #   svglib
@@ -341,6 +343,7 @@ lockfile==0.12.2
 lxml==4.9.1
     # via
     #   django-digid-eherkenning
+    #   mail-editor
     #   maykin-python3-saml
     #   svglib
     #   xmlsec
@@ -453,6 +456,7 @@ requests==2.31.0
     #   django-open-forms-client
     #   django-rosetta
     #   gemma-zds-client
+    #   mail-editor
     #   maykin-python3-saml
     #   messagebird
     #   mozilla-django-oidc

diff --git a/requirements/ci.txt b/requirements/ci.txt
@@ -122,6 +122,7 @@ css-inline==0.13.0
     # via
     #   -c requirements/base.txt
     #   -r requirements/base.txt
+    #   mail-editor
 cssselect==1.1.0
     # via pyquery
 cssselect2==0.4.1
@@ -605,6 +606,7 @@ lxml==4.9.1
     #   -c requirements/base.txt
     #   -r requirements/base.txt
     #   django-digid-eherkenning
+    #   mail-editor
     #   maykin-python3-saml
     #   pyquery
     #   svglib
@@ -834,6 +836,7 @@ requests==2.31.0
     #   django-open-forms-client
     #   django-rosetta
     #   gemma-zds-client
+    #   mail-editor
     #   maykin-python3-saml
     #   messagebird
     #   mozilla-django-oidc

diff --git a/requirements/dev.txt b/requirements/dev.txt
@@ -147,6 +147,7 @@ css-inline==0.13.0
     # via
     #   -c requirements/ci.txt
     #   -r requirements/ci.txt
+    #   mail-editor
 cssselect==1.1.0
     # via
     #   -c requirements/ci.txt
@@ -699,6 +700,7 @@ lxml==4.9.1
     #   -c requirements/ci.txt
     #   -r requirements/ci.txt
     #   django-digid-eherkenning
+    #   mail-editor
     #   maykin-python3-saml
     #   pyquery
     #   svglib
@@ -974,6 +976,7 @@ requests==2.31.0
     #   django-rosetta
     #   gemma-zds-client
     #   locust
+    #   mail-editor
     #   maykin-python3-saml
     #   messagebird
     #   mozilla-django-oidc

diff --git a/src/open_inwoner/accounts/tests/test_auth.py b/src/open_inwoner/accounts/tests/test_auth.py
@@ -1079,12 +1079,12 @@ def test_eherkenning_user_success(self, mock_kvk):
             {
                 "kvkNummer": "12345678",
                 "vestigingsnummer": "1234",
-                "handelsnaam": "Mijn bedrijf",
+                "naam": "Mijn bedrijf",
             },
             {
                 "kvkNummer": "12345678",
                 "vestigingsnummer": "5678",
-                "handelsnaam": "Mijn bedrijf",
+                "naam": "Mijn bedrijf",
             },
         ]
 

diff --git a/...n_inwoner/kvk/migrations/0004_api_root.py → ...n_inwoner/kvk/migrations/0003_api_root.py b/...n_inwoner/kvk/migrations/0004_api_root.py → ...n_inwoner/kvk/migrations/0003_api_root.py
@@ -17,7 +17,7 @@ def strip_version_number(apps, _):
 class Migration(migrations.Migration):
 
     dependencies = [
-        ("kvk", "0003_auto_20240209_1259"),
+        ("kvk", "0002_alter_kvkconfig_api_root"),
     ]
 
     operations = [

diff --git a/src/open_inwoner/kvk/migrations/0003_auto_20240209_1259.py b/src/open_inwoner/kvk/migrations/0003_auto_20240209_1259.py
diff --git a/src/open_inwoner/kvk/migrations/0004_alter_kvkconfig_api_root.py b/src/open_inwoner/kvk/migrations/0004_alter_kvkconfig_api_root.py
@@ -0,0 +1,24 @@
+# Generated by Django 3.2.23 on 2024-02-19 08:26
+
+from django.db import migrations, models
+import open_inwoner.kvk.validators
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("kvk", "0003_api_root"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="kvkconfig",
+            name="api_root",
+            field=models.URLField(
+                help_text="The root of the API without version number and endpoint (e.g. https://api.kvk.nl/api/ or https://api.kvk.nl/test/api).",
+                max_length=128,
+                validators=[open_inwoner.kvk.validators.validate_api_root],
+                verbose_name="API root",
+            ),
+        ),
+    ]
diff --git a/src/open_inwoner/kvk/models.py b/src/open_inwoner/kvk/models.py
@@ -10,7 +10,7 @@
 
 
 class KvKConfig(SingletonModel):
-    api_root = models.CharField(
+    api_root = models.URLField(
         verbose_name=_("API root"),
         max_length=128,
         validators=[validate_api_root],
@@ -29,7 +29,7 @@ class KvKConfig(SingletonModel):
         Certificate,
         blank=True,
         null=True,
-        help_text=_("The SSL certificate file used for client identification."),
+        help_text=_("The SSL certificate file used for client identification. "),
         on_delete=models.PROTECT,
         related_name="kvk_service_client",
     )

diff --git a/src/open_inwoner/kvk/tests/test_migrations.py b/src/open_inwoner/kvk/tests/test_migrations.py
@@ -2,8 +2,8 @@
 
 
 class APIRootMigrationTest(TestMigrations):
-    migrate_from = "0003_auto_20240209_1259"
-    migrate_to = "0004_api_root"
+    migrate_from = "0002_alter_kvkconfig_api_root"
+    migrate_to = "0003_api_root"
     app = "kvk"
 
     def setUpBeforeMigration(self, apps):

diff --git a/src/open_inwoner/kvk/tests/test_validators.py b/src/open_inwoner/kvk/tests/test_validators.py
@@ -9,11 +9,11 @@ class KvKAPIRootValidationTest(TestCase):
     @classmethod
     def setUpTestData(cls):
         cls.error_msg = _(
-            "The API root is incorrect. Please double-check that the host is correct "
-            "and you didn't include the version number or endpoint."
+            "The API root is incorrect. Please double-check that "
+            "you didn't include the version number or endpoint."
         )
 
-    def test_invalid_because_version_number_or_endpoint_included(self):
+    def test_kvk_api_root_invalid(self):
         test_cases = (
             "https://api.kvk.nl/api/v1",
             "https://api.kvk.nl/api/v1/test",
@@ -24,18 +24,7 @@ def test_invalid_because_version_number_or_endpoint_included(self):
                 with self.assertRaisesMessage(ValidationError, self.error_msg):
                     validate_api_root(test_string)
 
-    def test_invalid_kvk_host(self):
-        test_cases = (
-            "https://kvk.api.nl/api",
-            "https://apii.kvk.nl/test/api",
-            "http://api.kvk.nl/",
-        )
-        for i, test_string in enumerate(test_cases):
-            with self.subTest(i=i):
-                with self.assertRaisesMessage(ValidationError, self.error_msg):
-                    validate_api_root(test_string)
-
-    def test_valid_kvk_api_root(self):
+    def test_kvk_api_root_valid(self):
         test_cases = (
             "https://api.kvk.nl/api",
             "https://api.kvk.nl/api/",

diff --git a/src/open_inwoner/kvk/tests/test_views.py b/src/open_inwoner/kvk/tests/test_views.py
@@ -190,12 +190,12 @@ def test_get_branches_page_one_branch_found_sets_branch_check_done(
     def test_get_branches_page(self, mock_solo, mock_kvk):
         mock_kvk.return_value = [
             {
-                "handelsnaam": "Makers and Shakers",
+                "naam": "Makers and Shakers",
                 "kvkNummer": "12345678",
                 "vestigingsnummer": "1234",
             },
             {
-                "handelsnaam": "Makers and Shakers",
+                "naam": "Makers and Shakers",
                 "kvkNummer": "12345678",
                 "vestigingsnummer": "5678",
             },
@@ -221,3 +221,7 @@ def test_get_branches_page(self, mock_solo, mock_kvk):
         self.assertEqual(branch_inputs[0], doc.find("[id='branch-']")[0])
         self.assertEqual(branch_inputs[1], doc.find("[id='branch-1234']")[0])
         self.assertEqual(branch_inputs[2], doc.find("[id='branch-5678']")[0])
+
+        # chack that company name is displayed for every branch
+        company_name_displays = doc("p:Contains('Makers and Shakers')")
+        self.assertEqual(len(company_name_displays), 3)
diff --git a/src/open_inwoner/kvk/validators.py b/src/open_inwoner/kvk/validators.py
@@ -3,20 +3,14 @@
 from django.core.exceptions import ValidationError
 from django.utils.translation import gettext_lazy as _
 
-# There are only 4 possibilities, let's enumerate them
-regexes = (
-    re.compile(r"^https://api.kvk.nl/api$"),
-    re.compile(r"^https://api.kvk.nl/api/$"),
-    re.compile(r"^https://api.kvk.nl/test/api$"),
-    re.compile(r"^https://api.kvk.nl/test/api/$"),
-)
+regexp = re.compile("/api/[0-9a-z]")
 
 
 def validate_api_root(value):
-    if not any(regex.match(value) for regex in regexes):
+    if regexp.search(value, re.IGNORECASE):
         raise ValidationError(
             _(
-                "The API root is incorrect. Please double-check that the host is "
-                "correct and you didn't include the version number or endpoint."
+                "The API root is incorrect. Please double-check that "
+                "you didn't include the version number or endpoint."
             )
         )
diff --git a/src/open_inwoner/kvk/views.py b/src/open_inwoner/kvk/views.py
@@ -40,9 +40,7 @@ def get_context_data(self, **kwargs):
         # create pseudo-branch representing the company as a whole
         master_branch = {
             "vestigingsnummer": "",
-            "handelsnaam": company_branches[0].get("handelsnaam", "")
-            if company_branches
-            else "",
+            "naam": company_branches[0].get("naam", "") if company_branches else "",
         }
         company_branches.insert(0, master_branch)
 

diff --git a/src/open_inwoner/templates/pages/kvk/branches.html b/src/open_inwoner/templates/pages/kvk/branches.html
@@ -14,7 +14,7 @@
                 {% render_card direction='horizontal' %}
                     {% with company_id=branch.vestigingsnummer %}
                         <label class="left" for="branch-{{ company_id }}">
-                            <p>{{ branch.handelsnaam }}</p>
+                            <p>{{ branch.naam }}</p>
                             {% if branch.straatnaam and branch.plaats %}
                                 <p>{{ branch.straatnaam }}, {{ branch.plaats}}</p>
                             {% endif %}