Skip to content

Commit

Permalink
Make Site as optional (#244)
Browse files Browse the repository at this point in the history
* Make Site as optional

* Add a site to activate_user's arg

* Fix flake8 bugs
  • Loading branch information
Surgo authored and joshblum committed Apr 7, 2017
1 parent a88bb12 commit 279944e
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 34 deletions.
4 changes: 3 additions & 1 deletion docs/default-backend.rst
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ Additionally, :class:`RegistrationProfile` has a custom manager
This manager provides several convenience methods for creating and
working with instances of :class:`RegistrationProfile`:

.. method:: activate_user(activation_key)
.. method:: activate_user(activation_key, site)

Validates ``activation_key`` and, if valid, activates the
associated account by setting its ``is_active`` field to
Expand All @@ -214,6 +214,8 @@ Additionally, :class:`RegistrationProfile` has a custom manager
:param activation_key: The activation key to use for the
activation.
:type activation_key: string, a 40-character SHA1 hexdigest
:type site: ``django.contrib.sites.models.Site`` or
``django.contrib.sites.models.RequestSite``
:rtype: ``User`` or bool

.. method:: delete_expired_users
Expand Down
12 changes: 5 additions & 7 deletions registration/admin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.contrib import admin
from django.utils.translation import ugettext_lazy as _
from django.contrib.sites.requests import RequestSite
from django.apps import apps
from django.contrib.sites.shortcuts import get_current_site

from .models import RegistrationProfile
from .users import UsernameField
Expand All @@ -20,8 +19,10 @@ def activate_users(self, request, queryset):
activated.
"""

site = get_current_site(request)
for profile in queryset:
RegistrationProfile.objects.activate_user(profile.activation_key)
RegistrationProfile.objects.activate_user(profile.activation_key, site)
activate_users.short_description = _("Activate users")

def resend_activation_email(self, request, queryset):
Expand All @@ -34,11 +35,8 @@ def resend_activation_email(self, request, queryset):
activated.
"""
if apps.is_installed('django.contrib.sites'):
site = apps.get_model('sites', 'Site').objects.get_current()
else:
site = RequestSite(request)

site = get_current_site(request)
for profile in queryset:
user = profile.user
RegistrationProfile.objects.resend_activation_mail(user.email, site, request)
Expand Down
3 changes: 2 additions & 1 deletion registration/backends/default/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ def activate(self, *args, **kwargs):
"""
activation_key = kwargs.get('activation_key', '')
site = get_current_site(self.request)
activated_user = (self.registration_profile.objects
.activate_user(activation_key))
.activate_user(activation_key, site))
if activated_user:
signals.user_activated.send(sender=self.__class__,
user=activated_user,
Expand Down
10 changes: 4 additions & 6 deletions registration/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import string

from django.conf import settings
from django.contrib.sites.models import Site
from django.core.exceptions import ImproperlyConfigured
from django.core.exceptions import ObjectDoesNotExist
from django.core.mail import EmailMultiAlternatives
Expand Down Expand Up @@ -65,7 +64,7 @@ class RegistrationManager(models.Manager):
"""

def _activate(self, profile, get_profile):
def _activate(self, profile, site, get_profile):
"""
Activate the ``RegistrationProfile`` given as argument.
User is able to login, as ``is_active`` is set to ``True``
Expand All @@ -82,7 +81,7 @@ def _activate(self, profile, get_profile):
else:
return user

def activate_user(self, activation_key, get_profile=False):
def activate_user(self, activation_key, site, get_profile=False):
"""
Validate an activation key and activate the corresponding
``User`` if valid.
Expand Down Expand Up @@ -125,7 +124,7 @@ def activate_user(self, activation_key, get_profile=False):
return False

if not profile.activation_key_expired():
return self._activate(profile, get_profile)
return self._activate(profile, site, get_profile)

return False

Expand Down Expand Up @@ -412,7 +411,7 @@ def send_activation_email(self, site, request=None):

class SupervisedRegistrationManager(RegistrationManager):

def _activate(self, profile, get_profile):
def _activate(self, profile, site, get_profile):
"""
Activate the ``SupervisedRegistrationProfile`` given as argument.
Expand All @@ -422,7 +421,6 @@ def _activate(self, profile, get_profile):
"""

if not profile.user.is_active and not profile.activated:
site = Site.objects.get_current()
self.send_admin_approve_email(profile.user, site)

# do not set ``User.is_active`` as True. This will be set
Expand Down
45 changes: 26 additions & 19 deletions registration/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def test_valid_activation(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))
self.assertEqual(activated.id, new_user.id)
Expand All @@ -260,7 +260,8 @@ def test_valid_activation_with_profile(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated_profile = (self.registration_profile.objects
.activate_user(profile.activation_key, get_profile=True))
.activate_user(profile.activation_key, Site.objects.get_current(),
get_profile=True))

