From 2531ea6a8bd8b015280789d418b21450d04044c9 Mon Sep 17 00:00:00 2001 From: chenba Date: Thu, 5 Aug 2010 00:37:22 -0700 Subject: [PATCH] Bug 574276: - nickname blacklist admin add form no longer display the input after a successful submission - nickname blacklist admin add form now strip whitespaces on each nickname - all nicknames are added and checked in lowercase; - a cached dict is used for checking blacklisted nicknames now --- apps/users/admin.py | 8 ++++++++ apps/users/fixtures/users/test_backends.json | 2 +- apps/users/forms.py | 3 ++- apps/users/models.py | 11 +++++++---- apps/users/tests/test_forms.py | 5 +++-- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/apps/users/admin.py b/apps/users/admin.py index 81413de79d..917075054e 100644 --- a/apps/users/admin.py +++ b/apps/users/admin.py @@ -48,10 +48,17 @@ class BlacklistedNicknameAdmin(admin.ModelAdmin): inserted = 0 duplicates = 0 for n in form.cleaned_data['nicknames'].splitlines(): + # check with teh cache + if BlacklistedNickname.blocked(n): + duplicates += 1 + continue + n = n.decode().lower().encode('utf-8') try: BlacklistedNickname.objects.create(nickname=n) inserted += 1 except IntegrityError: + # although unlikely, someone else could have added + # the nickname. # note: unless we manage the transactions manually, # we do lose a primary id here duplicates += 1 @@ -59,6 +66,7 @@ class BlacklistedNicknameAdmin(admin.ModelAdmin): if duplicates: msg += ' %s duplicates were ignored.' % (duplicates) messages.success(request, msg) + form = forms.BlacklistedNicknameAddForm() # Default django admin change list view does not print messages # no redirect for now # return http.HttpResponseRedirect(reverse( diff --git a/apps/users/fixtures/users/test_backends.json b/apps/users/fixtures/users/test_backends.json index 194774f62f..28fb17d468 100644 --- a/apps/users/fixtures/users/test_backends.json +++ b/apps/users/fixtures/users/test_backends.json @@ -150,7 +150,7 @@ "pk": 1, "model": "users.blacklistednickname", "fields": { - "nickname": "IE6Fan", + "nickname": "ie6fan", "modified": "2010-07-21 23:32:05", "created": "2010-07-21 23:32:05" } diff --git a/apps/users/forms.py b/apps/users/forms.py index d17f0bbcb6..8b1659063b 100644 --- a/apps/users/forms.py +++ b/apps/users/forms.py @@ -176,7 +176,8 @@ class BlacklistedNicknameAddForm(forms.Form): if 'nicknames' in data: data['nicknames'] = os.linesep.join( - [s for s in data['nicknames'].splitlines() if s]) + [s.strip() for s in data['nicknames'].splitlines() + if s.strip()]) if 'nicknames' not in data or data['nicknames'] == '': msg = 'Please enter at least one nickname to blacklist.' self._errors['nicknames'] = ErrorList([msg]) diff --git a/apps/users/models.py b/apps/users/models.py index 2a93bf6178..2a2306d016 100644 --- a/apps/users/models.py +++ b/apps/users/models.py @@ -11,6 +11,7 @@ from django.core.mail import send_mail from django.db import models from django.template import Context, loader +import caching.base as caching import commonware.log from tower import ugettext as _ @@ -202,10 +203,12 @@ class BlacklistedNickname(amo.models.ModelBase): @classmethod def blocked(cls, nick): - """Check to see if a nickname is in the blacklist.""" - # Could also cache the entire blacklist and simply check if the - # nickname is in the list here. @TODO? - return cls.uncached.only('nickname').filter(nickname=nick).exists() + """Check to see if a nickname is in the (cached) blacklist.""" + nick = nick.decode().lower().encode('utf-8') + qs = cls.objects.all() + f = lambda: dict(qs.values_list('nickname', 'id')) + blacklist = caching.cached_with(qs, f, 'blocked') + return nick in blacklist class PersonaAuthor(unicode): diff --git a/apps/users/tests/test_forms.py b/apps/users/tests/test_forms.py index 4e4ee0edcd..3597e549c6 100644 --- a/apps/users/tests/test_forms.py +++ b/apps/users/tests/test_forms.py @@ -309,8 +309,9 @@ class TestBlacklistedNicknameAdminAddForm(UserFormBase): def test_add(self): self.client.login(username='testo@example.com', password='foo') url = reverse('admin:users_blacklistednickname_add') - data = {'nicknames': "IE6Fan\nfubar\n\n", } + data = {'nicknames': "IE6Fan\nFubar\n\n fubar \n", } r = self.client.post(url, data) msg = '1 new nicknames added to the blacklist. ' - msg += '1 duplicates were ignored.' + msg += '2 duplicates were ignored.' self.assertContains(r, msg) + self.assertNotContains(r, 'fubar')