-
Notifications
You must be signed in to change notification settings - Fork 119
Fixes issue with toolbar in some circumstances #351
base: master
Are you sure you want to change the base?
Conversation
LGTM |
c6d5081
to
f3e7c0b
Compare
Rebased |
@@ -22,7 +22,8 @@ class NewsBlogMenu(CMSAttachMenu): | |||
def get_queryset(self, request): | |||
"""Returns base queryset with support for preview-mode.""" | |||
queryset = Article.objects | |||
if not (request.toolbar and request.toolbar.edit_mode): | |||
if not (hasattr(request, "toolbar") and | |||
getattr(request.toolbar, "edit_mode", False)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a new version of Django cms we changed this and newsblog has this code to check edit_mode for old and new django-cms
https://github.com/divio/aldryn-newsblog/blob/master/aldryn_newsblog/compat.py
also:
https://github.com/divio/django-cms/blob/develop/cms/toolbar/toolbar.py#L81
https://github.com/divio/django-cms/blob/develop/cms/toolbar/toolbar.py#L493
but checking hasattr toolbar in a request is good in my opinion
@@ -54,7 +54,8 @@ class EditModeMixin(object): | |||
|
|||
def dispatch(self, request, *args, **kwargs): | |||
self.edit_mode = ( | |||
self.request.toolbar and self.request.toolbar.edit_mode) | |||
hasattr(self.request, "toolbar") and | |||
getattr(self.request.toolbar, "edit_mode")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, we should have here:
self.edit_mode = (
hasattr(request, "toolbar") and toolbar_edit_mode_active(request))
Thanks for your contribution @mkoistinen. Could you please look into my comments and also rebase your code? |
No description provided.