diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 44bfdea32908..76711eaa4ddc 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -145,7 +145,10 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= if CoursewareSearchIndexer.indexing_is_enabled() and CourseAboutSearchIndexer.indexing_is_enabled(): update_search_index.delay(course_key_str, datetime.now(UTC).isoformat()) - update_discussions_settings_from_course_task.delay(course_key_str) + update_discussions_settings_from_course_task.apply_async( + args=[course_key_str], + countdown=settings.DISCUSSION_SETTINGS['COURSE_PUBLISH_TASK_DELAY'], + ) # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index 6254579d6835..9a5f042b4952 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -138,7 +138,8 @@ def call_view( "group_id": self.student_cohort.id, "closed": False, "type": "thread", - "commentable_id": "non_team_dummy_id" + "commentable_id": "non_team_dummy_id", + "body": "test body", } ) request = RequestFactory().post("dummy_url", post_params or {}) @@ -411,7 +412,7 @@ def test_create_thread(self, mock_request): self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.split, 3, 6, 38), + (ModuleStoreEnum.Type.split, 3, 6, 42), ) @ddt.unpack @count_queries @@ -527,6 +528,7 @@ def test_delete_thread(self, mock_request): self._set_mock_request_data(mock_request, { "user_id": str(self.student.id), "closed": False, + "body": "test body", }) test_thread_id = "test_thread_id" request = RequestFactory().post("dummy_url", {"id": test_thread_id}) @@ -545,6 +547,7 @@ def test_delete_comment(self, mock_request): self._set_mock_request_data(mock_request, { "user_id": str(self.student.id), "closed": False, + "body": "test body", }) test_comment_id = "test_comment_id" request = RequestFactory().post("dummy_url", {"id": test_comment_id}) @@ -1594,7 +1597,8 @@ def test_delete_comment(self, user, comment_author, commentable_id, status_code, "commentable_id": commentable_id, "user_id": str(comment_author.id), "username": comment_author.username, - "course_id": str(self.course.id) + "course_id": str(self.course.id), + "body": "test body", }) response = self.client.post( @@ -1663,7 +1667,7 @@ def test_comment_actions(self, user, commentable_id, status_code, mock_request): commentable_id = getattr(self, commentable_id) self._setup_mock( user, mock_request, - {"closed": False, "commentable_id": commentable_id, "thread_id": "dummy_thread"}, + {"closed": False, "commentable_id": commentable_id, "thread_id": "dummy_thread", "body": 'dummy body'}, ) for action in ["upvote_comment", "downvote_comment", "un_flag_abuse_for_comment", "flag_abuse_for_comment"]: response = self.client.post( @@ -1684,7 +1688,7 @@ def test_threads_actions(self, user, commentable_id, status_code, mock_request): commentable_id = getattr(self, commentable_id) self._setup_mock( user, mock_request, - {"closed": False, "commentable_id": commentable_id}, + {"closed": False, "commentable_id": commentable_id, "body": "dummy body"}, ) for action in ["upvote_thread", "downvote_thread", "un_flag_abuse_for_thread", "flag_abuse_for_thread", "follow_thread", "unfollow_thread"]: diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index c842b2271446..d88c6e063a4f 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -167,6 +167,17 @@ def track_voted_event(request, course, obj, vote_value, undo_vote=False): track_forum_event(request, event_name, course, obj, event_data) +def track_forum_search_event(request, course, search_event_data): + """ + Send analytics event for discussions related search. + """ + event_name = 'edx.forum.searched' + + context = contexts.course_context_from_course_id(course.id) + with tracker.get_tracker().context(event_name, context): + tracker.emit(event_name, search_event_data) + + def track_thread_viewed_event(request, course, thread): """ Send analytics event for a viewed thread. @@ -263,6 +274,101 @@ def track_comment_deleted_event(request, course, comment): track_forum_event(request, event_name, course, comment, event_data) +def track_thread_reported_event(request, course, thread): + """ + Send analytics event for a reported thread. + """ + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='reported') + event_data = { + 'body': thread.body[:TRACKING_MAX_FORUM_BODY], + 'truncated': len(thread.body) > TRACKING_MAX_FORUM_BODY, + 'content_type': 'Post', + 'commentable_id': thread.get('commentable_id', ''), + 'thread_type': thread.get('thread_type', ''), + 'group_id': thread.get('group_id', ''), + } + if hasattr(thread, 'username'): + event_data['target_username'] = thread.get('username', '') + add_truncated_title_to_event_data(event_data, thread.get('title', '')) + track_forum_event(request, event_name, course, thread, event_data) + + +def track_comment_reported_event(request, course, comment): + """ + Send analytics event for a reported response or 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 = { + 'body': comment.body[:TRACKING_MAX_FORUM_BODY], + 'truncated': len(comment.body) > TRACKING_MAX_FORUM_BODY, + 'commentable_id': comment.get('commentable_id', ''), + 'content_type': obj_type.capitalize(), + } + if hasattr(comment, 'username'): + event_data['target_username'] = comment.get('username', '') + track_forum_event(request, event_name, course, comment, event_data) + + +def track_thread_unreported_event(request, course, thread): + """ + Send analytics event for a unreported thread. + """ + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='unreported') + event_data = { + 'body': thread.body[:TRACKING_MAX_FORUM_BODY], + 'truncated': len(thread.body) > TRACKING_MAX_FORUM_BODY, + 'content_type': 'Post', + 'commentable_id': thread.get('commentable_id', ''), + 'reported_status_cleared': not bool(thread.get('abuse_flaggers', [])), + 'thread_type': thread.get('thread_type', ''), + 'group_id': thread.get('group_id', ''), + + } + if hasattr(thread, 'username'): + event_data['target_username'] = thread.get('username', '') + add_truncated_title_to_event_data(event_data, thread.get('title', '')) + track_forum_event(request, event_name, course, thread, event_data) + + +def track_comment_unreported_event(request, course, comment): + """ + Send analytics event for a unreported response or 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 = { + 'body': comment.body[:TRACKING_MAX_FORUM_BODY], + 'truncated': len(comment.body) > TRACKING_MAX_FORUM_BODY, + 'commentable_id': comment.get('commentable_id', ''), + 'content_type': obj_type.capitalize(), + 'reported_status_cleared': not bool(comment.get('abuse_flaggers', [])), + } + if hasattr(comment, 'username'): + event_data['target_username'] = comment.get('username', '') + track_forum_event(request, event_name, course, comment, event_data) + + +def track_discussion_reported_event(request, course, cc_content): + """ + Helper method for discussion reported events. + """ + if cc_content.type == 'thread': + track_thread_reported_event(request, course, cc_content) + else: + track_comment_reported_event(request, course, cc_content) + + +def track_discussion_unreported_event(request, course, cc_content): + """ + Helper method for discussion unreported events. + """ + if cc_content.type == 'thread': + track_thread_unreported_event(request, course, cc_content) + else: + track_comment_unreported_event(request, course, cc_content) + + def permitted(func): """ View decorator to verify the user is authorized to access this endpoint. @@ -418,11 +524,11 @@ def update_thread(request, course_id, thread_id): user = request.user # The following checks should avoid issues we've seen during deploys, where end users are hitting an updated server # while their browser still has the old client code. This will avoid erasing present values in those cases. + course = get_course_with_access(user, 'load', course_key) if "thread_type" in request.POST: thread.thread_type = request.POST["thread_type"] if "commentable_id" in request.POST: commentable_id = request.POST["commentable_id"] - course = get_course_with_access(user, 'load', course_key) if thread_context == "course" and not discussion_category_id_access(course, user, commentable_id): return JsonError(_("Topic doesn't exist")) else: @@ -432,6 +538,7 @@ def update_thread(request, course_id, thread_id): thread_edited.send(sender=None, user=user, post=thread) + track_thread_edited_event(request, course, thread, None) if request.is_ajax(): return ajax_content_response(request, course_key, thread.to_dict()) else: @@ -510,9 +617,12 @@ def delete_thread(request, course_id, thread_id): this is ajax only """ course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) thread = cc.Thread.find(thread_id) thread.delete() thread_deleted.send(sender=None, user=request.user, post=thread) + + track_thread_deleted_event(request, course, thread) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -525,6 +635,7 @@ def update_comment(request, course_id, comment_id): handles static and ajax submissions """ course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) comment = cc.Comment.find(comment_id) if 'body' not in request.POST or not request.POST['body'].strip(): return JsonError(_("Body can't be empty")) @@ -533,6 +644,7 @@ def update_comment(request, course_id, comment_id): comment_edited.send(sender=None, user=request.user, post=comment) + track_comment_edited_event(request, course, comment, None) if request.is_ajax(): return ajax_content_response(request, course_key, comment.to_dict()) else: @@ -566,10 +678,13 @@ def openclose_thread(request, course_id, thread_id): ajax only """ course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) thread = cc.Thread.find(thread_id) - thread.closed = request.POST.get('closed', 'false').lower() == 'true' + close_thread = request.POST.get('closed', 'false').lower() == 'true' + thread.closed = close_thread thread.save() + track_thread_lock_unlock_event(request, course, thread, None, close_thread) return JsonResponse({ 'content': prepare_content(thread.to_dict(), course_key), 'ability': get_ability(course_key, thread.to_dict(), request.user), @@ -598,9 +713,11 @@ def delete_comment(request, course_id, comment_id): ajax only """ course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) comment = cc.Comment.find(comment_id) comment.delete() comment_deleted.send(sender=None, user=request.user, post=comment) + track_comment_deleted_event(request, course, comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -681,9 +798,10 @@ def flag_abuse_for_thread(request, course_id, thread_id): """ course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) + course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) thread.flagAbuse(user, thread) - + track_discussion_reported_event(request, course, thread) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -704,7 +822,7 @@ def un_flag_abuse_for_thread(request, course_id, thread_id): has_access(request.user, 'staff', course) ) thread.unFlagAbuse(user, thread, remove_all) - + track_discussion_unreported_event(request, course, thread) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -718,8 +836,10 @@ def flag_abuse_for_comment(request, course_id, comment_id): """ course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) + course = get_course_by_id(course_key) comment = cc.Comment.find(comment_id) comment.flagAbuse(user, comment) + track_discussion_reported_event(request, course, comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -740,6 +860,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): ) comment = cc.Comment.find(comment_id) comment.unFlagAbuse(user, comment, remove_all) + track_discussion_unreported_event(request, course, comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index e73278fb8818..d9e53ceafb1e 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -4,11 +4,15 @@ from __future__ import annotations import itertools +import re from collections import defaultdict +from datetime import datetime from enum import Enum from typing import Dict, Iterable, List, Literal, Optional, Set, Tuple from urllib.parse import urlencode, urlunparse +from pytz import UTC + from django.conf import settings from django.contrib.auth import get_user_model @@ -17,59 +21,59 @@ from django.http import Http404 from django.urls import reverse from edx_django_utils.monitoring import function_trace -from eventtracking import tracker from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import CourseKey from rest_framework import status from rest_framework.exceptions import PermissionDenied -from rest_framework.response import Response from rest_framework.request import Request - -from xmodule.course_module import CourseBlock -from xmodule.modulestore.django import modulestore -from xmodule.tabs import CourseTabList +from rest_framework.response import Response from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE, ENABLE_LEARNERS_TAB_IN_DISCUSSIONS_MFE from lms.djangoapps.discussion.toggles_utils import reported_content_email_notification_enabled -from lms.djangoapps.discussion.views import is_user_moderator +from lms.djangoapps.discussion.views import is_privileged_user from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, DiscussionTopicLink, Provider from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks from openedx.core.djangoapps.django_comment_common import comment_client from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment from openedx.core.djangoapps.django_comment_common.comment_client.course import ( get_course_commentable_counts, - get_course_user_stats, + get_course_user_stats ) from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread -from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError, \ - CommentClient500Error +from openedx.core.djangoapps.django_comment_common.comment_client.utils import ( + CommentClient500Error, + CommentClientRequestError +) from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, CourseDiscussionSettings, - Role, + Role ) from openedx.core.djangoapps.django_comment_common.signals import ( comment_created, comment_deleted, comment_edited, + comment_flagged, comment_voted, thread_created, thread_deleted, thread_edited, - thread_voted, thread_flagged, - comment_flagged, + thread_voted ) from openedx.core.djangoapps.user_api.accounts.api import get_account_settings from openedx.core.lib.exceptions import CourseNotFoundError, DiscussionNotFoundError, PageNotFoundError -from ..config.waffle import ENABLE_LEARNERS_STATS +from xmodule.course_module import CourseBlock +from xmodule.modulestore.django import modulestore +from xmodule.tabs import CourseTabList +from ..config.waffle import ENABLE_LEARNERS_STATS from ..django_comment_client.base.views import ( track_comment_created_event, track_comment_deleted_event, @@ -77,12 +81,15 @@ track_thread_deleted_event, track_thread_viewed_event, track_voted_event, + track_discussion_reported_event, + track_discussion_unreported_event, + track_forum_search_event ) from ..django_comment_client.utils import ( get_group_id_for_user, get_user_role_names, has_discussion_privileges, - is_commentable_divided, + is_commentable_divided ) from ..toggles import ENABLE_DISCUSSION_MODERATION_REASON_CODES from .exceptions import CommentNotFoundError, DiscussionBlackOutException, DiscussionDisabledError, ThreadNotFoundError @@ -92,7 +99,7 @@ can_delete, get_editable_fields, get_initializable_comment_fields, - get_initializable_thread_fields, + get_initializable_thread_fields ) from .serializers import ( CommentSerializer, @@ -101,14 +108,14 @@ ThreadSerializer, TopicOrdering, UserStatsSerializer, - get_context, + get_context ) - from .utils import ( + add_stats_for_users_with_no_discussion_content, discussion_open_for_user, + get_usernames_for_course, get_usernames_from_search_string, - add_stats_for_users_with_no_discussion_content, - set_attribute, get_usernames_for_course, + set_attribute ) User = get_user_model() @@ -378,12 +385,31 @@ def get_courseware_topics( courseware_topics = [] existing_topic_ids = set() + now = datetime.now(UTC) + discussion_xblocks = get_accessible_discussion_xblocks(course, request.user) xblocks_by_category = defaultdict(list) for xblock in discussion_xblocks: - xblocks_by_category[xblock.discussion_category].append(xblock) + if course.self_paced or (xblock.start and xblock.start < now): + xblocks_by_category[xblock.discussion_category].append(xblock) - for category in xblocks_by_category.keys(): + def sort_categories(category_list): + """ + Sorts the given iterable containing alphanumeric correctly. + Required arguments: + category_list -- list of categories. + """ + def convert(text): + if text.isdigit(): + return int(text) + return text + + def alphanum_key(key): + return [convert(c) for c in re.split('([0-9]+)', key)] + + return sorted(category_list, key=alphanum_key) + + for category in sort_categories(xblocks_by_category.keys()): children = [] for xblock in xblocks_by_category[category]: if not topic_ids or xblock.discussion_id in topic_ids: @@ -847,7 +873,7 @@ def get_thread_list( } if view: - if view in ["unread", "unanswered"]: + if view in ["unread", "unanswered", "unresponded"]: query_params[view] = "true" else: ValidationError({ @@ -892,7 +918,7 @@ def get_learner_active_thread_list(request, course_key, query_params): request: The django request objects used for build_absolute_uri course_key: The key of the course query_params: Parameters to fetch data from comments service. It must contain - user_id, course_id, page, per_page, group_id + user_id, course_id, page, per_page, group_id, count_flagged Returns: @@ -977,9 +1003,15 @@ def get_learner_active_thread_list(request, course_key, query_params): group_id = query_params.get('group_id', None) user_id = query_params.get('user_id', None) + count_flagged = query_params.get('count_flagged', None) if user_id is None: return Response({'detail': 'Invalid user id'}, status=status.HTTP_400_BAD_REQUEST) + if count_flagged and not context["has_moderation_privilege"]: + raise PermissionDenied("count_flagged can only be set by users with moderation roles.") + if "flagged" in query_params.keys() and not context["has_moderation_privilege"]: + raise PermissionDenied("Flagged filter is only available for moderators") + if group_id is None: comment_client_user = comment_client.User(id=user_id, course_id=course_key) else: @@ -1172,7 +1204,7 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con if field == "following": _handle_following_field(form_value, context["cc_requester"], cc_content) elif field == "abuse_flagged": - _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content) + _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content, request) elif field == "voted": _handle_voted_field(form_value, cc_content, api_content, request, context) elif field == "read": @@ -1191,11 +1223,13 @@ def _handle_following_field(form_value, user, cc_content): user.unfollow(cc_content) -def _handle_abuse_flagged_field(form_value, user, cc_content): +def _handle_abuse_flagged_field(form_value, user, cc_content, request): """mark or unmark thread/comment as abused""" course_key = CourseKey.from_string(cc_content.course_id) + course = get_course_with_access(request.user, 'load', course_key) if form_value: cc_content.flagAbuse(user, cc_content) + track_discussion_reported_event(request, course, cc_content) if ENABLE_DISCUSSIONS_MFE.is_enabled(course_key) and reported_content_email_notification_enabled( course_key): if cc_content.type == 'thread': @@ -1203,8 +1237,9 @@ def _handle_abuse_flagged_field(form_value, user, cc_content): else: comment_flagged.send(sender='flag_abuse_for_comment', user=user, post=cc_content) else: - remove_all = bool(is_user_moderator(course_key, User.objects.get(id=user.id))) + remove_all = bool(is_privileged_user(course_key, User.objects.get(id=user.id))) cc_content.unFlagAbuse(user, cc_content, remove_all) + track_discussion_unreported_event(request, course, cc_content) def _handle_voted_field(form_value, cc_content, api_content, request, context): @@ -1430,7 +1465,7 @@ def update_comment(request, comment_id, update_data): return api_comment -def get_thread(request, thread_id, requested_fields=None): +def get_thread(request, thread_id, requested_fields=None, course_id=None): """ Retrieve a thread. @@ -1441,6 +1476,8 @@ def get_thread(request, thread_id, requested_fields=None): thread_id: The id for the thread to retrieve + course_id: the id of the course the threads belongs to + requested_fields: Indicates which additional fields to return for thread. (i.e. ['profile_image']) """ @@ -1454,6 +1491,8 @@ def get_thread(request, thread_id, requested_fields=None): "user_id": str(request.user.id), } ) + if course_id and course_id != cc_thread.course_id: + raise ThreadNotFoundError("Thread not found.") return _serialize_discussion_entities(request, context, [cc_thread], requested_fields, DiscussionEntity.thread)[0] @@ -1665,7 +1704,7 @@ def get_course_discussion_user_stats( order_by = order_by or UserOrdering.BY_FLAGS else: order_by = order_by or UserOrdering.BY_ACTIVITY - if order_by != UserOrdering.BY_ACTIVITY: + if order_by == UserOrdering.BY_FLAGS: raise ValidationError({"order_by": "Invalid value"}) if not ENABLE_LEARNERS_STATS.is_enabled(course_key): @@ -1688,25 +1727,24 @@ def get_course_discussion_user_stats( comma_separated_usernames, matched_users_count, matched_users_pages = get_usernames_from_search_string( course_key, username_search_string, page, page_size ) + search_event_data = { + 'query': username_search_string, + 'search_type': 'Learner', + 'page': params.get('page'), + 'sort_key': params.get('sort_key'), + 'total_results': matched_users_count, + } + course = _get_course(course_key, request.user) + track_forum_search_event(request, course, search_event_data) if not comma_separated_usernames: return DiscussionAPIPagination(request, 0, 1).get_paginated_response({ "results": [], }) + params['usernames'] = comma_separated_usernames course_stats_response = get_course_user_stats(course_key, params) - tracker.emit( - 'edx.forum.searched', - { - 'query': username_search_string, - 'search_type': 'Learner', - 'page': params.get('page'), - 'sort_key': params.get('sort_key'), - 'total_results': course_stats_response.get('total_results'), - } - ) - if comma_separated_usernames: updated_course_stats = add_stats_for_users_with_no_discussion_content( course_stats_response["user_stats"], diff --git a/lms/djangoapps/discussion/rest_api/forms.py b/lms/djangoapps/discussion/rest_api/forms.py index b7a9286c2857..c7aa8c894b13 100644 --- a/lms/djangoapps/discussion/rest_api/forms.py +++ b/lms/djangoapps/discussion/rest_api/forms.py @@ -24,6 +24,7 @@ class UserOrdering(TextChoices): BY_ACTIVITY = 'activity' BY_FLAGS = 'flagged' + BY_RECENT_ACTIVITY = 'recency' class _PaginationForm(Form): @@ -58,7 +59,7 @@ class ThreadListGetForm(_PaginationForm): count_flagged = ExtendedNullBooleanField(required=False) flagged = ExtendedNullBooleanField(required=False) view = ChoiceField( - choices=[(choice, choice) for choice in ["unread", "unanswered"]], + choices=[(choice, choice) for choice in ["unread", "unanswered", "unresponded"]], required=False, ) order_by = ChoiceField( diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index a36ba5580ad9..8204ca0218f0 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -21,6 +21,7 @@ from opaque_keys.edx.locator import CourseLocator from pytz import UTC from rest_framework.exceptions import PermissionDenied + from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase @@ -408,17 +409,17 @@ def test_many(self): "B": {"id": "non-courseware-2"}, } self.store.update_item(self.course, self.user.id) - self.make_discussion_xblock("courseware-1", "A", "1") - self.make_discussion_xblock("courseware-2", "A", "2") - self.make_discussion_xblock("courseware-3", "B", "1") - self.make_discussion_xblock("courseware-4", "B", "2") - self.make_discussion_xblock("courseware-5", "C", "1") + self.make_discussion_xblock("courseware-1", "Week 1", "1") + self.make_discussion_xblock("courseware-2", "Week 1", "2") + self.make_discussion_xblock("courseware-3", "Week 10", "1") + self.make_discussion_xblock("courseware-4", "Week 10", "2") + self.make_discussion_xblock("courseware-5", "Week 9", "1") actual = self.get_course_topics() expected = { "courseware_topics": [ self.make_expected_tree( None, - "A", + "Week 1", [ self.make_expected_tree("courseware-1", "1"), self.make_expected_tree("courseware-2", "2"), @@ -426,17 +427,17 @@ def test_many(self): ), self.make_expected_tree( None, - "B", + "Week 9", + [self.make_expected_tree("courseware-5", "1")] + ), + self.make_expected_tree( + None, + "Week 10", [ self.make_expected_tree("courseware-3", "1"), self.make_expected_tree("courseware-4", "2"), ] ), - self.make_expected_tree( - None, - "C", - [self.make_expected_tree("courseware-5", "1")] - ), ], "non_courseware_topics": [ self.make_expected_tree("non-courseware-1", "A"), @@ -503,7 +504,6 @@ def test_access_control(self): ways in which a user may not have access are: * Module is visible to staff only - * Module has a start date in the future * Module is accessible only to a group the user is not in Also, there is a case that ensures that a category with no accessible @@ -575,12 +575,7 @@ def test_access_control(self): self.make_expected_tree("courseware-3", "Cohort B"), self.make_expected_tree("courseware-1", "Everybody"), ] - ), - self.make_expected_tree( - None, - "Second", - [self.make_expected_tree("courseware-5", "Future Start Date")] - ), + ) ], "non_courseware_topics": [ self.make_expected_tree("non-courseware-topic-id", "Test Topic"), @@ -605,7 +600,6 @@ def test_access_control(self): None, "Second", [ - self.make_expected_tree("courseware-5", "Future Start Date"), self.make_expected_tree("courseware-4", "Staff Only"), ] ), @@ -616,6 +610,44 @@ def test_access_control(self): } assert staff_actual == staff_expected + def test_un_released_discussion_topic(self): + """ + Test discussion topics that have not yet started + """ + staff = StaffFactory.create(course_key=self.course.id) + with self.store.bulk_operations(self.course.id, emit_signals=False): + self.store.update_item(self.course, self.user.id) + self.make_discussion_xblock( + "courseware-2", + "First", + "Released", + start=datetime.now(UTC) - timedelta(days=1) + ) + self.make_discussion_xblock( + "courseware-3", + "First", + "Future release", + start=datetime.now(UTC) + timedelta(days=1) + ) + + self.request.user = staff + staff_actual = self.get_course_topics() + staff_expected = { + "courseware_topics": [ + self.make_expected_tree( + None, + "First", + [ + self.make_expected_tree("courseware-2", "Released"), + ] + ), + ], + "non_courseware_topics": [ + self.make_expected_tree("non-courseware-topic-id", "Test Topic"), + ], + } + assert staff_actual == staff_expected + def test_discussion_topic(self): """ Tests discussion topic details against a requested topic id @@ -2799,7 +2831,8 @@ def test_vote_count_two_users( @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack - def test_abuse_flagged(self, old_flagged, new_flagged): + @mock.patch("eventtracking.tracker.emit") + def test_abuse_flagged(self, old_flagged, new_flagged, mock_emit): """ Test attempts to edit the "abuse_flagged" field. @@ -2826,12 +2859,36 @@ def test_abuse_flagged(self, old_flagged, new_flagged): assert httpretty.last_request().method == 'PUT' assert parsed_body(httpretty.last_request()) == {'user_id': [str(self.user.id)]} + expected_event_name = 'edx.forum.thread.reported' if new_flagged else 'edx.forum.thread.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_thread', + 'content_type': 'Post', + 'commentable_id': 'original_topic', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT], + 'target_username': self.user.username, + 'title_truncated': False, + 'title': 'Original Title', + 'thread_type': 'discussion', + 'group_id': None, + 'truncated': False, + } + if not new_flagged: + expected_event_data['reported_status_cleared'] = False + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( (False, True), (True, True), ) @ddt.unpack - def test_thread_un_abuse_flag_for_moderator_role(self, is_author, remove_all): + @mock.patch("eventtracking.tracker.emit") + def test_thread_un_abuse_flag_for_moderator_role(self, is_author, remove_all, mock_emit): """ Test un-abuse flag for moderator role. @@ -2852,6 +2909,28 @@ def test_thread_un_abuse_flag_for_moderator_role(self, is_author, remove_all): query_params.update({'all': ['True']}) assert parsed_body(httpretty.last_request()) == query_params + expected_event_name = 'edx.forum.thread.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_thread', + 'content_type': 'Post', + 'commentable_id': 'original_topic', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT, FORUM_ROLE_ADMINISTRATOR], + 'target_username': self.user.username, + 'title_truncated': False, + 'title': 'Original Title', + 'reported_status_cleared': False, + 'thread_type': 'discussion', + 'group_id': None, + 'truncated': False, + } + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + def test_invalid_field(self): self.register_thread() with pytest.raises(ValidationError) as assertion: @@ -3325,7 +3404,8 @@ def test_vote_count_two_users( @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack - def test_abuse_flagged(self, old_flagged, new_flagged): + @mock.patch("eventtracking.tracker.emit") + def test_abuse_flagged(self, old_flagged, new_flagged, mock_emit): """ Test attempts to edit the "abuse_flagged" field. @@ -3352,12 +3432,32 @@ def test_abuse_flagged(self, old_flagged, new_flagged): assert httpretty.last_request().method == 'PUT' assert parsed_body(httpretty.last_request()) == {'user_id': [str(self.user.id)]} + expected_event_name = 'edx.forum.response.reported' if new_flagged else 'edx.forum.response.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_comment', + 'content_type': 'Response', + 'commentable_id': 'dummy', + 'url': '', + 'truncated': False, + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT], + 'target_username': self.user.username, + } + if not new_flagged: + expected_event_data['reported_status_cleared'] = False + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( (False, True), (True, True), ) @ddt.unpack - def test_comment_un_abuse_flag_for_moderator_role(self, is_author, remove_all): + @mock.patch("eventtracking.tracker.emit") + def test_comment_un_abuse_flag_for_moderator_role(self, is_author, remove_all, mock_emit): """ Test un-abuse flag for moderator role. @@ -3378,6 +3478,24 @@ def test_comment_un_abuse_flag_for_moderator_role(self, is_author, remove_all): query_params.update({'all': ['True']}) assert parsed_body(httpretty.last_request()) == query_params + expected_event_name = 'edx.forum.response.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_comment', + 'content_type': 'Response', + 'commentable_id': 'dummy', + 'truncated': False, + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT, FORUM_ROLE_ADMINISTRATOR], + 'target_username': self.user.username, + 'reported_status_cleared': False, + } + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, @@ -3875,6 +3993,14 @@ def test_group_access(self, role_name, course_is_cohorted, thread_group_state): except ThreadNotFoundError: assert expected_error + def test_course_id_mismatch(self): + """ + Test if the api throws not found exception if course_id from params mismatches course_id in thread + """ + self.register_thread() + get_thread(self.request, self.thread_id, 'different_course_id') + assert ThreadNotFoundError + @mock.patch('lms.djangoapps.discussion.rest_api.api._get_course', mock.Mock()) class CourseTopicsV2Test(ModuleStoreTestCase): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index b856efeb836c..400c1be4aa11 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -955,7 +955,7 @@ def test_basic(self): "per_page": ["10"], }) - @ddt.data("unread", "unanswered") + @ddt.data("unread", "unanswered", "unresponded") def test_view_query(self, query): threads = [make_minimal_cs_thread()] self.register_get_user_response(self.user) @@ -1452,6 +1452,7 @@ def test_delete_nonexistent_thread(self): assert response.status_code == 404 +@ddt.ddt @httpretty.activate @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class LearnerThreadViewAPITest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): @@ -1601,6 +1602,160 @@ def test_not_authenticated(self): """ assert True + @ddt.data("None", "discussion", "question") + def test_thread_type_by(self, thread_type): + """ + Tests the thread_type parameter + + Arguments: + thread_type (str): Value of thread_type can be 'None', + 'discussion' and 'question' + """ + threads = [make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(self.user.id), + "username": self.user.username, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "title": "Test Title", + "body": "Test body", + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + })] + expected_cs_comments_response = { + "collection": threads, + "page": 1, + "num_pages": 1, + } + self.register_get_user_response(self.user) + self.register_user_active_threads(self.user.id, expected_cs_comments_response) + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "username": self.user.username, + "thread_type": thread_type, + } + ) + assert response.status_code == 200 + self.assert_last_query_params({ + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "page": ["1"], + "per_page": ["10"], + "thread_type": [thread_type], + "sort_key": ['activity'], + "count_flagged": ["False"] + }) + + @ddt.data( + ("last_activity_at", "activity"), + ("comment_count", "comments"), + ("vote_count", "votes") + ) + @ddt.unpack + def test_order_by(self, http_query, cc_query): + """ + Tests the order_by parameter for active threads + + Arguments: + http_query (str): Query string sent in the http request + cc_query (str): Query string used for the comments client service + """ + threads = [make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(self.user.id), + "username": self.user.username, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "title": "Test Title", + "body": "Test body", + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + })] + expected_cs_comments_response = { + "collection": threads, + "page": 1, + "num_pages": 1, + } + self.register_get_user_response(self.user) + self.register_user_active_threads(self.user.id, expected_cs_comments_response) + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "username": self.user.username, + "order_by": http_query, + } + ) + assert response.status_code == 200 + self.assert_last_query_params({ + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "page": ["1"], + "per_page": ["10"], + "sort_key": [cc_query], + "count_flagged": ["False"] + }) + + @ddt.data("flagged", "unanswered", "unread", "unresponded") + def test_status_by(self, post_status): + """ + Tests the post_status parameter + + Arguments: + post_status (str): Value of post_status can be 'flagged', + 'unanswered' and 'unread' + """ + threads = [make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(self.user.id), + "username": self.user.username, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "title": "Test Title", + "body": "Test body", + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + })] + expected_cs_comments_response = { + "collection": threads, + "page": 1, + "num_pages": 1, + } + self.register_get_user_response(self.user) + self.register_user_active_threads(self.user.id, expected_cs_comments_response) + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "username": self.user.username, + "status": post_status, + } + ) + if post_status == "flagged": + assert response.status_code == 403 + else: + assert response.status_code == 200 + self.assert_last_query_params({ + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "page": ["1"], + "per_page": ["10"], + post_status: ['True'], + "sort_key": ['activity'], + "count_flagged": ["False"] + }) + @ddt.ddt @httpretty.activate @@ -2960,9 +3115,11 @@ def test_moderator_user(self): @ddt.data( ("moderator", "flagged", "flagged"), ("moderator", "activity", "activity"), + ("moderator", "recency", "recency"), ("moderator", None, "flagged"), ("user", None, "activity"), ("user", "activity", "activity"), + ("user", "recency", "recency"), ) @ddt.unpack @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 3c3e4c545dab..daed977e2690 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -361,7 +361,7 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): * view: "unread" for threads the requesting user has not read, or "unanswered" for question threads with no marked answer. Only one - can be selected. + can be selected, or unresponded for discussion type posts with no response * requested_fields: (list) Indicates which additional fields to return for each thread. (supports 'profile_image') @@ -527,7 +527,8 @@ def retrieve(self, request, thread_id=None): Implements the GET method for thread ID """ requested_fields = request.GET.get('requested_fields') - return Response(get_thread(request, thread_id, requested_fields)) + course_id = request.GET.get('course_id') + return Response(get_thread(request, thread_id, requested_fields, course_id)) def create(self, request): """ @@ -571,6 +572,18 @@ class LearnerThreadView(APIView): * page: The (1-indexed) page to retrieve (default is 1) * page_size: The number of items per page (default is 10) + + * count_flagged: If True, return the count of flagged comments for each thread. + (can only be used by moderators or above) + + * thread_type: The type of thread to filter None, "discussion" or "question". + + * order_by: Sort order for threads "last_activity_at", "comment_count" or + "vote_count". + + * status: Filter for threads "flagged", "unanswered", "unread". + + * group_id: Filter threads w.r.t cohorts (Cohort ID). """ authentication_classes = ( @@ -590,6 +603,16 @@ def get(self, request, course_id=None): course_key = CourseKey.from_string(course_id) page_num = request.GET.get('page', 1) threads_per_page = request.GET.get('page_size', 10) + count_flagged = request.GET.get('count_flagged', False) + thread_type = request.GET.get('thread_type') + order_by = request.GET.get('order_by') + order_by_mapping = { + "last_activity_at": "activity", + "comment_count": "comments", + "vote_count": "votes" + } + order_by = order_by_mapping.get(order_by, 'activity') + post_status = request.GET.get('status', None) discussion_id = None username = request.GET.get('username', None) user = get_object_or_404(User, username=username) @@ -604,8 +627,19 @@ def get(self, request, course_id=None): "per_page": threads_per_page, "course_id": str(course_key), "user_id": user.id, - "group_id": group_id + "group_id": group_id, + "count_flagged": count_flagged, + "thread_type": thread_type, + "sort_key": order_by, } + if post_status: + if post_status not in ['flagged', 'unanswered', 'unread', 'unresponded']: + raise ValidationError({ + "status": [ + f"Invalid value. '{post_status}' must be 'flagged', 'unanswered', 'unread' or 'unresponded" + ] + }) + query_params[post_status] = True return get_learner_active_thread_list(request, course_key, query_params) diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index 90ece99da47a..7c2e68f31dc7 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -115,7 +115,6 @@ def send_ace_message_for_reported_content(context): # lint-amnesty, pylint: dis ) log.info(f'Sending forum reported content email notification with context {message_context}') ace.send(message) - # TODO: add tracking for reported content email def _track_notification_sent(message, context): diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html index a99739f07c48..be7bce1733e9 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html @@ -23,7 +23,6 @@ data-sort-preference="${sort_preference}" data-flag-moderator="${json.dumps(flag_moderator)}" data-user-group-id="${user_group_id}"> - <%include file="_switch_experience_fragment.html" />
## Breadcrumb navigation
diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index 12212e337c9c..4d14ea50748e 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -761,31 +761,16 @@ def is_privileged_user(course_key: CourseKey, user: User): """ Returns True if user has one of following course role Administrator, Moderator, Group Moderator, Community TA, - Global Staff, Course Instructor or Course Staff. + or Global Staff. """ forum_roles = [ FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, - FORUM_ROLE_ADMINISTRATOR + FORUM_ROLE_ADMINISTRATOR, ] has_course_role = Role.user_has_role_for_course(user, course_key, forum_roles) - return GlobalStaff().has_user(user) or is_course_staff(course_key, user) or has_course_role - - -def is_user_moderator(course_key: CourseKey, user: User): - """ - Returns True if user has one of following course role - Administrator, Moderator, Group Moderator, Community TA. - """ - forum_roles = [ - FORUM_ROLE_COMMUNITY_TA, - FORUM_ROLE_GROUP_MODERATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_ADMINISTRATOR - ] - has_course_role = Role.user_has_role_for_course(user, course_key, forum_roles) - return has_course_role + return GlobalStaff().has_user(user) or has_course_role class DiscussionBoardFragmentView(EdxFragmentView): diff --git a/lms/templates/discussion/_switch_experience_fragment.html b/lms/templates/discussion/_switch_experience_fragment.html deleted file mode 100644 index c718bed6dcad..000000000000 --- a/lms/templates/discussion/_switch_experience_fragment.html +++ /dev/null @@ -1,74 +0,0 @@ -## mako - -<%page expression_filter="h"/> - -<%! -from django.utils.translation import ugettext as _ -%> - -% if show_banner: -
- % if show_mfe: -
- ${_("Welcome to the new discussions experience. Please share your feedback.")} -
- - - - - - - ${_('View legacy experience')} - - -
-
- % else: -
- ${_("You are currently using the old discussions experience that will be deprecated soon. If you are facing any issues with the new experience, please share using the feedback button.")} -
- - ${_("Switch to new experience")} - - %endif - - % if share_feedback_url: - - ${_("Share feedback")} - - % endif -
-% endif - - - diff --git a/openedx/core/djangoapps/discussions/README.rst b/openedx/core/djangoapps/discussions/README.rst new file mode 100644 index 000000000000..5c1127d324cc --- /dev/null +++ b/openedx/core/djangoapps/discussions/README.rst @@ -0,0 +1,66 @@ +Discussions +=========== + +This Discussions app is responsible for providing support for configuring +discussion tools in the Open edX platform. This includes the in-built forum +tool that uses the `cs_comments_service`, but also other LTI-based tools. + + +Technical Overview +------------------ + +This app adds support for alternative Discussion providers, where a provider is +initially an LTI app that can be embedded in the discussion tab. + +A list of supported providers is included configured in this app, with the +features each app supports, its support level, and other details that are +returned by the API and drive the UI for selecting and comparing providers in +the course authoring MFE. + +Each context (course) has an associated `DiscussionConfiguration` object that +specifies the provider to use for that course, and includes other configuration +information for the course. For LTI-based providers, it's also linked to an +`LtiConfiguration` entry. + +The `plugin_configuration` field on this model is to store +provider/plugin-specific configuration that won't make sense for other plugins. + +Different parts of the course can be linked to different discussion topics. The +standard use case here to have a topic for each Unit in the course. The +`DiscussionTopicLink` model handles this association, and links a particular +usage key in a particular course to a topic. + +The new Discussion API is driven entirely by these `DiscussionTopicLinks` in +the database, so it's possible to have a third-party plugin that changes how +and where these topic links are created. + +When a course is published, the `listen_for_course_publish` signal handler is +called, and this signal in turn calls the +update_discussions_settings_from_course_task` in the background. + +That task goes through the module store and builds a +`CourseDiscussionConfigurationData` object that has all the relevant course +discussion configuration information such as the course key, the provider type, +whether in-context discussions are enabled, whether graded units are enabled, +when unit level visibility is enabled. Other plugin configuration and a list +of discussion contexts for which discussions are enabled. Each discussion +context has a usage key, a title (the units name) an external id +(the cs_comments_service id), it's ordering in the course, and additional +context. It then sends its own signal that has the discussion configuration +object attached. + +Finally, the handler for this discussion change signal, takes the information +from the discussion change signal and compares it to the topics in the +database, and does the following: + +- If it sees discussions contexts (units) without topics it creates new topics + link entries for the new units. This will happen if you create a new unit, + or enable discussions for a unit that previously had them disabled. + +- If it sees discussion topics without contexts it disables/archives them. + This could happen when a unit is deleted or if a unit that previously had + discussions enabled, now has discussions disabled. + +- If it sees any other change, i.e. unit name change etc. it applies that + change to the database as well. + diff --git a/openedx/core/djangoapps/discussions/handlers.py b/openedx/core/djangoapps/discussions/handlers.py index 8787da82cee2..f43aaf4b09eb 100644 --- a/openedx/core/djangoapps/discussions/handlers.py +++ b/openedx/core/djangoapps/discussions/handlers.py @@ -66,7 +66,7 @@ def update_course_discussion_config(configuration: CourseDiscussionConfiguration # If the section/subsection/unit a topic is in is deleted, add that context to title. topic_link.title = "{section}|{subsection}|{unit}".format(**topic_link.context) except KeyError: - # It's possible the context is if the topic link was created before the context field was added. + # It's possible the context is empty if the link was created before the context field was added. pass else: topic_link.enabled_in_context = True diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index 9cbfac57d599..1985da1c9aab 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -51,37 +51,42 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc provider_type = discussions_config.provider_type def iter_discussable_units(): - subsections = store.get_items(course_key, qualifiers={"category": "sequential"}) # Start at 99 so that the initial increment starts it at 100. # This leaves the first 100 slots for the course wide topics, which is only a concern if there are more # than that many. idx = 99 - for subsection in subsections: - section = store.get_item(subsection.parent) - for unit in subsection.get_children(): - # Increment index even for skipped units so that the index is more stable and won't change - # if settings change, only if a unit is added or removed. - idx += 1 - # If unit-level visibility is enabled and the unit doesn't have discussion enabled, skip it. - if unit_level_visibility and not getattr(unit, "discussion_enabled", False): + for section in course.get_children(): + if section.location.block_type != "chapter": + continue + for subsection in section.get_children(): + if subsection.location.block_type != "sequential": continue - # If the unit is in a graded section and graded sections aren't enabled skip it. - if subsection.graded and not enable_graded_units: - continue - # If the unit is an exam, skip it. - if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited: - continue - yield DiscussionTopicContext( - usage_key=unit.location, - title=unit.display_name, - group_id=None, - ordering=idx, - context={ - "section": section.display_name, - "subsection": subsection.display_name, - "unit": unit.display_name, - }, - ) + for unit in subsection.get_children(): + if unit.location.block_type != 'vertical': + continue + # Increment index even for skipped units so that the index is more stable and won't change + # if settings change, only if a unit is added or removed. + idx += 1 + # If unit-level visibility is enabled and the unit doesn't have discussion enabled, skip it. + if unit_level_visibility and not getattr(unit, "discussion_enabled", False): + continue + # If the unit is in a graded section and graded sections aren't enabled skip it. + if subsection.graded and not enable_graded_units: + continue + # If the unit is an exam, skip it. + if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited: + continue + yield DiscussionTopicContext( + usage_key=unit.location, + title=unit.display_name, + group_id=None, + ordering=idx, + context={ + "section": section.display_name, + "subsection": subsection.display_name, + "unit": unit.display_name, + }, + ) with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key): course = store.get_course(course_key)