diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 9d4c24f1bc18..091ce38561fb 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -17,11 +17,13 @@ from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from common.djangoapps.util.course import get_link_for_about_page from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin -from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.utils import API_V05, API_V1, API_V2, API_V3, API_V4 from lms.djangoapps.mobile_api.course_info.views import BlocksInfoInCourseView from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_experience import ENABLE_COURSE_GOALS +from openedx.features.offline_mode.constants import DEFAULT_OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.toggles import ENABLE_OFFLINE_MODE from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -460,12 +462,12 @@ def test_extend_block_info_with_offline_data( get_offline_block_content_path_mock: MagicMock, default_storage_mock: MagicMock, ) -> None: - url = reverse('blocks_info_in_course', kwargs={'api_version': 'v4'}) + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) offline_content_path_mock = '/offline_content_path_mock/' created_time_mock = 'created_time_mock' size_mock = 'size_mock' get_offline_block_content_path_mock.return_value = offline_content_path_mock - default_storage_mock.get_created_time.return_value = created_time_mock + default_storage_mock.get_modified_time.return_value = created_time_mock default_storage_mock.size.return_value = size_mock expected_offline_download_data = { @@ -483,14 +485,14 @@ def test_extend_block_info_with_offline_data( @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') @ddt.data( - ('v0.5', True), - ('v0.5', False), - ('v1', True), - ('v1', False), - ('v2', True), - ('v2', False), - ('v3', True), - ('v3', False), + (API_V05, True), + (API_V05, False), + (API_V1, True), + (API_V1, False), + (API_V2, True), + (API_V2, False), + (API_V3, True), + (API_V3, False), ) @ddt.unpack def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waffle_flag( @@ -507,3 +509,27 @@ def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waf self.assertEqual(response.status_code, status.HTTP_200_OK) for block_info in response.data['blocks'].values(): self.assertNotIn('offline_download', block_info) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_create_offline_content_integration_test(self, save_mathjax_to_xblock_assets_mock: MagicMock) -> None: + UserFactory.create(username='offline_mode_worker', password='password', is_staff=True) + handle_course_published_url = reverse('offline_mode:handle_course_published') + self.client.login(username='offline_mode_worker', password='password') + + handler_response = self.client.post(handle_course_published_url, {'course_id': str(self.course.id)}) + self.assertEqual(handler_response.status_code, status.HTTP_200_OK) + + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) + + response = self.verify_response(url=url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + if block_type := block_info.get('type'): + if block_type in DEFAULT_OFFLINE_SUPPORTED_XBLOCKS: + expected_offline_content_url = f'/uploads/{self.course.id}/{block_info["block_id"]}.zip' + self.assertIn('offline_download', block_info) + self.assertIn('file_url', block_info['offline_download']) + self.assertIn('last_modified', block_info['offline_download']) + self.assertIn('file_size', block_info['offline_download']) + self.assertEqual(expected_offline_content_url, block_info['offline_download']['file_url']) diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py index a3e8c776ff32..cf280900177e 100644 --- a/openedx/features/offline_mode/tests/test_html_manipulator.py +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -4,7 +4,7 @@ from bs4 import BeautifulSoup from unittest import TestCase -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH from openedx.features.offline_mode.html_manipulator import HtmlManipulator @@ -17,6 +17,8 @@ class HtmlManipulatorTestCase(TestCase): @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_iframe') @patch('openedx.features.offline_mode.html_manipulator.BeautifulSoup', return_value='soup_mock') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._copy_platform_fonts') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_external_links') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_mathjax_link') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_static_links') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_asset_links') @@ -25,6 +27,8 @@ def test_process_html( replace_asset_links_mock: MagicMock, replace_static_links_mock: MagicMock, replace_mathjax_link_mock: MagicMock, + replace_external_links: MagicMock, + copy_platform_fonts: MagicMock, beautiful_soup_mock: MagicMock, replace_iframe_mock: MagicMock, ) -> None: @@ -39,6 +43,8 @@ def test_process_html( replace_asset_links_mock.assert_called_once_with() replace_static_links_mock.assert_called_once_with() replace_mathjax_link_mock.assert_called_once_with() + replace_external_links.assert_called_once_with() + copy_platform_fonts.assert_called_once_with() beautiful_soup_mock.assert_called_once_with(html_manipulator.html_data, 'html.parser') replace_iframe_mock.assert_called_once_with(beautiful_soup_mock.return_value) self.assertEqual(result, expected_result) @@ -116,3 +122,42 @@ def test_replace_iframe(self): HtmlManipulator._replace_iframe(soup) # lint-amnesty, pylint: disable=protected-access self.assertEqual(f'{soup.find_all("p")[0]}', expected_html_markup) + + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_links(self, save_external_file_mock: MagicMock) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = """ + Example Image + + """ + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + html_manipulator._replace_external_links() # lint-amnesty, pylint: disable=protected-access + + self.assertEqual(save_external_file_mock.call_count, 2) + + @patch('openedx.features.offline_mode.html_manipulator.uuid.uuid4') + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_link( + self, + save_external_file_mock: MagicMock, + uuid_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = 'html_data_mock' + external_url_mock = 'https://cdn.example.com/image.jpg' + uuid_result_mock = '123e4567-e89b-12d3-a456-426655440000' + uuid_mock.return_value = uuid_result_mock + mock_match = MagicMock() + mock_match.group.side_effect = [external_url_mock, 'jpg'] + + expected_result = 'assets/external/123e4567-e89b-12d3-a456-426655440000.jpg' + expected_save_external_file_args = [call(temp_dir_mock, external_url_mock, expected_result)] + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + result = html_manipulator._replace_external_link(mock_match) # lint-amnesty, pylint: disable=protected-access + + self.assertListEqual(save_external_file_mock.call_args_list, expected_save_external_file_args) + self.assertEqual(result, expected_result) diff --git a/openedx/features/offline_mode/tests/test_storage_management.py b/openedx/features/offline_mode/tests/test_storage_management.py index c226c20ac1f8..f77f2ba614cd 100644 --- a/openedx/features/offline_mode/tests/test_storage_management.py +++ b/openedx/features/offline_mode/tests/test_storage_management.py @@ -18,91 +18,62 @@ class OfflineContentGeneratorTestCase(TestCase): """ Test case for the testing Offline Mode utils. """ - - @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') - @patch('openedx.features.offline_mode.storage_management.mkdtemp') - @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') - @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) - def test_generate_offline_content_for_modified_xblock( - self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - clean_outdated_xblock_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, - ) -> None: + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful(self, xblock_renderer_mock: MagicMock) -> None: xblock_mock = Mock() html_data_mock = 'html_markup_data_mock' - OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + result = OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_called_once_with(xblock_mock) - clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) - mkdtemp_mock.assert_called_once_with() - save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value) - create_zip_file_mock.assert_called_once_with( - mkdtemp_mock.return_value, - block_storage_path_mock.return_value, - f'{xblock_mock.location.block_id}.zip' + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + xblock_renderer_mock.return_value.render_xblock_from_lms.assert_called_once_with() + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) + + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful_no_html_data(self, xblock_renderer_mock: MagicMock) -> None: + xblock_mock = Mock() + expected_xblock_renderer_args_list = [call(str(xblock_mock.location)), call(str(xblock_mock.location))] + + result = OfflineContentGenerator(xblock_mock).render_block_html_data() + + self.assertListEqual(xblock_renderer_mock.call_args_list, expected_xblock_renderer_args_list) + self.assertListEqual( + xblock_renderer_mock.return_value.render_xblock_from_lms.call_args_list, [call(), call()] ) - shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) - @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') - @patch('openedx.features.offline_mode.storage_management.mkdtemp') - @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') - @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=False) - def test_generate_offline_content_is_not_modified( + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer', side_effect=Http404) + def test_render_block_html_data_http404( self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - clean_outdated_xblock_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, + xblock_renderer_mock: MagicMock, + logger_mock: MagicMock, ) -> None: xblock_mock = Mock() html_data_mock = 'html_markup_data_mock' - OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + with self.assertRaises(Http404): + OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_not_called() - clean_outdated_xblock_files_mock.assert_not_called() - mkdtemp_mock.assert_not_called() - save_xblock_html_mock.assert_not_called() - create_zip_file_mock.assert_not_called() - shutil_rmtree_mock.assert_not_called() + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + logger_mock.assert_called_once_with( + f'Block {str(xblock_mock.location)} cannot be fetched from course' + f' {xblock_mock.location.course_key} during offline content generation.' + ) @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.log.error') - @patch( - 'openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file', - side_effect=Http404, - ) + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') @patch('openedx.features.offline_mode.storage_management.mkdtemp') @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) - def test_generate_offline_content_is_not_modified_with_http_404_status( + def test_generate_offline_content_for_modified_xblock( self, - is_modified_mock: MagicMock, block_storage_path_mock: MagicMock, clean_outdated_xblock_files_mock: MagicMock, mkdtemp_mock: MagicMock, save_xblock_html_mock: MagicMock, create_zip_file_mock: MagicMock, - logger_mock: MagicMock, shutil_rmtree_mock: MagicMock, ) -> None: xblock_mock = Mock() @@ -110,7 +81,6 @@ def test_generate_offline_content_is_not_modified_with_http_404_status( OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() - is_modified_mock.assert_called_once_with(xblock_mock) block_storage_path_mock.assert_called_once_with(xblock_mock) clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) mkdtemp_mock.assert_called_once_with() @@ -120,10 +90,6 @@ def test_generate_offline_content_is_not_modified_with_http_404_status( block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' ) - logger_mock.assert_called_once_with( - f'Block {xblock_mock.location.block_id} cannot be fetched from course' - f' {xblock_mock.location.course_key} during offline content generation.' - ) shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) @patch('openedx.features.offline_mode.storage_management.os.path.join') diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py index 75512d8e8b96..ef468d825d7b 100644 --- a/openedx/features/offline_mode/tests/test_tasks.py +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -5,15 +5,17 @@ from unittest import TestCase from unittest.mock import MagicMock, Mock, call, patch +from ddt import data, ddt, unpack +from django.http.response import Http404 from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.features.offline_mode.constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS from openedx.features.offline_mode.tasks import ( generate_offline_content_for_block, generate_offline_content_for_course, ) -from xmodule.modulestore.exceptions import ItemNotFoundError +@ddt class GenerateOfflineContentTasksTestCase(TestCase): """ Test case for the testing generating offline content tacks. @@ -27,33 +29,29 @@ def test_generate_offline_content_for_block_success( offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block(block_id_mock, html_data_mock) + generate_offline_content_for_block(block_id_mock) modulestore_mock.assert_called_once_with() modulestore_mock.return_value.get_item.assert_called_once_with(UsageKey.from_string(block_id_mock)) - offline_content_generator_mock.assert_called_once_with( - modulestore_mock.return_value.get_item.return_value, html_data_mock - ) + offline_content_generator_mock.assert_called_once_with(modulestore_mock.return_value.get_item.return_value) offline_content_generator_mock.return_value.generate_offline_content.assert_called_once_with() @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') - @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=ItemNotFoundError) + @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=Http404) def test_generate_offline_content_for_block_with_exception_in_modulestore( self, modulestore_mock: MagicMock, offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + generate_offline_content_for_block.delay(block_id_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) offline_content_generator_mock.assert_not_called() - @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=FileNotFoundError) + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=Http404) @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_block_with_exception_in_offline_content_generation( self, @@ -61,38 +59,36 @@ def test_generate_offline_content_for_block_with_exception_in_offline_content_ge offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + generate_offline_content_for_block.delay(block_id_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) self.assertEqual(offline_content_generator_mock.call_count, MAX_RETRY_ATTEMPTS + 1) @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') - @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.is_modified') @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_course_supported_block_types( self, modulestore_mock: MagicMock, - xblock_renderer_mock: MagicMock, + is_modified_mock: MagicMock, generate_offline_content_for_block_mock: MagicMock, ) -> None: course_id_mock = 'course-v1:a+a+a' xblock_location_mock = 'xblock_location_mock' - html_data_mock = 'html_markup_data_mock' modulestore_mock.return_value.get_items.return_value = [ Mock(location=xblock_location_mock, closed=Mock(return_value=False)) ] - xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + expected_call_args_for_modulestore_get_items = [ call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS ] - expected_call_args_for_xblock_renderer_mock = [ - call(xblock_location_mock) for _ in OFFLINE_SUPPORTED_XBLOCKS + expected_call_args_is_modified_mock = [ + call(modulestore_mock.return_value.get_items.return_value[0]) for _ in OFFLINE_SUPPORTED_XBLOCKS ] expected_call_args_for_generate_offline_content_for_block_mock = [ - call([xblock_location_mock, html_data_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS + call([xblock_location_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS ] generate_offline_content_for_course(course_id_mock) @@ -101,30 +97,36 @@ def test_generate_offline_content_for_course_supported_block_types( self.assertListEqual( modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items ) - self.assertListEqual( - xblock_renderer_mock.call_args_list, expected_call_args_for_xblock_renderer_mock - ) + self.assertListEqual(is_modified_mock.call_args_list, expected_call_args_is_modified_mock) self.assertListEqual( generate_offline_content_for_block_mock.apply_async.call_args_list, expected_call_args_for_generate_offline_content_for_block_mock ) @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') - @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.is_modified') @patch('openedx.features.offline_mode.tasks.modulestore') - def test_generate_offline_content_for_course_supported_block_types_closed_xblock( + @data( + (False, False), + (True, False), + (False, True), + ) + @unpack + def test_generate_offline_content_for_course_supported_block_types_for_closed_or_not_modified_xblock( self, + is_modified_value_mock: bool, + is_closed_value_mock: bool, modulestore_mock: MagicMock, - xblock_renderer_mock: MagicMock, + is_modified_mock: MagicMock, generate_offline_content_for_block_mock: MagicMock, ) -> None: course_id_mock = 'course-v1:a+a+a' xblock_location_mock = 'xblock_location_mock' - html_data_mock = 'html_markup_data_mock' modulestore_mock.return_value.get_items.return_value = [ - Mock(location=xblock_location_mock, closed=Mock(return_value=True)) + Mock(location=xblock_location_mock, closed=Mock(return_value=is_closed_value_mock)) ] - xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + is_modified_mock.return_value = is_modified_value_mock + expected_call_args_for_modulestore_get_items = [ call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS @@ -136,5 +138,4 @@ def test_generate_offline_content_for_course_supported_block_types_closed_xblock self.assertListEqual( modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items ) - xblock_renderer_mock.assert_not_called() generate_offline_content_for_block_mock.assert_not_called()