Merge lp:~ralsina/ubuntu-sso-client/validate-harder into lp:ubuntu-sso-client

[Roberto Alsina]

Implement enabling/disabling of the "submit" button according to the input data's validation in 3 pages:

* Current User Sign In
* Reset Password
* Confirm Email

Implement enabling/disabling of the "submit" button according to the input data's validation in 3 pages:

* Current User Sign In
* Reset Password
* Confirm Email

[lp:~ralsina/ubuntu-sso-client/validate-harder]

759. By Roberto Alsina on 2011-08-04

rename test classes

760. By Roberto Alsina on 2011-08-04

missing .

[Natalia Bidart (nataliabidart)]

For the validation to be correct, I think this method:

    def is_correct_password_confirmation(self, password):
        """Return if the password is correct."""
        return self.view.ui.password_line_edit.text() == password

should be:

    def is_correct_password_confirmation(self, password):
        """Return if the password is correct."""
        return unicode(self.view.ui.password_line_edit.text()) == password

Also, all docstrings need to end with a dot.

Question, shouldn't these checks in our code assert over a specific validation callback instead of any callable?

self.view.ui.password_edit.textChanged.connect(MATCH(callable))

Great work on all the tests added!

review: Needs Fixing

[lp:~ralsina/ubuntu-sso-client/validate-harder]

761. By Roberto Alsina on 2011-08-04

missing .s

[Roberto Alsina (ralsina)]

> For the validation to be correct, I think this method:
>
> def is_correct_password_confirmation(self, password):
> """Return if the password is correct."""
> return self.view.ui.password_line_edit.text() == password
>
> should be:
>
> def is_correct_password_confirmation(self, password):
> """Return if the password is correct."""
> return unicode(self.view.ui.password_line_edit.text()) == password

Those two are equivalent because QStrings and unicode strings comparison works.
Added it anyway to be explicit.

> Also, all docstrings need to end with a dot.

Fixed

> Question, shouldn't these checks in our code assert over a specific validation
> callback instead of any callable?
>
> self.view.ui.password_edit.textChanged.connect(MATCH(callable))

I agree, but it's the way it's done in all the other tests, and improving the mocker-based tests is
not exactly my forte ;-)

> Great work on all the tests added!

Thanks!

[lp:~ralsina/ubuntu-sso-client/validate-harder]

762. By Roberto Alsina on 2011-08-04

fixes

763. By Roberto Alsina on 2011-08-04

Lint fixes

[Natalia Bidart (nataliabidart)]

There is a little conflict in

Text conflict in ubuntu_sso/qt/controllers.py
1 conflicts encountered.

[Natalia Bidart (nataliabidart)]

Other than that, the branch looks good!

review: Approve

[lp:~ralsina/ubuntu-sso-client/validate-harder]

764. By Roberto Alsina on 2011-08-05

resolve conflict

=== modified file 'data/qt/current_user_sign_in.ui'
--- data/qt/current_user_sign_in.ui	2011-08-04 15:25:42 +0000
+++ data/qt/current_user_sign_in.ui	2011-08-05 14:40:27 +0000
@@ -122,6 +122,9 @@
               </item>
               <item>
                <widget class="QPushButton" name="sign_in_button">
+                <property name="enabled">
+                 <bool>false</bool>
+                </property>
                 <property name="text">
                  <string>Sign In</string>
                 </property>

=== modified file 'data/qt/forgotten_password.ui'
--- data/qt/forgotten_password.ui	2011-08-04 15:25:42 +0000
+++ data/qt/forgotten_password.ui	2011-08-05 14:40:27 +0000
@@ -109,6 +109,9 @@
      </item>
      <item>
       <widget class="QPushButton" name="send_button">
+       <property name="enabled">
+        <bool>false</bool>
+       </property>
        <property name="text">
         <string/>
        </property>

=== modified file 'data/qt/reset_password.ui'
--- data/qt/reset_password.ui	2011-08-04 15:25:42 +0000
+++ data/qt/reset_password.ui	2011-08-05 14:40:27 +0000
@@ -65,6 +65,9 @@
          </item>
          <item>
           <widget class="QPushButton" name="reset_password_button">