self.failUnless(isinstance(activated_profile, self.registration_profile))
self.assertEqual(activated_profile.id, profile.id)
Expand All @@ -284,7 +285,7 @@ def test_expired_activation(self):

profile = self.registration_profile.objects.get(user=new_user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failIf(isinstance(activated, UserModel()))
self.failIf(activated)
Expand All @@ -301,7 +302,8 @@ def test_activation_invalid_key(self):
fails.
"""
self.failIf(self.registration_profile.objects.activate_user('foo'))
self.failIf(self.registration_profile.objects.activate_user(
'foo', Site.objects.get_current()))

def test_activation_already_activated(self):
"""
Expand All @@ -311,11 +313,12 @@ def test_activation_already_activated(self):
new_user = self.registration_profile.objects.create_inactive_user(
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
self.registration_profile.objects.activate_user(profile.activation_key)
self.registration_profile.objects.activate_user(
profile.activation_key, Site.objects.get_current())

profile = self.registration_profile.objects.get(user=new_user)
self.assertEqual(self.registration_profile.objects.activate_user(
profile.activation_key), new_user)
profile.activation_key, Site.objects.get_current()), new_user)

def test_activation_deactivated(self):
"""
Expand All @@ -324,15 +327,16 @@ def test_activation_deactivated(self):
new_user = self.registration_profile.objects.create_inactive_user(
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
self.registration_profile.objects.activate_user(profile.activation_key)
self.registration_profile.objects.activate_user(
profile.activation_key, Site.objects.get_current())

# Deactivate the new user.
new_user.is_active = False
new_user.save()

# Try to activate again and ensure False is returned.
failed = self.registration_profile.objects.activate_user(
profile.activation_key)
profile.activation_key, Site.objects.get_current())
self.assertFalse(failed)

def test_activation_nonexistent_key(self):
Expand All @@ -344,7 +348,8 @@ def test_activation_nonexistent_key(self):
# Due to the way activation keys are constructed during
# registration, this will never be a valid key.
invalid_key = hashlib.sha1(six.b('foo')).hexdigest()
self.failIf(self.registration_profile.objects.activate_user(invalid_key))
self.failIf(self.registration_profile.objects.activate_user(
invalid_key, Site.objects.get_current()))

def test_expired_user_deletion(self):
"""
Expand Down Expand Up @@ -460,7 +465,7 @@ def test_resend_activation_email_activated_user(self):

profile = self.registration_profile.objects.get(user=user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))
self.assertTrue(activated.is_active)

self.assertFalse(self.registration_profile.objects.resend_activation_mail(
Expand Down Expand Up @@ -516,7 +521,7 @@ def old_method(self, save=True):

self.registration_profile.create_new_activation_key = current_method
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))
self.assertEqual(activated.id, new_user.id)
Expand Down Expand Up @@ -554,7 +559,7 @@ def test_valid_activation(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))
self.assertEqual(activated.id, new_user.id)
Expand All @@ -573,7 +578,8 @@ def test_valid_activation_with_profile(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated_profile = (self.registration_profile.objects
.activate_user(profile.activation_key, get_profile=True))
.activate_user(profile.activation_key, Site.objects.get_current(),
get_profile=True))

self.failUnless(isinstance(activated_profile, self.registration_profile))
self.assertEqual(activated_profile.id, profile.id)
Expand All @@ -593,7 +599,7 @@ def test_resend_activation_email_activated_user(self):

profile = self.registration_profile.objects.get(user=user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))
self.assertFalse(activated.is_active)

self.assertFalse(self.registration_profile.objects.resend_activation_mail(
Expand Down Expand Up @@ -752,7 +758,7 @@ def test_valid_admin_approval(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))

Expand Down Expand Up @@ -783,7 +789,7 @@ def test_admin_approval_already_approved(self):
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))

Expand Down Expand Up @@ -813,11 +819,12 @@ def test_activation_already_activated(self):
new_user = self.registration_profile.objects.create_inactive_user(
site=Site.objects.get_current(), **self.user_info)
profile = self.registration_profile.objects.get(user=new_user)
self.registration_profile.objects.activate_user(profile.activation_key)
self.registration_profile.objects.activate_user(
profile.activation_key, Site.objects.get_current())

profile = self.registration_profile.objects.get(user=new_user)
self.assertEqual(self.registration_profile.objects.activate_user(
profile.activation_key), False)
profile.activation_key, Site.objects.get_current()), False)

def test_activation_key_backwards_compatibility(self):
"""
Expand Down Expand Up @@ -846,7 +853,7 @@ def old_method(self, save=True):

self.registration_profile.create_new_activation_key = current_method
activated = (self.registration_profile.objects
.activate_user(profile.activation_key))
.activate_user(profile.activation_key, Site.objects.get_current()))

self.failUnless(isinstance(activated, UserModel()))
self.assertEqual(activated.id, new_user.id)
Expand Down

0 comments on commit 279944e

Please sign in to comment.