Merge pull request #6427 from OCHA-DAP/feature/HDX-10056-allow-crisis…

…-tagging-for-sysadmins

HDX-10056 allow only sysadmins to use existing "crisis-" tags

ckanext-hdx_package/ckanext/hdx_package/helpers/custom_validator.py

@@ -748,10 +748,12 @@ def hdx_tag_name_approved_validator(key, data, errors, context):
     approved_tags = get_action('cached_approved_tags_list')(context, {})
 
     if tag_name not in approved_tags:
-        # Allow sysadmins to add tags starting with "crisis-"
-        if not (allowed_to_add_crisis_tags and tag_name.startswith('crisis-') and tag_name != 'crisis-'):
-            approved_tags_url = 'https://data.humdata.org/rdr/spreadsheets/approved-tags'
-            errors[key].append("Tag name '{}' is not in the approved list of tags. Check the list at: {}".format(tag_name, approved_tags_url))
+        approved_tags_url = 'https://data.humdata.org/rdr/spreadsheets/approved-tags'
+        errors[key].append("Tag name '{}' is not in the approved list of tags. Check the list at: {}".format(tag_name, approved_tags_url))
+    # Only sysadmins are allowed to use tags starting with "crisis-"
+    if tag_name.startswith('crisis-') and not allowed_to_add_crisis_tags:
+        errors[key].append("Tag name '{}' can only be added by sysadmins".format(tag_name))
+
 
 def hdx_update_last_modified_if_url_changed(key: FlattenKey, data: FlattenDataDict,
                         errors: FlattenErrorDict, context: Context) -> Any:

ckanext-hdx_package/ckanext/hdx_package/helpers/helpers.py

@@ -21,6 +21,7 @@
 import ckan.model.package as package
 import ckan.plugins.toolkit as tk
 from ckan.common import _, c, request
+from ckan.types import Context, DataDict
 
 log = logging.getLogger(__name__)
 
@@ -197,16 +198,14 @@ def hdx_find_license_name(license_id, license_name):
 #     return extra_vars
 
 
