diff --git a/futurex_openedx_extensions/dashboard/views.py b/futurex_openedx_extensions/dashboard/views.py index af89342b..7bd98dc1 100644 --- a/futurex_openedx_extensions/dashboard/views.py +++ b/futurex_openedx_extensions/dashboard/views.py @@ -1435,6 +1435,7 @@ def put(self, request: Any, tenant_id: int) -> Response: data = request.data try: key = data['key'] + sub_key = (data.get('sub_key') or '').strip() if not isinstance(key, str): raise FXCodedException( code=FXExceptionCodes.INVALID_INPUT, message='Key name must be a string.' @@ -1458,17 +1459,17 @@ def put(self, request: Any, tenant_id: int) -> Response: update_draft_tenant_config( tenant_id=int(tenant_id), - config_path=key_access_info.path, + config_path=f'{key_access_info.path}.{sub_key}' if sub_key else key_access_info.path, current_revision_id=int(current_revision_id), new_value=new_value, reset=reset, user=request.user, ) - data = get_tenant_config(tenant_id=int(tenant_id), keys=[key], published_only=False) + result_data = get_tenant_config(tenant_id=int(tenant_id), keys=[key], published_only=False) return Response( status=http_status.HTTP_200_OK, - data=serializers.TenantConfigSerializer(data, context={'request': request}).data, + data=serializers.TenantConfigSerializer(result_data, context={'request': request}).data, ) except KeyError as exc: diff --git a/futurex_openedx_extensions/helpers/tenants.py b/futurex_openedx_extensions/helpers/tenants.py index f0328308..bcb0d5d8 100644 --- a/futurex_openedx_extensions/helpers/tenants.py +++ b/futurex_openedx_extensions/helpers/tenants.py @@ -499,6 +499,24 @@ def get_tenant_readable_lms_config(tenant_id: int) -> dict: return result +def _get_tenant_config_non_published( + tenant_id: int, lms_configs: Any, config_access_control: Any, cleaned_keys: set[str], revision_ids: Dict[str, int], +) -> Dict[str, Any]: + """Helper function to get non-published config values""" + search_keys = list(cleaned_keys & set(config_access_control.keys())) + config_paths = [config_access_control[key]['path'] for key in search_keys] + config_paths_reverse = { + value['path']: key for key, value in config_access_control.items() if key in search_keys + } + for _path in config_paths: + for extra_config_path, revision_id in DraftConfig.objects.filter( + config_path__startswith=f'{_path}.' + ).values_list('config_path', 'revision_id'): + extra_config_path = config_paths_reverse[_path] + extra_config_path[len(_path):] + revision_ids[extra_config_path] = revision_id + return DraftConfig.loads_into(tenant_id=tenant_id, config_paths=config_paths, dest=lms_configs) + + def get_tenant_config(tenant_id: int, keys: List[str], published_only: bool = True) -> Dict[str, Any]: """ Retrieve tenant configuration details for the given tenant ID. @@ -510,21 +528,24 @@ def get_tenant_config(tenant_id: int, keys: List[str], published_only: bool = Tr :raises FXCodedException: If the tenant is not found. """ lms_configs = get_tenant_readable_lms_config(tenant_id) + config_access_control = get_config_access_control() cleaned_keys = {key.strip() for key in keys} - draft_configs = {} + revision_ids: Dict[str, int] = {} if not published_only: - search_keys = list(cleaned_keys & set(config_access_control.keys())) - config_paths = [config_access_control[key]['path'] for key in search_keys] - draft_configs = DraftConfig.loads_into(tenant_id=tenant_id, config_paths=config_paths, dest=lms_configs) + draft_configs = _get_tenant_config_non_published( + tenant_id, lms_configs, config_access_control, cleaned_keys, revision_ids, + ) + else: + draft_configs = {} details: Dict[str, Any] = { 'values': {}, 'not_permitted': [], 'bad_keys': [], - 'revision_ids': {}, + 'revision_ids': revision_ids, } for key in list(cleaned_keys): @@ -533,7 +554,7 @@ def get_tenant_config(tenant_id: int, keys: List[str], published_only: bool = Tr _, config_value = dot_separated_path_get_value(lms_configs, config['path']) details['values'][key] = config_value if not published_only: - details['revision_ids'][key] = draft_configs[config['path']]['revision_id'] + revision_ids[key] = draft_configs[config['path']]['revision_id'] else: details['bad_keys'].append(key) diff --git a/tests/test_dashboard/test_views.py b/tests/test_dashboard/test_views.py index 04d9e69e..0e10aed5 100644 --- a/tests/test_dashboard/test_views.py +++ b/tests/test_dashboard/test_views.py @@ -2501,24 +2501,42 @@ def _prepare_data(tenant_id, config_path): assert tenant_config.lms_configs['platform_name'] == 's1 platform name' assert DraftConfig.objects.filter(tenant_id=tenant_id).count() == 1, 'bad test data' - ConfigAccessControl.objects.create(key_name='platform_name', path=config_path, writable=True) + ConfigAccessControl.objects.create(key_name='platform_name_key', path=config_path, writable=True) assert DraftConfig.objects.filter(tenant_id=tenant_id, config_path=config_path).count() == 0, 'bad test data' @patch('futurex_openedx_extensions.dashboard.views.ThemeConfigDraftView.validate_input') @patch('futurex_openedx_extensions.dashboard.views.update_draft_tenant_config') - def test_draft_config_update(self, mock_update_draft, mocked_validate_input): + @ddt.data( + (None, 'platform_name_key', 'platform_name'), + ('sub-key', 'platform_name_key.sub-key', 'platform_name.sub-key'), + ('sub-key.another_sub', 'platform_name_key.sub-key.another_sub', 'platform_name.sub-key.another_sub'), + ) + @ddt.unpack + def test_draft_config_update( + self, sub_key, final_key, result_config_path, mock_update_draft, mock_validate_input, + ): # pylint: disable=too-many-arguments """Verify that the view returns the correct response""" def _update_draft(**kwargs): """mock update_draft_tenant_config effect""" draft_config = DraftConfig.objects.create( tenant_id=1, config_path=config_path, - config_value=new_value, + config_value='get_tenant_config should fetch the value of the main key', created_by_id=1, updated_by_id=1, ) - draft_config.revision_id = 987 + draft_config.revision_id = 123 draft_config.save() + if sub_key: + draft_config = DraftConfig.objects.create( + tenant_id=1, + config_path=result_config_path, + config_value='should not be fetched by get_tenant_config', + created_by_id=1, + updated_by_id=1, + ) + draft_config.revision_id = 987 + draft_config.save() tenant_id = 1 config_path = 'platform_name' @@ -2527,38 +2545,42 @@ def _update_draft(**kwargs): self._prepare_data(tenant_id, config_path) - new_value = 's1 new name' mock_update_draft.side_effect = _update_draft + sub_key_new_value = 'should not be fetched by get_tenant_config, because it fetches the value of the main key' response = self.client.put( self.url, data={ - 'key': 'platform_name', - 'new_value': new_value, + 'key': 'platform_name_key', + 'sub_key': sub_key, + 'new_value': sub_key_new_value, 'current_revision_id': '456', }, format='json' ) self.assertEqual(response.status_code, http_status.HTTP_200_OK, response.data) - self.assertEqual(response.data, { - 'bad_keys': [], - 'not_permitted': [], - 'revision_ids': { - 'platform_name': '987', - }, - 'values': { - 'platform_name': new_value, - }, - }) mock_update_draft.assert_called_once_with( tenant_id=tenant_id, - config_path='platform_name', + config_path=result_config_path, current_revision_id=456, - new_value=new_value, + new_value=sub_key_new_value, reset=False, user=ANY, ) - mocked_validate_input.assert_called_once_with('456') + expected_response_data = { + 'bad_keys': [], + 'not_permitted': [], + 'revision_ids': { + 'platform_name_key': '123', + }, + 'values': { + 'platform_name_key': 'get_tenant_config should fetch the value of the main key', + }, + } + if sub_key: + expected_response_data['revision_ids'][final_key] = '987' + self.assertEqual(response.data, expected_response_data) + mock_validate_input.assert_called_once_with('456') @patch('futurex_openedx_extensions.dashboard.views.ThemeConfigDraftView.validate_input') @patch('futurex_openedx_extensions.dashboard.views.update_draft_tenant_config') @@ -2582,7 +2604,7 @@ def test_draft_config_update_reset(self, reset_value, expected_passed_value, moc self.client.put( self.url, data={ - 'key': 'platform_name', + 'key': 'platform_name_key', 'new_value': 'anything', 'current_revision_id': '0', 'reset': reset_value, diff --git a/tests/test_helpers/test_tenants.py b/tests/test_helpers/test_tenants.py index cb6361d2..0d435849 100644 --- a/tests/test_helpers/test_tenants.py +++ b/tests/test_helpers/test_tenants.py @@ -602,6 +602,49 @@ def test_get_tenant_config( f'FAILED: {usecase} - Expected {expected_bad_keys}, got {result["bad_keys"]}' +@pytest.mark.django_db +@pytest.mark.parametrize('published_only, expected_revision_ids', [ + (False, { + 'pages_key': 123, 'pages_key.draft_page2': 567, 'courses_key': 345, 'courses_key.course1.chapters': 789, + }), + (True, {}), +]) +def test_get_tenant_config_draft_sub_keys( + base_data, published_only, expected_revision_ids, +): # pylint: disable=unused-argument + """Verify get_tenant_config returns the revision IDs for all draft sub-keys.""" + def _new_draft_config(config_path, config_value, revision_id): + """Helper to create a new draft config.""" + draft_config = DraftConfig.objects.create( + tenant_id=1, config_path=config_path, config_value=config_value, + created_by_id=1, updated_by_id=1, + ) + draft_config.revision_id = revision_id + draft_config.save() + + draft_config_template = { + 'pages': {'draft_page1': {'value': 'test1'}, 'draft_page2': {'value': 'test2'}}, + 'courses': {'course1': {'chapters': {'c1': 'chapter1'}}, 'course2': {'chapters': {'c2': 'chapter2'}}}, + 'something': {'else': {'val': 'val'}}, + } + assert DraftConfig.objects.count() == 0, 'bad test data, DraftConfig should be empty before the test' + + _new_draft_config('pages', draft_config_template['pages'], 123) + _new_draft_config('courses', draft_config_template['courses'], 345) + _new_draft_config('something', draft_config_template['something'], 555) + _new_draft_config('pages.draft_page2', draft_config_template['pages']['draft_page2'], 567) + _new_draft_config('courses.course1.chapters', draft_config_template['courses']['course1']['chapters'], 789) + _new_draft_config('something.else', draft_config_template['something']['else'], 888) + + ConfigAccessControl.objects.create(key_name='pages_key', path='pages', key_type='dict') + ConfigAccessControl.objects.create(key_name='courses_key', path='courses', key_type='dict') + ConfigAccessControl.objects.create(key_name='something_key', path='something', key_type='dict') + + assert tenants.get_tenant_config( + 1, ['pages_key', 'courses_key'], published_only, + )['revision_ids'] == expected_revision_ids + + @pytest.mark.django_db def test_get_tenant_config_for_non_exist_tenant(): """Test the get_tenant_config for non exist tenant_id."""