diff --git a/cfl_common/common/helpers/emails.py b/cfl_common/common/helpers/emails.py index 88f0611b4..ebead2d8a 100644 --- a/cfl_common/common/helpers/emails.py +++ b/cfl_common/common/helpers/emails.py @@ -1,6 +1,5 @@ import datetime import json -import re from enum import Enum, auto from uuid import uuid4 @@ -66,13 +65,14 @@ def send_email( 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): +def send_verification_email(request, user, data, new_email=None, age=None, school=None): """ Sends emails relating to email address verification. On registration: - if the user is under 13, send a verification email addressed to the parent / guardian - if the user is over 13, send a regular verification email + - if the user is a student who just got released, send a verification email explaining the situation - if the user is a student who has requested to sign up to the newsletter, handle their Dotmailer subscription On email address update: @@ -92,16 +92,25 @@ def send_verification_email(request, user, data, new_email=None, age=None): if not new_email: verification = generate_token(user) - # if the user is a teacher if age is None: - url = f"{request.build_absolute_uri(reverse('verify_email', kwargs={'token': verification}))}" + # if the user is a released student + if hasattr(user, "new_student"): + url = f"{request.build_absolute_uri(reverse('verify_email', kwargs={'token': verification}))}" + + send_dotdigital_email( + campaign_ids["verify_released_student"], [user.email], + personalization_values={"VERIFICATION_LINK": url, "SCHOOL_NAME": school.name} + ) + # if the user is a teacher + else: + url = f"{request.build_absolute_uri(reverse('verify_email', kwargs={'token': verification}))}" - send_dotdigital_email( - campaign_ids["verify_new_user"], [user.email], personalization_values={"VERIFICATION_LINK": url} - ) + send_dotdigital_email( + campaign_ids["verify_new_user"], [user.email], personalization_values={"VERIFICATION_LINK": url} + ) - if _newsletter_ticked(data): - add_to_dotmailer(user.first_name, user.last_name, user.email, DotmailerUserType.TEACHER) + if _newsletter_ticked(data): + add_to_dotmailer(user.first_name, user.last_name, user.email, DotmailerUserType.TEACHER) # if the user is an independent student else: if age < 13: diff --git a/cfl_common/common/mail.py b/cfl_common/common/mail.py index e2a0d87dd..007eb7986 100644 --- a/cfl_common/common/mail.py +++ b/cfl_common/common/mail.py @@ -26,6 +26,7 @@ "verify_new_user_first_reminder": 1557170, "verify_new_user_second_reminder": 1557173, "verify_new_user_via_parent": 1551587, + "verify_released_student": 1580574, } diff --git a/portal/forms/play.py b/portal/forms/play.py index 6b3d56837..3ab2342b9 100644 --- a/portal/forms/play.py +++ b/portal/forms/play.py @@ -277,15 +277,19 @@ class StudentJoinOrganisationForm(forms.Form): def clean(self): access_code = self.cleaned_data.get("access_code", None) + join_error_text = "The class code you entered either does not exist or is not currently accepting join requests. Please double check that you have entered the correct class code and contact the teacher of the class to ensure their class is currently accepting join requests." if access_code: classes = Class.objects.filter(access_code=access_code) if len(classes) != 1: - raise forms.ValidationError("Cannot find the school or club and/or class") + raise forms.ValidationError(join_error_text) + self.klass = classes[0] - if not self.klass.always_accept_requests: - if self.klass.accept_requests_until is None: - raise forms.ValidationError("Cannot find the school or club and/or class") - elif (self.klass.accept_requests_until - timezone.now()) < timedelta(): - raise forms.ValidationError("Cannot find the school or club and/or class") + + if not self.klass.always_accept_requests and ( + self.klass.accept_requests_until is None + or self.klass.accept_requests_until - timezone.now() + < timedelta() + ): + raise forms.ValidationError(join_error_text) return self.cleaned_data diff --git a/portal/tests/test_independent_student.py b/portal/tests/test_independent_student.py index 0589fd809..e66d94076 100644 --- a/portal/tests/test_independent_student.py +++ b/portal/tests/test_independent_student.py @@ -471,7 +471,9 @@ def test_join_class_nonexistent_class(self): ) assert self.is_join_class_page(page) - assert page.has_join_request_failed("Cannot find the school or club and/or class") + assert page.has_join_request_failed( + "The class code you entered either does not exist or is not currently accepting join requests. Please double check that you have entered the correct class code and contact the teacher of the class to ensure their class is currently accepting join requests." + ) def test_join_class_not_accepting_requests(self): teacher_email, _ = signup_teacher_directly() @@ -490,7 +492,9 @@ def test_join_class_not_accepting_requests(self): ) assert self.is_join_class_page(page) - assert page.has_join_request_failed("Cannot find the school or club and/or class") + assert page.has_join_request_failed( + "The class code you entered either does not exist or is not currently accepting join requests. Please double check that you have entered the correct class code and contact the teacher of the class to ensure their class is currently accepting join requests." + ) def test_join_class_revoked(self): teacher_email, _ = signup_teacher_directly() diff --git a/portal/tests/test_views.py b/portal/tests/test_views.py index a2e30f86a..a512449f5 100644 --- a/portal/tests/test_views.py +++ b/portal/tests/test_views.py @@ -53,8 +53,11 @@ class TestTeacherViews(TestCase): @classmethod def setUpTestData(cls): cls.email, cls.password = signup_teacher_directly() + cls.school = create_organisation_directly(cls.email) _, _, cls.class_access_code = create_class_directly(cls.email) - _, _, cls.student = create_school_student_directly(cls.class_access_code) + _, cls.password_student, cls.student = create_school_student_directly( + cls.class_access_code + ) def login(self): c = Client() @@ -63,7 +66,9 @@ def login(self): def test_reminder_cards(self): c = self.login() - url = reverse("teacher_print_reminder_cards", args=[self.class_access_code]) + url = reverse( + "teacher_print_reminder_cards", args=[self.class_access_code] + ) # First test with 2 dummy students NAME1 = "Test name" @@ -97,7 +102,9 @@ def test_reminder_cards(self): # page number students_per_page = REMINDER_CARDS_PDF_ROWS * REMINDER_CARDS_PDF_COLUMNS for _ in range(len(studentlist), students_per_page + 1): - studentlist.append({"name": NAME1, "password": PASSWORD1, "login_url": URL}) + studentlist.append( + {"name": NAME1, "password": PASSWORD1, "login_url": URL} + ) assert len(studentlist) == students_per_page + 1 @@ -136,7 +143,9 @@ def test_csv(self): reader = csv.reader(io.StringIO(content)) access_code = self.class_access_code - class_url = reverse("student_login", kwargs={"access_code": access_code}) + class_url = reverse( + "student_login", kwargs={"access_code": access_code} + ) row0 = next(reader) assert row0[0].strip() == access_code assert class_url in row0[1].strip() @@ -175,7 +184,9 @@ def test_organisation_kick_has_correct_permissions(self): def test_daily_activity_student_details(self): c = self.login() - url = reverse("teacher_print_reminder_cards", args=[self.class_access_code]) + url = reverse( + "teacher_print_reminder_cards", args=[self.class_access_code] + ) data = { "data": json.dumps( @@ -221,6 +232,48 @@ def test_daily_activity_student_details(self): with pytest.raises(Exception): count_student_details_click("Wrong download method") + def test_release_verified_student(self): + c = Client() + student_login_url = reverse( + "student_login", args=[self.class_access_code] + ) + response = c.post( + student_login_url, + { + "username": self.student.new_user.first_name, + "password": self.password_student, + }, + ) + assert response.status_code == 302 + + student = Student.objects.get(pk=self.student.pk) + assert student.user.is_verified + + c.logout() + c.login(username=self.email, password=self.password) + + release_url = reverse( + "teacher_dismiss_students", args=[self.class_access_code] + ) + response = c.post( + release_url, + { + "form-TOTAL_FORMS": 1, + "form-INITIAL_FORMS": 1, + "form-MIN_NUM_FORMS": 0, + "form-MAX_NUM_FORMS": 1000, + "form-0-orig_name": self.student.new_user.first_name, + "form-0-name": self.student.new_user.first_name, + "form-0-email": "independent@gmail.com", + "form-0-confirm_email": "independent@gmail.com", + "submit_dismiss": "", + }, + ) + assert response.status_code == 302 + + student = Student.objects.get(pk=self.student.pk) + assert not student.user.is_verified + class TestLoginViews(TestCase): @classmethod @@ -238,7 +291,9 @@ def _set_up_test_data(self): teacher_email, teacher_password = signup_teacher_directly() create_organisation_directly(teacher_email) _, _, class_access_code = create_class_directly(teacher_email) - student_name, student_password, _ = create_school_student_directly(class_access_code) + student_name, student_password, _ = create_school_student_directly( + class_access_code + ) return ( teacher_email, @@ -271,9 +326,16 @@ def _create_and_login_school_student(self, next_url=False): _, _, name, password, class_access_code = self._set_up_test_data() if next_url: - url = reverse("student_login", kwargs={"access_code": class_access_code}) + "?next=/" + url = ( + reverse( + "student_login", kwargs={"access_code": class_access_code} + ) + + "?next=/" + ) else: - url = reverse("student_login", kwargs={"access_code": class_access_code}) + url = reverse( + "student_login", kwargs={"access_code": class_access_code} + ) c = Client() response = c.post(url, {"username": name, "password": password}) @@ -312,7 +374,9 @@ def test_teacher_session(self): def _get_user_class(self, name, class_access_code): klass = Class.objects.get(access_code=class_access_code) - 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 + ) assert len(students) == 1 user = students[0].new_user return user, klass @@ -354,7 +418,9 @@ def test_student_session_class_link(self): _, _, name, password, class_access_code = self._set_up_test_data() c = Client() - url = reverse("student_login", kwargs={"access_code": class_access_code}) + url = reverse( + "student_login", kwargs={"access_code": class_access_code} + ) c.post(url, {"username": name, "password": password}) # check if there's a UserSession data within the last 10 secs @@ -375,7 +441,9 @@ def test_student_login_failed(self): randomname = "randomname" c = Client() - url = reverse("student_login", kwargs={"access_code": class_access_code}) + url = reverse( + "student_login", kwargs={"access_code": class_access_code} + ) c.post(url, {"username": randomname, "password": "xx"}) # check if there's a UserSession data within the last 10 secs @@ -401,7 +469,9 @@ def test_indep_student_session(self): def test_student_direct_login(self): _, _, _, _, class_access_code = self._set_up_test_data() - student, login_id, _, _ = create_student_with_direct_login(class_access_code) + student, login_id, _, _ = create_student_with_direct_login( + class_access_code + ) c = Client() assert c.login(user_id=student.new_user.id, login_id=login_id) == True @@ -523,7 +593,9 @@ def test_student_dashboard_view(self): c = Client() # Login and check initial data - url = reverse("student_login", kwargs={"access_code": class_access_code}) + url = reverse( + "student_login", kwargs={"access_code": class_access_code} + ) c.post(url, {"username": student_name, "password": student_password}) student_dashboard_url = reverse("student_details") @@ -602,7 +674,9 @@ def test_delete_account(self, mock_send_dotdigital_email: Mock): # try again with the correct password url = reverse("delete_account") - response = c.post(url, {"password": password, "unsubscribe_newsletter": "on"}) + response = c.post( + url, {"password": password, "unsubscribe_newsletter": "on"} + ) assert response.status_code == 302 mock_send_dotdigital_email.assert_called_once() @@ -684,7 +758,9 @@ def test_delete_account_admin(self, mock_send_dotdigital_email: Mock): school_id = school.id school_name = school.name - teachers = Teacher.objects.filter(school=school).order_by("new_user__last_name", "new_user__first_name") + teachers = Teacher.objects.filter(school=school).order_by( + "new_user__last_name", "new_user__first_name" + ) assert len(teachers) == 3 # one of the remaining teachers should be admin (the second in our case, as it's alphabetical) @@ -715,7 +791,9 @@ def test_delete_account_admin(self, mock_send_dotdigital_email: Mock): self.assertEqual(mock_send_dotdigital_email.call_count, 2) # 2 teachers left - teachers = Teacher.objects.filter(school=school).order_by("new_user__last_name", "new_user__first_name") + teachers = Teacher.objects.filter(school=school).order_by( + "new_user__last_name", "new_user__first_name" + ) assert len(teachers) == 2 # teacher2 should still be admin, teacher4 is not passed admin role because there is teacher2 @@ -727,7 +805,9 @@ def test_delete_account_admin(self, mock_send_dotdigital_email: Mock): # delete teacher4 anonymise(user4) - teachers = Teacher.objects.filter(school=school).order_by("new_user__last_name", "new_user__first_name") + teachers = Teacher.objects.filter(school=school).order_by( + "new_user__last_name", "new_user__first_name" + ) assert len(teachers) == 1 u = User.objects.get(id=usrid2) assert u.new_teacher.is_admin @@ -785,13 +865,17 @@ def test_logged_in_as_admin_check(self): c.logout() @patch("common.helpers.emails.send_dotdigital_email") - def test_registrations_increment_data(self, mock_send_dotdigital_email: Mock): + def test_registrations_increment_data( + self, mock_send_dotdigital_email: Mock + ): c = Client() total_activity = TotalActivity.objects.get(id=1) teacher_registration_count = total_activity.teacher_registrations student_registration_count = total_activity.student_registrations - independent_registration_count = total_activity.independent_registrations + independent_registration_count = ( + total_activity.independent_registrations + ) response = c.post( reverse("register"), @@ -811,7 +895,10 @@ def test_registrations_increment_data(self, mock_send_dotdigital_email: Mock): total_activity = TotalActivity.objects.get(id=1) - assert total_activity.teacher_registrations == teacher_registration_count + 1 + assert ( + total_activity.teacher_registrations + == teacher_registration_count + 1 + ) response = c.post( reverse("register"), @@ -833,7 +920,10 @@ def test_registrations_increment_data(self, mock_send_dotdigital_email: Mock): total_activity = TotalActivity.objects.get(id=1) - assert total_activity.independent_registrations == independent_registration_count + 1 + assert ( + total_activity.independent_registrations + == independent_registration_count + 1 + ) teacher_email, teacher_password = signup_teacher_directly() create_organisation_directly(teacher_email) @@ -849,7 +939,10 @@ def test_registrations_increment_data(self, mock_send_dotdigital_email: Mock): total_activity = TotalActivity.objects.get(id=1) - assert total_activity.student_registrations == student_registration_count + 3 + assert ( + total_activity.student_registrations + == student_registration_count + 3 + ) # CRON view tests @@ -868,8 +961,12 @@ def generic( secure=False, **extra, ): - wsgi_response = super().generic(method, path, data, content_type, secure, **extra) - assert 200 <= wsgi_response.status_code < 300, f"Response has error status code: {wsgi_response.status_code}" + wsgi_response = super().generic( + method, path, data, content_type, secure, **extra + ) + assert ( + 200 <= wsgi_response.status_code < 300 + ), f"Response has error status code: {wsgi_response.status_code}" return wsgi_response @@ -888,7 +985,9 @@ def setUp(self): indy_email, _, _ = create_independent_student_directly() self.teacher_user = User.objects.get(email=teacher_email) - self.teacher_user_profile = UserProfile.objects.get(user=self.teacher_user) + self.teacher_user_profile = UserProfile.objects.get( + user=self.teacher_user + ) self.indy_user = User.objects.get(email=indy_email) self.indy_user_profile = UserProfile.objects.get(user=self.indy_user) @@ -904,11 +1003,17 @@ def send_verify_email_reminder( assert_called: bool, mock_send_dotdigital_email: Mock, ): - self.teacher_user.date_joined = timezone.now() - timedelta(days=days, hours=12) + self.teacher_user.date_joined = timezone.now() - timedelta( + days=days, hours=12 + ) self.teacher_user.save() - self.student_user.date_joined = timezone.now() - timedelta(days=days, hours=12) + self.student_user.date_joined = timezone.now() - timedelta( + days=days, hours=12 + ) self.student_user.save() - self.indy_user.date_joined = timezone.now() - timedelta(days=days, hours=12) + self.indy_user.date_joined = timezone.now() - timedelta( + days=days, hours=12 + ) self.indy_user.save() self.teacher_user_profile.is_verified = is_verified @@ -919,9 +1024,13 @@ def send_verify_email_reminder( self.client.get(reverse(view_name)) if assert_called: - mock_send_dotdigital_email.assert_any_call(ANY, [self.teacher_user.email], personalization_values=ANY) + mock_send_dotdigital_email.assert_any_call( + ANY, [self.teacher_user.email], personalization_values=ANY + ) - mock_send_dotdigital_email.assert_any_call(ANY, [self.indy_user.email], personalization_values=ANY) + mock_send_dotdigital_email.assert_any_call( + ANY, [self.indy_user.email], personalization_values=ANY + ) # Check only two emails are sent - the student should never be included. assert mock_send_dotdigital_email.call_count == 2 @@ -931,22 +1040,40 @@ def send_verify_email_reminder( mock_send_dotdigital_email.reset_mock() def test_first_verify_email_reminder_view(self): - self.send_verify_email_reminder(6, False, "first-verify-email-reminder", False) - self.send_verify_email_reminder(7, False, "first-verify-email-reminder", True) - self.send_verify_email_reminder(7, True, "first-verify-email-reminder", False) - self.send_verify_email_reminder(8, False, "first-verify-email-reminder", False) + self.send_verify_email_reminder( + 6, False, "first-verify-email-reminder", False + ) + self.send_verify_email_reminder( + 7, False, "first-verify-email-reminder", True + ) + self.send_verify_email_reminder( + 7, True, "first-verify-email-reminder", False + ) + self.send_verify_email_reminder( + 8, False, "first-verify-email-reminder", False + ) def test_second_verify_email_reminder_view(self): - self.send_verify_email_reminder(13, False, "second-verify-email-reminder", False) - self.send_verify_email_reminder(14, False, "second-verify-email-reminder", True) - self.send_verify_email_reminder(14, True, "second-verify-email-reminder", False) - self.send_verify_email_reminder(15, False, "second-verify-email-reminder", False) + self.send_verify_email_reminder( + 13, False, "second-verify-email-reminder", False + ) + self.send_verify_email_reminder( + 14, False, "second-verify-email-reminder", True + ) + self.send_verify_email_reminder( + 14, True, "second-verify-email-reminder", False + ) + self.send_verify_email_reminder( + 15, False, "second-verify-email-reminder", False + ) def test_anonymise_unverified_accounts_view(self): now = timezone.now() for user in [self.teacher_user, self.indy_user, self.student_user]: - user.date_joined = now - timedelta(days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS + 1) + user.date_joined = now - timedelta( + days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS + 1 + ) user.save() for user_profile in [self.teacher_user_profile, self.indy_user_profile]: @@ -1011,7 +1138,9 @@ def anonymise_unverified_users( new_user=indy_user, ) - activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] + activity_today = DailyActivity.objects.get_or_create( + date=datetime.now().date() + )[0] daily_teacher_count = activity_today.anonymised_unverified_teachers daily_indy_count = activity_today.anonymised_unverified_independents @@ -1034,16 +1163,30 @@ def anonymise_unverified_users( assert indy_user_active == assert_active assert student_user_active - activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] + activity_today = DailyActivity.objects.get_or_create( + date=datetime.now().date() + )[0] total_activity = TotalActivity.objects.get(id=1) if not teacher_user_active: - assert activity_today.anonymised_unverified_teachers == daily_teacher_count + 1 - assert total_activity.anonymised_unverified_teachers == total_teacher_count + 1 + assert ( + activity_today.anonymised_unverified_teachers + == daily_teacher_count + 1 + ) + assert ( + total_activity.anonymised_unverified_teachers + == total_teacher_count + 1 + ) if not indy_user_active: - assert activity_today.anonymised_unverified_independents == daily_indy_count + 1 - assert total_activity.anonymised_unverified_independents == total_indy_count + 1 + assert ( + activity_today.anonymised_unverified_independents + == daily_indy_count + 1 + ) + assert ( + total_activity.anonymised_unverified_independents + == total_indy_count + 1 + ) teacher_user.delete() indy_user.delete() diff --git a/portal/views/teacher/teach.py b/portal/views/teacher/teach.py index 23ffd108d..afdf370d1 100644 --- a/portal/views/teacher/teach.py +++ b/portal/views/teacher/teach.py @@ -552,15 +552,16 @@ def process_dismiss_student_form(request, formset, klass, access_code): student.new_user.first_name = data["name"] student.new_user.username = data["email"] student.new_user.email = data["email"] + student.user.is_verified = False student.save() student.new_user.save() - student.new_user.userprofile.save() + student.user.save() # log the data joinrelease = JoinReleaseStudent.objects.create(student=student, action_type=JoinReleaseStudent.RELEASE) joinrelease.save() - send_verification_email(request, student.new_user, data) + send_verification_email(request, student.new_user, data, school=klass.teacher.school) if not failed_users: messages.success(request, "The students have been released successfully from the class.")