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: blocks v2 will filter discussion xblocks #31820

Merged
merged 1 commit into from
Mar 1, 2023
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
40 changes: 38 additions & 2 deletions lms/djangoapps/course_api/blocks/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""
Tests for Blocks Views
"""

import ddt

from datetime import datetime
from unittest import mock
Expand All @@ -16,8 +16,12 @@
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 xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import ToyCourseFactory # 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 @@ -398,6 +402,7 @@ def test_with_list_field_url(self):
self.verify_response_with_requested_fields(response)


@ddt.ddt
class TestBlocksInCourseView(TestBlocksView, CompletionWaffleTestMixin): # pylint: disable=test-inherits-tests
"""
Test class for BlocksInCourseView
Expand Down Expand Up @@ -492,6 +497,37 @@ def test_completion_all_course_with_nav_depth(self):
for block_id in self.non_orphaned_block_usage_keys:
assert response.data['blocks'][block_id].get('completion')

@ddt.data(
False,
True,
)
def test_filter_discussion_xblocks(self, is_openedx_provider):
"""
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
return False

BlockFactory.create(
parent_location=self.course.location,
category="discussion",
discussion_id='topic_id',
discussion_category='category',
discussion_target='subcategory',
)
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)

has_discussion_xblock = blocks_has_discussion_xblock(response.data.get('blocks', {}))
if is_openedx_provider:
assert not has_discussion_xblock
else:
assert has_discussion_xblock


class TestBlockMetadataView(SharedModuleStoreTestCase): # pylint: disable=test-inherits-tests
"""
Expand Down
22 changes: 22 additions & 0 deletions lms/djangoapps/course_api/blocks/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"""
Utils for Blocks
"""
from openedx.core.djangoapps.discussions.models import (
DiscussionsConfiguration,
Provider,
)


def filter_discussion_xblocks_from_response(response, course_key):
"""
Removes discussion xblocks if discussion provider is openedx
"""
configuration = DiscussionsConfiguration.get(context_key=course_key)
provider = configuration.provider_type
if provider == Provider.OPEN_EDX:
response.data['blocks'] = {
key: value
for key, value in response.data.get('blocks', {}).items()
if value.get('type') != 'discussion'
}
return response
2 changes: 2 additions & 0 deletions lms/djangoapps/course_api/blocks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from .api import get_block_metadata, get_blocks
from .forms import BlockListGetForm
from .utils import filter_discussion_xblocks_from_response


@method_decorator(transaction.non_atomic_requests, name='dispatch')
Expand Down Expand Up @@ -311,6 +312,7 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments
raise ValidationError(f"'{str(course_key_string)}' is not a valid course key.") # lint-amnesty, pylint: disable=raise-missing-from
response = super().list(request, course_usage_key,
hide_access_denials=hide_access_denials)
response = filter_discussion_xblocks_from_response(response, course_key)

# Record user activity for tracking progress towards a user's course goals (for mobile app)
UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True)
Expand Down