From ddf0d264fff90b6c09d4c354016a9ef0e45494ae Mon Sep 17 00:00:00 2001 From: Johan Seto Kaiba <51926076+johanseto@users.noreply.github.com> Date: Mon, 28 Oct 2024 18:28:38 -0500 Subject: [PATCH] feat: add engines model (#219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add engines model feat: add test utils feat: add tests * chore: apply suggestions from code review Co-authored-by: Andrey Cañon <36200299+andrey-canon@users.noreply.github.com> * fix: unify ead and rti courses count * fix: one to one correct relationship reference * chore: pr recommendation --- .../0012_add_pearson_engines_register.py | 29 +++ eox_nelp/pearson_vue/admin.py | 35 +++- eox_nelp/pearson_vue/models.py | 175 ++++++++++++++++++ eox_nelp/pearson_vue/tests/test_models.py | 157 ++++++++++++++++ eox_nelp/pearson_vue/tests/test_utils.py | 115 +++++++++++- eox_nelp/pearson_vue/utils.py | 15 ++ 6 files changed, 524 insertions(+), 2 deletions(-) create mode 100644 eox_nelp/migrations/0012_add_pearson_engines_register.py create mode 100644 eox_nelp/pearson_vue/tests/test_models.py diff --git a/eox_nelp/migrations/0012_add_pearson_engines_register.py b/eox_nelp/migrations/0012_add_pearson_engines_register.py new file mode 100644 index 00000000..f1c3f10f --- /dev/null +++ b/eox_nelp/migrations/0012_add_pearson_engines_register.py @@ -0,0 +1,29 @@ +# Generated by Django 3.2.25 on 2024-09-30 18:02 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('eox_nelp', '0011_pearsonrtenevent_course'), + ] + + operations = [ + migrations.CreateModel( + name='PearsonEngine', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('rti_triggers', models.PositiveIntegerField(default=0)), + ('cdd_triggers', models.PositiveIntegerField(default=0)), + ('ead_triggers', models.PositiveIntegerField(default=0)), + ('courses', models.JSONField(default=dict)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('user', models.OneToOneField(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/eox_nelp/pearson_vue/admin.py b/eox_nelp/pearson_vue/admin.py index c42cc5d4..4b39948b 100644 --- a/eox_nelp/pearson_vue/admin.py +++ b/eox_nelp/pearson_vue/admin.py @@ -9,7 +9,7 @@ from django.contrib import admin from django.contrib.auth import get_user_model -from eox_nelp.pearson_vue.models import PearsonRTENEvent +from eox_nelp.pearson_vue.models import PearsonEngine, PearsonRTENEvent User = get_user_model() @@ -27,4 +27,37 @@ class PearsonRTENEventAdmin(admin.ModelAdmin): list_filter = ["event_type"] +class PearsonEngineAdmin(admin.ModelAdmin): + """Admin class for PearsonEngineAdmin. + + attributes: + list_display: Fields to be shown in admin interface. + search_fields: fields that use to search and find matches. + """ + @admin.display(ordering="user__username", description="User") + def get_user_username(self, obj) -> str: + """Return username from User instance""" + return obj.user.username + + list_display = ( + "get_user_username", + "rti_triggers", + "cdd_triggers", + "ead_triggers", + "courses", + "created_at", + "updated_at", + ) + search_fields = ("user__username",) + raw_id_fields = ("user",) + ordering = ("-updated_at",) + readonly_fields = ( + "rti_triggers", + "cdd_triggers", + "ead_triggers", + "courses", + ) + + admin.site.register(PearsonRTENEvent, PearsonRTENEventAdmin) +admin.site.register(PearsonEngine, PearsonEngineAdmin) diff --git a/eox_nelp/pearson_vue/models.py b/eox_nelp/pearson_vue/models.py index a9ed6361..d2aea22a 100644 --- a/eox_nelp/pearson_vue/models.py +++ b/eox_nelp/pearson_vue/models.py @@ -5,6 +5,7 @@ Classes: PearsonRTENEvent: A model representing an event for the Pearson VUE RTEN service. + PearsonEngine: A model representing the send to the PearsonEngine service per user. Constants: EVENT_TYPE_CHOICES: A list of tuples representing the possible choices @@ -52,3 +53,177 @@ class PearsonRTENEvent(models.Model): event_type = models.CharField(max_length=20, choices=EVENT_TYPE_CHOICES) candidate = models.ForeignKey(User, null=True, on_delete=models.SET_NULL) course = models.ForeignKey(CourseOverview, null=True, on_delete=models.DO_NOTHING) + + +class PearsonEngine(models.Model): + """ + This model contains data related to user and the integration with PearsonVue engine service. + + Attributes: + user (User): OneToOne relationship with User model. + rti_triggers (int): Number of RTI triggers. + cdd_triggers (int): Number of CDD triggers. + ead_triggers (int): Number of EAD triggers. + courses (dict): JSON field storing EAD course data. + created_at (datetime): Timestamp of model instance creation. + updated_at (datetime): Timestamp of last model instance update. + """ + + user = models.OneToOneField(User, on_delete=models.CASCADE, null=True) + rti_triggers = models.PositiveIntegerField(default=0) + cdd_triggers = models.PositiveIntegerField(default=0) + ead_triggers = models.PositiveIntegerField(default=0) + courses = models.JSONField(default=dict) + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + + def get_triggers(self, trigger_type): + """ + Get the number of triggers for a specific type. + + Args: + trigger_type (str): The type of trigger ('rti', 'cdd', or 'ead'). + + Returns: + int: The number of triggers for the specified type. + + Raises: + ValueError: If an invalid trigger type is provided. + """ + trigger_name = f"{trigger_type}_triggers" + + if not hasattr(self, trigger_name): + raise ValueError(f"Invalid trigger name: {trigger_name}") + + return getattr(self, trigger_name, None) + + def set_triggers(self, trigger_type, value): + """ + Set the number of triggers for a specific type. + + Args: + trigger_type (str): The type of trigger ('rti', 'cdd', or 'ead'). + value (int): The new value for the trigger count. + + Raises: + ValueError: If the value is not a non-negative integer or if an invalid trigger type is provided. + """ + trigger_name = f"{trigger_type}_triggers" + + if not isinstance(value, int) or value < 0: + raise ValueError("Trigger value must be a non-negative integer") + + if not hasattr(self, trigger_name): + raise ValueError(f"Invalid trigger name: {trigger_name}") + + setattr(self, trigger_name, value) + self.save() + + def increment_trigger(self, trigger_type, increment=1): + """ + Increment the number of triggers for a specific type. + + Args: + trigger_type (str): The type of trigger ('rti', 'cdd', or 'ead'). + increment (int, optional): The amount to increment. Defaults to 1. + """ + current_value = self.get_triggers(trigger_type) + self.set_triggers(trigger_type, current_value + increment) + + def get_courses(self): + """ + Get the courses for a specific action type. + + Returns: + dict: The courses for the specified action type. + + """ + return self.courses + + def get_course_value(self, course_id): + """ + Get the value of a specific course. + + Args: + course_id (str): The ID of the course. + + Returns: + int: The value of the specified course, or 0 if not found. + """ + courses = self.get_courses() + return courses.get(course_id, 0) + + def set_course_value(self, course_id, value): + """ + Set the value of a specific course. + + Args: + course_id (str): The ID of the course. + value (int or float): The new value for the course. + + Raises: + ValueError: If the value is not a non-negative number. + """ + if not isinstance(value, (int, float)) or value < 0: + raise ValueError("Course value must be a non-negative number") + courses = self.get_courses() + courses[course_id] = value + self.courses = courses + self.save() + + def increment_course_value(self, course_id, increment=1): + """ + Increment the value of a specific course. + + Args: + course_id (str): The ID of the course. + increment (int or float, optional): The amount to increment. Defaults to 1. + """ + current_value = self.get_course_value(course_id) + self.set_course_value(course_id, current_value + increment) + + def remove_course(self, course_id): + """ + Remove a course from the specified action type. + + Args: + course_id (str): The ID of the course to remove. + + Raises: + ValueError: If the course is not found in the specified action type. + """ + courses = self.get_courses() + if course_id in courses: + del courses[course_id] + self.courses = courses + self.save() + else: + raise ValueError(f"Course {course_id} not found in courses") + + def get_course_ids(self): + """ + Get all course IDs for a specific action type. + + Returns: + list: A list of all course IDs for the specified action type. + """ + return list(self.get_courses().keys()) + + def get_total_course_value(self): + """ + Get the total value of all courses for a specific action type. + + Returns: + float: The sum of all course values for the specified action type. + """ + return sum(self.get_courses().values()) + + @property + def total_triggers(self): + """ + Get the total number of triggers across all types. + + Returns: + int: The sum of RTI, CDD, and EAD triggers. + """ + return self.rti_triggers + self.cdd_triggers + self.ead_triggers diff --git a/eox_nelp/pearson_vue/tests/test_models.py b/eox_nelp/pearson_vue/tests/test_models.py new file mode 100644 index 00000000..171e144f --- /dev/null +++ b/eox_nelp/pearson_vue/tests/test_models.py @@ -0,0 +1,157 @@ +"""This file contains all the test for the pearson vue models.py file.""" +from django.contrib.auth import get_user_model +from django.test import TestCase + +from eox_nelp.pearson_vue.models import PearsonEngine + +User = get_user_model() + + +class PearsonEngineTestCase(TestCase): + """ + Test case for the PearsonEngine model. + + This test suite covers the functionality of the PearsonEngine model, + including its methods for managing triggers and courses. + + Attributes: + user (User): A test user for the PearsonEngine instances. + engine (PearsonEngine): A PearsonEngine instance for testing. + + """ + # pylint: disable=no-member + def setUp(self): + """ + Set up the test environment before each test method. + + Creates a test user and a PearsonEngine instance associated with that user. + """ + self.user, _ = User.objects.get_or_create(username='test-user', password='12345') + self.engine, _ = PearsonEngine.objects.get_or_create(user=self.user) + + def test_trigger_operations(self): + """ + Test trigger-related operations. + + Tests getting, setting, and incrementing triggers for different types. + + Expected behavior: + - get_triggers should return the correct value for each trigger type. + - set_triggers should update the trigger value correctly. + - increment_trigger should increase the trigger value by the specified amount. + - Invalid trigger types should raise ValueError. + - Setting negative trigger values should raise ValueError. + """ + # Test initial values + self.assertEqual(self.engine.get_triggers('rti'), 0) + self.assertEqual(self.engine.get_triggers('cdd'), 0) + self.assertEqual(self.engine.get_triggers('ead'), 0) + + # Test setting triggers + self.engine.set_triggers('rti', 5) + self.assertEqual(self.engine.get_triggers('rti'), 5) + + # Test incrementing triggers + self.engine.increment_trigger('cdd', 3) + self.assertEqual(self.engine.get_triggers('cdd'), 3) + + # Test invalid trigger type + with self.assertRaises(ValueError): + self.engine.get_triggers('invalid') + + # Test setting negative value + with self.assertRaises(ValueError): + self.engine.set_triggers('ead', -1) + + # Test setting invalid value + with self.assertRaises(ValueError): + self.engine.set_triggers('invalid', 1) + + def test_course_operations(self): + """ + Test course-related operations. + + Tests getting, setting, incrementing, and removing course values. + + Expected behavior: + - get_courses should return the correct dictionary for each action type. + - get_course_value should return the correct value for a specific course. + - set_course_value should update the course value correctly. + - increment_course_value should increase the course value by the specified amount. + - remove_course should delete the course from the dictionary. ALso invalid type raise Valuerror. + - Invalid action types should raise ValueError. + - Setting negative course values should raise ValueError. + """ + # Test setting and getting course values + self.engine.set_course_value('course1', 10) + self.engine.set_course_value('course33', 44) + self.assertEqual(self.engine.get_course_value('course1'), 10) + + # Test incrementing course values + self.engine.increment_course_value('course1', 5) + self.assertEqual(self.engine.get_course_value('course1'), 15) + + # Test removing courses + self.engine.remove_course('course1') + self.engine.remove_course('course33') + with self.assertRaises(ValueError): + self.engine.remove_course('course1') + + def test_course_aggregation(self): + """ + Test course aggregation methods. + + Tests getting course IDs and total course values. + + Expected behavior: + - get_course_ids should return a list of all course IDs for a specific action. + - get_total_course_value should return the sum of all course values for a specific action. + """ + self.engine.set_course_value('course1', 10) + self.engine.set_course_value('course2', 20) + self.engine.set_course_value('course3', 30) + + self.assertEqual(set(self.engine.get_course_ids()), {'course1', 'course2', 'course3'}) + self.assertEqual(self.engine.get_total_course_value(), 60) + + def test_total_triggers_property(self): + """ + Test the total_triggers property. + + Expected behavior: + - total_triggers should return the sum of all trigger types. + """ + self.engine.set_triggers('rti', 5) + self.engine.set_triggers('cdd', 3) + self.engine.set_triggers('ead', 2) + self.assertEqual(self.engine.total_triggers, 10) + + def test_model_creation(self): + """ + Test PearsonEngine model creation and default values. + + Expected behavior: + - A new PearsonEngine instance should be created with default values. + - The created_at and updated_at fields should be automatically set. + """ + PearsonEngine.objects.all().delete() + new_engine = PearsonEngine.objects.create(user=self.user) + self.assertEqual(new_engine.rti_triggers, 0) + self.assertEqual(new_engine.cdd_triggers, 0) + self.assertEqual(new_engine.ead_triggers, 0) + self.assertEqual(new_engine.courses, {}) + self.assertIsNotNone(new_engine.created_at) + self.assertIsNotNone(new_engine.updated_at) + + def test_user_relationship(self): + """ + Test the relationship between PearsonEngine and User models. + + Expected behavior: + - Each PearsonEngine instance should be associated with a unique User. + - Deleting a User should delete the associated PearsonEngine instance (CASCADE). + """ + self.assertEqual(self.engine.user, self.user) + User.objects.get(id=self.user.id).delete() + with self.assertRaises(PearsonEngine.DoesNotExist): + PearsonEngine.objects.get(id=self.engine.id) diff --git a/eox_nelp/pearson_vue/tests/test_utils.py b/eox_nelp/pearson_vue/tests/test_utils.py index d36c0d8c..23e89ba1 100644 --- a/eox_nelp/pearson_vue/tests/test_utils.py +++ b/eox_nelp/pearson_vue/tests/test_utils.py @@ -13,7 +13,13 @@ from eox_nelp.edxapp_wrapper.student import AnonymousUserId, CourseEnrollment from eox_nelp.pearson_vue.constants import PAYLOAD_CDD, PAYLOAD_EAD -from eox_nelp.pearson_vue.utils import generate_client_authorization_id, is_cp1252, update_xml_with_dict +from eox_nelp.pearson_vue.models import PearsonEngine +from eox_nelp.pearson_vue.utils import ( + generate_client_authorization_id, + is_cp1252, + update_user_engines, + update_xml_with_dict, +) User = get_user_model() @@ -451,3 +457,110 @@ def test_special_chars_false(self): text = "This string has ©®€ symbols" self.assertFalse(is_cp1252(text)) + + +class TestUpdateUserEngineCustomForm(TestCase): + """Class to test update_user_engines function""" + # pylint: disable=no-member + + def test_creates_pearson_engine_if_none_exists(self): + """Tests if a `PearsonEngine` instance is created if it doesn't exist. + + Expected Behavior: + + - If a `PearsonEngine` instance does not exist for the given user, + a new instance should be created and associated with the user. + """ + + user = User.objects.create(username="not_exists_rti_user") + + update_user_engines(user, "cdd") + + self.assertIsInstance(user.pearsonengine, PearsonEngine) + + def test_increments_trigger_for_cdd(self): + """Tests if the `increment_trigger` method is called and the trigger count is incremented. + Expected Behavior: + - The `cdd_triggers` attribute of the `PearsonEngine` instance should be incremented by 1. + - Other colums would not be affected + """ + initial_cdd_count = 12 + user = User.objects.create(username="incrementcdd") + PearsonEngine.objects.create(user=user, cdd_triggers=initial_cdd_count) + + update_user_engines(user, "cdd") + + user.pearsonengine.refresh_from_db() + self.assertEqual(user.pearsonengine.cdd_triggers, initial_cdd_count + 1) + self.assertEqual(user.pearsonengine.ead_triggers, 0) + self.assertEqual(user.pearsonengine.rti_triggers, 0) + self.assertDictEqual(user.pearsonengine.courses, {}) + + def test_increments_course_value_for_ead(self): + """Tests if the `increment_course_value` method is called for "ead" actions + and the course count is incremented. + Expected Behavior: + - The `rti_triggers` attribute of the `PearsonEngine` instance should be incremented by 1. + - The course dict would be increment by one in the desired course. + - Other colums would not be affected + """ + initial_ead_count = 23 + course_id = "course-v1:test+awesome" + initial_course_id_count = 99 + user = User.objects.create(username="incrementead") + PearsonEngine.objects.create( + user=user, + ead_triggers=initial_ead_count, + courses={ + course_id: initial_course_id_count + } + ) + + update_user_engines(user, "ead", course_id) + user.pearsonengine.refresh_from_db() + + self.assertEqual(user.pearsonengine.ead_triggers, initial_ead_count + 1) + self.assertEqual(user.pearsonengine.cdd_triggers, 0) + self.assertEqual(user.pearsonengine.rti_triggers, 0) + self.assertEqual(user.pearsonengine.courses[course_id], initial_course_id_count + 1) + + def test_increments_course_value_for_rti(self): + """Tests if the `increment_course_value` method is called for "rti" actions + and the course count is incremented. + Test not previous pearson engine one-one-relation instance but created. + Expected Behavior: + - The `rti_triggers` attribute of the `PearsonEngine` instance should be incremented by 1. + - The course dict would be increment by one in the desired course. + - Other colums would not be affected + """ + user = User.objects.create(username="incrementrti") + course_id = "course-v1:test+awesome" + + update_user_engines(user, "rti", course_id) + + user.pearsonengine.refresh_from_db() + self.assertEqual(user.pearsonengine.rti_triggers, 1) + self.assertEqual(user.pearsonengine.ead_triggers, 0) + self.assertEqual(user.pearsonengine.cdd_triggers, 0) + self.assertEqual(user.pearsonengine.courses[course_id], 1) + + def test_does_not_increment_course_value_for_other_actions(self): + """Tests if the `increment_course_value` method is not called for other action names. + Expected Behavior: + + - If the `action_name` is not "ead", "rti" or "cdd", raise ValueError. + - The pearsonengine attributes should remain unchanged. + """ + user = User.objects.create( + username="otheactionincrement", + pearsonengine=PearsonEngine.objects.create() + ) + course_id = "course-v1:test+awesome" + + with self.assertRaises(ValueError): + update_user_engines(user, "other_action", course_id) + + self.assertEqual(user.pearsonengine.cdd_triggers, 0) + self.assertEqual(user.pearsonengine.ead_triggers, 0) + self.assertEqual(user.pearsonengine.rti_triggers, 0) + self.assertDictEqual(user.pearsonengine.courses, {}) diff --git a/eox_nelp/pearson_vue/utils.py b/eox_nelp/pearson_vue/utils.py index 17d655d6..2aa3de97 100644 --- a/eox_nelp/pearson_vue/utils.py +++ b/eox_nelp/pearson_vue/utils.py @@ -8,6 +8,7 @@ from pydantic.v1.utils import deep_update from eox_nelp.edxapp_wrapper.student import AnonymousUserId, CourseEnrollment, anonymous_id_for_user +from eox_nelp.pearson_vue.models import PearsonEngine # pylint: disable=import-outside-toplevel def update_xml_with_dict(xml: str, update_dict: dict) -> str: @@ -52,3 +53,17 @@ def is_cp1252(text): cp1252_regex = r'^[\x00-\x7F\x80-\x9F\xA0-\xFF]*$' return re.match(cp1252_regex, text) is not None + + +def update_user_engines(user, action_type, course_id=None): + """Update engines model using pearson vue actions. + + Args: + user (User): User instance relation to update engine. + action_type (str): The type of action to trigger ('rti', 'cdd', or 'ead'). + course_id (str): course_id to add update. Defaults to None. + """ + pearson_engine, _ = PearsonEngine.objects.get_or_create(user=user) # pylint: disable=no-member + pearson_engine.increment_trigger(action_type) + if course_id: + pearson_engine.increment_course_value(course_id)