From 58b2d29ac417672c130419abd093bd3c5e16fe60 Mon Sep 17 00:00:00 2001 From: Shane Tomlinson <stomlinson@mozilla.com> Date: Fri, 16 Dec 2011 14:44:15 +0000 Subject: [PATCH] Adding a check to ensure requiredEmail is actually an email address. * Add a new action - doError. * Add a new template invalidRequiredEmail * Add a check in the "start" state to see if the requiredEmail is legit. close #632 --- .../static/dialog/controllers/actions.js | 14 ++++++ .../static/dialog/resources/state_machine.js | 12 +++-- resources/static/dialog/views/error.ejs | 2 +- .../dialog/views/invalidRequiredEmail.ejs | 8 ++++ .../qunit/controllers/actions_unit_test.js | 47 ++++++++++++------- .../resources/state_machine_unit_test.js | 23 ++++++++- 6 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 resources/static/dialog/views/invalidRequiredEmail.ejs diff --git a/resources/static/dialog/controllers/actions.js b/resources/static/dialog/controllers/actions.js index 96317b255..9e253e9ae 100644 --- a/resources/static/dialog/controllers/actions.js +++ b/resources/static/dialog/controllers/actions.js @@ -82,6 +82,20 @@ BrowserID.Modules.Actions = (function() { } }, + /** + * Show an error message + * @method doError + * @param {string} [template] - template to use, if not given, use "error" + * @param {object} [info] - info to send to template + */ + doError: function(template, info) { + if(!info) { + info = template; + template = "error"; + } + this.renderError(template, info); + }, + doOffline: function() { this.renderError("offline", {}); }, diff --git a/resources/static/dialog/resources/state_machine.js b/resources/static/dialog/resources/state_machine.js index 46a30588f..ef2dbef54 100644 --- a/resources/static/dialog/resources/state_machine.js +++ b/resources/static/dialog/resources/state_machine.js @@ -39,7 +39,8 @@ mediator = bid.Mediator, subscriptions = [], stateStack = [], - controller; + controller, + errors = bid.Errors; function subscribe(message, cb) { subscriptions.push(mediator.subscribe(message, cb)); @@ -92,9 +93,14 @@ self.hostname = info.hostname; self.allowPersistent = !!info.allowPersistent; - self.requiredEmail = info.requiredEmail; + var email = self.requiredEmail = info.requiredEmail; - gotoState("doCheckAuth"); + if(email && !(bid.verifyEmail(email))) { + gotoState("doError", "invalidRequiredEmail", { email: email }); + } + else { + gotoState("doCheckAuth"); + } }); subscribe("cancel", function() { diff --git a/resources/static/dialog/views/error.ejs b/resources/static/dialog/views/error.ejs index 6440c09d0..df449b06e 100644 --- a/resources/static/dialog/views/error.ejs +++ b/resources/static/dialog/views/error.ejs @@ -2,7 +2,7 @@ <% if (typeof network !== "undefined" && network.status == 503) { %> <h2 id="error_503">We are very sorry, the server is under extreme load!</h2> <% } else { %> - <h2>We are very sorry, there has been an error!</h2> + <h2 id="defaultError">We are very sorry, there has been an error!</h2> <% } %> <p> diff --git a/resources/static/dialog/views/invalidRequiredEmail.ejs b/resources/static/dialog/views/invalidRequiredEmail.ejs new file mode 100644 index 000000000..1c4d71ff7 --- /dev/null +++ b/resources/static/dialog/views/invalidRequiredEmail.ejs @@ -0,0 +1,8 @@ + + <h2 id="invalidRequiredEmail">*<%= email %>* is not a valid email address!</h2> + + <p> + To continue, please close the window and enter a valid address. + </p> + + diff --git a/resources/static/test/qunit/controllers/actions_unit_test.js b/resources/static/test/qunit/controllers/actions_unit_test.js index 54bcb64e3..340c91498 100644 --- a/resources/static/test/qunit/controllers/actions_unit_test.js +++ b/resources/static/test/qunit/controllers/actions_unit_test.js @@ -39,41 +39,52 @@ var bid = BrowserID, controller, - el; - - function reset() { - el = $("#controller_head"); - el.find("#formWrap .contents").html(""); - el.find("#wait .contents").html(""); - el.find("#error .contents").html(""); - } + el, + testHelpers = bid.TestHelpers; function createController(config) { controller = BrowserID.Modules.Actions.create(); controller.start(config); } - // XXX Make a test helper class for this. - function checkNetworkError() { - ok($("#error .contents").text().length, "contents have been written"); - ok($("#error #action").text().length, "action contents have been written"); - ok($("#error #network").text().length, "network contents have been written"); - } - module("controllers/actions", { setup: function() { - reset(); + testHelpers.setup(); }, teardown: function() { if(controller) { controller.destroy(); } - reset(); + testHelpers.teardown(); } }); - asyncTest("doOffline", function() { + asyncTest("doError with no template should display default error screen", function() { + createController({ + ready: function() { + equal(testHelpers.errorVisible(), false, "Error is not yet visible"); + controller.doError({}); + ok(testHelpers.errorVisible(), "Error is visible"); + equal($("#defaultError").length, 1, "default error screen is shown"); + start(); + } + }); + }); + + asyncTest("doError with with template should display error screen", function() { + createController({ + ready: function() { + equal(testHelpers.errorVisible(), false, "Error is not yet visible"); + controller.doError("invalidRequiredEmail", {email: "email"}); + equal($("#invalidRequiredEmail").length, 1, "default error screen is shown"); + ok(testHelpers.errorVisible(), "Error is visible"); + start(); + } + }); + }); + + asyncTest("doOffline should print offline error screen", function() { createController({ ready: function() { controller.doOffline(); diff --git a/resources/static/test/qunit/resources/state_machine_unit_test.js b/resources/static/test/qunit/resources/state_machine_unit_test.js index be8aabb25..6d96c41f3 100644 --- a/resources/static/test/qunit/resources/state_machine_unit_test.js +++ b/resources/static/test/qunit/resources/state_machine_unit_test.js @@ -98,8 +98,11 @@ doCancel: function() { this.cancelled = true; - } + }, + doError: function() { + this.error = true; + } }; function createMachine() { @@ -239,12 +242,28 @@ equal(controllerMock.email, "testuser@testuser.com", "authenticate with testuser@testuser.com"); }); - test("start", function() { + test("start with no required email address should go straight to checking auth", function() { mediator.publish("start"); equal(controllerMock.checkingAuth, true, "checking auth on start"); }); + test("start with invalid requiredEmail prints error screen", function() { + mediator.publish("start", { + requiredEmail: "bademail" + }); + + equal(controllerMock.error, true, "error screen is shown"); + }); + + test("start with valid requiredEmail goes to auth", function() { + mediator.publish("start", { + requiredEmail: "testuser@testuser.com" + }); + + equal(controllerMock.checkingAuth, true, "checking auth on start"); + }); + test("cancel", function() { mediator.publish("cancel"); -- GitLab