fix: county field

cfl_common/common/migrations/0047_school_location.py

@@ -0,0 +1,36 @@
+# Generated by Django 3.2.20 on 2023-11-06 18:56
+
+from django.apps.registry import Apps
+from django.db import migrations, models
+
+
+def unique_school_names(apps: Apps, *args):
+    School = apps.get_model("common", "School")
+
+    schools = School.objects.values("name").annotate(name_count=models.Count("name")).filter(name_count__gt=1)
+    for school in schools:
+        schools = list(School.objects.filter(name=school["name"]).order_by("id"))
+        for index in range(school["name_count"]):
+            if index > 0:
+                school = schools[index]
+                school.name += f" {index + 1}"
+                school.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("common", "0046_alter_school_country"),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="school",
+            name="postcode",
+        ),
+        migrations.RunPython(code=unique_school_names),
+        migrations.AlterField(
+            model_name="school",
+            name="name",
+            field=models.CharField(max_length=200, unique=True),
+        ),
+    ]

cfl_common/common/models.py

@@ -8,8 +8,6 @@
 from django.utils import timezone
 from django_countries.fields import CountryField
 
-from .helpers.organisation import sanitise_uk_postcode
-
 
 class UserProfile(models.Model):
     user = models.OneToOneField(User, on_delete=models.CASCADE)
@@ -40,8 +38,7 @@ def get_queryset(self):
 
 
 class School(models.Model):
-    name = models.CharField(max_length=200)
-    postcode = models.CharField(max_length=10, null=True)
+    name = models.CharField(max_length=200, unique=True)
     country = CountryField(blank_label="(select country)", null=True, blank=True)
     # TODO: Create an Address model to house address details
     county = models.CharField(max_length=50, blank=True, null=True)
@@ -69,22 +66,9 @@ def admins(self):
 
     def anonymise(self):
         self.name = uuid4().hex
-        self.postcode = ""
         self.is_active = False
         self.save()
 
-    def save(self, **kwargs):
-        self.county = ""
-
-        if self.country == "GB":
-            if self.postcode.replace(" ", "") == "":
-                self.county = "nan"
-            else:
-                nomi = pgeocode.Nominatim("GB")
-                self.county = nomi.query_postal_code(sanitise_uk_postcode(self.postcode)).county_name
-
-        super(School, self).save(**kwargs)
-
 
 class TeacherModelManager(models.Manager):
     def factory(self, first_name, last_name, email, password):

cfl_common/common/tests/test_models.py

@@ -1,12 +1,12 @@
-from common.models import Student, Teacher, DailyActivity
+from common.models import DailyActivity, Student, Teacher
 from django.test import TestCase
 from django.utils import timezone
 
+from ..helpers.organisation import sanitise_uk_postcode
 from .utils.classes import create_class_directly
 from .utils.organisation import create_organisation_directly, join_teacher_to_organisation
 from .utils.student import create_independent_student_directly
 from .utils.teacher import signup_teacher_directly
-from ..helpers.organisation import sanitise_uk_postcode
 
 
 class TestModels(TestCase):
@@ -57,8 +57,8 @@ def test_school_admins(self):
         email2, password2 = signup_teacher_directly()
         email3, password3 = signup_teacher_directly()
         school = create_organisation_directly(email1)
-        join_teacher_to_organisation(email2, school.name, school.postcode)
-        join_teacher_to_organisation(email3, school.name, school.postcode, is_admin=True)
+        join_teacher_to_organisation(email2, school.name)
+        join_teacher_to_organisation(email3, school.name, is_admin=True)
 
         teacher1 = Teacher.objects.get(new_user__username=email1)
         teacher2 = Teacher.objects.get(new_user__username=email2)
@@ -69,28 +69,6 @@ def test_school_admins(self):
         assert teacher2 not in school.admins()
         assert teacher3 in school.admins()
 
