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

c.user not defined fix #4574

Merged
merged 2 commits into from Dec 7, 2018

Conversation

Projects
None yet
3 participants
@frafra
Copy link
Contributor

commented Dec 6, 2018

Fixes #4247
This issue should have been ready fixed, but it seems that the patch did not covered all the cases.
This is not suppose to be the perfect patch for this issue, it seems a workaround for me, but this seems enough for my usage of CKAN.

More about it on: #4247 (comment)

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@smotornyuk

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

Just two small alterations:

  • Previously, pylons used to return empty string('') for any undefined value, so let's use the same, instead of None
  • We are replacing c global variable with g - they are pretty same, but latter is more Flask style, so could you switch to it as well?

@smotornyuk smotornyuk self-assigned this Dec 6, 2018

@frafra frafra force-pushed the frafra:c-user-not-defined branch 3 times, most recently from a9e5c91 to 6fada50 Dec 7, 2018

@frafra

This comment has been minimized.

Copy link
Contributor Author

commented Dec 7, 2018

Hi @smotornyuk, thank you for your suggestions.
I can use an empty string instead of None, but switching from c to g would not fix the issue. I tried to use g.user instead of c.user without any additional code, but it fails. I tried to setup g.user and then use c.user, but it still fails. The only way I found to avoid this issue is just to set c.user.

@smotornyuk

This comment has been minimized.

Copy link
Member

commented Dec 7, 2018

No, I'm perfectly ok with your solution and switching to g didn't suppose to fix the problem, because g and c are the same, as I said before. You just should use g instead of c in the feature, because c - it's how Pylons calls global context, and g - is a way of naming it from Flask. As we are migrating to Flask, I just want to use flask-style names for variable
https://github.com/ckan/ckan/wiki/Migration-from-Pylons-to-Flask#c-object

@frafra frafra force-pushed the frafra:c-user-not-defined branch from 6fada50 to 4db437b Dec 7, 2018

@frafra

This comment has been minimized.

Copy link
Contributor Author

commented Dec 7, 2018

Thank you for the clarification. I did not understand that they were equivalent and I was confused by the fact that using g.user is not enough, even if the previous patch set it.

I had to import g, as helpers.py uses c only.

@smotornyuk smotornyuk merged commit a4ec78e into ckan:master Dec 7, 2018

2 checks passed

ci/circleci: test Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@frafra frafra deleted the frafra:c-user-not-defined branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.