+           <property name="enabled">
+            <bool>false</bool>
+           </property>
            <property name="text">
             <string/>
            </property>

=== modified file 'ubuntu_sso/qt/controllers.py'
--- ubuntu_sso/qt/controllers.py	2011-08-04 16:24:35 +0000
+++ ubuntu_sso/qt/controllers.py	2011-08-05 14:40:27 +0000
@@ -194,6 +194,17 @@
         self.backend.on_logged_in_cb = self.on_logged_in
         self.view.ui.forgot_password_label.linkActivated.connect(
                                                     self.on_forgotten_password)
+        self.view.ui.email_edit.textChanged.connect(self._validate)
+        self.view.ui.password_edit.textChanged.connect(self._validate)
+
+    def _validate(self):
+        """Perform input validation."""
+        valid = True
+        if not is_correct_email(unicode(self.view.ui.email_edit.text())):
+            valid = False
+        elif not unicode(self.view.ui.password_edit.text()):
+            valid = False
+        self.view.ui.sign_in_button.setEnabled(valid)
 
     def login(self):
         """Perform the login using the self.backend."""
@@ -465,12 +476,12 @@
     def is_correct_email_confirmation(self, email_address):
         """Return that the email is the same."""
         logger.debug('SetUpAccountController.is_correct_email_confirmation')
-        return self.view.ui.email_edit.text() == email_address
+        return unicode(self.view.ui.email_edit.text()) == email_address
 
     def is_correct_password_confirmation(self, password):
         """Return that the passwords are correct."""
         logger.debug('SetUpAccountController.is_correct_password_confirmation')
-        return self.view.ui.password_edit.text() == password
+        return unicode(self.view.ui.password_edit.text()) == password
 
     #pylint: disable=C0103
     @inlineCallbacks
@@ -621,8 +632,12 @@
 class ForgottenPasswordController(BackendController):
     """Controller used to deal with the forgotten pwd page."""
 
-    def __init__(self, title='', subtitle=''):
+    def __init__(self, message_box=None, title='', subtitle=''):
         """Create a new instance."""
+        super(ForgottenPasswordController, self).__init__()
+        if message_box is None:
+            message_box = QMessageBox
+        self.message_box = message_box
         super(ForgottenPasswordController, self).__init__(title, subtitle)
 
     def _register_fields(self):
@@ -647,6 +662,7 @@
 
     def _connect_ui(self):
         """Connect the diff signals from the Ui."""
