diff --git a/.codecov.yml b/.codecov.yml
index 2e60774f8..5f7650737 100644
--- a/.codecov.yml
+++ b/.codecov.yml
@@ -6,6 +6,9 @@ coverage:
patch:
default:
target: 90%
+ project:
+ default:
+ target: 90%
ignore:
- "portal/tests/*.py"
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 54b108e69..2c145b1c9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -6,9 +6,11 @@ on:
jobs:
test:
name: Run tests
- runs-on: ubuntu-20.04
+ runs-on: ubuntu-latest
env:
LANG: C.UTF-8
+ COVERAGE_REPORT: coverage.xml # NOTE: COVERAGE_FILE is reserved - do not use.
+ OCADO_TECH_ORG_ID: 2088731
steps:
- name: Checkout
uses: actions/checkout@v3
@@ -48,4 +50,9 @@ jobs:
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload coverage to Codecov
- uses: codecov/codecov-action@v3
+ if: github.repository_owner_id == env.OCADO_TECH_ORG_ID
+ uses: codecov/codecov-action@v4
+ with:
+ fail_ci_if_error: true
+ token: ${{ secrets.CODECOV_TOKEN }}
+ file: ${{ env.COVERAGE_REPORT }}
diff --git a/.github/workflows/publish-python-package.yml b/.github/workflows/publish-python-package.yml
index beac2edd0..aac41fa7c 100644
--- a/.github/workflows/publish-python-package.yml
+++ b/.github/workflows/publish-python-package.yml
@@ -9,7 +9,7 @@ on:
jobs:
publish-pypi-packages:
name: Publish PyPi Packages
- runs-on: ubuntu-20.04
+ runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
diff --git a/.github/workflows/semantic-pull-request-check.yml b/.github/workflows/semantic-pull-request-check.yml
index 7371d424c..8c0d9e4ba 100644
--- a/.github/workflows/semantic-pull-request-check.yml
+++ b/.github/workflows/semantic-pull-request-check.yml
@@ -10,7 +10,7 @@ on:
jobs:
main:
name: Validate PR title
- runs-on: ubuntu-20.04
+ runs-on: ubuntu-latest
steps:
- uses: amannn/action-semantic-pull-request@v5
env:
diff --git a/.github/workflows/snyk.yaml b/.github/workflows/snyk.yaml
index 0872aa8e4..2aad94ad6 100644
--- a/.github/workflows/snyk.yaml
+++ b/.github/workflows/snyk.yaml
@@ -6,7 +6,7 @@ on:
jobs:
security:
name: Run Snyk
- runs-on: ubuntu-20.04
+ runs-on: ubuntu-latest
env:
LANG: C.UTF-8
steps:
diff --git a/cfl_common/common/email_messages.py b/cfl_common/common/email_messages.py
index 4e9f91f05..e3394cdbe 100644
--- a/cfl_common/common/email_messages.py
+++ b/cfl_common/common/email_messages.py
@@ -29,30 +29,6 @@ def kickedEmail(request, schoolName):
}
-def adminGivenEmail(request, schoolName):
-
- url = request.build_absolute_uri(reverse("dashboard"))
-
- return {
- "subject": f"You have been made a school or club administrator",
- "message": (
- f"Administrator control of the school or club '{schoolName}' has been "
- f"given to you. Go to {url} to start managing your school or club."
- ),
- }
-
-
-def adminRevokedEmail(request, schoolName):
- return {
- "subject": f"You are no longer a school or club administrator",
- "message": (
- f"Your administrator control of the school or club '{schoolName}' has been "
- f"revoked. If you think this is an error, please contact one of the other "
- f"administrators in your school or club."
- ),
- }
-
-
def studentJoinRequestSentEmail(request, schoolName, accessCode):
return {
"subject": f"School or club join request sent",
diff --git a/cfl_common/common/helpers/emails.py b/cfl_common/common/helpers/emails.py
index 8d1b5ef9f..88f0611b4 100644
--- a/cfl_common/common/helpers/emails.py
+++ b/cfl_common/common/helpers/emails.py
@@ -6,14 +6,11 @@
import jwt
from common import app_settings
-from common.app_settings import domain
-from common.mail import campaign_ids, send_dotdigital_email
+from common.mail import campaign_ids, django_send_email, send_dotdigital_email
from common.models import Student, Teacher
from django.conf import settings
from django.contrib.auth.models import User
-from django.core.mail import EmailMultiAlternatives
from django.http import HttpResponse
-from django.template import loader
from django.urls import reverse
from django.utils import timezone
from requests import delete, get, post, put
@@ -31,41 +28,6 @@ class DotmailerUserType(Enum):
NO_ACCOUNT = auto()
-def send_email(
- sender,
- recipients,
- subject,
- text_content,
- title,
- replace_url=None,
- plaintext_template="email.txt",
- html_template="email.html",
-):
- # add in template for templates to message
-
- # setup templates
- plaintext = loader.get_template(plaintext_template)
- html = loader.get_template(html_template)
- plaintext_email_context = {"content": text_content}
- html_email_context = {"content": text_content, "title": title, "url_prefix": domain()}
-
- # render templates
- plaintext_body = plaintext.render(plaintext_email_context)
- original_html_body = html.render(html_email_context)
- html_body = original_html_body
-
- if replace_url:
- verify_url = replace_url["verify_url"]
- verify_replace_url = re.sub(f"(.*/verify_email/)(.*)", f"\\1", verify_url)
- html_body = re.sub(f"({verify_url})(.*){verify_url}", f"\\1\\2{verify_replace_url}", original_html_body)
-
- # make message using templates
- message = EmailMultiAlternatives(subject, plaintext_body, sender, recipients)
- message.attach_alternative(html_body, "text/html")
-
- message.send()
-
-
def generate_token(user, new_email="", preverified=False):
if preverified:
user.userprofile.is_verified = preverified
@@ -91,6 +53,19 @@ def _newsletter_ticked(data):
return "newsletter_ticked" in data and data["newsletter_ticked"]
+def send_email(
+ sender,
+ recipients,
+ subject,
+ text_content,
+ title,
+ replace_url=None,
+ plaintext_template="email.txt",
+ html_template="email.html",
+):
+ django_send_email(sender, recipients, subject, text_content, title, replace_url, plaintext_template, html_template)
+
+
def send_verification_email(request, user, data, new_email=None, age=None):
"""
Sends emails relating to email address verification.
diff --git a/cfl_common/common/mail.py b/cfl_common/common/mail.py
index 06f0ffe48..751c8c572 100644
--- a/cfl_common/common/mail.py
+++ b/cfl_common/common/mail.py
@@ -3,8 +3,14 @@
import requests
from common import app_settings
+from common.app_settings import MODULE_NAME, domain
+from django.core.mail import EmailMultiAlternatives
+from django.template import loader
campaign_ids = {
+ "delete_account": 1567477,
+ "admin_given": 1569057,
+ "admin_revoked": 1569071,
"delete_account": 1567477,
"email_change_notification": 1551600,
"email_change_verification": 1551594,
@@ -35,6 +41,41 @@ class EmailAttachment:
content: str
+def django_send_email(
+ sender,
+ recipients,
+ subject,
+ text_content,
+ title,
+ replace_url=None,
+ plaintext_template="email.txt",
+ html_template="email.html",
+):
+ # add in template for templates to message
+
+ # setup templates
+ plaintext = loader.get_template(plaintext_template)
+ html = loader.get_template(html_template)
+ plaintext_email_context = {"content": text_content}
+ html_email_context = {"content": text_content, "title": title, "url_prefix": domain()}
+
+ # render templates
+ plaintext_body = plaintext.render(plaintext_email_context)
+ original_html_body = html.render(html_email_context)
+ html_body = original_html_body
+
+ if replace_url:
+ verify_url = replace_url["verify_url"]
+ verify_replace_url = re.sub(f"(.*/verify_email/)(.*)", f"\\1", verify_url)
+ html_body = re.sub(f"({verify_url})(.*){verify_url}", f"\\1\\2{verify_replace_url}", original_html_body)
+
+ # make message using templates
+ message = EmailMultiAlternatives(subject, plaintext_body, sender, recipients)
+ message.attach_alternative(html_body, "text/html")
+
+ message.send()
+
+
# pylint: disable-next=too-many-arguments
def send_dotdigital_email(
campaign_id: int,
@@ -72,47 +113,51 @@ def send_dotdigital_email(
"""
# pylint: enable=line-too-long
- if auth is None:
- auth = app_settings.DOTDIGITAL_AUTH
-
- body = {
- "campaignId": campaign_id,
- "toAddresses": to_addresses,
- }
- if cc_addresses is not None:
- body["ccAddresses"] = cc_addresses
- if bcc_addresses is not None:
- body["bccAddresses"] = bcc_addresses
- if from_address is not None:
- body["fromAddress"] = from_address
- if personalization_values is not None:
- body["personalizationValues"] = [
- {
- "name": key,
- "value": value,
- }
- for key, value in personalization_values.items()
- ]
- if metadata is not None:
- body["metadata"] = metadata
- if attachments is not None:
- body["attachments"] = [
- {
- "fileName": attachment.file_name,
- "mimeType": attachment.mime_type,
- "content": attachment.content,
- }
- for attachment in attachments
- ]
-
- response = requests.post(
- url=f"https://{region}-api.dotdigital.com/v2/email/triggered-campaign",
- json=body,
- headers={
- "accept": "text/plain",
- "authorization": auth,
- },
- timeout=timeout,
- )
-
- assert response.ok, "Failed to send email." f" Reason: {response.reason}." f" Text: {response.text}."
+ # Dotdigital emails don't work locally, so if testing emails locally use Django to send a dummy email instead
+ if MODULE_NAME == "local":
+ django_send_email(from_address, to_addresses, "dummy_subject", "dummy_text_content", "dummy_title")
+ else:
+ if auth is None:
+ auth = app_settings.DOTDIGITAL_AUTH
+
+ body = {
+ "campaignId": campaign_id,
+ "toAddresses": to_addresses,
+ }
+ if cc_addresses is not None:
+ body["ccAddresses"] = cc_addresses
+ if bcc_addresses is not None:
+ body["bccAddresses"] = bcc_addresses
+ if from_address is not None:
+ body["fromAddress"] = from_address
+ if personalization_values is not None:
+ body["personalizationValues"] = [
+ {
+ "name": key,
+ "value": value,
+ }
+ for key, value in personalization_values.items()
+ ]
+ if metadata is not None:
+ body["metadata"] = metadata
+ if attachments is not None:
+ body["attachments"] = [
+ {
+ "fileName": attachment.file_name,
+ "mimeType": attachment.mime_type,
+ "content": attachment.content,
+ }
+ for attachment in attachments
+ ]
+
+ response = requests.post(
+ url=f"https://{region}-api.dotdigital.com/v2/email/triggered-campaign",
+ json=body,
+ headers={
+ "accept": "text/plain",
+ "authorization": auth,
+ },
+ timeout=timeout,
+ )
+
+ assert response.ok, "Failed to send email." f" Reason: {response.reason}." f" Text: {response.text}."
diff --git a/cfl_common/common/migrations/0049_anonymise_orphan_users.py b/cfl_common/common/migrations/0049_anonymise_orphan_users.py
new file mode 100644
index 000000000..e2435d0db
--- /dev/null
+++ b/cfl_common/common/migrations/0049_anonymise_orphan_users.py
@@ -0,0 +1,29 @@
+from django.apps.registry import Apps
+from django.db import migrations
+
+from portal.views.api import __anonymise_user
+
+
+def anonymise_orphan_users(apps: Apps, *args):
+ """
+ Users should never exist without a user-type linked to them. Anonymise all
+ instances of User objects without a Teacher or Student instance.
+ """
+ User = apps.get_model("auth", "User")
+
+ active_orphan_users = User.objects.filter(
+ new_teacher__isnull=True, new_student__isnull=True, is_active=True
+ )
+
+ for active_orphan_user in active_orphan_users:
+ __anonymise_user(active_orphan_user)
+
+
+class Migration(migrations.Migration):
+ dependencies = [("common", "0048_unique_school_names")]
+
+ operations = [
+ migrations.RunPython(
+ code=anonymise_orphan_users, reverse_code=migrations.RunPython.noop
+ ),
+ ]
diff --git a/cfl_common/common/migrations/0050_anonymise_orphan_schools.py b/cfl_common/common/migrations/0050_anonymise_orphan_schools.py
new file mode 100644
index 000000000..d677cf5ea
--- /dev/null
+++ b/cfl_common/common/migrations/0050_anonymise_orphan_schools.py
@@ -0,0 +1,30 @@
+from uuid import uuid4
+
+from django.apps.registry import Apps
+from django.db import migrations
+
+
+def anonymise_orphan_schools(apps: Apps, *args):
+ """
+ Schools without any teachers or students should be anonymised (inactive).
+ Mark all active orphan schools as inactive.
+ """
+ School = apps.get_model("common", "School")
+
+ active_orphan_schools = School.objects.filter(teacher_school__isnull=True)
+
+ for active_orphan_school in active_orphan_schools:
+ active_orphan_school.name = uuid4().hex
+ active_orphan_school.is_active = False
+ active_orphan_school.save()
+
+
+class Migration(migrations.Migration):
+ dependencies = [("common", "0049_anonymise_orphan_users")]
+
+ operations = [
+ migrations.RunPython(
+ code=anonymise_orphan_schools,
+ reverse_code=migrations.RunPython.noop,
+ ),
+ ]
diff --git a/cfl_common/common/migrations/0051_verify_returning_users.py b/cfl_common/common/migrations/0051_verify_returning_users.py
new file mode 100644
index 000000000..4944ae50c
--- /dev/null
+++ b/cfl_common/common/migrations/0051_verify_returning_users.py
@@ -0,0 +1,30 @@
+from django.apps.registry import Apps
+from django.db import migrations
+
+
+def verify_returning_users(apps: Apps, *args):
+ """
+ Users cannot be unverified after having logged in at least once. Grab all
+ instances of unverified UserProfile where the User has logged in and mark it
+ as verified.
+ """
+ UserProfile = apps.get_model("common", "UserProfile")
+
+ unverified_returning_userprofiles = UserProfile.objects.filter(
+ user__last_login__isnull=False, is_verified=False
+ )
+
+ for unverified_returning_userprofile in unverified_returning_userprofiles:
+ unverified_returning_userprofile.is_verified = True
+ unverified_returning_userprofile.save()
+
+
+class Migration(migrations.Migration):
+ dependencies = [("common", "0050_anonymise_orphan_schools")]
+
+ operations = [
+ migrations.RunPython(
+ code=verify_returning_users,
+ reverse_code=migrations.RunPython.noop,
+ ),
+ ]
diff --git a/cfl_common/common/models.py b/cfl_common/common/models.py
index 9bd198f90..871fac32b 100644
--- a/cfl_common/common/models.py
+++ b/cfl_common/common/models.py
@@ -2,7 +2,6 @@
from datetime import timedelta
from uuid import uuid4
-import pgeocode
from django.contrib.auth.models import User
from django.db import models
from django.utils import timezone
diff --git a/cfl_common/common/tests/test_migration_anonymise_orphan_schools.py b/cfl_common/common/tests/test_migration_anonymise_orphan_schools.py
new file mode 100644
index 000000000..f30d354c8
--- /dev/null
+++ b/cfl_common/common/tests/test_migration_anonymise_orphan_schools.py
@@ -0,0 +1,30 @@
+import pytest
+from django_test_migrations.migrator import Migrator
+
+
+@pytest.mark.django_db
+def test_migration_anonymise_orphan_schools(migrator: Migrator):
+ state = migrator.apply_initial_migration(
+ ("common", "0049_anonymise_orphan_users")
+ )
+ User = state.apps.get_model("auth", "User")
+ UserProfile = state.apps.get_model("common", "UserProfile")
+ Teacher = state.apps.get_model("common", "Teacher")
+ School = state.apps.get_model("common", "School")
+
+ orphan_school = School.objects.create(name="OrphanSchool")
+ teacher_school = School.objects.create(name="TeacherSchool")
+
+ teacher_user = User.objects.create_user("TeacherUser", password="password")
+ teacher_userprofile = UserProfile.objects.create(user=teacher_user)
+ Teacher.objects.create(
+ user=teacher_userprofile, new_user=teacher_user, school=teacher_school
+ )
+
+ migrator.apply_tested_migration(("common", "0050_anonymise_orphan_schools"))
+
+ def assert_school_anonymised(pk: int, anonymised: bool):
+ assert School.objects.get(pk=pk).is_active != anonymised
+
+ assert_school_anonymised(orphan_school.pk, True)
+ assert_school_anonymised(teacher_school.pk, False)
diff --git a/cfl_common/common/tests/test_migration_anonymise_orphan_users.py b/cfl_common/common/tests/test_migration_anonymise_orphan_users.py
new file mode 100644
index 000000000..4c52c4065
--- /dev/null
+++ b/cfl_common/common/tests/test_migration_anonymise_orphan_users.py
@@ -0,0 +1,30 @@
+import pytest
+from django_test_migrations.migrator import Migrator
+
+
+@pytest.mark.django_db
+def test_migration_anonymise_orphan_users(migrator: Migrator):
+ state = migrator.apply_initial_migration(
+ ("common", "0048_unique_school_names")
+ )
+ User = state.apps.get_model("auth", "User")
+ UserProfile = state.apps.get_model("common", "UserProfile")
+ Teacher = state.apps.get_model("common", "Teacher")
+ Student = state.apps.get_model("common", "Student")
+
+ orphan_user = User.objects.create_user("OrphanUser", password="password")
+ teacher_user = User.objects.create_user("TeacherUser", password="password")
+ student_user = User.objects.create_user("StudentUser", password="password")
+ teacher_userprofile = UserProfile.objects.create(user=teacher_user)
+ student_userprofile = UserProfile.objects.create(user=student_user)
+ Teacher.objects.create(user=teacher_userprofile, new_user=teacher_user)
+ Student.objects.create(user=student_userprofile, new_user=student_user)
+
+ migrator.apply_tested_migration(("common", "0049_anonymise_orphan_users"))
+
+ def assert_user_anonymised(pk: int, anonymised: bool):
+ assert User.objects.get(pk=pk).is_active != anonymised
+
+ assert_user_anonymised(orphan_user.pk, True)
+ assert_user_anonymised(teacher_user.pk, False)
+ assert_user_anonymised(student_user.pk, False)
diff --git a/cfl_common/common/tests/test_0048_unique_school_names.py b/cfl_common/common/tests/test_migration_unique_school_names.py
similarity index 74%
rename from cfl_common/common/tests/test_0048_unique_school_names.py
rename to cfl_common/common/tests/test_migration_unique_school_names.py
index 1ad02f502..2e5ee144c 100644
--- a/cfl_common/common/tests/test_0048_unique_school_names.py
+++ b/cfl_common/common/tests/test_migration_unique_school_names.py
@@ -3,8 +3,10 @@
@pytest.mark.django_db
-def test_0048_unique_school_names(migrator: Migrator):
- state = migrator.apply_initial_migration(("common", "0047_delete_school_postcode"))
+def test_migration_unique_school_names(migrator: Migrator):
+ state = migrator.apply_initial_migration(
+ ("common", "0047_delete_school_postcode")
+ )
School = state.apps.get_model("common", "School")
school_name = "ExampleSchool"
@@ -15,7 +17,9 @@ def test_0048_unique_school_names(migrator: Migrator):
School(name=f"{school_name} 1"),
]
)
- school_ids = list(School.objects.order_by("-id")[:3].values_list("id", flat=True))
+ school_ids = list(
+ School.objects.order_by("-id")[:3].values_list("id", flat=True)
+ )
school_ids.reverse()
migrator.apply_tested_migration(("common", "0048_unique_school_names"))
diff --git a/cfl_common/common/tests/test_migration_verify_returning_users.py b/cfl_common/common/tests/test_migration_verify_returning_users.py
new file mode 100644
index 000000000..f10919228
--- /dev/null
+++ b/cfl_common/common/tests/test_migration_verify_returning_users.py
@@ -0,0 +1,59 @@
+from datetime import datetime, timezone
+
+import pytest
+from django_test_migrations.migrator import Migrator
+
+from portal.views.api import __anonymise_user
+
+
+@pytest.mark.django_db
+def test_migration_verify_returning_users(migrator: Migrator):
+ state = migrator.apply_initial_migration(
+ ("common", "0050_anonymise_orphan_schools")
+ )
+ User = state.apps.get_model("auth", "User")
+ UserProfile = state.apps.get_model("common", "UserProfile")
+
+ returning_user = User.objects.create_user(
+ "ReturningUser",
+ password="password",
+ last_login=datetime.now(tz=timezone.utc),
+ )
+ returning_userprofile = UserProfile.objects.create(user=returning_user)
+
+ non_returning_user = User.objects.create_user(
+ "NonReturningUser", password="password"
+ )
+ non_returning_userprofile = UserProfile.objects.create(
+ user=non_returning_user
+ )
+
+ anonymised_returning_user = User.objects.create_user(
+ "AnonReturningUser",
+ password="password",
+ last_login=datetime.now(tz=timezone.utc),
+ )
+ anonymised_returning_userprofile = UserProfile.objects.create(
+ user=anonymised_returning_user
+ )
+ __anonymise_user(anonymised_returning_user)
+
+ anonymised_non_returning_user = User.objects.create_user(
+ "AnonNonReturningUser", password="password"
+ )
+ anonymised_non_returning_userprofile = UserProfile.objects.create(
+ user=anonymised_non_returning_user
+ )
+ __anonymise_user(anonymised_non_returning_user)
+
+ migrator.apply_tested_migration(("common", "0051_verify_returning_users"))
+
+ def assert_userprofile_is_verified(pk: int, verified: bool):
+ assert UserProfile.objects.get(pk=pk).is_verified == verified
+
+ assert_userprofile_is_verified(returning_userprofile.pk, True)
+ assert_userprofile_is_verified(non_returning_userprofile.pk, False)
+ assert_userprofile_is_verified(anonymised_returning_userprofile.pk, True)
+ assert_userprofile_is_verified(
+ anonymised_non_returning_userprofile.pk, False
+ )
diff --git a/portal/__init__.py b/portal/__init__.py
index 3d0e834b2..cf3c09654 100644
--- a/portal/__init__.py
+++ b/portal/__init__.py
@@ -1 +1 @@
-__version__ = "6.42.0"
+__version__ = "6.43.1"
diff --git a/portal/tests/test_api.py b/portal/tests/test_api.py
index 88ebf0af7..902cae354 100644
--- a/portal/tests/test_api.py
+++ b/portal/tests/test_api.py
@@ -5,7 +5,10 @@
import pytest
from common.models import Class, School, Student, 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.organisation import (
+ 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 common.tests.utils.user import create_user_directly, get_superuser
@@ -19,7 +22,10 @@
class APITests(APITestCase):
def test_valid_date_registered(self):
- url = reverse("registered-users", kwargs={"year": "2016", "month": "04", "day": "01"})
+ url = reverse(
+ "registered-users",
+ kwargs={"year": "2016", "month": "04", "day": "01"},
+ )
superuser = get_superuser()
self.client.force_authenticate(user=superuser)
response = self.client.get(url)
@@ -27,14 +33,20 @@ def test_valid_date_registered(self):
assert_that(isinstance(response.data, int))
def test_invalid_date_registered(self):
- url = reverse("registered-users", kwargs={"year": "2016", "month": "05", "day": "35"})
+ url = reverse(
+ "registered-users",
+ kwargs={"year": "2016", "month": "05", "day": "35"},
+ )
superuser = get_superuser()
self.client.force_authenticate(user=superuser)
response = self.client.get(url)
assert_that(response, has_status_code(status.HTTP_404_NOT_FOUND))
def test_valid_date_lastconnectedsince(self):
- url = reverse("last-connected-since", kwargs={"year": "2016", "month": "04", "day": "01"})
+ url = reverse(
+ "last-connected-since",
+ kwargs={"year": "2016", "month": "04", "day": "01"},
+ )
superuser = get_superuser()
self.client.force_authenticate(user=superuser)
response = self.client.get(url)
@@ -42,7 +54,10 @@ def test_valid_date_lastconnectedsince(self):
assert_that(isinstance(response.data, int))
def test_invalid_date_lastconnectedsince(self):
- url = reverse("last-connected-since", kwargs={"year": "2016", "month": "05", "day": "35"})
+ url = reverse(
+ "last-connected-since",
+ kwargs={"year": "2016", "month": "05", "day": "35"},
+ )
superuser = get_superuser()
self.client.force_authenticate(user=superuser)
response = self.client.get(url)
@@ -67,7 +82,9 @@ def test_get_inactive_users_if_admin(self):
assert len(response.data) == 1
@patch("portal.views.api.IS_CLOUD_SCHEDULER_FUNCTION", return_value=True)
- def test_get_inactive_users_if_appengine(self, mock_is_cloud_scheduler_function):
+ def test_get_inactive_users_if_appengine(
+ self, mock_is_cloud_scheduler_function
+ ):
client = APIClient()
create_user_directly(active=False)
create_user_directly(active=True)
@@ -86,7 +103,9 @@ def test_get_inactive_users_if_unauthorised(self):
assert response.status_code == status.HTTP_403_FORBIDDEN
@patch("portal.views.api.IS_CLOUD_SCHEDULER_FUNCTION", return_value=True)
- def test_delete_inactive_users_if_appengine(self, mock_is_cloud_scheduler_function):
+ def test_delete_inactive_users_if_appengine(
+ self, mock_is_cloud_scheduler_function
+ ):
client = APIClient()
create_user_directly(active=False)
create_user_directly(active=False)
@@ -94,14 +113,25 @@ def test_delete_inactive_users_if_appengine(self, mock_is_cloud_scheduler_functi
response = client.get(url)
users = response.data
assert len(users) == 2
+
+ # NOTE: Migration 0049 causes user 34 (created via migration 0001) to
+ # be marked as inactive. Slightly tweaked this test so it still
+ # passes but takes into account this new anonymisation.
+ old_deleted_users = list(User.objects.filter(is_active=False))
+ assert len(old_deleted_users) == 1
+
response = client.delete(url)
assert mock_is_cloud_scheduler_function.called
assert response.status_code == status.HTTP_204_NO_CONTENT
+
for user in users:
with pytest.raises(User.DoesNotExist):
User.objects.get(username=user["username"])
+
deleted_users = list(User.objects.filter(is_active=False))
- assert len(deleted_users) == 2
+ new_deleted_users_count = len(deleted_users) - len(old_deleted_users)
+ assert new_deleted_users_count == 2
+
for user in deleted_users:
assert user.first_name == "Deleted"
assert user.last_name == "User"
@@ -112,27 +142,41 @@ def test_delete_inactive_users_if_appengine(self, mock_is_cloud_scheduler_functi
assert len(response.data) == 0
@patch("portal.views.api.IS_CLOUD_SCHEDULER_FUNCTION", return_value=True)
- def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler_function):
+ def test_orphan_schools_and_classes_are_anonymised(
+ self, mock_is_cloud_scheduler_function
+ ):
client = APIClient()
# Create a school with an active teacher
school1_teacher1_email, _ = signup_teacher_directly()
school1 = create_organisation_directly(school1_teacher1_email)
- klass11, _, access_code11 = create_class_directly(school1_teacher1_email)
+ klass11, _, access_code11 = create_class_directly(
+ school1_teacher1_email
+ )
_, _, student11 = create_school_student_directly(access_code11)
# Create a school with one active non-admin teacher and one inactive admin teacher
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, is_admin=True)
- klass21, _, access_code21 = create_class_directly(school2_teacher1_email)
+ 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)
+ klass22, _, access_code22 = create_class_directly(
+ school2_teacher2_email
+ )
_, _, student22 = create_school_student_directly(access_code22)
- school2_teacher1 = Teacher.objects.get(new_user__email=school2_teacher1_email)
+ school2_teacher1 = Teacher.objects.get(
+ new_user__email=school2_teacher1_email
+ )
school2_teacher1.is_admin = False
school2_teacher1.save()
- school2_teacher2 = Teacher.objects.get(new_user__email=school2_teacher2_email)
+ school2_teacher2 = Teacher.objects.get(
+ new_user__email=school2_teacher2_email
+ )
school2_teacher2.new_user.is_active = False
school2_teacher2.new_user.save()
@@ -141,28 +185,40 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler
school3_teacher2_email, _ = signup_teacher_directly()
school3 = create_organisation_directly(school3_teacher1_email)
join_teacher_to_organisation(school3_teacher2_email, school3.name)
- klass31, _, access_code31 = create_class_directly(school3_teacher1_email)
+ 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)
+ klass32, _, access_code32 = create_class_directly(
+ school3_teacher2_email
+ )
_, _, student32 = create_school_student_directly(access_code32)
- school3_teacher1 = Teacher.objects.get(new_user__email=school3_teacher1_email)
+ school3_teacher1 = Teacher.objects.get(
+ new_user__email=school3_teacher1_email
+ )
school3_teacher1.new_user.is_active = False
school3_teacher1.new_user.save()
- school3_teacher2 = Teacher.objects.get(new_user__email=school3_teacher2_email)
+ school3_teacher2 = Teacher.objects.get(
+ new_user__email=school3_teacher2_email
+ )
school3_teacher2.new_user.is_active = False
school3_teacher2.new_user.save()
# Create a school with no active teachers
school4_teacher1_email, _ = signup_teacher_directly()
school4 = create_organisation_directly(school4_teacher1_email)
- school4_teacher1 = Teacher.objects.get(new_user__email=school4_teacher1_email)
+ school4_teacher1 = Teacher.objects.get(
+ new_user__email=school4_teacher1_email
+ )
school4_teacher1.new_user.is_active = False
school4_teacher1.new_user.save()
# Create a school with no teachers
school5_teacher1_email, _ = signup_teacher_directly()
school5 = create_organisation_directly(school5_teacher1_email)
- school5_teacher1 = Teacher.objects.get(new_user__email=school5_teacher1_email)
+ school5_teacher1 = Teacher.objects.get(
+ new_user__email=school5_teacher1_email
+ )
school5_teacher1.delete()
# Call the API
@@ -182,7 +238,9 @@ def test_orphan_schools_and_classes_are_anonymised(self, mock_is_cloud_scheduler
assert Student.objects.filter(pk=student21.pk).exists()
assert not Student.objects.get(pk=student22.pk).new_user.is_active
# Also check the first teacher is now an admin
- assert Teacher.objects.get(new_user__email=school2_teacher1_email).is_admin
+ 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).exists()
@@ -250,14 +308,19 @@ def test_remove_fake_accounts(self):
last_name=random_account["last_name"],
)
- assert len(User.objects.all()) == len(random_accounts) + initial_users_length
+ assert (
+ len(User.objects.all())
+ == len(random_accounts) + initial_users_length
+ )
client.login(username=admin_username, password=admin_password)
response = client.get(reverse("remove_fake_accounts"))
assert response.status_code == 204
# check if after deletion all the users are still there
- assert len(User.objects.all()) == initial_users_length + 2 # mentioned in the fake_accounts description
+ assert (
+ len(User.objects.all()) == initial_users_length + 2
+ ) # mentioned in the fake_accounts description
def has_status_code(status_code):
@@ -272,7 +335,11 @@ def _matches(self, response):
return response.status_code == self.status_code
def describe_to(self, description):
- description.append_text("has status code ").append_text(self.status_code)
+ description.append_text("has status code ").append_text(
+ self.status_code
+ )
def describe_mismatch(self, response, mismatch_description):
- mismatch_description.append_text("had status code ").append_text(response.status_code)
+ mismatch_description.append_text("had status code ").append_text(
+ response.status_code
+ )
diff --git a/portal/tests/test_organisation.py b/portal/tests/test_organisation.py
index c4512640d..bf93df780 100644
--- a/portal/tests/test_organisation.py
+++ b/portal/tests/test_organisation.py
@@ -4,9 +4,11 @@
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
diff --git a/portal/views/login/student.py b/portal/views/login/student.py
index 02d582af6..92fda8829 100644
--- a/portal/views/login/student.py
+++ b/portal/views/login/student.py
@@ -51,7 +51,8 @@ def _add_logged_in_as_message(self, request):
class_name = self.kwargs["access_code"].upper()
messages.info(
request,
- f"You are logged in to class: " f"{escape(class_name)}",
+ f"You are logged in to class: "
+ f"{escape(class_name)}",
extra_tags="safe message--student",
)
@@ -72,7 +73,9 @@ def _add_login_data(self, form, login_type):
klass = classes[0]
name = form.cleaned_data.get("username")
- students = Student.objects.filter(new_user__first_name__iexact=name, class_field=klass)
+ students = Student.objects.filter(
+ new_user__first_name__iexact=name, class_field=klass
+ )
try:
student = students[0]
except IndexError:
@@ -81,32 +84,44 @@ def _add_login_data(self, form, login_type):
raise Exception(msg)
# Log the login time, class, and login type
- session = UserSession(user=student.new_user, class_field=klass, login_type=login_type)
+ session = UserSession(
+ user=student.new_user, class_field=klass, login_type=login_type
+ )
session.save()
+ student.user.is_verified = True
+ student.user.save()
+
def form_valid(self, form):
"""Security check complete. Log the user in."""
-
# Reset ratelimit cache upon successful login
clear_ratelimit_cache_for_user(form.cleaned_data["username"])
- login_type = self.kwargs.get("login_type", "classlink") # default to "classlink" if not specified
+ login_type = self.kwargs.get(
+ "login_type", "classlink"
+ ) # default to "classlink" if not specified
self._add_login_data(form, login_type)
return super(StudentLoginView, self).form_valid(form)
def post(self, request, *args, **kwargs):
"""
- If the first name and access code found under the url inputted in the form corresponds to that of a blocked
- account, this redirects the user to the locked out page. However, if the lockout
- time is more than 24 hours before this is executed, the account is unlocked.
+ If the first name and access code found under the url inputted in the
+ form corresponds to that of a blocked account, this redirects the user
+ to the locked out page. However, if the lockout time is more than 24
+ hours before this is executed, the account is unlocked.
"""
username = request.POST.get("username")
# get access code from the current url
access_code = get_access_code_from_request(request)
- if Student.objects.filter(new_user__first_name=username, class_field__access_code=access_code).exists():
- student = Student.objects.get(new_user__first_name=username, class_field__access_code=access_code)
+ if Student.objects.filter(
+ new_user__first_name=username, class_field__access_code=access_code
+ ).exists():
+ student = Student.objects.get(
+ new_user__first_name=username,
+ class_field__access_code=access_code,
+ )
if student.blocked_time is not None:
if has_user_lockout_expired(student):
@@ -129,9 +144,15 @@ def student_direct_login(request, user_id, login_id):
if user:
# Log the login time and class
student = Student.objects.get(new_user=user)
- session = UserSession(user=user, class_field=student.class_field, login_type="direct")
+ session = UserSession(
+ user=user, class_field=student.class_field, login_type="direct"
+ )
session.save()
login(request, user)
+
+ student.user.is_verified = True
+ student.user.save()
+
return HttpResponseRedirect(reverse_lazy("student_details"))
return HttpResponseRedirect(reverse_lazy("home"))
diff --git a/portal/views/teacher/dashboard.py b/portal/views/teacher/dashboard.py
index 9dc4d84fa..3ae649e74 100644
--- a/portal/views/teacher/dashboard.py
+++ b/portal/views/teacher/dashboard.py
@@ -2,12 +2,24 @@
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.mail import campaign_ids, send_dotdigital_email
+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
@@ -16,7 +28,7 @@
from django.contrib.auth.models import User
from django.http import Http404, HttpResponseRedirect
from django.shortcuts import get_object_or_404, render
-from django.urls import reverse_lazy
+from django.urls import reverse, reverse_lazy
from django.utils import timezone
from django.views.decorators.http import require_POST
from game.level_management import levels_shared_with, unshare_level
@@ -25,14 +37,20 @@
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
@@ -380,19 +398,22 @@ def invite_toggle_admin(request, invite_id):
if invite.invited_teacher_is_admin:
messages.success(request, "Administrator invite status has been given successfully")
- emailMessage = email_messages.adminGivenEmail(request, invite.school)
+ send_dotdigital_email(
+ campaign_ids["admin_given"],
+ [invite.invited_teacher_email],
+ personalization_values={
+ "SCHOOL_CLUB_NAME": invite.school,
+ "MANAGEMENT_LINK": request.build_absolute_uri(reverse("dashboard")),
+ },
+ )
else:
messages.success(request, "Administrator invite status has been revoked successfully")
- emailMessage = email_messages.adminRevokedEmail(request, invite.school)
-
- send_email(
- NOTIFICATION_EMAIL,
- [invite.invited_teacher_email],
- emailMessage["subject"],
- emailMessage["message"],
- emailMessage["subject"],
- )
+ send_dotdigital_email(
+ campaign_ids["admin_revoked"],
+ [invite.invited_teacher_email],
+ personalization_values={"SCHOOL_CLUB_NAME": invite.school},
+ )
return HttpResponseRedirect(reverse_lazy("dashboard"))
@@ -411,7 +432,14 @@ def organisation_toggle_admin(request, pk):
if teacher.is_admin:
messages.success(request, "Administrator status has been given successfully.")
- email_message = email_messages.adminGivenEmail(request, teacher.school.name)
+ send_dotdigital_email(
+ campaign_ids["admin_given"],
+ [teacher.new_user.email],
+ personalization_values={
+ "SCHOOL_CLUB_NAME": teacher.school.name,
+ "MANAGEMENT_LINK": request.build_absolute_uri(reverse("dashboard")),
+ },
+ )
else:
# Remove access to all levels that are from other teachers' students
[
@@ -420,15 +448,11 @@ def organisation_toggle_admin(request, pk):
if hasattr(level.owner, "student") and not teacher.teaches(level.owner)
]
messages.success(request, "Administrator status has been revoked successfully.")
- email_message = email_messages.adminRevokedEmail(request, teacher.school.name)
-
- send_email(
- NOTIFICATION_EMAIL,
- [teacher.new_user.email],
- email_message["subject"],
- email_message["message"],
- email_message["subject"],
- )
+ send_dotdigital_email(
+ campaign_ids["admin_revoked"],
+ [teacher.new_user.email],
+ personalization_values={"SCHOOL_CLUB_NAME": teacher.school.name},
+ )
return HttpResponseRedirect(reverse_lazy("dashboard"))