Merge lp:~diegosarmentero/ubuntu-sso-client/not-validated-account into lp:ubuntu-sso-client

[Diego Sarmentero]

Fixed: When logging in with an no-yet-validated account, there is no useful message (LP: #851885).

Fixed: When logging in with an no-yet-validated account, there is no useful message (LP: #851885).

[Alejandro J. Cura (alecu)]

nNice branch!

Two small fixes needed:
Typo: "Test for SSOLoginRott" -> "Test for SSOLoginRoot"

CALL_SUCCESS is a constant, so the "if-else" in execute_fake_callback that checks the value of that constant does not make sense to me. (line 93-96 in the diff)
Perhaps it should be changed to just call result_cb, or made an instance attribute instead of a constant.

review: Needs Fixing

[Diego Sarmentero (diegosarmentero)]

> nNice branch!
>
> Two small fixes needed:
> Typo: "Test for SSOLoginRott" -> "Test for SSOLoginRoot"
>
> CALL_SUCCESS is a constant, so the "if-else" in execute_fake_callback that
> checks the value of that constant does not make sense to me. (line 93-96 in
> the diff)
> Perhaps it should be changed to just call result_cb, or made an instance
> attribute instead of a constant.

Thanks for the review!

Fixed!

[lp:~diegosarmentero/ubuntu-sso-client/not-validated-account]

820. By Diego Sarmentero on 2011-11-28

Docstring fixed, unnecessary constant removed.

[Manuel de la Peña (mandel)]

Looks good.

review: Approve

[Alejandro J. Cura (alecu)]

I'm gettting this error, both with "run-tests" and "run-tests -qt":

    test_login_user_not_validated ... Exception in thread Thread-7:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 552, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 505, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/alecu/canonical/ubuntu-sso-client/review_not-validated-account/ubuntu_sso/main/linux.py", line 67, in _in_thread
    error_cb(app_name, except_to_errdict(e))
TypeError: errback() takes at most 2 arguments (3 given)

Traceback (most recent call last):
Failure: twisted.internet.defer.TimeoutError: <ubuntu_sso.main.tests.test_linux.SsoDbusTestCase testMethod=test_login_user_not_validated> (test_login_user_not_validated) still running at 2.0 secs
[ERROR]

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/not-validated-account]

821. By Diego Sarmentero on 2011-12-01

Removing redundant mocker test.

[Alejandro J. Cura (alecu)]

Removing the mocker based test that was replaced with the new test sounds like a good idea.

So, I think this is only stuff remaining:

== Python Lint Notices ==

ubuntu_sso/main/tests/test_common.py:
    49: [C0111, FakeProcessor] Missing docstring
    78: [W0201, SSOLoginRootTestCase.test_login_not_validated_email] Attribute 'fake_callback' defined outside __init__

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/not-validated-account]

822. By Diego Sarmentero on 2011-12-12

Fixing lint issues.

823. By Diego Sarmentero on 2011-12-12

merge

[Alejandro J. Cura (alecu)]

No more issues, thanks for working on this!

review: Approve

=== modified file 'ubuntu_sso/main/__init__.py'
--- ubuntu_sso/main/__init__.py	2011-11-15 16:04:08 +0000
+++ ubuntu_sso/main/__init__.py	2011-12-12 13:56:31 +0000
@@ -1,9 +1,5 @@
 # -*- coding: utf-8 -*-
 #
-# Author: Natalia Bidart <[e-mail]>
-# Author: Alejandro J. Cura <[e-mail]>
-# Author: Manuel de la Pena <[e-mail]>
-#
 # Copyright 2011 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
@@ -110,7 +106,11 @@
                              error_cb(app_name,
                                       except_to_errdict(failure.value)))
             else:
