Add email validation in person new and edit (#501)

* Add new email regex for person edit and rename file person edit to person form

* Add email validation for create new person in backend

* Add german validation message and rename person form to person edit

* Make rubocop happy

* Write backend test for email validation

* Write frontend test for email validation

* Write Acceptance test for invalid email in person new

* Use other click method to call submit button

* Remove unused code in people-new.js

* Fix frontend test for email validation on create person

* Fix type fault

app/models/person.rb

@@ -44,10 +44,14 @@ class Person < ApplicationRecord
   has_many :roles, through: :person_roles
 
   validates :birthdate, :location, :name, :nationality,
-            :title, :marital_status, presence: true
+            :title, :marital_status, :email, presence: true
   validates :location, :name, :title,
             :email, :shortname, length: { maximum: 100 }
 
+  validates :email,
+            format: { with: /\A[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}\z/,
+                      message: 'Format nicht gültig' }
+
   validates :nationality,
             inclusion: { in: ISO3166::Country.all.collect(&:alpha2) }
   validates :nationality2,

frontend/app/components/person-new.js

@@ -4,6 +4,7 @@ import { computed } from "@ember/object";
 import { isBlank } from "@ember/utils";
 import { getNames as countryNames } from "ember-i18n-iso-countries";
 import Person from "../models/person";
+import config from "../config/environment";
 
 export default ApplicationComponent.extend({
   intl: service(),
@@ -99,7 +100,7 @@ export default ApplicationComponent.extend({
               `person.${attribute}`
             );
             this.get("notify").alert(`${translated_attribute} ${message}`, {
-              closeAfter: 8000
+              closeAfter: config.environment === "test" ? null : 8000
             });
           });
         });

frontend/app/templates/components/person-new.hbs

@@ -13,13 +13,13 @@
           <table class="table-person mb-3">
             <tbody>
               <tr>
-                <td><label for="name">{{t "person-new.name"}}</label><Input @id="name" @value={{this.newPerson.name}} @type="text"/></td>
+                <td><label for="name">{{t "person-new.name"}}</label><Input name="name" @id="name" @value={{this.newPerson.name}} @type="text"/></td>
               </tr>
               <tr>
-                <td><label for="email">Email</label><Input @id="email" @value={{this.newPerson.email}} @type="text"/></td>
+                <td><label for="email">Email</label><Input name="email" @id="email" @value={{this.newPerson.email}} @type="text"/></td>
               </tr>
               <tr>
-                <td><label for="title">{{t "person-new.degree"}}</label><Input @id="title" @value={{this.newPerson.title}} @type="text"/></td>
+                <td><label for="title">{{t "person-new.degree"}}</label><Input name="title" @id="title" @value={{this.newPerson.title}} @type="text"/></td>
               </tr>
               <tr>
                 <label>{{t "person-new.functions"}}</label>
@@ -47,7 +47,7 @@
                         {{/if}}
                       </div>
                       <div class="percent-field">
-                        <input placeholder="in %" type="number" value={{personRole.percent}} onchange={{action "setRolePercent" personRole}} max="9999">
+                        <input name="rolePercent" placeholder="in %" type="number" value={{personRole.percent}} onchange={{action "setRolePercent" personRole}} max="9999">
                       </div>
                       <div class="delete-role-icon">
                         <DeleteWithConfirmation @entry={{personRole}} @class="edit-buttons" />
@@ -81,7 +81,7 @@
                 </td>
               </tr>
               <tr>
-                <td><label for="location">{{t "person-new.placeOfResidence"}}</label><Input @id="location" @value={{this.newPerson.location}} @type="text"/></td>
+                <td><label for="location">{{t "person-new.placeOfResidence"}}</label><Input name="location" @id="location" @value={{this.newPerson.location}} @type="text"/></td>
               </tr>
             </tbody>
           </table>
@@ -132,7 +132,7 @@
                 </td>
               </tr>
               <tr>
-                <td><label for="shortname">{{t "person-new.shortname"}}</label><Input @id="shortname" @value={{this.newPerson.shortname}} @type="text"/></td>
+                <td><label for="shortname">{{t "person-new.shortname"}}</label><Input name="shortname" @id="shortname" @value={{this.newPerson.shortname}} @type="text"/></td>
               </tr>
             </tbody>
           </table>

