feat: added API to sync discussions topics (#36529)
This commit is contained in:
@@ -181,7 +181,12 @@ def is_discussable_unit(unit, store, enable_graded_units, subsection):
|
|||||||
return True
|
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.
|
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.
|
course_key (CourseKey): CourseKey for course.
|
||||||
user_id (int): User id for the user performing this operation.
|
user_id (int): User id for the user performing this operation.
|
||||||
force (bool): Force migration of data even if not using legacy provider
|
force (bool): Force migration of data even if not using legacy provider
|
||||||
|
async_topics (bool): If True, run the task asynchronously.
|
||||||
"""
|
"""
|
||||||
store = modulestore()
|
store = modulestore()
|
||||||
course = store.get_course(course_key)
|
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.enable_graded_units = enable_graded_subsections
|
||||||
discussion_config.unit_level_visibility = True
|
discussion_config.unit_level_visibility = True
|
||||||
discussion_config.save()
|
discussion_config.save()
|
||||||
# added delay of 30 minutes to allow for the course to be published
|
|
||||||
update_discussions_settings_from_course_task.apply_async(
|
if async_topics:
|
||||||
args=[str(course_key), [str(unit) for unit in discussable_units]],
|
# added delay of 30 minutes to allow for the course to be published
|
||||||
countdown=1800,
|
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],
|
||||||
|
)
|
||||||
|
|||||||
@@ -5,28 +5,26 @@ Test app view logic
|
|||||||
import itertools
|
import itertools
|
||||||
from contextlib import contextmanager
|
from contextlib import contextmanager
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
from unittest.mock import Mock, patch
|
||||||
|
|
||||||
import ddt
|
import ddt
|
||||||
from django.core.exceptions import ValidationError
|
from django.core.exceptions import ValidationError
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
from edx_toggles.toggles.testutils import override_waffle_flag
|
from edx_toggles.toggles.testutils import override_waffle_flag
|
||||||
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
|
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
|
||||||
|
from opaque_keys.edx.keys import CourseKey
|
||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
from rest_framework.test import APITestCase
|
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 import ModuleStoreEnum
|
||||||
from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase
|
from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase
|
||||||
from xmodule.modulestore.tests.factories import CourseFactory
|
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 ..config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS
|
||||||
|
from ..models import AVAILABLE_PROVIDER_MAP, DEFAULT_CONFIG_ENABLED, Provider, get_default_provider_type
|
||||||
from ..models import (
|
from ..permissions import IsStaffOrCourseTeam
|
||||||
AVAILABLE_PROVIDER_MAP,
|
|
||||||
DEFAULT_CONFIG_ENABLED,
|
|
||||||
Provider,
|
|
||||||
get_default_provider_type,
|
|
||||||
)
|
|
||||||
|
|
||||||
DATA_LEGACY_COHORTS = {
|
DATA_LEGACY_COHORTS = {
|
||||||
'divided_inline_discussions': [],
|
'divided_inline_discussions': [],
|
||||||
@@ -830,3 +828,91 @@ class PIISettingsAPITests(DataTest):
|
|||||||
response_data = self.get()
|
response_data = self.get()
|
||||||
# the GET should pull back the same data as the POST
|
# the GET should pull back the same data as the POST
|
||||||
assert response_data == data
|
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)
|
||||||
|
|||||||
@@ -1,11 +1,15 @@
|
|||||||
"""
|
"""
|
||||||
Configure URL endpoints for the djangoapp
|
Configure URL endpoints for the djangoapp
|
||||||
"""
|
"""
|
||||||
from django.urls import re_path
|
|
||||||
from django.conf import settings
|
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 = [
|
urlpatterns = [
|
||||||
re_path(
|
re_path(
|
||||||
@@ -23,4 +27,9 @@ urlpatterns = [
|
|||||||
DiscussionsProvidersView.as_view(),
|
DiscussionsProvidersView.as_view(),
|
||||||
name='discussions-providers',
|
name='discussions-providers',
|
||||||
),
|
),
|
||||||
|
re_path(
|
||||||
|
fr'^v0/course/{settings.COURSE_KEY_PATTERN}/sync_discussion_topics$',
|
||||||
|
SyncDiscussionTopicsView.as_view(),
|
||||||
|
name='sync-discussion-topics',
|
||||||
|
),
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -6,7 +6,9 @@ from typing import Dict
|
|||||||
import edx_api_doc_tools as apidocs
|
import edx_api_doc_tools as apidocs
|
||||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||||
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
|
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.exceptions import ValidationError
|
||||||
|
from rest_framework.permissions import IsAuthenticated
|
||||||
from rest_framework.request import Request
|
from rest_framework.request import Request
|
||||||
from rest_framework.response import Response
|
from rest_framework.response import Response
|
||||||
from rest_framework.views import APIView
|
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.djangoapps.content.course_overviews.models import CourseOverview
|
||||||
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
||||||
from openedx.core.lib.api.view_utils import validate_course_key
|
from openedx.core.lib.api.view_utils import validate_course_key
|
||||||
|
|
||||||
from .config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS
|
from .config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS
|
||||||
from .models import AVAILABLE_PROVIDER_MAP, DiscussionsConfiguration, Features, Provider
|
from .models import AVAILABLE_PROVIDER_MAP, DiscussionsConfiguration, Features, Provider
|
||||||
from .permissions import IsStaffOrCourseTeam, check_course_permissions
|
from .permissions import IsStaffOrCourseTeam, check_course_permissions
|
||||||
from .serializers import (
|
from .serializers import DiscussionsConfigurationSerializer, DiscussionsProvidersSerializer
|
||||||
DiscussionsConfigurationSerializer,
|
from .tasks import update_unit_discussion_state_from_discussion_blocks
|
||||||
DiscussionsProvidersSerializer,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
class DiscussionsConfigurationSettingsView(APIView):
|
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."
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user