From 78f0970d0f945730680fe03a3676b7e657e4f76b Mon Sep 17 00:00:00 2001 From: Catalin Date: Wed, 7 Aug 2024 13:08:22 +0300 Subject: [PATCH 1/2] HDX-10035 add in_hapi field --- .../ckanext/hdx_package/actions/authorize.py | 5 +++++ .../ckanext/hdx_package/actions/patch.py | 19 +++++++++++++++++++ .../ckanext/hdx_package/helpers/constants.py | 2 ++ .../ckanext/hdx_package/plugin.py | 16 ++++++++++++++-- .../ckanext/hdx_users/helpers/permissions.py | 4 ++++ 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/ckanext-hdx_package/ckanext/hdx_package/actions/authorize.py b/ckanext-hdx_package/ckanext/hdx_package/actions/authorize.py index 32c5c15431..7c12351ad5 100644 --- a/ckanext-hdx_package/ckanext/hdx_package/actions/authorize.py +++ b/ckanext-hdx_package/ckanext/hdx_package/actions/authorize.py @@ -2,6 +2,7 @@ import ckan.logic.auth.create as create import ckan.logic.auth.update as update import ckan.plugins.toolkit as tk +from ckan.types import Context, DataDict from ckanext.hdx_users.helpers.permissions import Permissions log = logging.getLogger(__name__) @@ -158,3 +159,7 @@ def hdx_send_mail_request_tags(context, data_dict): 'success': False, 'msg': _('Not authorized to perform this request') } + + +def hdx_mark_resource_in_hapi(context: Context, data_dict: DataDict): + return _check_hdx_user_permission(context, Permissions.PERMISSION_MANAGE_IN_HAPI_FLAG) diff --git a/ckanext-hdx_package/ckanext/hdx_package/actions/patch.py b/ckanext-hdx_package/ckanext/hdx_package/actions/patch.py index 6595acdeae..d36f6f27d4 100644 --- a/ckanext-hdx_package/ckanext/hdx_package/actions/patch.py +++ b/ckanext-hdx_package/ckanext/hdx_package/actions/patch.py @@ -3,6 +3,7 @@ import ckan.plugins.toolkit as tk import ckanext.hdx_package.helpers.resource_triggers.fs_check as fs_check +from ckan.types import Context, DataDict from ckanext.hdx_package.actions.update import process_skip_validation, process_batch_mode, package_update, \ SKIP_VALIDATION from ckanext.hdx_package.helpers.analytics import QAQuarantineAnalyticsSender @@ -363,3 +364,21 @@ def hdx_p_coded_resource_update(context, data_dict): return next((r for r in result['package']['resources'] if r['id'] == resource_id), None) +def hdx_mark_resource_in_hapi(context: Context, data_dict: DataDict): + """ + This action uses PERMISSIONS! Please be careful if changing the scope of its changes! + """ + _check_access('hdx_mark_resource_in_hapi', context, data_dict) + + if 'id' in data_dict and 'in_hapi' in data_dict: + new_data_dict = { + 'id': _get_or_bust(data_dict, 'id'), + 'in_hapi': _get_or_bust(data_dict, 'in_hapi') + } + else: + raise NotFound('Resource ID or key were not provided.') + + context['ignore_auth'] = True + context['allow_resource_in_hapi_field'] = True + + return _get_action('resource_patch')(context, new_data_dict) diff --git a/ckanext-hdx_package/ckanext/hdx_package/helpers/constants.py b/ckanext-hdx_package/ckanext/hdx_package/helpers/constants.py index 93f6fe9148..37cb6e93df 100644 --- a/ckanext-hdx_package/ckanext/hdx_package/helpers/constants.py +++ b/ckanext-hdx_package/ckanext/hdx_package/helpers/constants.py @@ -80,3 +80,5 @@ 'package_id': None, 'dataset_id': 'Dataset ID', 'resource_type': 'Resource Type', 'name': 'Resource Name', 'mimetype': 'MIME Type', 'size': 'Size'} + +IN_HAPI_FLAG_VALUES = [NO_DATA, 'yes'] diff --git a/ckanext-hdx_package/ckanext/hdx_package/plugin.py b/ckanext-hdx_package/ckanext/hdx_package/plugin.py index 69a93e2577..53f1063248 100644 --- a/ckanext-hdx_package/ckanext/hdx_package/plugin.py +++ b/ckanext-hdx_package/ckanext/hdx_package/plugin.py @@ -30,7 +30,7 @@ import ckanext.resourceproxy.plugin as resourceproxy_plugin from ckan.lib import uploader from ckan.common import c -from ckanext.hdx_package.helpers.constants import UNWANTED_DATASET_PROPERTIES, COD_VALUES_MAP +from ckanext.hdx_package.helpers.constants import UNWANTED_DATASET_PROPERTIES, COD_VALUES_MAP, IN_HAPI_FLAG_VALUES from ckanext.hdx_package.helpers.freshness_calculator import UPDATE_FREQ_INFO from ckanext.hdx_users.helpers.permissions import Permissions @@ -275,7 +275,13 @@ def _modify_package_schema(self, schema): tk.get_validator('hdx_delete_unless_authorized_to_update_p_coded'), tk.get_validator('ignore_missing'), # if None, don't save 'None' string tk.get_validator('boolean_validator'), - ] + ], + 'in_hapi': [ + tk.get_validator('hdx_keep_unless_allow_resource_in_hapi_field'), + tk.get_validator('ignore_missing'), + tk.get_validator('hdx_in_hapi_flag_values'), + tk.get_validator('hdx_delete_if_marked_with_no_data'), + ], } ) @@ -462,6 +468,7 @@ def get_actions(self): 'hdx_dataseries_link': hdx_patch.hdx_dataseries_link, 'hdx_dataseries_unlink': hdx_patch.hdx_dataseries_unlink, 'hdx_p_coded_resource_update': hdx_patch.hdx_p_coded_resource_update, + 'hdx_mark_resource_in_hapi': hdx_patch.hdx_mark_resource_in_hapi, } # IValidators @@ -520,6 +527,10 @@ def get_validators(self): 'hdx_tag_name_approved_validator': vd.hdx_tag_name_approved_validator, 'hdx_update_last_modified_if_url_changed': vd.hdx_update_last_modified_if_url_changed, 'hdx_disable_live_frequency_filestore_resources_only': vd.hdx_disable_live_frequency_filestore_resources_only, + 'hdx_in_hapi_flag_values': vd.hdx_value_in_list_wrapper(IN_HAPI_FLAG_VALUES, True), + 'hdx_keep_unless_allow_resource_in_hapi_field': + vd.hdx_package_keep_prev_value_unless_field_in_context_wrapper( + 'allow_resource_in_hapi_field', resource_level=True), } def get_auth_functions(self): @@ -539,6 +550,7 @@ def get_auth_functions(self): 'hdx_send_mail_request_tags': authorize.hdx_send_mail_request_tags, 'hdx_dataseries_update': authorize.hdx_dataseries_update, 'hdx_p_coded_resource_update': authorize.hdx_p_coded_resource_update, + 'hdx_mark_resource_in_hapi': authorize.hdx_mark_resource_in_hapi, } def make_middleware(self, app, config): diff --git a/ckanext-hdx_users/ckanext/hdx_users/helpers/permissions.py b/ckanext-hdx_users/ckanext/hdx_users/helpers/permissions.py index c71cbf13e9..fc6cf4e16e 100644 --- a/ckanext-hdx_users/ckanext/hdx_users/helpers/permissions.py +++ b/ckanext-hdx_users/ckanext/hdx_users/helpers/permissions.py @@ -28,6 +28,8 @@ class Permissions(object): LABEL_PERMISSION_MANAGE_QUICK_LINKS = 'Manage Quick Links' PERMISSION_MANAGE_QA = 'permission_manage_qa' # QA Complete and quarantine LABEL_PERMISSION_MANAGE_QA = 'Manage QA' + PERMISSION_MANAGE_IN_HAPI_FLAG = 'permission_manage_in_hapi_flag' + LABEL_PERMISSION_MANAGE_IN_HAPI_FLAG = 'Manage In HAPI Flag' # These are tasks that a bot needs to trigger: HDX daily stats, api token expiry emails. # Note that this permission shouldn't allow for any change to be done to HDX @@ -44,6 +46,7 @@ class Permissions(object): PERMISSION_VIEW_REQUEST_DATA, PERMISSION_MANAGE_QUICK_LINKS, PERMISSION_MANAGE_BASIC_SCHEDULED_TASKS, + PERMISSION_MANAGE_IN_HAPI_FLAG, ] ALL_PERMISSIONS_LABELS_DICT = { @@ -56,6 +59,7 @@ class Permissions(object): PERMISSION_MANAGE_QUICK_LINKS: LABEL_PERMISSION_MANAGE_QUICK_LINKS, PERMISSION_MANAGE_BASIC_SCHEDULED_TASKS: LABEL_PERMISSION_MANAGE_BASIC_SCHEDULED_TASKS, PERMISSION_MANAGE_QA: LABEL_PERMISSION_MANAGE_QA, + PERMISSION_MANAGE_IN_HAPI_FLAG: LABEL_PERMISSION_MANAGE_IN_HAPI_FLAG, } USER_EXTRA_FIELD = 'hdx_permissions' From 850b4bdf17a34d6d42c22b0b5f7a2cb5a8bb1e9f Mon Sep 17 00:00:00 2001 From: Catalin Date: Wed, 7 Aug 2024 19:17:05 +0300 Subject: [PATCH 2/2] HDX-10035 add tests for in_hapi field --- .../test_in_hapi.py | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 ckanext-hdx_package/ckanext/hdx_package/tests/test_dataset_special_fields/test_in_hapi.py diff --git a/ckanext-hdx_package/ckanext/hdx_package/tests/test_dataset_special_fields/test_in_hapi.py b/ckanext-hdx_package/ckanext/hdx_package/tests/test_dataset_special_fields/test_in_hapi.py new file mode 100644 index 0000000000..20bd2f9946 --- /dev/null +++ b/ckanext-hdx_package/ckanext/hdx_package/tests/test_dataset_special_fields/test_in_hapi.py @@ -0,0 +1,140 @@ +import pytest + +import ckan.tests.factories as factories +import ckan.model as model +import ckan.plugins.toolkit as tk + +import ckanext.hdx_theme.tests.hdx_test_base as hdx_test_base + +from ckanext.hdx_org_group.helpers.static_lists import ORGANIZATION_TYPE_LIST +from ckanext.hdx_users.helpers.permissions import Permissions + +config = tk.config +NotAuthorized = tk.NotAuthorized +ValidationError = tk.ValidationError + + +class TestResourceInHapiField(hdx_test_base.HdxBaseTest): + NORMAL_USER = 'hapi_user' + SYSADMIN_USER = 'testsysadmin' + PACKAGE_ID = 'test_dataset_1' + + PACKAGE = { + 'package_creator': 'test function', + 'private': False, + 'dataset_date': '01/01/1960-12/31/2012', + 'caveats': 'These are the caveats', + 'license_other': 'TEST OTHER LICENSE', + 'methodology': 'This is a test methodology', + 'dataset_source': 'Test data', + 'license_id': 'hdx-other', + 'name': PACKAGE_ID, + 'notes': 'This is a test dataset', + 'title': 'Test Dataset for HAPI', + 'owner_org': 'org_name_4_hapi', + 'groups': [{'name': 'roger'}], + 'resources': [ + { + 'package_id': 'test_private_dataset_1', + 'url': config.get('ckan.site_url', '') + '/storage/f/test_folder/hdx_test.csv', + 'resource_type': 'file.upload', + 'format': 'CSV', + 'name': 'hdx_test.csv' + } + ] + } + + @classmethod + def _get_action(cls, action_name): + return tk.get_action(action_name) + + @classmethod + def setup_class(cls): + super(TestResourceInHapiField, cls).setup_class() + + factories.User(name=cls.NORMAL_USER, email='hapi_user@hdx.hdxtest.org') + + factories.Organization( + name='org_name_4_hapi', + title='ORG NAME FOR HAPI', + users=[ + {'name': cls.NORMAL_USER, 'capacity': 'admin'}, + ], + hdx_org_type=ORGANIZATION_TYPE_LIST[0][1], + org_url='https://hdx.hdxtest.org/' + ) + + context = {'model': model, 'session': model.Session, 'user': cls.NORMAL_USER} + dataset_dict = cls._get_action('package_create')(context, cls.PACKAGE) + cls.RESOURCE_UPLOAD_ID = dataset_dict['resources'][0]['id'] + + def test_sysadmin_can_set_in_hapi_flag(self): + context_sysadmin = {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER} + + self._hdx_mark_resource_in_hapi(self.RESOURCE_UPLOAD_ID, 'in_hapi', 'yes', self.SYSADMIN_USER) + package_sysadmin_dict = self._get_action('package_show')(context_sysadmin, {'id': self.PACKAGE_ID}) + + assert package_sysadmin_dict['resources'][0]['in_hapi'] == 'yes', 'sysadmins should be able to set in_hapi flag' + + def test_sysadmin_can_unset_in_hapi_flag(self): + context_sysadmin = {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER} + + self._hdx_mark_resource_in_hapi(self.RESOURCE_UPLOAD_ID, 'in_hapi', 'no-data', self.SYSADMIN_USER) + package_sysadmin_dict = self._get_action('package_show')(context_sysadmin, {'id': self.PACKAGE_ID}) + + assert 'in_hapi' not in package_sysadmin_dict['resources'][0], 'sysadmins should be able to unset in_hapi flag' + + def test_sysadmin_cannot_set_invalid_in_hapi_flag(self): + context_sysadmin = {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER} + + try: + self._hdx_mark_resource_in_hapi(self.RESOURCE_UPLOAD_ID, 'in_hapi', 'invalid-value', self.SYSADMIN_USER) + except ValidationError as e: + assert 'in_hapi' in e.error_dict, 'hdx_mark_resource_in_hapi should fail when using invalid values' + + package_sysadmin_dict = self._get_action('package_show')(context_sysadmin, {'id': self.PACKAGE_ID}) + assert 'in_hapi' not in package_sysadmin_dict['resources'][0], 'in_hapi flag should not be set on invalid value' + + def test_normal_user_cannot_set_in_hapi_flag(self): + context = {'model': model, 'session': model.Session, 'user': self.NORMAL_USER} + + try: + self._hdx_mark_resource_in_hapi(self.RESOURCE_UPLOAD_ID, 'in_hapi', 'yes', self.NORMAL_USER) + except NotAuthorized: + assert True + + package_user_dict = self._get_action('package_show')(context, {'id': self.PACKAGE_ID}) + assert 'in_hapi' not in package_user_dict['resources'][0], 'normal users should not be able to set in_hapi flag' + + def test_normal_user_with_permissions_can_set_in_hapi_flag(self): + context = {'model': model, 'session': model.Session, 'user': self.NORMAL_USER} + + Permissions(self.NORMAL_USER).set_permissions( + {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER}, + [Permissions.PERMISSION_MANAGE_IN_HAPI_FLAG] + ) + + self._hdx_mark_resource_in_hapi(self.RESOURCE_UPLOAD_ID, 'in_hapi', 'yes', self.NORMAL_USER) + + package_user_dict = self._get_action('package_show')(context, {'id': self.PACKAGE_ID}) + assert package_user_dict['resources'][0][ + 'in_hapi'] == 'yes', 'normal users with the right permission should be able to set in_hapi flag' + + def test_in_hapi_flag_persists_on_package_update(self): + context_sysadmin = {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER} + + package_sysadmin_dict = self._get_action('package_show')(context_sysadmin, {'id': self.PACKAGE_ID}) + assert package_sysadmin_dict['resources'][0]['in_hapi'] == 'yes' + + dataset_dict_modified = self.PACKAGE + dataset_dict_modified['notes'] = 'This is a modified dataset' + self._get_action('package_update')(context_sysadmin, dataset_dict_modified) + + assert package_sysadmin_dict['resources'][0][ + 'in_hapi'] == 'yes', 'in_hapi flag should persist after package_update' + + def _hdx_mark_resource_in_hapi(self, resource_id, key, new_value, username): + context = {'model': model, 'session': model.Session, 'user': username} + data_dict = {'id': resource_id, key: new_value} + + self._get_action('hdx_mark_resource_in_hapi')(context, data_dict)