-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[#4310] Add global properties controller
and action
#4319
[#4310] Add global properties controller
and action
#4319
Conversation
There are a bunch of extensions that relies on this props. This PR is attempting to add them inside `before_each_request` hook. If url.endpoint contains dot, than part before dot will be considered as controller and part after the dot - as action. otherwise(routes without dot, like test /hello route), view name is used for both, controller and action name
Should we also deprecate these attributes, for example by adding a note in the change log? |
Looks good. This is not going to cover all cases though. eg any dataset endpoint will set @torfsen I'm keen on your opinion as a extension maintainer here. Is it fair to document this in the changelog and ask extensions to do a bit of handling like try:
is_dataset_request = toolkit.c.controller == 'package' # CKAN < 2.8
except AttributeError:
is_dataset_request = toolkit.request.endpoint.startswith('dataset.') # CKAN >= 2.8 I guess that for the |
Personally, I'm definitely +1 for moving towards a consistent behavior without hard-coded exceptions, even if that requires modifications for our extensions. As long as the changes are well-documented, this should be no problem. You could make things easier for extension maintainers by providing something like the following as a toolkit function (untested code): def get_endpoint():
try:
# CKAN >= 2.8
return tuple(toolkit.request.endpoint.split('.'))
except AttributeError:
try:
# CKAN < 2.8
return toolkit.c.controller, toolkit.c.action
except AttributeError:
return None That way one also has to update the extensions, but
One might still need to perform multiple checks due to difference in the versions (e.g.
|
@smotornyuk will add the toolkit function suggested by @torfsen |
4deec0b
to
6880ef3
Compare
I've added function and updated changelog notes |
fixes #4310.
There are a bunch of extensions that rely on this props. This PR
is attempting to add them inside
before_each_request
hook. Ifurl.endpoint contains a dot, then part before dot will be considered
as controller and part after the dot - as action. otherwise(routes
without the dot, like test /hello route), view name is used for both,
controller and action name