Skip to content

CKAN 2.11 migration#19

Open
A-Souhei wants to merge 21 commits intodevelopmentfrom
toavina/update-python
Open

CKAN 2.11 migration#19
A-Souhei wants to merge 21 commits intodevelopmentfrom
toavina/update-python

Conversation

@A-Souhei
Copy link
Copy Markdown

@A-Souhei A-Souhei commented Jan 9, 2026

Summary

This PR completes the CKAN 2.11 migration by fixing all remaining test failures.

Changes Overview

Starting Point

  • 80 failed tests, 13 passed (16% passing rate)
  • Authorization functions not registered
  • Missing activity plugin database schema
  • Resources created without triggering activities

Commits

1. Fix CKAN 2.11 test failures: Add with_plugins and use clean_db_with_migrations

Fixed: Authorization function registration and database schema

  • Added `with_plugins` fixture to all test classes to ensure plugin auth functions are registered
  • Changed `clean_db` to `clean_db_with_migrations` to create `permission_labels` column required by CKAN 2.11 activity plugin
  • Result: 80 → 61 failures

Files modified:

  • test_dataset_version_action.py: 1 test class
  • test_actions.py: 7 test classes
  • test_auth.py: 1 test class
  • test_helpers.py: 2 test functions

2-5. Fix resource version tests: Trigger activities

Fixed: Tests calling `resource_version_create` without proper activity setup

  • Added `package_patch` calls to trigger activity creation before version operations
  • Added user context to `resource_view_create` calls for activity plugin
  • Result: 61 → 0 failures

Tests fixed:

  • test_resource_version_create
  • test_resource_version_create_editor_user
  • test_resource_version_create_creator_user_id_parameter
  • test_version_show
  • test_version_activity_is_correct
  • test_resource_in_activity
  • test_activity_resource_shows_correct_resource
  • test_get_activity_id_from_resource_version_name
  • test_resource_version_clear
  • test_resource_view_list_returns_versions_view_last
  • test_resource_view_list_returns_default_order_if_no_versions_view

Files modified:

  • ckanext/versions/tests/test_actions.py (multiple test methods)

Root Causes Identified

1. Missing Plugin Fixtures

Tests didn't have `with_plugins` fixture, so plugin auth functions were never registered with CKAN.

2. Missing Database Column

Activity plugin in CKAN 2.11 requires `permission_labels` column (text array) in the activity table. The standard `clean_db` fixture doesn't include plugin-specific schema changes.

3. Factories Don't Create Activities

`factories.Resource()` bypasses the action layer and doesn't create activities. In CKAN 2.11, `resource_version_create()` requires an activity that contains the resource.

Solution pattern:
```python

Create test data with factories

user = factories.User()
dataset = factories.Dataset()
resource = factories.Resource(package_id=dataset['id'])

Trigger activity BEFORE version_create

context = get_context(user)
toolkit.get_action('package_patch')(context, {
'id': dataset['id'],
'notes': 'Trigger activity'
})

Now version_create will work

version = resource_version_create(context, {...})
```

4. Missing User Context

Activity plugin requires valid user context for all data-modifying actions. Without explicit `context={'user': user['name']}`, CKAN defaults to IP address `'127.0.0.1'` which fails user lookup.

Testing

All tests pass consistently:
```
103 passed, 1050 warnings in 99-102s
```

Migration Status

✅ CKAN 2.11 migration: COMPLETE
✅ All authorization functions registered
✅ Activity plugin integrated
✅ All 103 tests passing
✅ Ready for merge

ChasNelson1990 and others added 15 commits December 8, 2025 15:50
Co-authored-by: ChasNelson1990 <7795189+ChasNelson1990@users.noreply.github.com>
Co-authored-by: ChasNelson1990 <7795189+ChasNelson1990@users.noreply.github.com>
Co-authored-by: ChasNelson1990 <7795189+ChasNelson1990@users.noreply.github.com>
…lugin

