diff --git a/cms/envs/common.py b/cms/envs/common.py index 61c982b83f..9cbbb45127 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1376,6 +1376,7 @@ INSTALLED_APPS = [ # Discussion 'openedx.core.djangoapps.django_comment_common', + 'openedx.core.djangoapps.discussions', # for course creator table 'django.contrib.admin', diff --git a/lms/envs/common.py b/lms/envs/common.py index c5e00559d6..3d343dd054 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2558,6 +2558,7 @@ INSTALLED_APPS = [ # Discussion forums 'openedx.core.djangoapps.django_comment_common', + 'openedx.core.djangoapps.discussions', # Notes 'lms.djangoapps.edxnotes', diff --git a/openedx/core/djangoapps/discussions/__init__.py b/openedx/core/djangoapps/discussions/__init__.py new file mode 100644 index 0000000000..7aac22d8e4 --- /dev/null +++ b/openedx/core/djangoapps/discussions/__init__.py @@ -0,0 +1,4 @@ +""" +Handle discussions integrations +""" +default_app_config = 'openedx.core.djangoapps.discussions.apps.DiscussionsConfig' diff --git a/openedx/core/djangoapps/discussions/admin.py b/openedx/core/djangoapps/discussions/admin.py new file mode 100644 index 0000000000..39a59fc421 --- /dev/null +++ b/openedx/core/djangoapps/discussions/admin.py @@ -0,0 +1,22 @@ +""" +Customize the django admin experience +""" +from django.contrib import admin +from simple_history.admin import SimpleHistoryAdmin + +from .models import DiscussionsConfiguration + + +class DiscussionsConfigurationAdmin(SimpleHistoryAdmin): + search_fields = ( + 'context_key', + 'enabled', + 'provider_type', + ) + list_filter = ( + 'enabled', + 'provider_type', + ) + + +admin.site.register(DiscussionsConfiguration, DiscussionsConfigurationAdmin) diff --git a/openedx/core/djangoapps/discussions/apps.py b/openedx/core/djangoapps/discussions/apps.py new file mode 100644 index 0000000000..51b2803668 --- /dev/null +++ b/openedx/core/djangoapps/discussions/apps.py @@ -0,0 +1,11 @@ +""" +Configure the django app +""" +from django.apps import AppConfig + + +class DiscussionsConfig(AppConfig): + """ + Configure the discussions django app + """ + name = 'openedx.core.djangoapps.discussions' diff --git a/openedx/core/djangoapps/discussions/docs/decisions/0001-discussion-configuration-api.rst b/openedx/core/djangoapps/discussions/docs/decisions/0001-discussion-configuration-api.rst new file mode 100644 index 0000000000..556c2407f5 --- /dev/null +++ b/openedx/core/djangoapps/discussions/docs/decisions/0001-discussion-configuration-api.rst @@ -0,0 +1,73 @@ +Discussion provider configuration +================================= + + +Status +------ + +Proposal + + +Context +------- + +As part of the BD-03 initiative (Blended Development, Project 3), +we want to enable third-party discussion providers to replace the +default forums experience in edx-platform. + +To accomplish this, we should establish a configuration system to +enable/disable/configure these new discussion-provider plugins; +the existing forums experience will be included as one of these plugins. + +This ADR proposes a new configuration system that allows operators to create +pre-populated configurations for specific discussions providers, +which can then be used and customized by course authors/admins. + + +Decision +-------- + +We propose to implement this as a Django database-backed model with +historical records, `DiscussionsConfiguration`. + +The underlying model can be represented as: + +.. code-block:: python + class DiscussionsConfiguration: + context_key = LearningContextKeyField(primary_key=True, ...) + enabled = models.BooleanField(default=True, ...) + lti_configuration = models.ForeignKey(LtiConfiguration, blank=True, ...) + plugin_configuration = JSONField(default={}, ...) + provider_type = models.CharField(blank=False, ...) + history = HistoricalRecords() + +- Discussions can be disabled on a course by setting `enabled=False`; if + no record exists, the plugin is considered to be disabled by default. + +- `lti_configuration` is a collection of LTI launch configuration data + (URL, key, secret, etc.), but this data structure should be extracted + into its own model; see notes below (`Dependent Work`). + +- `plugin_configuration` is a free-form dictionary of configuration + values, to be passed to plugin's renderer + +- `provider_type` is the (arbitrary, but stable) id for the discussion provider. + +- `history` is tracked using Django Simple History. + + +Future Work +----------- + +- `LtiConfiguration` should be extended for feature-parity with_out_ + XBlock dependencies. + +- Discussion apps/plugins/providers, implemented as Python models, will + be available via some discovery mechanism (Python entry points?, + hard-coded list?) + They will be responsible for: + - the provider_type, referenced by `DiscussionsConfiguration` + - any relevant renderers, accessors, etc. + +- No site/organization-based provider restrictions will be implemented, at this time. + This could be added later via an allow/deny-list. diff --git a/openedx/core/djangoapps/discussions/migrations/0001_initial.py b/openedx/core/djangoapps/discussions/migrations/0001_initial.py new file mode 100644 index 0000000000..b42d633998 --- /dev/null +++ b/openedx/core/djangoapps/discussions/migrations/0001_initial.py @@ -0,0 +1,60 @@ +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import jsonfield.encoder +import jsonfield.fields +import model_utils.fields +import opaque_keys.edx.django.models +import simple_history.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('lti_consumer', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalDiscussionsConfiguration', + fields=[ + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('context_key', opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, max_length=255, verbose_name='Learning Context Key')), + ('enabled', models.BooleanField(default=True, help_text='If disabled, the discussions in the associated learning context/course will be disabled.')), + ('plugin_configuration', jsonfield.fields.JSONField(blank=True, default={}, dump_kwargs={'cls': jsonfield.encoder.JSONEncoder, 'separators': (',', ':')}, help_text='The plugin configuration data for this context/provider.', load_kwargs={})), + ('provider_type', models.CharField(help_text="The discussion tool/provider's id", max_length=100, verbose_name='Discussion provider')), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('lti_configuration', models.ForeignKey(blank=True, db_constraint=False, help_text='The LTI configuration data for this context/provider.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='lti_consumer.LtiConfiguration')), + ], + options={ + 'verbose_name': 'historical discussions configuration', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': 'history_date', + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='DiscussionsConfiguration', + fields=[ + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('context_key', opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, max_length=255, primary_key=True, serialize=False, unique=True, verbose_name='Learning Context Key')), + ('enabled', models.BooleanField(default=True, help_text='If disabled, the discussions in the associated learning context/course will be disabled.')), + ('plugin_configuration', jsonfield.fields.JSONField(blank=True, default={}, dump_kwargs={'cls': jsonfield.encoder.JSONEncoder, 'separators': (',', ':')}, help_text='The plugin configuration data for this context/provider.', load_kwargs={})), + ('provider_type', models.CharField(help_text="The discussion tool/provider's id", max_length=100, verbose_name='Discussion provider')), + ('lti_configuration', models.ForeignKey(blank=True, help_text='The LTI configuration data for this context/provider.', null=True, on_delete=django.db.models.deletion.SET_NULL, to='lti_consumer.LtiConfiguration')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/openedx/core/djangoapps/discussions/migrations/__init__.py b/openedx/core/djangoapps/discussions/migrations/__init__.py new file mode 100644 index 0000000000..b5adbcbe2b --- /dev/null +++ b/openedx/core/djangoapps/discussions/migrations/__init__.py @@ -0,0 +1,3 @@ +""" +Provide django migrations +""" diff --git a/openedx/core/djangoapps/discussions/models.py b/openedx/core/djangoapps/discussions/models.py new file mode 100644 index 0000000000..ec511fee5b --- /dev/null +++ b/openedx/core/djangoapps/discussions/models.py @@ -0,0 +1,69 @@ +""" +Provide django models to back the discussions app +""" +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.translation import ugettext_lazy as _ +from jsonfield import JSONField +from model_utils.models import TimeStampedModel +from opaque_keys.edx.django.models import LearningContextKeyField +from simple_history.models import HistoricalRecords + +from lti_consumer.models import LtiConfiguration + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + +class DiscussionsConfiguration(TimeStampedModel): + """ + Associates a learning context with discussion provider and configuration + """ + + context_key = LearningContextKeyField( + primary_key=True, + db_index=True, + unique=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"), + ) + enabled = models.BooleanField( + default=True, + help_text=_("If disabled, the discussions in the associated learning context/course will be disabled.") + ) + lti_configuration = models.ForeignKey( + LtiConfiguration, + on_delete=models.SET_NULL, + blank=True, + null=True, + help_text=_("The LTI configuration data for this context/provider."), + ) + plugin_configuration = JSONField( + blank=True, + default={}, + help_text=_("The plugin configuration data for this context/provider."), + ) + provider_type = models.CharField( + blank=False, + max_length=100, + verbose_name=_("Discussion provider"), + help_text=_("The discussion tool/provider's id"), + ) + history = HistoricalRecords() + + def clean(self): + """ + Validate the model + + Currently, this only support courses, this can be extended + whenever discussions are available in other contexts + """ + if not CourseOverview.course_exists(self.context_key): + raise ValidationError('Context Key should be an existing learning context.') + + def __str__(self): + return "{context_key}: provider={provider} enabled={enabled}".format( + context_key=self.context_key, + provider=self.provider_type, + enabled=self.enabled, + ) diff --git a/openedx/core/djangoapps/discussions/tests/__init__.py b/openedx/core/djangoapps/discussions/tests/__init__.py new file mode 100644 index 0000000000..be45251866 --- /dev/null +++ b/openedx/core/djangoapps/discussions/tests/__init__.py @@ -0,0 +1,3 @@ +""" +Perform basic validation on the app +""" diff --git a/openedx/core/djangoapps/discussions/tests/test_models.py b/openedx/core/djangoapps/discussions/tests/test_models.py new file mode 100644 index 0000000000..3a52752c77 --- /dev/null +++ b/openedx/core/djangoapps/discussions/tests/test_models.py @@ -0,0 +1,84 @@ +""" +Perform basic validation of the models +""" +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey + +from ..models import DiscussionsConfiguration + + +class DiscussionsConfigurationModelTest(TestCase): + """ + Perform basic validation on the configuration model + """ + + def setUp(self): + """ + Configure shared test data (configuration, course_key, etc.) + """ + self.course_key_with_defaults = CourseKey.from_string("course-v1:TestX+Course+Configured") + self.course_key_without_config = CourseKey.from_string("course-v1:TestX+Course+NoConfig") + self.course_key_with_values = CourseKey.from_string("course-v1:TestX+Course+Values") + self.configuration_with_defaults = DiscussionsConfiguration( + context_key=self.course_key_with_defaults, + ) + self.configuration_with_defaults.save() + self.configuration_with_values = DiscussionsConfiguration( + context_key=self.course_key_with_values, + enabled=False, + provider_type='cs_comments_service', + plugin_configuration={ + 'url': 'http://localhost', + }, + ) + self.configuration_with_values.save() + pass + + def test_get_nonexistent(self): + """ + Assert we can not fetch a non-existent record + """ + with self.assertRaises(DiscussionsConfiguration.DoesNotExist): + configuration = DiscussionsConfiguration.objects.get( + context_key=self.course_key_without_config, + ) + + def test_get_with_defaults(self): + """ + Assert we can lookup a record with default values + """ + configuration = DiscussionsConfiguration.objects.get(context_key=self.course_key_with_defaults) + assert configuration is not None + assert configuration.enabled # by default + assert configuration.lti_configuration is None + assert len(configuration.plugin_configuration.keys()) == 0 + assert not configuration.provider_type + + def test_get_with_values(self): + """ + Assert we can lookup a record with custom values + """ + configuration = DiscussionsConfiguration.objects.get(context_key=self.course_key_with_values) + assert configuration is not None + assert not configuration.enabled + assert configuration.lti_configuration is None + assert configuration.plugin_configuration['url'] == self.configuration_with_values.plugin_configuration['url'] + assert configuration.provider_type == self.configuration_with_values.provider_type + + def test_update_defaults(self): + """ + Assert we can update an existing record + """ + configuration = DiscussionsConfiguration.objects.get(context_key=self.course_key_with_defaults) + configuration.enabled = False + configuration.plugin_configuration = { + 'url': 'http://localhost', + } + configuration.provider_type = 'cs_comments_service' + configuration.save() + configuration = DiscussionsConfiguration.objects.get(context_key=self.course_key_with_defaults) + assert configuration is not None + assert not configuration.enabled + assert configuration.lti_configuration is None + assert configuration.plugin_configuration['url'] == 'http://localhost' + assert configuration.provider_type == 'cs_comments_service' diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 80534cc629..5e92e3c58c 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -60,7 +60,9 @@ INSTALLED_APPS = ( 'django.contrib.sessions', 'django.contrib.sites', 'django_sites_extensions', + 'lti_consumer', 'openedx.core.djangoapps.django_comment_common', + 'openedx.core.djangoapps.discussions', 'openedx.core.djangoapps.video_config', 'openedx.core.djangoapps.video_pipeline', 'openedx.core.djangoapps.bookmarks.apps.BookmarksConfig',