From 611ffa24e57d096ef684781ee773f36075eef518 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 10 Apr 2017 12:22:21 +0200 Subject: [PATCH] de-duplicate email validation code Use an utility function for validating email adresses (rather than a full-blown service). IMO Services should be used when the service is a object with several method. In this case instantiating a service makes sense. When the code is a simple function, an utility in `utils/` is simpler, and involves less boilerplate. This commit has the nice side-effect of actually simplifying the code that uses the email validation: no need to inject a custom service, or to define a private helper function. --- live/app/components/follower-form.js | 11 ++------ live/app/services/email-validator.js | 15 ----------- live/app/utils/email-validator.js | 7 ++++-- .../email-validator-test.js | 25 ++++++------------- 4 files changed, 14 insertions(+), 44 deletions(-) delete mode 100644 live/app/services/email-validator.js rename live/tests/unit/{services => utils}/email-validator-test.js (62%) diff --git a/live/app/components/follower-form.js b/live/app/components/follower-form.js index 46d51eff5..40e6c63f0 100644 --- a/live/app/components/follower-form.js +++ b/live/app/components/follower-form.js @@ -1,5 +1,6 @@ import Ember from 'ember'; import ENV from 'pix-live/config/environment'; +import isEmailValid from 'pix-live/utils/email-validator'; const messageDisplayDuration = 1500; @@ -21,7 +22,6 @@ export default Ember.Component.extend({ classNames: ['follower-form'], - emailValidator: Ember.inject.service('email-validator'), store: Ember.inject.service(), errorType:'invalid' , status: 'empty', // empty | pending | success | error @@ -63,18 +63,11 @@ export default Ember.Component.extend({ return (this.get('status') === 'pending') ? 'envoi en cours' : 's\'inscrire'; }), - _checkEmail(email){ - if (!this.get('emailValidator').emailIsValid(email)) { - return false; - } - return true; - }, - actions: { submit(){ this.set('status', 'pending'); const email = (this.get('followerEmail'))? this.get('followerEmail').trim() : ''; - if (!this._checkEmail(email) || email.length < 1) { + if (!isEmailValid(email)) { this.set('status', 'error'); hideMessageDiv(this); return; diff --git a/live/app/services/email-validator.js b/live/app/services/email-validator.js deleted file mode 100644 index 9648cf8c5..000000000 --- a/live/app/services/email-validator.js +++ /dev/null @@ -1,15 +0,0 @@ -import Ember from 'ember'; - -export default Ember.Service.extend({ - emailIsValid(email) { - if (!email) { - return false; - } - - //XXX: Cf - http://stackoverflow.com/a/46181/5430854 - const pattern = /^(([^<>()\[\]\.,;:\s@\"]+(\.[^<>()\[\]\.,;:\s@\"]+)*)|(\".+\"))@(([^<>()[\]\.,;:\s@\"]+\.)+[^<>()[\]\.,;:\s@\"]{2,})$/i; - - return pattern.test(email.trim()); - } - -}); diff --git a/live/app/utils/email-validator.js b/live/app/utils/email-validator.js index b403e4847..aa992d4d3 100644 --- a/live/app/utils/email-validator.js +++ b/live/app/utils/email-validator.js @@ -1,5 +1,8 @@ -export default function isValidate(email) { - //XXX: Cf - http://stackoverflow.com/a/46181/5430854 +export default function isEmailValid(email) { + if (!email) { + return false; + } + // From http://stackoverflow.com/a/46181/5430854 const pattern = /^(([^<>()\[\]\.,;:\s@\"]+(\.[^<>()\[\]\.,;:\s@\"]+)*)|(\".+\"))@(([^<>()[\]\.,;:\s@\"]+\.)+[^<>()[\]\.,;:\s@\"]{2,})$/i; return pattern.test(email.trim()); } diff --git a/live/tests/unit/services/email-validator-test.js b/live/tests/unit/utils/email-validator-test.js similarity index 62% rename from live/tests/unit/services/email-validator-test.js rename to live/tests/unit/utils/email-validator-test.js index 626fb7ee7..8f308e36b 100644 --- a/live/tests/unit/services/email-validator-test.js +++ b/live/tests/unit/utils/email-validator-test.js @@ -1,20 +1,9 @@ import {expect} from 'chai'; import {describe, it} from 'mocha'; -import {setupTest} from 'ember-mocha'; +import isEmailValid from 'pix-live/utils/email-validator'; -describe('Unit | Service | EmailValidatorService', function () { - - setupTest('service:email-validator', {}); - let validator; - beforeEach(function () { - validator = this.subject(); - }); - - it('exists', function () { - expect(validator).to.be.ok; - }); - - describe('Test all case Invalid and then valid email', function () { +describe('Unit | Utility | email validator', function () { + describe('Invalid emails', function () { [ '', ' ', @@ -27,10 +16,12 @@ describe('Unit | Service | EmailValidatorService', function () { '@pix' ].forEach(function (badEmail) { it(`should return false when email is invalid: ${badEmail}`, function () { - expect(validator.emailIsValid(badEmail)).to.be.false; + expect(isEmailValid(badEmail)).to.be.false; }); }); + }); + describe('Valid emails', function () { [ 'follower@pix.fr', 'follower@pix.fr ', @@ -43,10 +34,8 @@ describe('Unit | Service | EmailValidatorService', function () { 'follower+beta@pix.beta.gouv.fr' ].forEach(function (validEmail) { it(`should return true if provided email is valid: ${validEmail}`, function () { - expect(validator.emailIsValid(validEmail)).to.be.true; + expect(isEmailValid(validEmail)).to.be.true; }); }); }); - - });