-    def test_school_postcode(self):
-        """
-        Test that the school's county field gets populated according to country and postcode.
-        """
-        # Check that the county is found correctly (create school helper function uses Herts postcode)
-        teacher_email, _ = signup_teacher_directly()
-        school = create_organisation_directly(teacher_email)
-
-        assert school.county == "Hertfordshire"
-
-        # Check that an empty postcode outputs "nan" as county and doesn't throw an exception
-        school.postcode = " "
-        school.save()
-
-        assert school.county == "nan"
-
-        # Check that changing the country to something other than "GB" clears the county field
-        school.country = "FR"
-        school.save()
-
-        assert school.county == ""
-
     def test_sanitise_uk_postcode(self):
         postcode_with_space = "AL10 9NE"
         postcode_without_space = "AL109UL"

cfl_common/common/tests/utils/organisation.py

@@ -1,22 +1,21 @@
-from common.models import Teacher, School
+from common.models import School, Teacher
 
 
 def generate_details(**kwargs):
     name = kwargs.get("name", "School %d" % generate_details.next_id)
-    postcode = kwargs.get("postcode", "Al10 9NE")
 
     generate_details.next_id += 1
 
-    return name, postcode
+    return name
 
 
 generate_details.next_id = 1
 
 
 def create_organisation_directly(teacher_email, **kwargs):
-    name, postcode = generate_details(**kwargs)
+    name = generate_details(**kwargs)
 
-    school = School.objects.create(name=name, postcode=postcode, country="GB")
+    school = School.objects.create(name=name, country="GB")
 
     teacher = Teacher.objects.get(new_user__email=teacher_email)
     teacher.school = school
@@ -26,18 +25,17 @@ def create_organisation_directly(teacher_email, **kwargs):
     return school
 
 
-def join_teacher_to_organisation(teacher_email, org_name, postcode, is_admin=False):
+def join_teacher_to_organisation(teacher_email, org_name, is_admin=False):
     teacher = Teacher.objects.get(new_user__email=teacher_email)
-    school = School.objects.get(name=org_name, postcode=postcode)
+    school = School.objects.get(name=org_name)
 
     teacher.school = school
     teacher.is_admin = is_admin
     teacher.save()
 
 
 def create_organisation(page, password):
+    name = generate_details()
+    page = page.create_organisation(name, password)
 
-    name, postcode = generate_details()
-    page = page.create_organisation(name, password, postcode)
-
-    return page, name, postcode
+    return page, name

portal/admin.py

@@ -2,14 +2,14 @@
 
 from common.models import (
     Class,
+    DailyActivity,
+    DynamicElement,
     School,
     SchoolTeacherInvitation,
     Student,
     Teacher,
-    UserProfile,
-    DynamicElement,
-    DailyActivity,
     TotalActivity,
+    UserProfile,
 )
 from django.contrib import admin
 from django.contrib.auth.admin import UserAdmin
@@ -31,9 +31,9 @@ def teacher_school(self, obj):
 
 
 class SchoolAdmin(admin.ModelAdmin, ExportActionMixin):
-    search_fields = ["name", "country", "postcode", "county"]
-    list_filter = ["county", "postcode", "country"]
-    list_display = ["__str__", "country", "county", "postcode", "number_of_teachers", "number_of_classes"]
+    search_fields = ["name", "country", "county"]
+    list_filter = ["county", "country"]
+    list_display = ["__str__", "country", "county", "number_of_teachers", "number_of_classes"]
 
     def number_of_teachers(self, obj):
         return len(obj.teacher_school.all())

portal/forms/organisation.py

@@ -8,38 +8,160 @@
 
 
 class OrganisationForm(forms.ModelForm):
