Skip to content

feat: support config sub-keys #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
shadinaif wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: support config sub-keys #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions futurex_openedx_extensions/dashboard/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand All @@ -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:
Expand Down
33 changes: 27 additions & 6 deletions futurex_openedx_extensions/helpers/tenants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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):
Expand All @@ -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)

Expand Down
64 changes: 43 additions & 21 deletions tests/test_dashboard/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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')
Expand All @@ -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,
Expand Down
43 changes: 43 additions & 0 deletions tests/test_helpers/test_tenants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down