From 6547ceab88ead473d65da370ec33002fd403d060 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 1 Jul 2025 13:56:32 +0300 Subject: [PATCH 1/4] fix: [AXM-2405] Fix 500 on return_type is list --- .../course_api/blocks/tests/test_views.py | 2 +- lms/djangoapps/course_api/blocks/utils.py | 38 ++++++++++++------- lms/djangoapps/course_api/blocks/views.py | 4 +- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index f577762ad70f..b561ad3ca5d3 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -516,7 +516,7 @@ def test_filter_discussion_xblocks(self, is_openedx_provider, return_type): def blocks_has_discussion_xblock(blocks): if isinstance(blocks, ReturnList): for value in blocks: - if value.get('type') == 'discussion': + if value['type'] == 'discussion': return True else: for key, value in blocks.items(): diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 0af24b951ade..7e3a3c799dd0 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -28,11 +28,11 @@ def filter_discussion_xblocks_from_response(response, course_key): ] # Filtering discussion xblocks keys from blocks if isinstance(response.data, ReturnList): - filtered_blocks = { - value.get('id'): value + filtered_blocks = [ + value for value in response.data if value.get('type') != 'discussion' - } + ] else: filtered_blocks = { key: value @@ -41,17 +41,29 @@ def filter_discussion_xblocks_from_response(response, course_key): } # 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(): - for key in ['descendants', 'children']: - descendants = block_data.get(key, []) - if descendants: - descendants = [ - descendant for descendant in descendants - if descendant not in discussion_xblocks - ] - block_data[key] = descendants if isinstance(response.data, ReturnList): - response.data = filtered_blocks + for block_data in filtered_blocks: + _put_xblock_descendants(block_data, discussion_xblocks) + else: + for _, block_data in filtered_blocks.items(): + _put_xblock_descendants(block_data, discussion_xblocks) + + if isinstance(response.data, ReturnList): + response.data = ReturnList(filtered_blocks, serializer=None) else: response.data['blocks'] = filtered_blocks return response + + +def _put_xblock_descendants(block_data, discussion_xblocks): + """ + Put descendants into xblock data if they existed before filtering. + """ + for key in ['descendants', 'children']: + descendants = block_data.get(key, []) + if descendants: + descendants = [ + descendant for descendant in descendants + if descendant not in discussion_xblocks + ] + block_data[key] = descendants diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 0697e9321989..96679a562957 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -330,12 +330,14 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments if course_block.get('type') == 'course': root = course_block['id'] + else: + root = str(course_usage_key) else: root = response.data['root'] course_blocks = response.data['blocks'] if not root: - raise ValueError(f"Unable to find course block in {course_key_string}") + raise ValidationError(f"Unable to find course block in '{course_key_string}'") recurse_mark_complete(root, course_blocks) return response From 5b66a877a01362b88237f36fb6cbde09c9bb9595 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 8 Jul 2025 15:55:03 +0300 Subject: [PATCH 2/4] refactor: [AXM-2405] refactor filter_discussion_xblocks_from_response --- lms/djangoapps/course_api/blocks/utils.py | 95 +++++++++++------------ 1 file changed, 45 insertions(+), 50 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 7e3a3c799dd0..28772e1357d2 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -11,59 +11,54 @@ def filter_discussion_xblocks_from_response(response, course_key): """ - Removes discussion xblocks if discussion provider is openedx + Removes discussion xblocks if discussion provider is openedx. """ configuration = DiscussionsConfiguration.get(context_key=course_key) provider = configuration.provider_type - if provider == Provider.OPEN_EDX: - # Finding ids of discussion xblocks - 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 - if isinstance(response.data, ReturnList): - filtered_blocks = [ - 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 - if isinstance(response.data, ReturnList): - for block_data in filtered_blocks: - _put_xblock_descendants(block_data, discussion_xblocks) - else: - for _, block_data in filtered_blocks.items(): - _put_xblock_descendants(block_data, discussion_xblocks) - if isinstance(response.data, ReturnList): - response.data = ReturnList(filtered_blocks, serializer=None) - else: - response.data['blocks'] = filtered_blocks - return response + if provider != Provider.OPEN_EDX: + return response + is_list_response = isinstance(response.data, ReturnList) -def _put_xblock_descendants(block_data, discussion_xblocks): - """ - Put descendants into xblock data if they existed before filtering. - """ - for key in ['descendants', 'children']: - descendants = block_data.get(key, []) - if descendants: - descendants = [ - descendant for descendant in descendants - if descendant not in discussion_xblocks - ] - block_data[key] = descendants + # Find discussion xblock IDs + if is_list_response: + discussion_xblocks = [ + block.get('id') for block in response.data + if block.get('type') == 'discussion' + ] + else: + discussion_xblocks = [ + key for key, value in response.data.get('blocks', {}).items() + if value.get('type') == 'discussion' + ] + + # Filter out discussion blocks + if is_list_response: + filtered_blocks = [ + block for block in response.data + if block.get('type') != 'discussion' + ] + else: + filtered_blocks = { + key: value for key, value in response.data.get('blocks', {}).items() + if value.get('type') != 'discussion' + } + + # Remove references to discussion xblocks + blocks_iterable = filtered_blocks if is_list_response else filtered_blocks.values() + for block_data in blocks_iterable: + for key in ['descendants', 'children']: + if key in block_data: + block_data[key] = [ + descendant for descendant in block_data[key] + if descendant not in discussion_xblocks + ] + + # Update response + if is_list_response: + response.data = ReturnList(filtered_blocks, serializer=None) + else: + response.data['blocks'] = filtered_blocks + + return response From 09715ed155beb310b2f602d8e42cae0de124f560 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 8 Sep 2025 10:07:51 +0300 Subject: [PATCH 3/4] docs: restore a comment --- lms/djangoapps/course_api/blocks/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 28772e1357d2..6f371624b7df 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -46,6 +46,7 @@ def filter_discussion_xblocks_from_response(response, course_key): } # Remove references to discussion xblocks + # These references needs to be removed because they no longer exist blocks_iterable = filtered_blocks if is_list_response else filtered_blocks.values() for block_data in blocks_iterable: for key in ['descendants', 'children']: From bd03bdbcf1e6d1d8562a05bea9fd39c2ed734b0d Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 8 Sep 2025 10:08:36 +0300 Subject: [PATCH 4/4] style: use .get() in blocks loop --- lms/djangoapps/course_api/blocks/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index b561ad3ca5d3..f577762ad70f 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -516,7 +516,7 @@ def test_filter_discussion_xblocks(self, is_openedx_provider, return_type): def blocks_has_discussion_xblock(blocks): if isinstance(blocks, ReturnList): for value in blocks: - if value['type'] == 'discussion': + if value.get('type') == 'discussion': return True else: for key, value in blocks.items():