From 62652c742260b221c1ab39953c2504d06503cd70 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 20 Jun 2023 15:30:27 +0500 Subject: [PATCH 1/2] enhancement: add support for caching programs for one site (#32380) (#32506) * perf!: add support for caching programs on per site bases --- .../management/commands/cache_programs.py | 13 +++- .../commands/tests/test_cache_programs.py | 77 +++++++++++++++---- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py index b815f8b4e4e5..9e0664f4930f 100644 --- a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py @@ -45,8 +45,17 @@ class Command(BaseCommand): """ help = "Rebuild the LMS' cache of program data." + def add_arguments(self, parser): + parser.add_argument( + '--domain', + dest='domain', + type=str, + help='Help in caching the programs for one site' + ) + # lint-amnesty, pylint: disable=bad-option-value, unicode-format-string def handle(self, *args, **options): # lint-amnesty, pylint: disable=too-many-statements + domain = options.get('domain', '') failure = False logger.info('populate-multitenant-programs switch is ON') @@ -68,7 +77,9 @@ def handle(self, *args, **options): # lint-amnesty, pylint: disable=too-many-st programs_by_type = {} programs_by_type_slug = {} organizations = {} - for site in Site.objects.all(): + + sites = Site.objects.filter(domain=domain) if domain else Site.objects.all() + for site in sites: site_config = getattr(site, 'configuration', None) if site_config is None or not site_config.get_value('COURSE_CATALOG_API_URL'): logger.info(f'Skipping site {site.domain}. No configuration.') diff --git a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py index 331d53a73042..4078b2d90d55 100644 --- a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py @@ -48,29 +48,53 @@ def setUp(self): } ) + self.site_domain2 = 'testsite2.com' + self.site2 = self.set_up_site( + self.site_domain2, + { + 'COURSE_CATALOG_API_URL': self.catalog_integration.get_internal_api_url().rstrip('/') + } + ) + self.list_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/programs/' self.detail_tpl = self.list_url.rstrip('/') + '/{uuid}/' self.pathway_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/pathways/' self.programs = ProgramFactory.create_batch(3) + self.programs2 = ProgramFactory.create_batch(3) self.pathways = PathwayFactory.create_batch(3) + self.pathways2 = PathwayFactory.create_batch(3) self.child_program = ProgramFactory.create() + self.child_program2 = ProgramFactory.create() self.programs[0]['curricula'][0]['programs'].append(self.child_program) self.programs.append(self.child_program) - self.programs[0]['authoring_organizations'] = OrganizationFactory.create_batch(2) + self.programs2[0]['curricula'][0]['programs'].append(self.child_program2) + self.programs2.append(self.child_program2) + self.programs2[0]['authoring_organizations'] = OrganizationFactory.create_batch(2) + for pathway in self.pathways: self.programs += pathway['programs'] - self.uuids = [program['uuid'] for program in self.programs] + for pathway in self.pathways2: + self.programs2 += pathway['programs'] + + self.uuids = { + f"{self.site_domain}": [program["uuid"] for program in self.programs], + f"{self.site_domain2}": [program["uuid"] for program in self.programs2], + } # add some of the previously created programs to some pathways self.pathways[0]['programs'].extend([self.programs[0], self.programs[1]]) self.pathways[1]['programs'].append(self.programs[0]) - def mock_list(self): + # add some of the previously created programs to some pathways + self.pathways2[0]['programs'].extend([self.programs2[0], self.programs2[1]]) + self.pathways2[1]['programs'].append(self.programs2[0]) + + def mock_list(self, site=""): """ Mock the data returned by the program listing API endpoint. """ # pylint: disable=unused-argument def list_callback(request, uri, headers): @@ -81,8 +105,8 @@ def list_callback(request, uri, headers): 'uuids_only': ['1'] } assert request.querystring == expected - - return (200, headers, json.dumps(self.uuids)) + uuids = self.uuids[self.site_domain2] if site else self.uuids[self.site_domain] + return (200, headers, json.dumps(uuids)) httpretty.register_uri( httpretty.GET, @@ -130,17 +154,36 @@ def pathways_callback(request, uri, headers): # pylint: disable=unused-argument return (200, headers, json.dumps(body)) - # NOTE: httpretty does not properly match against query strings here (using match_querystring arg) - # as such, it does not actually look at the query parameters (for page num), but returns items in a LIFO order. - # this means that for multiple pages, you must call this function starting from the last page. - # we do assert the page number query param above, however httpretty.register_uri( httpretty.GET, - self.pathway_url, + self.pathway_url + f'?exclude_utm=1&page={page_number}', body=pathways_callback, content_type='application/json', + match_querystring=True, ) + def test_handle_domain(self): + """ + Verify that the command argument is working correct or not. + """ + UserFactory(username=self.catalog_integration.service_username) + + programs = { + PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']): program for program in self.programs2 + } + + self.mock_list(self.site2) + self.mock_pathways(self.pathways2) + + for uuid in self.uuids[self.site_domain2]: + program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] + self.mock_detail(uuid, program) + + call_command('cache_programs', f'--domain={self.site_domain2}') + + cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain2)) + assert set(cached_uuids) == set(self.uuids[self.site_domain2]) + def test_handle_programs(self): """ Verify that the command requests and caches program UUIDs and details. @@ -158,14 +201,14 @@ def test_handle_programs(self): self.mock_list() self.mock_pathways(self.pathways) - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) call_command('cache_programs') cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain)) - assert set(cached_uuids) == set(self.uuids) + assert set(cached_uuids) == set(self.uuids[self.site_domain]) program_keys = list(programs.keys()) cached_programs = cache.get_many(program_keys) @@ -228,7 +271,7 @@ def test_handle_pathways(self): self.mock_list() self.mock_pathways(self.pathways) - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -267,7 +310,7 @@ def test_pathways_multiple_pages(self): } self.mock_list() - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -337,7 +380,7 @@ def test_handle_missing_pathways(self): self.mock_list() - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -365,7 +408,7 @@ def test_handle_missing_programs(self): self.mock_list() - for uuid in self.uuids[:2]: + for uuid in self.uuids[self.site_domain][:2]: program = partial_programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -375,7 +418,7 @@ def test_handle_missing_programs(self): assert context.value.code == 1 cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain)) - assert set(cached_uuids) == set(self.uuids) + assert set(cached_uuids) == set(self.uuids[self.site_domain]) program_keys = list(all_programs.keys()) cached_programs = cache.get_many(program_keys) From 42e84e386e833f6707eb2b6d4371ea8bd6cc693a Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Wed, 28 Sep 2022 18:03:46 +0300 Subject: [PATCH 2/2] fix: save scroll position on exit from video xblock fullscreen mode --- xmodule/js/src/video/04_video_full_screen.js | 23 ++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/xmodule/js/src/video/04_video_full_screen.js b/xmodule/js/src/video/04_video_full_screen.js index 0730300d105e..9da85d8dfb67 100644 --- a/xmodule/js/src/video/04_video_full_screen.js +++ b/xmodule/js/src/video/04_video_full_screen.js @@ -161,7 +161,21 @@ return this.videoFullScreen.height; } - /** + function notifyParent(fullscreenOpen) { + if (window !== window.parent) { + // This is used by the Learning MFE to know about changing fullscreen mode. + // The MFE is then able to respond appropriately and scroll window to the previous position. + window.parent.postMessage({ + type: 'plugin.videoFullScreen', + payload: { + open: fullscreenOpen + } + }, document.referrer + ); + } + } + + /** * Event handler to toggle fullscreen mode. * @param {jquery Event} event */ @@ -192,6 +206,8 @@ this.resizer.delta.reset().setMode('width'); } this.el.trigger('fullscreen', [this.isFullScreen]); + + this.videoFullScreen.notifyParent(false); } function handleEnter() { @@ -202,6 +218,8 @@ return; } + this.videoFullScreen.notifyParent(true); + this.videoFullScreen.fullScreenState = this.isFullScreen = true; fullScreenClassNameEl.addClass('video-fullscreen'); this.videoFullScreen.fullScreenEl @@ -267,7 +285,8 @@ handleFullscreenChange: handleFullscreenChange, toggle: toggle, toggleHandler: toggleHandler, - updateControlsHeight: updateControlsHeight + updateControlsHeight: updateControlsHeight, + notifyParent: notifyParent }; state.bindTo(methodsDict, state.videoFullScreen, state);