From aac6c4c4cb5ded540281d4bc41aa4f6e004cd6ae Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Wed, 6 Oct 2021 16:37:16 +0500 Subject: [PATCH] fix: default discussion config (#28900) * fix: default discussion config * refactor: add closing bracket Co-authored-by: Asad --- openedx/core/djangoapps/course_apps/admin.py | 16 ++++++++++++++++ openedx/core/djangoapps/course_apps/models.py | 3 +++ openedx/core/djangoapps/discussions/models.py | 3 ++- .../djangoapps/discussions/tests/test_models.py | 16 ++++++++-------- .../djangoapps/discussions/tests/test_views.py | 9 ++++----- 5 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 openedx/core/djangoapps/course_apps/admin.py diff --git a/openedx/core/djangoapps/course_apps/admin.py b/openedx/core/djangoapps/course_apps/admin.py new file mode 100644 index 0000000000..34bd1c58c8 --- /dev/null +++ b/openedx/core/djangoapps/course_apps/admin.py @@ -0,0 +1,16 @@ +""" +Django Admin pages for course_apps. +""" + +from django.contrib import admin + +from .models import CourseAppStatus + + +class CourseAppStatusAdmin(admin.ModelAdmin): + """Admin for CourseAppStatus""" + search_fields = ('course_key', ) + list_display = ('course_key', 'app_id', 'enabled') + + +admin.site.register(CourseAppStatus, CourseAppStatusAdmin) diff --git a/openedx/core/djangoapps/course_apps/models.py b/openedx/core/djangoapps/course_apps/models.py index 49868910d0..f23c9b606f 100644 --- a/openedx/core/djangoapps/course_apps/models.py +++ b/openedx/core/djangoapps/course_apps/models.py @@ -23,6 +23,9 @@ class CourseAppStatus(TimeStampedModel): history = HistoricalRecords() + def __str__(self): + return f'CourseAppStatus(course_key="{self.course_key}", app_id="{self.app_id}", enabled="{self.enabled})"' + @classmethod def get_all_app_status_data_for_course(cls, course_key: CourseKey) -> Dict[str, bool]: """ diff --git a/openedx/core/djangoapps/discussions/models.py b/openedx/core/djangoapps/discussions/models.py index cbd95b99ed..38e7394607 100644 --- a/openedx/core/djangoapps/discussions/models.py +++ b/openedx/core/djangoapps/discussions/models.py @@ -26,6 +26,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ log = logging.getLogger(__name__) DEFAULT_PROVIDER_TYPE = 'legacy' +DEFAULT_CONFIG_ENABLED = True ProviderExternalLinks = namedtuple( 'ProviderExternalLinks', @@ -403,7 +404,7 @@ class DiscussionsConfiguration(TimeStampedModel): except cls.DoesNotExist: configuration = cls( context_key=context_key, - enabled=False, + enabled=DEFAULT_CONFIG_ENABLED, provider_type=DEFAULT_PROVIDER_TYPE, ) return configuration diff --git a/openedx/core/djangoapps/discussions/tests/test_models.py b/openedx/core/djangoapps/discussions/tests/test_models.py index 31cfb0d5e6..91a3984df2 100644 --- a/openedx/core/djangoapps/discussions/tests/test_models.py +++ b/openedx/core/djangoapps/discussions/tests/test_models.py @@ -9,7 +9,7 @@ from django.test import TestCase from opaque_keys.edx.keys import CourseKey from organizations.models import Organization -from ..models import DEFAULT_PROVIDER_TYPE +from ..models import DEFAULT_CONFIG_ENABLED, DEFAULT_PROVIDER_TYPE from ..models import DiscussionsConfiguration from ..models import ProviderFilter @@ -197,10 +197,10 @@ class DiscussionsConfigurationModelTest(TestCase): def test_is_enabled_nonexistent(self): """ - Assert that discussions are disabled, when no configuration exists + Assert that discussions are enabled by default even when no configuration exists """ - is_enabled = DiscussionsConfiguration.is_enabled(self.course_key_without_config) - assert not is_enabled + is_enabled = DiscussionsConfiguration.is_enabled(context_key=self.course_key_without_config) + assert is_enabled == DEFAULT_CONFIG_ENABLED def test_is_enabled_default(self): """ @@ -211,21 +211,21 @@ class DiscussionsConfigurationModelTest(TestCase): def test_is_enabled_explicit(self): """ - Assert that discussions can be explitly disabled + Assert that discussions can be explicitly disabled """ is_enabled = DiscussionsConfiguration.is_enabled(self.course_key_with_values) assert not is_enabled def test_get_nonexistent_defaults_to_legacy(self): """ - Assert we get a "legacy" model back for nonexistent records + Assert we get default provider "legacy" and default enabled model instance back for nonexistent records """ configuration = DiscussionsConfiguration.get(self.course_key_without_config) assert configuration is not None - assert not configuration.enabled + assert configuration.enabled == DEFAULT_CONFIG_ENABLED + assert configuration.provider_type == DEFAULT_PROVIDER_TYPE assert not configuration.lti_configuration assert not configuration.plugin_configuration - assert configuration.provider_type == DEFAULT_PROVIDER_TYPE def test_get_defaults(self): """ diff --git a/openedx/core/djangoapps/discussions/tests/test_views.py b/openedx/core/djangoapps/discussions/tests/test_views.py index b9a4a1fa65..12d0813590 100644 --- a/openedx/core/djangoapps/discussions/tests/test_views.py +++ b/openedx/core/djangoapps/discussions/tests/test_views.py @@ -19,8 +19,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory -from ..models import AVAILABLE_PROVIDER_MAP - +from ..models import AVAILABLE_PROVIDER_MAP, DEFAULT_CONFIG_ENABLED, DEFAULT_PROVIDER_TYPE DATA_LEGACY_COHORTS = { 'divided_inline_discussions': [], @@ -144,8 +143,8 @@ class DataTest(AuthorizedApiTest): """ data = response.json() assert response.status_code == self.expected_response_code - assert not data['enabled'] - assert data['provider_type'] == 'legacy' + assert data['enabled'] == DEFAULT_CONFIG_ENABLED + assert data['provider_type'] == DEFAULT_PROVIDER_TYPE assert data['providers']['available']['legacy'] == AVAILABLE_PROVIDER_MAP['legacy'] assert not [ name for name, spec in data['providers']['available'].items() @@ -265,7 +264,7 @@ class DataTest(AuthorizedApiTest): 'non-existent-key': 'value', } response = self._post(payload) - self._assert_defaults(response) + assert response.status_code == self.expected_response_code def test_configuration_valid(self): """