+        self.view.email_address_line_edit.textChanged.connect(self._validate)
         self.view.send_button.clicked.connect(
                             lambda: self.backend.request_password_reset_token(
                                                 self.view.wizard().app_name,
@@ -657,6 +673,11 @@
                                     self.on_password_reset_token_sent()
         self.backend.on_password_reset_error_cb = self.on_password_reset_error
 
+    def _validate(self):
+        """Validate that we have an email."""
+        email = unicode(self.view.email_address_line_edit.text())
+        self.view.send_button.setEnabled(is_correct_email(email))
+
     def on_try_again(self):
         """Set back the widget to the initial state."""
         self.view.error_label.setVisible(False)
@@ -725,6 +746,25 @@
         self.backend.on_password_changed_cb = self.on_password_changed
         self.backend.on_password_change_error_cb = \
                                                 self.on_password_change_error
+        self.view.ui.reset_code_line_edit.textChanged.connect(self._validate)
+        self.view.ui.password_line_edit.textChanged.connect(self._validate)
+        self.view.ui.confirm_password_line_edit.textChanged.connect(
+            self._validate)
+
+    def _validate(self):
+        """Enable the submit button if data is valid."""
+        enabled = True
+        code = unicode(self.view.ui.reset_code_line_edit.text())
+        password = unicode(self.view.ui.password_line_edit.text())
+        confirm_password = unicode(
+            self.view.ui.confirm_password_line_edit.text())
+        if not is_min_required_password(password):
+            enabled = False
+        elif not self.is_correct_password_confirmation(confirm_password):
+            enabled = False
+        elif not code:
+            enabled = False
+        self.view.ui.reset_password_button.setEnabled(enabled)
 
     def _add_line_edits_validations(self):
         """Add the validations to be use by the line edits."""
@@ -756,7 +796,7 @@
 
     def is_correct_password_confirmation(self, password):
         """Return if the password is correct."""
-        return self.view.ui.password_line_edit.text() == password
+        return unicode(self.view.ui.password_line_edit.text()) == password
 
     #pylint: disable=C0103
     @inlineCallbacks

=== modified file 'ubuntu_sso/qt/tests/test_windows.py'
--- ubuntu_sso/qt/tests/test_windows.py	2011-08-04 15:25:42 +0000
+++ ubuntu_sso/qt/tests/test_windows.py	2011-08-05 14:40:27 +0000
@@ -70,7 +70,7 @@
     is_correct_email)
 
 #ignore the comon mocker issues with lint
-# pylint: disable=W0212,W0104,W0106
+# pylint: disable=W0212,W0104,W0106,E1103
 
 
 class ChooseSignInControllerTestCase(MockerTestCase):
@@ -157,6 +157,8 @@
         self.backend.on_logged_in_cb = MATCH(callable)
         self.view.ui.forgot_password_label.linkActivated.connect(
                                                         MATCH(callable))
+        self.view.ui.email_edit.textChanged.connect(MATCH(callable))
+        self.view.ui.password_edit.textChanged.connect(MATCH(callable))
         self.mocker.replay()
         self.controller._connect_ui()
 
@@ -167,6 +169,94 @@
         self.assertEqual(self.controller._subtitle, 'the subtitle')
 
 
+class FakeLineEdit(object):
+
+    """A fake QLineEdit."""
+
+    def __init__(self, *args):
+        """Initialize."""
+        self._text = u""
+
+    def text(self):
+        """Save text."""
+        return self._text
+
+    # setText is inherited
+    # pylint: disable=C0103
+    def setText(self, text):
+        """Return saved text."""
+        self._text = text
+
+
+class FakeCurrentUserPage(object):
+
+    """Fake CurrentuserPage."""
+
+    def __init__(self, *args):
+        """Initialize."""
+        self.ui = self
+        self.ui.email_edit = FakeLineEdit()
+        self.ui.password_edit = FakeLineEdit()
+        self.sign_in_button = self
+        self._enabled = False
+
+    # setEnabled is inherited
+    # pylint: disable=C0103
+    def setEnabled(self, value):
+        """Fake setEnabled."""
+        self._enabled = value
+
+    def enabled(self):
+        """Fake enabled."""
+        return self._enabled
+
+
+class CurrentUserControllerValidationTest(TestCase):
+
+    """Tests for CurrentUserController, but without Mocker."""
+
+    def setUp(self):
+        """Setup test."""
+        super(CurrentUserControllerValidationTest, self).setUp()
+        self.message_box = FakeMessageBox()
+        self.controller = CurrentUserController(
+            message_box=self.message_box)
+        self.controller.view = FakeCurrentUserPage()
+        self._called = False
+
+    def _set_called(self, *args, **kwargs):
+        """Store 'args' and 'kwargs' for test assertions."""
+        self._called = (args, kwargs)
+
+    def test_valid(self):
+        """Enable the button with a valid email/password."""
+        self.controller.view.email_edit.setText("a@b")
+        self.controller.view.password_edit.setText("pass")
+        self.controller._validate()
+        self.assertTrue(self.controller.view.sign_in_button.enabled())
+
+    def test_invalid_email(self):
+        """The submit button should be disabled with an invalid email."""
+        self.controller.view.email_edit.setText("ab")
+        self.controller.view.password_edit.setText("pass")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.sign_in_button.enabled())
+
+    def test_invalid_password(self):
+        """The submit button should be disabled with an invalid password."""
+        self.controller.view.email_edit.setText("a@b")
+        self.controller.view.password_edit.setText("")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.sign_in_button.enabled())
+
+    def test_invalid_both(self):
+        """The submit button should be disabled with invalid data."""
+        self.controller.view.email_edit.setText("ab")
+        self.controller.view.password_edit.setText("")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.sign_in_button.enabled())
+
+
 class SetUpAccountControllerTestCase(MockerTestCase):
     """test the controller used to setup a new account."""
 
