Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Course blocks API with param return_type=list #34424

Merged
merged 2 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 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,29 @@
"""
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 ( # lint-amnesty, pylint: disable=wrong-import-order
SharedModuleStoreTestCase
)
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 +500,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 +531,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
Loading