+    county = forms.ChoiceField(
+        choices=[
+            [None, "(select county)"],
+            ["Aberdeen City", "Aberdeen City"],
+            ["Aberdeenshire", "Aberdeenshire"],
+            ["Angus", "Angus"],
+            ["Argyll and Bute", "Argyll and Bute"],
+            ["Bedfordshire", "Bedfordshire"],
+            ["Belfast", "Belfast"],
+            ["Belfast Greater", "Belfast Greater"],
+            ["Berkshire", "Berkshire"],
+            ["Blaenau Gwent", "Blaenau Gwent"],
+            ["Bridgend", "Bridgend"],
+            ["Buckinghamshire", "Buckinghamshire"],
+            ["Caerphilly", "Caerphilly"],
+            ["Cambridgeshire", "Cambridgeshire"],
+            ["Cardiff", "Cardiff"],
+            ["Carmarthenshire", "Carmarthenshire"],
+            ["Ceredigion", "Ceredigion"],
+            ["Channel Islands", "Channel Islands"],
+            ["Cheshire", "Cheshire"],
+            ["City of Edinburgh", "City of Edinburgh"],
+            ["Clackmannanshire", "Clackmannanshire"],
+            ["Conwy", "Conwy"],
+            ["Cornwall", "Cornwall"],
+            ["County Antrim", "County Antrim"],
+            ["County Armagh", "County Armagh"],
+            ["County Down", "County Down"],
+            ["County Fermanagh", "County Fermanagh"],
+            ["County Londonderry", "County Londonderry"],
+            ["County Tyrone", "County Tyrone"],
+            ["County of Bristol", "County of Bristol"],
+            ["Cumbria", "Cumbria"],
+            ["Denbighshire", "Denbighshire"],
+            ["Derbyshire", "Derbyshire"],
+            ["Devon", "Devon"],
+            ["Dorset", "Dorset"],
+            ["Dumfries and Galloway", "Dumfries and Galloway"],
+            ["Dunbartonshire", "Dunbartonshire"],
+            ["Dundee City", "Dundee City"],
+            ["Durham", "Durham"],
+            ["East Ayrshire", "East Ayrshire"],
+            ["East Dunbartonshire", "East Dunbartonshire"],
+            ["East Lothian", "East Lothian"],
+            ["East Renfrewshire", "East Renfrewshire"],
+            ["East Riding of Yorkshire", "East Riding of Yorkshire"],
+            ["East Sussex", "East Sussex"],
+            ["Essex", "Essex"],
+            ["Falkirk", "Falkirk"],
+            ["Fife", "Fife"],
+            ["Flintshire", "Flintshire"],
+            ["Glasgow City", "Glasgow City"],
+            ["Gloucestershire", "Gloucestershire"],
+            ["Greater London", "Greater London"],
+            ["Greater Manchester", "Greater Manchester"],
+            ["Guernsey Channel Islands", "Guernsey Channel Islands"],
+            ["Gwynedd", "Gwynedd"],
+            ["Hampshire", "Hampshire"],
+            ["Hereford and Worcester", "Hereford and Worcester"],
+            ["Herefordshire", "Herefordshire"],
+            ["Hertfordshire", "Hertfordshire"],
+            ["Highland", "Highland"],
+            ["Inverclyde", "Inverclyde"],
+            ["Inverness", "Inverness"],
+            ["Isle of Anglesey", "Isle of Anglesey"],
+            ["Isle of Barra", "Isle of Barra"],
+            ["Isle of Man", "Isle of Man"],
+            ["Isle of Wight", "Isle of Wight"],
+            ["Jersey Channel Islands", "Jersey Channel Islands"],
+            ["Kent", "Kent"],
+            ["Lancashire", "Lancashire"],
+            ["Leicestershire", "Leicestershire"],
+            ["Lincolnshire", "Lincolnshire"],
+            ["Merseyside", "Merseyside"],
+            ["Merthyr Tydfil", "Merthyr Tydfil"],
+            ["Midlothian", "Midlothian"],
+            ["Monmouthshire", "Monmouthshire"],
+            ["Moray", "Moray"],
+            ["Neath Port Talbot", "Neath Port Talbot"],
+            ["Newport", "Newport"],
+            ["Norfolk", "Norfolk"],
+            ["North Ayrshire", "North Ayrshire"],
+            ["North Lanarkshire", "North Lanarkshire"],
+            ["North Yorkshire", "North Yorkshire"],
+            ["Northamptonshire", "Northamptonshire"],
+            ["Northumberland", "Northumberland"],
+            ["Nottinghamshire", "Nottinghamshire"],
+            ["Orkney", "Orkney"],
+            ["Orkney Islands", "Orkney Islands"],
+            ["Oxfordshire", "Oxfordshire"],
+            ["Pembrokeshire", "Pembrokeshire"],
+            ["Perth and Kinross", "Perth and Kinross"],
+            ["Powys", "Powys"],
+            ["Renfrewshire", "Renfrewshire"],
+            ["Rhondda Cynon Taff", "Rhondda Cynon Taff"],
+            ["Rutland", "Rutland"],
+            ["Scottish Borders", "Scottish Borders"],
+            ["Shetland Islands", "Shetland Islands"],
+            ["Shropshire", "Shropshire"],
+            ["Somerset", "Somerset"],
+            ["South Ayrshire", "South Ayrshire"],
+            ["South Lanarkshire", "South Lanarkshire"],
+            ["South Yorkshire", "South Yorkshire"],
+            ["Staffordshire", "Staffordshire"],
+            ["Stirling", "Stirling"],
+            ["Suffolk", "Suffolk"],
+            ["Surrey", "Surrey"],
+            ["Swansea", "Swansea"],
+            ["Torfaen", "Torfaen"],
+            ["Tyne and Wear", "Tyne and Wear"],
+            ["Vale of Glamorgan", "Vale of Glamorgan"],
+            ["Warwickshire", "Warwickshire"],
+            ["West Dunbart", "West Dunbart"],
+            ["West Lothian", "West Lothian"],
+            ["West Midlands", "West Midlands"],
+            ["West Sussex", "West Sussex"],
+            ["West Yorkshire", "West Yorkshire"],
+            ["Western Isles", "Western Isles"],
+            ["Wiltshire", "Wiltshire"],
+            ["Worcestershire", "Worcestershire"],
+            ["Wrexham", "Wrexham"],
+        ],
+        required=False,
+        help_text="County (optional)",
+    )
+
     class Meta(object):
         model = School
