Merge branch 'master' into move-account-deletion-email

.codecov.yml

@@ -6,6 +6,9 @@ coverage:
     patch:
       default:
         target: 90%
+    project:
+      default:
+        target: 90%
 
 ignore:
   - "portal/tests/*.py"

.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 }}

.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

.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:

.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:

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",

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.

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}."

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
+        ),
+    ]

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,
+        ),
+    ]