fusionbox / django-widgy

A CMS framework for Django built on a heterogenous tree editor.
http://wid.gy
Other
332 stars 52 forks source link

Django 1.7 compatibility #303

Closed gavinwahl closed 9 years ago

gavinwahl commented 9 years ago

This models breaks migrations in Django 1.7. We don't need to override this model in new versions of Django anyway.

We can remove this file completely when we drop Django 1.4, but that's probably 9 months away if we follow Django's support policy.

rockymeza commented 9 years ago

I thought support for 1.4 would be dropped when 1.8 is released, seeing as it's the new LTS. Is this not true?

gavinwahl commented 9 years ago

The old LTS is supported for 6 months after the release of the new one.

gavinwahl commented 9 years ago

I'm updating this pull request to include all the django 1.7 commits. I've got it all working except for one thing I can't figure out.

Before, Content._meta.app_label was 'models', not 'widgy' like you might expect (it's abstract so it didn't really matter, except for template hierarchy). In django 1.7, with the new app loading, it's 'widgy'. TestTemplateHierarchy demonstrates the problem. Explicitly setting the app_label to widgy to use the new django behavior causes other tests to fail, so I'm not sure what to do.

gavinwahl commented 9 years ago

This is one way to fix it, but it seems pretty dirty:

diff --git a/tests/modeltests/core_tests/tests/templates.py b/tests/modeltests/core_tests/tests/templates.py
index 4ee73cd..22021a8 100644
--- a/tests/modeltests/core_tests/tests/templates.py
+++ b/tests/modeltests/core_tests/tests/templates.py
@@ -114,10 +114,10 @@ class TestTemplateHierarchy(TestCase):
         self.assertEqual(list(unique_everseen(MyInvisibleBucket.get_templates_hierarchy(template_name='test'))), [
             'widgy/core_tests/myinvisiblebucket/test.html',
             'widgy/mixins/invisible/test.html',
-            'widgy/models/content/test.html',
+            'widgy/widgy/content/test.html',
             'widgy/core_tests/test.html',
             'widgy/mixins/test.html',
-            'widgy/models/test.html',
+            'widgy/widgy/test.html',
             'widgy/test.html',
         ])

@@ -126,8 +126,8 @@ class TestTemplateHierarchy(TestCase):
             'widgy/core_tests/weirdpkbucket/test.html',
             'widgy/core_tests/weirdpkbucketbase/test.html',
             'widgy/core_tests/weirdpkbase/test.html',
-            'widgy/models/content/test.html',
+            'widgy/widgy/content/test.html',
             'widgy/core_tests/test.html',
-            'widgy/models/test.html',
+            'widgy/widgy/test.html',
             'widgy/test.html',
         ])
diff --git a/widgy/models/base.py b/widgy/models/base.py
index 39c8d97..656a5a3 100644
--- a/widgy/models/base.py
+++ b/widgy/models/base.py
@@ -424,6 +424,7 @@ class Content(models.Model):

     class Meta:
         abstract = True
+        app_label = 'widgy'

     def __init__(self, *args, **kwargs):
         super(Content, self).__init__(*args, **kwargs)
@@ -824,6 +825,10 @@ class Content(models.Model):
         return self.get_attributes() == other.get_attributes()

+# Otherwise, mixins will inherit Content.Meta and get wierd app_labels
+del Content.Meta
+
+
 class UnknownWidget(Content):
     """
     A placeholder Content class used when the correct one can't be found. For

Based on https://code.djangoproject.com/ticket/22758 I think this is going to be the only solution.

gavinwahl commented 9 years ago

I fixed the rest of the contrib failures, this should be good to merge.

plee commented 9 years ago

:shipit: :v:

rockymeza commented 9 years ago

This is not good to merge:

https://code.djangoproject.com/ticket/24470

acatton commented 9 years ago

We should workaround the serialization bug (by manually fixing the migrations) and try to merge this.

gavinwahl commented 9 years ago

Added the migrations, this should be good to go