Skip to content
This repository has been archived by the owner on Oct 29, 2019. It is now read-only.

Fixes issue with toolbar in some circumstances #351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mkoistinen
Copy link
Contributor

No description provided.

@cyberstar
Copy link
Contributor

LGTM

@mkoistinen
Copy link
Contributor Author

Rebased

@coveralls
Copy link

coveralls commented Jun 28, 2016

Coverage Status

Coverage decreased (-0.08%) to 90.482% when pulling f3e7c0b on fix_for_toolbar into 2887897 on master.

@@ -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)):

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"))

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))

@bplociennik
Copy link

Thanks for your contribution @mkoistinen. Could you please look into my comments and also rebase your code?

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

Successfully merging this pull request may close these issues.

4 participants