diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c0c6d21..94389bd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,18 +1,17 @@ name: Tests -on: [push, pull_request] +on: [pull_request] jobs: test: + name: CKAN 2.11 - Focus on failing tests runs-on: ubuntu-latest container: - # The CKAN version tag of the Solr and Postgres containers should match - # the one of the container the tests run on. - # You can switch this base image with a custom image tailored to your project - image: openknowledge/ckan-dev:2.9 + image: ckan/ckan-dev:2.11-py3.10 + options: --user root services: solr: - image: ckan/ckan-solr-dev:2.9 + image: ckan/ckan-solr:2.11-solr9 postgres: - image: ckan/ckan-postgres-dev:2.9 + image: ckan/ckan-postgres-dev:2.11 env: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -29,7 +28,7 @@ jobs: CKAN_REDIS_URL: redis://redis:6379/1 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v6 - name: Install requirements # Install any extra requirements your extension has here (dev requirements, other extensions etc) run: | @@ -43,6 +42,9 @@ jobs: sed -i -e 's/use = config:.*/use = config:\/srv\/app\/src\/ckan\/test-core.ini/' test.ini ckan -c test.ini db init + ckan -c test.ini db upgrade - name: Run tests - run: pytest --ckan-ini=test.ini --cov=ckanext.fork --disable-warnings ckanext/fork + run: | + # Run ALL tests + pytest --ckan-ini=test.ini --disable-warnings ckanext/fork/tests/ diff --git a/ckanext/fork/actions.py b/ckanext/fork/actions.py index 6758af8..721a22a 100644 --- a/ckanext/fork/actions.py +++ b/ckanext/fork/actions.py @@ -63,11 +63,19 @@ def resource_autocomplete(context, data_dict): datasets = _get_dataset_from_resource_uuid(context, q_lower) if not datasets: - datasets = toolkit.get_action('package_search')(context, { - "q": q, + # CKAN's Solr schema indexes `res_name` but NOT `res_id`, so we cannot + # filter by resource UUID substring via package_search. We fetch datasets + # and match resource names/IDs in Python. Capped at rows=10: instances + # with more datasets won't match resource IDs beyond the top 10. + search_results = toolkit.get_action('package_search')(context, { + "q": "*:*", "rows": 10, "include_private": True - })['results'] + }) + datasets = search_results['results'] + + # Split query into tokens for dataset-level matching (allows partial matches like "01") + query_tokens = q_lower.split() for dataset in datasets: @@ -75,10 +83,28 @@ def resource_autocomplete(context, data_dict): continue resources = [] + has_matching_resource = False for resource in dataset['resources']: last_modified = toolkit.h.time_ago_from_timestamp(resource['last_modified']) - match = q_lower in resource['name'].lower() or q_lower == resource['id'] + # For resources: match based on number of matching tokens + # - Single token queries: require exact match (full string) + # - Multi-token queries: require at least 2 tokens to match + resource_lower = resource['name'].lower() + resource_id_lower = resource['id'].lower() + + if len(query_tokens) == 1: + # Single token: use full string matching + match = q_lower in resource_lower or q_lower in resource_id_lower + else: + # Multi-token: require at least 2 tokens to match + match = sum( + 1 for token in query_tokens + if token in resource_lower or token in resource_id_lower + ) >= 2 + + if match: + has_matching_resource = True resources.append({ 'id': resource['id'], 'name': resource['name'], @@ -89,15 +115,31 @@ def resource_autocomplete(context, data_dict): }) organization_title = dataset.get('organization', {}).get('title', "") - match = q_lower in dataset['name'].lower() or q_lower in dataset['title'].lower() - pkg_list.append({ - 'id': dataset['id'], - 'name': dataset['name'], - 'title': dataset['title'], - 'owner_org': organization_title, - 'match': match, - 'resources': resources - }) + # For datasets: match any token from query (allows "01" in "Resource 01" to match "test-dataset-01") + dataset_match = any( + token in dataset['name'].lower() or token in dataset['title'].lower() + for token in query_tokens + ) + + # Only include dataset if it matches at dataset level OR has matching resources + if dataset_match or has_matching_resource: + pkg_list.append({ + 'id': dataset['id'], + 'name': dataset['name'], + 'title': dataset['title'], + 'owner_org': organization_title, + 'match': dataset_match, + 'resources': resources + }) + + # Sort results: datasets with name/title matches first, then resource-only matches + # Within each group, maintain the order from package_search + for i, item in enumerate(pkg_list): + item['_original_index'] = i + pkg_list.sort(key=lambda x: (not x['match'], x['_original_index'])) + # Remove the temporary index + for item in pkg_list: + del item['_original_index'] return pkg_list @@ -118,7 +160,7 @@ def _get_dataset_from_resource_uuid(context, uuid): {"id": resource['package_id']} ) return [package] - except toolkit.NotFound: + except logic.NotFound: return [] diff --git a/ckanext/fork/tests/conftest.py b/ckanext/fork/tests/conftest.py index 677af9d..543d892 100644 --- a/ckanext/fork/tests/conftest.py +++ b/ckanext/fork/tests/conftest.py @@ -1,23 +1,32 @@ import pytest +from ckan import model from ckan.tests import factories from ckan.tests.helpers import call_action @pytest.fixture def forked_data(): + """ + Creates test data with fork metadata (requires activity plugin). + + Note: Tests using this fixture must use: + @pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') + """ giftless_metadata = { "sha256": "dummysha", "size": 999, "lfs_prefix": "test/resource", "url_type": "upload" } + user = factories.User() organization = factories.Organization() forked_dataset = factories.Dataset(owner_org=organization['id']) forked_resource = factories.Resource( package_id=forked_dataset['id'], **giftless_metadata ) - call_action('package_patch', id=forked_dataset['id'], notes='An activity') + # Trigger activity creation (factories don't create activities) + call_action('package_patch', context={'user': user['name']}, id=forked_dataset['id'], notes='An activity') forked_activity_id = call_action( 'package_activity_list', id=forked_dataset['id'] @@ -27,3 +36,20 @@ def forked_data(): 'resource': forked_resource, 'activity_id': forked_activity_id } + + +@pytest.fixture +def clean_db_with_migrations(clean_db): + """ + Extends the standard clean_db fixture to add activity plugin schema changes. + + In CKAN 2.11, the activity plugin adds a permission_labels column (text[]) + to the activity table. The clean_db fixture rebuilds the database without + plugin-specific migrations, so we manually add the column here. + """ + # Add the permission_labels column required by CKAN 2.11 activity plugin + model.Session.execute(""" + ALTER TABLE activity + ADD COLUMN IF NOT EXISTS permission_labels text[]; + """) + model.Session.commit() diff --git a/ckanext/fork/tests/test_actions.py b/ckanext/fork/tests/test_actions.py index 4358f66..7f78bbb 100644 --- a/ckanext/fork/tests/test_actions.py +++ b/ckanext/fork/tests/test_actions.py @@ -45,7 +45,7 @@ def datasets(reset_db, reset_index): return datasets -@pytest.mark.usefixtures('with_request_context') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins', 'with_request_context') class TestResourceAutocomplete(): def test_resource_autocomplete_raises_error_if_no_query(self): @@ -91,7 +91,7 @@ def test_resource_autocomplete_output_format(self, datasets): } -@pytest.mark.usefixtures('clean_db') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') class TestResourceShow(): def test_synced_fork_display(self, forked_data): @@ -104,7 +104,8 @@ def test_synced_fork_display(self, forked_data): assert response['fork_synced'] def test_unsynced_fork(self, forked_data): - call_action('resource_patch', id=forked_data['resource']['id'], sha256='newsha') + user = factories.User() + call_action('resource_patch', context={'user': user['name']}, id=forked_data['resource']['id'], sha256='newsha') dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -121,7 +122,8 @@ def test_no_access_to_forked_resource(self, forked_data): fork_resource=forked_data['resource']['id'], fork_activity=forked_data['activity_id'] ) - call_action('package_patch', id=forked_data['dataset']['id'], private=True) + user = factories.User() + call_action('package_patch', context={'user': user['name']}, id=forked_data['dataset']['id'], private=True) user = factories.User() response = toolkit.get_action('resource_show')( {'user': user['name']}, @@ -130,7 +132,7 @@ def test_no_access_to_forked_resource(self, forked_data): assert response['fork_synced'] -@pytest.mark.usefixtures('clean_db') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') class TestResourceCreate(): def test_not_fork_resource_create(self): @@ -141,9 +143,11 @@ def test_not_fork_resource_create(self): assert not resource.get('fork_synced', False) def test_fork_resource_create_with_no_activity_id(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = call_action( "resource_create", + context={'user': user['name']}, package_id=dataset['id'], fork_resource=forked_data['resource']['id'] ) @@ -153,10 +157,12 @@ def test_fork_resource_create_with_no_activity_id(self, forked_data): assert resource[key] == forked_data['resource'][key] def test_fork_resource_create_with_activity_id(self, forked_data): - call_action('resource_patch', id=forked_data['resource']['id'], sha256='newsha') + user = factories.User() + call_action('resource_patch', context={'user': user['name']}, id=forked_data['resource']['id'], sha256='newsha') dataset = factories.Dataset() resource = call_action( "resource_create", + context={'user': user['name']}, package_id=dataset['id'], fork_resource=forked_data['resource']['id'], fork_activity=forked_data['activity_id'] @@ -166,10 +172,11 @@ def test_fork_resource_create_with_activity_id(self, forked_data): assert resource[key] == forked_data['resource'][key] -@pytest.mark.usefixtures('clean_db') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') class TestResourceUpdate(): def test_fork_resource_update_with_no_activity_id(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -177,6 +184,7 @@ def test_fork_resource_update_with_no_activity_id(self, forked_data): ) forked_data['resource'] = call_action( 'resource_patch', + context={'user': user['name']}, id=forked_data['resource']['id'], sha256='newsha' ) @@ -186,6 +194,7 @@ def test_fork_resource_update_with_no_activity_id(self, forked_data): )[0]['id'] resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], fork_resource=forked_data['resource']['id'] ) @@ -195,7 +204,8 @@ def test_fork_resource_update_with_no_activity_id(self, forked_data): assert resource[key] == forked_data['resource'][key] def test_fork_resource_update_with_activity_id(self, forked_data): - call_action('resource_patch', id=forked_data['resource']['id'], sha256='newsha') + user = factories.User() + call_action('resource_patch', context={'user': user['name']}, id=forked_data['resource']['id'], sha256='newsha') dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -203,6 +213,7 @@ def test_fork_resource_update_with_activity_id(self, forked_data): ) resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], fork_resource=forked_data['resource']['id'], fork_activity=forked_data['activity_id'] @@ -210,6 +221,7 @@ def test_fork_resource_update_with_activity_id(self, forked_data): resource = factories.Resource(package_id=dataset['id']) resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], fork_resource=forked_data['resource']['id'], fork_activity=forked_data['activity_id'] @@ -219,12 +231,14 @@ def test_fork_resource_update_with_activity_id(self, forked_data): assert resource[key] == forked_data['resource'][key] def test_non_fork_resource_update_with_new_fork_resource(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], ) resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], fork_resource=forked_data['resource']['id'], ) @@ -234,6 +248,7 @@ def test_non_fork_resource_update_with_new_fork_resource(self, forked_data): assert resource[key] == forked_data['resource'][key] def test_fork_resource_update_with_new_non_fork_resource_details(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -241,6 +256,7 @@ def test_fork_resource_update_with_new_non_fork_resource_details(self, forked_da ) resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], url='http://link.to.some.data' ) @@ -248,6 +264,7 @@ def test_fork_resource_update_with_new_non_fork_resource_details(self, forked_da assert resource['fork_activity'] == '' def test_fork_resource_update_with_new_metadata(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -256,6 +273,7 @@ def test_fork_resource_update_with_new_metadata(self, forked_data): resource['description'] = 'New description' resource = call_action( "resource_update", + context={'user': user['name']}, **resource ) assert resource['fork_resource'] == forked_data['resource']['id'] @@ -263,12 +281,14 @@ def test_fork_resource_update_with_new_metadata(self, forked_data): assert resource['description'] == 'New description' def test_non_fork_resource_update_with_new_fork_details(self, forked_data): + user = factories.User() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], ) resource = call_action( "resource_update", + context={'user': user['name']}, id=resource['id'], fork_resource=forked_data['resource']['id'] ) @@ -278,7 +298,8 @@ def test_non_fork_resource_update_with_new_fork_details(self, forked_data): assert resource[key] == forked_data['resource'][key] def test_fork_resource_update_with_new_non_fork_resource_details_via_api_call(self, app, forked_data): - user = factories.Sysadmin() + """Test that uploading a file via HTTP API clears fork metadata.""" + user = factories.SysadminWithToken() dataset = factories.Dataset() resource = factories.Resource( package_id=dataset['id'], @@ -286,7 +307,7 @@ def test_fork_resource_update_with_new_non_fork_resource_details_via_api_call(se ) headers = { - 'Authorization': user['apikey'], + 'Authorization': user['token'], } files = { 'id': resource['id'], @@ -314,29 +335,41 @@ def test_fork_resource_update_with_new_non_fork_resource_details_via_api_call(se @pytest.fixture def dataset(): + """ + Creates a test dataset with 3 resources containing blob storage metadata. + + Used by TestDatasetFork and TestResourceFork to test forking functionality. + """ + user = factories.User() org = factories.Organization() dataset = factories.Dataset( - type='auto-generate-name-from-title', - id="test-id", + # CKAN 2.11 Fix: Removed type='auto-generate-name-from-title' + # This was an invalid dataset type that caused routing errors in CKAN 2.11: + # "Could not build url for endpoint 'auto-generate-name-from-title_resource.download'" + # The plugin doesn't define custom types (package_types() returns []) + # Removing this parameter allows CKAN to use the default 'dataset' type + # type='auto-generate-name-from-title', owner_org=org['id'], ) dataset['resources'] = [factories.Resource( - package_id='test-id', + package_id=dataset['id'], sha256='testsha256', size=999, lfs_prefix='test/prefix', url_type='upload' ) for i in range(3)] - call_action('package_patch', id=dataset['id'], notes="Create an activity") + call_action('package_patch', context={'user': user['name']}, id=dataset['id'], notes="Create an activity") return call_action('package_show', id=dataset['id']) -@pytest.mark.usefixtures('clean_db', 'with_plugins') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') class TestDatasetFork(): def test_dataset_metadata_duplicated(self, dataset): + user = factories.User() result = call_action( 'dataset_fork', + context={'user': user['name']}, id=dataset['id'], name="duplicated-dataset" ) @@ -345,8 +378,10 @@ def test_dataset_metadata_duplicated(self, dataset): assert all(duplicated), f"Duplication failed: {list(zip(fields, duplicated))}" def test_dataset_metadata_not_duplicated(self, dataset): + user = factories.User() result = call_action( 'dataset_fork', + context={'user': user['name']}, id=dataset['id'], name="duplicated-dataset" ) @@ -355,8 +390,10 @@ def test_dataset_metadata_not_duplicated(self, dataset): assert all(not_duplicated), f"Duplication occured: {list(zip(fields, not_duplicated))}" def test_resource_metadata_duplicated(self, dataset): + user = factories.User() result = call_action( 'dataset_fork', + context={'user': user['name']}, id=dataset['id'], name="duplicated-dataset" ) @@ -369,9 +406,11 @@ def test_resource_metadata_duplicated(self, dataset): assert duplicated, f"Field {f} did not duplicate for resource {i}" def test_dataset_not_found(self): + user = factories.User() with pytest.raises(toolkit.ObjectNotFound): call_action( 'dataset_fork', + context={'user': user['name']}, id='non-existant-id', name="duplicated-dataset" ) @@ -383,7 +422,9 @@ def test_dataset_not_found(self): ('resources', []) ]) def test_metadata_overidden(self, key, value, dataset): + user = factories.User() data_dict = { + 'context': {'user': user['name']}, 'id': dataset['id'], 'name': "duplicated-dataset", key: value @@ -392,13 +433,15 @@ def test_metadata_overidden(self, key, value, dataset): assert result[key] == value -@pytest.mark.usefixtures('clean_db', 'with_plugins') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') class TestResourceFork(): def test_resource_metadata_duplicated(self, dataset): + user = factories.User() resource = dataset['resources'][0] result = call_action( 'resource_fork', + context={'user': user['name']}, id=resource['id'] ) fields = ['name', 'sha256', 'size', 'lfs_prefix', 'url_type'] @@ -406,16 +449,20 @@ def test_resource_metadata_duplicated(self, dataset): assert all(duplicated), f"Duplication failed: {list(zip(fields, duplicated))}" def test_resource_metadata_not_duplicated(self, dataset): + user = factories.User() result = call_action( 'resource_fork', + context={'user': user['name']}, id=dataset['resources'][0]['id'] ) assert dataset['resources'][0]['id'] != result['id'] def test_resource_not_found(self): + user = factories.User() with pytest.raises(toolkit.ObjectNotFound): call_action( 'resource_fork', + context={'user': user['name']}, id='non-existant-id' ) @@ -426,7 +473,9 @@ def test_resource_not_found(self): ('format', 'JSON') ]) def test_metadata_overidden(self, key, value, dataset): + user = factories.User() data_dict = { + 'context': {'user': user['name']}, 'id': dataset['resources'][0]['id'], key: value } diff --git a/ckanext/fork/tests/test_helpers.py b/ckanext/fork/tests/test_helpers.py index 8a91019..7e47c8f 100644 --- a/ckanext/fork/tests/test_helpers.py +++ b/ckanext/fork/tests/test_helpers.py @@ -2,10 +2,10 @@ from ckan.tests import factories from ckanext.fork import helpers from ckan.plugins import toolkit -import mock +from unittest import mock -@pytest.mark.usefixtures('clean_db', 'with_request_context') +@pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins', 'with_request_context') class TestForkMetadata(): def test_expected_behaviour(self, forked_data): diff --git a/ckanext/fork/tests/test_validators.py b/ckanext/fork/tests/test_validators.py index 218fecf..a1b70e8 100644 --- a/ckanext/fork/tests/test_validators.py +++ b/ckanext/fork/tests/test_validators.py @@ -2,8 +2,8 @@ from contextlib import nullcontext as does_not_raise import ckanext.fork.validators as fork_validators import ckan.plugins.toolkit as toolkit -from ckan.tests import factories -import mock +from ckan.tests import factories, helpers +from unittest import mock @pytest.mark.usefixtures('clean_db') @@ -42,16 +42,28 @@ def test_valid_resource_id(self, value, result): with result: fork_validators.valid_resource_id(key, flattened_data, {}, {'user': 'user'}) + @pytest.mark.usefixtures('clean_db_with_migrations', 'with_plugins') @pytest.mark.parametrize("value, result", VALID_ID_PARAMS) def test_valid_activity_id(self, value, result): user = factories.User() - resource = factories.Resource() - dataset = factories.Dataset(resources=[resource]) - activity = factories.Activity( - activity_type="changed package", - object_id=dataset["id"], - user_id=user["id"] + dataset = factories.Dataset() + resource = factories.Resource(package_id=dataset["id"]) + + # Trigger an activity by making a change (factories don't create activities) + # Must provide user context for activity plugin + helpers.call_action( + 'package_patch', + context={'user': user['name']}, + id=dataset['id'], + notes='Trigger activity' + ) + + # Get the activity created by the patch + activity_list = helpers.call_action( + 'package_activity_list', + id=dataset['id'] ) + activity = activity_list[0] if value: value = value.format(good_object_id=activity['id']) diff --git a/test.ini b/test.ini index 0b6ae61..7a42b1c 100644 --- a/test.ini +++ b/test.ini @@ -8,7 +8,7 @@ use = config:../ckan/test-core.ini # Insert any custom config settings to be used when running your extension's # tests here. These will override the one defined in CKAN core's test-core.ini -ckan.plugins = fork +ckan.plugins = activity fork # Logging configuration