From 9f4bc9900bd336ee7ac3b5f415f7980cf41cab25 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Tue, 7 Dec 2021 06:10:48 +0000 Subject: [PATCH] feat: add discussions context to course blocks API (#29300) Add a new course blocks transformer that adds discussion context for units. --- cms/envs/test.py | 1 + lms/djangoapps/course_api/blocks/api.py | 10 ++- .../course_api/blocks/serializers.py | 18 +++-- lms/djangoapps/discussion/plugins.py | 6 +- lms/djangoapps/discussion/views.py | 6 +- lms/envs/common.py | 2 +- lms/envs/test.py | 1 + .../content/block_structure/transformer.py | 1 - .../discussions/tests/test_transformer.py | 80 +++++++++++++++++++ .../djangoapps/discussions/transformers.py | 49 ++++++++++++ .../djangoapps/discussions/url_helpers.py | 35 ++++++++ setup.py | 1 + 12 files changed, 196 insertions(+), 14 deletions(-) create mode 100644 openedx/core/djangoapps/discussions/tests/test_transformer.py create mode 100644 openedx/core/djangoapps/discussions/transformers.py create mode 100644 openedx/core/djangoapps/discussions/url_helpers.py diff --git a/cms/envs/test.py b/cms/envs/test.py index 772be14a25..3736e76360 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -138,6 +138,7 @@ LMS_ROOT_URL = f"http://{LMS_BASE}" FEATURES['PREVIEW_LMS_BASE'] = "preview.localhost" COURSE_AUTHORING_MICROFRONTEND_URL = "http://course-authoring-mfe" +DISCUSSIONS_MICROFRONTEND_URL = "http://discussions-mfe" CACHES = { # This is the cache used for most things. Askbot will not work without a diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index c69edd689d..7c14dfd4c3 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -7,6 +7,7 @@ import lms.djangoapps.course_blocks.api as course_blocks_api from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers +from openedx.core.djangoapps.discussions.transformers import DiscussionsTopicLinkTransformer from openedx.features.effort_estimation.api import EffortEstimationTransformer from .serializers import BlockDictSerializer, BlockSerializer @@ -75,6 +76,10 @@ def get_blocks( include_gated_sections = 'show_gated_sections' in requested_fields include_has_scheduled_content = 'has_scheduled_content' in requested_fields include_special_exams = 'special_exam_info' in requested_fields + include_discussions_context = ( + DiscussionsTopicLinkTransformer.EMBED_URL in requested_fields or + DiscussionsTopicLinkTransformer.EXTERNAL_ID in requested_fields + ) if user is not None: transformers += course_blocks_api.get_course_block_access_transformers(user) @@ -99,13 +104,16 @@ def get_blocks( if include_effort_estimation: transformers += [EffortEstimationTransformer()] + if include_discussions_context: + transformers += [DiscussionsTopicLinkTransformer()] + transformers += [ BlocksAPITransformer( block_counts, student_view_data, depth, nav_depth - ) + ), ] # transform diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index 277bf2a668..ac031600a0 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -2,12 +2,12 @@ Serializers for Course Blocks related return objects. """ - from django.conf import settings from rest_framework import serializers from rest_framework.reverse import reverse from lms.djangoapps.course_blocks.transformers.visibility import VisibilityTransformer +from openedx.core.djangoapps.discussions.transformers import DiscussionsTopicLinkTransformer from .transformers.block_completion import BlockCompletionTransformer from .transformers.block_counts import BlockCountsTransformer @@ -21,13 +21,14 @@ class SupportedFieldType: """ Metadata about fields supported by different transformers """ + def __init__( - self, - block_field_name, - transformer=None, - requested_field_name=None, - serializer_field_name=None, - default_value=None + self, + block_field_name, + transformer=None, + requested_field_name=None, + serializer_field_name=None, + default_value=None ): self.transformer = transformer self.block_field_name = block_field_name @@ -82,6 +83,8 @@ SUPPORTED_FIELDS = [ SupportedFieldType(BlockCompletionTransformer.COMPLETION, BlockCompletionTransformer), SupportedFieldType(BlockCompletionTransformer.COMPLETE), SupportedFieldType(BlockCompletionTransformer.RESUME_BLOCK), + SupportedFieldType(DiscussionsTopicLinkTransformer.EXTERNAL_ID), + SupportedFieldType(DiscussionsTopicLinkTransformer.EMBED_URL), *[SupportedFieldType(field_name) for field_name in ExtraFieldsTransformer.get_requested_extra_fields()], ] @@ -111,6 +114,7 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho """ Serializer for single course block """ + def _get_field(self, block_key, transformer, field_name, default): """ Get the field value requested. The field may be an XBlock field, a diff --git a/lms/djangoapps/discussion/plugins.py b/lms/djangoapps/discussion/plugins.py index b35461c3e5..21c74b71e5 100644 --- a/lms/djangoapps/discussion/plugins.py +++ b/lms/djangoapps/discussion/plugins.py @@ -9,6 +9,7 @@ from django.utils.translation import gettext_noop import lms.djangoapps.discussion.django_comment_client.utils as utils from lms.djangoapps.courseware.tabs import EnrolledTab from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE +from openedx.core.djangoapps.discussions.url_helpers import get_discussions_mfe_url from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab from xmodule.tabs import TabFragmentViewMixin @@ -35,8 +36,9 @@ class DiscussionTab(TabFragmentViewMixin, EnrolledTab): def link_func(course, reverse_func): """ Returns a function that returns the course tab's URL. """ - if ENABLE_DISCUSSIONS_MFE.is_enabled(course.id) and settings.DISCUSSIONS_MICROFRONTEND_URL: - return f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/discussions/{course.id}/" + mfe_url = get_discussions_mfe_url(course.id) + if ENABLE_DISCUSSIONS_MFE.is_enabled(course.id) and mfe_url: + return mfe_url return _link_func(course, reverse_func) return link_func diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index e232476031..1e2304eb92 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -48,6 +48,7 @@ from lms.djangoapps.discussion.exceptions import TeamDiscussionHiddenFromUserExc from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from lms.djangoapps.teams import api as team_api +from openedx.core.djangoapps.discussions.url_helpers import get_discussions_mfe_url from openedx.core.djangoapps.discussions.utils import ( available_division_schemes, get_discussion_categories_ids, @@ -720,11 +721,12 @@ class DiscussionBoardFragmentView(EdxFragmentView): Fragment: The fragment representing the discussion board """ course_key = CourseKey.from_string(course_id) - if ENABLE_DISCUSSIONS_MFE.is_enabled(course_key) and settings.DISCUSSIONS_MICROFRONTEND_URL: + mfe_url = get_discussions_mfe_url(course_key) + if ENABLE_DISCUSSIONS_MFE.is_enabled(course_key) and mfe_url: fragment = Fragment( HTML( "" - ).format(src=f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/discussions/{course_id}/") + ).format(src=mfe_url) ) fragment.add_css( """ diff --git a/lms/envs/common.py b/lms/envs/common.py index 0101d0b562..6f6f48ddb6 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4698,7 +4698,7 @@ PROGRAM_CONSOLE_MICROFRONTEND_URL = None LEARNING_MICROFRONTEND_URL = None # .. setting_name: DISCUSSIONS_MICROFRONTEND_URL # .. setting_default: None -# .. setting_description: Base URL of the micro-frontend-based dicussions page. +# .. setting_description: Base URL of the micro-frontend-based discussions page. # .. setting_warning: Also set site's courseware.discussions_mfe waffle flag. DISCUSSIONS_MICROFRONTEND_URL = None # .. toggle_name: ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY diff --git a/lms/envs/test.py b/lms/envs/test.py index ad1910f9e5..39f5476cf1 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -579,6 +579,7 @@ ACCOUNT_MICROFRONTEND_URL = "http://account-mfe" AUTHN_MICROFRONTEND_URL = "http://authn-mfe" AUTHN_MICROFRONTEND_DOMAIN = "authn-mfe" LEARNING_MICROFRONTEND_URL = "http://learning-mfe" +DISCUSSIONS_MICROFRONTEND_URL = "http://discussions-mfe" ########################## limiting dashboard courses ###################### diff --git a/openedx/core/djangoapps/content/block_structure/transformer.py b/openedx/core/djangoapps/content/block_structure/transformer.py index b1ea1c62a1..1bcde62e06 100644 --- a/openedx/core/djangoapps/content/block_structure/transformer.py +++ b/openedx/core/djangoapps/content/block_structure/transformer.py @@ -106,7 +106,6 @@ class BlockStructureTransformer: block structure that is to be modified with collected data to be cached for the transformer. """ - pass # lint-amnesty, pylint: disable=unnecessary-pass @abstractmethod def transform(self, usage_info, block_structure): diff --git a/openedx/core/djangoapps/discussions/tests/test_transformer.py b/openedx/core/djangoapps/discussions/tests/test_transformer.py new file mode 100644 index 0000000000..48c17499a8 --- /dev/null +++ b/openedx/core/djangoapps/discussions/tests/test_transformer.py @@ -0,0 +1,80 @@ +""" +Tests for discussions course block transformer +""" + +from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.course_blocks.transformers.tests.helpers import TransformerRegistryTestMixin +from openedx.core.djangoapps.discussions.models import DEFAULT_PROVIDER_TYPE, DiscussionTopicLink +from openedx.core.djangoapps.discussions.transformers import DiscussionsTopicLinkTransformer +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class DiscussionsTopicLinkTransformerTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase): + """ + Tests behaviour of BlockCompletionTransformer + """ + TRANSFORMER_CLASS_TO_TEST = DiscussionsTopicLinkTransformer + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + def setUp(self): + super().setUp() + self.test_topic_id = 'test-topic-id' + self.course = CourseFactory.create() + section = ItemFactory.create( + parent_location=self.course.location, + category="chapter", + ) + subsection1 = ItemFactory.create( + parent_location=section.location, + category="sequential", + ) + self.discussable_unit = ItemFactory.create( + parent_location=subsection1.location, + category="vertical", + # This won't really be used, but set it anyway + discussion_enabled=True, + ) + DiscussionTopicLink.objects.create( + context_key=self.course.id, + usage_key=self.discussable_unit.location, + title=self.discussable_unit.display_name, + provider_id=DEFAULT_PROVIDER_TYPE, + external_id=self.test_topic_id, + ) + self.non_discussable_unit = ItemFactory.create( + parent_location=subsection1.location, + category="vertical", + discussion_enabled=False, + ) + + def test_transform_aggregators(self): + """ + Tests that a unit that has a discussion topic link created will return the link + and topic id in the course block data. + """ + block_structure = get_course_blocks(self.user, self.course.location, self.transformers) + + embed_url = block_structure.get_xblock_field( + self.discussable_unit.location, + self.TRANSFORMER_CLASS_TO_TEST.EMBED_URL, + ) + assert embed_url == f"http://discussions-mfe/discussions/{self.course.id}/topics/{self.test_topic_id}" + + external_id = block_structure.get_xblock_field( + self.discussable_unit.location, + self.TRANSFORMER_CLASS_TO_TEST.EXTERNAL_ID, + ) + assert external_id == self.test_topic_id + + embed_url = block_structure.get_xblock_field( + self.non_discussable_unit.location, + self.TRANSFORMER_CLASS_TO_TEST.EMBED_URL, + ) + assert embed_url is None + + external_id = block_structure.get_xblock_field( + self.non_discussable_unit.location, + self.TRANSFORMER_CLASS_TO_TEST.EXTERNAL_ID, + ) + assert external_id is None diff --git a/openedx/core/djangoapps/discussions/transformers.py b/openedx/core/djangoapps/discussions/transformers.py new file mode 100644 index 0000000000..8fcf78bc1f --- /dev/null +++ b/openedx/core/djangoapps/discussions/transformers.py @@ -0,0 +1,49 @@ +""" +Discussions Topic Link Transformer +""" + +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer +from openedx.core.djangoapps.discussions.models import DiscussionTopicLink, DiscussionsConfiguration +from openedx.core.djangoapps.discussions.url_helpers import get_discussions_mfe_topic_url + + +class DiscussionsTopicLinkTransformer(BlockStructureTransformer): + """ + A transformer that adds discussion topic context to the xblock. + """ + WRITE_VERSION = 1 + READ_VERSION = 1 + EXTERNAL_ID = "discussions_id" + EMBED_URL = "discussions_url" + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "discussions_link" + + def transform(self, usage_info, block_structure): + """ + loads override data into blocks + """ + provider_type = DiscussionsConfiguration.get(usage_info.course_key).provider_type + topic_links = DiscussionTopicLink.objects.filter( + context_key=usage_info.course_key, + provider_id=provider_type, + enabled_in_context=True, + ) + for topic_link in topic_links: + block_structure.override_xblock_field( + topic_link.usage_key, + DiscussionsTopicLinkTransformer.EXTERNAL_ID, + topic_link.external_id, + ) + mfe_embed_link = get_discussions_mfe_topic_url(usage_info.course_key, topic_link.external_id) + if mfe_embed_link: + block_structure.override_xblock_field( + topic_link.usage_key, + DiscussionsTopicLinkTransformer.EMBED_URL, + mfe_embed_link, + ) diff --git a/openedx/core/djangoapps/discussions/url_helpers.py b/openedx/core/djangoapps/discussions/url_helpers.py new file mode 100644 index 0000000000..b9a4e5953f --- /dev/null +++ b/openedx/core/djangoapps/discussions/url_helpers.py @@ -0,0 +1,35 @@ +""" +Helps for building discussions URLs +""" +from django.conf import settings +from opaque_keys.edx.keys import CourseKey + + +def get_discussions_mfe_url(course_key: CourseKey) -> str: + """ + Returns the url for discussions for the specified course in the discussions MFE. + + Args: + course_key (CourseKey): course key of course for which to get url + + Returns: + (str) URL link for MFE. Empty if the base url isn't configured + """ + if settings.DISCUSSIONS_MICROFRONTEND_URL is not None: + return f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/discussions/{course_key}/" + return '' + + +def get_discussions_mfe_topic_url(course_key: CourseKey, topic_id: str) -> str: + """ + Returns the url for discussions for the specified course and topic in the discussions MFE. + + Args: + course_key (CourseKey): course key of course for which to get url + + Returns: + (str) URL link for MFE. Empty if the base url isn't configured + """ + if settings.DISCUSSIONS_MICROFRONTEND_URL is not None: + return f"{get_discussions_mfe_url(course_key)}topics/{topic_id}" + return '' diff --git a/setup.py b/setup.py index 7680203d6c..0cdcaa04f3 100644 --- a/setup.py +++ b/setup.py @@ -79,6 +79,7 @@ setup( "access_denied_message_filter = lms.djangoapps.course_blocks.transformers.access_denied_filter:AccessDeniedMessageFilterTransformer", # lint-amnesty, pylint: disable=line-too-long "open_assessment_transformer = lms.djangoapps.courseware.transformers:OpenAssessmentDateTransformer", 'effort_estimation = openedx.features.effort_estimation.api:EffortEstimationTransformer', + 'discussions_link = openedx.core.djangoapps.discussions.transformers:DiscussionsTopicLinkTransformer', ], "openedx.ace.policy": [ "bulk_email_optout = lms.djangoapps.bulk_email.policies:CourseEmailOptout"