-def hdx_tag_autocomplete_list(context, data_dict):
-    '''Return a list of tag names that contain a given string.
+def hdx_tag_autocomplete_list(context: Context, data_dict: DataDict):
+    """Return a list of tag names that contain a given string.
 
-    By default only free tags (tags that don't belong to any vocabulary) are
+    By default, only free tags (tags that don't belong to any vocabulary) are
     searched. If the ``vocabulary_id`` argument is given then only tags
     belonging to that vocabulary will be searched instead.
 
     This was changed to return a list of approved tag names that contain a given string.
-    This searches for tags in the approved list that contain the provided query string. If the query string starts
-    with "crisis-" and the user is a sysadmin, the query itself can be added as a new tag.
 
     :param query: the string to search for
     :type query: string
@@ -223,16 +222,22 @@ def hdx_tag_autocomplete_list(context, data_dict):
 
     :rtype: list of strings
 
-    '''
+    """
     _check_access('tag_autocomplete', context, data_dict)
 
     approved_tags = get_action('cached_approved_tags_list')(context, {})
     query = data_dict.get('q', '').lower()
-    matching_tags = [tag for tag in approved_tags if query in tag.lower()]
 
-    # Allow sysadmins to add tags starting with "crisis-"
-    if new_authz.is_sysadmin(c.user) and query.startswith('crisis-') and query != 'crisis-':
-        matching_tags.append(query)
+    is_sysadmin = new_authz.is_sysadmin(c.user)
+
+    matching_tags = []
+
+    for tag in approved_tags:
+        if query in tag.lower():
+            # Only sysadmins are allowed to use tags starting with "crisis-"
+            if tag.startswith('crisis-') and not is_sysadmin:
+                continue
+            matching_tags.append(tag)
 
     return matching_tags
 

ckanext-hdx_package/ckanext/hdx_package/tests/test_actions/test_package_create.py

@@ -189,6 +189,7 @@ def test_create_valid_tags(self):
         assert 'boys' in [tag['name'] for tag in pkg.get('tags')]
 
     def test_create_valid_crisis_tags(self):
+        crisis_tag_name = 'crisis-opt-israel-hostilities'
         package = {
             "package_creator": "test function",
             "private": False,
@@ -203,7 +204,7 @@ def test_create_valid_crisis_tags(self):
             "owner_org": "hdx-test-org",
             'name': 'test_activity_6',
             'title': 'Test Activity 6',
-            'tags': [{'name': 'crisis-ebola'}, {'name': 'crisis-response'}]
+            'tags': [{'name': crisis_tag_name}]
         }
 
         context = {'model': model, 'session': model.Session, 'user': 'testsysadmin'}
@@ -212,9 +213,8 @@ def test_create_valid_crisis_tags(self):
 
         pkg = self._get_action('package_show')(context, {'id': package['name']})
 
-        assert len(pkg.get('tags')) == 2
-        assert 'crisis-ebola' in [tag['name'] for tag in pkg.get('tags')]
-        assert 'crisis-response' in [tag['name'] for tag in pkg.get('tags')]
+        assert len(pkg.get('tags')) == 1
+        assert crisis_tag_name in [tag['name'] for tag in pkg.get('tags')]
 
     def test_create_invalid_tags(self):
         package = {

ckanext-hdx_package/ckanext/hdx_package/tests/test_actions/test_package_update.py

@@ -399,27 +399,29 @@ def test_hdx_package_tags_validation(self):
         assert len(modified_package.get('tags')) == 1
         assert 'children' in [tag['name'] for tag in modified_package.get('tags')]
 
+        crisis_tag_name = 'crisis-opt-israel-hostilities'
         data_dict = self._modify_field(context, testsysadmin, package['name'], 'tags',
-                                       [{'name': 'crisis-ebola'}, {'name': 'children'}])
+                                       [{'name': crisis_tag_name}, {'name': 'children'}])
         modified_package = data_dict.get('modified_package')
 
         assert len(modified_package.get('tags')) == 2
-        assert 'crisis-ebola' in [tag['name'] for tag in modified_package.get('tags')]
+        assert crisis_tag_name in [tag['name'] for tag in modified_package.get('tags')]
         assert 'children' in [tag['name'] for tag in modified_package.get('tags')]
 
         try:
-            self._modify_field(context, testsysadmin, package['name'], 'tags', [{'name': 'invalid_tag1'}, {'name': 'invalid_tag2'}])
+            self._modify_field(context, testsysadmin, package['name'], 'tags',
+                               [{'name': 'invalid_tag1'}, {'name': 'invalid_tag2'}])
         except ValidationError as e:
             assert 'tags' in e.error_dict, 'package_update should fail when using invalid tags'
             assert len(e.error_dict.get('tags')) == 2, 'There should be two invalid tags'
             assert "Tag name 'invalid_tag1' is not in the approved list of tags" in e.error_dict.get('tags')[0]
 
         try:
-            self._modify_field(context_user, joeadmin, package['name'], 'tags', [{'name': 'crisis-food'}])
+            self._modify_field(context_user, joeadmin, package['name'], 'tags', [{'name': crisis_tag_name}])
         except ValidationError as e:
             assert 'tags' in e.error_dict, 'Only sysadmins are allowed to add tags starting with "crisis-"'
-            assert "Tag name 'crisis-food' is not in the approved list of tags" in e.error_dict.get('tags')[0], \
-                'Only sysadmins are allowed to add tags starting with "crisis-"'
+            assert "Tag name '{}' can only be added by sysadmins".format(crisis_tag_name) in e.error_dict.get('tags')[
+                0], 'Only sysadmins are allowed to add tags starting with "crisis-"'
 
 
     def _modify_field(self, context, user, package_id, key, value):

ckanext-hdx_theme/ckanext/hdx_theme/helpers/helpers.py

@@ -942,13 +942,22 @@ def hdx_dataset_is_p_coded(resource_list):
     return False
 
 
-def hdx_get_approved_tags_list():
+def hdx_get_allowed_tags_list():
     context = {'model': model, 'session': model.Session, 'user': c.user}
 
     approved_tags = logic.get_action('cached_approved_tags_list')(context, {})
-    approved_tags_dict_list = [{'value': tag, 'text': tag} for tag in approved_tags]
 
-    return approved_tags_dict_list
+    is_sysadmin = new_authz.is_sysadmin(c.user)
+    allowed_tags = []
+    for tag in approved_tags:
+        # Only sysadmins are allowed to use tags starting with "crisis-"
+        if tag.startswith('crisis-') and not is_sysadmin:
+            continue
+        allowed_tags.append(tag)
+
+    allowed_tags_dict_list = [{'value': tag, 'text': tag} for tag in allowed_tags]
+
+    return allowed_tags_dict_list
 
 
 def bs5_build_nav_icon(menu_item, title, **kw):

ckanext-hdx_theme/ckanext/hdx_theme/plugin.py

@@ -256,7 +256,7 @@ def get_helpers(self):
             'hdx_get_request_param': hdx_helpers.hdx_get_request_param,
             'hdx_pending_request_data': hdx_helpers.hdx_pending_request_data,
             'hdx_dataset_is_p_coded': hdx_helpers.hdx_dataset_is_p_coded,
-            'hdx_get_approved_tags_list': hdx_helpers.hdx_get_approved_tags_list,
+            'hdx_get_allowed_tags_list': hdx_helpers.hdx_get_allowed_tags_list,
             'are_new_p_code_filters_enabled': hdx_helpers.are_new_p_code_filters_enabled,
             'bs5_build_nav_icon': hdx_helpers.bs5_build_nav_icon,
             'hdx_decode_markup': hdx_helpers.hdx_decode_markup,

ckanext-hdx_theme/ckanext/hdx_theme/templates/contribute_flow/create_edit.html

@@ -695,21 +695,10 @@ <h3>Metadata</h3>
                                             {{ form.markdown('caveats', id='field_caveats', label=_('Caveats / Comments'),value=data.caveats, error=errors.caveats, classes=['mb-3'], attrs={'style': 'height: 80px;'}) }}
                                             </div>
 
-                                            {# replaced with ajax one below
-                                                {% set tags = h.hdx_tag_list() %}
-                                                {{ form.select('tag_string', id='field_tags', label=_('Tags'), options=tags, selected=data.tag_string, error=errors.tag, classes=['form-group'], attrs={'class':'form-control choices-orange', 'multiple':'true'}) }}
-                                            #}
-
-                                            {% set tag_input_attrs = {'data-module': 'hdx_autocomplete',
-                                              'data-module-tags': 'false',
-                                              'data-module-multiple': 'multiple',
-                                              'data-module-width': '500px',
-                                              'class':'choices-orange', 'multiple':'true',
-                                              'data-module-source': '/api/2/util/tag/autocomplete?incomplete=?'} %}
-                                            {% set approved_tags = h.hdx_get_approved_tags_list() %}
+                                            {% set allowed_tags = h.hdx_get_allowed_tags_list() %}
                                             {% set selected_tags = data.tag_string.split(', ') if data.tag_string else '' %}
                                             <div data-module="hdx_form_element_manager" data-module-element_name="tag_string" data-module-broadcast_change="true" data-module-required="requestdata">
-                                              {{ form.input('tag_string', id='field_tag_string', label=_('Tags'), value=data.tag_string, error=errors.tag_string, classes=['', 'mb-3', 'tags-select'], attrs=tag_input_attrs) }}
+                                              {{ form.select('tag_string', id='field_tag_string', label=_('Tags'), options=allowed_tags, selected=selected_tags, error=errors.tag_string, classes=['mb-3', 'tags-select'], attrs={'class':'choices-orange', 'multiple':'true' }) }}
                                               <div data-module="hdx_tag_recommender" data-module-tag_input_selector="#field_tag_string"
                                                    data-module-recommended_tag_wrapper_selector="#recommended-tags-wrapper" class="mb-3">
                                                 <div id="recommended-tags-wrapper" class="recommended-tags-wrapper" style="display: none;">