Merge lp:~forrest-aldridge/methanal/verified-password-input into lp:methanal

Proposed by Forrest Aldridge
Status: Rejected
Rejected by: Tristan Seligmann
Proposed branch: lp:~forrest-aldridge/methanal/verified-password-input
Merge into: lp:methanal
Diff against target: 136 lines
4 files modified
methanal/js/Methanal/Tests/TestView.js (+43/-0)
methanal/js/Methanal/View.js (+35/-0)
methanal/themes/methanal-base/methanal-verified-password-input.html (+15/-0)
methanal/view.py (+9/-0)
To merge this branch: bzr merge lp:~forrest-aldridge/methanal/verified-password-input
Reviewer Review Type Date Requested Status
Jonathan Jacobs Approve
Review via email: mp+13130@code.launchpad.net

This proposal supersedes a proposal from 2009-10-08.

To post a comment you must log in.
Revision history for this message
Jonathan Jacobs (jjacobs) wrote : Posted in a previous version of this proposal

  1. Some empty lines have trailing whitespace, please remove this.
  2. Can you align the second line of the 3rd point of "test_inputValidation"'s docstring with the first line?
  3. "test_inputValidation" tests a bunch of different cases, these probably don't need to be split into separate methods but if you could add a comment to indicate which condition is being tested, that would help.
  4. Some lines (excluding subclass lines) are wider than 80 columns.
  5. Please use braces for *ALL* blocks, regardless of the length of their body. "VerifiedPasswordInput.baseValidator" is one specific case I'm looking at.
  6. Replacing the template for things that inherit from "TextInput" makes me twitch, I really don't like this. Unfortunately in this case it doesn't seem to be easily avoidable. The template "methanal-verified-password-input.html" is missing a number of elements that TextInput could be expecting, can you copy "methanal-text-input.html" and modify whatever needs to be?

review: Needs Fixing
Revision history for this message
Jonathan Jacobs (jjacobs) wrote :

This looks good.

I've branched this to add some additional flexibility to the strength checking functionality.

lp:~jjacobs/methanal/verified-password-input

review: Approve

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'methanal/js/Methanal/Tests/TestView.js'
--- methanal/js/Methanal/Tests/TestView.js 2009-10-04 11:18:09 +0000
+++ methanal/js/Methanal/Tests/TestView.js 2009-10-09 15:30:24 +0000
@@ -754,6 +754,49 @@
754754
755755
756/**756/**
757 * Tests for L{Methanal.View.VerifiedPasswordInput}.
758 */
759Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestVerifiedPasswordInput').methods(
760 function setUp(self) {
761 self.controlType = Methanal.View.VerifiedPasswordInput;
762 },
763
764
765 function createControl(self, args) {
766 var control = Methanal.Tests.TestView.TestVerifiedPasswordInput.upcall(
767 self, 'createControl', args);
768 Methanal.Tests.TestView.makeWidgetChildNode(
769 control, 'input', 'confirmPassword');
770 return control;
771 },
772
773
774 /**
775 * Validation will fail under the following conditions:
776 * 1. The input and confirmPasswordNode node values don't match.
777 * 2. If either of the above node values have no length (are blank).
778 * 3. If the password value is not strong. By default, 'strong' means
779 * more than 4 characters long.
780 */
781 function test_inputValidation(self) {
782 self.testControl({value: null},
783 function (control) {
784 // Test condition 1
785 control._confirmPasswordNode.value = 'no match';
786 self.assertInvalidInput(control, 'match');
787 control._confirmPasswordNode.value = 'match';
788 self.assertValidInput(control, 'match');
789 // Test condition 2
790 control._confirmPasswordNode.value = '';
791 self.assertInvalidInput(control, '');
792 // Test condition 3
793 self.assertInvalidInput(control, '123');
794 });
795 });
796
797
798
799/**
757 * Tests for L{Methanal.View.InputContainer}.800 * Tests for L{Methanal.View.InputContainer}.
758 */801 */
759Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods(802Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods(
760803
=== modified file 'methanal/js/Methanal/View.js'
--- methanal/js/Methanal/View.js 2009-10-05 00:15:43 +0000
+++ methanal/js/Methanal/View.js 2009-10-09 15:30:24 +0000
@@ -1793,3 +1793,38 @@
1793 return 'Percentage values must be between 0% and 100%'1793 return 'Percentage values must be between 0% and 100%'
1794 }1794 }
1795 });1795 });
1796
1797
1798
1799/**
1800 * Password input with a verification field and strength checking.
1801 */
1802Methanal.View.TextInput.subclass(Methanal.View, 'VerifiedPasswordInput').methods(
1803 function nodeInserted(self) {
1804 self._confirmPasswordNode = self.nodeById('confirmPassword');
1805 Methanal.View.VerifiedPasswordInput.upcall(self, 'nodeInserted');
1806 },
1807
1808
1809 /**
1810 * Override this method to change the definition of a 'strong' password.
1811 */
1812 function passwordIsStrong(self, password) {
1813 return password.length > 4;
1814 },
1815
1816
1817 /**
1818 * This default validator ensures that the password is strong and that
1819 * the password given in both fields have length > 0 and match exactly.
1820 */
1821 function baseValidator(self, value) {
1822 if (value !== self._confirmPasswordNode.value || value === null ||
1823 self._confirmPasswordNode.value === null) {
1824 return 'Passwords do not match.';
1825 }
1826
1827 if (!self.passwordIsStrong(value)) {
1828 return 'Password is too weak.';
1829 }
1830 });
17961831
=== added file 'methanal/themes/methanal-base/methanal-verified-password-input.html'
--- methanal/themes/methanal-base/methanal-verified-password-input.html 1970-01-01 00:00:00 +0000
+++ methanal/themes/methanal-base/methanal-verified-password-input.html 2009-10-09 15:30:24 +0000
@@ -0,0 +1,15 @@
1<div xmlns:nevow="http://nevow.com/ns/nevow/0.1" xmlns:athena="http://divmod.org/ns/athena/0.7" nevow:render="liveElement">
2 <input class="methanal-input" type="password">
3 <nevow:attr name="value" nevow:render="value" />
4 <athena:handler event="onchange" handler="onChange" />
5 <athena:handler event="onkeyup" handler="onKeyUp" />
6 <athena:handler event="onblur" handler="onBlur" />
7 <athena:handler event="onfocus" handler="onFocus" />
8 </input>
9 <input class="methanal-input" type="password" id="confirmPassword">
10 <nevow:attr name="value" nevow:render="value" />
11 <athena:handler event="onchange" handler="onChange" />
12 </input>
13 <span style="display: none;" class="friendly-representation" id="displayValue" />
14 <span class="methanal-error" id="error" />
15</div>
016
=== modified file 'methanal/view.py'
--- methanal/view.py 2009-10-04 10:17:26 +0000
+++ methanal/view.py 2009-10-09 15:30:23 +0000
@@ -511,6 +511,15 @@
511511
512512
513513
514class VerifiedPasswordInput(TextInput):
515 """
516 Password input with verification and strength checking.
517 """
518 fragmentName = 'methanal-verified-password-input'
519 jsClass = u'Methanal.View.VerifiedPasswordInput'
520
521
522
514class ChoiceInput(FormInput):523class ChoiceInput(FormInput):
515 """524 """
516 Abstract input with multiple options.525 Abstract input with multiple options.

Subscribers

People subscribed via source and target branches