From eb2b222ab8ed4a47b21d1fcdae59e60e053b16e6 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 6 May 2024 20:07:04 +0200 Subject: [PATCH 01/23] First steps with new JSON importer --- evap/staff/importers/json.py | 82 +++++++++++++++++++++++ evap/staff/tests/test_json_importer.py | 91 ++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 evap/staff/importers/json.py create mode 100644 evap/staff/tests/test_json_importer.py diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py new file mode 100644 index 0000000000..1a17c252ed --- /dev/null +++ b/evap/staff/importers/json.py @@ -0,0 +1,82 @@ +from dataclasses import dataclass +from typing import TypedDict + +from django.db import transaction + +from evap.evaluation.models import Semester, UserProfile +from evap.evaluation.tools import clean_email + + +class ImportStudent(TypedDict): + gguid: str + email: str + name: str + christianname: str + + +class ImportLecturer(TypedDict): + gguid: str + email: str + name: str + christianname: str + titlefront: str + + +class ImportCourse(TypedDict): + cprid: str + scale: str + + +class ImportRelated(TypedDict): + gguid: str + + +class ImportAppointment(TypedDict): + begin: str + end: str + + +class ImportEvent(TypedDict): + gguid: str + lvnr: int + title: str + title_en: str + type: str + isexam: bool + courses: list[ImportCourse] + relatedevents: list[ImportRelated] + appointments: list[ImportAppointment] + lecturers: list[ImportRelated] + students: list[ImportStudent] + + +class ImportDict(TypedDict): + students: list[ImportStudent] + lecturers: list[ImportLecturer] + events: list[ImportEvent] + + +class JSONImporter: + def __init__(self, semester: Semester): + self.semester = semester + self.user_profile_map: dict[str, UserProfile] = {} + + def _import_students(self, data: list[ImportStudent]): + for entry in data: + email = clean_email(entry["email"]) + user_profile = UserProfile.objects.update_or_create( + email=email, + defaults=dict(last_name=entry["name"], first_name_given=entry["christianname"]), + ) + + self.user_profile_map[entry["gguid"]] = user_profile + + def _import_lecturers(self, data: list[ImportLecturer]): + pass + + def _import_events(self, data: list[ImportEvent]): + pass + + @transaction.atomic + def import_json(self, data: ImportDict): + pass diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py new file mode 100644 index 0000000000..538f4b1912 --- /dev/null +++ b/evap/staff/tests/test_json_importer.py @@ -0,0 +1,91 @@ +import json + +from django.test import TestCase +from model_bakery import baker + +from evap.evaluation.models import Semester, UserProfile +from evap.staff.importers.json import ImportDict, JSONImporter + +EXAMPLE_DATA: ImportDict = { + "students": [ + {"gguid": "0x1", "email": "1@example.com", "name": "1", "christianname": "1"}, + {"gguid": "0x2", "email": "2@example.com", "name": "2", "christianname": "2"}, + ], + "lecturers": [ + {"gguid": "0x3", "email": "3@example.com", "name": "3", "christianname": "3", "titlefront": "Prof. Dr."}, + {"gguid": "0x4", "email": "4@example.com", "name": "4", "christianname": "4", "titlefront": "Dr."}, + ], + "events": [ + { + "gguid": "0x5", + "lvnr": "1", + "title": "Prozessorientierte Informationssysteme", + "title_en": "Process-oriented information systems", + "type": "Vorlesung", + "isexam": "false", + "courses": [ + {"cprid": "BA-Inf", "scale": "GRADE_PARTICIPATION"}, + {"cprid": "MA-Inf", "scale": "GRADE_PARTICIPATION"}, + ], + "relatedevents": {"gguid": "0x6"}, + "appointments": [{"begin": "15.04.2024 10:15", "end": "15.07.2024 11:45"}], + "lecturers": [{"gguid": "0x3"}], + "students": [{"gguid": "0x1"}, {"gguid": "0x2"}], + }, + { + "gguid": "0x6", + "lvnr": "2", + "title": "Prozessorientierte Informationssysteme", + "title_en": "Process-oriented information systems", + "type": "Klausur", + "isexam": "true", + "courses": [ + {"cprid": "BA-Inf", "scale": "GRADE_TO_A_THIRD"}, + {"cprid": "MA-Inf", "scale": "GRADE_TO_A_THIRD"}, + ], + "relatedevents": {"gguid": "0x5"}, + "appointments": [{"begin": "29.07.2024 10:15", "end": "29.07.2024 11:45"}], + "lecturers": [{"gguid": "0x3"}, {"gguid": "0x4"}], + "students": [{"gguid": "0x1"}, {"gguid": "0x2"}], + }, + ], +} +EXAMPLE_JSON = json.dumps(EXAMPLE_DATA) + + +class ImportStudentsTestCase(TestCase): + @classmethod + def setUp(self): + self.students = EXAMPLE_DATA["students"] + + self.semester = baker.make(Semester) + + def test_import_students(self): + self.assertEqual(UserProfile.objects.all().count(), 0) + + importer = JSONImporter(self.semester) + importer._import_students(self.students) + + user_profiles = UserProfile.objects.all() + self.assertEqual(user_profiles.count(), 2) + + for i, user_profile in enumerate(user_profiles): + self.assertEqual(user_profile.email, self.students[i]["email"]) + self.assertEqual(user_profile.last_name, self.students[i]["name"]) + self.assertEqual(user_profile.first_name_given, self.students[i]["christianname"]) + + def test_import_existing_students(self): + + user_profile = baker.make(UserProfile, email=self.students[0]["email"]) + print(user_profile.email) + + importer = JSONImporter(self.semester) + importer._import_students(self.students) + + assert UserProfile.objects.all().count() == 2 + + user_profile.refresh_from_db() + + self.assertEqual(user_profile.email, self.students[0]["email"]) + self.assertEqual(user_profile.last_name, self.students[0]["name"]) + self.assertEqual(user_profile.first_name_given, self.students[0]["christianname"]) From 115d19e089bd3c5556742fe19280ea21240594d8 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 6 May 2024 20:11:21 +0200 Subject: [PATCH 02/23] [JSON import] Import lecturers --- evap/staff/importers/json.py | 11 ++++++++- evap/staff/tests/test_json_importer.py | 33 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 1a17c252ed..37eba63ac0 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -72,7 +72,16 @@ def _import_students(self, data: list[ImportStudent]): self.user_profile_map[entry["gguid"]] = user_profile def _import_lecturers(self, data: list[ImportLecturer]): - pass + for entry in data: + email = clean_email(entry["email"]) + user_profile = UserProfile.objects.update_or_create( + email=email, + defaults=dict( + last_name=entry["name"], first_name_given=entry["christianname"], title=entry["titlefront"] + ), + ) + + self.user_profile_map[entry["gguid"]] = user_profile def _import_events(self, data: list[ImportEvent]): pass diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 538f4b1912..3890084c87 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -57,6 +57,7 @@ class ImportStudentsTestCase(TestCase): @classmethod def setUp(self): self.students = EXAMPLE_DATA["students"] + self.lecturers = EXAMPLE_DATA["lecturers"] self.semester = baker.make(Semester) @@ -89,3 +90,35 @@ def test_import_existing_students(self): self.assertEqual(user_profile.email, self.students[0]["email"]) self.assertEqual(user_profile.last_name, self.students[0]["name"]) self.assertEqual(user_profile.first_name_given, self.students[0]["christianname"]) + + def test_import_lecturers(self): + self.assertEqual(UserProfile.objects.all().count(), 0) + + importer = JSONImporter(self.semester) + importer._import_lecturers(self.lecturers) + + user_profiles = UserProfile.objects.all() + self.assertEqual(user_profiles.count(), 2) + + for i, user_profile in enumerate(user_profiles): + self.assertEqual(user_profile.email, self.lecturers[i]["email"]) + self.assertEqual(user_profile.last_name, self.lecturers[i]["name"]) + self.assertEqual(user_profile.first_name_given, self.lecturers[i]["christianname"]) + self.assertEqual(user_profile.title, self.lecturers[i]["titlefront"]) + + def test_import_existing_lecturers(self): + + user_profile = baker.make(UserProfile, email=self.lecturers[0]["email"]) + print(user_profile.email) + + importer = JSONImporter(self.semester) + importer._import_lecturers(self.lecturers) + + assert UserProfile.objects.all().count() == 2 + + user_profile.refresh_from_db() + + self.assertEqual(user_profile.email, self.lecturers[0]["email"]) + self.assertEqual(user_profile.last_name, self.lecturers[0]["name"]) + self.assertEqual(user_profile.first_name_given, self.lecturers[0]["christianname"]) + self.assertEqual(user_profile.title, self.lecturers[0]["titlefront"]) From 814dbee2acc6114531b8e56cd2d3522e020e007c Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 13 May 2024 20:16:09 +0200 Subject: [PATCH 03/23] [JSON importer] Import courses and evaluations --- .../0143_course_evaluation_cms_id.py | 35 +++++ evap/evaluation/models.py | 12 +- evap/staff/importers/json.py | 127 ++++++++++++++++-- evap/staff/tests/test_json_importer.py | 78 +++++++++-- 4 files changed, 230 insertions(+), 22 deletions(-) create mode 100644 evap/evaluation/migrations/0143_course_evaluation_cms_id.py diff --git a/evap/evaluation/migrations/0143_course_evaluation_cms_id.py b/evap/evaluation/migrations/0143_course_evaluation_cms_id.py new file mode 100644 index 0000000000..b2c9470650 --- /dev/null +++ b/evap/evaluation/migrations/0143_course_evaluation_cms_id.py @@ -0,0 +1,35 @@ +# Generated by Django 5.0.4 on 2024-05-13 20:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("evaluation", "0142_alter_evaluation_state"), + ] + + operations = [ + migrations.AddField( + model_name="course", + name="cms_id", + field=models.CharField(blank=True, max_length=255, verbose_name="campus management system id"), + ), + migrations.AddField( + model_name="evaluation", + name="cms_id", + field=models.CharField(blank=True, max_length=255, verbose_name="campus management system id"), + ), + migrations.AddConstraint( + model_name="course", + constraint=models.UniqueConstraint( + condition=models.Q(("cms_id", ""), _negated=True), fields=("cms_id",), name="unique_cms_id_course" + ), + ), + migrations.AddConstraint( + model_name="evaluation", + constraint=models.UniqueConstraint( + condition=models.Q(("cms_id", ""), _negated=True), fields=("cms_id",), name="unique_cms_id_evaluation" + ), + ), + ] diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index befc963aa0..f6e67029e9 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -16,7 +16,7 @@ from django.core.exceptions import ValidationError from django.core.mail import EmailMultiAlternatives from django.db import IntegrityError, models, transaction -from django.db.models import CheckConstraint, Count, F, Manager, OuterRef, Q, Subquery, Value +from django.db.models import CheckConstraint, Count, F, Manager, OuterRef, Q, Subquery, UniqueConstraint, Value from django.db.models.functions import Coalesce, Lower, NullIf, TruncDate from django.dispatch import Signal, receiver from django.template import Context, Template @@ -322,6 +322,9 @@ class Course(LoggedModel): # grade publishers can set this to True, then the course will be handled as if final grades have already been uploaded gets_no_grade_documents = models.BooleanField(verbose_name=_("gets no grade documents"), default=False) + # unique reference for import from campus management system + cms_id = models.CharField(verbose_name=_("campus management system id"), blank=True, max_length=255) + class Meta: unique_together = [ ["semester", "name_de"], @@ -329,6 +332,9 @@ class Meta: ] verbose_name = _("course") verbose_name_plural = _("courses") + constraints = [ + UniqueConstraint(fields=["cms_id"], condition=~Q(cms_id=""), name="unique_cms_id_course"), + ] def __str__(self): return self.name @@ -444,6 +450,9 @@ class State: verbose_name=_("wait for grade upload before publishing"), default=True ) + # unique reference for import from campus management system + cms_id = models.CharField(verbose_name=_("campus management system id"), blank=True, max_length=255) + class TextAnswerReviewState(Enum): do_not_call_in_templates = True # pylint: disable=invalid-name NO_TEXTANSWERS = auto() @@ -468,6 +477,7 @@ class Meta: check=~(Q(_participant_count__isnull=True) ^ Q(_voter_count__isnull=True)), name="check_evaluation_participant_count_and_voter_count_both_set_or_not_set", ), + UniqueConstraint(fields=["cms_id"], condition=~Q(cms_id=""), name="unique_cms_id_evaluation"), ] def __str__(self): diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 37eba63ac0..a0e3256445 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -1,9 +1,9 @@ -from dataclasses import dataclass +from datetime import datetime, timedelta from typing import TypedDict from django.db import transaction -from evap.evaluation.models import Semester, UserProfile +from evap.evaluation.models import Course, CourseType, Degree, Evaluation, Semester, UserProfile from evap.evaluation.tools import clean_email @@ -44,10 +44,10 @@ class ImportEvent(TypedDict): type: str isexam: bool courses: list[ImportCourse] - relatedevents: list[ImportRelated] + relatedevents: ImportRelated appointments: list[ImportAppointment] lecturers: list[ImportRelated] - students: list[ImportStudent] + students: list[ImportRelated] class ImportDict(TypedDict): @@ -57,16 +57,40 @@ class ImportDict(TypedDict): class JSONImporter: + DATETIME_FORMAT = "%d.%m.%Y %H:%M" + def __init__(self, semester: Semester): self.semester = semester self.user_profile_map: dict[str, UserProfile] = {} + self.course_type_cache: dict[str, CourseType] = {} + self.degree_cache: dict[str, Degree] = {} + self.course_map: dict[str, Course] = {} + + def _get_course_type(self, name: str) -> CourseType: + if name in self.course_type_cache: + return self.course_type_cache[name] + + course_type = CourseType.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] + self.course_type_cache[name] = course_type + return course_type + + def _get_degree(self, name: str) -> Degree: + if name in self.degree_cache: + return self.degree_cache[name] + + degree = Degree.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] + self.degree_cache[name] = degree + return degree + + def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: + return [self.user_profile_map[related["gguid"]] for related in data] def _import_students(self, data: list[ImportStudent]): for entry in data: email = clean_email(entry["email"]) - user_profile = UserProfile.objects.update_or_create( + user_profile, __ = UserProfile.objects.update_or_create( email=email, - defaults=dict(last_name=entry["name"], first_name_given=entry["christianname"]), + defaults={"last_name": entry["name"], "first_name_given": entry["christianname"]}, ) self.user_profile_map[entry["gguid"]] = user_profile @@ -74,18 +98,97 @@ def _import_students(self, data: list[ImportStudent]): def _import_lecturers(self, data: list[ImportLecturer]): for entry in data: email = clean_email(entry["email"]) - user_profile = UserProfile.objects.update_or_create( + user_profile, __ = UserProfile.objects.update_or_create( email=email, - defaults=dict( - last_name=entry["name"], first_name_given=entry["christianname"], title=entry["titlefront"] - ), + defaults={ + "last_name": entry["name"], + "first_name_given": entry["christianname"], + "title": entry["titlefront"], + }, ) self.user_profile_map[entry["gguid"]] = user_profile + def _import_course(self, data: ImportEvent) -> Course: + course_type = self._get_course_type(data["type"]) + degrees = [self._get_degree(c["cprid"]) for c in data["courses"]] + responsibles = self._get_user_profiles(data["lecturers"]) + course, __ = Course.objects.update_or_create( + semester=self.semester, + cms_id=data["gguid"], + defaults={"name_de": data["title"], "name_en": data["title_en"], "type": course_type}, + ) + course.degrees.set(degrees) + course.responsibles.set(responsibles) + + self.course_map[data["gguid"]] = course + + return course + + def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: + course_end = datetime.strptime(data["appointments"][0]["end"], self.DATETIME_FORMAT) + + if data["isexam"]: + # Set evaluation time frame of three days for exam evaluations: + # Start datetime is at 8:00 am one day after the event ends + evaluation_start_datetime = course_end.replace(hour=8, minute=0) + timedelta(days=1) + # End date is three days after the event ends + evaluation_end_date = (course_end + timedelta(days=3)).date() + + name_de = "Klausur" + name_en = "Exam" + else: + # Set evaluation time frame of two weeks for normal evaluations: + # Start datetime is at 8:00 am on the monday in the week before the event ends + evaluation_start_datetime = course_end.replace(hour=8, minute=0) - timedelta( + weeks=1, days=course_end.weekday() + ) + # End date is on the sunday in the week the event ends + evaluation_end_date = (course_end + timedelta(days=6 - course_end.weekday())).date() + + name_de, name_en = "", "" + + # If events are graded for any degree, wait for grade upload before publishing + wait_for_grade_upload_before_publishing = any(filter(lambda grade: grade["scale"], data["courses"])) + + participants = self._get_user_profiles(data["students"]) + + evaluation, __ = Evaluation.objects.update_or_create( + course=course, + cms_id=data["gguid"], + defaults={ + "name_de": name_de, + "name_en": name_en, + "vote_start_datetime": evaluation_start_datetime, + "vote_end_date": evaluation_end_date, + "wait_for_grade_upload_before_publishing": wait_for_grade_upload_before_publishing, + }, + ) + evaluation.participants.set(participants) + + return evaluation + def _import_events(self, data: list[ImportEvent]): - pass + # Divide in two lists so corresponding courses are imported before their exams + normal_events = filter(lambda e: not e["isexam"], data) + exam_events = filter(lambda e: e["isexam"], data) + + for event in normal_events: + event: ImportEvent + + course = self._import_course(event) + + self._import_evaluation(course, event) + + for event in exam_events: + event: ImportEvent + + course = self.course_map[event["relatedevents"]["gguid"]] + + self._import_evaluation(course, event) @transaction.atomic def import_json(self, data: ImportDict): - pass + self._import_students(data["students"]) + self._import_lecturers(data["lecturers"]) + self._import_events(data["events"]) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 3890084c87..44c3a5f9c4 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -1,9 +1,10 @@ import json +from datetime import date, datetime from django.test import TestCase from model_bakery import baker -from evap.evaluation.models import Semester, UserProfile +from evap.evaluation.models import Course, Evaluation, Semester, UserProfile from evap.staff.importers.json import ImportDict, JSONImporter EXAMPLE_DATA: ImportDict = { @@ -18,11 +19,11 @@ "events": [ { "gguid": "0x5", - "lvnr": "1", + "lvnr": 1, "title": "Prozessorientierte Informationssysteme", "title_en": "Process-oriented information systems", "type": "Vorlesung", - "isexam": "false", + "isexam": False, "courses": [ {"cprid": "BA-Inf", "scale": "GRADE_PARTICIPATION"}, {"cprid": "MA-Inf", "scale": "GRADE_PARTICIPATION"}, @@ -34,14 +35,14 @@ }, { "gguid": "0x6", - "lvnr": "2", + "lvnr": 2, "title": "Prozessorientierte Informationssysteme", "title_en": "Process-oriented information systems", "type": "Klausur", - "isexam": "true", + "isexam": True, "courses": [ - {"cprid": "BA-Inf", "scale": "GRADE_TO_A_THIRD"}, - {"cprid": "MA-Inf", "scale": "GRADE_TO_A_THIRD"}, + {"cprid": "BA-Inf", "scale": ""}, + {"cprid": "MA-Inf", "scale": ""}, ], "relatedevents": {"gguid": "0x5"}, "appointments": [{"begin": "29.07.2024 10:15", "end": "29.07.2024 11:45"}], @@ -53,8 +54,7 @@ EXAMPLE_JSON = json.dumps(EXAMPLE_DATA) -class ImportStudentsTestCase(TestCase): - @classmethod +class ImportUserProfilesTestCase(TestCase): def setUp(self): self.students = EXAMPLE_DATA["students"] self.lecturers = EXAMPLE_DATA["lecturers"] @@ -122,3 +122,63 @@ def test_import_existing_lecturers(self): self.assertEqual(user_profile.last_name, self.lecturers[0]["name"]) self.assertEqual(user_profile.first_name_given, self.lecturers[0]["christianname"]) self.assertEqual(user_profile.title, self.lecturers[0]["titlefront"]) + + +class ImportEventsTestCase(TestCase): + def setUp(self): + self.semester = baker.make(Semester) + + def _import(self): + importer = JSONImporter(self.semester) + importer.import_json(EXAMPLE_DATA) + return importer + + def test_import_courses(self): + importer = self._import() + + self.assertEqual(Course.objects.all().count(), 1) + course = Course.objects.all()[0] + + self.assertEqual(course.semester, self.semester) + self.assertEqual(course.cms_id, EXAMPLE_DATA["events"][0]["gguid"]) + self.assertEqual(course.name_de, EXAMPLE_DATA["events"][0]["title"]) + self.assertEqual(course.name_en, EXAMPLE_DATA["events"][0]["title_en"]) + self.assertEqual(course.type.name_de, EXAMPLE_DATA["events"][0]["type"]) + self.assertListEqual( + [d.name_de for d in course.degrees.all()], [d["cprid"] for d in EXAMPLE_DATA["events"][0]["courses"]] + ) + self.assertListEqual( + list(course.responsibles.all()), + [importer.user_profile_map[lecturer["gguid"]] for lecturer in EXAMPLE_DATA["events"][0]["lecturers"]], + ) + + main_evaluation = Evaluation.objects.get(name_en="") + self.assertEqual(main_evaluation.course, course) + self.assertEqual(main_evaluation.name_de, "") + self.assertEqual(main_evaluation.name_en, "") + # [{"begin": "15.04.2024 10:15", "end": "15.07.2024 11:45"}] + self.assertEqual(main_evaluation.vote_start_datetime, datetime(2024, 7, 8, 8, 0)) + self.assertEqual(main_evaluation.vote_end_date, date(2024, 7, 21)) + self.assertListEqual( + list(main_evaluation.participants.all()), + [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][0]["students"]], + ) + self.assertTrue(main_evaluation.wait_for_grade_upload_before_publishing) + # FIXME lecturers + + exam_evaluation = Evaluation.objects.get(name_en="Exam") + self.assertEqual(exam_evaluation.course, course) + self.assertEqual(exam_evaluation.name_de, "Klausur") + self.assertEqual(exam_evaluation.name_en, "Exam") + # [{"begin": "29.07.2024 10:15", "end": "29.07.2024 11:45"}] + self.assertEqual(exam_evaluation.vote_start_datetime, datetime(2024, 7, 30, 8, 0)) + self.assertEqual(exam_evaluation.vote_end_date, date(2024, 8, 1)) + self.assertListEqual( + list(exam_evaluation.participants.all()), + [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][1]["students"]], + ) + self.assertFalse(exam_evaluation.wait_for_grade_upload_before_publishing) + # FIXME lecturers + + def test_import_courses_update(self): + pass From 85080801409db0d08baa90a8ecae8002a124e6ca Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 13 May 2024 21:04:53 +0200 Subject: [PATCH 04/23] Use unique attribute for cms_id --- .../0143_course_cms_id_evaluation_cms_id.py | 27 ++++++++++++++ .../0143_course_evaluation_cms_id.py | 35 ------------------- evap/evaluation/models.py | 14 ++++---- 3 files changed, 34 insertions(+), 42 deletions(-) create mode 100644 evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py delete mode 100644 evap/evaluation/migrations/0143_course_evaluation_cms_id.py diff --git a/evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py b/evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py new file mode 100644 index 0000000000..851199d2f9 --- /dev/null +++ b/evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py @@ -0,0 +1,27 @@ +# Generated by Django 5.0.4 on 2024-05-13 20:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("evaluation", "0142_alter_evaluation_state"), + ] + + operations = [ + migrations.AddField( + model_name="course", + name="cms_id", + field=models.CharField( + blank=True, max_length=255, null=True, unique=True, verbose_name="campus management system id" + ), + ), + migrations.AddField( + model_name="evaluation", + name="cms_id", + field=models.CharField( + blank=True, max_length=255, null=True, unique=True, verbose_name="campus management system id" + ), + ), + ] diff --git a/evap/evaluation/migrations/0143_course_evaluation_cms_id.py b/evap/evaluation/migrations/0143_course_evaluation_cms_id.py deleted file mode 100644 index b2c9470650..0000000000 --- a/evap/evaluation/migrations/0143_course_evaluation_cms_id.py +++ /dev/null @@ -1,35 +0,0 @@ -# Generated by Django 5.0.4 on 2024-05-13 20:12 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("evaluation", "0142_alter_evaluation_state"), - ] - - operations = [ - migrations.AddField( - model_name="course", - name="cms_id", - field=models.CharField(blank=True, max_length=255, verbose_name="campus management system id"), - ), - migrations.AddField( - model_name="evaluation", - name="cms_id", - field=models.CharField(blank=True, max_length=255, verbose_name="campus management system id"), - ), - migrations.AddConstraint( - model_name="course", - constraint=models.UniqueConstraint( - condition=models.Q(("cms_id", ""), _negated=True), fields=("cms_id",), name="unique_cms_id_course" - ), - ), - migrations.AddConstraint( - model_name="evaluation", - constraint=models.UniqueConstraint( - condition=models.Q(("cms_id", ""), _negated=True), fields=("cms_id",), name="unique_cms_id_evaluation" - ), - ), - ] diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index f6e67029e9..c95feb7a8f 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -16,7 +16,7 @@ from django.core.exceptions import ValidationError from django.core.mail import EmailMultiAlternatives from django.db import IntegrityError, models, transaction -from django.db.models import CheckConstraint, Count, F, Manager, OuterRef, Q, Subquery, UniqueConstraint, Value +from django.db.models import CheckConstraint, Count, F, Manager, OuterRef, Q, Subquery, Value from django.db.models.functions import Coalesce, Lower, NullIf, TruncDate from django.dispatch import Signal, receiver from django.template import Context, Template @@ -323,7 +323,9 @@ class Course(LoggedModel): gets_no_grade_documents = models.BooleanField(verbose_name=_("gets no grade documents"), default=False) # unique reference for import from campus management system - cms_id = models.CharField(verbose_name=_("campus management system id"), blank=True, max_length=255) + cms_id = models.CharField( + verbose_name=_("campus management system id"), blank=True, null=True, unique=True, max_length=255 + ) class Meta: unique_together = [ @@ -332,9 +334,6 @@ class Meta: ] verbose_name = _("course") verbose_name_plural = _("courses") - constraints = [ - UniqueConstraint(fields=["cms_id"], condition=~Q(cms_id=""), name="unique_cms_id_course"), - ] def __str__(self): return self.name @@ -451,7 +450,9 @@ class State: ) # unique reference for import from campus management system - cms_id = models.CharField(verbose_name=_("campus management system id"), blank=True, max_length=255) + cms_id = models.CharField( + verbose_name=_("campus management system id"), blank=True, null=True, unique=True, max_length=255 + ) class TextAnswerReviewState(Enum): do_not_call_in_templates = True # pylint: disable=invalid-name @@ -477,7 +478,6 @@ class Meta: check=~(Q(_participant_count__isnull=True) ^ Q(_voter_count__isnull=True)), name="check_evaluation_participant_count_and_voter_count_both_set_or_not_set", ), - UniqueConstraint(fields=["cms_id"], condition=~Q(cms_id=""), name="unique_cms_id_evaluation"), ] def __str__(self): From 66e8f482350875fdfb4384a43242047d462dbfad Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 13 May 2024 21:05:51 +0200 Subject: [PATCH 05/23] Clean up JSON import --- evap/staff/importers/json.py | 2 -- evap/staff/tests/test_json_importer.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index a0e3256445..4cba7a88b1 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -130,9 +130,7 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: if data["isexam"]: # Set evaluation time frame of three days for exam evaluations: - # Start datetime is at 8:00 am one day after the event ends evaluation_start_datetime = course_end.replace(hour=8, minute=0) + timedelta(days=1) - # End date is three days after the event ends evaluation_end_date = (course_end + timedelta(days=3)).date() name_de = "Klausur" diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 44c3a5f9c4..08ff8f5b9d 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -54,7 +54,7 @@ EXAMPLE_JSON = json.dumps(EXAMPLE_DATA) -class ImportUserProfilesTestCase(TestCase): +class TestImportUserProfiles(TestCase): def setUp(self): self.students = EXAMPLE_DATA["students"] self.lecturers = EXAMPLE_DATA["lecturers"] @@ -124,7 +124,7 @@ def test_import_existing_lecturers(self): self.assertEqual(user_profile.title, self.lecturers[0]["titlefront"]) -class ImportEventsTestCase(TestCase): +class TestImportEvents(TestCase): def setUp(self): self.semester = baker.make(Semester) From e4b0300f420fbd35935708f24793c56e5889baf5 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 13 May 2024 21:36:16 +0200 Subject: [PATCH 06/23] Import contributions from JSON --- evap/staff/importers/json.py | 14 +++++++++++++- evap/staff/tests/test_json_importer.py | 24 +++++++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 4cba7a88b1..2ac5802a25 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -3,7 +3,7 @@ from django.db import transaction -from evap.evaluation.models import Course, CourseType, Degree, Evaluation, Semester, UserProfile +from evap.evaluation.models import Contribution, Course, CourseType, Degree, Evaluation, Semester, UserProfile from evap.evaluation.tools import clean_email @@ -164,8 +164,20 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: ) evaluation.participants.set(participants) + for lecturer in data["lecturers"]: + self._import_contribution(evaluation, lecturer) + return evaluation + def _import_contribution(self, evaluation: Evaluation, data: ImportRelated): + user_profile = self.user_profile_map[data["gguid"]] + + contribution, __ = Contribution.objects.update_or_create( + evaluation=evaluation, + contributor=user_profile, + ) + return contribution + def _import_events(self, data: list[ImportEvent]): # Divide in two lists so corresponding courses are imported before their exams normal_events = filter(lambda e: not e["isexam"], data) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 08ff8f5b9d..3b39338fe0 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -4,7 +4,7 @@ from django.test import TestCase from model_bakery import baker -from evap.evaluation.models import Course, Evaluation, Semester, UserProfile +from evap.evaluation.models import Contribution, Course, Evaluation, Semester, UserProfile from evap.staff.importers.json import ImportDict, JSONImporter EXAMPLE_DATA: ImportDict = { @@ -164,7 +164,16 @@ def test_import_courses(self): [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][0]["students"]], ) self.assertTrue(main_evaluation.wait_for_grade_upload_before_publishing) - # FIXME lecturers + + self.assertEqual(Contribution.objects.filter(evaluation=main_evaluation).count(), 2) + self.assertListEqual( + list( + Contribution.objects.filter(evaluation=main_evaluation, contributor__isnull=False).values_list( + "contributor_id", flat=True + ) + ), + [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][0]["lecturers"]], + ) exam_evaluation = Evaluation.objects.get(name_en="Exam") self.assertEqual(exam_evaluation.course, course) @@ -178,7 +187,16 @@ def test_import_courses(self): [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][1]["students"]], ) self.assertFalse(exam_evaluation.wait_for_grade_upload_before_publishing) - # FIXME lecturers + + self.assertEqual(Contribution.objects.filter(evaluation=exam_evaluation).count(), 3) + self.assertListEqual( + list( + Contribution.objects.filter(evaluation=exam_evaluation, contributor__isnull=False).values_list( + "contributor_id", flat=True + ) + ), + [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][1]["lecturers"]], + ) def test_import_courses_update(self): pass From cd7d40fd10b8a044eefa4c3e61fc386a2d2e57a5 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 27 May 2024 18:27:54 +0200 Subject: [PATCH 07/23] [JSON import] Don't import data for evaluations in approved state --- evap/staff/importers/json.py | 19 +++++++++++------ evap/staff/tests/test_json_importer.py | 29 +++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 2ac5802a25..b66c6b35dd 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -151,7 +151,7 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: participants = self._get_user_profiles(data["students"]) - evaluation, __ = Evaluation.objects.update_or_create( + evaluation, __ = Evaluation.objects.get_or_create( course=course, cms_id=data["gguid"], defaults={ @@ -159,13 +159,20 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: "name_en": name_en, "vote_start_datetime": evaluation_start_datetime, "vote_end_date": evaluation_end_date, - "wait_for_grade_upload_before_publishing": wait_for_grade_upload_before_publishing, }, ) - evaluation.participants.set(participants) - - for lecturer in data["lecturers"]: - self._import_contribution(evaluation, lecturer) + if evaluation.state < Evaluation.State.APPROVED: + evaluation.name_de = name_de + evaluation.name_en = name_en + evaluation.vote_start_datetime = evaluation_start_datetime + evaluation.vote_end_date = evaluation_end_date + evaluation.wait_for_grade_upload_before_publishing = wait_for_grade_upload_before_publishing + evaluation.save() + + evaluation.participants.set(participants) + + for lecturer in data["lecturers"]: + self._import_contribution(evaluation, lecturer) return evaluation diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 3b39338fe0..8625e70ad8 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -4,7 +4,7 @@ from django.test import TestCase from model_bakery import baker -from evap.evaluation.models import Contribution, Course, Evaluation, Semester, UserProfile +from evap.evaluation.models import Contribution, Course, Evaluation, Questionnaire, Semester, UserProfile from evap.staff.importers.json import ImportDict, JSONImporter EXAMPLE_DATA: ImportDict = { @@ -198,5 +198,32 @@ def test_import_courses(self): [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][1]["lecturers"]], ) + def test_import_courses_evaluation_approved(self): + self._import() + + evaluation = Evaluation.objects.get(name_en="") + + evaluation.name_en = "Test" + evaluation.save() + + self._import() + + evaluation = Evaluation.objects.get(pk=evaluation.pk) + + self.assertEqual(evaluation.name_en, "") + + evaluation.general_contribution.questionnaires.add( + baker.make(Questionnaire, type=Questionnaire.Type.CONTRIBUTOR) + ) + evaluation.manager_approve() + evaluation.name_en = "Test" + evaluation.save() + + self._import() + + evaluation = Evaluation.objects.get(pk=evaluation.pk) + + self.assertEqual(evaluation.name_en, "Test") + def test_import_courses_update(self): pass From 34b20dd351aa423cbcb50ccf4347b3a9e28a0ecf Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 10 Jun 2024 21:04:41 +0200 Subject: [PATCH 08/23] [JSON import] Create statistics during the import --- evap/staff/importers/json.py | 93 +++++++++++++++++++++----- evap/staff/tests/test_json_importer.py | 54 ++++++++++++--- evap/staff/tools.py | 37 +++++++++- 3 files changed, 155 insertions(+), 29 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index b66c6b35dd..157384b326 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass, field from datetime import datetime, timedelta from typing import TypedDict @@ -5,6 +6,7 @@ from evap.evaluation.models import Contribution, Course, CourseType, Degree, Evaluation, Semester, UserProfile from evap.evaluation.tools import clean_email +from evap.staff.tools import update_or_create_with_changes, update_with_changes class ImportStudent(TypedDict): @@ -56,6 +58,24 @@ class ImportDict(TypedDict): events: list[ImportEvent] +@dataclass +class NameChange: + old_last_name: str + old_first_name_given: str + new_last_name: str + new_first_name_given: str + + +@dataclass +class ImportStatistics: + name_changes: list[NameChange] = field(default_factory=list) + new_courses: list[Course] = field(default_factory=list) + new_evaluations: list[Evaluation] = field(default_factory=list) + updated_courses: list[Course] = field(default_factory=list) + updated_evaluations: list[Evaluation] = field(default_factory=list) + attempted_changes: list[Evaluation] = field(default_factory=list) + + class JSONImporter: DATETIME_FORMAT = "%d.%m.%Y %H:%M" @@ -65,6 +85,7 @@ def __init__(self, semester: Semester): self.course_type_cache: dict[str, CourseType] = {} self.degree_cache: dict[str, Degree] = {} self.course_map: dict[str, Course] = {} + self.statistics = ImportStatistics() def _get_course_type(self, name: str) -> CourseType: if name in self.course_type_cache: @@ -88,10 +109,28 @@ def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: def _import_students(self, data: list[ImportStudent]): for entry in data: email = clean_email(entry["email"]) - user_profile, __ = UserProfile.objects.update_or_create( + user_profile, __, changes = update_or_create_with_changes( + UserProfile, email=email, defaults={"last_name": entry["name"], "first_name_given": entry["christianname"]}, ) + user_profile: UserProfile + if changes: + change = NameChange( + old_last_name=changes["last_name"][0] if changes.get("last_name") else user_profile.last_name, + old_first_name_given=( + changes["first_name_given"][0] + if changes.get("first_name_given") + else user_profile.first_name_given + ), + new_last_name=changes["last_name"][1] if changes.get("last_name") else user_profile.last_name, + new_first_name_given=( + changes["first_name_given"][1] + if changes.get("first_name_given") + else user_profile.first_name_given + ), + ) + self.statistics.name_changes.append(change) self.user_profile_map[entry["gguid"]] = user_profile @@ -113,18 +152,26 @@ def _import_course(self, data: ImportEvent) -> Course: course_type = self._get_course_type(data["type"]) degrees = [self._get_degree(c["cprid"]) for c in data["courses"]] responsibles = self._get_user_profiles(data["lecturers"]) - course, __ = Course.objects.update_or_create( + course, created, changes = update_or_create_with_changes( + Course, semester=self.semester, cms_id=data["gguid"], defaults={"name_de": data["title"], "name_en": data["title_en"], "type": course_type}, ) + course: Course course.degrees.set(degrees) course.responsibles.set(responsibles) + if changes: + self.statistics.updated_courses.append(course) + if created: + self.statistics.new_courses.append(course) + self.course_map[data["gguid"]] = course return course + # pylint: disable=too-many-locals def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: course_end = datetime.strptime(data["appointments"][0]["end"], self.DATETIME_FORMAT) @@ -151,39 +198,49 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: participants = self._get_user_profiles(data["students"]) - evaluation, __ = Evaluation.objects.get_or_create( + defaults = { + "name_de": name_de, + "name_en": name_en, + "vote_start_datetime": evaluation_start_datetime, + "vote_end_date": evaluation_end_date, + "wait_for_grade_upload_before_publishing": wait_for_grade_upload_before_publishing, + } + evaluation, created = Evaluation.objects.get_or_create( course=course, cms_id=data["gguid"], - defaults={ - "name_de": name_de, - "name_en": name_en, - "vote_start_datetime": evaluation_start_datetime, - "vote_end_date": evaluation_end_date, - }, + defaults=defaults, ) if evaluation.state < Evaluation.State.APPROVED: - evaluation.name_de = name_de - evaluation.name_en = name_en - evaluation.vote_start_datetime = evaluation_start_datetime - evaluation.vote_end_date = evaluation_end_date - evaluation.wait_for_grade_upload_before_publishing = wait_for_grade_upload_before_publishing - evaluation.save() + direct_changes = update_with_changes(evaluation, defaults) + participant_changes = set(evaluation.participants.all()) != set(participants) evaluation.participants.set(participants) + lecturers_changes = False for lecturer in data["lecturers"]: - self._import_contribution(evaluation, lecturer) + __, lecturer_created, lecturer_changes = self._import_contribution(evaluation, lecturer) + if lecturer_changes or lecturer_created: + lecturers_changes = True + + if direct_changes or participant_changes or lecturers_changes: + self.statistics.updated_evaluations.append(evaluation) + else: + self.statistics.attempted_changes.append(evaluation) + + if created: + self.statistics.new_evaluations.append(evaluation) return evaluation def _import_contribution(self, evaluation: Evaluation, data: ImportRelated): user_profile = self.user_profile_map[data["gguid"]] - contribution, __ = Contribution.objects.update_or_create( + contribution, created, changes = update_or_create_with_changes( + Contribution, evaluation=evaluation, contributor=user_profile, ) - return contribution + return contribution, created, changes def _import_events(self, data: list[ImportEvent]): # Divide in two lists so corresponding courses are imported before their exams diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 8625e70ad8..0ab1cc7ec3 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -5,7 +5,7 @@ from model_bakery import baker from evap.evaluation.models import Contribution, Course, Evaluation, Questionnaire, Semester, UserProfile -from evap.staff.importers.json import ImportDict, JSONImporter +from evap.staff.importers.json import ImportDict, JSONImporter, NameChange EXAMPLE_DATA: ImportDict = { "students": [ @@ -75,10 +75,12 @@ def test_import_students(self): self.assertEqual(user_profile.last_name, self.students[i]["name"]) self.assertEqual(user_profile.first_name_given, self.students[i]["christianname"]) - def test_import_existing_students(self): + self.assertEqual(importer.statistics.name_changes, []) - user_profile = baker.make(UserProfile, email=self.students[0]["email"]) - print(user_profile.email) + def test_import_existing_students(self): + user_profile = baker.make( + UserProfile, email=self.students[0]["email"], last_name="Doe", first_name_given="Jane" + ) importer = JSONImporter(self.semester) importer._import_students(self.students) @@ -91,6 +93,18 @@ def test_import_existing_students(self): self.assertEqual(user_profile.last_name, self.students[0]["name"]) self.assertEqual(user_profile.first_name_given, self.students[0]["christianname"]) + self.assertEqual( + importer.statistics.name_changes, + [ + NameChange( + old_last_name="Doe", + old_first_name_given="Jane", + new_last_name=self.students[0]["name"], + new_first_name_given=self.students[0]["christianname"], + ) + ], + ) + def test_import_lecturers(self): self.assertEqual(UserProfile.objects.all().count(), 0) @@ -107,14 +121,12 @@ def test_import_lecturers(self): self.assertEqual(user_profile.title, self.lecturers[i]["titlefront"]) def test_import_existing_lecturers(self): - user_profile = baker.make(UserProfile, email=self.lecturers[0]["email"]) - print(user_profile.email) importer = JSONImporter(self.semester) importer._import_lecturers(self.lecturers) - assert UserProfile.objects.all().count() == 2 + self.assertEqual(UserProfile.objects.all().count(), 2) user_profile.refresh_from_db() @@ -198,6 +210,9 @@ def test_import_courses(self): [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][1]["lecturers"]], ) + self.assertEqual(len(importer.statistics.new_courses), 1) + self.assertEqual(len(importer.statistics.new_evaluations), 2) + def test_import_courses_evaluation_approved(self): self._import() @@ -206,11 +221,12 @@ def test_import_courses_evaluation_approved(self): evaluation.name_en = "Test" evaluation.save() - self._import() + importer = self._import() evaluation = Evaluation.objects.get(pk=evaluation.pk) self.assertEqual(evaluation.name_en, "") + self.assertEqual(len(importer.statistics.attempted_changes), 0) evaluation.general_contribution.questionnaires.add( baker.make(Questionnaire, type=Questionnaire.Type.CONTRIBUTOR) @@ -219,11 +235,29 @@ def test_import_courses_evaluation_approved(self): evaluation.name_en = "Test" evaluation.save() - self._import() + importer = self._import() evaluation = Evaluation.objects.get(pk=evaluation.pk) self.assertEqual(evaluation.name_en, "Test") + self.assertEqual(len(importer.statistics.attempted_changes), 1) + def test_import_courses_update(self): - pass + importer = self._import() + + self.assertEqual(Course.objects.all().count(), 1) + course = Course.objects.all()[0] + course.name_de = "Doe" + course.name_en = "Jane" + course.save() + + importer = self._import() + + course.refresh_from_db() + + self.assertEqual(course.name_de, EXAMPLE_DATA["events"][0]["title"]) + self.assertEqual(course.name_en, EXAMPLE_DATA["events"][0]["title_en"]) + + self.assertEqual(len(importer.statistics.updated_courses), 1) + self.assertEqual(len(importer.statistics.new_courses), 0) diff --git a/evap/staff/tools.py b/evap/staff/tools.py index ac289daa54..6cb9c4541c 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -8,7 +8,7 @@ from django.contrib.auth.models import Group from django.core.exceptions import SuspiciousOperation from django.db import transaction -from django.db.models import Count +from django.db.models import Count, Model from django.urls import reverse from django.utils.html import escape, format_html, format_html_join from django.utils.safestring import SafeString @@ -381,3 +381,38 @@ def user_edit_link(user_id): reverse("staff:user_edit", kwargs={"user_id": user_id}), _("edit user"), ) + + +def update_or_create_with_changes( + model: type[Model], + defaults=None, + **kwargs, +) -> tuple[Model, bool, dict[str, tuple[any, any]]]: + """Do update_or_create and track changed values.""" + + if not defaults: + defaults = {} + + obj, created = model.objects.get_or_create(**kwargs, defaults=defaults) + + if created: + return obj, True, {} + + changes = update_with_changes(obj, defaults) + + return obj, False, changes + + +def update_with_changes(obj: Model, defaults: dict[str, any]) -> dict[str, tuple[any, any]]: + """Update a model instance and track changed values.""" + + changes = {} + for key, value in defaults.items(): + if getattr(obj, key) != value: + changes[key] = (getattr(obj, key), value) + setattr(obj, key, value) + + if changes: + obj.save() + + return changes From 4914f980f4e960aa4730748538d3842c37b38fe7 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 17 Jun 2024 20:32:32 +0200 Subject: [PATCH 09/23] [JSON import] Add log handler for email sending --- evap/settings.py | 8 ++++ evap/staff/importers/json.py | 54 ++++++++++++++++++++++ evap/staff/log_handler.py | 27 +++++++++++ evap/staff/management/commands/__init__.py | 0 4 files changed, 89 insertions(+) create mode 100644 evap/staff/log_handler.py create mode 100644 evap/staff/management/commands/__init__.py diff --git a/evap/settings.py b/evap/settings.py index ac977bd37c..76800d66b0 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -181,6 +181,10 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): "level": "ERROR", "class": "django.utils.log.AdminEmailHandler", }, + "mail_managers": { + "level": "INFO", + "class": "evap.staff.log_handler.ManagerEmailHandler", + }, "console": { "class": "logging.StreamHandler", "formatter": "default", @@ -202,6 +206,10 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): "level": "DEBUG", "propagate": True, }, + "import": { + "handlers": ["console", "file", "mail_managers"], + "level": "INFO", + }, }, } diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 157384b326..ed3e60f391 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -1,13 +1,17 @@ +import logging from dataclasses import dataclass, field from datetime import datetime, timedelta from typing import TypedDict from django.db import transaction +from django.utils.timezone import now from evap.evaluation.models import Contribution, Course, CourseType, Degree, Evaluation, Semester, UserProfile from evap.evaluation.tools import clean_email from evap.staff.tools import update_or_create_with_changes, update_with_changes +logger = logging.getLogger("import") + class ImportStudent(TypedDict): gguid: str @@ -75,6 +79,51 @@ class ImportStatistics: updated_evaluations: list[Evaluation] = field(default_factory=list) attempted_changes: list[Evaluation] = field(default_factory=list) + @staticmethod + def _make_heading(heading: str) -> str: + heading += "\n" + "".join(["-" for i in heading]) + "\n" + return heading + + @staticmethod + def _make_total(total: int) -> str: + return f"({total} in total)\n\n" + + def get_log(self) -> str: + log = "JSON IMPORTER REPORT\n" + log += "====================\n\n" + log += f"Import finished at {now()}\n\n" + log += self._make_heading("Name Changes") + for name_change in self.name_changes: + log += f"- {name_change.old_first_name_given} {name_change.old_last_name} → {name_change.new_first_name_given} {name_change.new_last_name}\n" + log += self._make_total(len(self.name_changes)) + + log += self._make_heading("New Courses") + for new_course in self.new_courses: + log += f"- {new_course}\n" + log += self._make_total(len(self.new_courses)) + + log += self._make_heading("New Evaluations") + for new_evaluation in self.new_evaluations: + log += f"- {new_evaluation}\n" + log += self._make_total(len(self.new_evaluations)) + + log += self._make_heading("Updated Courses") + for updated_course in self.updated_courses: + log += f"- {updated_course}\n" + log += self._make_total(len(self.updated_courses)) + + log += self._make_heading("Updated Evaluations") + for updated_evaluation in self.updated_evaluations: + log += f"- {updated_evaluation}\n" + log += self._make_total(len(self.updated_evaluations)) + + log += self._make_heading("Attempted Changes") + for attempted_change in self.attempted_changes: + log += f"- {attempted_change}\n" + log += self._make_total(len(self.attempted_changes)) + + return log + class JSONImporter: DATETIME_FORMAT = "%d.%m.%Y %H:%M" @@ -261,8 +310,13 @@ def _import_events(self, data: list[ImportEvent]): self._import_evaluation(course, event) + def _process_log(self): + log = self.statistics.get_log() + logger.info(log) + @transaction.atomic def import_json(self, data: ImportDict): self._import_students(data["students"]) self._import_lecturers(data["lecturers"]) self._import_events(data["events"]) + self._process_log() diff --git a/evap/staff/log_handler.py b/evap/staff/log_handler.py new file mode 100644 index 0000000000..c5902d93d3 --- /dev/null +++ b/evap/staff/log_handler.py @@ -0,0 +1,27 @@ +from django.conf import settings +from django.core.mail import EmailMultiAlternatives +from django.utils.log import AdminEmailHandler + + +def mail_managers(subject, message, fail_silently=False, connection=None, html_message=None): + """Send a message to the managers, as defined by the MANAGERS setting.""" + from evap.evaluation.models import UserProfile + + managers = UserProfile.objects.filter(groups__name="Manager", email__isnull=False) + if not managers: + return + mail = EmailMultiAlternatives( + f"{settings.EMAIL_SUBJECT_PREFIX}{subject}", + message, + settings.SERVER_EMAIL, + [manager.email for manager in managers], + connection=connection, + ) + if html_message: + mail.attach_alternative(html_message, "text/html") + mail.send(fail_silently=fail_silently) + + +class ManagerEmailHandler(AdminEmailHandler): + def send_mail(self, subject, message, *args, **kwargs): + mail_managers(subject, message, *args, connection=self.connection(), **kwargs) diff --git a/evap/staff/management/commands/__init__.py b/evap/staff/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 From a886322bc86528ee35ca50fa6d3eb7df3f178c10 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 17 Jun 2024 20:32:59 +0200 Subject: [PATCH 10/23] [JSON import] Add management command for import --- evap/staff/management/commands/json_import.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 evap/staff/management/commands/json_import.py diff --git a/evap/staff/management/commands/json_import.py b/evap/staff/management/commands/json_import.py new file mode 100644 index 0000000000..24ccec5384 --- /dev/null +++ b/evap/staff/management/commands/json_import.py @@ -0,0 +1,34 @@ +import json +import logging + +from django.core.management.base import BaseCommand + +from evap.evaluation.management.commands.tools import log_exceptions +from evap.evaluation.models import Semester +from evap.staff.importers.json import JSONImporter + +logger = logging.getLogger(__name__) + + +@log_exceptions +class Command(BaseCommand): + help = "Import enrollments from JSON file." + + def add_arguments(self, parser): + # Positional arguments + parser.add_argument("semester", type=int) + parser.add_argument("file", type=str) + + def handle(self, *args, **options): + print(args, options) + try: + semester = Semester.objects.get(pk=options["semester"]) + except Semester.DoesNotExist: + self.stdout.write(self.style.ERROR("Semester does not exist.")) + return + + with open(options["file"]) as file: + + import_dict = json.load(file) + importer = JSONImporter(semester) + importer.import_json(import_dict) From a6c8309a752c843457faeb96c9ab39efcf79641d Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 17 Jun 2024 20:34:12 +0200 Subject: [PATCH 11/23] Fix log_exceptions to correctly pass args to handle --- evap/evaluation/management/commands/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/evaluation/management/commands/tools.py b/evap/evaluation/management/commands/tools.py index fef8f38d0a..ee07ec9e4a 100644 --- a/evap/evaluation/management/commands/tools.py +++ b/evap/evaluation/management/commands/tools.py @@ -35,7 +35,7 @@ def log_exceptions(cls): class NewClass(cls): def handle(self, *args, **options): try: - super().handle(args, options) + super().handle(*args, **options) except Exception: logger.exception("Management command '%s' failed. Traceback follows: ", sys.argv[1]) raise From 0ce1464f2c815a829b6dbc44de69082d741afd42 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 24 Jun 2024 19:45:36 +0200 Subject: [PATCH 12/23] Improve JSON importer code --- evap/staff/importers/json.py | 29 +++--- evap/staff/log_handler.py | 1 + evap/staff/tests/test_json_importer.py | 4 +- evap/staff/tools.py | 8 +- test_data.json | 117 +++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 test_data.json diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index ed3e60f391..b7055f5a0d 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -1,3 +1,4 @@ +import json import logging from dataclasses import dataclass, field from datetime import datetime, timedelta @@ -128,7 +129,7 @@ def get_log(self) -> str: class JSONImporter: DATETIME_FORMAT = "%d.%m.%Y %H:%M" - def __init__(self, semester: Semester): + def __init__(self, semester: Semester) -> None: self.semester = semester self.user_profile_map: dict[str, UserProfile] = {} self.course_type_cache: dict[str, CourseType] = {} @@ -155,7 +156,7 @@ def _get_degree(self, name: str) -> Degree: def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: return [self.user_profile_map[related["gguid"]] for related in data] - def _import_students(self, data: list[ImportStudent]): + def _import_students(self, data: list[ImportStudent]) -> None: for entry in data: email = clean_email(entry["email"]) user_profile, __, changes = update_or_create_with_changes( @@ -183,7 +184,7 @@ def _import_students(self, data: list[ImportStudent]): self.user_profile_map[entry["gguid"]] = user_profile - def _import_lecturers(self, data: list[ImportLecturer]): + def _import_lecturers(self, data: list[ImportLecturer]) -> None: for entry in data: email = clean_email(entry["email"]) user_profile, __ = UserProfile.objects.update_or_create( @@ -281,7 +282,9 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: return evaluation - def _import_contribution(self, evaluation: Evaluation, data: ImportRelated): + def _import_contribution( + self, evaluation: Evaluation, data: ImportRelated + ) -> tuple[Contribution, bool, dict[str, tuple[any, any]]]: user_profile = self.user_profile_map[data["gguid"]] contribution, created, changes = update_or_create_with_changes( @@ -291,32 +294,32 @@ def _import_contribution(self, evaluation: Evaluation, data: ImportRelated): ) return contribution, created, changes - def _import_events(self, data: list[ImportEvent]): + def _import_events(self, data: list[ImportEvent]) -> None: # Divide in two lists so corresponding courses are imported before their exams - normal_events = filter(lambda e: not e["isexam"], data) - exam_events = filter(lambda e: e["isexam"], data) + normal_events = (event for event in data if not event["isexam"]) + exam_events = (event for event in data if event["isexam"]) for event in normal_events: - event: ImportEvent - course = self._import_course(event) self._import_evaluation(course, event) for event in exam_events: - event: ImportEvent - course = self.course_map[event["relatedevents"]["gguid"]] self._import_evaluation(course, event) - def _process_log(self): + def _process_log(self) -> None: log = self.statistics.get_log() logger.info(log) @transaction.atomic - def import_json(self, data: ImportDict): + def import_dict(self, data: ImportDict) -> None: self._import_students(data["students"]) self._import_lecturers(data["lecturers"]) self._import_events(data["events"]) self._process_log() + + def import_json(self, data: str) -> None: + data = json.loads(data) + self.import_dict(data) diff --git a/evap/staff/log_handler.py b/evap/staff/log_handler.py index c5902d93d3..4f007645a6 100644 --- a/evap/staff/log_handler.py +++ b/evap/staff/log_handler.py @@ -5,6 +5,7 @@ def mail_managers(subject, message, fail_silently=False, connection=None, html_message=None): """Send a message to the managers, as defined by the MANAGERS setting.""" + # pylint: disable=import-outside-toplevel from evap.evaluation.models import UserProfile managers = UserProfile.objects.filter(groups__name="Manager", email__isnull=False) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 0ab1cc7ec3..a79e722670 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -85,7 +85,7 @@ def test_import_existing_students(self): importer = JSONImporter(self.semester) importer._import_students(self.students) - assert UserProfile.objects.all().count() == 2 + self.assertEqual(UserProfile.objects.all().count(), 2) user_profile.refresh_from_db() @@ -244,7 +244,7 @@ def test_import_courses_evaluation_approved(self): self.assertEqual(len(importer.statistics.attempted_changes), 1) def test_import_courses_update(self): - importer = self._import() + self._import() self.assertEqual(Course.objects.all().count(), 1) course = Course.objects.all()[0] diff --git a/evap/staff/tools.py b/evap/staff/tools.py index 6cb9c4541c..2d47bacd13 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -2,6 +2,7 @@ from collections.abc import Iterable from datetime import date, datetime, timedelta from enum import Enum +from typing import TypeVar from django.conf import settings from django.contrib import messages @@ -383,11 +384,14 @@ def user_edit_link(user_id): ) +T = TypeVar("T") + + def update_or_create_with_changes( - model: type[Model], + model: type[T], defaults=None, **kwargs, -) -> tuple[Model, bool, dict[str, tuple[any, any]]]: +) -> tuple[T, bool, dict[str, tuple[any, any]]]: """Do update_or_create and track changed values.""" if not defaults: diff --git a/test_data.json b/test_data.json new file mode 100644 index 0000000000..ac59f1419c --- /dev/null +++ b/test_data.json @@ -0,0 +1,117 @@ +{ + "students": [ + { + "gguid": "0x1", + "email": "1@example.com", + "name": "1", + "christianname": "1" + }, + { + "gguid": "0x2", + "email": "2@example.com", + "name": "2", + "christianname": "2" + } + ], + "lecturers": [ + { + "gguid": "0x3", + "email": "3@example.com", + "name": "3", + "christianname": "3", + "titlefront": "Prof. Dr." + }, + { + "gguid": "0x4", + "email": "4@example.com", + "name": "4", + "christianname": "4", + "titlefront": "Dr." + } + ], + "events": [ + { + "gguid": "0x5", + "lvnr": 1, + "title": "Prozessorientierte Informationssysteme", + "title_en": "Process-oriented information systems", + "type": "Vorlesung", + "isexam": false, + "courses": [ + { + "cprid": "BA-Inf", + "scale": "GRADE_PARTICIPATION" + }, + { + "cprid": "MA-Inf", + "scale": "GRADE_PARTICIPATION" + } + ], + "relatedevents": { + "gguid": "0x6" + }, + "appointments": [ + { + "begin": "15.04.2024 10:15", + "end": "15.07.2024 11:45" + } + ], + "lecturers": [ + { + "gguid": "0x3" + } + ], + "students": [ + { + "gguid": "0x1" + }, + { + "gguid": "0x2" + } + ] + }, + { + "gguid": "0x6", + "lvnr": 2, + "title": "Prozessorientierte Informationssysteme", + "title_en": "Process-oriented information systems", + "type": "Klausur", + "isexam": true, + "courses": [ + { + "cprid": "BA-Inf", + "scale": "" + }, + { + "cprid": "MA-Inf", + "scale": "" + } + ], + "relatedevents": { + "gguid": "0x5" + }, + "appointments": [ + { + "begin": "29.07.2024 10:15", + "end": "29.07.2024 11:45" + } + ], + "lecturers": [ + { + "gguid": "0x3" + }, + { + "gguid": "0x4" + } + ], + "students": [ + { + "gguid": "0x1" + }, + { + "gguid": "0x2" + } + ] + } + ] +} From cc0f0452bc0a03d722d6ec6a0175ab4c23d38df3 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 1 Jul 2024 18:07:28 +0200 Subject: [PATCH 13/23] Refactor and optimize JSON importer --- evap/staff/importers/json.py | 80 ++++++++----------- evap/staff/management/commands/json_import.py | 7 +- evap/staff/tests/test_json_importer.py | 3 +- 3 files changed, 34 insertions(+), 56 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index b7055f5a0d..27295caa75 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -81,47 +81,36 @@ class ImportStatistics: attempted_changes: list[Evaluation] = field(default_factory=list) @staticmethod - def _make_heading(heading: str) -> str: - heading += "\n" + "".join(["-" for i in heading]) + "\n" - return heading + def _make_heading(heading: str, separator: str = "-") -> str: + return "\n" + separator * len(heading) + "\n" @staticmethod def _make_total(total: int) -> str: return f"({total} in total)\n\n" + @staticmethod + def _make_stats(heading: str, new_objects: list) -> str: + log = ImportStatistics._make_heading(heading) + log += ImportStatistics._make_total(len(new_objects)) + for new_course in new_objects: + log += f"- {new_course}\n" + return log + def get_log(self) -> str: - log = "JSON IMPORTER REPORT\n" - log += "====================\n\n" + log = self._make_heading("JSON IMPORTER REPORT", "=") + log += "\n" log += f"Import finished at {now()}\n\n" + log += self._make_heading("Name Changes") + log += self._make_total(len(self.name_changes)) for name_change in self.name_changes: log += f"- {name_change.old_first_name_given} {name_change.old_last_name} → {name_change.new_first_name_given} {name_change.new_last_name}\n" - log += self._make_total(len(self.name_changes)) - - log += self._make_heading("New Courses") - for new_course in self.new_courses: - log += f"- {new_course}\n" - log += self._make_total(len(self.new_courses)) - - log += self._make_heading("New Evaluations") - for new_evaluation in self.new_evaluations: - log += f"- {new_evaluation}\n" - log += self._make_total(len(self.new_evaluations)) - - log += self._make_heading("Updated Courses") - for updated_course in self.updated_courses: - log += f"- {updated_course}\n" - log += self._make_total(len(self.updated_courses)) - - log += self._make_heading("Updated Evaluations") - for updated_evaluation in self.updated_evaluations: - log += f"- {updated_evaluation}\n" - log += self._make_total(len(self.updated_evaluations)) - log += self._make_heading("Attempted Changes") - for attempted_change in self.attempted_changes: - log += f"- {attempted_change}\n" - log += self._make_total(len(self.attempted_changes)) + log += self._make_stats("New Courses", self.new_courses) + log += self._make_stats("New Evaluations", self.new_evaluations) + log += self._make_stats("Updated Courses", self.updated_courses) + log += self._make_stats("Updated Evaluations", self.updated_evaluations) + log += self._make_stats("Attempted Changes", self.attempted_changes) return log @@ -164,7 +153,6 @@ def _import_students(self, data: list[ImportStudent]) -> None: email=email, defaults={"last_name": entry["name"], "first_name_given": entry["christianname"]}, ) - user_profile: UserProfile if changes: change = NameChange( old_last_name=changes["last_name"][0] if changes.get("last_name") else user_profile.last_name, @@ -173,12 +161,8 @@ def _import_students(self, data: list[ImportStudent]) -> None: if changes.get("first_name_given") else user_profile.first_name_given ), - new_last_name=changes["last_name"][1] if changes.get("last_name") else user_profile.last_name, - new_first_name_given=( - changes["first_name_given"][1] - if changes.get("first_name_given") - else user_profile.first_name_given - ), + new_last_name=user_profile.last_name, + new_first_name_given=user_profile.first_name_given, ) self.statistics.name_changes.append(change) @@ -208,7 +192,6 @@ def _import_course(self, data: ImportEvent) -> Course: cms_id=data["gguid"], defaults={"name_de": data["title"], "name_en": data["title_en"], "type": course_type}, ) - course: Course course.degrees.set(degrees) course.responsibles.set(responsibles) @@ -227,7 +210,9 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: if data["isexam"]: # Set evaluation time frame of three days for exam evaluations: - evaluation_start_datetime = course_end.replace(hour=8, minute=0) + timedelta(days=1) + evaluation_start_datetime = course_end.replace(hour=8, minute=0, second=0, microsecond=0) + timedelta( + days=1 + ) evaluation_end_date = (course_end + timedelta(days=3)).date() name_de = "Klausur" @@ -235,7 +220,7 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: else: # Set evaluation time frame of two weeks for normal evaluations: # Start datetime is at 8:00 am on the monday in the week before the event ends - evaluation_start_datetime = course_end.replace(hour=8, minute=0) - timedelta( + evaluation_start_datetime = course_end.replace(hour=8, minute=0, second=0, microsecond=0) - timedelta( weeks=1, days=course_end.weekday() ) # End date is on the sunday in the week the event ends @@ -244,7 +229,7 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: name_de, name_en = "", "" # If events are graded for any degree, wait for grade upload before publishing - wait_for_grade_upload_before_publishing = any(filter(lambda grade: grade["scale"], data["courses"])) + wait_for_grade_upload_before_publishing = any(grade["scale"] for grade in data["courses"]) participants = self._get_user_profiles(data["students"]) @@ -266,13 +251,13 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: participant_changes = set(evaluation.participants.all()) != set(participants) evaluation.participants.set(participants) - lecturers_changes = False + any_lecturers_changed = False for lecturer in data["lecturers"]: __, lecturer_created, lecturer_changes = self._import_contribution(evaluation, lecturer) if lecturer_changes or lecturer_created: - lecturers_changes = True + any_lecturers_changed = True - if direct_changes or participant_changes or lecturers_changes: + if direct_changes or participant_changes or any_lecturers_changed: self.statistics.updated_evaluations.append(evaluation) else: self.statistics.attempted_changes.append(evaluation) @@ -296,10 +281,10 @@ def _import_contribution( def _import_events(self, data: list[ImportEvent]) -> None: # Divide in two lists so corresponding courses are imported before their exams - normal_events = (event for event in data if not event["isexam"]) + non_exam_events = (event for event in data if not event["isexam"]) exam_events = (event for event in data if event["isexam"]) - for event in normal_events: + for event in non_exam_events: course = self._import_course(event) self._import_evaluation(course, event) @@ -321,5 +306,4 @@ def import_dict(self, data: ImportDict) -> None: self._process_log() def import_json(self, data: str) -> None: - data = json.loads(data) - self.import_dict(data) + self.import_dict(json.loads(data)) diff --git a/evap/staff/management/commands/json_import.py b/evap/staff/management/commands/json_import.py index 24ccec5384..3c7c7e1e78 100644 --- a/evap/staff/management/commands/json_import.py +++ b/evap/staff/management/commands/json_import.py @@ -1,4 +1,3 @@ -import json import logging from django.core.management.base import BaseCommand @@ -20,7 +19,6 @@ def add_arguments(self, parser): parser.add_argument("file", type=str) def handle(self, *args, **options): - print(args, options) try: semester = Semester.objects.get(pk=options["semester"]) except Semester.DoesNotExist: @@ -28,7 +26,4 @@ def handle(self, *args, **options): return with open(options["file"]) as file: - - import_dict = json.load(file) - importer = JSONImporter(semester) - importer.import_json(import_dict) + JSONImporter(semester).import_json(file.read()) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index a79e722670..9d154c0476 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -112,7 +112,6 @@ def test_import_lecturers(self): importer._import_lecturers(self.lecturers) user_profiles = UserProfile.objects.all() - self.assertEqual(user_profiles.count(), 2) for i, user_profile in enumerate(user_profiles): self.assertEqual(user_profile.email, self.lecturers[i]["email"]) @@ -142,7 +141,7 @@ def setUp(self): def _import(self): importer = JSONImporter(self.semester) - importer.import_json(EXAMPLE_DATA) + importer.import_json(EXAMPLE_JSON) return importer def test_import_courses(self): From 5d2b213d643f3f0e48776b0e66a9037f025c1ccd Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 1 Jul 2024 20:16:45 +0200 Subject: [PATCH 14/23] Test management command for JSON import --- evap/staff/tests/test_json_importer.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 9d154c0476..faf0db1216 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -1,6 +1,11 @@ import json +import os from datetime import date, datetime +from io import StringIO +from tempfile import TemporaryDirectory +from unittest.mock import patch +from django.core.management import call_command from django.test import TestCase from model_bakery import baker @@ -260,3 +265,15 @@ def test_import_courses_update(self): self.assertEqual(len(importer.statistics.updated_courses), 1) self.assertEqual(len(importer.statistics.new_courses), 0) + + @patch("evap.staff.importers.json.JSONImporter.import_json") + def test_management_command(self, mock_import_json): + output = StringIO() + + with TemporaryDirectory() as temp_dir: + test_filename = os.path.join(temp_dir, "test.json") + with open(test_filename, "w", encoding="utf-8") as f: + f.write(EXAMPLE_JSON) + call_command("json_import", self.semester.id, test_filename, stdout=output) + + mock_import_json.assert_called_once_with(EXAMPLE_JSON) From 5ab13759ed583ca2b0e309aedd389da939c69420 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 1 Jul 2024 20:16:55 +0200 Subject: [PATCH 15/23] Remove test_data.json --- test_data.json | 117 ------------------------------------------------- 1 file changed, 117 deletions(-) delete mode 100644 test_data.json diff --git a/test_data.json b/test_data.json deleted file mode 100644 index ac59f1419c..0000000000 --- a/test_data.json +++ /dev/null @@ -1,117 +0,0 @@ -{ - "students": [ - { - "gguid": "0x1", - "email": "1@example.com", - "name": "1", - "christianname": "1" - }, - { - "gguid": "0x2", - "email": "2@example.com", - "name": "2", - "christianname": "2" - } - ], - "lecturers": [ - { - "gguid": "0x3", - "email": "3@example.com", - "name": "3", - "christianname": "3", - "titlefront": "Prof. Dr." - }, - { - "gguid": "0x4", - "email": "4@example.com", - "name": "4", - "christianname": "4", - "titlefront": "Dr." - } - ], - "events": [ - { - "gguid": "0x5", - "lvnr": 1, - "title": "Prozessorientierte Informationssysteme", - "title_en": "Process-oriented information systems", - "type": "Vorlesung", - "isexam": false, - "courses": [ - { - "cprid": "BA-Inf", - "scale": "GRADE_PARTICIPATION" - }, - { - "cprid": "MA-Inf", - "scale": "GRADE_PARTICIPATION" - } - ], - "relatedevents": { - "gguid": "0x6" - }, - "appointments": [ - { - "begin": "15.04.2024 10:15", - "end": "15.07.2024 11:45" - } - ], - "lecturers": [ - { - "gguid": "0x3" - } - ], - "students": [ - { - "gguid": "0x1" - }, - { - "gguid": "0x2" - } - ] - }, - { - "gguid": "0x6", - "lvnr": 2, - "title": "Prozessorientierte Informationssysteme", - "title_en": "Process-oriented information systems", - "type": "Klausur", - "isexam": true, - "courses": [ - { - "cprid": "BA-Inf", - "scale": "" - }, - { - "cprid": "MA-Inf", - "scale": "" - } - ], - "relatedevents": { - "gguid": "0x5" - }, - "appointments": [ - { - "begin": "29.07.2024 10:15", - "end": "29.07.2024 11:45" - } - ], - "lecturers": [ - { - "gguid": "0x3" - }, - { - "gguid": "0x4" - } - ], - "students": [ - { - "gguid": "0x1" - }, - { - "gguid": "0x2" - } - ] - } - ] -} From a8793e116722e8cbffda703129b41d1bac02ab10 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 1 Jul 2024 20:31:08 +0200 Subject: [PATCH 16/23] Fix problems with JSON importer tests --- evap/staff/tests/test_json_importer.py | 42 +++++++++++++------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index faf0db1216..0a6f846b21 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -75,7 +75,7 @@ def test_import_students(self): user_profiles = UserProfile.objects.all() self.assertEqual(user_profiles.count(), 2) - for i, user_profile in enumerate(user_profiles): + for i, user_profile in enumerate(user_profiles.order_by("email")): self.assertEqual(user_profile.email, self.students[i]["email"]) self.assertEqual(user_profile.last_name, self.students[i]["name"]) self.assertEqual(user_profile.first_name_given, self.students[i]["christianname"]) @@ -118,7 +118,7 @@ def test_import_lecturers(self): user_profiles = UserProfile.objects.all() - for i, user_profile in enumerate(user_profiles): + for i, user_profile in enumerate(user_profiles.order_by("email")): self.assertEqual(user_profile.email, self.lecturers[i]["email"]) self.assertEqual(user_profile.last_name, self.lecturers[i]["name"]) self.assertEqual(user_profile.first_name_given, self.lecturers[i]["christianname"]) @@ -160,12 +160,12 @@ def test_import_courses(self): self.assertEqual(course.name_de, EXAMPLE_DATA["events"][0]["title"]) self.assertEqual(course.name_en, EXAMPLE_DATA["events"][0]["title_en"]) self.assertEqual(course.type.name_de, EXAMPLE_DATA["events"][0]["type"]) - self.assertListEqual( - [d.name_de for d in course.degrees.all()], [d["cprid"] for d in EXAMPLE_DATA["events"][0]["courses"]] + self.assertSetEqual( + {d.name_de for d in course.degrees.all()}, {d["cprid"] for d in EXAMPLE_DATA["events"][0]["courses"]} ) - self.assertListEqual( - list(course.responsibles.all()), - [importer.user_profile_map[lecturer["gguid"]] for lecturer in EXAMPLE_DATA["events"][0]["lecturers"]], + self.assertSetEqual( + set(course.responsibles.values_list("email", flat=True)), + {"3@example.com"}, ) main_evaluation = Evaluation.objects.get(name_en="") @@ -175,20 +175,20 @@ def test_import_courses(self): # [{"begin": "15.04.2024 10:15", "end": "15.07.2024 11:45"}] self.assertEqual(main_evaluation.vote_start_datetime, datetime(2024, 7, 8, 8, 0)) self.assertEqual(main_evaluation.vote_end_date, date(2024, 7, 21)) - self.assertListEqual( - list(main_evaluation.participants.all()), - [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][0]["students"]], + self.assertSetEqual( + set(main_evaluation.participants.values_list("email", flat=True)), + {"1@example.com", "2@example.com"}, ) self.assertTrue(main_evaluation.wait_for_grade_upload_before_publishing) self.assertEqual(Contribution.objects.filter(evaluation=main_evaluation).count(), 2) - self.assertListEqual( - list( + self.assertSetEqual( + set( Contribution.objects.filter(evaluation=main_evaluation, contributor__isnull=False).values_list( - "contributor_id", flat=True + "contributor__email", flat=True ) ), - [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][0]["lecturers"]], + {"3@example.com"}, ) exam_evaluation = Evaluation.objects.get(name_en="Exam") @@ -198,20 +198,20 @@ def test_import_courses(self): # [{"begin": "29.07.2024 10:15", "end": "29.07.2024 11:45"}] self.assertEqual(exam_evaluation.vote_start_datetime, datetime(2024, 7, 30, 8, 0)) self.assertEqual(exam_evaluation.vote_end_date, date(2024, 8, 1)) - self.assertListEqual( - list(exam_evaluation.participants.all()), - [importer.user_profile_map[student["gguid"]] for student in EXAMPLE_DATA["events"][1]["students"]], + self.assertSetEqual( + set(exam_evaluation.participants.values_list("email", flat=True)), + {"1@example.com", "2@example.com"}, ) self.assertFalse(exam_evaluation.wait_for_grade_upload_before_publishing) self.assertEqual(Contribution.objects.filter(evaluation=exam_evaluation).count(), 3) - self.assertListEqual( - list( + self.assertSetEqual( + set( Contribution.objects.filter(evaluation=exam_evaluation, contributor__isnull=False).values_list( - "contributor_id", flat=True + "contributor__email", flat=True ) ), - [importer.user_profile_map[student["gguid"]].id for student in EXAMPLE_DATA["events"][1]["lecturers"]], + {"3@example.com", "4@example.com"}, ) self.assertEqual(len(importer.statistics.new_courses), 1) From f2da64c7eb9e13b93f349ba49cdb02e24225a6bd Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 8 Jul 2024 18:38:48 +0200 Subject: [PATCH 17/23] [JSON importer] Also create name changes for lecturer changes --- evap/staff/importers/json.py | 28 +++++++++++++++----------- evap/staff/tests/test_json_importer.py | 18 ++++++++++++++++- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 27295caa75..c45aa22858 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -145,6 +145,17 @@ def _get_degree(self, name: str) -> Degree: def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: return [self.user_profile_map[related["gguid"]] for related in data] + def _create_name_change_from_changes(self, user_profile: UserProfile, changes: dict[str, tuple[any, any]]): + change = NameChange( + old_last_name=changes["last_name"][0] if changes.get("last_name") else user_profile.last_name, + old_first_name_given=( + changes["first_name_given"][0] if changes.get("first_name_given") else user_profile.first_name_given + ), + new_last_name=user_profile.last_name, + new_first_name_given=user_profile.first_name_given, + ) + self.statistics.name_changes.append(change) + def _import_students(self, data: list[ImportStudent]) -> None: for entry in data: email = clean_email(entry["email"]) @@ -154,24 +165,15 @@ def _import_students(self, data: list[ImportStudent]) -> None: defaults={"last_name": entry["name"], "first_name_given": entry["christianname"]}, ) if changes: - change = NameChange( - old_last_name=changes["last_name"][0] if changes.get("last_name") else user_profile.last_name, - old_first_name_given=( - changes["first_name_given"][0] - if changes.get("first_name_given") - else user_profile.first_name_given - ), - new_last_name=user_profile.last_name, - new_first_name_given=user_profile.first_name_given, - ) - self.statistics.name_changes.append(change) + self._create_name_change_from_changes(user_profile, changes) self.user_profile_map[entry["gguid"]] = user_profile def _import_lecturers(self, data: list[ImportLecturer]) -> None: for entry in data: email = clean_email(entry["email"]) - user_profile, __ = UserProfile.objects.update_or_create( + user_profile, __, changes = update_or_create_with_changes( + UserProfile, email=email, defaults={ "last_name": entry["name"], @@ -179,6 +181,8 @@ def _import_lecturers(self, data: list[ImportLecturer]) -> None: "title": entry["titlefront"], }, ) + if changes: + self._create_name_change_from_changes(user_profile, changes) self.user_profile_map[entry["gguid"]] = user_profile diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 0a6f846b21..20a6eca193 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -124,8 +124,12 @@ def test_import_lecturers(self): self.assertEqual(user_profile.first_name_given, self.lecturers[i]["christianname"]) self.assertEqual(user_profile.title, self.lecturers[i]["titlefront"]) + self.assertEqual(importer.statistics.name_changes, []) + def test_import_existing_lecturers(self): - user_profile = baker.make(UserProfile, email=self.lecturers[0]["email"]) + user_profile = baker.make( + UserProfile, email=self.lecturers[0]["email"], last_name="Doe", first_name_given="Jane" + ) importer = JSONImporter(self.semester) importer._import_lecturers(self.lecturers) @@ -139,6 +143,18 @@ def test_import_existing_lecturers(self): self.assertEqual(user_profile.first_name_given, self.lecturers[0]["christianname"]) self.assertEqual(user_profile.title, self.lecturers[0]["titlefront"]) + self.assertEqual( + importer.statistics.name_changes, + [ + NameChange( + old_last_name="Doe", + old_first_name_given="Jane", + new_last_name=self.lecturers[0]["name"], + new_first_name_given=self.lecturers[0]["christianname"], + ) + ], + ) + class TestImportEvents(TestCase): def setUp(self): From 2c23d53b33fbd431ac97d1f8ffd427810ec182c0 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 29 Jul 2024 18:23:56 +0200 Subject: [PATCH 18/23] Fix some code style issues --- evap/staff/importers/json.py | 12 ++++++------ evap/staff/tests/test_json_importer.py | 2 +- evap/staff/tools.py | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index c45aa22858..d5ff046d84 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -2,7 +2,7 @@ import logging from dataclasses import dataclass, field from datetime import datetime, timedelta -from typing import TypedDict +from typing import Any, TypedDict from django.db import transaction from django.utils.timezone import now @@ -89,11 +89,11 @@ def _make_total(total: int) -> str: return f"({total} in total)\n\n" @staticmethod - def _make_stats(heading: str, new_objects: list) -> str: + def _make_stats(heading: str, objects: list) -> str: log = ImportStatistics._make_heading(heading) - log += ImportStatistics._make_total(len(new_objects)) - for new_course in new_objects: - log += f"- {new_course}\n" + log += ImportStatistics._make_total(len(objects)) + for obj in objects: + log += f"- {obj}\n" return log def get_log(self) -> str: @@ -145,7 +145,7 @@ def _get_degree(self, name: str) -> Degree: def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: return [self.user_profile_map[related["gguid"]] for related in data] - def _create_name_change_from_changes(self, user_profile: UserProfile, changes: dict[str, tuple[any, any]]): + def _create_name_change_from_changes(self, user_profile: UserProfile, changes: dict[str, tuple[Any, Any]]) -> None: change = NameChange( old_last_name=changes["last_name"][0] if changes.get("last_name") else user_profile.last_name, old_first_name_given=( diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 20a6eca193..9a0dd23a54 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -169,7 +169,7 @@ def test_import_courses(self): importer = self._import() self.assertEqual(Course.objects.all().count(), 1) - course = Course.objects.all()[0] + course = Course.objects.first() self.assertEqual(course.semester, self.semester) self.assertEqual(course.cms_id, EXAMPLE_DATA["events"][0]["gguid"]) diff --git a/evap/staff/tools.py b/evap/staff/tools.py index 2d47bacd13..ad64a11b32 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -2,7 +2,7 @@ from collections.abc import Iterable from datetime import date, datetime, timedelta from enum import Enum -from typing import TypeVar +from typing import Any, TypeVar from django.conf import settings from django.contrib import messages @@ -391,7 +391,7 @@ def update_or_create_with_changes( model: type[T], defaults=None, **kwargs, -) -> tuple[T, bool, dict[str, tuple[any, any]]]: +) -> tuple[T, bool, dict[str, tuple[Any, Any]]]: """Do update_or_create and track changed values.""" if not defaults: @@ -407,7 +407,7 @@ def update_or_create_with_changes( return obj, False, changes -def update_with_changes(obj: Model, defaults: dict[str, any]) -> dict[str, tuple[any, any]]: +def update_with_changes(obj: Model, defaults: dict[str, any]) -> dict[str, tuple[Any, Any]]: """Update a model instance and track changed values.""" changes = {} From 2b3fa2cf0fa82cc7c7f0fd394d493594deefc387 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 29 Jul 2024 20:48:33 +0200 Subject: [PATCH 19/23] Add cms_id to excluded fields in copy form --- evap/staff/forms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 1a067e5ff4..159206f254 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -309,6 +309,7 @@ def __init__(self, data=None, *, instance: Course): "_voter_count", "voters", "votetimestamp", + "cms_id", } CONTRIBUTION_COPIED_FIELDS = { From 315f7b24f0d78c08f0c0191a4b99f3dcecb4da8f Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 5 Aug 2024 18:16:33 +0200 Subject: [PATCH 20/23] Fix headings in JSON importer --- evap/staff/importers/json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index d5ff046d84..4e9175a96d 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -82,7 +82,7 @@ class ImportStatistics: @staticmethod def _make_heading(heading: str, separator: str = "-") -> str: - return "\n" + separator * len(heading) + "\n" + return f"{heading}\n{separator * len(heading)}\n" @staticmethod def _make_total(total: int) -> str: From 73fb0f7b110e6be924dac5d739f40375b125aeec Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 7 Oct 2024 20:02:13 +0200 Subject: [PATCH 21/23] Fix migrations and model names after merge (Degree to Program) --- ...> 0147_course_cms_id_evaluation_cms_id.py} | 2 +- evap/staff/importers/json.py | 22 +++++++++---------- evap/staff/tests/test_json_importer.py | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) rename evap/evaluation/migrations/{0143_course_cms_id_evaluation_cms_id.py => 0147_course_cms_id_evaluation_cms_id.py} (92%) diff --git a/evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py b/evap/evaluation/migrations/0147_course_cms_id_evaluation_cms_id.py similarity index 92% rename from evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py rename to evap/evaluation/migrations/0147_course_cms_id_evaluation_cms_id.py index 851199d2f9..932db7ca95 100644 --- a/evap/evaluation/migrations/0143_course_cms_id_evaluation_cms_id.py +++ b/evap/evaluation/migrations/0147_course_cms_id_evaluation_cms_id.py @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ("evaluation", "0142_alter_evaluation_state"), + ("evaluation", "0146_grade_reminder_template"), ] operations = [ diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 4e9175a96d..9c4a03da40 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -7,7 +7,7 @@ from django.db import transaction from django.utils.timezone import now -from evap.evaluation.models import Contribution, Course, CourseType, Degree, Evaluation, Semester, UserProfile +from evap.evaluation.models import Contribution, Course, CourseType, Evaluation, Program, Semester, UserProfile from evap.evaluation.tools import clean_email from evap.staff.tools import update_or_create_with_changes, update_with_changes @@ -122,7 +122,7 @@ def __init__(self, semester: Semester) -> None: self.semester = semester self.user_profile_map: dict[str, UserProfile] = {} self.course_type_cache: dict[str, CourseType] = {} - self.degree_cache: dict[str, Degree] = {} + self.program_cache: dict[str, Program] = {} self.course_map: dict[str, Course] = {} self.statistics = ImportStatistics() @@ -134,13 +134,13 @@ def _get_course_type(self, name: str) -> CourseType: self.course_type_cache[name] = course_type return course_type - def _get_degree(self, name: str) -> Degree: - if name in self.degree_cache: - return self.degree_cache[name] + def _get_program(self, name: str) -> Program: + if name in self.program_cache: + return self.program_cache[name] - degree = Degree.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] - self.degree_cache[name] = degree - return degree + program = Program.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] + self.program_cache[name] = program + return program def _get_user_profiles(self, data: list[ImportRelated]) -> list[UserProfile]: return [self.user_profile_map[related["gguid"]] for related in data] @@ -188,7 +188,7 @@ def _import_lecturers(self, data: list[ImportLecturer]) -> None: def _import_course(self, data: ImportEvent) -> Course: course_type = self._get_course_type(data["type"]) - degrees = [self._get_degree(c["cprid"]) for c in data["courses"]] + programs = [self._get_program(c["cprid"]) for c in data["courses"]] responsibles = self._get_user_profiles(data["lecturers"]) course, created, changes = update_or_create_with_changes( Course, @@ -196,7 +196,7 @@ def _import_course(self, data: ImportEvent) -> Course: cms_id=data["gguid"], defaults={"name_de": data["title"], "name_en": data["title_en"], "type": course_type}, ) - course.degrees.set(degrees) + course.programs.set(programs) course.responsibles.set(responsibles) if changes: @@ -232,7 +232,7 @@ def _import_evaluation(self, course: Course, data: ImportEvent) -> Evaluation: name_de, name_en = "", "" - # If events are graded for any degree, wait for grade upload before publishing + # If events are graded for any program, wait for grade upload before publishing wait_for_grade_upload_before_publishing = any(grade["scale"] for grade in data["courses"]) participants = self._get_user_profiles(data["students"]) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 9a0dd23a54..648aa5e87e 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -177,7 +177,7 @@ def test_import_courses(self): self.assertEqual(course.name_en, EXAMPLE_DATA["events"][0]["title_en"]) self.assertEqual(course.type.name_de, EXAMPLE_DATA["events"][0]["type"]) self.assertSetEqual( - {d.name_de for d in course.degrees.all()}, {d["cprid"] for d in EXAMPLE_DATA["events"][0]["courses"]} + {d.name_de for d in course.programs.all()}, {d["cprid"] for d in EXAMPLE_DATA["events"][0]["courses"]} ) self.assertSetEqual( set(course.responsibles.values_list("email", flat=True)), From e4d0ccc94534cb5a043c431b10ac93bbbd070d06 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 7 Oct 2024 20:18:44 +0200 Subject: [PATCH 22/23] [JSON importer] Send useful log email --- evap/settings.py | 8 -------- evap/staff/importers/json.py | 27 +++++++++++++++++++------ evap/staff/log_handler.py | 28 -------------------------- evap/staff/tests/test_json_importer.py | 13 ++++++++++++ 4 files changed, 34 insertions(+), 42 deletions(-) delete mode 100644 evap/staff/log_handler.py diff --git a/evap/settings.py b/evap/settings.py index 8517f52ee8..6ed5f68754 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -192,10 +192,6 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): "level": "ERROR", "class": "django.utils.log.AdminEmailHandler", }, - "mail_managers": { - "level": "INFO", - "class": "evap.staff.log_handler.ManagerEmailHandler", - }, "console": { "class": "logging.StreamHandler", "formatter": "default", @@ -217,10 +213,6 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): "level": "DEBUG", "propagate": True, }, - "import": { - "handlers": ["console", "file", "mail_managers"], - "level": "INFO", - }, }, } diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index 9c4a03da40..d06a46ec98 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -4,6 +4,8 @@ from datetime import datetime, timedelta from typing import Any, TypedDict +from django.conf import settings +from django.core.mail import EmailMultiAlternatives from django.db import transaction from django.utils.timezone import now @@ -69,6 +71,7 @@ class NameChange: old_first_name_given: str new_last_name: str new_first_name_given: str + email: str @dataclass @@ -94,6 +97,7 @@ def _make_stats(heading: str, objects: list) -> str: log += ImportStatistics._make_total(len(objects)) for obj in objects: log += f"- {obj}\n" + log += "\n" return log def get_log(self) -> str: @@ -104,7 +108,7 @@ def get_log(self) -> str: log += self._make_heading("Name Changes") log += self._make_total(len(self.name_changes)) for name_change in self.name_changes: - log += f"- {name_change.old_first_name_given} {name_change.old_last_name} → {name_change.new_first_name_given} {name_change.new_last_name}\n" + log += f"- {name_change.old_first_name_given} {name_change.old_last_name} → {name_change.new_first_name_given} {name_change.new_last_name} (email: {name_change.email})\n" log += self._make_stats("New Courses", self.new_courses) log += self._make_stats("New Evaluations", self.new_evaluations) @@ -114,6 +118,20 @@ def get_log(self) -> str: return log + def send_mail(self): + subject = "[EvaP] JSON importer log" + + managers = UserProfile.objects.filter(groups__name="Manager", email__isnull=False) + if not managers: + return + mail = EmailMultiAlternatives( + subject, + self.get_log(), + settings.SERVER_EMAIL, + [manager.email for manager in managers], + ) + mail.send() + class JSONImporter: DATETIME_FORMAT = "%d.%m.%Y %H:%M" @@ -153,6 +171,7 @@ def _create_name_change_from_changes(self, user_profile: UserProfile, changes: d ), new_last_name=user_profile.last_name, new_first_name_given=user_profile.first_name_given, + email=user_profile.email, ) self.statistics.name_changes.append(change) @@ -298,16 +317,12 @@ def _import_events(self, data: list[ImportEvent]) -> None: self._import_evaluation(course, event) - def _process_log(self) -> None: - log = self.statistics.get_log() - logger.info(log) - @transaction.atomic def import_dict(self, data: ImportDict) -> None: self._import_students(data["students"]) self._import_lecturers(data["lecturers"]) self._import_events(data["events"]) - self._process_log() + self.statistics.send_mail() def import_json(self, data: str) -> None: self.import_dict(json.loads(data)) diff --git a/evap/staff/log_handler.py b/evap/staff/log_handler.py deleted file mode 100644 index 4f007645a6..0000000000 --- a/evap/staff/log_handler.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.conf import settings -from django.core.mail import EmailMultiAlternatives -from django.utils.log import AdminEmailHandler - - -def mail_managers(subject, message, fail_silently=False, connection=None, html_message=None): - """Send a message to the managers, as defined by the MANAGERS setting.""" - # pylint: disable=import-outside-toplevel - from evap.evaluation.models import UserProfile - - managers = UserProfile.objects.filter(groups__name="Manager", email__isnull=False) - if not managers: - return - mail = EmailMultiAlternatives( - f"{settings.EMAIL_SUBJECT_PREFIX}{subject}", - message, - settings.SERVER_EMAIL, - [manager.email for manager in managers], - connection=connection, - ) - if html_message: - mail.attach_alternative(html_message, "text/html") - mail.send(fail_silently=fail_silently) - - -class ManagerEmailHandler(AdminEmailHandler): - def send_mail(self, subject, message, *args, **kwargs): - mail_managers(subject, message, *args, connection=self.connection(), **kwargs) diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 648aa5e87e..4ff42932d5 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -5,11 +5,13 @@ from tempfile import TemporaryDirectory from unittest.mock import patch +from django.core import mail from django.core.management import call_command from django.test import TestCase from model_bakery import baker from evap.evaluation.models import Contribution, Course, Evaluation, Questionnaire, Semester, UserProfile +from evap.evaluation.tests.tools import make_manager from evap.staff.importers.json import ImportDict, JSONImporter, NameChange EXAMPLE_DATA: ImportDict = { @@ -106,6 +108,7 @@ def test_import_existing_students(self): old_first_name_given="Jane", new_last_name=self.students[0]["name"], new_first_name_given=self.students[0]["christianname"], + email=self.students[0]["email"], ) ], ) @@ -151,6 +154,7 @@ def test_import_existing_lecturers(self): old_first_name_given="Jane", new_last_name=self.lecturers[0]["name"], new_first_name_given=self.lecturers[0]["christianname"], + email=self.lecturers[0]["email"], ) ], ) @@ -282,6 +286,15 @@ def test_import_courses_update(self): self.assertEqual(len(importer.statistics.updated_courses), 1) self.assertEqual(len(importer.statistics.new_courses), 0) + def test_importer_log_email_sent(self): + manager = make_manager() + + self._import() + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].subject, "[EvaP] JSON importer log") + self.assertEqual(mail.outbox[0].recipients(), [manager.email]) + @patch("evap.staff.importers.json.JSONImporter.import_json") def test_management_command(self, mock_import_json): output = StringIO() From af64f843d6c235f1da2aa3cca388cc8c8ede0a13 Mon Sep 17 00:00:00 2001 From: Jonathan Weth Date: Mon, 7 Oct 2024 20:27:15 +0200 Subject: [PATCH 23/23] Improve code style in JSON importer (tests) --- evap/staff/importers/json.py | 4 ++-- evap/staff/tests/test_json_importer.py | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/evap/staff/importers/json.py b/evap/staff/importers/json.py index d06a46ec98..0a69730b65 100644 --- a/evap/staff/importers/json.py +++ b/evap/staff/importers/json.py @@ -148,7 +148,7 @@ def _get_course_type(self, name: str) -> CourseType: if name in self.course_type_cache: return self.course_type_cache[name] - course_type = CourseType.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] + course_type, __ = CourseType.objects.get_or_create(name_de=name, defaults={"name_en": name}) self.course_type_cache[name] = course_type return course_type @@ -156,7 +156,7 @@ def _get_program(self, name: str) -> Program: if name in self.program_cache: return self.program_cache[name] - program = Program.objects.get_or_create(name_de=name, defaults={"name_en": name})[0] + program, __ = Program.objects.get_or_create(name_de=name, defaults={"name_en": name}) self.program_cache[name] = program return program diff --git a/evap/staff/tests/test_json_importer.py b/evap/staff/tests/test_json_importer.py index 4ff42932d5..e82ab17130 100644 --- a/evap/staff/tests/test_json_importer.py +++ b/evap/staff/tests/test_json_importer.py @@ -69,13 +69,12 @@ def setUp(self): self.semester = baker.make(Semester) def test_import_students(self): - self.assertEqual(UserProfile.objects.all().count(), 0) + self.assertEqual(UserProfile.objects.count(), 0) importer = JSONImporter(self.semester) importer._import_students(self.students) user_profiles = UserProfile.objects.all() - self.assertEqual(user_profiles.count(), 2) for i, user_profile in enumerate(user_profiles.order_by("email")): self.assertEqual(user_profile.email, self.students[i]["email"]) @@ -92,7 +91,7 @@ def test_import_existing_students(self): importer = JSONImporter(self.semester) importer._import_students(self.students) - self.assertEqual(UserProfile.objects.all().count(), 2) + self.assertEqual(UserProfile.objects.count(), 2) user_profile.refresh_from_db() @@ -114,7 +113,7 @@ def test_import_existing_students(self): ) def test_import_lecturers(self): - self.assertEqual(UserProfile.objects.all().count(), 0) + self.assertEqual(UserProfile.objects.count(), 0) importer = JSONImporter(self.semester) importer._import_lecturers(self.lecturers) @@ -137,7 +136,7 @@ def test_import_existing_lecturers(self): importer = JSONImporter(self.semester) importer._import_lecturers(self.lecturers) - self.assertEqual(UserProfile.objects.all().count(), 2) + self.assertEqual(UserProfile.objects.count(), 2) user_profile.refresh_from_db() @@ -172,7 +171,7 @@ def _import(self): def test_import_courses(self): importer = self._import() - self.assertEqual(Course.objects.all().count(), 1) + self.assertEqual(Course.objects.count(), 1) course = Course.objects.first() self.assertEqual(course.semester, self.semester) @@ -270,7 +269,7 @@ def test_import_courses_evaluation_approved(self): def test_import_courses_update(self): self._import() - self.assertEqual(Course.objects.all().count(), 1) + self.assertEqual(Course.objects.count(), 1) course = Course.objects.all()[0] course.name_de = "Doe" course.name_en = "Jane"