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

Fix 19 - Forward to country landingpage if top topics not set for country #47

Merged
merged 6 commits into from
Feb 29, 2016

Conversation

chris-aeviator
Copy link
Contributor

Please check proposed implementation and if https://github.com/OpenDevelopmentMekong/ckanext-odm_nav/compare/fix-19?expand=1#diff-46755a2818ff8b9c240c0ecf717e0b57R184 is a valid python approach ( not returning something if country_code is not in list )

For the proposed solution the setting
ckan.odm_nav_concept.disable_top_topic_links_for_countries = laos myanmar vietnam thailand

has been added to the ckan configuration file

def check_top_topics_disabled(country_code):
for item in disabled_top_topic_links:
if item == country_code:
return True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acorbi
is this a valid python approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

What I understand here is that you want to be able to specify certain country codes (i.e kh, vi,...) in a config variable named ckan.odm_nav_concept.disable_top_topic_links_for_countries in order to not show top topics menu in certain sites? If so, yes, looks consistant,

👍

chris-aeviator pushed a commit that referenced this pull request Feb 29, 2016
Fix 19 - Forward to country landingpage if top topics not set for country
@chris-aeviator chris-aeviator merged commit 4eefa2e into master Feb 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants