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 776d60316..f5bc9a1f8 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 b786b830a..f4412ae93 100644 --- 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 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/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"))