From 73a69582d0e67434c7694559cb0561affbf48b5c Mon Sep 17 00:00:00 2001 From: "qasim.gulzar" Date: Tue, 26 Mar 2024 14:43:20 +0500 Subject: [PATCH 1/2] fix: Course blocks API with param return_type=list error when the new discussion is enabled --- .../course_api/blocks/tests/test_views.py | 45 ++++++++++++------- lms/djangoapps/course_api/blocks/utils.py | 37 ++++++++++----- 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index e2426708d6bc..d26ad37c99c1 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -1,27 +1,28 @@ """ Tests for Blocks Views """ -import ddt - from datetime import datetime from unittest import mock from unittest.mock import Mock from urllib.parse import urlencode, urlunparse +import ddt from completion.test_utils import CompletionWaffleTestMixin, submit_completions_for_testing from django.conf import settings from django.urls import reverse from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from rest_framework.utils.serializer_helpers import ReturnList from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseDataResearcherRole from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory -from openedx.core.djangoapps.discussions.models import ( - DiscussionsConfiguration, - Provider, +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider +from xmodule.modulestore.tests.django_utils import \ + SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.factories import ( # lint-amnesty, pylint: disable=wrong-import-order + BlockFactory, + ToyCourseFactory ) -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import BlockFactory, ToyCourseFactory # lint-amnesty, pylint: disable=wrong-import-order from .helpers import deserialize_usage_key @@ -498,17 +499,26 @@ def test_completion_all_course_with_nav_depth(self): assert response.data['blocks'][block_id].get('completion') @ddt.data( - False, - True, + (False, 'list'), + (True, 'list'), + (False, 'dict'), + (True, 'dict'), ) - def test_filter_discussion_xblocks(self, is_openedx_provider): + @ddt.unpack + def test_filter_discussion_xblocks(self, is_openedx_provider, return_type): """ Tests if discussion xblocks are hidden for openedx provider """ + def blocks_has_discussion_xblock(blocks): - for key, value in blocks.items(): - if value.get('type') == 'discussion': - return True + if isinstance(blocks, ReturnList): + for value in blocks: + if value.get('type') == 'discussion': + return True + else: + for key, value in blocks.items(): + if value.get('type') == 'discussion': + return True return False BlockFactory.create( @@ -520,9 +530,14 @@ def blocks_has_discussion_xblock(blocks): ) if is_openedx_provider: DiscussionsConfiguration.objects.create(context_key=self.course_key, provider_type=Provider.OPEN_EDX) - response = self.client.get(self.url, self.query_params) + params = self.query_params.copy() + if return_type == 'list': + params['return_type'] = 'list' + response = self.client.get(self.url, params) - has_discussion_xblock = blocks_has_discussion_xblock(response.data.get('blocks', {})) + has_discussion_xblock = blocks_has_discussion_xblock( + response.data if isinstance(response.data, ReturnList) else response.data.get('blocks', {}) + ) if is_openedx_provider: assert not has_discussion_xblock else: diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 3a9fea117c23..0af24b951ade 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -1,6 +1,8 @@ """ Utils for Blocks """ +from rest_framework.utils.serializer_helpers import ReturnList + from openedx.core.djangoapps.discussions.models import ( DiscussionsConfiguration, Provider, @@ -15,16 +17,28 @@ def filter_discussion_xblocks_from_response(response, course_key): provider = configuration.provider_type if provider == Provider.OPEN_EDX: # Finding ids of discussion xblocks - discussion_xblocks = [ - key for key, value in response.data.get('blocks', {}).items() - if value.get('type') == 'discussion' - ] + if isinstance(response.data, ReturnList): + discussion_xblocks = [ + value.get('id') for value in response.data if value.get('type') == 'discussion' + ] + else: + discussion_xblocks = [ + key for key, value in response.data.get('blocks', {}).items() + if value.get('type') == 'discussion' + ] # Filtering discussion xblocks keys from blocks - filtered_blocks = { - key: value - for key, value in response.data.get('blocks', {}).items() - if value.get('type') != 'discussion' - } + if isinstance(response.data, ReturnList): + filtered_blocks = { + value.get('id'): value + for value in response.data + if value.get('type') != 'discussion' + } + else: + filtered_blocks = { + key: value + for key, value in response.data.get('blocks', {}).items() + if value.get('type') != 'discussion' + } # Removing reference of discussion xblocks from unit # These references needs to be removed because they no longer exist for _, block_data in filtered_blocks.items(): @@ -36,5 +50,8 @@ def filter_discussion_xblocks_from_response(response, course_key): if descendant not in discussion_xblocks ] block_data[key] = descendants - response.data['blocks'] = filtered_blocks + if isinstance(response.data, ReturnList): + response.data = filtered_blocks + else: + response.data['blocks'] = filtered_blocks return response From 3f709bae3f5f14e07984da5028565be8aa021cfb Mon Sep 17 00:00:00 2001 From: "qasim.gulzar" Date: Thu, 2 May 2024 11:53:49 +0500 Subject: [PATCH 2/2] fix: add parentheses to import --- lms/djangoapps/course_api/blocks/tests/test_views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index d26ad37c99c1..c1f673652096 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -17,8 +17,9 @@ from common.djangoapps.student.roles import CourseDataResearcherRole from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider -from xmodule.modulestore.tests.django_utils import \ - SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order + SharedModuleStoreTestCase +) from xmodule.modulestore.tests.factories import ( # lint-amnesty, pylint: disable=wrong-import-order BlockFactory, ToyCourseFactory