-        fields = ["name", "postcode", "country"]
+        fields = ["name", "country", "county"]
         widgets = {
             "name": forms.TextInput(attrs={"autocomplete": "off", "placeholder": "Name of school or club"}),
-            "postcode": forms.TextInput(attrs={"autocomplete": "off", "placeholder": "Postcode / Zipcode"}),
             "country": CountrySelectWidget(layout="{widget}"),
         }
         help_texts = {
             "name": "Name of school or club",
-            "postcode": "Postcode / Zipcode",
             "country": "Country (optional)",
         }
 
     def __init__(self, *args, **kwargs):
         self.user = kwargs.pop("user", None)
         self.current_school = kwargs.pop("current_school", None)
         super(OrganisationForm, self).__init__(*args, **kwargs)
-        self.fields["postcode"].strip = False
 
     def clean(self):
         name = self.cleaned_data.get("name", None)
-        postcode = self.cleaned_data.get("postcode", None)
 
-        if name and postcode:
+        if name:
             try:
-                school = School.objects.get(name=name, postcode=postcode)
+                school = School.objects.get(name=name)
             except ObjectDoesNotExist:
                 return self.cleaned_data
 
             if not self.current_school or self.current_school.id != school.id:
-                raise forms.ValidationError("There is already a school or club registered with that name and postcode")
+                raise forms.ValidationError("There is already a school or club registered with that name")
 
         return self.cleaned_data
 
@@ -58,12 +180,3 @@ def clean_name(self):
                 raise forms.ValidationError("Please make sure your organisation name is valid")
 
         return name
-
-    def clean_postcode(self):
-        postcode = self.cleaned_data.get("postcode", None)
-
-        if postcode:
-            if len(postcode.replace(" ", "")) > 10 or len(postcode.replace(" ", "")) == 0:
-                raise forms.ValidationError("Please enter a valid postcode or ZIP code")
-
-        return postcode