-                not_validated_cb(app_name, email)
+                error_dict = {
+                    'errtype': 'UserNotValidated',
+                    'message': email
+                }
+                not_validated_cb(app_name, error_dict)
         thread_execute(f, app_name, success_cb, error_cb)
 
     def validate_email(self, app_name, email, password, email_token,

=== modified file 'ubuntu_sso/main/tests/test_common.py'
--- ubuntu_sso/main/tests/test_common.py	2011-09-27 14:06:12 +0000
+++ ubuntu_sso/main/tests/test_common.py	2011-12-12 13:56:31 +0000
@@ -2,10 +2,6 @@
 #
 # test_main - tests for ubuntu_sso.main
 #
-# Author: Natalia Bidart <[e-mail]>
-# Author: Alejandro J. Cura <[e-mail]>
-# Author: Manuel de la Pena <[e-mail]>
-#
 # Copyright 2009-2010 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
@@ -21,16 +17,81 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Tests share by diff platforms."""
 
-from unittest import TestCase
+from twisted.trial.unittest import TestCase
+from twisted.internet.defer import inlineCallbacks
 
 from mocker import MockerTestCase, MATCH
 
+from ubuntu_sso import main
 from ubuntu_sso.main import (
     CredentialsManagement,
     SSOLogin,
+    SSOLoginRoot,
 )
 
 
+class FakeCallbacks(object):
+
+    """Fake callbacks class."""
+
+    def __init__(self):
+        self.args = None
+
+    def result_cb(self, *args):
+        """Fake result callbacks function."""
+        self.args = args
+
+    def error_cb(self, *args):
+        """Fake error callbacks function."""
+        self.args = args
+
+
+class FakeProcessor(object):
+
+    """Fake Processor."""
+
+    def __init__(self, *args, **kwargs):
+        self.args = args
+        self.kwargs = kwargs
+        self.validated = True
+
+    def is_validated(self, credentials):
+        """Fake is validated function for Account Processor."""
+        return self.validated
+
+
+def execute_fake_success_callback(function, app_name, success_cb, error_cb):
+    """Fake execute callback function."""
+    success_cb(app_name, object())
+
+
+class SSOLoginRootTestCase(TestCase):
+
+    """Test for SSOLoginRoot."""
+
+    @inlineCallbacks
+    def setUp(self):
+        yield super(SSOLoginRootTestCase, self).setUp()
+        self.root = SSOLoginRoot(FakeProcessor)
+        self.patch(main, "thread_execute", execute_fake_success_callback)
+
+    def test_login_not_validated_email(self):
+        """Test Login function with an email not validated."""
+        fake_callback = FakeCallbacks()
+        self.patch(self.root.processor, "validated", False)
+        app_name = 'app_name'
+        email = '[e-mail]'
+        password = 'password'
+        self.root.login(app_name, email, password,
+            fake_callback.result_cb, fake_callback.error_cb,
+            fake_callback.error_cb)
+        self.assertEqual(fake_callback.args[0], 'app_name')
+        self.assertEqual(fake_callback.args[1]['errtype'],
+            'UserNotValidated')
+        self.assertEqual(fake_callback.args[1]['message'],
+            email)
+
+
 class SSOLoginMockedTestCase(MockerTestCase):
     """Test that the call are relied correctly."""
 
@@ -120,9 +181,10 @@
 class CredentialsManagementMockedTestCase(MockerTestCase, TestCase):
     """Test that the call are relied correctly."""
 
+    @inlineCallbacks
     def setUp(self):
         """Setup tests."""
-        super(CredentialsManagementMockedTestCase, self).setUp()
+        yield super(CredentialsManagementMockedTestCase, self).setUp()
         self.root = self.mocker.mock()
         self.cred = CredentialsManagement(None, None)
         self.cred.root = self.root

=== modified file 'ubuntu_sso/main/tests/test_linux.py'
--- ubuntu_sso/main/tests/test_linux.py	2011-11-16 12:14:00 +0000
+++ ubuntu_sso/main/tests/test_linux.py	2011-12-12 13:56:31 +0000
@@ -238,32 +238,6 @@
         client.login(APP_NAME, EMAIL, PASSWORD)
         return d
 
-    def test_login_user_not_validated(self):
-        """Test that the login sends EmailNotValidated signal."""
-        d = defer.Deferred()
-        processor = self.create_mock_processor()
-        processor.login(EMAIL, PASSWORD, TOKEN_NAME)
-        self.mocker.result(TOKEN)
-        processor.is_validated(TOKEN)
-        self.mocker.result(False)
-        self.patch(ubuntu_sso.main.linux, "blocking", fake_ok_blocking)
-        self.mocker.replay()
-
-        def verify(app_name, email):
-            """The actual test."""
-            self.assertEqual(app_name, APP_NAME)
-            self.assertEqual(email, EMAIL)
-            self.assertFalse(self.keyring_was_set, "Keyring should not be set")
-            d.callback("Ok")
-
-        client = SSOLogin(self.mockbusname,
-                          sso_login_processor_class=self.mockprocessorclass)
-        self.patch(client, "LoggedIn", d.errback)
-        self.patch(client, "LoginError", d.errback)
-        self.patch(client, "UserNotValidated", verify)
-        client.login(APP_NAME, EMAIL, PASSWORD)
-        return d
-
     def test_login_error_get_token_name(self):
         """The login method fails as expected when get_token_name fails."""
         d = defer.Deferred()

=== modified file 'ubuntu_sso/qt/controllers.py'
--- ubuntu_sso/qt/controllers.py	2011-11-11 20:34:57 +0000
+++ ubuntu_sso/qt/controllers.py	2011-12-12 13:56:31 +0000
@@ -233,8 +233,16 @@
         # let the user know
         logger.error('Got error when login %s, error: %s',
                      self.view.wizard().app_name, error)
-        self.message_box.critical(_build_general_error_message(error),
-            self.view)
+        if isinstance(error, collections.Mapping) and \
+        error.get('errtype', None) == 'UserNotValidated':
+            self.view.setField('email_address', self.view.ui.email_edit.text())
+            self.view.setField('password', self.view.ui.password_edit.text())
+            app_name = self.view.wizard().app_name
+            self.view.wizard().registrationIncomplete.emit(
+                app_name, error['message'])
+        else:
+            self.message_box.critical(_build_general_error_message(error),
+                self.view)
 
     def on_logged_in(self, app_name, result):
         """We managed to log in."""