feat: support config sub-keys

Author: shadinaif

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:

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)
 

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,

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."""