diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 677b74b1bf..151c6ab5b8 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -36,6 +36,7 @@ from common.djangoapps.student.models import ( BulkChangeEnrollmentConfiguration, BulkUnenrollConfiguration, CourseAccessRole, + CourseAccessRoleHistory, CourseEnrollment, CourseEnrollmentAllowed, CourseEnrollmentCelebration, @@ -229,6 +230,131 @@ class CourseAccessRoleAdmin(admin.ModelAdmin): 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 0000000000..ba2f1da9ef --- /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 94cb99d0ce..6d16a5a95b 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -32,7 +32,7 @@ from django.core.exceptions import ObjectDoesNotExist 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 @@ class CourseAccessRole(models.Model): 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 UserPasswordToggleHistory(TimeStampedModel): 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 da1aad19a8..92dcf19576 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -4,10 +4,13 @@ Tests of student.roles 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 @@ class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-clas 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]) diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index b81a8fd144..95d5a02010 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -378,6 +378,7 @@ def track_comment_reported_event(request, course, comment): obj_type = 'comment' if comment.get('parent_id') else 'response' event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='reported') event_data = { + 'discussion': {'id': comment.thread_id}, 'body': comment.body[:TRACKING_MAX_FORUM_BODY], 'truncated': len(comment.body) > TRACKING_MAX_FORUM_BODY, 'commentable_id': comment.get('commentable_id', ''), @@ -416,6 +417,7 @@ def track_comment_unreported_event(request, course, comment): obj_type = 'comment' if comment.get('parent_id') else 'response' event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='unreported') event_data = { + 'discussion': {'id': comment.thread_id}, 'body': comment.body[:TRACKING_MAX_FORUM_BODY], 'truncated': len(comment.body) > TRACKING_MAX_FORUM_BODY, 'commentable_id': comment.get('commentable_id', ''), diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py index 62284b904c..6543453858 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py @@ -896,6 +896,7 @@ class UpdateCommentTest( else "edx.forum.response.unreported" ) expected_event_data = { + "discussion": {'id': 'test_thread'}, "body": "Original body", "id": "test_comment", "content_type": "Response", @@ -958,6 +959,7 @@ class UpdateCommentTest( "body": "Original body", "id": "test_comment", "content_type": "Response", + "discussion": {'id': 'test_thread'}, "commentable_id": "dummy", "truncated": False, "url": "", diff --git a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py index 5e0640c64e..247bd23540 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py @@ -2,6 +2,7 @@ Unit tests for the DiscussionNotificationSender class """ import re +import django import unittest from unittest.mock import MagicMock, patch @@ -108,11 +109,14 @@ class TestCleanThreadHtmlBody(unittest.TestCase):

Script test:

Some other content that should remain.

""" - expected_output = ('

This is a link to a page.

' - '

Here is an image:

' - '

Embedded video:

' - '

Script test: alert("hello");

' - '

Some other content that should remain.

') + excepted_script_quot = 'alert("hello");' if django.VERSION >= (5, 0) else 'alert("hello");' + expected_output = ( + f'

This is a link to a page.

' + f'

Here is an image:

' + f'

Embedded video:

' + f'

Script test: {excepted_script_quot}

' + f'

Some other content that should remain.

' + ) result = clean_thread_html_body(html_body) diff --git a/openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py b/openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py index f4736c6b3e..6c66831201 100644 --- a/openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py +++ b/openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py @@ -313,7 +313,7 @@ def get_receiver_names(): """Return an unordered set of receiver names (full.module.path.function)""" return { name_from_fn(fn_ref()) - for _, fn_ref in Command.course_published_signal.receivers + for _, fn_ref, *_ in Command.course_published_signal.receivers } @@ -321,7 +321,7 @@ def get_receiver_fns(): """Return the list of active receiver functions.""" return [ fn_ref() # fn_ref is a weakref to a function, fn_ref() gives us the function - for _, fn_ref in Command.course_published_signal.receivers + for _, fn_ref, *_ in Command.course_published_signal.receivers ] diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 3ce2590d18..1f761c0860 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -6,13 +6,14 @@ import datetime from bs4 import BeautifulSoup from django.conf import settings from django.contrib.auth import get_user_model +from django.core.exceptions import BadRequest from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ from pytz import utc from waffle import get_waffle_flag_model # pylint: disable=invalid-django-waffle-import from lms.djangoapps.branding.api import get_logo_url_for_email -from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher, UsernameDecryptionException from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.notifications.base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES from openedx.core.djangoapps.notifications.config.waffle import ENABLE_EMAIL_NOTIFICATIONS @@ -384,6 +385,19 @@ def decrypt_string(string): return UsernameCipher.decrypt(string).decode() +def username_from_hash(group, request): + """ + Django ratelimit key to return username from hash + """ + username = request.resolver_match.kwargs.get("username") + if username: + try: + return decrypt_string(username) + except UsernameDecryptionException as exc: + raise BadRequest("Bad request") from exc + return None + + def update_user_preferences_from_patch(encrypted_username): """ Decrypt username and patch and updates user preferences diff --git a/openedx/core/djangoapps/notifications/events.py b/openedx/core/djangoapps/notifications/events.py index 8dd1f51868..ea5f62b40b 100644 --- a/openedx/core/djangoapps/notifications/events.py +++ b/openedx/core/djangoapps/notifications/events.py @@ -4,7 +4,6 @@ from eventtracking import tracker from common.djangoapps.track import contexts, segment -NOTIFICATION_PREFERENCES_VIEWED = 'edx.notifications.preferences.viewed' NOTIFICATION_GENERATED = 'edx.notifications.generated' NOTIFICATION_READ = 'edx.notifications.read' NOTIFICATION_APP_ALL_READ = 'edx.notifications.app_all_read' @@ -46,35 +45,6 @@ def notification_event_context(user, course_id, notification): } -def notification_preferences_viewed_event(request, course_id=None): - """ - Emit an event when a user views their notification preferences. - """ - event_data = { - 'user_id': str(request.user.id), - 'course_id': None, - 'user_forum_roles': [], - 'user_course_roles': [], - 'type': 'account' - } - if not course_id: - tracker.emit( - NOTIFICATION_PREFERENCES_VIEWED, - event_data - ) - return - context = contexts.course_context_from_course_id(course_id) - with tracker.get_tracker().context(NOTIFICATION_PREFERENCES_VIEWED, context): - event_data['course_id']: str(course_id) - event_data['user_forum_roles'] = get_user_forums_roles(request.user, course_id) - event_data['user_course_roles'] = get_user_course_roles(request.user, course_id) - event_data['type'] = 'course' - tracker.emit( - NOTIFICATION_PREFERENCES_VIEWED, - event_data - ) - - def notification_generated_event(user_ids, app_name, notification_type, course_key, content_url, content, sender_id=None): """ diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index de02141745..d5b2237303 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -7,6 +7,7 @@ from unittest import mock import ddt from django.conf import settings from django.contrib.auth import get_user_model +from django.core.cache import cache from django.test.utils import override_settings from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag @@ -481,6 +482,7 @@ class UpdatePreferenceFromEncryptedDataView(ModuleStoreTestCase): """ Setup test case """ + cache.clear() super().setUp() password = 'password' self.user = UserFactory(password=password) @@ -488,6 +490,22 @@ class UpdatePreferenceFromEncryptedDataView(ModuleStoreTestCase): self.course = CourseFactory.create(display_name='test course 1', run="Testing_course_1") CourseNotificationPreference(course_id=self.course.id, user=self.user).save() + @override_settings(LMS_BASE="example.com", ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT='1/d') + def test_rate_limit_on_unsub(self): + """ + Test rate limit on unsub + """ + self.client.logout() + user_hash = encrypt_string(self.user.username) + url_params = { + "username": user_hash, + } + url = reverse("preference_update_view", kwargs=url_params) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + response = self.client.get(url) + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + @override_settings(LMS_BASE="") @ddt.data('get', 'post') def test_if_preference_is_updated(self, request_type): diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index d3a9dd1f48..57ef88cde1 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta from django.conf import settings from django.db.models import Count +from django_ratelimit.core import is_ratelimited from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ from pytz import UTC @@ -14,7 +15,7 @@ from rest_framework.generics import UpdateAPIView from rest_framework.response import Response from rest_framework.views import APIView -from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch +from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch, username_from_hash from openedx.core.djangoapps.notifications.models import NotificationPreference from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user @@ -241,6 +242,11 @@ def preference_update_from_encrypted_username_view(request, username, patch=""): View to update user preferences from encrypted username and patch. username and patch must be string """ + if is_ratelimited( + request=request, group="unsubscribe", key=username_from_hash, + rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, increment=True, + ): + return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS) update_user_preferences_from_patch(username) return Response({"result": "success"}, status=status.HTTP_200_OK) diff --git a/openedx/core/lib/xblock_pipeline/finder.py b/openedx/core/lib/xblock_pipeline/finder.py index c4a0c4d478..a87b564afc 100644 --- a/openedx/core/lib/xblock_pipeline/finder.py +++ b/openedx/core/lib/xblock_pipeline/finder.py @@ -3,14 +3,13 @@ Django pipeline finder for handling static assets required by XBlocks. """ import os -from datetime import datetime +from datetime import datetime, timezone import importlib.resources as resources from django.contrib.staticfiles import utils from django.contrib.staticfiles.finders import BaseFinder from django.contrib.staticfiles.storage import FileSystemStorage from django.core.files.storage import Storage -from django.utils import timezone from xblock.core import XBlock from openedx.core.lib.xblock_utils import xblock_resource_pkg diff --git a/openedx/envs/common.py b/openedx/envs/common.py index 767bfdbd3e..c888ec8fd7 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -828,6 +828,8 @@ USERNAME_PATTERN = fr'(?P{USERNAME_REGEX_PARTIAL})' DISCUSSION_RATELIMIT = '100/m' SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS = 0 +ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT = '100/m' + LMS_ROOT_URL = None LMS_INTERNAL_ROOT_URL = Derived(lambda settings: settings.LMS_ROOT_URL) diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index c42a530e3c..3e3dae1298 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -1190,9 +1190,10 @@ class TestContentTypeGatingService(ModuleStoreTestCase): self.user, blocks_dict['not_graded_1'], course['course'].id ) is None - @patch.object(ContentTypeGatingService, '_get_user', return_value=UserFactory.build()) - def test_check_children_for_content_type_gating_paywall(self, mocked_user): # pylint: disable=unused-argument + @patch.object(ContentTypeGatingService, "_get_user") + def test_check_children_for_content_type_gating_paywall(self, mocked_get_user): ''' Verify that the method returns a content type gate when appropriate ''' + mocked_get_user.return_value = UserFactory.create() # saved user object with id course = self._create_course() blocks_dict = course['blocks'] CourseEnrollmentFactory.create( diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index 4b99071256..193af3cec3 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -18,9 +18,9 @@ cryptography==45.0.6 # via -r requirements/edx-sandbox/base.in cycler==0.12.1 # via matplotlib -fonttools==4.59.1 +fonttools==4.59.2 # via matplotlib -joblib==1.5.1 +joblib==1.5.2 # via nltk kiwisolver==1.4.9 # via matplotlib diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index e0af39f108..3bc9ef2463 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -68,14 +68,14 @@ bleach[css]==6.2.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/kernel.in -boto3==1.40.18 +boto3==1.40.19 # via # -r requirements/edx/kernel.in # django-ses # fs-s3fs # ora2 # snowflake-connector-python -botocore==1.40.18 +botocore==1.40.19 # via # -r requirements/edx/kernel.in # boto3 @@ -340,7 +340,7 @@ django-sekizai==4.1.0 # openedx-django-wiki django-ses==4.4.0 # via -r requirements/edx/bundled.in -django-simple-history==3.8.0 +django-simple-history==3.10.1 # via # -r requirements/edx/kernel.in # edx-enterprise @@ -489,7 +489,7 @@ edx-i18n-tools==1.9.0 # xblocks-contrib edx-milestones==1.1.0 # via -r requirements/edx/kernel.in -edx-name-affirmation==3.0.1 +edx-name-affirmation==3.0.2 # via -r requirements/edx/kernel.in edx-opaque-keys[django]==3.0.0 # via @@ -509,7 +509,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.2.1 +edx-organizations==7.3.0 # via -r requirements/edx/kernel.in edx-proctoring==5.2.0 # via -r requirements/edx/kernel.in @@ -684,7 +684,7 @@ jmespath==1.0.1 # via # boto3 # botocore -joblib==1.5.1 +joblib==1.5.2 # via nltk jsondiff==2.2.1 # via edx-enterprise @@ -886,7 +886,7 @@ platformdirs==4.4.0 # via snowflake-connector-python polib==1.2.0 # via edx-i18n-tools -prompt-toolkit==3.0.51 +prompt-toolkit==3.0.52 # via click-repl propcache==0.3.2 # via @@ -1072,7 +1072,7 @@ requests-oauthlib==2.0.0 # via # -r requirements/edx/kernel.in # social-auth-core -rpds-py==0.27.0 +rpds-py==0.27.1 # via # jsonschema # referencing @@ -1148,7 +1148,7 @@ sortedcontainers==2.4.0 # via # -r requirements/edx/kernel.in # snowflake-connector-python -soupsieve==2.7 +soupsieve==2.8 # via beautifulsoup4 sqlparse==0.5.3 # via django diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d44c3e38c4..32c74d8cc6 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -136,7 +136,7 @@ boto==2.49.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -boto3==1.40.18 +boto3==1.40.19 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -144,7 +144,7 @@ boto3==1.40.18 # fs-s3fs # ora2 # snowflake-connector-python -botocore==1.40.18 +botocore==1.40.19 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -559,7 +559,7 @@ django-ses==4.4.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -django-simple-history==3.8.0 +django-simple-history==3.10.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -773,7 +773,7 @@ edx-milestones==1.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-name-affirmation==3.0.1 +edx-name-affirmation==3.0.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -796,7 +796,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.2.1 +edx-organizations==7.3.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1134,7 +1134,7 @@ jmespath==1.0.1 # -r requirements/edx/testing.txt # boto3 # botocore -joblib==1.5.1 +joblib==1.5.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1497,7 +1497,7 @@ polib==1.2.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-i18n-tools -prompt-toolkit==3.0.51 +prompt-toolkit==3.0.52 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1859,7 +1859,7 @@ roman-numerals-py==3.1.0 # via # -r requirements/edx/doc.txt # sphinx -rpds-py==0.27.0 +rpds-py==0.27.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1983,7 +1983,7 @@ sortedcontainers==2.4.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # snowflake-connector-python -soupsieve==2.7 +soupsieve==2.8 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 26ebd8fe44..0e46cf4603 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -103,14 +103,14 @@ bleach[css]==6.2.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/base.txt -boto3==1.40.18 +boto3==1.40.19 # via # -r requirements/edx/base.txt # django-ses # fs-s3fs # ora2 # snowflake-connector-python -botocore==1.40.18 +botocore==1.40.19 # via # -r requirements/edx/base.txt # boto3 @@ -412,7 +412,7 @@ django-sekizai==4.1.0 # openedx-django-wiki django-ses==4.4.0 # via -r requirements/edx/base.txt -django-simple-history==3.8.0 +django-simple-history==3.10.1 # via # -r requirements/edx/base.txt # edx-enterprise @@ -573,7 +573,7 @@ edx-i18n-tools==1.9.0 # xblocks-contrib edx-milestones==1.1.0 # via -r requirements/edx/base.txt -edx-name-affirmation==3.0.1 +edx-name-affirmation==3.0.2 # via -r requirements/edx/base.txt edx-opaque-keys[django]==3.0.0 # via @@ -593,7 +593,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.2.1 +edx-organizations==7.3.0 # via -r requirements/edx/base.txt edx-proctoring==5.2.0 # via -r requirements/edx/base.txt @@ -825,7 +825,7 @@ jmespath==1.0.1 # -r requirements/edx/base.txt # boto3 # botocore -joblib==1.5.1 +joblib==1.5.2 # via # -r requirements/edx/base.txt # nltk @@ -1076,7 +1076,7 @@ polib==1.2.0 # via # -r requirements/edx/base.txt # edx-i18n-tools -prompt-toolkit==3.0.51 +prompt-toolkit==3.0.52 # via # -r requirements/edx/base.txt # click-repl @@ -1307,7 +1307,7 @@ requests-oauthlib==2.0.0 # social-auth-core roman-numerals-py==3.1.0 # via sphinx -rpds-py==0.27.0 +rpds-py==0.27.1 # via # -r requirements/edx/base.txt # jsonschema @@ -1403,7 +1403,7 @@ sortedcontainers==2.4.0 # via # -r requirements/edx/base.txt # snowflake-connector-python -soupsieve==2.7 +soupsieve==2.8 # via # -r requirements/edx/base.txt # beautifulsoup4 diff --git a/requirements/edx/semgrep.txt b/requirements/edx/semgrep.txt index 286030caa1..d55d6f3e35 100644 --- a/requirements/edx/semgrep.txt +++ b/requirements/edx/semgrep.txt @@ -105,7 +105,7 @@ requests==2.32.5 # semgrep rich==13.5.3 # via semgrep -rpds-py==0.27.0 +rpds-py==0.27.1 # via # jsonschema # referencing diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 9b63b3e79b..bc4a7dac63 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -100,14 +100,14 @@ bleach[css]==6.2.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/base.txt -boto3==1.40.18 +boto3==1.40.19 # via # -r requirements/edx/base.txt # django-ses # fs-s3fs # ora2 # snowflake-connector-python -botocore==1.40.18 +botocore==1.40.19 # via # -r requirements/edx/base.txt # boto3 @@ -438,7 +438,7 @@ django-sekizai==4.1.0 # openedx-django-wiki django-ses==4.4.0 # via -r requirements/edx/base.txt -django-simple-history==3.8.0 +django-simple-history==3.10.1 # via # -r requirements/edx/base.txt # edx-enterprise @@ -596,7 +596,7 @@ edx-lint==5.6.0 # via -r requirements/edx/testing.in edx-milestones==1.1.0 # via -r requirements/edx/base.txt -edx-name-affirmation==3.0.1 +edx-name-affirmation==3.0.2 # via -r requirements/edx/base.txt edx-opaque-keys[django]==3.0.0 # via @@ -616,7 +616,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.2.1 +edx-organizations==7.3.0 # via -r requirements/edx/base.txt edx-proctoring==5.2.0 # via -r requirements/edx/base.txt @@ -867,7 +867,7 @@ jmespath==1.0.1 # -r requirements/edx/base.txt # boto3 # botocore -joblib==1.5.1 +joblib==1.5.2 # via # -r requirements/edx/base.txt # grimp @@ -1134,7 +1134,7 @@ polib==1.2.0 # -r requirements/edx/base.txt # -r requirements/edx/testing.in # edx-i18n-tools -prompt-toolkit==3.0.51 +prompt-toolkit==3.0.52 # via # -r requirements/edx/base.txt # click-repl @@ -1415,7 +1415,7 @@ requests-oauthlib==2.0.0 # via # -r requirements/edx/base.txt # social-auth-core -rpds-py==0.27.0 +rpds-py==0.27.1 # via # -r requirements/edx/base.txt # jsonschema @@ -1510,7 +1510,7 @@ sortedcontainers==2.4.0 # via # -r requirements/edx/base.txt # snowflake-connector-python -soupsieve==2.7 +soupsieve==2.8 # via # -r requirements/edx/base.txt # beautifulsoup4 diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index b11cfde50f..471b303326 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -10,9 +10,9 @@ attrs==25.3.0 # via zeep backoff==2.2.1 # via -r scripts/user_retirement/requirements/base.in -boto3==1.40.18 +boto3==1.40.19 # via -r scripts/user_retirement/requirements/base.in -botocore==1.40.18 +botocore==1.40.19 # via # boto3 # s3transfer diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 58ddd51424..e186a99fca 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -14,11 +14,11 @@ attrs==25.3.0 # zeep backoff==2.2.1 # via -r scripts/user_retirement/requirements/base.txt -boto3==1.40.18 +boto3==1.40.19 # via # -r scripts/user_retirement/requirements/base.txt # moto -botocore==1.40.18 +botocore==1.40.19 # via # -r scripts/user_retirement/requirements/base.txt # boto3 diff --git a/xmodule/modulestore/mixed.py b/xmodule/modulestore/mixed.py index 1d9fd1f84e..b196eb2ea4 100644 --- a/xmodule/modulestore/mixed.py +++ b/xmodule/modulestore/mixed.py @@ -7,6 +7,7 @@ In this way, courses can be served up via either SplitMongoModuleStore or MongoM import functools import itertools import logging +from datetime import datetime, timezone from contextlib import contextmanager from opaque_keys import InvalidKeyError @@ -21,7 +22,6 @@ from openedx_events.content_authoring.signals import ( XBLOCK_UPDATED ) -from django.utils.timezone import datetime, timezone from xmodule.assetstore import AssetMetadata from . import XMODULE_FIELDS_WITH_USAGE_KEYS, ModuleStoreWriteBase