@@ -446,7 +536,7 @@
 
 class FakeView(object):
 
-    """A fake view"""
+    """A fake view."""
 
     app_name = "TestApp"
 
@@ -455,12 +545,13 @@
         return self
 
 
-class SetupAccountControllerTest2(TestCase):
+class SetupAccountControllerValidationTest(TestCase):
 
-    """Tests for SetupAccountController, but without Mocker"""
+    """Tests for SetupAccountController, but without Mocker."""
 
     def setUp(self):
-        super(SetupAccountControllerTest2, self).setUp()
+        """Set the different tests."""
+        super(SetupAccountControllerValidationTest, self).setUp()
         self.message_box = FakeMessageBox()
         self.controller = SetUpAccountController(message_box=self.message_box)
         self.patch(self.controller, '_refresh_captcha', self._set_called)
@@ -591,11 +682,12 @@
         self.controller.validate_email()
 
 
-class EmailVerificationControllerTestCase2(TestCase):
-    """Tests for EmailVerificationController, but without Mocker"""
+class EmailVerificationControllerValidationTestCase(TestCase):
+    """Tests for EmailVerificationController, but without Mocker."""
 
     def setUp(self):
-        super(EmailVerificationControllerTestCase2, self).setUp()
+        """Set the different tests."""
+        super(EmailVerificationControllerValidationTestCase, self).setUp()
         self.message_box = FakeMessageBox()
         self.controller = EmailVerificationController(
             message_box=self.message_box)
@@ -768,6 +860,75 @@
         self.controller.setupUi(self.view)
 
 
+class FakeForgottenPasswordPage(FakeView):
+    """Fake the page."""
+
+    def __init__(self):
+        self.email_address_line_edit = self
+        self.send_button = self
+        self._text = ""
+        self._enabled = False
+        super(FakeForgottenPasswordPage, self).__init__()
+
+    def text(self):
+        """Return text."""
+        return self._text
+
+    # setText, setEnabled are inherited
+    # pylint: disable=C0103
+    def setText(self, text):
+        """Save text."""
+        self._text = text
+
+    def setEnabled(self, value):
+        """Fake setEnabled."""
+        self._enabled = value
+
+    def enabled(self):
+        """Fake enabled."""
+        return self._enabled
+
+
+class ForgottenPasswordControllerValidationTest(TestCase):
+
+    """Tests for ForgottenPasswordController, but without Mocker."""
+
+    def setUp(self):
+        """Set the different tests."""
+        super(ForgottenPasswordControllerValidationTest, self).setUp()
+        self.message_box = FakeMessageBox()
+        self.controller = ForgottenPasswordController(
+            message_box=self.message_box)
+        self.controller.view = FakeForgottenPasswordPage()
+        self._called = False
+
+    def _set_called(self, *args, **kwargs):
+        """Store 'args' and 'kwargs' for test assertions."""
+        self._called = (args, kwargs)
+
+    def test_valid(self):
+        """The submit button should be enabled with a valid email."""
+        self.controller.view.email_address_line_edit.setText("a@b")
+        self.assertNotEqual(unicode(
+            self.controller.view.email_address_line_edit.text()), u"")
+        self.controller._validate()
+        self.assertTrue(self.controller.view.send_button.enabled())
+
+    def test_invalid(self):
+        """The submit button should be disabled with an invalid email."""
+        self.controller.view.email_address_line_edit.setText("ab")
+        self.assertNotEqual(
+            unicode(self.controller.view.email_address_line_edit.text()), u"")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.send_button.enabled())
+
+    def test_empty(self):
+        """The submit button should be disabled without email."""
+        self.assertEqual(
+            unicode(self.controller.view.email_address_line_edit.text()), u"")
+        self.assertFalse(self.controller.view.send_button.enabled())
+
+
 class ForgottenPasswordControllerTestCase(MockerTestCase):
     """Test the controller of the fogotten password page."""
 
