From 4d035ea3d4ccf3ea7b4aec670ad5fcd9f8892377 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Tue, 20 May 2025 20:18:14 +0500 Subject: [PATCH] feat: added API to sync discussions topics (#36529) --- openedx/core/djangoapps/discussions/tasks.py | 26 +++-- .../discussions/tests/test_views.py | 104 ++++++++++++++++-- openedx/core/djangoapps/discussions/urls.py | 15 ++- openedx/core/djangoapps/discussions/views.py | 40 ++++++- 4 files changed, 162 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index fea20dc59b..50a46065fe 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -181,7 +181,12 @@ def is_discussable_unit(unit, store, enable_graded_units, subsection): return True -def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, user_id: int, force=False) -> None: +def update_unit_discussion_state_from_discussion_blocks( + course_key: CourseKey, + user_id: int, + force=False, + async_topics=True +) -> None: """ Migrate existing courses to the new mechanism for linking discussion to units. @@ -192,7 +197,7 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u course_key (CourseKey): CourseKey for course. user_id (int): User id for the user performing this operation. force (bool): Force migration of data even if not using legacy provider - + async_topics (bool): If True, run the task asynchronously. """ store = modulestore() course = store.get_course(course_key) @@ -255,8 +260,15 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u discussion_config.enable_graded_units = enable_graded_subsections discussion_config.unit_level_visibility = True discussion_config.save() - # added delay of 30 minutes to allow for the course to be published - update_discussions_settings_from_course_task.apply_async( - args=[str(course_key), [str(unit) for unit in discussable_units]], - countdown=1800, - ) + + if async_topics: + # added delay of 30 minutes to allow for the course to be published + update_discussions_settings_from_course_task.apply_async( + args=[str(course_key), [str(unit) for unit in discussable_units]], + countdown=1800, + ) + else: + update_discussions_settings_from_course_task( + str(course_key), + [str(unit) for unit in discussable_units], + ) diff --git a/openedx/core/djangoapps/discussions/tests/test_views.py b/openedx/core/djangoapps/discussions/tests/test_views.py index cc17b56d22..3935e8ab6d 100644 --- a/openedx/core/djangoapps/discussions/tests/test_views.py +++ b/openedx/core/djangoapps/discussions/tests/test_views.py @@ -5,28 +5,26 @@ Test app view logic import itertools from contextlib import contextmanager from datetime import datetime, timedelta, timezone +from unittest.mock import Mock, patch import ddt from django.core.exceptions import ValidationError from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from lti_consumer.models import CourseAllowPIISharingInLTIFlag +from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.test import APITestCase + +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from common.djangoapps.student.tests.factories import UserFactory -from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory from ..config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS - -from ..models import ( - AVAILABLE_PROVIDER_MAP, - DEFAULT_CONFIG_ENABLED, - Provider, - get_default_provider_type, -) +from ..models import AVAILABLE_PROVIDER_MAP, DEFAULT_CONFIG_ENABLED, Provider, get_default_provider_type +from ..permissions import IsStaffOrCourseTeam DATA_LEGACY_COHORTS = { 'divided_inline_discussions': [], @@ -830,3 +828,91 @@ class PIISettingsAPITests(DataTest): response_data = self.get() # the GET should pull back the same data as the POST assert response_data == data + + +class SyncDiscussionTopicsViewTests(ModuleStoreTestCase, APITestCase): + """ + Tests for SyncDiscussionTopicsView + """ + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.course_key_string = str(self.course.id) + self.staff_user = UserFactory.create(is_staff=True) + self.instructor_user = UserFactory.create() + self.student_user = UserFactory.create() + self.url = reverse('sync-discussion-topics', kwargs={'course_key_string': self.course_key_string}) + + # Mock the permission class for course team checking + self.original_has_permission = IsStaffOrCourseTeam.has_permission + IsStaffOrCourseTeam.has_permission = Mock(return_value=True) + + def tearDown(self): + # Restore original permission method + IsStaffOrCourseTeam.has_permission = self.original_has_permission + super().tearDown() + + @patch('openedx.core.djangoapps.discussions.views.update_unit_discussion_state_from_discussion_blocks') + def test_sync_discussion_topics_staff_user(self, mock_update): + """ + Test that staff users can sync discussion topics + """ + self.client.force_authenticate(user=self.staff_user) + response = self.client.post(self.url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['status'], 'success') + mock_update.assert_called_once_with( + course_key=CourseKey.from_string(self.course_key_string), + user_id=self.staff_user.id, + force=True, + async_topics=False + ) + + @patch('openedx.core.djangoapps.discussions.views.update_unit_discussion_state_from_discussion_blocks') + def test_sync_discussion_topics_course_team(self, mock_update): + """ + Test that course team members can sync discussion topics + """ + self.client.force_authenticate(user=self.instructor_user) + + # Mock the course team permission check + IsStaffOrCourseTeam.has_permission = Mock(return_value=True) + + response = self.client.post(self.url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['status'], 'success') + mock_update.assert_called_once() + + def test_sync_discussion_topics_unauthorized(self): + """ + Test that unauthorized users cannot sync discussion topics + """ + # Don't authenticate the request + response = self.client.post(self.url) + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_sync_discussion_topics_forbidden(self): + """ + Test that authenticated but unauthorized users cannot sync discussion topics + """ + self.client.force_authenticate(user=self.student_user) + + # Mock the course team permission check to return False + IsStaffOrCourseTeam.has_permission = Mock(return_value=False) + + response = self.client.post(self.url) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_invalid_http_method(self): + """ + Test that only POST method is allowed + """ + self.client.force_authenticate(user=self.staff_user) + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) diff --git a/openedx/core/djangoapps/discussions/urls.py b/openedx/core/djangoapps/discussions/urls.py index 7a68616ce0..6b70ca5f98 100644 --- a/openedx/core/djangoapps/discussions/urls.py +++ b/openedx/core/djangoapps/discussions/urls.py @@ -1,11 +1,15 @@ """ Configure URL endpoints for the djangoapp """ -from django.urls import re_path from django.conf import settings +from django.urls import re_path -from .views import CombinedDiscussionsConfigurationView, DiscussionsConfigurationSettingsView, DiscussionsProvidersView - +from .views import ( + CombinedDiscussionsConfigurationView, + DiscussionsConfigurationSettingsView, + DiscussionsProvidersView, + SyncDiscussionTopicsView +) urlpatterns = [ re_path( @@ -23,4 +27,9 @@ urlpatterns = [ DiscussionsProvidersView.as_view(), name='discussions-providers', ), + re_path( + fr'^v0/course/{settings.COURSE_KEY_PATTERN}/sync_discussion_topics$', + SyncDiscussionTopicsView.as_view(), + name='sync-discussion-topics', + ), ] diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 9d27d8f5f8..07faffb9c1 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -6,7 +6,9 @@ from typing import Dict import edx_api_doc_tools as apidocs from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from opaque_keys.edx.keys import CourseKey from rest_framework.exceptions import ValidationError +from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -14,13 +16,12 @@ from rest_framework.views import APIView from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import validate_course_key + from .config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS from .models import AVAILABLE_PROVIDER_MAP, DiscussionsConfiguration, Features, Provider from .permissions import IsStaffOrCourseTeam, check_course_permissions -from .serializers import ( - DiscussionsConfigurationSerializer, - DiscussionsProvidersSerializer, -) +from .serializers import DiscussionsConfigurationSerializer, DiscussionsProvidersSerializer +from .tasks import update_unit_discussion_state_from_discussion_blocks class DiscussionsConfigurationSettingsView(APIView): @@ -251,3 +252,34 @@ class CombinedDiscussionsConfigurationView(DiscussionsConfigurationSettingsView) }, } ) + + +class SyncDiscussionTopicsView(APIView): + """ + View for syncing discussion topics for a course. + """ + authentication_classes = (BearerAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsStaffOrCourseTeam) + + def post(self, request, course_key_string): + """ + Sync discussion topics for the course based on data in the request. + + Args: + request (Request): a DRF request + course_key_string (str): a course key string + + Returns: + Response: modified course configuration data + """ + update_unit_discussion_state_from_discussion_blocks( + course_key=CourseKey.from_string(course_key_string), + user_id=request.user.id, + force=True, + async_topics=False + ) + + return Response({ + "status": "success", + "message": "Discussion topics synced successfully." + })