Merge lp:~matiasb/canonical-identity-provider/add-verified-manager into lp:canonical-identity-provider/release

[Matias Bordese]

Added verified queryset option to Account and EmailAddress managers.

Added verified queryset option to Account and EmailAddress managers.
Added model-utils requirement.

[Ricardo Kirkner (ricardokirkner)]

LGTM, except that the new dependency needs to be added for production. As this dependency is not packaged in Ubuntu yet, we need to add it as a sourcedep.

review: Needs Fixing

[Matias Bordese (matiasb)]

> LGTM, except that the new dependency needs to be added for production. As this
> dependency is not packaged in Ubuntu yet, we need to add it as a sourcedep.

Added django-model-utils as sourcedep.
Just notice that I needed to fall back to previous stable version (1.1, revno 110) because there was a bug in 1.2 that breaks our code (solved in master ~17 days ago, https://github.com/carljm/django-model-utils/issues/24).

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

=== modified file 'identityprovider/models/account.py'
--- identityprovider/models/account.py	2013-02-27 21:03:58 +0000
+++ identityprovider/models/account.py	2013-02-28 21:28:19 +0000
@@ -13,6 +13,7 @@
 from django.db import models
 from django.db.models.query import EmptyQuerySet
 from django.utils.translation import ugettext_lazy as _
+from model_utils.managers import PassThroughManager
 from oauth_backend.models import Consumer
 from south.modelsinspector import add_introspection_rules
 
@@ -44,7 +45,14 @@
 ]
 
 