@@ -814,6 +975,7 @@
 
     def test_connect_ui(self):
         """Test that the correct ui signals are connected."""
+        self.view.email_address_line_edit.textChanged.connect(MATCH(callable))
         self.view.send_button.clicked.connect(MATCH(callable))
         self.view.try_again_button.clicked.connect(
                                                   self.controller.on_try_again)
@@ -892,6 +1054,13 @@
                                         self.controller.on_password_changed
         self.backend.on_password_change_error_cb = \
                                     self.controller.on_password_change_error
+        self.view.ui.reset_code_line_edit.textChanged.connect(
+            MATCH(callable))
+        self.view.ui.password_line_edit.textChanged.connect(
+            MATCH(callable))
+        self.view.ui.confirm_password_line_edit.textChanged.connect(
+            MATCH(callable))
+
         self.mocker.replay()
         self.controller._connect_ui()
 
@@ -947,3 +1116,75 @@
         self.mocker.replay()
         self.assertFalse(self.controller.is_correct_password_confirmation(
                                                                     password))
+
+
+class FakeResetPasswordPage(object):
+
+    """Fake ResetPasswordPage."""
+
+    def __init__(self, *args):
+        """Initialize."""
+        self.ui = self
+        self.ui.reset_code_line_edit = FakeLineEdit()
+        self.ui.password_line_edit = FakeLineEdit()
+        self.ui.confirm_password_line_edit = FakeLineEdit()
+        self.reset_password_button = self
+        self._enabled = 9  # Intentionally wrong.
+
+    # setEnabled is inherited
+    # pylint: disable=C0103
+    def setEnabled(self, value):
+        """Fake setEnabled."""
+        self._enabled = value
+
+    def enabled(self):
+        """Fake enabled."""
+        return self._enabled
+
+
+class ResetPasswordControllerValidationTest(TestCase):
+
+    """Tests for ResetPasswordController, but without Mocker."""
+
+    def setUp(self):
+        """Setup test."""
+        super(ResetPasswordControllerValidationTest, self).setUp()
+        self.controller = ResetPasswordController()
+        self.controller.view = FakeResetPasswordPage()
+        self._called = False
+
+    def _set_called(self, *args, **kwargs):
+        """Store 'args' and 'kwargs' for test assertions."""
+        self._called = (args, kwargs)
+
+    def test_valid(self):
+        """Enable the button with valid data."""
+        self.controller.view.reset_code_line_edit.setText("ABCD")
+        self.controller.view.password_line_edit.setText("1234567A")
+        self.controller.view.confirm_password_line_edit.setText("1234567A")
+        self.controller._validate()
+        self.assertTrue(self.controller.view.reset_password_button.enabled())
+
+    def test_invalid_code(self):
+        """Disable the button with an invalid code."""
+        self.controller.view.reset_code_line_edit.setText("")
+        self.controller.view.password_line_edit.setText("1234567A")
+        self.controller.view.confirm_password_line_edit.setText("1234567A")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.reset_password_button.enabled())
+
+    def test_invalid_password(self):
+        """Disable the button with an invalid password."""
+        self.controller.view.reset_code_line_edit.setText("")
+        self.controller.view.password_line_edit.setText("1234567")
+        self.controller.view.confirm_password_line_edit.setText("1234567")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.reset_password_button.enabled())
+
+    def test_invalid_confirm(self):
+        """Disable the button with an invalid password confirm."""
+        self.controller.view.reset_code_line_edit.setText("")
+        self.controller.view.password_line_edit.setText("1234567A")
+        self.controller.view.confirm_password_line_edit.setText("1234567")
+        self.controller._validate()
+        self.assertFalse(self.controller.view.reset_password_button.enabled())