diff --git a/.gitignore b/.gitignore index 860f893ec..b7db5e738 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,5 @@ node_modules # tunnel software ngrok + +.venv \ No newline at end of file diff --git a/cfl_common/common/migrations/0047_school_location.py b/cfl_common/common/migrations/0047_school_location.py new file mode 100644 index 000000000..3dc3f63e9 --- /dev/null +++ b/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), + ), + ] diff --git a/cfl_common/common/models.py b/cfl_common/common/models.py index ebfa30f99..9bd198f90 100644 --- a/cfl_common/common/models.py +++ b/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): diff --git a/cfl_common/common/tests/test_models.py b/cfl_common/common/tests/test_models.py index 488cef54b..a45cf7136 100644 --- a/cfl_common/common/tests/test_models.py +++ b/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" diff --git a/cfl_common/common/tests/utils/organisation.py b/cfl_common/common/tests/utils/organisation.py index a559121c5..33843b53b 100644 --- a/cfl_common/common/tests/utils/organisation.py +++ b/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,9 +25,9 @@ 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 @@ -36,8 +35,7 @@ def join_teacher_to_organisation(teacher_email, org_name, postcode, is_admin=Fal 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 diff --git a/portal/admin.py b/portal/admin.py index 86dcc0641..ccf68575a 100644 --- a/portal/admin.py +++ b/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()) diff --git a/portal/forms/organisation.py b/portal/forms/organisation.py index fea2fc740..98697810c 100644 --- a/portal/forms/organisation.py +++ b/portal/forms/organisation.py @@ -8,17 +8,141 @@ 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)", } @@ -26,20 +150,18 @@ 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 diff --git a/portal/static/portal/js/school.js b/portal/static/portal/js/school.js new file mode 100644 index 000000000..116f19331 --- /dev/null +++ b/portal/static/portal/js/school.js @@ -0,0 +1,13 @@ +$(document).ready(() => { + if ($("#id_country").val() !== 'GB') { + $('#form-row-county').hide(); + } + + $('#id_country').on('change', (event) => { + if (event.target.value === 'GB') { + $('#form-row-county').show(); + } else { + $('#form-row-county').hide(); + } + }); +}); diff --git a/portal/templates/portal/teach/dashboard.html b/portal/templates/portal/teach/dashboard.html index 4f91c5fcd..b50266de1 100644 --- a/portal/templates/portal/teach/dashboard.html +++ b/portal/templates/portal/teach/dashboard.html @@ -59,7 +59,7 @@

Welcome back, {{ user|make_into_username }}
-

Your school: {% if user.new_teacher.school %} {{ user.new_teacher.school.name }} ({{ user.new_teacher.school.postcode }}){% endif %}

+

Your school: {% if user.new_teacher.school %} {{ user.new_teacher.school.name }}{% endif %}

{% if is_admin %}

@@ -235,10 +235,10 @@

Update details of your school or club
{{ update_school_form.non_field_errors }}
{% for field in update_school_form %} -
+
{{ field }} - {% if not field == update_school_form.country %} + {% if not field == update_school_form.country and not field == update_school_form.county %} {% endif %}
@@ -553,4 +553,7 @@
Delete account
+ + {% endblock content %} diff --git a/portal/templates/portal/teach/onboarding_school.html b/portal/templates/portal/teach/onboarding_school.html index 2c1cbb406..6893f3582 100644 --- a/portal/templates/portal/teach/onboarding_school.html +++ b/portal/templates/portal/teach/onboarding_school.html @@ -36,11 +36,11 @@

Create a school or club

{{ create_form.non_field_errors }} {% for field in create_form %} -
+
{{ field }} - {% if not field == create_form.country %} + {% if not field == create_form.country and not field == create_form.county %} {% endif %}
@@ -60,4 +60,7 @@

Create a school or club

