diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index c70a801ca71f..377c01c1bf2b 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -12,15 +12,15 @@ from cms.djangoapps.contentstore.courseware_index import ( CourseAboutSearchIndexer, CoursewareSearchIndexer, - LibrarySearchIndexer + LibrarySearchIndexer, ) from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type from common.djangoapps.util.module_utils import yield_dynamic_descriptor_descendants from lms.djangoapps.grades.api import task_compute_all_grades_for_course from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines +from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task from openedx.core.lib.gating import api as gating_api from xmodule.modulestore.django import SignalHandler, modulestore - from .signals import GRADING_POLICY_CHANGED log = logging.getLogger(__name__) @@ -64,11 +64,13 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Push the course outline to learning_sequences asynchronously. update_outline_from_modulestore_task.delay(course_key_str) - # Finally call into the course search subsystem + # Finally, call into the course search subsystem # to kick off an indexing action 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) + @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument diff --git a/lms/djangoapps/course_wiki/tab.py b/lms/djangoapps/course_wiki/tab.py index 8c87767fe868..1bb352020bff 100644 --- a/lms/djangoapps/course_wiki/tab.py +++ b/lms/djangoapps/course_wiki/tab.py @@ -5,7 +5,7 @@ from django.conf import settings -from django.utils.translation import gettext_noop +from django.utils.translation import gettext_noop as _ from lms.djangoapps.courseware.tabs import EnrolledTab @@ -16,7 +16,7 @@ class WikiTab(EnrolledTab): """ type = "wiki" - title = gettext_noop('Wiki') + title = _('Wiki') view_name = "course_wiki" is_hideable = True is_default = False diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index 908ff91d521f..2e79706b825c 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/lms/djangoapps/edxnotes/plugins.py @@ -5,7 +5,7 @@ from django.conf import settings from django.contrib.auth import get_user_model -from django.utils.translation import ugettext_noop as _ +from django.utils.translation import gettext_noop as _ from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.django import modulestore diff --git a/lms/djangoapps/teams/plugins.py b/lms/djangoapps/teams/plugins.py index af699238e1dd..33542ba7cda9 100644 --- a/lms/djangoapps/teams/plugins.py +++ b/lms/djangoapps/teams/plugins.py @@ -5,7 +5,7 @@ from django.conf import settings from django.contrib.auth import get_user_model -from django.utils.translation import ugettext_noop as _ +from django.utils.translation import gettext_noop as _ from opaque_keys.edx.keys import CourseKey from lms.djangoapps.courseware.tabs import EnrolledTab diff --git a/openedx/core/djangoapps/discussions/apps.py b/openedx/core/djangoapps/discussions/apps.py index 2a820b162a66..ba0b66942586 100644 --- a/openedx/core/djangoapps/discussions/apps.py +++ b/openedx/core/djangoapps/discussions/apps.py @@ -30,3 +30,6 @@ class DiscussionsConfig(AppConfig): PluginSettings.CONFIG: { }, } + + def ready(self): + from . import handlers # pylint: disable=unused-import diff --git a/openedx/core/djangoapps/discussions/data.py b/openedx/core/djangoapps/discussions/data.py new file mode 100644 index 000000000000..4d0bc7fcc1a1 --- /dev/null +++ b/openedx/core/djangoapps/discussions/data.py @@ -0,0 +1,45 @@ +""" +Data classes for discussions +""" +from typing import List + +import attr +from opaque_keys.edx.keys import CourseKey, UsageKey + +# TODO: These data models will be moved to openedx_events. They are currently here to simplify the PR. + + +@attr.s(frozen=True) +class DiscussionTopicContext: + """ + Context for linking the external id for a discussion topic to its associated usage key. + + The ``title`` is cached to improve the performance, since otherwise we'd + need to look it up in the course structure each time. + + The ``group_id`` can be used for providers that don't internally support + cohorting but we can emulate that wuth different contexts for different groups. + """ + title = attr.ib(type=str) + usage_key = attr.ib(type=UsageKey, default=None) + group_id = attr.ib(type=int, default=None) + external_id = attr.ib(type=str, default=None) + + +@attr.s(frozen=True) +class CourseDiscussionConfigurationData: + """ + Course configuration information for discussions. + + Contains all the metadata needed to configure discussions for a course. + + The ``contexts`` field contains all the contexts for which discussion + is to be enabled. + """ + course_key = attr.ib(type=CourseKey) + provider_type = attr.ib(type=str) + enable_in_context = attr.ib(type=bool, default=True) + enable_graded_units = attr.ib(type=bool, default=False) + unit_level_visibility = attr.ib(type=bool, default=False) + plugin_configuration = attr.ib(type=dict, default={}) + contexts = attr.ib(type=List[DiscussionTopicContext], factory=list) diff --git a/openedx/core/djangoapps/discussions/handlers.py b/openedx/core/djangoapps/discussions/handlers.py new file mode 100644 index 000000000000..009b2a05b824 --- /dev/null +++ b/openedx/core/djangoapps/discussions/handlers.py @@ -0,0 +1,97 @@ +""" +Signal handlers for discussions events +""" +import logging +from uuid import uuid4 + +from django.db import transaction + +from openedx.core.djangoapps.discussions.data import CourseDiscussionConfigurationData +from openedx.core.djangoapps.discussions.models import ( + DEFAULT_PROVIDER_TYPE, + DiscussionTopicLink, + DiscussionsConfiguration, +) +from openedx.core.djangoapps.discussions.signals import COURSE_DISCUSSIONS_UPDATED + +log = logging.getLogger(__name__) + + +# pylint: disable=unused-argument +def handle_course_discussion_config_update(sender, configuration: CourseDiscussionConfigurationData, **kwargs): + """ + Updates the database models for course topics and configuration when settings are updated in the course. + + Args: + sender: Ignored + configuration (CourseDiscussionConfigurationData): configuration data for the course + + """ + update_course_discussion_config(configuration) + + +def update_course_discussion_config(configuration: CourseDiscussionConfigurationData): + """ + Update the database version of the configuration if it changes in the course structure. + + This function accepts a discussion configuration object that represents the current + configuration and applies that state to the database. It will go over the list of topic + links in the configuration, find the corresponding topic link in the database and apply + any changes if needed. If a new topic link has been introduced it will create an entry. + If a topic has been removed, it will deactivate the entry. + + When this runs on a new course it will create a new DiscussionConfiguration entry for + the course. + + Args: + configuration (CourseDiscussionConfigurationData): configuration data for the course + """ + course_key = configuration.course_key + provider_id = configuration.provider_type or DEFAULT_PROVIDER_TYPE + new_topic_map = { + (topic_context.usage_key or topic_context.external_id): topic_context + for topic_context in configuration.contexts + } + with transaction.atomic(): + log.info(f"Updating existing discussion topic links for {course_key}") + for topic_link in DiscussionTopicLink.objects.filter( + context_key=course_key, provider_id=provider_id, + ): + lookup_key = topic_link.usage_key or topic_link.external_id + topic_context = new_topic_map.pop(lookup_key, None) + # TODO: handle deleting topics that are no longer in use + # currently this will simply not work for course-wide topics since deleting the link will + # remove access to all posts in the topic. + if topic_context is None: + topic_link.enabled_in_context = False + else: + topic_link.enabled_in_context = True + topic_link.title = topic_context.title + topic_link.save() + log.info(f"Creating new discussion topic links for {course_key}") + + DiscussionTopicLink.objects.bulk_create([ + DiscussionTopicLink( + context_key=course_key, + usage_key=topic_context.usage_key, + title=topic_context.title, + provider_id=provider_id, + external_id=topic_context.external_id or uuid4(), + enabled_in_context=True, + ) + for topic_context in new_topic_map.values() + ]) + + if not DiscussionsConfiguration.objects.filter(context_key=course_key).exists(): + log.info(f"Course {course_key} doesn't have discussion configuration model yet. Creating a new one.") + DiscussionsConfiguration( + context_key=course_key, + provider_type=provider_id, + plugin_configuration=configuration.plugin_configuration, + enable_in_context=configuration.enable_in_context, + enable_graded_units=configuration.enable_graded_units, + unit_level_visibility=configuration.unit_level_visibility, + ).save() + + +COURSE_DISCUSSIONS_UPDATED.connect(handle_course_discussion_config_update) diff --git a/openedx/core/djangoapps/discussions/migrations/0007_add_discussion_topic_link.py b/openedx/core/djangoapps/discussions/migrations/0007_add_discussion_topic_link.py new file mode 100644 index 000000000000..4a4b4790925c --- /dev/null +++ b/openedx/core/djangoapps/discussions/migrations/0007_add_discussion_topic_link.py @@ -0,0 +1,57 @@ +# Generated by Django 3.2.8 on 2021-10-21 12:02 + +import django.db.models.deletion +import django_mysql.models +import opaque_keys.edx.django.models +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('course_groups', '0003_auto_20170609_1455'), + ('discussions', '0006_auto_20211006_0441'), + ] + + operations = [ + migrations.AlterField( + model_name='providerfilter', + name='allow', + field=django_mysql.models.ListCharField(models.CharField( + choices=[('legacy', 'legacy'), ('openedx', 'openedx'), ('ed-discuss', 'ed-discuss'), + ('inscribe', 'inscribe'), ('piazza', 'piazza'), ('yellowdig', 'yellowdig')], max_length=20), + blank=True, + help_text='Comma-separated list of providers to allow, eg: legacy,openedx,ed-discuss,inscribe,piazza,yellowdig', + max_length=63, size=3, verbose_name='Allow List'), + ), + migrations.AlterField( + model_name='providerfilter', + name='deny', + field=django_mysql.models.ListCharField(models.CharField( + choices=[('legacy', 'legacy'), ('openedx', 'openedx'), ('ed-discuss', 'ed-discuss'), + ('inscribe', 'inscribe'), ('piazza', 'piazza'), ('yellowdig', 'yellowdig')], max_length=20), + blank=True, + help_text='Comma-separated list of providers to deny, eg: legacy,openedx,ed-discuss,inscribe,piazza,yellowdig', + max_length=63, size=3, verbose_name='Deny List'), + ), + migrations.CreateModel( + name='DiscussionTopicLink', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('context_key', opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, + help_text='Context key for context in which this discussion topic exists.', max_length=255, + verbose_name='Learning Context Key')), + ('usage_key', opaque_keys.edx.django.models.UsageKeyField(blank=True, db_index=True, + help_text='Usage key for in-context discussion topic. Set to null for course-level topics.', + max_length=255, null=True)), + ('title', models.CharField(help_text='Title for discussion topic.', max_length=255)), + ('provider_id', models.CharField(help_text='Provider id for discussion provider.', max_length=32)), + ('external_id', models.CharField(db_index=True, + help_text='Discussion context ID in external forum provider. e.g. commentable_id for cs_comments_service.', + max_length=255)), + ('enabled_in_context', models.BooleanField(default=True, + help_text='Whether this topic should be shown in-context in the course.')), + ('group', models.ForeignKey(blank=True, help_text='Group for divided discussions.', null=True, + on_delete=django.db.models.deletion.SET_NULL, to='course_groups.courseusergroup')), + ], + ), + ] diff --git a/openedx/core/djangoapps/discussions/models.py b/openedx/core/djangoapps/discussions/models.py index 99a56df325f2..ab2505715917 100644 --- a/openedx/core/djangoapps/discussions/models.py +++ b/openedx/core/djangoapps/discussions/models.py @@ -16,12 +16,13 @@ from jsonfield import JSONField from lti_consumer.models import LtiConfiguration from model_utils.models import TimeStampedModel -from opaque_keys.edx.django.models import LearningContextKeyField +from opaque_keys.edx.django.models import LearningContextKeyField, UsageKeyField from opaque_keys.edx.keys import CourseKey from simple_history.models import HistoricalRecords from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers log = logging.getLogger(__name__) @@ -118,6 +119,7 @@ def pii_sharing_required_message(provider_name): Features.COURSE_COHORT_SUPPORT.value, Features.RESEARCH_DATA_EVENTS.value, ], + 'supports_lti': False, 'external_links': ProviderExternalLinks( learn_more='', configuration='', @@ -128,6 +130,33 @@ def pii_sharing_required_message(provider_name): 'messages': [], 'has_full_support': True }, + 'openedx': { + 'features': [ + Features.BASIC_CONFIGURATION.value, + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, + Features.QUESTION_DISCUSSION_SUPPORT.value, + Features.COMMUNITY_TA_SUPPORT.value, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value, + Features.AUTOMATIC_LEARNER_ENROLLMENT.value, + Features.ANONYMOUS_POSTING.value, + Features.INTERNATIONALIZATION_SUPPORT.value, + Features.WCAG_2_0_SUPPORT.value, + Features.BLACKOUT_DISCUSSION_DATES.value, + Features.COURSE_COHORT_SUPPORT.value, + Features.RESEARCH_DATA_EVENTS.value, + ], + 'supports_lti': False, + 'external_links': ProviderExternalLinks( + learn_more='', + configuration='', + general='', + accessibility='', + contact_email='', + )._asdict(), + 'messages': [], + 'has_full_support': True, + 'supports_in_context_discussions': True, + }, 'ed-discuss': { 'features': [ Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, @@ -144,6 +173,7 @@ def pii_sharing_required_message(provider_name): Features.IN_PLATFORM_NOTIFICATIONS.value, Features.USER_MENTIONS.value, ], + 'supports_lti': True, 'external_links': ProviderExternalLinks( learn_more='', configuration='', @@ -171,6 +201,7 @@ def pii_sharing_required_message(provider_name): Features.IN_PLATFORM_NOTIFICATIONS.value, Features.DISCUSSION_CONTENT_PROMPTS.value, ], + 'supports_lti': True, 'external_links': ProviderExternalLinks( learn_more='', configuration='', @@ -194,6 +225,7 @@ def pii_sharing_required_message(provider_name): Features.WCAG_2_0_SUPPORT.value, Features.BLACKOUT_DISCUSSION_DATES.value, ], + 'supports_lti': True, 'external_links': ProviderExternalLinks( learn_more='https://piazza.com/product/overview', configuration='https://support.piazza.com/support/solutions/articles/48001065447-configure-piazza-within-edx', # pylint: disable=line-too-long @@ -219,6 +251,7 @@ def pii_sharing_required_message(provider_name): Features.DIRECT_MESSAGES_FROM_INSTRUCTORS.value, Features.USER_MENTIONS.value, ], + 'supports_lti': True, 'external_links': ProviderExternalLinks( learn_more='https://www.youtube.com/watch?v=ZACief-qMwY', configuration='', @@ -402,6 +435,12 @@ def __str__(self): enabled=self.enabled, ) + def supports_in_context_discussions(self): + """ + Returns is the provider supports in-context discussions + """ + return AVAILABLE_PROVIDER_MAP.get(self.provider_type, {}).get('supports_in_context_discussions', False) + def supports(self, feature: str) -> bool: """ Check if the provider supports some feature @@ -412,7 +451,7 @@ def supports(self, feature: str) -> bool: def supports_lti(self) -> bool: """Returns a boolean indicating if the provider supports lti discussion view.""" - return self.provider_type != DEFAULT_PROVIDER_TYPE + return AVAILABLE_PROVIDER_MAP.get(self.provider_type, {}).get('supports_lti', False) @classmethod def is_enabled(cls, context_key: CourseKey) -> bool: @@ -510,3 +549,58 @@ def is_enabled(cls, program_uuid) -> bool: return configuration.enabled except cls.DoesNotExist: return False + + +class DiscussionTopicLink(models.Model): + """ + A model linking discussion topics ids to the part of a course they are linked to. + """ + context_key = LearningContextKeyField( + db_index=True, + max_length=255, + # Translators: A key specifying a course, library, program, + # website, or some other collection of content where learning + # happens. + verbose_name=_("Learning Context Key"), + help_text=_("Context key for context in which this discussion topic exists.") + ) + usage_key = UsageKeyField( + db_index=True, + max_length=255, + null=True, + blank=True, + help_text=_("Usage key for in-context discussion topic. Set to null for course-level topics.") + ) + title = models.CharField( + max_length=255, + help_text=_("Title for discussion topic.") + ) + group = models.ForeignKey( + CourseUserGroup, + null=True, + blank=True, + on_delete=models.SET_NULL, + help_text=_("Group for divided discussions.") + ) + provider_id = models.CharField( + max_length=32, + help_text=_("Provider id for discussion provider.") + ) + external_id = models.CharField( + db_index=True, + max_length=255, + help_text=_("Discussion context ID in external forum provider. e.g. commentable_id for cs_comments_service.") + ) + enabled_in_context = models.BooleanField( + default=True, + help_text=_("Whether this topic should be shown in-context in the course.") + ) + + def __str__(self): + return ( + f'DiscussionTopicLink(' + f'context_key="{self.context_key}", usage_key="{self.usage_key}", title="{self.title}", ' + f'group={self.group}, provider_id="{self.provider_id}", external_id="{self.external_id}", ' + f'enabled_in_context={self.enabled_in_context}' + f')' + ) diff --git a/openedx/core/djangoapps/discussions/signals.py b/openedx/core/djangoapps/discussions/signals.py new file mode 100644 index 000000000000..53eb9608a3a4 --- /dev/null +++ b/openedx/core/djangoapps/discussions/signals.py @@ -0,0 +1,18 @@ +""" +Signals for discussions +""" +from openedx_events.tooling import OpenEdxPublicSignal + +from openedx.core.djangoapps.discussions.data import CourseDiscussionConfigurationData + +# TODO: This will be moved to openedx_events. It's currently here to simplify the PR. +# .. event_type: org.openedx.learning.discussions.configuration.change.v1 +# .. event_name: COURSE_DISCUSSIONS_UPDATED +# .. event_description: emitted when the configuration for a course's discussions changes in the course +# .. event_data: CourseDiscussionConfigurationData +COURSE_DISCUSSIONS_UPDATED = OpenEdxPublicSignal( + event_type="org.openedx.learning.discussions.configuration.change.v1", + data={ + "configuration": CourseDiscussionConfigurationData + } +) diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py new file mode 100644 index 000000000000..6b4d540ea799 --- /dev/null +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -0,0 +1,92 @@ +""" +Tasks for discussions +""" +import logging + +from celery import shared_task +from edx_django_utils.monitoring import set_code_owner_attribute +from opaque_keys.edx.keys import CourseKey + +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from .data import CourseDiscussionConfigurationData, DiscussionTopicContext +from .models import DiscussionsConfiguration +from .signals import COURSE_DISCUSSIONS_UPDATED + +log = logging.getLogger(__name__) + + +@shared_task +@set_code_owner_attribute +def update_discussions_settings_from_course_task(course_key_str: str): + """ + Celery task that creates or updates discussions settings for a course. + + Args: + course_key_str (str): course key string + """ + course_key = CourseKey.from_string(course_key_str) + config_data = update_discussions_settings_from_course(course_key) + COURSE_DISCUSSIONS_UPDATED.send_event(configuration=config_data) + + +def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDiscussionConfigurationData: + """ + When there are changes to a course, construct a new data structure containing all the context needed to update the + course's discussion settings in the database. + + Args: + course_key (CourseKey): The course that was recently updated. + + Returns: + (CourseDiscussionConfigurationData): structured discusion configuration data. + """ + log.info(f"Updating discussion settings for course: {course_key}") + store = modulestore() + + supports_in_context = DiscussionsConfiguration.get(course_key).supports_in_context_discussions() + + def iter_discussable_units(): + sections = store.get_items(course_key, qualifiers={'category': 'sequential'}) + for section in sections: + for unit in section.get_children(): + # 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 section.graded and not enable_graded_units: + continue + yield DiscussionTopicContext( + usage_key=unit.location, + title=unit.display_name, + group_id=None, + ) + + with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key): + course = store.get_course(course_key) + provider = course.discussions_settings.get('provider') + enable_in_context = course.discussions_settings.get('enable_in_context', True) + provider_config = course.discussions_settings.get(provider, {}) + unit_level_visibility = course.discussions_settings.get('unit_level_visibility', False) + enable_graded_units = course.discussions_settings.get('enable_graded_units', False) + contexts = [] + if supports_in_context: + contexts = [ + DiscussionTopicContext( + title=topic_name, + external_id=topic_config.get('id', None), + ) + for topic_name, topic_config in course.discussion_topics.items() + ] + if enable_in_context: + contexts.extend(list(iter_discussable_units())) + config_data = CourseDiscussionConfigurationData( + course_key=course_key, + enable_in_context=enable_in_context, + enable_graded_units=enable_graded_units, + unit_level_visibility=unit_level_visibility, + provider_type=provider, + plugin_configuration=provider_config, + contexts=contexts, + ) + return config_data diff --git a/openedx/core/djangoapps/discussions/tests/test_handlers.py b/openedx/core/djangoapps/discussions/tests/test_handlers.py new file mode 100644 index 000000000000..90e077ccb14d --- /dev/null +++ b/openedx/core/djangoapps/discussions/tests/test_handlers.py @@ -0,0 +1,163 @@ +""" +Tests for discussions signal handlers +""" +from unittest.mock import patch +from uuid import uuid4 + +import ddt +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey + +from openedx.core.djangoapps.discussions.data import CourseDiscussionConfigurationData, DiscussionTopicContext +from openedx.core.djangoapps.discussions.handlers import update_course_discussion_config +from openedx.core.djangoapps.discussions.models import DiscussionTopicLink, DiscussionsConfiguration + + +@ddt.ddt +class UpdateCourseDiscussionsConfigTestCase(TestCase): + """ + Tests for the discussion config update handler. + """ + def setUp(self) -> None: + super().setUp() + self.course_key = CourseKey.from_string("course-v1:test+test+test") + self.discussion_config = DiscussionsConfiguration.objects.create( + context_key=self.course_key, + provider_type="openedx", + ) + + def create_contexts(self, general=0, unit=0): + """ + Create context data for topics + """ + for idx in range(general): + yield DiscussionTopicContext( + title=f"General topic {idx}", + external_id=f"general-topic-{idx}", + ) + for idx in range(unit): + yield DiscussionTopicContext( + title=f"Unit {idx}", + usage_key=self.course_key.make_usage_key("vertical", f"unit-{idx}"), + ) + + def test_configuration_for_new_course(self): + """ + Test that a new course gets a new discussion configuration object + """ + new_key = CourseKey.from_string("course-v1:test+test+test2") + config_data = CourseDiscussionConfigurationData( + course_key=new_key, + provider_type="openedx", + ) + assert not DiscussionsConfiguration.objects.filter(context_key=new_key).exists() + update_course_discussion_config(config_data) + assert DiscussionsConfiguration.objects.filter(context_key=new_key).exists() + db_config = DiscussionsConfiguration.objects.get(context_key=new_key) + assert db_config.provider_type == "openedx" + + def test_creating_new_links(self): + """ + Test that new links are created in the db when they are added in the config. + """ + contexts = list(self.create_contexts(general=2, unit=3)) + config_data = CourseDiscussionConfigurationData( + course_key=self.course_key, + provider_type="openedx", + contexts=contexts, + ) + update_course_discussion_config(config_data) + topic_links = DiscussionTopicLink.objects.filter(context_key=self.course_key) + assert topic_links.count() == len(contexts) # 2 general + 3 units + + def test_updating_existing_links(self): + """ + Test that updating existing links works as expected. + """ + contexts = list(self.create_contexts(general=2, unit=3)) + config_data = CourseDiscussionConfigurationData( + course_key=self.course_key, + provider_type="openedx", + contexts=contexts, + ) + existing_external_id = uuid4() + existing_topic_link = DiscussionTopicLink.objects.create( + context_key=self.course_key, + usage_key=self.course_key.make_usage_key("vertical", "unit-2"), + title="Old title", + provider_id="openedx", + external_id=existing_external_id, + enabled_in_context=True, + ) + update_course_discussion_config(config_data) + existing_topic_link.refresh_from_db() + # Make sure that the title changes, but nothing else + assert existing_topic_link.title == "Unit 2" + assert existing_topic_link.provider_id == "openedx" + assert existing_topic_link.external_id == str(existing_external_id) + assert existing_topic_link.enabled_in_context + + @patch.dict( + "openedx.core.djangoapps.discussions.models.AVAILABLE_PROVIDER_MAP", + {"test": {"supports_in_context_discussions": True}}, + ) + def test_provider_change(self): + """ + Test that changing providers creates new links, and doesn't update existing ones. + """ + contexts = list(self.create_contexts(general=2, unit=3)) + config_data = CourseDiscussionConfigurationData( + course_key=self.course_key, + provider_type="test", + contexts=contexts, + ) + existing_external_id = uuid4() + existing_usage_key = self.course_key.make_usage_key("vertical", "unit-2") + existing_topic_link = DiscussionTopicLink.objects.create( + context_key=self.course_key, + usage_key=existing_usage_key, + title="Old title", + provider_id="openedx", + external_id=existing_external_id, + enabled_in_context=True, + ) + update_course_discussion_config(config_data) + existing_topic_link.refresh_from_db() + # If the provider has changed, new links should be created, the existing on remains the same + assert existing_topic_link.title == "Old title" + assert existing_topic_link.provider_id == "openedx" + assert existing_topic_link.external_id == str(existing_external_id) + assert existing_topic_link.enabled_in_context + new_link = DiscussionTopicLink.objects.get( + context_key=self.course_key, + provider_id="test", + usage_key=existing_usage_key, + ) + assert new_link.title == "Unit 2" + # The new link will get a new id + assert new_link.external_id != str(existing_external_id) + + def test_enabled_units_change(self): + """ + Test that when enabled units change, old unit links are disabled in context. + """ + contexts = list(self.create_contexts(general=2, unit=3)) + config_data = CourseDiscussionConfigurationData( + course_key=self.course_key, + provider_type="openedx", + contexts=contexts, + ) + existing_external_id = uuid4() + existing_usage_key = self.course_key.make_usage_key("vertical", "unit-10") + existing_topic_link = DiscussionTopicLink.objects.create( + context_key=self.course_key, + usage_key=existing_usage_key, + title="Unit 10", + provider_id="openedx", + external_id=existing_external_id, + enabled_in_context=True, + ) + update_course_discussion_config(config_data) + existing_topic_link.refresh_from_db() + # If the unit has an existing link but is disabled or removed + assert not existing_topic_link.enabled_in_context diff --git a/openedx/core/djangoapps/discussions/tests/test_tasks.py b/openedx/core/djangoapps/discussions/tests/test_tasks.py new file mode 100644 index 000000000000..b9df4f61f6a8 --- /dev/null +++ b/openedx/core/djangoapps/discussions/tests/test_tasks.py @@ -0,0 +1,152 @@ +""" +Tests for discussions tasks. +""" +import ddt +import mock + +from openedx.core.djangoapps.discussions.data import DiscussionTopicContext +from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +@ddt.ddt +@mock.patch('openedx.core.djangoapps.discussions.tasks.DiscussionsConfiguration', mock.Mock()) +class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): + """ + Tests for the discussions settings update tasks + """ + def setUp(self): + super().setUp() + self.course = course = CourseFactory.create() + self.course_key = course_key = self.course.id + with self.store.bulk_operations(course_key): + section = ItemFactory.create( + parent_location=course.location, + category="chapter", + display_name="Section" + ) + sequence = ItemFactory.create( + parent_location=section.location, + category="sequential", + display_name="Sequence" + ) + unit = ItemFactory.create( + parent_location=sequence.location, + category="vertical", + display_name="Unit" + ) + ItemFactory.create( + parent_location=sequence.location, + category="vertical", + display_name="Discussable Unit", + discussion_enabled=True, + ) + ItemFactory.create( + parent_location=sequence.location, + category="vertical", + display_name="Non-Discussable Unit", + discussion_enabled=False, + ) + ItemFactory.create( + parent_location=unit.location, + category="html", + display_name="An HTML Module" + ) + graded_sequence = ItemFactory.create( + parent_location=section.location, + category="sequential", + display_name="Graded Sequence", + graded=True, + ) + graded_unit = ItemFactory.create( + parent_location=graded_sequence.location, + category="vertical", + display_name="Graded Unit" + ) + ItemFactory.create( + parent_location=graded_sequence.location, + category="vertical", + display_name="Discussable Graded Unit", + discussion_enabled=True, + ) + ItemFactory.create( + parent_location=graded_sequence.location, + category="vertical", + display_name="Non-Discussable Graded Unit", + discussion_enabled=False, + ) + ItemFactory.create( + parent_location=graded_unit.location, + category="html", + display_name="Graded HTML Module" + ) + + def update_course_field(self, **update): + """ + Update the test course using provided parameters. + """ + for key, value in update.items(): + setattr(self.course, key, value) + self.update_course(self.course, self.user.id) + + def update_discussions_settings(self, settings): + """ + Update course discussion settings based on the provided discussion settings. + """ + self.course.discussions_settings.update(settings) + self.update_course(self.course, self.user.id) + + def test_default(self): + """ + Test that the course defaults. + """ + config_data = update_discussions_settings_from_course(self.course.id) + assert config_data.course_key == self.course.id + assert config_data.enable_graded_units is False + assert config_data.unit_level_visibility is False + assert config_data.provider_type is None + assert config_data.plugin_configuration == {} + assert len(config_data.contexts) == 4 + + def test_general_topics(self): + """ + Test the handling of course general topics. + """ + self.update_course_field(discussion_topics={ + "General": {"id": "general-topic"}, + "Test Topic": {"id": "test-topic"}, + }) + config_data = update_discussions_settings_from_course(self.course.id) + assert len(config_data.contexts) == 5 + assert DiscussionTopicContext( + title="General", + external_id="general-topic", + ) in config_data.contexts + assert DiscussionTopicContext( + title="Test Topic", + external_id="test-topic", + ) in config_data.contexts + + @ddt.data( + ({"enable_in_context": False}, 1, set(), {"Unit", "Graded Unit"}), + ({"enable_graded_units": False}, 4, {"Unit", "Discussable Unit", "Non-Discussable Unit"}, + {"Graded Unit"}), + ({"enable_graded_units": True}, 7, {"Unit", "Graded Unit", "Discussable Graded Unit"}, set()), + ({"unit_level_visibility": True}, 2, {"Discussable Unit"}, + {"Unit", "Graded Unit", "Non-Discussable Unit", "Discussable Graded Unit", "Non-Discussable Graded Unit"}), + ({"unit_level_visibility": True, "enable_graded_units": True}, 3, + {"Discussable Unit", "Discussable Graded Unit"}, + {"Graded Unit", "Non-Discussable Unit", "Non-Discussable Graded Unit"}), + ) + @ddt.unpack + def test_custom_discussion_settings(self, settings, context_count, present_units, missing_units): + """ + Test different combinations of settings and their impact on the units that are returned. + """ + self.update_discussions_settings(settings) + config_data = update_discussions_settings_from_course(self.course.id) + assert len(config_data.contexts) == context_count + units_in_config = {context.title for context in config_data.contexts} + assert present_units <= units_in_config + assert not missing_units & units_in_config diff --git a/openedx/features/lti_course_tab/tests.py b/openedx/features/lti_course_tab/tests.py index 91074d661e80..ce0f483b584d 100644 --- a/openedx/features/lti_course_tab/tests.py +++ b/openedx/features/lti_course_tab/tests.py @@ -22,7 +22,8 @@ def setUp(self): self.discussion_config = DiscussionsConfiguration.objects.create( context_key=self.course.id, enabled=False, - provider_type="lti_provider", + # Pick a provider that supports LTI + provider_type="yellowdig", ) self.discussion_config.lti_configuration = LtiConfiguration.objects.create( config_store=LtiConfiguration.CONFIG_ON_DB,