Skip to content
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

Django 1.9 - part1 #4812

Merged
merged 47 commits into from
Jan 26, 2016
Merged

Django 1.9 - part1 #4812

merged 47 commits into from
Jan 26, 2016

Conversation

yakky
Copy link
Member

@yakky yakky commented Dec 3, 2015

Discussion at #4790

This brings minimal support. Some deprecation warning still emitted, but this is the minimal set to have a working 1.9 support.
Deprecation will be fixed in further PRs

Code changes in core are minimal an mostly related to some major deprecations, minor import refactoring
Most of the changes are in the tests due to the new changeform URL in the admin: I refactored a bit how the URLs are checked in tests

@yakky yakky added this to the 3.2.x milestone Dec 3, 2015
@yakky
Copy link
Member Author

yakky commented Dec 4, 2015

Only 15 errors to go!

@timgraham
Copy link
Contributor

In working on an unrelated task of removing render_to_response() usage, I found some doc and code updates that need to be made for the deprecation of RequestContext's current_app argument. You'll need to pick up current_app off the request instead of the context in pluginmodel.py.

I'll just leave this start here (currently doesn't work until that change is made).

diff --git a/docs/how_to/apphooks.rst b/docs/how_to/apphooks.rst
index fc00d3b..0546ddf 100644
--- a/docs/how_to/apphooks.rst
+++ b/docs/how_to/apphooks.rst
@@ -205,10 +205,12 @@ hook like this::

 If you use app namespace you will need to give all your view ``context`` a ``current_app``::

-  def my_view(request):
-      current_app = resolve(request.path_info).namespace
-      context = RequestContext(request, current_app=current_app)
-      return render_to_response("my_templace.html", context_instance=context)
+    from django.core.urlresolvers import resolve
+    from django.shortcuts import render
+
+    def my_view(request):
+        request.current_app = resolve(request.path_info).namespace
+        return render(request, "my_templace.html")

 .. note::
     You need to set the current_app explicitly in all your view contexts as Django does not allow
diff --git a/docs/how_to/placeholders.rst b/docs/how_to/placeholders.rst
index f51633c..e67b4d7 100644
--- a/docs/how_to/placeholders.rst
+++ b/docs/how_to/placeholders.rst
@@ -143,15 +143,14 @@ The view in which you render your placeholder field must return the
 :attr:`request <django.http.HttpRequest>` object in the context. This is
 typically achieved in Django applications by using :class:`RequestContext`::

-    from django.shortcuts import get_object_or_404, render_to_response
-    from django.template.context import RequestContext
+    from django.shortcuts import get_object_or_404, render
     from myapp.models import MyModel

     def my_model_detail(request, id):
         object = get_object_or_404(MyModel, id=id)
-        return render_to_response('my_model_detail.html', {
+        return render(request, 'my_model_detail.html', {
             'object': object,
-        }, context_instance=RequestContext(request))
+        })

 If you want to render plugins from a specific language, you can use the tag
 like this:

Technically, you could skip all this for adding 1.9 support, but there won't be a way for projects to run deprecation warning free.

As a separate enhancement, you could make django-cms automatically use resolve(request.path_info).namespace as the current_app in pluginmodel.py similar to what we did with the {% url %} tag in Django 1.9 (django/django@bc7923b). Then you can remove the note "You need to set the current_app explicitly in all your view contexts..." in docs/how_to/apphooks.rst.

@yakky
Copy link
Member Author

yakky commented Dec 23, 2015

@timgraham thanks! This task is at the moment waiting for reversion 1.10 one, I'll integrate your comments as soon as I land the other one

@yakky yakky changed the title [WiP] Django 1.9 Django 1.9 - part1 Jan 15, 2016
@yakky
Copy link
Member Author

yakky commented Jan 15, 2016

@mkoistinen @timgraham @czpython And it's done :)
As stated above, this only bring compatibility. Deprecations will be fixed in separate PRs for easier review.
See description for PR summary

@yakky
Copy link
Member Author

yakky commented Jan 15, 2016

@richardasaurus any early test is very much welcome!

@mkoistinen
Copy link
Contributor

AWESOME! Will test locally, and give feedback.

@mkoistinen
Copy link
Contributor

To anyone testing, you will likely run into issues with djangocms-file (which is installed along with CMS when using djangocms-helper), there's a fix here: https://github.com/divio/djangocms-file/tree/rename_file_to_source

@mkoistinen
Copy link
Contributor

@yakky So, I have been able to create and work with a new django CMS project with Django 1.9, Django Reversions 1.10 and the above-referenced version of djangocms-file plugin. However, I welcome any further testing anyone can provide, especially in more complicated, existing projects. Anyone?

@timgraham
Copy link
Contributor

Pinning of django-treebeard==3.0 in setup.py needs to be fixed.

@yakky
Copy link
Member Author

yakky commented Jan 19, 2016

@timgraham oops!
Fixed, thanks for spotting this

@czpython
Copy link
Contributor

@yakky hmm django-treebeard 4.0 has dropped django 1.6 support. This makes django cms 3.2.x supporting django 1.6 very tricky. I guess we'll have to provide a warning in the docs to use treebeard 3.0 in 1.6 and treebeard 4.0 in >= 1.7

@yakky
Copy link
Member Author

yakky commented Jan 19, 2016

@czpython will add

@yakky
Copy link
Member Author

yakky commented Jan 19, 2016

@czpython Added not in the install section. setup.py declare dependency on treebeard>=3.0, so it's up to the users/pip to pick the right version

@yakky
Copy link
Member Author

yakky commented Jan 25, 2016

@mkoistinen @czpython should we merge this "as is" and eventually change things aftewards?

@yakky
Copy link
Member Author

yakky commented Jan 26, 2016

Merging this. Any further 1.9 issue can be addressed in later releases

yakky added a commit that referenced this pull request Jan 26, 2016
@yakky yakky merged commit a1ca379 into django-cms:release/3.2.x Jan 26, 2016
@coveralls
Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage decreased (-0.06%) to 89.298% when pulling 1836748 on yakky:feature/3_2_1_9 into ec93eb7 on divio:release/3.2.x.

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

Successfully merging this pull request may close these issues.

None yet

6 participants