frontend/app/validations/person-edit.js

@@ -1,8 +1,16 @@
-import { validatePresence } from "ember-changeset-validations/validators";
-
+import {
+  validatePresence,
+  validateFormat
+} from "ember-changeset-validations/validators";
 export default {
   name: [validatePresence(true)],
-  email: [validatePresence(true)],
+  email: [
+    validatePresence({ presence: true, message: "Email kann nicht leer sein" }),
+    validateFormat({
+      type: "email",
+      message: "Gib eine gültige Email Adresse ein"
+    })
+  ],
   title: [validatePresence(true)],
   birthdate: [validatePresence(true)],
   location: [validatePresence(true)]

frontend/tests/acceptance/create-person-test.js

@@ -2,8 +2,10 @@ import { module, test, skip } from "qunit";
 import page from "frontend/tests/pages/people-new";
 import { openDatepicker } from "ember-pikaday/helpers/pikaday";
 import $ from "jquery";
+import { click } from "@ember/test-helpers";
 import setupApplicationTest from "frontend/tests/helpers/setup-application-test";
 import { currentURL } from "@ember/test-helpers";
+import { selectChoose } from "ember-power-select/test-support";
 
 module("Acceptance | create person", function(hooks) {
   setupApplicationTest(hooks);
@@ -77,4 +79,66 @@ module("Acceptance | create person", function(hooks) {
     assert.equal(currentURL(), "/people/new");
     // TODO expect errors!
   });
+
+  test("should display two errors when email is empty", async function(assert) {
+    await page.newPersonPage.visit();
+    assert.equal(currentURL(), "/people/new");
+
+    page.newPersonPage.toggleNewForm();
+
+    await page.newForm.name("Findus");
+    await page.newForm.title("Sofware Developer");
+    await page.newForm.shortname("FI");
+    await page.newForm.location("Bern");
+
+    let interactor = openDatepicker($(".birthdate_pikaday > input"));
+
+    interactor.selectDate(new Date(2019, 1, 19));
+
+    await selectChoose("#department", "/dev/one");
+    await selectChoose("#company", "Firma");
+    await selectChoose("#maritalStatus", ".ember-power-select-option", 0);
+
+    await click("button#submit-button");
+
+    assert.equal(
+      document.querySelectorAll(".ember-notify")[0].querySelector(".message")
+        .innerText,
+      "Email muss ausgefüllt werden"
+    );
+    assert.equal(
+      document.querySelectorAll(".ember-notify")[1].querySelector(".message")
+        .innerText,
+      "Email Format nicht gültig"
+    );
+  });
+
+  test("should display one error when email format is invalid", async function(assert) {
+    await page.newPersonPage.visit();
+    assert.equal(currentURL(), "/people/new");
+
+    page.newPersonPage.toggleNewForm();
+
+    await page.newForm.name("Findus");
+    await page.newForm.email("findus.puzzle");
+    await page.newForm.title("Sofware Developer");
+    await page.newForm.shortname("FI");
+    await page.newForm.location("Bern");
+
+    let interactor = openDatepicker($(".birthdate_pikaday > input"));
+
+    interactor.selectDate(new Date(2019, 1, 19));
+
+    await selectChoose("#department", "/dev/one");
+    await selectChoose("#company", "Firma");
+    await selectChoose("#maritalStatus", ".ember-power-select-option", 0);
+
+    await click("button#submit-button");
+
+    assert.equal(
+      document.querySelectorAll(".ember-notify")[0].querySelector(".message")
+        .innerText,
+      "Email Format nicht gültig"
+    );
+  });
 });

frontend/tests/integration/components/person-edit-test.js

@@ -75,4 +75,47 @@ module("Integration | Component | person-edit", function(hooks) {
         .includes("Birthdate can't be blank")
     );
   });
