Merge pull request #4759 from stsewd/dont-use-requestscontext

Don't use RequestsContext

readthedocs/profiles/urls/private.py

@@ -1,23 +1,31 @@
 """URL patterns for views to modify user profiles."""
 
-from __future__ import absolute_import
+from __future__ import (
+    absolute_import,
+    division,
+    print_function,
+    unicode_literals,
+)
+
 from django.conf.urls import url
 
 from readthedocs.core.forms import UserProfileForm
 from readthedocs.profiles import views
 
+
 urlpatterns = [
-    url(r'^create/', views.create_profile,
-        {
-            'form_class': UserProfileForm,
-        },
-        name='profiles_profile_create'),
-    url(r'^edit/', views.edit_profile,
+    url(
+        r'^edit/', views.edit_profile,
         {
             'form_class': UserProfileForm,
             'template_name': 'profiles/private/edit_profile.html',
         },
-        name='profiles_profile_edit'),
+        name='profiles_profile_edit'
+    ),
     url(r'^delete/', views.delete_account, name='delete_account'),
-    url(r'^advertising/$', views.account_advertising, name='account_advertising'),
+    url(
+        r'^advertising/$',
+        views.account_advertising,
+        name='account_advertising',
+    ),
 ]

readthedocs/profiles/views.py

@@ -2,114 +2,22 @@
 """Views for creating, editing and viewing site-specific user profiles."""
 
 from __future__ import (
-    absolute_import, division, print_function, unicode_literals)
+    absolute_import,
+    division,
+    print_function,
+    unicode_literals,
+)
 
 from django.contrib import messages
 from django.contrib.auth import logout
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist
 from django.core.urlresolvers import reverse
-from django.http import Http404, HttpResponseRedirect
+from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404, redirect, render
-from django.template.context import RequestContext
 from django.utils.translation import ugettext_lazy as _
 
-from readthedocs.core.forms import UserDeleteForm, UserAdvertisingForm
-
-
-def create_profile(
-        request, form_class, success_url=None,
-        template_name='profiles/private/create_profile.html',
-        extra_context=None):
-    """
-    Create a profile for the current user, if one doesn't already exist.
-
-    If the user already has a profile, a redirect will be issued to the
-    :view:`profiles.views.edit_profile` view.
-
-    **Optional arguments:**
-
-    ``extra_context``
-        A dictionary of variables to add to the template context. Any
-        callable object in this dictionary will be called to produce
-        the end result which appears in the context.
-
-    ``form_class``
-        The form class to use for validating and creating the user
-        profile. This form class must define a method named
-        ``save()``, implementing the same argument signature as the
-        ``save()`` method of a standard Django ``ModelForm`` (this
-        view will call ``save(commit=False)`` to obtain the profile
-        object, and fill in the user before the final save). If the
-        profile object includes many-to-many relations, the convention
-        established by ``ModelForm`` of using a method named
-        ``save_m2m()`` will be used, and so your form class should
-        also define this method.
-
-    ``success_url``
-        The URL to redirect to after successful profile creation. If
-        this argument is not supplied, this will default to the URL of
-        :view:`profiles.views.profile_detail` for the newly-created
-        profile object.
-
-    ``template_name``
-        The template to use when displaying the profile-creation
-        form. If not supplied, this will default to
-        :template:`profiles/create_profile.html`.
-
-    **Context:**
-
-    ``form``
-        The profile-creation form.
-
-    **Template:**
-
-    ``template_name`` keyword argument, or
-    :template:`profiles/create_profile.html`.
-    """
-    try:
-        profile_obj = request.user.profile
-        return HttpResponseRedirect(reverse('profiles_edit_profile'))
-    except ObjectDoesNotExist:
-        pass
-
-    #
-    # We set up success_url here, rather than as the default value for
-    # the argument. Trying to do it as the argument's default would
-    # mean evaluating the call to reverse() at the time this module is
-    # first imported, which introduces a circular dependency: to
-    # perform the reverse lookup we need access to profiles/urls.py,
-    # but profiles/urls.py in turn imports this module.
-    #
-
-    if success_url is None:
-        success_url = reverse(
-            'profiles_profile_detail',
-            kwargs={'username': request.user.username})
-    if request.method == 'POST':
-        form = form_class(data=request.POST, files=request.FILES)
-        if form.is_valid():
-            profile_obj = form.save(commit=False)
-            profile_obj.user = request.user
-            profile_obj.save()
-            if hasattr(form, 'save_m2m'):
-                form.save_m2m()
-            return HttpResponseRedirect(success_url)
-    else:
-        form = form_class()
-
-    if extra_context is None:
-        extra_context = {}
-    context = RequestContext(request)
-    for key, value in list(extra_context.items()):
-        context[key] = (value() if callable(value) else value)
-
-    context.update({'form': form})
-    return render(request, template_name, context=context)
-
-
-create_profile = login_required(create_profile)
+from readthedocs.core.forms import UserAdvertisingForm, UserDeleteForm
 
 
 def edit_profile(
@@ -118,9 +26,6 @@ def edit_profile(
     """
     Edit the current user's profile.
 
-    If the user does not already have a profile, a redirect will be issued to
-    the :view:`profiles.views.create_profile` view.
-
     **Optional arguments:**
 
     ``extra_context``
@@ -160,11 +65,7 @@ def edit_profile(
     ``template_name`` keyword argument or
     :template:`profiles/edit_profile.html`.
     """
-    try:
-        profile_obj = request.user.profile
-    except ObjectDoesNotExist:
-        return HttpResponseRedirect(reverse('profiles_profile_create'))
-
+    profile_obj = request.user.profile
     if success_url is None:
         success_url = reverse(
             'profiles_profile_detail',
@@ -180,10 +81,10 @@ def edit_profile(
 
     if extra_context is None:
         extra_context = {}
-    context = RequestContext(request)
-    for key, value in list(extra_context.items()):
-        context[key] = (value() if callable(value) else value)
-
+    context = {
+        key: value() if callable(value) else value
+        for key, value in extra_context.items()
+    }
     context.update({
         'form': form,
         'profile': profile_obj,
@@ -204,7 +105,7 @@ def delete_account(request):
         form = UserDeleteForm(instance=request.user, data=request.POST)
         if form.is_valid():
             # Delete the user permanently
-            # It will also delete some projects where he is the only owner
+            # It will also delete some projects where the user is the only owner
             request.user.delete()
             logout(request)
             messages.info(request, 'You have successfully deleted your account')
@@ -221,8 +122,7 @@ def profile_detail(
     """
     Detail view of a user's profile.
 
-    If the user has not yet created a profile, ``Http404`` will be
-    raised.
+    If the user does not exists, ``Http404`` will be raised.
 
     **Required arguments:**
 
@@ -264,33 +164,25 @@ def profile_detail(
     :template:`profiles/profile_detail.html`.
     """
     user = get_object_or_404(User, username=username)
-    try:
-        profile_obj = user.profile
-    except ObjectDoesNotExist:
-        raise Http404
-    if public_profile_field is not None and \
-       not getattr(profile_obj, public_profile_field):
+    profile_obj = user.profile
+    if (public_profile_field is not None and
+            not getattr(profile_obj, public_profile_field)):
         profile_obj = None
 
     if extra_context is None:
         extra_context = {}
-    context = RequestContext(request)
-    for key, value in list(extra_context.items()):
-        context[key] = (value() if callable(value) else value)
-
+    context = {
+        key: value() if callable(value) else value
+        for key, value in extra_context.items()
+    }
     context.update({'profile': profile_obj})
     return render(request, template_name, context=context)
 
 
 @login_required
 def account_advertising(request):
     success_url = reverse(account_advertising)
-
-    try:
-        profile_obj = request.user.profile
-    except ObjectDoesNotExist:
-        return HttpResponseRedirect(reverse('profiles_profile_create'))
-
+    profile_obj = request.user.profile
     if request.method == 'POST':
         form = UserAdvertisingForm(
             data=request.POST,

readthedocs/rtd_tests/tests/test_profile_views.py

@@ -0,0 +1,88 @@
+from __future__ import division, print_function, unicode_literals
+
+from django.contrib.auth.models import User
+from django.core.urlresolvers import reverse
+from django.test import TestCase
+from django_dynamic_fixture import get
+
+
+class ProfileViewsTest(TestCase):
+
+    def setUp(self):
+        self.user = get(User)
+        self.user.set_password('test')
+        self.user.save()
+        self.client.login(username=self.user.username, password='test')
+
+    def test_edit_profile(self):
+        resp = self.client.get(
+            reverse('profiles_profile_edit'),
+        )
+        self.assertTrue(resp.status_code, 200)
+        resp = self.client.post(
+            reverse('profiles_profile_edit'),
+            data={
+                'first_name': 'Read',
+                'last_name': 'Docs',
+                'homepage': 'readthedocs.org',
+            }
+        )
+        self.assertTrue(resp.status_code, 200)
+
+        self.user.refresh_from_db()
+        self.user.profile.refresh_from_db()
+        self.assertEqual(self.user.first_name, 'Read')
+        self.assertEqual(self.user.last_name, 'Docs')
+        self.assertEqual(self.user.profile.homepage, 'readthedocs.org')
+
+    def test_delete_account(self):
+        resp = self.client.get(
+            reverse('delete_account')
+        )
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.post(
+            reverse('delete_account'),
+            data={
+                'username': self.user.username,
+            }
+        )
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(resp['Location'], reverse('homepage'))
+
+        self.assertFalse(
+            User.objects.filter(username=self.user.username).exists()
+        )
+
+    def test_profile_detail(self):
+        resp = self.client.get(
+            reverse('profiles_profile_detail', args=(self.user.username,)),
+        )
+        self.assertTrue(resp.status_code, 200)
+
+    def test_profile_detail_logout(self):
+        self.client.logout()
+        resp = self.client.get(
+            reverse('profiles_profile_detail', args=(self.user.username,)),
+        )
+        self.assertTrue(resp.status_code, 200)
+
+    def test_profile_detail_not_found(self):
+        resp = self.client.get(
+            reverse('profiles_profile_detail', args=('not-found',)),
+        )
+        self.assertTrue(resp.status_code, 404)
+
+    def test_account_advertising(self):
+        resp = self.client.get(
+            reverse('account_advertising')
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertTrue(self.user.profile.allow_ads)
+        resp = self.client.post(
+            reverse('account_advertising'),
+            data={'allow_ads': False},
+        )
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(resp['Location'], reverse('account_advertising'))
+        self.user.profile.refresh_from_db()
+        self.assertFalse(self.user.profile.allow_ads)