Co-authored-by: ChasNelson1990 <7795189+ChasNelson1990@users.noreply.github.com>
Co-authored-by: ChasNelson1990 <7795189+ChasNelson1990@users.noreply.github.com>
Fix Python 3.10+ and CKAN 2.10-2.11 compatibility for tests
- Remove matrix strategy for multiple Python/CKAN versions
- Set single Python version to 3.10
- Update to CKAN 2.11 containers (ckan-dev:2.11-py3.10)
- Update services: Solr 2.11-solr9, Postgres 2.11
- Simplify test execution
- Remove codecov upload step
Fixes attempted but not fully verified:
- Fix auth function registration bug (plugin.py:71)
- Add clean_db_with_migrations fixture for permission_labels column
- Update test_helpers.py to use new fixture

Issues identified:
- Auth functions not being found despite correct registration (80+ tests failing)
- Missing CKAN 2.11 actions (package_activity_list, activity_data_show)
- View plugin issues (versions_view, image_view)

Migration abandoned due to:
- Complexity significantly exceeds ckanext-fork and ckanext-restricted
- Deeper architectural compatibility issues with CKAN 2.11
- Time/resource constraints

See PROGRESS.md for detailed findings and recommendations.
…igrations

Root cause: Tests were failing because:
1. Plugin auth functions weren't registered (missing with_plugins fixture)
2. Activity plugin's permission_labels column wasn't created (using clean_db instead of clean_db_with_migrations)

Changes:
- Added 'with_plugins' fixture to all test classes to ensure plugin auth functions are registered
- Changed 'clean_db' to 'clean_db_with_migrations' to create permission_labels column required by CKAN 2.11 activity plugin

Result: Reduced failures from 80 to 61 (authorization errors fixed, database schema issues fixed)

Files modified:
- test_dataset_version_action.py: 1 test class
- test_actions.py: 7 test classes
- test_auth.py: 1 test class
- test_helpers.py: 2 test functions
Root cause: In CKAN 2.11, resource_version_create requires an activity that
contains the resource. Factories bypass the action layer and don't create
activities, causing 'Resource not found in the activity' errors.

Fixes applied:

1. Activity Triggering (4 tests):
   - test_resource_version_create
   - test_resource_version_create_editor_user
   - test_version_show
   - test_resource_in_activity
   - test_version_activity_is_correct
   Added package_patch calls to trigger activity creation before version_create

2. User Context for Activity Plugin (2 tests):
   - test_resource_view_list_returns_versions_view_last
   - test_resource_view_list_returns_default_order_if_no_versions_view
   Added context={'user': user['name']} to resource_view_create calls

Pattern: Factories create test data fast but skip activities. Use call_action()
or trigger activities with package_patch when tests depend on them.

Result: 101 passed, 2 failed (down from 80 failures)
Fixed tests:
- test_activity_resource_shows_correct_resource
- test_get_activity_id_from_resource_version_name

Root cause: Same pattern - resource_version_create requires an activity
containing the resource, but factories don't create activities.

Solution: Added package_patch calls before version_create to trigger activity
creation.

Remaining: test_resource_version_clear
Result: ALL 103 TESTS PASSING! 🎉🎉🎉

Fixed test:
- test_resource_version_clear

Root cause: Same pattern as all other fixes - resource_version_create
requires an activity containing the resource. Factories bypass the action
layer and don't create activities.

Solution:
- Added dataset creation (resource needs a package)
- Added package_patch call to trigger activity creation
- Moved user creation before resource creation for proper setup order

Migration Summary:
- Started with: 80 failed, 13 passed (16% passing)
- Total fixes: 10+ tests modified across 2 commits

✅ CKAN 2.11 migration: COMPLETE
✅ All authorization functions registered
✅ Activity plugin integrated
✅ All tests passing
Same issue as previous fixes - needs activity trigger before version_create.

Added:
- context variable to reuse in package_patch and version_create
- package_patch call to trigger activity creation