+
+  test("it renders person-edit with validation error at empty email", async function(assert) {
+    let person = run(() =>
+      this.owner.lookup("service:store").createRecord("person")
+    );
+    person.name = "Hans Rudolf";
+    person.title = "Construction Consultant";
+    person.location = "Bern";
+    person.birthdate = new Date(2019, 1, 19);
+    person.shortname = "HR";
+    this.set("person", person);
+
+    await render(hbs`{{person-edit person=person}}`);
+    this.$("button")[0].click();
+    await settled();
+    assert.ok(
+      this.$("#validation-error")
+        .text()
+        .includes("Email kann nicht leer sein")
+    );
+  });
+
+  test("it renders person-edit with validation error at invalid email", async function(assert) {
+    let person = run(() =>
+      this.owner.lookup("service:store").createRecord("person")
+    );
+    person.name = "Hans Rudolf";
+    person.email = "hans";
+    person.title = "Construction Consultant";
+    person.location = "Bern";
+    person.birthdate = new Date(2019, 1, 19);
+    person.shortname = "HR";
+    this.set("person", person);
+
+    await render(hbs`{{person-edit person=person}}`);
+    this.$("button")[0].click();
+    await settled();
+    assert.ok(
+      this.$("#validation-error")
+        .text()
+        .includes("Gib eine gültige Email Adresse ein")
+    );
+  });
 });

frontend/tests/pages/people-new.js

@@ -1,10 +1,10 @@
 import RSVP from "rsvp";
 import {
+  clickable,
   create,
-  visitable,
   fillable,
-  clickable,
-  text
+  text,
+  visitable
 } from "ember-cli-page-object";
 
 const { resolve } = RSVP;
@@ -13,10 +13,12 @@ export default create({
   newPersonPage: {
     visit: visitable("/people/new"),
     submit: clickable("#submit-button"),
+    toggleNewFormButton: clickable("[data-test-person-new-form-toggle]"),
+    toggleNationalitiesCheckbox: clickable("#toggle-nationalities-id"),
 
-    name: fillable('[name="person[name]"]'),
-    title: fillable('[name="person[title]"]'),
-    location: fillable('[name="person[location]"]'),
+    toggleNewForm() {
+      return this.newForm;
+    },
 
     async createPerson(person) {
       await Object.keys(person).reduce(
@@ -28,14 +30,29 @@ export default create({
     }
   },
 
+  newForm: {
+    scope: "#profil",
+    submit: clickable('[type="submit"]'),
+    name: fillable('[name="name"]'),
+    email: fillable('[name="email"]'),
+    shortname: fillable('[name="shortname"]'),
+    title: fillable('[name="title"]'),
+    location: fillable('[name="location"]'),
+    rolePercent: fillable('[name="rolePercent"]')
+  },
+
   profileData: {
     name: text("#data-test-person-name"),
+    email: text("#data-test-person-email"),
     title: text("#data-test-person-title"),
     role: text("#data-test-person-role"),
+    department: text("#data-test-person-department"),
+    company: text("#data-test-person-company"),
     birthdate: text("#data-test-person-birthdate"),
     nationalities: text("#data-test-person-nationalities"),
     location: text("#data-test-person-location"),
     language: text("[data-test-person-language]", { multiple: true }),
-    maritalStatus: text("#data-test-person-marital-status")
+    maritalStatus: text("#data-test-person-marital-status"),
+    shortname: text("#data-test-person-shortname")
   }
 });

frontend/translations/de.yml

@@ -125,6 +125,7 @@ person:
     title: Titel
     projects: Projekte
     personCompetences: Kompetenzen
+    email: Email
 personCompetence:
     category: Kategorie
     offer: Angebot

spec/models/person_spec.rb

@@ -73,6 +73,30 @@
       end
     end
 
+    context 'email' do
+
+      it 'should be invalid when email is nil' do
+        person = people(:bob)
+        person.email = nil
+        expect(person).not_to be_valid
+        expect(person.errors.messages[:email].first).to eq('muss ausgefüllt werden')
+      end
+
+      it 'should be invalid when email format is invalid' do
+        person = people(:bob)
+        person.email = "email"
+        expect(person).not_to be_valid
+        expect(person.errors.messages[:email].first).to eq('Format nicht gültig')
+      end
+
+      it 'should be valid when email format is correct' do
+        person = people(:bob)
+        person.email = "[email protected]"
+        expect(person).to be_valid
+      end
+
+    end
+
     it 'should not be more than 100 characters' do
       person = people(:bob)
       person.location = SecureRandom.hex(100)