Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Pylons routes for build_nav_icon #477

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frafra
Copy link
Contributor

@frafra frafra commented Oct 29, 2021

Fix #476

I am not sure why admin and about require harvester. as prefix instead of the variable dataset_type, but it seems to work for me.

This PR does not replace all the Pylons routes: there are still a lot of deprecation warnings. This is just the bare minimum to make ckanext-harvest work with the current CKAN. I also prefer to send small fixes as they can be more easily reviewed instead of trying to replace all the routes at once.

@Zharktas
Copy link
Member

This requires conditionals, as the extension still needs to work on older ckan versions. Thats why tests are failing.

@frafra
Copy link
Contributor Author

frafra commented Mar 2, 2022

Any suggestion/example on how to handle these conditionals properly?

@tino097
Copy link
Member

tino097 commented Mar 7, 2022

@frafra maybe you can use the

 if h.ckan_version() > '2.9' 

in the templates to use the newer routes

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.

Cannot load pages due to old Pylons route syntax
3 participants