-class AccountManager(models.Manager):
+class AccountQuerySet(models.query.QuerySet):
+    def verified(self):
+        return self.filter(
+            emailaddress__status__in=(EmailStatus.VALIDATED,
+                                      EmailStatus.PREFERRED)).distinct()
+
+
+class AccountManager(PassThroughManager):
     """Extend the default manager.
 
     Add a method that takes care of creating an Account, and the necessary
@@ -109,7 +117,7 @@
         account = get_account(user)
         if account is None:
             try:
-                account = Account.objects.get(openid_identifier=user.username)
+                account = self.get(openid_identifier=user.username)
             except Account.DoesNotExist:
                 pass
         return account
@@ -149,7 +157,7 @@
     twofactor_attempts = models.SmallIntegerField(default=0, null=True)
     warn_about_backup_device = models.BooleanField(default=True)
 
-    objects = AccountManager()
+    objects = AccountManager.for_queryset_class(AccountQuerySet)()
 
     class Meta:
         app_label = 'identityprovider'

=== modified file 'identityprovider/models/emailaddress.py'
--- identityprovider/models/emailaddress.py	2013-02-18 16:36:33 +0000
+++ identityprovider/models/emailaddress.py	2013-02-28 21:28:19 +0000
@@ -10,6 +10,7 @@
 from django.db import models
 from django.template.defaultfilters import force_escape
 from django.utils.translation import ugettext_lazy as _
+from model_utils.managers import PassThroughManager
 
 from identityprovider.models import Account
 from identityprovider.models.const import EmailStatus
@@ -24,7 +25,13 @@
                                        re.IGNORECASE)
 
 
-class EmailAddressManager(models.Manager):
+class EmailAddressQuerySet(models.query.QuerySet):
+    def verified(self):
+        return self.filter(
+            status__in=(EmailStatus.VALIDATED, EmailStatus.PREFERRED))
+
+
+class EmailAddressManager(PassThroughManager):
 
     def _generate_email_from_phone_id(self, phone_id):
         # replace chars not validated by django validate_email by #
@@ -54,7 +61,7 @@
     account = models.ForeignKey(
         Account, db_column='account', blank=True, null=True)
 
-    objects = EmailAddressManager()
+    objects = EmailAddressManager.for_queryset_class(EmailAddressQuerySet)()
 
     class Meta:
         app_label = 'identityprovider'

=== modified file 'identityprovider/tests/unit/test_models_account.py'
--- identityprovider/tests/unit/test_models_account.py	2013-02-27 21:03:58 +0000
+++ identityprovider/tests/unit/test_models_account.py	2013-02-28 21:28:19 +0000
@@ -25,6 +25,7 @@
 from identityprovider.models.account import (
     Account,
     AccountPassword,
+    AccountQuerySet,
 )
 from identityprovider.models import account as a
 from identityprovider.models.emailaddress import EmailAddress
@@ -54,6 +55,19 @@
         return self.value
 
 
+class AccountQuerySetTestCase(SSOBaseTestCase):
+
+    def setUp(self):
+        super(AccountQuerySetTestCase, self).setUp()
+        self.qs = AccountQuerySet(model=Account)
+
+    def test_verified_list_only_verified_accounts(self):
+        accounts = self.qs.verified()
+        for account in accounts:
+            emails = account.emailaddress_set.all()
+            self.assertTrue(any([email.is_verified() for email in emails]))
+
+
 class AccountManagerTestCase(SSOBaseTestCase):
 
     def test_objects_get_by_email_fails(self):
@@ -179,6 +193,16 @@
             else:
                 self.assertIsNone(result)
 
+    def test_verified_list_only_verified_accounts(self):
+        accounts = Account.objects.verified()
+        for account in accounts:
+            emails = account.emailaddress_set.all()
+            self.assertTrue(any([email.is_verified() for email in emails]))
+
+    def test_verified_no_duplicates(self):
+        accounts = Account.objects.verified()
+        self.assertEqual(accounts.count(), accounts.distinct().count())
+
 
 class AccountTestCase(SSOBaseTestCase):
 

=== modified file 'identityprovider/tests/unit/test_models_emailaddress.py'
--- identityprovider/tests/unit/test_models_emailaddress.py	2013-02-14 20:55:54 +0000
+++ identityprovider/tests/unit/test_models_emailaddress.py	2013-02-28 21:28:19 +0000
@@ -2,10 +2,25 @@
 
 from identityprovider.models import Account, EmailAddress
 from identityprovider.models.const import EmailStatus
-from identityprovider.models.emailaddress import PHONE_EMAIL_DOMAIN
+from identityprovider.models.emailaddress import (
+    PHONE_EMAIL_DOMAIN,
+    EmailAddressQuerySet,
+)
 from identityprovider.tests.utils import SSOBaseTestCase
 
 
+class EmailAddressQuerySetTestCase(SSOBaseTestCase):
+
+    def setUp(self):
+        super(EmailAddressQuerySetTestCase, self).setUp()
+        self.qs = EmailAddressQuerySet(model=EmailAddress)
+
+    def test_verified_list_only_verified_emails(self):
+        emails = self.qs.verified()
+        for email in emails:
+            self.assertTrue(email.is_verified())
+
+
 class EmailAddressManagerTestCase(SSOBaseTestCase):
     fixtures = ['test']
 
@@ -29,6 +44,11 @@
             EmailAddress.DoesNotExist,
             EmailAddress.objects.get_from_phone_id, 'tel:+123')
 
+    def test_verified(self):
+        emails = EmailAddress.objects.verified()
+        for email in emails:
+            self.assertTrue(email.is_verified())
+
 
 class EmailAddressTestCase(SSOBaseTestCase):
     fixtures = ['test']

=== added symlink 'lib/model_utils'
=== target is u'../branches/django-model-utils/model_utils'
=== modified file 'requirements/config-manager.txt'
--- requirements/config-manager.txt	2013-02-22 12:16:54 +0000
+++ requirements/config-manager.txt	2013-02-28 21:28:19 +0000
@@ -4,6 +4,7 @@
 ./branches/django-pgtools        bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/django-pgtools/trunk;revno=7
 
 # Upstream dependencies
+./branches/django-model-utils    bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/django-model-utils/stable;revno=110
 ./branches/django-modeldict      bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/django-modeldict/stable;revno=1.4.1
 ./branches/oauthlib              bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/oauthlib/stable;revno=0.3.7
 ./branches/requests              bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/requests/stable;revno=v1.1.0