Skip to content

Clean old c attribute from helpers.py #6765

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

Merged
merged 6 commits into from
Mar 31, 2022

Conversation

pdelboca
Copy link
Member

@pdelboca pdelboca commented Mar 23, 2022

The c object is a legacy attribute that adds noise and complexity to our code base. While working with Pylons, the c object was used a lot to store variables needed when rendering templates. Currently we have the extra_vars dict so some of the changes is to explicitly pass the variable to the snippets and do not rely on it being magically stored in the c object.

This PR cleans its usage in the helpers.py module.

Changes

  • There are some direct replacements c -> g since currently we proxy the value
  • Add some explicit search_facets parameters to remove the dependency on c.search_facets

@pdelboca
Copy link
Member Author

@tino097 I've added the removal file to the changes folder :)

@tino097 tino097 merged commit 9d17022 into ckan:master Mar 31, 2022
@@ -1,4 +1,4 @@
{% set tags = h.get_facet_items_dict('tags', c.search_facets, limit=3) %}
{% set tags = h.get_facet_items_dict('tags', search_facets, limit=3) %}

Choose a reason for hiding this comment

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

FYI This always returns no tags because search_facets does not exist because it is not being set in the extra_vars object passed to this template from the blueprint home.index.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @MarkCalvert ! Would you like to create a PR fixing it? :)

Choose a reason for hiding this comment

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

Sure. We will add it to our list to contribute back a PR.

Copy link
Member

Choose a reason for hiding this comment

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

This was fixed in 2.10 dev branch but i didnt added the labels for backports
#7519

@tino097
Copy link
Member

tino097 commented Nov 23, 2023

@amercader this PR shoudl be included in 2.10.2

@amercader
Copy link
Member

@tino097 I guess you mean #7520 should be included right? Both #6765 and #7520 are backported to the dev-v2.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants