Skip to content

Commit

Permalink
fix: Course blocks API with param return_type=list error when the new…
Browse files Browse the repository at this point in the history
… discussion is enabled
  • Loading branch information
qasim.gulzar committed Mar 28, 2024
1 parent 68c3e24 commit 73a6958
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 25 deletions.
45 changes: 30 additions & 15 deletions lms/djangoapps/course_api/blocks/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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(
Expand All @@ -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:
Expand Down
37 changes: 27 additions & 10 deletions lms/djangoapps/course_api/blocks/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""
Utils for Blocks
"""
from rest_framework.utils.serializer_helpers import ReturnList

from openedx.core.djangoapps.discussions.models import (
DiscussionsConfiguration,
Provider,
Expand All @@ -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():
Expand All @@ -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

0 comments on commit 73a6958

Please sign in to comment.