Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Django 1.5 compatibility, and performance setting #1637

Closed
wants to merge 16 commits into
from

Conversation

Projects
None yet
6 participants

ntucker commented Feb 14, 2013

Includes support for customizable User models
Performance setting will disable expensive permission checks if those are not needed.

@ojii ojii and 1 other commented on an outdated diff Feb 15, 2013

cms/__init__.py
@@ -1,3 +1,25 @@
# -*- coding: utf-8 -*-
__version__ = '2.4.0.beta'
+# patch settings
+try:
@ojii

ojii Feb 15, 2013

Collaborator

why was this re-introduced?

@ntucker

ntucker Feb 15, 2013

Setup won't work without this unless DJANGO_SETTINGS_MODULE is configured, which is not true in some deployments.

@ojii

ojii Mar 19, 2013

Collaborator

this whole "patch settings" stuff was removed a while ago on develop. and it should really not come back!

@kux kux referenced this pull request Feb 28, 2013

Closed

Fix for #1609 #1645

@kux kux commented on an outdated diff Feb 28, 2013

cms/admin/pageadmin.py
@@ -444,8 +444,11 @@ def get_form(self, request, obj=None, **kwargs):
return form
- def get_inline_instances(self, request):
- inlines = super(PageAdmin, self).get_inline_instances(request)
+ def get_inline_instances(self, request, obj=None):
+ if django.VERSION[:2] < (1, 5):
+ inlines = super(PageAdmin, self).get_inline_instances(request)
+ else:
+ inlines = super(PageAdmin, self).get_inline_instances(request, obj)
if get_cms_setting('PERMISSION') and hasattr(self, '_current_page')\
@kux

kux Feb 28, 2013

Contributor

Mind that _current_page was set as a class attribute as an workaround to the fact that get_inline_instances didn't receive obj as a parameter.

You can make this look 'nicer' by setting obj to self._current_page if the version is smaller than 1.5.

Something like:

        if django.VERSION[:2] < (1, 5):
            inlines = super(PageAdmin, self).get_inline_instances(request)
            if hasattr(self, '_current_page'):
                obj = self._current_page
        else:
            inlines = super(PageAdmin, self).get_inline_instances(request, obj)
        if get_cms_setting('PERMISSION') and obj:
            filtered_inlines = []
            for inline in inlines:
                if (isinstance(inline, PagePermissionInlineAdmin)
                        and not isinstance(inline, ViewRestrictionInlineAdmin)):
                    if "recover" in request.path or "history" in request.path:
                        # do not display permissions in recover mode
                        continue
                    if not obj.has_change_permissions_permission(request):
                        continue
                filtered_inlines.append(inline)
            inlines = filtered_inlines
        return inlines

You forgot the return statement.

Comparing versions using strings is no good idea. Use distutils.version.LooseVersion instead. The other django-cms code uses constants like DJANGO_1_3 and DJANGO_1_4. I would prefer that to be consistent.

Contributor

bikeshedder commented Mar 26, 2013

This pull request includes a lot of noise. I'm preparing a pure Django 1.5 compatibility pull request which is a combination from the patches from @kux and @ntucker. I'm going to leave the new swappable User model aside for now as this is a rather huge patch and should be part of a separate pull request.

Owner

ntucker commented on 7e2a4b7 Mar 26, 2013

Incorporated comments in new comment. Thanks for the input

Member

digi604 commented Mar 28, 2013

i am going to close this pull request as #1674 was merged.... it would be nice to have a pull request with the custom User model though or other things missing.

@digi604 digi604 closed this Mar 28, 2013

Contributor

bikeshedder commented Mar 28, 2013

I started working on the custom user model but had to realize that the PageUser model is going to cause troubles. I started a discussion on the mailing list: https://groups.google.com/d/msg/django-cms/Ji4qBDA3_DA/jrIYwQ98TSMJ

Coverage Status

Changes Unknown when pulling c7b176e on ntucker:perf into * on divio:develop*.

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