{% endif %} + + {% endblock content %} diff --git a/portal/tests/pageObjects/portal/teach/dashboard_page.py b/portal/tests/pageObjects/portal/teach/dashboard_page.py index 4edcfda1e..6a5f72921 100644 --- a/portal/tests/pageObjects/portal/teach/dashboard_page.py +++ b/portal/tests/pageObjects/portal/teach/dashboard_page.py @@ -3,11 +3,12 @@ from selenium.webdriver.common.by import By from selenium.webdriver.support.ui import Select -from .add_independent_student_to_class_page import AddIndependentStudentToClassPage +from ..email_verification_needed_page import EmailVerificationNeededPage +from .add_independent_student_to_class_page import \ + AddIndependentStudentToClassPage from .class_page import TeachClassPage from .move_classes_page import TeachMoveClassesPage from .teach_base_page import TeachBasePage -from ..email_verification_needed_page import EmailVerificationNeededPage class TeachDashboardPage(TeachBasePage): @@ -54,7 +55,7 @@ def change_organisation_details(self, details): def has_edit_failed(self): self.wait_for_element_by_id("edit_form") errorlist = self.browser.find_element(By.ID, "edit_form").find_element(By.CLASS_NAME, "errorlist").text - error = "There is already a school or club registered with that name and postcode" + error = "There is already a school or club registered with that name" return error in errorlist def create_class(self, name, classmate_progress, teacher_id=None): @@ -92,7 +93,8 @@ def change_password(self, first_name, last_name, new_password, password): } ) - from portal.tests.pageObjects.portal.teacher_login_page import TeacherLoginPage + from portal.tests.pageObjects.portal.teacher_login_page import \ + TeacherLoginPage return TeacherLoginPage(self.browser) diff --git a/portal/tests/pageObjects/portal/teach/onboarding_organisation_page.py b/portal/tests/pageObjects/portal/teach/onboarding_organisation_page.py index d0895c014..0e44253bd 100644 --- a/portal/tests/pageObjects/portal/teach/onboarding_organisation_page.py +++ b/portal/tests/pageObjects/portal/teach/onboarding_organisation_page.py @@ -13,13 +13,13 @@ def __init__(self, browser): assert self.on_correct_page("onboarding_organisation_page") - def create_organisation(self, name, password, postcode, country="GB"): - self._create_organisation(name, password, postcode, country) + def create_organisation(self, name, password, country="GB"): + self._create_organisation(name, password, country) return onboarding_classes_page.OnboardingClassesPage(self.browser) - def create_organisation_failure(self, name, password, postcode, country="GB"): - self._create_organisation(name, password, postcode, country) + def create_organisation_failure(self, name, password, country="GB"): + self._create_organisation(name, password, country) return self @@ -28,9 +28,8 @@ def create_organisation_empty(self): return self - def _create_organisation(self, name, password, postcode, country): + def _create_organisation(self, name, password, country): self.browser.find_element(By.ID, "id_name").send_keys(name) - self.browser.find_element(By.ID, "id_postcode").send_keys(postcode) country_element = self.browser.find_element(By.ID, "id_country") select = Select(country_element) select.select_by_value(country) diff --git a/portal/tests/test_api.py b/portal/tests/test_api.py index f53a732e8..88ebf0af7 100644 --- a/portal/tests/test_api.py +++ b/portal/tests/test_api.py @@ -124,7 +124,7 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler school2_teacher1_email, _ = signup_teacher_directly() school2_teacher2_email, _ = signup_teacher_directly() school2 = create_organisation_directly(school2_teacher1_email) - join_teacher_to_organisation(school2_teacher2_email, school2.name, school2.postcode, is_admin=True) + join_teacher_to_organisation(school2_teacher2_email, school2.name, is_admin=True) klass21, _, access_code21 = create_class_directly(school2_teacher1_email) _, _, student21 = create_school_student_directly(access_code21) klass22, _, access_code22 = create_class_directly(school2_teacher2_email) @@ -140,7 +140,7 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler school3_teacher1_email, _ = signup_teacher_directly() school3_teacher2_email, _ = signup_teacher_directly() school3 = create_organisation_directly(school3_teacher1_email) - join_teacher_to_organisation(school3_teacher2_email, school3.name, school3.postcode) + join_teacher_to_organisation(school3_teacher2_email, school3.name) klass31, _, access_code31 = create_class_directly(school3_teacher1_email) _, _, student31 = create_school_student_directly(access_code31) klass32, _, access_code32 = create_class_directly(school3_teacher2_email) @@ -171,12 +171,12 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler assert response.status_code == status.HTTP_204_NO_CONTENT # Check the first school/class/student still exist - assert School.objects.filter(name=school1.name, postcode=school1.postcode).exists() + assert School.objects.filter(name=school1.name).exists() assert Class.objects.filter(pk=klass11.pk).exists() assert Student.objects.filter(pk=student11.pk).exists() # Check the second school exists and its first class/student, but the second ones are anonymised - assert School.objects.filter(name=school2.name, postcode=school2.postcode).exists() + assert School.objects.filter(name=school2.name).exists() assert Class.objects.filter(pk=klass21.pk).exists() assert not Class.objects.filter(pk=klass22.pk).exists() assert Student.objects.filter(pk=student21.pk).exists() @@ -185,17 +185,17 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler assert Teacher.objects.get(new_user__email=school2_teacher1_email).is_admin # Check the third school is anonymised together with its classes and students - assert not School.objects.filter(name=school3.name, postcode=school3.postcode).exists() + assert not School.objects.filter(name=school3.name).exists() assert not Class.objects.filter(pk=klass31.pk).exists() assert not Class.objects.filter(pk=klass32.pk).exists() assert not Student.objects.get(pk=student31.pk).new_user.is_active assert not Student.objects.get(pk=student32.pk).new_user.is_active # Check that the fourth school is anonymised - assert not School.objects.filter(name=school4.name, postcode=school4.postcode).exists() + assert not School.objects.filter(name=school4.name).exists() # Check that the fifth school is anonymised - assert not School.objects.filter(name=school5.name, postcode=school5.postcode).exists() + assert not School.objects.filter(name=school5.name).exists() def test_remove_fake_accounts(self): client = APIClient() diff --git a/portal/tests/test_class.py b/portal/tests/test_class.py index 39e7279ee..a2e661ebc 100644 --- a/portal/tests/test_class.py +++ b/portal/tests/test_class.py @@ -1,7 +1,9 @@ from __future__ import absolute_import +from datetime import datetime, timedelta + from aimmo.models import Game -from common.models import Class, Teacher +from common.models import Class, DailyActivity, Teacher from common.tests.utils.classes import create_class_directly from common.tests.utils.organisation import create_organisation_directly, join_teacher_to_organisation from common.tests.utils.student import create_school_student_directly @@ -16,11 +18,6 @@ from .utils.messages import is_class_created_message_showing -from common.models import DailyActivity - -from datetime import timedelta, datetime - - class TestClass(TestCase): def test_class_deletion_deletes_game(self): email, password = signup_teacher_directly() @@ -257,7 +254,7 @@ def test_transfer_class(self): email1, password1 = signup_teacher_directly() email2, password2 = signup_teacher_directly() school = create_organisation_directly(email1) - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) klass1, _, access_code1 = create_class_directly(email1) klass2, _, access_code2 = create_class_directly(email2) _, _, student1 = create_school_student_directly(access_code1) @@ -312,7 +309,7 @@ def test_create_class_as_admin_for_another_teacher(self): email2, password2 = signup_teacher_directly() teacher2 = Teacher.objects.get(new_user__email=email2) school = create_organisation_directly(email1) - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) # Check teacher 2 doesn't have any classes page = self.go_to_homepage().go_to_teacher_login_page().login(email2, password2).open_classes_tab() @@ -348,7 +345,7 @@ def test_create_dashboard_non_admin(self): school = create_organisation_directly(email_1) klass_1, class_name_1, access_code_1 = create_class_directly(email_1) create_school_student_directly(access_code_1) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) klass_2, class_name_2, access_code_2 = create_class_directly(email_2) create_school_student_directly(access_code_2) diff --git a/portal/tests/test_independent_student.py b/portal/tests/test_independent_student.py index 0bf2a0095..a81886d1a 100644 --- a/portal/tests/test_independent_student.py +++ b/portal/tests/test_independent_student.py @@ -23,15 +23,16 @@ from selenium.webdriver.support.wait import WebDriverWait from portal.forms.error_messages import INVALID_LOGIN_MESSAGE + from .base_test import BaseTest from .pageObjects.portal.home_page import HomePage from .utils.messages import ( + is_email_updated_message_showing, is_email_verified_message_showing, is_indep_student_join_request_received_message_showing, is_indep_student_join_request_revoked_message_showing, - is_student_details_updated_message_showing, - is_email_updated_message_showing, is_password_updated_message_showing, + is_student_details_updated_message_showing, ) @@ -561,7 +562,7 @@ def test_join_class_denied_and_accepted_by_admin(self): admin_email, admin_password1 = signup_teacher_directly() standard_email, _ = signup_teacher_directly() school = create_organisation_directly(admin_email) - join_teacher_to_organisation(standard_email, school.name, school.postcode, is_admin=False) + join_teacher_to_organisation(standard_email, school.name, is_admin=False) # Create class for standard teacher which always accepts external requests klass, class_name, access_code = create_class_directly(standard_email) diff --git a/portal/tests/test_organisation.py b/portal/tests/test_organisation.py index 7e39b554b..c4512640d 100644 --- a/portal/tests/test_organisation.py +++ b/portal/tests/test_organisation.py @@ -4,11 +4,9 @@ from common.models import Teacher from common.tests.utils.classes import create_class_directly -from common.tests.utils.organisation import ( - create_organisation, - create_organisation_directly, - join_teacher_to_organisation, -) +from common.tests.utils.organisation import (create_organisation, + create_organisation_directly, + join_teacher_to_organisation) from common.tests.utils.student import create_school_student_directly from common.tests.utils.teacher import signup_teacher_directly from selenium.webdriver.common.by import By @@ -16,6 +14,7 @@ from portal.tests.pageObjects.portal.base_page import BasePage from portal.tests.pageObjects.portal.home_page import HomePage from portal.tests.test_invite_teacher import FADE_TIME + from .base_test import BaseTest from .utils.messages import is_organisation_created_message_showing @@ -27,33 +26,9 @@ def test_create(self): self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login_no_school(email, password) - page, name, postcode = create_organisation(page, password) + page, name = create_organisation(page, password) assert is_organisation_created_message_showing(self.selenium, name) - def test_create_clash(self): - email_1, _ = signup_teacher_directly() - email_2, password_2 = signup_teacher_directly() - school = create_organisation_directly(email_1) - - self.selenium.get(self.live_server_url) - page = ( - HomePage(self.selenium) - .go_to_teacher_login_page() - .login_no_school(email_2, password_2) - .create_organisation_failure(school.name, password_2, school.postcode) - ) - - assert page.has_creation_failed() - - def test_create_invalid_postcode(self): - email, password = signup_teacher_directly() - - self.selenium.get(self.live_server_url) - page = HomePage(self.selenium).go_to_teacher_login_page().login_no_school(email, password) - - page = page.create_organisation_failure("School", password, " ") - assert page.was_postcode_invalid() - def test_kick(self): email_1, password_1 = signup_teacher_directly() email_2, password_2 = signup_teacher_directly() @@ -61,7 +36,7 @@ def test_kick(self): _, _, access_code = create_class_directly(email_1) create_school_student_directly(access_code) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_1, password_1) @@ -85,7 +60,7 @@ def test_move_classes_and_kick(self): _, _, access_code_2 = create_class_directly(email_2) create_school_student_directly(access_code_2) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_1, password_1) @@ -113,7 +88,7 @@ def test_leave_organisation(self): _, class_name_2, access_code_2 = create_class_directly(email_2) create_school_student_directly(access_code_2) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_2, password_2) @@ -138,7 +113,7 @@ def test_toggle_admin(self): school = create_organisation_directly(email_1) _, _, access_code = create_class_directly(email_1) create_school_student_directly(access_code) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_1, password_1) @@ -163,7 +138,7 @@ def test_disable_2FA(self): school = create_organisation_directly(email_1) _, _, access_code = create_class_directly(email_1) create_school_student_directly(access_code) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_1, password_1) @@ -192,13 +167,12 @@ def test_edit_details(self): self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email, password) - assert page.check_organisation_details({"name": school.name, "postcode": school.postcode}) + assert page.check_organisation_details({"name": school.name}) new_name = "new " + school.name - new_postcode = "OX2 6LE" - page.change_organisation_details({"name": new_name, "postcode": new_postcode}) - assert page.check_organisation_details({"name": new_name, "postcode": new_postcode}) + page.change_organisation_details({"name": new_name}) + assert page.check_organisation_details({"name": new_name}) def test_edit_clash(self): email_1, _ = signup_teacher_directly() @@ -213,8 +187,8 @@ def test_edit_clash(self): self.selenium.get(self.live_server_url) page = HomePage(self.selenium).go_to_teacher_login_page().login(email_2, password_2) - assert not page.check_organisation_details({"name": school1.name, "postcode": school1.postcode}) + assert not page.check_organisation_details({"name": school1.name}) - page = page.change_organisation_details({"name": school1.name, "postcode": school1.postcode}) + page = page.change_organisation_details({"name": school1.name}) assert page.has_edit_failed() diff --git a/portal/tests/test_teacher.py b/portal/tests/test_teacher.py index 2b01e3bb3..978a91afd 100644 --- a/portal/tests/test_teacher.py +++ b/portal/tests/test_teacher.py @@ -30,14 +30,15 @@ from portal.forms.error_messages import INVALID_LOGIN_MESSAGE from portal.tests.base_test import click_buttons_by_id from portal.tests.test_invite_teacher import WAIT_TIME + from .base_test import BaseTest from .pageObjects.portal.home_page import HomePage from .utils.messages import ( - is_message_showing, - is_email_verified_message_showing, - is_teacher_details_updated_message_showing, is_email_updated_message_showing, + is_email_verified_message_showing, + is_message_showing, is_password_updated_message_showing, + is_teacher_details_updated_message_showing, ) @@ -107,7 +108,7 @@ def test_moved_class_has_correct_permissions_for_students_and_teachers(self): # Create teacher 2 -> class 2 -> student 2 email2, password2 = signup_teacher_directly() - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) klass2, _, access_code2 = create_class_directly(email2, "Class 2") create_school_student_directly(access_code2) @@ -189,7 +190,7 @@ def test_moved_student_has_access_to_only_new_teacher_games(self): create_school_student_directly(access_code1) email2, password2 = signup_teacher_directly() - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) klass2, _, access_code2 = create_class_directly(email2, "Class 2") create_school_student_directly(access_code2) @@ -521,7 +522,7 @@ def test_edit_details_non_admin(self): school = create_organisation_directly(email_1) _, _, access_code_1 = create_class_directly(email_1) create_school_student_directly(access_code_1) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) _, _, access_code_2 = create_class_directly(email_2) create_school_student_directly(access_code_2) @@ -671,7 +672,7 @@ def test_admin_sees_all_school_classes(self): # create non_admin account to join the school # check if they cannot see classes standard_email, standard_password = signup_teacher_directly() - join_teacher_to_organisation(standard_email, school.name, school.postcode) + join_teacher_to_organisation(standard_email, school.name) page = ( self.go_to_homepage().go_to_teacher_login_page().login(standard_email, standard_password).open_classes_tab() @@ -684,7 +685,7 @@ def test_admin_sees_all_school_classes(self): # if the teacher can see the classes admin_email, admin_password = signup_teacher_directly() - join_teacher_to_organisation(admin_email, school.name, school.postcode, is_admin=True) + join_teacher_to_organisation(admin_email, school.name, is_admin=True) page = self.go_to_homepage().go_to_teacher_login_page().login(admin_email, admin_password).open_classes_tab() class_code_field = page.browser.find_element(By.ID, f"class-code-{access_code}") @@ -698,7 +699,7 @@ def test_admin_student_edit(self): student_name, student_password, student_student = create_school_student_directly(access_code) joining_email, joining_password = signup_teacher_directly() - join_teacher_to_organisation(joining_email, school.name, school.postcode, is_admin=True) + join_teacher_to_organisation(joining_email, school.name, is_admin=True) page = ( self.go_to_homepage().go_to_teacher_login_page().login(joining_email, joining_password).open_classes_tab() @@ -758,7 +759,7 @@ def test_make_admin_popup(self): # Non admin teacher joined - make admin should also make a popup - join_teacher_to_organisation(joining_email, school.name, school.postcode) + join_teacher_to_organisation(joining_email, school.name) # refresh the page and scroll to the buttons page.browser.execute_script("location.reload()") diff --git a/portal/tests/test_teacher_student.py b/portal/tests/test_teacher_student.py index 8f7dc0aef..12ac78ec6 100644 --- a/portal/tests/test_teacher_student.py +++ b/portal/tests/test_teacher_student.py @@ -20,6 +20,7 @@ from selenium.webdriver.common.by import By from portal.tests.pageObjects.portal.home_page import HomePage + from .base_test import BaseTest @@ -473,7 +474,7 @@ def test_move_cancel_disambiguate(self): old_teacher_email, password_1 = signup_teacher_directly() email_2, _ = signup_teacher_directly() school = create_organisation_directly(old_teacher_email) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) _, _, access_code_1 = create_class_directly(old_teacher_email) create_class_directly(email_2) student_name, _, _ = create_school_student_directly(access_code_1) @@ -498,7 +499,7 @@ def test_move(self): email_1, password_1 = signup_teacher_directly() email_2, password_2 = signup_teacher_directly() school = create_organisation_directly(email_1) - join_teacher_to_organisation(email_2, school.name, school.postcode) + join_teacher_to_organisation(email_2, school.name) _, _, access_code_1 = create_class_directly(email_1) create_class_directly(email_2) student_name_1, _, _ = create_school_student_directly(access_code_1) diff --git a/portal/tests/test_views.py b/portal/tests/test_views.py index fb0d07873..cfba21ea6 100644 --- a/portal/tests/test_views.py +++ b/portal/tests/test_views.py @@ -151,8 +151,8 @@ def test_csv(self): def test_organisation_kick_has_correct_permissions(self): teacher2_email, _ = signup_teacher_directly() school = create_organisation_directly(self.email) - join_teacher_to_organisation(self.email, school.name, school.postcode, is_admin=True) - join_teacher_to_organisation(teacher2_email, school.name, school.postcode) + join_teacher_to_organisation(self.email, school.name, is_admin=True) + join_teacher_to_organisation(teacher2_email, school.name) teacher2_id = Teacher.objects.get(new_user__email=teacher2_email).id client = self.login() @@ -599,12 +599,12 @@ def test_delete_account_admin(self): _, _, student = create_school_student_directly(access_code_1) student_user_id = student.new_user.id - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) _, _, access_code_2 = create_class_directly(email2) create_school_student_directly(access_code_2) - join_teacher_to_organisation(email3, school.name, school.postcode) - join_teacher_to_organisation(email4, school.name, school.postcode) + join_teacher_to_organisation(email3, school.name) + join_teacher_to_organisation(email4, school.name) c = Client() url = reverse("teacher_login") @@ -675,7 +675,6 @@ def test_delete_account_admin(self): # school should be anonymised school = School._base_manager.get(id=school_id) assert school.name != school_name - assert school.postcode == "" assert not school.is_active with pytest.raises(School.DoesNotExist): @@ -691,7 +690,7 @@ def test_logged_in_as_admin_check(self): email1, password1 = signup_teacher_directly() email2, password2 = signup_teacher_directly() school = create_organisation_directly(email1) - join_teacher_to_organisation(email2, school.name, school.postcode) + join_teacher_to_organisation(email2, school.name) teacher1 = Teacher.objects.get(new_user__username=email1) teacher2 = Teacher.objects.get(new_user__username=email2) diff --git a/portal/views/organisation.py b/portal/views/organisation.py index 79bbce9f1..ef4750225 100644 --- a/portal/views/organisation.py +++ b/portal/views/organisation.py @@ -21,10 +21,9 @@ def organisation_create(request): if create_form.is_valid(): data = create_form.cleaned_data name = data.get("name", "") - postcode = data.get("postcode", "").upper() country = data.get("country") - school = School.objects.create(name=name, postcode=postcode, country=country) + school = School.objects.create(name=name, country=country) teacher.school = school teacher.is_admin = True diff --git a/portal/views/teacher/dashboard.py b/portal/views/teacher/dashboard.py index b28b98217..9dc4d84fa 100644 --- a/portal/views/teacher/dashboard.py +++ b/portal/views/teacher/dashboard.py @@ -2,17 +2,12 @@ from uuid import uuid4 from common import email_messages -from common.helpers.emails import ( - INVITE_FROM, - NOTIFICATION_EMAIL, - DotmailerUserType, - add_to_dotmailer, - generate_token, - send_email, - update_email, -) +from common.helpers.emails import (INVITE_FROM, NOTIFICATION_EMAIL, + DotmailerUserType, add_to_dotmailer, + generate_token, send_email, update_email) from common.helpers.generators import get_random_username -from common.models import Class, JoinReleaseStudent, SchoolTeacherInvitation, Student, Teacher +from common.models import (Class, JoinReleaseStudent, SchoolTeacherInvitation, + Student, Teacher) from common.permissions import check_teacher_authorised, logged_in_as_teacher from common.utils import using_two_factor from django.contrib import messages as messages @@ -30,20 +25,14 @@ from portal.forms.invite_teacher import InviteTeacherForm from portal.forms.organisation import OrganisationForm from portal.forms.registration import DeleteAccountForm -from portal.forms.teach import ( - ClassCreationForm, - InvitedTeacherForm, - TeacherAddExternalStudentForm, - TeacherEditAccountForm, -) +from portal.forms.teach import (ClassCreationForm, InvitedTeacherForm, + TeacherAddExternalStudentForm, + TeacherEditAccountForm) from portal.helpers.decorators import ratelimit from portal.helpers.password import check_update_password -from portal.helpers.ratelimit import ( - RATELIMIT_LOGIN_GROUP, - RATELIMIT_LOGIN_RATE, - RATELIMIT_METHOD, - clear_ratelimit_cache_for_user, -) +from portal.helpers.ratelimit import (RATELIMIT_LOGIN_GROUP, + RATELIMIT_LOGIN_RATE, RATELIMIT_METHOD, + clear_ratelimit_cache_for_user) from .teach import create_class @@ -92,8 +81,8 @@ def dashboard_teacher_view(request, is_admin): update_school_form = OrganisationForm(user=request.user, current_school=school) update_school_form.fields["name"].initial = school.name - update_school_form.fields["postcode"].initial = school.postcode update_school_form.fields["country"].initial = school.country + update_school_form.fields["county"].initial = school.county invite_teacher_form = InviteTeacherForm() @@ -259,12 +248,10 @@ def process_update_school_form(request, school, old_anchor): if update_school_form.is_valid(): data = update_school_form.cleaned_data name = data.get("name", "") - postcode = data.get("postcode", "") country = data.get("country") county = school.county school.name = name - school.postcode = postcode school.country = country school.save()