From 8f24262b84e8b00362aa257b258ebd587c701ad5 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Wed, 27 Aug 2025 15:11:19 +0500 Subject: [PATCH] feat: implement course access role history table --- common/djangoapps/student/admin.py | 126 +++++++++ .../0047_courseaccessrolehistory.py | 37 +++ common/djangoapps/student/models/user.py | 92 ++++++- common/djangoapps/student/tests/test_roles.py | 248 ++++++++++++++++++ 4 files changed, 502 insertions(+), 1 deletion(-) create mode 100644 common/djangoapps/student/migrations/0047_courseaccessrolehistory.py diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 677b74b1bf94..151c6ab5b8ca 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -36,6 +36,7 @@ BulkChangeEnrollmentConfiguration, BulkUnenrollConfiguration, CourseAccessRole, + CourseAccessRoleHistory, CourseEnrollment, CourseEnrollmentAllowed, CourseEnrollmentCelebration, @@ -229,6 +230,131 @@ def save_model(self, request, obj, form, change): super().save_model(request, obj, form, change) +@admin.register(CourseAccessRoleHistory) +class CourseAccessRoleHistoryAdmin(admin.ModelAdmin): + """Admin panel for the Course Access Role History.""" + list_display = ( + 'id', 'user', 'org', 'course_id', 'role', 'action_type', 'changed_by', 'created' + ) + list_filter = ( + 'action_type', 'org', 'role' + ) + search_fields = ( + 'user__username', 'org', 'course_id', 'role', 'action_type', 'changed_by__username' + ) + readonly_fields = ( + 'user', 'org', 'course_id', 'role', 'action_type', 'changed_by', 'created', 'modified' + ) + actions = ['revert_selected_history', 'delete_selected_history_entries'] + + def has_add_permission(self, request): + return False + + def has_change_permission(self, request, obj=None): + return False + + def has_delete_permission(self, request, obj=None): + return False + + def revert_selected_history(self, request, queryset): + """ + Admin action to revert selected CourseAccessRoleHistory entries. + """ + if not request.user.has_perm('student.can_revert_course_access_role'): + self.message_user(request, "You do not have permission to revert course access roles.", level='ERROR') + return + + reverted_count = 0 + for history_record in queryset: + try: + with transaction.atomic(): + if history_record.action_type == 'created': + CourseAccessRole.objects.filter( + user=history_record.user, + org=history_record.org, + course_id=history_record.course_id, + role=history_record.role + ).delete() + self.message_user( + request, f"Successfully reverted creation of role for " + f"{history_record.user.username} in {history_record.course_id}" + ) + elif history_record.action_type == 'updated': + if history_record.old_values: + CourseAccessRole.objects.update_or_create( + user_id=history_record.old_values['user_id'], + org=history_record.old_values['org'], + course_id=history_record.old_values['course_id'], + defaults={'role': history_record.old_values['role']} + ) + self.message_user( + request, f"Successfully reverted update of role for " + f"{history_record.user.username} to {history_record.old_values['role']} " + f"in {history_record.course_id}" + ) + else: + self.message_user( + request, f"Cannot revert update for record {history_record.id}: " + f"old_values not found.", level='WARNING' + ) + elif history_record.action_type == 'deleted': + CourseAccessRole.objects.update_or_create( + user=history_record.user, + org=history_record.org, + course_id=history_record.course_id, + role=history_record.role + ) + self.message_user( + request, f"Successfully reverted deletion of role for " + f"{history_record.user.username} in {history_record.course_id}" + ) + reverted_count += 1 + except Exception as e: # lint-amnesty, pylint: disable=broad-except + self.message_user(request, f"Error reverting record {history_record.id}: {e}", level='ERROR') + + if reverted_count > 0: + self.message_user( + request, + ngettext( + "Successfully reverted %(count)d selected history entry.", + "Successfully reverted %(count)d selected history entries.", + reverted_count + ) % {'count': reverted_count}, + ) + revert_selected_history.short_description = "Revert selected history entries" + + def delete_selected_history_entries(self, request, queryset): + """ + Admin action to delete selected CourseAccessRoleHistory entries. + """ + if not request.user.has_perm('student.can_delete_course_access_role_history'): + self.message_user( + request, "You do not have permission to delete course access role history entries.", + level='ERROR' + ) + return + + deleted_count = 0 + for history_record in queryset: + try: + history_record.delete() + deleted_count += 1 + except Exception as e: # lint-amnesty, pylint: disable=broad-except + self.message_user(request, f"Error deleting record {history_record.id}: {e}", level='ERROR') + + if deleted_count > 0: + self.message_user( + request, + ngettext( + "Successfully deleted %(count)d selected history entry.", + "Successfully deleted %(count)d selected history entries.", + deleted_count + ) % {'count': deleted_count}, + level='SUCCESS', + ) + delete_selected_history_entries.short_description = "Delete selected history entries" + + @admin.register(LinkedInAddToProfileConfiguration) class LinkedInAddToProfileConfigurationAdmin(admin.ModelAdmin): """Admin interface for the LinkedIn Add to Profile configuration. """ diff --git a/common/djangoapps/student/migrations/0047_courseaccessrolehistory.py b/common/djangoapps/student/migrations/0047_courseaccessrolehistory.py new file mode 100644 index 000000000000..ba2f1da9ef76 --- /dev/null +++ b/common/djangoapps/student/migrations/0047_courseaccessrolehistory.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.23 on 2025-08-22 10:13 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('student', '0046_alter_userprofile_phone_number'), + ] + + operations = [ + migrations.CreateModel( + name='CourseAccessRoleHistory', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('org', models.CharField(blank=True, db_index=True, max_length=64)), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(blank=True, db_index=True, max_length=255)), + ('role', models.CharField(db_index=True, max_length=64)), + ('action_type', models.CharField(choices=[('created', 'Created'), ('updated', 'Updated'), ('deleted', 'Deleted')], db_index=True, max_length=10)), + ('old_values', models.JSONField(blank=True, help_text="Stores old values of fields for 'updated' actions.", null=True)), + ('changed_by', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='courseaccessrole_history_changer', to=settings.AUTH_USER_MODEL)), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'permissions': (('can_revert_course_access_role', 'Can revert course access role changes'), ('can_delete_course_access_role_history', 'Can delete course access role history')), + }, + ), + ] diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 94cb99d0ce3f..6d16a5a95b6a 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -32,7 +32,7 @@ from django.core.validators import FileExtensionValidator, RegexValidator from django.db import IntegrityError, models from django.db.models import Q -from django.db.models.signals import post_save, pre_save +from django.db.models.signals import post_save, pre_save, post_delete from django.db.utils import ProgrammingError from django.dispatch import receiver from django.utils.translation import gettext_lazy as _ @@ -1103,6 +1103,41 @@ def __str__(self): return f"[CourseAccessRole] user: {self.user.username} role: {self.role} org: {self.org} course: {self.course_id}" # lint-amnesty, pylint: disable=line-too-long +class CourseAccessRoleHistory(TimeStampedModel): + """ + Stores the change history for CourseAccessRole objects. + """ + ACTION_CHOICES = ( + ('created', 'Created'), + ('updated', 'Updated'), + ('deleted', 'Deleted'), + ) + + user = models.ForeignKey(User, on_delete=models.CASCADE) + org = models.CharField(max_length=64, db_index=True, blank=True) + course_id = CourseKeyField(max_length=255, db_index=True, blank=True) + role = models.CharField(max_length=64, db_index=True) + action_type = models.CharField(max_length=10, choices=ACTION_CHOICES, db_index=True) + changed_by = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + related_name='courseaccessrole_history_changer', + ) + old_values = models.JSONField(null=True, blank=True, help_text="Stores old values of fields for 'updated' actions.") + + class Meta: + permissions = (("can_revert_course_access_role", "Can revert course access role changes"), + ("can_delete_course_access_role_history", "Can delete course access role history"),) + + def __str__(self): + return ( + f"[CourseAccessRoleHistory] user: {self.user.username} role: {self.role} " + f"org: {self.org} course: {self.course_id} action: {self.action_type} " + f"changed_by: {self.changed_by.username if self.changed_by else 'N/A'} at {self.created}" + ) + + #### Helper methods for use from python manage.py shell and other classes. def strip_if_string(value): @@ -1879,3 +1914,58 @@ class Meta: def __str__(self): return self.comment + + +@receiver(pre_save, sender=CourseAccessRole) +def pre_save_course_access_role(sender, instance, **kwargs): + """ + Captures the current state of a CourseAccessRole before it is saved for update tracking. + """ + if instance.pk: + try: + old_instance = sender.objects.get(pk=instance.pk) + # pylint: disable=protected-access + instance._old_values = { + 'user_id': old_instance.user_id, + 'org': old_instance.org, + 'course_id': str(old_instance.course_id) if old_instance.course_id else None, + 'role': old_instance.role, + } + except sender.DoesNotExist: + # pylint: disable=protected-access + instance._old_values = None + + +@receiver(post_save, sender=CourseAccessRole) +def create_course_access_role_history_on_save(sender, instance, created, **kwargs): + """ + Handle create and update actions for CourseAccessRole objects. + """ + action_type = 'created' if created else 'updated' + current_user = crum.get_current_user() + old_values = getattr(instance, '_old_values', None) if not created else None + CourseAccessRoleHistory.objects.create( + user=instance.user, + org=instance.org, + course_id=instance.course_id, + role=instance.role, + action_type=action_type, + changed_by=current_user if current_user and current_user.is_authenticated else None, + old_values=old_values + ) + + +@receiver(post_delete, sender=CourseAccessRole) +def create_course_access_role_history_on_delete(sender, instance, **kwargs): + """ + Handle delete actions for CourseAccessRole objects. + """ + current_user = crum.get_current_user() + CourseAccessRoleHistory.objects.create( + user=instance.user, + org=instance.org, + course_id=instance.course_id, + role=instance.role, + action_type='deleted', + changed_by=current_user if current_user and current_user.is_authenticated else None + ) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index da1aad19a803..92dcf1957626 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -4,10 +4,13 @@ import ddt +from django.contrib.auth.models import Permission from django.test import TestCase from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator +from common.djangoapps.student.admin import CourseAccessRoleHistoryAdmin +from common.djangoapps.student.models import CourseAccessRoleHistory, User from common.djangoapps.student.roles import ( CourseAccessRole, CourseBetaTesterRole, @@ -309,3 +312,248 @@ def test_role_cache_roles_by_course_id(self): assert roles_dict.get('library-v1:edX+quizzes').pop().course_id.course == 'quizzes' assert roles_dict.get('course-v1:edX+toy+2012_Summer').pop().course_id.course == 'toy' assert roles_dict.get('course-v1:edX+toy2+2013_Fall').pop().course_id.course == 'toy2' + + +class CourseAccessRoleHistoryTest(TestCase): + """ + Tests for the CourseAccessRoleHistory model and associated signals/admin actions. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory(username="test_user", email="test@example.com") + self.admin_user = UserFactory( + username="admin_user", + email="admin@example.com", + is_staff=True, + is_superuser=True, + ) + self.course_key = CourseKey.from_string("course-v1:OrgX+CourseY+2023_Fall") + self.org = "OrgX" + + revert_permission = Permission.objects.get( + codename="can_revert_course_access_role", content_type__app_label="student" + ) + delete_history_permission = Permission.objects.get( + codename="can_delete_course_access_role_history", + content_type__app_label="student", + ) + self.admin_user.user_permissions.add( + revert_permission, delete_history_permission + ) + self.admin_user = User.objects.get(pk=self.admin_user.pk) + + def test_create_logs_history(self): + """ + Test that creating a CourseAccessRole logs a history entry. + """ + CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="student" + ) + + history = CourseAccessRoleHistory.objects.first() + self.assertIsNotNone(history) + self.assertEqual(history.user, self.user) + self.assertEqual(history.org, self.org) + self.assertEqual(history.course_id, self.course_key) + self.assertEqual(history.role, "student") + self.assertEqual(history.action_type, "created") + self.assertIsNone(history.old_values) + + def test_update_logs_history(self): + """ + Test that updating a CourseAccessRole logs a history entry with old_values. + """ + role_instance = CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="student" + ) + role_instance.role = "staff" + role_instance.save() + + history_entries = CourseAccessRoleHistory.objects.filter( + user=self.user, course_id=self.course_key + ).order_by("created") + self.assertEqual(history_entries.count(), 2) + + update_history = history_entries.last() + self.assertEqual(update_history.action_type, "updated") + self.assertIsNotNone(update_history.old_values) + self.assertEqual(update_history.old_values["role"], "student") + self.assertEqual(update_history.role, "staff") + + def test_delete_logs_history(self): + """ + Test that deleting a CourseAccessRole logs a history entry. + """ + role_instance = CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="student" + ) + + role_instance.delete() + + history_entries = CourseAccessRoleHistory.objects.filter( + user=self.user, course_id=self.course_key + ).order_by("created") + self.assertEqual(history_entries.count(), 2) + + delete_history = history_entries.last() + self.assertEqual(delete_history.action_type, "deleted") + self.assertIsNone(delete_history.old_values) + self.assertEqual(delete_history.role, "student") + + +class CourseAccessRoleAdminActionsTest(TestCase): + """ + Tests for the admin actions (revert, delete) on CourseAccessRoleHistory. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory( + username="test_user_admin", email="test_admin@example.com" + ) + self.admin_user = UserFactory( + username="admin_action_user", + email="admin_action@example.com", + is_staff=True, + is_superuser=True, + ) + self.course_key = CourseKey.from_string( + "course-v1:AdminOrg+AdminCourse+2024_Spring" + ) + self.org = "AdminOrg" + revert_permission = Permission.objects.get( + codename="can_revert_course_access_role", content_type__app_label="student" + ) + delete_history_permission = Permission.objects.get( + codename="can_delete_course_access_role_history", + content_type__app_label="student", + ) + self.admin_user.user_permissions.add( + revert_permission, delete_history_permission + ) + self.admin_user = User.objects.get(pk=self.admin_user.pk) + self.messages = [] + + def _get_admin_action_response(self, action, queryset): + """Helper to call admin actions and capture messages.""" + from django.contrib.admin import AdminSite + + model_admin = CourseAccessRoleHistoryAdmin(CourseAccessRoleHistory, AdminSite()) + request = self.client.get("/") + request.user = self.admin_user + + def mock_message_user(request, message, level=None): + self.messages.append(message) + + model_admin.message_user = mock_message_user + + response = action(model_admin, request, queryset) + return response + + def test_revert_created_action(self): + """ + Test reverting a 'created' history entry should delete the CourseAccessRole. + """ + CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="beta_tester" + ) + self.assertEqual(CourseAccessRole.objects.count(), 1) + created_history = CourseAccessRoleHistory.objects.filter( + action_type="created" + ).first() + self.assertIsNotNone(created_history) + + self._get_admin_action_response( + CourseAccessRoleHistoryAdmin.revert_selected_history, + CourseAccessRoleHistory.objects.filter(pk=created_history.pk), + ) + + self.assertEqual(CourseAccessRole.objects.count(), 0) + self.assertIn( + f"Successfully reverted creation of role for {self.user.username} in {self.course_key}", + self.messages[0], + ) + + def test_revert_updated_action(self): + """ + Test reverting an 'updated' history entry should restore the CourseAccessRole to its old_values. + """ + role_instance = CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="old_role" + ) + + role_instance.role = "new_role" + role_instance.save() + + self.assertEqual(CourseAccessRole.objects.get().role, "new_role") + updated_history = CourseAccessRoleHistory.objects.filter( + action_type="updated" + ).first() + self.assertIsNotNone(updated_history) + self.assertEqual(updated_history.old_values["role"], "old_role") + + self._get_admin_action_response( + CourseAccessRoleHistoryAdmin.revert_selected_history, + CourseAccessRoleHistory.objects.filter(pk=updated_history.pk), + ) + + self.assertEqual(CourseAccessRole.objects.get().role, "old_role") + self.assertIn( + f"Successfully reverted update of role for {self.user.username} to old_role in {self.course_key}", + self.messages[0], + ) + + def test_revert_deleted_action(self): + """ + Test reverting a 'deleted' history entry should recreate the CourseAccessRole. + """ + role_instance = CourseAccessRole.objects.create( + user=self.user, + org=self.org, + course_id=self.course_key, + role="to_be_deleted", + ) + self.assertEqual(CourseAccessRole.objects.count(), 1) + initial_history_count = CourseAccessRoleHistory.objects.count() + + role_instance.delete() + self.assertEqual(CourseAccessRole.objects.count(), 0) + deleted_history = CourseAccessRoleHistory.objects.filter( + action_type="deleted" + ).first() + self.assertIsNotNone(deleted_history) + + self._get_admin_action_response( + CourseAccessRoleHistoryAdmin.revert_selected_history, + CourseAccessRoleHistory.objects.filter(pk=deleted_history.pk), + ) + + self.assertEqual(CourseAccessRole.objects.count(), 1) + reverted_role = CourseAccessRole.objects.first() + self.assertEqual(reverted_role.user, self.user) + self.assertEqual(reverted_role.org, self.org) + self.assertEqual(reverted_role.course_id, self.course_key) + self.assertEqual(reverted_role.role, "to_be_deleted") + self.assertIn( + f"Successfully reverted deletion of role for {self.user.username} in {self.course_key}", + self.messages[0], + ) + + def test_delete_history_action(self): + """ + Test the admin action to delete selected history entries. + """ + CourseAccessRole.objects.create( + user=self.user, org=self.org, course_id=self.course_key, role="some_role" + ) + self.assertEqual(CourseAccessRoleHistory.objects.count(), 1) + history_entry = CourseAccessRoleHistory.objects.first() + + self._get_admin_action_response( + CourseAccessRoleHistoryAdmin.delete_selected_history_entries, + CourseAccessRoleHistory.objects.filter(pk=history_entry.pk), + ) + + self.assertEqual(CourseAccessRoleHistory.objects.count(), 0) + self.assertIn("Successfully deleted 1 selected history entry.", self.messages[0])