From 0ac4c8249bd6c873942d8a20f5268bd51e799c10 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Tue, 24 Sep 2024 16:42:12 +0200 Subject: [PATCH 1/2] update import_councils for DB changes respect marking session, send_welcome email --- .../management/commands/import_councils.py | 63 ++++- .../tests/data/council_new_joint_users.csv | 5 + crowdsourcer/tests/data/council_new_users.csv | 5 + .../tests/data/council_split_users.csv | 5 + crowdsourcer/tests/test_commands.py | 257 +++++++++++++++++- 5 files changed, 325 insertions(+), 10 deletions(-) create mode 100644 crowdsourcer/tests/data/council_new_joint_users.csv create mode 100644 crowdsourcer/tests/data/council_new_users.csv create mode 100644 crowdsourcer/tests/data/council_split_users.csv diff --git a/crowdsourcer/management/commands/import_councils.py b/crowdsourcer/management/commands/import_councils.py index ef4b795b..66268288 100644 --- a/crowdsourcer/management/commands/import_councils.py +++ b/crowdsourcer/management/commands/import_councils.py @@ -4,7 +4,13 @@ import pandas as pd -from crowdsourcer.models import Assigned, Marker, PublicAuthority, ResponseType +from crowdsourcer.models import ( + Assigned, + Marker, + MarkingSession, + PublicAuthority, + ResponseType, +) YELLOW = "\033[33m" RED = "\033[31m" @@ -22,13 +28,20 @@ def add_arguments(self, parser): "-q", "--quiet", action="store_true", help="Silence progress bars." ) + parser.add_argument( + "--session", + action="store", + required=True, + help="Marking session to use assignments with", + ) + parser.add_argument( "--add_users", action="store_true", help="add users to database" ) parser.add_argument("--council_list", help="file to import data from") - def handle(self, quiet: bool = False, *args, **options): + def handle(self, quiet: bool = False, session: str = None, *args, **options): if options.get("council_list") is not None: self.council_file = options["council_list"] @@ -43,6 +56,7 @@ def handle(self, quiet: bool = False, *args, **options): ], ) + session = MarkingSession.objects.get(label=session) rt = ResponseType.objects.get(type="Right of Reply") for index, row in df.iterrows(): if pd.isna(row["email"]) or pd.isna(row["gssNumber"]): @@ -63,12 +77,41 @@ def handle(self, quiet: bool = False, *args, **options): continue if Marker.objects.filter(authority=council).exists(): - self.stdout.write(f"user already exists for council: {row['council']}") - continue + m = Marker.objects.get(authority=council) + + if ( + m.user.email == row["email"] + and m.marking_session.filter(pk=session.pk).exists() + ): + self.stdout.write( + f"user already exists for council: {row['council']}" + ) + continue if User.objects.filter(username=row["email"]).exists(): u = User.objects.get(username=row["email"]) - if u.marker.authority is not None and u.marker.authority != council: + if ( + u.marker.authority == council + and not u.marker.marking_session.filter(pk=session.pk).exists() + ): + u.marker.marking_session.set([session]) + self.stdout.write( + f"updating marker to current session: {row['email']} ({council}, {u.marker.authority}" + ) + elif ( + u.marker.authority is None + and not Assigned.objects.filter( + user=u, authority=council, marking_session=session + ).exists() + ): + self.stdout.write( + f"updating marker to council: {row['email']} ({council}, {u.marker.authority}" + ) + if options["add_users"]: + u.marker.authority = council + u.marker.save() + u.marker.marking_session.set([session]) + elif u.marker.authority is not None and u.marker.authority != council: self.stdout.write( f"dual email for councils: {row['email']} ({council}, {u.marker.authority}" ) @@ -76,10 +119,14 @@ def handle(self, quiet: bool = False, *args, **options): for c in [council, u.marker.authority]: if options["add_users"]: a, _ = Assigned.objects.update_or_create( - user=u, authority=c + user=u, + authority=c, + marking_session=session, ) u.marker.authority = None + u.marker.send_welcome_email = True u.marker.save() + u.marker.marking_session.set([session]) continue self.stdout.write(f"user already exists for email: {row['email']}") continue @@ -98,4 +145,8 @@ def handle(self, quiet: bool = False, *args, **options): user=u, authority=council, response_type=rt, + defaults={ + "send_welcome_email": True, + }, ) + m.marking_session.set([session]) diff --git a/crowdsourcer/tests/data/council_new_joint_users.csv b/crowdsourcer/tests/data/council_new_joint_users.csv new file mode 100644 index 00000000..305c5264 --- /dev/null +++ b/crowdsourcer/tests/data/council_new_joint_users.csv @@ -0,0 +1,5 @@ +,firstName,surname,council,councilInternalName,gssNumber,email,official-name +0,Armagh,Staff,"Armagh, Banbridge and Craigavon","Armagh City, Banbridge and Craigavon Borough Council",N09000002,armagh@example.org,"Armagh City, Banbridge and Craigavon Borough Council" +1,Aberdeen,Staff,Aberdeenshire,Aberdeenshire Council,S12000034,aberdeenshire@example.org,Aberdeenshire Council +2,Aberdeen,Staff,Aberdeen City,Aberdeen City Council,S12000033,aberdeenshire@example.org,Aberdeen City Council +3,Adur,Staff,Adur,Adur District Council,E07000223,adur@example.org,Adur District Council diff --git a/crowdsourcer/tests/data/council_new_users.csv b/crowdsourcer/tests/data/council_new_users.csv new file mode 100644 index 00000000..989c09dc --- /dev/null +++ b/crowdsourcer/tests/data/council_new_users.csv @@ -0,0 +1,5 @@ +,firstName,surname,council,councilInternalName,gssNumber,email,official-name +0,Armagh,Staff,"Armagh, Banbridge and Craigavon","Armagh City, Banbridge and Craigavon Borough Council",N09000002,armagh@example.org,"Armagh City, Banbridge and Craigavon Borough Council" +1,Aberdeen,Staff,Aberdeenshire,Aberdeenshire Council,S12000034,aberdeen@example.org,Aberdeenshire Council +2,Aberdeen,Staff,Aberdeen City,Aberdeen City Council,S12000033,aberdeen@example.org,Aberdeen City Council +3,Adur,Staff,Adur,Adur District Council,E07000223,new_adur@example.org,Adur District Council diff --git a/crowdsourcer/tests/data/council_split_users.csv b/crowdsourcer/tests/data/council_split_users.csv new file mode 100644 index 00000000..f6ab775a --- /dev/null +++ b/crowdsourcer/tests/data/council_split_users.csv @@ -0,0 +1,5 @@ +,firstName,surname,council,councilInternalName,gssNumber,email,official-name +0,Armagh,Staff,"Armagh, Banbridge and Craigavon","Armagh City, Banbridge and Craigavon Borough Council",N09000002,armagh@example.org,"Armagh City, Banbridge and Craigavon Borough Council" +1,Aberdeen,Staff,Aberdeenshire,Aberdeenshire Council,S12000034,aberdeenshire@example.org,Aberdeenshire Council +2,Aberdeen,Staff,Aberdeen City,Aberdeen City Council,S12000033,aberdeen@example.org,Aberdeen City Council +3,Adur,Staff,Adur,Adur District Council,E07000223,adur@example.org,Adur District Council diff --git a/crowdsourcer/tests/test_commands.py b/crowdsourcer/tests/test_commands.py index bb586a1e..af9a5184 100644 --- a/crowdsourcer/tests/test_commands.py +++ b/crowdsourcer/tests/test_commands.py @@ -1,6 +1,5 @@ import pathlib from io import StringIO -from unittest import skip from django.contrib.auth.models import User from django.core import mail @@ -103,7 +102,6 @@ def test_unassign(self): ) -@skip("need to fix adding marking session with assigment") class ImportCouncilsTestCase(BaseCommandTestCase): fixtures = [ "authorities.json", @@ -115,7 +113,7 @@ def test_import_councils_no_commit(self): pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" ) - self.call_command("import_councils", council_list=data_file) + self.call_command("import_councils", session="Default", council_list=data_file) self.assertEquals(0, Assigned.objects.count()) self.assertEquals(0, Marker.objects.count()) @@ -125,7 +123,12 @@ def test_import_councils(self): pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" ) - self.call_command("import_councils", add_users=True, council_list=data_file) + self.call_command( + "import_councils", + session="Default", + add_users=True, + council_list=data_file, + ) self.assertEquals(2, User.objects.count()) self.assertEquals(2, Assigned.objects.count()) @@ -151,6 +154,252 @@ def test_import_councils(self): self.assertFalse(User.objects.filter(username="armagh@example.org").exists()) + def test_import_for_new_session_all_same_users(self): + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" + ) + + self.call_command( + "import_councils", + session="Default", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(2, User.objects.count()) + self.assertEquals(2, Assigned.objects.count()) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Default").count() + ) + + self.call_command( + "import_councils", + session="Second Session", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(2, User.objects.count()) + self.assertEquals(4, Assigned.objects.count()) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Default").count() + ) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Second Session").count() + ) + self.assertEquals( + 0, Marker.objects.filter(marking_session__label="Default").count() + ) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Second Session").count() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="aberdeen@example.org", + authority__isnull=True, + ).exists() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="adur@example.org", + authority__name="Adur District Council", + ).exists() + ) + + def test_import_for_new_session_new_users(self): + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" + ) + + self.call_command( + "import_councils", + session="Default", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(2, User.objects.count()) + self.assertEquals(2, Assigned.objects.count()) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Default").count() + ) + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "council_new_users.csv" + ) + + self.call_command( + "import_councils", + session="Second Session", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(3, User.objects.count()) + self.assertEquals(3, Marker.objects.count()) + self.assertEquals(4, Assigned.objects.count()) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Default").count() + ) + for council in ["Aberdeen City Council", "Aberdeenshire Council"]: + self.assertTrue( + Assigned.objects.filter( + user__email="aberdeen@example.org", + marking_session__label="Default", + authority__name=council, + ).exists() + ) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Second Session").count() + ) + for council in ["Aberdeen City Council", "Aberdeenshire Council"]: + self.assertTrue( + Assigned.objects.filter( + user__email="aberdeen@example.org", + marking_session__label="Second Session", + authority__name=council, + ).exists() + ) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Second Session").count() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="aberdeen@example.org", + authority__isnull=True, + ).exists() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="new_adur@example.org", + authority__name="Adur District Council", + ).exists() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Default", + user__email="adur@example.org", + authority__name="Adur District Council", + ).exists() + ) + + def test_import_for_new_session_council_split_users(self): + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" + ) + + self.call_command( + "import_councils", + session="Default", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(2, User.objects.count()) + self.assertEquals(2, Assigned.objects.count()) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Default").count() + ) + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "council_split_users.csv" + ) + + self.call_command( + "import_councils", + session="Second Session", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(3, User.objects.count()) + self.assertEquals(3, Marker.objects.count()) + self.assertEquals(2, Assigned.objects.count()) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Default").count() + ) + self.assertEquals( + 0, Assigned.objects.filter(marking_session__label="Second Session").count() + ) + self.assertEquals( + 3, Marker.objects.filter(marking_session__label="Second Session").count() + ) + + def test_import_for_new_session_council_new_joint_users(self): + data_file = ( + pathlib.Path(__file__).parent.resolve() / "data" / "merged_contacts.csv" + ) + + self.call_command( + "import_councils", + session="Default", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(2, User.objects.count()) + self.assertEquals(2, Assigned.objects.count()) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Default").count() + ) + data_file = ( + pathlib.Path(__file__).parent.resolve() + / "data" + / "council_new_joint_users.csv" + ) + + self.call_command( + "import_councils", + session="Second Session", + add_users=True, + council_list=data_file, + ) + + self.assertEquals(3, User.objects.count()) + self.assertEquals(3, Marker.objects.count()) + self.assertEquals(4, Assigned.objects.count()) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Default").count() + ) + for council in ["Aberdeen City Council", "Aberdeenshire Council"]: + self.assertTrue( + Assigned.objects.filter( + user__email="aberdeen@example.org", + marking_session__label="Default", + authority__name=council, + ).exists() + ) + self.assertEquals( + 2, Assigned.objects.filter(marking_session__label="Second Session").count() + ) + for council in ["Aberdeen City Council", "Aberdeenshire Council"]: + self.assertTrue( + Assigned.objects.filter( + user__email="aberdeenshire@example.org", + marking_session__label="Second Session", + authority__name=council, + ).exists() + ) + self.assertEquals( + 2, Marker.objects.filter(marking_session__label="Second Session").count() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="aberdeenshire@example.org", + authority__isnull=True, + ).exists() + ) + self.assertTrue( + Marker.objects.filter( + marking_session__label="Second Session", + user__email="adur@example.org", + authority__name="Adur District Council", + ).exists() + ) + class RemoveIdenticalDuplicatesTestCase(BaseCommandTestCase): fixtures = [ From 1252c737d55055ac789c33af494d9774eeebb454 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 2 Oct 2024 11:57:27 +0100 Subject: [PATCH 2/2] update welcome email command to allow per stage templates --- .../commands/send_welcome_emails.py | 23 +++++-- crowdsourcer/tests/test_commands.py | 64 ++++++++++++++++++- .../council_repeat_password_email.html | 21 ++++++ 3 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 templates/registration/council_repeat_password_email.html diff --git a/crowdsourcer/management/commands/send_welcome_emails.py b/crowdsourcer/management/commands/send_welcome_emails.py index 961046d6..89fcdaf6 100644 --- a/crowdsourcer/management/commands/send_welcome_emails.py +++ b/crowdsourcer/management/commands/send_welcome_emails.py @@ -22,7 +22,10 @@ def add_arguments(self, parser): parser.add_argument("--send_emails", action="store_true", help="Send emails") parser.add_argument( - "--stage", action="store", help="Only send emails to people in this stage" + "--stage", + required=True, + action="store", + help="Use template for this stage and only send emails to people in this stage", ) parser.add_argument( @@ -42,6 +45,16 @@ def get_config(self, session): return None + def get_templates(self, config, user, stage="First Mark"): + if config.get(stage): + config = config[stage] + + template = config["new_user_template"] + if user.password != "": + template = config["previous_user_template"] + + return (template, config["subject_template"]) + def handle(self, *args, **kwargs): if not kwargs["send_emails"]: self.stdout.write( @@ -94,12 +107,12 @@ def handle(self, *args, **kwargs): if user.email: self.stdout.write(f"Sending email for to this email: {user.email}") if kwargs["send_emails"]: - template = config["new_user_template"] + template, subject_template = self.get_templates( + config, user, kwargs["stage"] + ) if user.password == "": user.set_password(get_random_string(length=20)) user.save() - else: - template = config["previous_user_template"] form = PasswordResetForm({"email": user.email}) assert form.is_valid() @@ -111,7 +124,7 @@ def handle(self, *args, **kwargs): domain_override=config["server_name"], use_https=True, from_email=config["from_email"], - subject_template_name=config["subject_template"], + subject_template_name=subject_template, email_template_name=template, ) marker.send_welcome_email = False diff --git a/crowdsourcer/tests/test_commands.py b/crowdsourcer/tests/test_commands.py index af9a5184..2672f991 100644 --- a/crowdsourcer/tests/test_commands.py +++ b/crowdsourcer/tests/test_commands.py @@ -799,7 +799,7 @@ class SendWelcomeEmails(BaseCommandTestCase): def test_required_args(self): self.assertEquals(len(mail.outbox), 0) with self.assertRaisesRegex( - CommandError, r"following arguments are required: --session" + CommandError, r"following arguments are required: --stage, --session" ): self.call_command( "send_welcome_emails", @@ -810,12 +810,14 @@ def test_basic_run(self): self.assertEquals(len(mail.outbox), 0) self.call_command( "send_welcome_emails", + stage="First Mark", session="Default", ) self.assertEquals(len(mail.outbox), 0) self.call_command( "send_welcome_emails", + stage="First Mark", session="Default", send_emails=True, ) @@ -823,6 +825,7 @@ def test_basic_run(self): self.call_command( "send_welcome_emails", + stage="First Mark", session="Default", send_emails=True, ) @@ -837,6 +840,7 @@ def test_only_sends_if_flag_set(self): self.call_command( "send_welcome_emails", + stage="First Mark", session="Default", send_emails=True, ) @@ -848,6 +852,7 @@ def test_only_sends_if_flag_set(self): def test_email_comtent(self): self.call_command( "send_welcome_emails", + stage="First Mark", session="Default", send_emails=True, ) @@ -883,6 +888,7 @@ def test_limit_session(self): self.assertEquals(len(mail.outbox), 0) self.call_command( "send_welcome_emails", + stage="First Mark", send_emails=True, session="Second Session", ) @@ -890,6 +896,7 @@ def test_limit_session(self): self.call_command( "send_welcome_emails", + stage="First Mark", send_emails=True, session="Default", ) @@ -903,6 +910,7 @@ def test_config_loading(self): self.assertEquals(len(mail.outbox), 0) self.call_command( "send_welcome_emails", + stage="First Mark", send_emails=True, session="Second Session", ) @@ -913,6 +921,7 @@ def test_config_loading(self): mail.outbox = [] self.call_command( "send_welcome_emails", + stage="First Mark", send_emails=True, session="Default", ) @@ -920,11 +929,64 @@ def test_config_loading(self): email = mail.outbox[0] self.assertEquals(email.from_email, "Default From ") + @override_settings( + WELCOME_EMAIL={ + "Default": { + "server_name": "example.org", + "from_email": "Default From ", + "subject_template": "registration/initial_password_email_subject.txt", + "new_user_template": "registration/initial_password_email.html", + "previous_user_template": "registration/repeat_password_email.html", + "Right of Reply": { + "subject_template": "registration/council_password_email_subject.txt", + "new_user_template": "registration/council_password_email.html", + "previous_user_template": "registration/council_repeat_password_email.html", + }, + }, + } + ) + def test_template_loading(self): + marker = Marker.objects.get(user__email="new_marker@example.org") + rt = ResponseType.objects.get(type="Right of Reply") + marker.response_type = rt + marker.save() + + self.assertEquals(len(mail.outbox), 0) + self.call_command( + "send_welcome_emails", + stage="First Mark", + send_emails=True, + session="Default", + ) + self.assertEquals(len(mail.outbox), 1) + email = mail.outbox[0] + self.assertEquals( + email.subject, + "Registration link for CEUK Council Climate Scorecards Scoring System", + ) + self.assertRegex(email.body, r"Thanks for volunteering") + + mail.outbox = [] + self.call_command( + "send_welcome_emails", + stage="Right of Reply", + send_emails=True, + session="Default", + ) + self.assertEquals(len(mail.outbox), 1) + email = mail.outbox[0] + self.assertEquals( + email.subject, + "Registration link for CEUK Council Climate Scorecards Scoring System", + ) + self.assertRegex(email.body, r"council’s contact to receive") + @override_settings(WELCOME_EMAIL={}) def test_error_if_no_config(self): self.assertEquals(len(mail.outbox), 0) _, err = self.call_command( "send_welcome_emails", + stage="First Mark", send_emails=True, session="Default", ) diff --git a/templates/registration/council_repeat_password_email.html b/templates/registration/council_repeat_password_email.html new file mode 100644 index 00000000..c17c8ca5 --- /dev/null +++ b/templates/registration/council_repeat_password_email.html @@ -0,0 +1,21 @@ +{% autoescape off %} +Hi, + +You're receiving this email because you are your council’s contact to receive the Right of Reply for Climate Emergency UK’s Council Climate Scorecards. This email allows you to log into the online data collection system to submit the Right of Reply and respond to your council’s first mark in the Council Climate Scorecards. + +Please go to the following page and choose a new password: +{% block reset_link %} +{{ protocol }}://{{ domain }}{% url 'password_reset_confirm' uidb64=uid token=token %} +{% endblock %} +Your username is {{ user.get_username }} + +Please Note: Multiple people will be able to login to GRACE as long as they have this username and password. Please only share this with relevant staff within your council. We will be unable to set up multiple accounts for your Council. + +You will also receive a second email containing further information, a guide to the Right of Reply process and a recording of the Right of Reply briefing. This email will be sent to every officer contact for your council on our email list. + +Kind Regards, + +The CEUK team. + +{% endautoescape %} +