chore (8/8): CKAN 2.11 post-migration cleanup & regression fixes

ckanext/unaids/assets/custom.css

@@ -384,13 +384,19 @@ a.dropdown-item{
   margin-top: 0;
 }
 
-.masthead .nav > li > a,
-.masthead .nav > li > a:focus,
-.masthead .nav > li > a:hover,
-.masthead .nav > .active > a,
-.masthead .nav > .active > a:hover,
-.masthead .nav > .active > a:focus {
+/* Masthead nav, matching CKAN 2.9 production: dark-grey links, with a light-grey
+   background and brand-red text when active or hovered. Targets the BS5 markup
+   (`.main-navbar ul li a`, not BS3 `.nav > li > a`) and overrides vanilla's
+   white text / dark-teal (#003647) active background. */
+.masthead .main-navbar ul li a {
+  color: #666;
+}
+
+.masthead .main-navbar ul li:hover a,
+.masthead .main-navbar ul li:focus a,
+.masthead .main-navbar ul li.active a {
   color: #e31837;
+  background-color: #e6e6e6;
 }
 
 .col-xs-1, .col-sm-1, .col-md-1, .col-lg-1, .col-xs-2, .col-sm-2, .col-md-2, .col-lg-2, .col-xs-3, .col-sm-3, .col-md-3, .col-lg-3, .col-xs-4, .col-sm-4, .col-md-4, .col-lg-4, .col-xs-5, .col-sm-5, .col-md-5, .col-lg-5, .col-xs-6, .col-sm-6, .col-md-6, .col-lg-6, .col-xs-7, .col-sm-7, .col-md-7, .col-lg-7, .col-xs-8, .col-sm-8, .col-md-8, .col-lg-8, .col-xs-9, .col-sm-9, .col-md-9, .col-lg-9, .col-xs-10, .col-sm-10, .col-md-10, .col-lg-10, .col-xs-11, .col-sm-11, .col-md-11, .col-lg-11, .col-xs-12, .col-sm-12, .col-md-12, .col-lg-12 {
@@ -940,3 +946,11 @@ div.ckanext-datapreview{
   border: 1px solid #e31837;
   color: #fff;
 }
+
+/* Bootstrap 5 removed the `.hidden` utility that Bootstrap 3 provided.
+   Our scheming schemas use `"classes": ["hidden"]` on autogenerated/fixed
+   fields (title, url/name, tags, license) to keep them out of the form.
+   Restore the utility so those fields stay hidden as in the BS3 (CKAN 2.9) UI. */
+.hidden {
+  display: none !important;
+}

ckanext/unaids/auth_logic.py

@@ -26,15 +26,6 @@ class OAuth2AuthorizationError(ActionError):
     pass
 
 
-def json_response_omitting_challenge_decider(environ, status, headers):
-    h_dict = dict(headers)
-    ct = h_dict.get('Content-Type')
-    if ct is not None:
-        return not ct.startswith('text/json') and status.startswith('401 ')
-
-    return status.startswith('401 ')
-
-
 # based on work from https://auth0.com/docs/quickstart/backend/python/01-authorization
 def access_token_present_and_valid_and_user_authorized():
     token = request.headers.get('Authorization', '')
@@ -87,7 +78,6 @@ def validate_and_decode_token(encoded):
         except Exception:
             raise OAuth2AuthenticationError(message="Unable to parse authentication token")
 
-        g.current_user = payload
         return payload
     raise OAuth2AuthenticationError(message="Unable to find appropriate key")
 

ckanext/unaids/blueprints/svg_map_options.py

@@ -2,7 +2,6 @@
 from collections import defaultdict
 
 import pycountry
-import six
 from flask import Blueprint, jsonify
 
 from ckan.plugins import toolkit
@@ -27,7 +26,7 @@ def map_options():
     dataset_search = toolkit.get_action("package_search")({}, {"rows": 0, "facet.field": ["geo-location"]})
     location_facet = dataset_search['facets']['geo-location']
     values = defaultdict(dataset_count)
-    for geo_location, count in six.iteritems(location_facet):
+    for geo_location, count in location_facet.items():
         if geo_location:
             try:
                 country_code = _country_code_from_location_name(geo_location)

ckanext/unaids/blueprints/user_info_blueprint.py

@@ -3,7 +3,6 @@
 from ckan.common import g, _
 from ckan.lib import helpers as h
 from ckan.logic import get_action
-from six import ensure_str
 import dominate.tags as dom_tags
 from flask import Blueprint
 
@@ -32,7 +31,7 @@ def display_user_details(locale=None):
             u'type': u'button',
             u'class': u'btn btn-default btn-xs',
             u'data-module': u'copy-into-buffer',
-            u'data-module-copy-value': ensure_str(apikey)
+            u'data-module-copy-value': str(apikey)
         })
         h.flash_success(
             _('Your api key is: '

ckanext/unaids/blueprints/validate_user_affiliation.py

@@ -34,18 +34,18 @@ def check_user_affiliation():
     except (toolkit.ValidationError, AttributeError):
         return h.redirect_to(helpers.get_profile_editor_url(
             after_save_url=h.url_for("validate_user_affiliation.success_callback", _external=True),
-            back_url=h.url_for('dashboard.index', _external=True),
+            back_url=h.url_for('activity.dashboard', _external=True),
             flash_message=_("UNAIDS asks that further information be added "
                             "to your user profile. Please complete the required "
                             "fields missing below before continuing...")
         ))
 
-    return h.redirect_to('dashboard.index')
+    return h.redirect_to('activity.dashboard')
 
 
 @validate_user_affiliation.route('/success-callback/')
 def success_callback():
     h.flash_success(
         _("Thank you for updating your profile information.")
     )
-    return h.redirect_to('dashboard.index')
+    return h.redirect_to('activity.dashboard')

ckanext/unaids/helpers.py

@@ -11,11 +11,6 @@
 from ckan.lib.helpers import build_nav_main as core_build_nav_main
 
 
-try:
-    from html import escape as html_escape
-except ImportError:
-    from cgi import escape as html_escape
-
 from urllib.parse import quote, urlencode
 
 log = logging.getLogger()
@@ -167,8 +162,10 @@ def get_language_code():
 
 def build_pages_nav_main(*args):
     """
-    This helper is overriding build_pages_nav_main from ckanext-pages
-    to add localization to pages urls and titles
+    This helper is overriding build_pages_nav_main from ckanext-pages.
+    Unlike the original, this version does NOT add pages to the top navbar.
+    Pages are accessible via direct links, the homepage, and the footer.
+    The "2026 Instructions" link is added separately in header.html template.
     """
     about_menu = toolkit.asbool(toolkit.config.get('ckanext.pages.about_menu', True))
     group_menu = toolkit.asbool(toolkit.config.get('ckanext.pages.group_menu', True))
@@ -191,25 +188,12 @@ def build_pages_nav_main(*args):
 
     output = core_build_nav_main(*new_args)
 
-    # do not display any private pages in menu even for sysadmins
-    pages_list = toolkit.get_action('ckanext_pages_list')(None, {'order': True, 'private': False})
-
-    page_name = ''
-    is_current_page = toolkit.get_endpoint() in (('pages', 'show'), ('pages', 'blog_show'))
-
-    if is_current_page:
-        page_name = toolkit.request.path.split('/')[-1]
-
-    for page in pages_list:
-        type_ = 'blog' if page['page_type'] == 'blog' else 'pages'
-        name = quote(page['name'])
-        title = html_escape(_(page['title']))
-        link = toolkit.h.literal(u'<a href="/{}/{}/{}">{}</a>'.format(toolkit.h.lang(), type_, name, title))
-        if page['name'] == page_name:
-            li = toolkit.literal('<li class="active">') + link + toolkit.literal('</li>')
-        else:
-            li = toolkit.literal('<li>') + link + toolkit.literal('</li>')
-        output = output + li
+    # NOTE: We intentionally do NOT add pages to the navbar here.
+    # Pages clutter the top navigation and are better accessed via:
+    # - The homepage help panel (Instructions buttons)
+    # - The footer (About, Terms, Cookie Policy links)
+    # - Direct URLs
+    # The "2026 Instructions" link is added in header.html template block.
 
     return output
 
@@ -280,7 +264,7 @@ def unaids_get_validation_badge(resource, in_listing=False):
 
     link_visibility = ""
     if status in ['success', 'unknown']:
-        link_visibility = 'hidden'
+        link_visibility = 'd-none'
 
     badge_html = '''
 <a href="{validation_url}" {tags} class="validation-badge">

ckanext/unaids/logic.py

@@ -5,20 +5,19 @@
 
 
 def validate_resource_upload_fields(context, resource_dict):
-    upload_has_sha256 = "sha256" in resource_dict
-    upload_has_lfs_prefix = "lfs_prefix" in resource_dict
-    upload_has_size = "size" in resource_dict
-    valid_blob_storage_upload = (upload_has_sha256 == upload_has_lfs_prefix == upload_has_size)
-    no_upload_fields_present = not any([
-        resource_dict.get("sha256", ""),
-        resource_dict.get("lfs_prefix", ""),
-        resource_dict.get("size", "")
-    ])
-
-    if not valid_blob_storage_upload:
+    # sha256 and lfs_prefix are the giftless blob-storage upload fields. `size`
+    # is a standard CKAN field present on any stored resource, so it must NOT
+    # gate the "is this a blob-storage upload" check. Otherwise re-saves that
+    # carry `size` but not the giftless fields (e.g. ckanext-validation patching
+    # validation_status) wrongly fail with "Invalid blob storage upload fields",
+    # which silently blocks validation on the resource-create path.
+    upload_has_sha256 = bool(resource_dict.get("sha256"))
+    upload_has_lfs_prefix = bool(resource_dict.get("lfs_prefix"))
+
+    if upload_has_sha256 != upload_has_lfs_prefix:
         raise toolkit.ValidationError(
-            ["Invalid blob storage upload fields. sha256, size and lfs_prefix needs to be provided."])
-    elif no_upload_fields_present:
+            ["Invalid blob storage upload fields. sha256 and lfs_prefix needs to be provided."])
+    elif not upload_has_sha256:
         return None
     else:
         sha256 = resource_dict.get("sha256")

ckanext/unaids/plugin.py

@@ -185,24 +185,51 @@ def can_validate(self, context, data_dict):
     # IPackageController
     resources_to_validate_package = {}
 
-    def after_create(self, context, pkg_dict):
-        if pkg_dict.get("validate_package"):
+    def after_resource_create(self, context, resource):
+        # NOTE: this hook was named `after_create` for CKAN 2.9; CKAN 2.11
+        # renamed IResourceController.after_create -> after_resource_create, so
+        # the old name was never called.
+        if resource.get("validate_package"):
             toolkit.get_action("resource_validation_run_batch")(
-                context, {"dataset_ids": pkg_dict["package_id"]}
+                context, {"dataset_ids": resource["package_id"]}
             )
+            return
 
-    def after_update(self, context, data_dict):
-        if "extras" in data_dict:
-            org_to_allow_transfer_to = [
-                item["value"]
-                for item in data_dict["extras"]
-                if item["key"] == "org_to_allow_transfer_to" and item["value"]
-            ]
-            if org_to_allow_transfer_to:
-                send_dataset_transfer_emails(
-                    dataset_id=data_dict["id"],
-                    recipient_org_id=org_to_allow_transfer_to[0],
-                )
+        # ckanext-validation skips validation on the resource_create path because
+        # `schema` and `format` are applied as scheming defaults (from
+        # resource_type) only AFTER its can_validate gate runs — so bulk-uploaded
+        # core resources never get validated. By the time this hook fires the
+        # resource is stored with its schema, so trigger validation here.
+        supported_formats = toolkit.config.get(
+            "ckanext.validation.formats", "csv xlsx xls").split()
+        if (
+            resource.get("schema")
+            and (resource.get("url") or resource.get("url_type") == "upload")
+            and (resource.get("format") or "").lower() in supported_formats
+        ):
+            toolkit.get_action("resource_validation_run")(
+                context, {"resource_id": resource["id"], "async": True}
+            )
+
+    def after_dataset_update(self, context, data_dict):
+        # NOTE: this hook was named `after_update` for CKAN 2.9; CKAN 2.11
+        # renamed IPackageController.after_update -> after_dataset_update, so the
+        # old name was never called (dataset-transfer emails silently broke).
+        # org_to_allow_transfer_to is a scheming field, so it comes through
+        # top-level; fall back to the legacy `extras` representation just in case.
+        recipient_org_id = data_dict.get("org_to_allow_transfer_to")
+        if not recipient_org_id:
+            recipient_org_id = next(
+                (item["value"]
+                 for item in data_dict.get("extras", [])
+                 if item.get("key") == "org_to_allow_transfer_to" and item.get("value")),
+                None,
+            )
+        if recipient_org_id:
+            send_dataset_transfer_emails(
+                dataset_id=data_dict["id"],
+                recipient_org_id=recipient_org_id,
+            )
 
     def _process_schema_fields(self, data_dict):
         """

ckanext/unaids/tests/test_auth_logic.py

@@ -10,18 +10,6 @@
 from ckan.tests.helpers import call_action
 
 
-@pytest.mark.parametrize("status,headers,expected ", [
-    ("401 ", {}, True),
-    ("401 ", {"Content-Type": "text/html"}, True),
-    ("401 ", {"Content-Type": "text/json"}, False),
-    ("404 ", {}, False),
-    ("404 ", {"Content-Type": "text/html"}, False),
-    ("404 ", {"Content-Type": "text/json"}, False)
-])
-def test_json_response_omitting_challenge_decider(status, headers, expected):
-    assert auth_logic.json_response_omitting_challenge_decider(None, status, headers) == expected
-
-
 class TestExtractToken(object):
     def test_should_pass_when_right_token(self):
         assert auth_logic.extract_token("Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6I") == "eyJhbGciOiJSUzI1NiIsInR5cCI6I"

ckanext/unaids/tests/test_dataset_transfer.py

@@ -1,4 +1,5 @@
 # encoding: utf-8
+import ckan.model as model
 from ckan.tests import helpers
 from ckan.lib.helpers import url_for
 from ckan.tests import factories
@@ -168,19 +169,25 @@ def test_send_dataset_transfer_email_subject(self, mocked_mail_recipient, app):
         assert dataset_name in email_subject
 
     def test_send_dataset_transfer_emails_errors(self, app):
-        user_1, user_2 = [
-            factories.User()
-            for x in range(2)
-        ]
-        org_1, org_2 = [
-            factories.Organization(user=user)
-            for user in [user_1, user_2]
-        ]
+        # When the recipient org has no admin with an email address there are no
+        # eligible recipients, so no transfer emails should be sent.
+        user_1 = factories.User(email='user_1@example.com')
+        user_2 = factories.User(email='user_2@example.com')
+        org_1 = factories.Organization(
+            user={'name': user_1['name'], 'capacity': 'admin'})
+        org_2 = factories.Organization(
+            user={'name': user_2['name'], 'capacity': 'admin'})
         dataset = factories.Dataset(
             owner_org=org_1['id'],
             type='test-schema',
             org_to_allow_transfer_to=org_2['id']
         )
+        # Remove the email from the recipient org's only admin: it should no
+        # longer be considered an eligible recipient (get_org_admins_with_email
+        # _addresses filters on email is not None).
+        admin = model.User.get(user_2['id'])
+        admin.email = None
+        model.Session.commit()
         with app.flask_app.test_request_context():
             emails_succeeded = send_dataset_transfer_emails(
                 dataset_id=dataset['id'],
@@ -237,3 +244,35 @@ def test_dataset_transfer_request(self, app):
         )
         assert result['owner_org'] == org_2['id']
         assert 'org_to_allow_transfer_to' not in result
+
+    @mock.patch('ckanext.unaids.plugin.send_dataset_transfer_emails')
+    def test_after_dataset_update_sends_transfer_email(self, mock_send, app):
+        # Setting org_to_allow_transfer_to on a dataset update must fire the
+        # after_dataset_update hook, which notifies the target organisation.
+        # (Regression guard: CKAN 2.11 renamed after_update -> after_dataset_update,
+        # which silently killed this email.)
+        user_1, user_2 = [
+            factories.User(email="user_{}_@example.com".format(x)) for x in range(2)
+        ]
+        org_1, org_2 = [
+            factories.Organization(
+                users=[{'name': user['name'], 'capacity': 'admin'}]
+            )
+            for user in [user_1, user_2]
+        ]
+        dataset = factories.Dataset(owner_org=org_1['id'], type='test-schema')
+
+        # Creating the dataset (no transfer target) must not send a transfer email
+        mock_send.assert_not_called()
+
+        helpers.call_action(
+            'package_patch',
+            context={'user': user_1['name']},
+            id=dataset['id'],
+            org_to_allow_transfer_to=org_2['id'],
+        )
+
+        mock_send.assert_called_once_with(
+            dataset_id=dataset['id'],
+            recipient_org_id=org_2['id'],
+        )