fix: move admin given/revoked emails to dotdigital (#2292)

* move admin given/revoked emails to dotdigital * correct teacher email * fix tests * address PR comment * Merge branch 'master' into move-admin-given-and-revoked-emails * Try codecov quick fix * Ubuntu latest everywhere * Add project to codecov yaml Co-Authored-By: faucomte97 <[email protected]>
ocadotechnology · May 2, 2024 · c9fddfb · c9fddfb
1 parent dcbaa2f
commit c9fddfb
Show file tree

Hide file tree

Showing 10 changed files with 178 additions and 147 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -3,8 +3,13 @@
 
 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 = {
+    "admin_given": 1569057,
+    "admin_revoked": 1569071,
     "email_change_notification": 1551600,
     "email_change_verification": 1551594,
     "reset_password": 1557153,
@@ -34,6 +39,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,
@@ -71,47 +111,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/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