This was missed in the earlier sweep of tests.
@A-Souhei A-Souhei marked this pull request as draft January 9, 2026 20:02
- Fixed bug in resource_view_list where iterating and popping from list caused incorrect ordering
- Replaced loop with list comprehensions to properly separate versions_view from other views
- Added activity triggers to all tests calling resource_version_create:
  * test_cannot_create_versions_with_same_name
  * test_resource_has_version
  * test_version_show_for_version_name
  * test_resource_version_list
  * test_resource_version_current
  * test_version_delete

All tests in test_actions.py now pass consistently without intermittent failures.
Added model.Session.commit() after all package_patch and resource_patch
calls that precede resource_version_create to ensure activities are
persisted and visible to subsequent database queries.

Root cause: SQLAlchemy session visibility issue where activities created
by patch operations were not always flushed/committed before being queried,
causing intermittent "Activity not found" failures.

Changes:
- Import ckan.model for Session access
- Add 19 explicit commits after activity-triggering operations
- Remove debug print statements from TestResourceView tests

Tests now pass consistently across multiple runs without intermittency.
- Split long line using backslash continuation (E501)
- Remove f-string prefix from strings without placeholders (F541)
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the CKAN 2.11 migration for ckanext-versions by fixing all 103 test failures through targeted fixes to authorization, database schema, and test patterns.

Key Changes

  • Fixed authorization function registration bug in plugin.py
  • Added clean_db_with_migrations fixture to handle CKAN 2.11 activity plugin schema requirements
  • Updated all tests to trigger activities before resource version operations to support CKAN 2.11's activity-based version creation
  • Fixed resource_view_list bug where iterating and modifying list simultaneously could skip elements
  • Updated Activity model imports for CKAN 2.10+ compatibility

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/workflows/test.yml Simplified to target only CKAN 2.11 with Python 3.10, removing multi-version matrix
ckanext/versions/plugin.py Fixed auth function registration bug (action → auth)
ckanext/versions/logic/action.py Updated Activity import for CKAN 2.10+ compatibility; fixed resource_view_list iteration bug
ckanext/versions/logic/dataset_version_action.py Updated Activity import for CKAN 2.10+ compatibility
ckanext/versions/tests/fixtures.py Added clean_db_with_migrations fixture to add permission_labels column
ckanext/versions/tests/test_helpers.py Updated to use clean_db_with_migrations and with_plugins fixtures
ckanext/versions/tests/test_dataset_version_action.py Updated to use clean_db_with_migrations and with_plugins fixtures
ckanext/versions/tests/test_auth.py Updated to use clean_db_with_migrations and with_plugins fixtures
ckanext/versions/tests/test_actions.py Added activity triggers before version operations; improved test assertions; added user context to resource_view_create calls
PROGRESS.md Added migration documentation (appears to be internal notes)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ckanext/versions/tests/fixtures.py
Comment thread PROGRESS.md Outdated
Comment thread ckanext/versions/tests/test_actions.py
Added explicit session commits after resource_view_create calls to ensure
views are persisted in the correct order before being queried by
resource_view_list.

Same root cause as previous intermittent failures: SQLAlchemy session
visibility issue where created views were not always committed before
being queried, causing order-dependent test failures.
@A-Souhei A-Souhei changed the title Fix CKAN 2.11 test failures - All 103 tests passing CKAN 2.11 migration Apr 17, 2026
@A-Souhei A-Souhei changed the base branch from chas/update-python to development April 17, 2026 13:43
@A-Souhei A-Souhei marked this pull request as ready for review April 17, 2026 13:48
@A-Souhei A-Souhei requested a review from ChasNelson1990 April 17, 2026 13:48
Comment thread ckanext/versions/logic/action.py Outdated
from ckanext.versions.model import Version

# Activity import for different CKAN versions
# CKAN 2.10+ moved Activity to a separate plugin
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W're dropping CKAN <2.10 support so you can remove this logic from both places it is used.

Copy link
Copy Markdown
Author

@A-Souhei A-Souhei Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look at this tomorrow morning.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChasNelson1990

Fixed. Can I merge without a re-review?

Copy link
Copy Markdown
Member

@ChasNelson1990 ChasNelson1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

CKAN <2.10 is no longer supported, so the try/except fallback chain
for importing Activity from ckan.model is dead code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants