feat: Adds a new discussion topic configuration mechanism [BD-38] [TNL-8623] [BB-4968] (#29082)
* feat: Adds a new discussion topic configuration mechanism The new discussion configuration system links discussion topics directly to the course structure. This change adds a new task that sends a discussion update signal if there are any changes to the course. This signal includes all the context needed to update the configuration of the course. The handler for this new event will create a new entry for each unit that needs a topic in the database. In the future this will be used to see the topics in the course. * fix: add support for marking a provider as supporting LTI * fix: review feedback
This commit is contained in:
@@ -12,15 +12,15 @@ from pytz import UTC
|
||||
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
|
||||
|
||||
@@ -5,7 +5,7 @@ a user has on an article.
|
||||
|
||||
|
||||
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
|
||||
|
||||
@@ -5,7 +5,7 @@ from typing import Dict, Optional
|
||||
|
||||
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
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ from typing import Dict, Optional
|
||||
|
||||
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
|
||||
|
||||
@@ -30,3 +30,6 @@ class DiscussionsConfig(AppConfig):
|
||||
PluginSettings.CONFIG: {
|
||||
},
|
||||
}
|
||||
|
||||
def ready(self):
|
||||
from . import handlers # pylint: disable=unused-import
|
||||
|
||||
45
openedx/core/djangoapps/discussions/data.py
Normal file
45
openedx/core/djangoapps/discussions/data.py
Normal file
@@ -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)
|
||||
97
openedx/core/djangoapps/discussions/handlers.py
Normal file
97
openedx/core/djangoapps/discussions/handlers.py
Normal file
@@ -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)
|
||||
@@ -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')),
|
||||
],
|
||||
),
|
||||
]
|
||||
@@ -16,12 +16,13 @@ from enum import Enum
|
||||
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 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
Features.COURSE_COHORT_SUPPORT.value,
|
||||
Features.RESEARCH_DATA_EVENTS.value,
|
||||
],
|
||||
'supports_lti': False,
|
||||
'external_links': ProviderExternalLinks(
|
||||
learn_more='',
|
||||
configuration='',
|
||||
@@ -128,6 +130,33 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
'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 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
Features.IN_PLATFORM_NOTIFICATIONS.value,
|
||||
Features.USER_MENTIONS.value,
|
||||
],
|
||||
'supports_lti': True,
|
||||
'external_links': ProviderExternalLinks(
|
||||
learn_more='',
|
||||
configuration='',
|
||||
@@ -171,6 +201,7 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
Features.IN_PLATFORM_NOTIFICATIONS.value,
|
||||
Features.DISCUSSION_CONTENT_PROMPTS.value,
|
||||
],
|
||||
'supports_lti': True,
|
||||
'external_links': ProviderExternalLinks(
|
||||
learn_more='',
|
||||
configuration='',
|
||||
@@ -194,6 +225,7 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
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 @@ AVAILABLE_PROVIDER_MAP = {
|
||||
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 @@ class DiscussionsConfiguration(TimeStampedModel):
|
||||
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 @@ class DiscussionsConfiguration(TimeStampedModel):
|
||||
|
||||
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 @@ class ProgramDiscussionsConfiguration(TimeStampedModel):
|
||||
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')'
|
||||
)
|
||||
|
||||
18
openedx/core/djangoapps/discussions/signals.py
Normal file
18
openedx/core/djangoapps/discussions/signals.py
Normal file
@@ -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
|
||||
}
|
||||
)
|
||||
92
openedx/core/djangoapps/discussions/tasks.py
Normal file
92
openedx/core/djangoapps/discussions/tasks.py
Normal file
@@ -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
|
||||
163
openedx/core/djangoapps/discussions/tests/test_handlers.py
Normal file
163
openedx/core/djangoapps/discussions/tests/test_handlers.py
Normal file
@@ -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
|
||||
152
openedx/core/djangoapps/discussions/tests/test_tasks.py
Normal file
152
openedx/core/djangoapps/discussions/tests/test_tasks.py
Normal file
@@ -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
|
||||
@@ -22,7 +22,8 @@ class DiscussionLtiCourseTabTestCase(TabTestCase):
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user