feat: Gather UK counties (#2158)

* feat: Gather UK counties

* Refactor tests

* Add county info to admin pages

* Feedback

cfl_common/common/migrations/0040_school_county.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.20 on 2023-08-04 14:35
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('common', '0039_copy_email_to_username'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='school',
+            name='county',
+            field=models.CharField(blank=True, max_length=50, null=True),
+        ),
+    ]

cfl_common/common/migrations/0041_populate_gb_counties.py

@@ -0,0 +1,23 @@
+import pgeocode
+from django.db import migrations
+
+from portal.helpers.organisation import sanitise_uk_postcode
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("common", "0040_school_county"),
+    ]
+
+    def forwards(apps, schema_editor):
+        """Populate the county field for schools in GB"""
+        School = apps.get_model("common", "School")
+        gb_schools = School.objects.filter(country="GB")
+        nomi = pgeocode.Nominatim("GB")
+
+        for school in gb_schools:
+            county = nomi.query_postal_code(sanitise_uk_postcode(school.postcode)).county_name
+            school.county = county
+            school.save()
+
+    operations = [migrations.RunPython(forwards, reverse_code=migrations.RunPython.noop)]

cfl_common/common/models.py

@@ -1,3 +1,4 @@
+import pgeocode
 import re
 from datetime import timedelta
 from uuid import uuid4
@@ -7,6 +8,8 @@
 from django.utils import timezone
 from django_countries.fields import CountryField
 
+from portal.helpers.organisation import sanitise_uk_postcode
+
 
 class UserProfile(models.Model):
     user = models.OneToOneField(User, on_delete=models.CASCADE)
@@ -38,6 +41,8 @@ class School(models.Model):
     name = models.CharField(max_length=200)
     postcode = models.CharField(max_length=10, null=True)
     country = CountryField(blank_label="(select country)")
+    # TODO: Create an Address model to house address details
+    county = models.CharField(max_length=50, blank=True, null=True)
     creation_time = models.DateTimeField(default=timezone.now, null=True)
     is_active = models.BooleanField(default=True)
 
@@ -66,6 +71,16 @@ def anonymise(self):
         self.is_active = False
         self.save()
 
+    def save(self, **kwargs):
+        county = ""
+
+        if self.country == "GB" and self.postcode != "":
+            nomi = pgeocode.Nominatim("GB")
+            county = nomi.query_postal_code(sanitise_uk_postcode(self.postcode)).county_name
+
+        self.county = county
+        super(School, self).save(**kwargs)
+
 
 class TeacherModelManager(models.Manager):
     def factory(self, first_name, last_name, email, password):

cfl_common/setup.py

@@ -19,6 +19,7 @@
         "django-two-factor-auth==1.13.2",
         "django-countries==7.3.1",
         "pyjwt==2.6.0",
+        "pgeocode==0.4.0",
     ],
     tests_require=[],
     test_suite="tests",

portal/admin.py

@@ -30,9 +30,9 @@ def teacher_school(self, obj):
 
 
 class SchoolAdmin(admin.ModelAdmin, ExportActionMixin):
-    search_fields = ["name", "country", "postcode"]
-    list_filter = ["postcode", "country"]
-    list_display = ["__str__", "postcode", "country", "number_of_teachers", "number_of_classes"]
+    search_fields = ["name", "country", "postcode", "county"]
+    list_filter = ["county", "postcode", "country"]
+    list_display = ["__str__", "country", "county", "postcode", "number_of_teachers", "number_of_classes"]
 
     def number_of_teachers(self, obj):
         return len(obj.teacher_school.all())

portal/helpers/organisation.py

@@ -0,0 +1,10 @@
+# TODO: Move to Address model once we create it
+def sanitise_uk_postcode(postcode):
+    if len(postcode) >= 5:  # Valid UK postcodes are at least 5 chars long
+        outcode = postcode[:-3]  # UK incodes are always 3 characters
+
+        # Insert a space between outcode and incode if there isn't already one
+        if not outcode.endswith(" "):
+            postcode = postcode[:-3] + " " + postcode[-3:]
+
+    return postcode

portal/tests/test_helper_methods.py

@@ -1,7 +1,8 @@
-from portal.helpers.password import is_password_pwned
-import pytest
-from unittest.mock import patch, Mock
 import hashlib
+from unittest.mock import patch, Mock
+
+from portal.helpers.password import is_password_pwned
+from portal.helpers.organisation import sanitise_uk_postcode
 
 
 class TestClass:
@@ -11,42 +12,29 @@ def test_is_password_pwned(self):
         assert is_password_pwned(weak_password)
         assert not is_password_pwned(strong_password)
 
+    @patch("requests.get")
+    def test_is_password_pwned__status_code_not_200(self, mock_get):
+        # Arrange
+        password = "password123"
+        sha1_hash = hashlib.sha1(password.encode()).hexdigest()
 
-# This is your pytest test
-@patch("requests.get")
-def test_is_password_pwned(mock_get):
-    # Arrange
-    password = "password123"
-    sha1_hash = hashlib.sha1(password.encode()).hexdigest()
-
-    mock_response = Mock()
-    mock_response.status_code = 200
-    mock_response.text = f"{sha1_hash[5:]}\r\n"
-
-    mock_get.return_value = mock_response
-
-    # Act
-    result = is_password_pwned(password)
-
-    # Assert
-    mock_get.assert_called_once_with(f"https://api.pwnedpasswords.com/range/{sha1_hash[:5]}")
-    assert result == True
-
+        mock_response = Mock()
+        mock_response.status_code = 500
 
-@patch("requests.get")
-def test_is_password_pwned_status_code_not_200(mock_get):
-    # Arrange
-    password = "password123"
-    sha1_hash = hashlib.sha1(password.encode()).hexdigest()
+        mock_get.return_value = mock_response
 
-    mock_response = Mock()
-    mock_response.status_code = 500
+        # Act
+        result = is_password_pwned(password)
 
-    mock_get.return_value = mock_response
+        # Assert
+        mock_get.assert_called_once_with(f"https://api.pwnedpasswords.com/range/{sha1_hash[:5]}")
+        assert not result
 
-    # Act
-    result = is_password_pwned(password)
+    def test_sanitise_uk_postcode(self):
+        postcode_with_space = "AL10 9NE"
+        postcode_without_space = "AL109UL"
+        invalid_postcode = "123"
 
-    # Assert
-    mock_get.assert_called_once_with(f"https://api.pwnedpasswords.com/range/{sha1_hash[:5]}")
-    assert result == False
+        assert sanitise_uk_postcode(postcode_with_space) == "AL10 9NE"  # Check it stays the same
+        assert sanitise_uk_postcode(postcode_without_space) == "AL10 9UL"  # Check a space is added
+        assert sanitise_uk_postcode(invalid_postcode) == "123"  # Check nothing happens

portal/views/teacher/dashboard.py

@@ -260,6 +260,7 @@ def process_update_school_form(request, school, old_anchor):
         name = data.get("name", "")
         postcode = data.get("postcode", "")
         country = data.get("country", "")
+        county = school.county
 
         school.name = name
         school.postcode = postcode