Bug 1098251 - Allow a contact to be created with either a phone number or an email address. r=MattN a=loop-only
authorJared Wein <jwein@mozilla.com>
Fri, 14 Nov 2014 10:58:57 -0500
changeset 233932 d8f242cbe2a2bee0594fdb12d8525afef9e56801
parent 233931 00e65cb7f78043180f49274a7a9e80f8d4486649
child 233933 120b2b6378eaa183e5bd1b3262d89b41f26a9c9c
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, loop-only
bugs1098251
milestone35.0a2
Bug 1098251 - Allow a contact to be created with either a phone number or an email address. r=MattN a=loop-only
browser/components/loop/content/js/contacts.js
browser/components/loop/content/js/contacts.jsx
browser/components/loop/test/desktop-local/contacts_test.js
--- a/browser/components/loop/content/js/contacts.js
+++ b/browser/components/loop/content/js/contacts.js
@@ -47,46 +47,43 @@ loop.contacts = (function(_, mozL10n) {
     }
     return contact[field].find(e => e.pref) || contact[field][0];
   };
 
   /** Used to set the preferred email or phone number
    *  for the contact. Both fields are optional.
    * @param   {object} contact
    *          The contact object to get the field from.
-   * @param   {object} data
-   *          An object that has a 'field' property. If the 'optional' field
-   *          is defined and true, then the field will only be set if the
-   *          'value' is not empty or if the previous value was defined.
+   * @param   {string} field
+   *          The field within the contact to set.
+   * @param   {string} value
+   *          The value that the field should be set to.
    */
-  let setPreferred = function(contact, data) {
-    if (data.optional) {
-      // Don't clear the field if it doesn't exist.
-      if (!data.value &&
-          (!contact[data.field] || !contact[data.field].length)) {
+  let setPreferred = function(contact, field, value) {
+    // Don't clear the field if it doesn't exist.
+    if (!value && (!contact[field] || !contact[field].length)) {
+      return;
+    }
+
+    if (!contact[field]) {
+      contact[field] = [];
+    }
+
+    if (!contact[field].length) {
+      contact[field][0] = {"value": value};
+      return;
+    }
+    // Set the value in the preferred tuple and return.
+    for (let i in contact[field]) {
+      if (contact[field][i].pref) {
+        contact[field][i].value = value;
         return;
       }
     }
-
-    if (!contact[data.field]) {
-      contact[data.field] = [];
-    }
-
-    if (!contact[data.field].length) {
-      contact[data.field][0] = {"value": data.value};
-      return;
-    }
-    // Set the value in the preferred tuple and return.
-    for (let i in contact[data.field]) {
-      if (contact[data.field][i].pref) {
-        contact[data.field][i].value = data.value;
-        return;
-      }
-    }
-    contact[data.field][0].value = data.value;
+    contact[field][0].value = value;
   };
 
   const ContactDropdown = React.createClass({displayName: 'ContactDropdown',
     propTypes: {
       handleAction: React.PropTypes.func.isRequired,
       canEdit: React.PropTypes.bool
     },
 
@@ -558,32 +555,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleAcceptButtonClick: function() {
       // Allow validity error indicators to be displayed.
       this.setState({
         pristine: false,
       });
 
+      let emailInput = this.refs.email.getDOMNode();
+      let telInput = this.refs.tel.getDOMNode();
       if (!this.refs.name.getDOMNode().checkValidity() ||
-          !this.refs.email.getDOMNode().checkValidity()) {
+          ((emailInput.required || emailInput.value) && !emailInput.checkValidity()) ||
+          ((telInput.required || telInput.value) && !telInput.checkValidity())) {
         return;
       }
 
       this.props.selectTab("contacts");
 
       let contactsAPI = navigator.mozLoop.contacts;
 
       switch (this.props.mode) {
         case "edit":
           this.state.contact.name[0] = this.state.name.trim();
-          setPreferred(this.state.contact,
-                       {field: "email", value: this.state.email.trim()});
-          setPreferred(this.state.contact,
-                       {field: "tel", value: this.state.tel.trim(), optional: true});
+          setPreferred(this.state.contact, "email", this.state.email.trim());
+          setPreferred(this.state.contact, "tel", this.state.tel.trim());
           contactsAPI.update(this.state.contact, err => {
             if (err) {
               throw err;
             }
           });
           this.setState({
             contact: null,
           });
@@ -617,32 +615,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleCancelButtonClick: function() {
       this.props.selectTab("contacts");
     },
 
     render: function() {
       let cx = React.addons.classSet;
-      // XXX do we need the ref="" attributes below?
+      let phoneOrEmailRequired = !this.state.email && !this.state.tel;
+
       return (
         React.DOM.div({className: "content-area contact-form"}, 
           React.DOM.header(null, this.props.mode == "add"
                    ? mozL10n.get("add_contact_button")
                    : mozL10n.get("edit_contact_title")), 
           React.DOM.label(null, mozL10n.get("edit_contact_name_label")), 
           React.DOM.input({ref: "name", required: true, pattern: "\\s*\\S.*", type: "text", 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("name")}), 
           React.DOM.label(null, mozL10n.get("edit_contact_email_label")), 
-          React.DOM.input({ref: "email", required: true, type: "email", 
+          React.DOM.input({ref: "email", type: "email", required: phoneOrEmailRequired, 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("email")}), 
           React.DOM.label(null, mozL10n.get("new_contact_phone_placeholder")), 
-          React.DOM.input({ref: "tel", type: "tel", 
+          React.DOM.input({ref: "tel", type: "tel", required: phoneOrEmailRequired, 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("tel")}), 
           ButtonGroup(null, 
             Button({additionalClass: "button-cancel", 
                     caption: mozL10n.get("cancel_button"), 
                     onClick: this.handleCancelButtonClick}), 
             Button({additionalClass: "button-accept", 
                     caption: this.props.mode == "add"
--- a/browser/components/loop/content/js/contacts.jsx
+++ b/browser/components/loop/content/js/contacts.jsx
@@ -47,46 +47,43 @@ loop.contacts = (function(_, mozL10n) {
     }
     return contact[field].find(e => e.pref) || contact[field][0];
   };
 
   /** Used to set the preferred email or phone number
    *  for the contact. Both fields are optional.
    * @param   {object} contact
    *          The contact object to get the field from.
-   * @param   {object} data
-   *          An object that has a 'field' property. If the 'optional' field
-   *          is defined and true, then the field will only be set if the
-   *          'value' is not empty or if the previous value was defined.
+   * @param   {string} field
+   *          The field within the contact to set.
+   * @param   {string} value
+   *          The value that the field should be set to.
    */
-  let setPreferred = function(contact, data) {
-    if (data.optional) {
-      // Don't clear the field if it doesn't exist.
-      if (!data.value &&
-          (!contact[data.field] || !contact[data.field].length)) {
+  let setPreferred = function(contact, field, value) {
+    // Don't clear the field if it doesn't exist.
+    if (!value && (!contact[field] || !contact[field].length)) {
+      return;
+    }
+
+    if (!contact[field]) {
+      contact[field] = [];
+    }
+
+    if (!contact[field].length) {
+      contact[field][0] = {"value": value};
+      return;
+    }
+    // Set the value in the preferred tuple and return.
+    for (let i in contact[field]) {
+      if (contact[field][i].pref) {
+        contact[field][i].value = value;
         return;
       }
     }
-
-    if (!contact[data.field]) {
-      contact[data.field] = [];
-    }
-
-    if (!contact[data.field].length) {
-      contact[data.field][0] = {"value": data.value};
-      return;
-    }
-    // Set the value in the preferred tuple and return.
-    for (let i in contact[data.field]) {
-      if (contact[data.field][i].pref) {
-        contact[data.field][i].value = data.value;
-        return;
-      }
-    }
-    contact[data.field][0].value = data.value;
+    contact[field][0].value = value;
   };
 
   const ContactDropdown = React.createClass({
     propTypes: {
       handleAction: React.PropTypes.func.isRequired,
       canEdit: React.PropTypes.bool
     },
 
@@ -558,32 +555,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleAcceptButtonClick: function() {
       // Allow validity error indicators to be displayed.
       this.setState({
         pristine: false,
       });
 
+      let emailInput = this.refs.email.getDOMNode();
+      let telInput = this.refs.tel.getDOMNode();
       if (!this.refs.name.getDOMNode().checkValidity() ||
-          !this.refs.email.getDOMNode().checkValidity()) {
+          ((emailInput.required || emailInput.value) && !emailInput.checkValidity()) ||
+          ((telInput.required || telInput.value) && !telInput.checkValidity())) {
         return;
       }
 
       this.props.selectTab("contacts");
 
       let contactsAPI = navigator.mozLoop.contacts;
 
       switch (this.props.mode) {
         case "edit":
           this.state.contact.name[0] = this.state.name.trim();
-          setPreferred(this.state.contact,
-                       {field: "email", value: this.state.email.trim()});
-          setPreferred(this.state.contact,
-                       {field: "tel", value: this.state.tel.trim(), optional: true});
+          setPreferred(this.state.contact, "email", this.state.email.trim());
+          setPreferred(this.state.contact, "tel", this.state.tel.trim());
           contactsAPI.update(this.state.contact, err => {
             if (err) {
               throw err;
             }
           });
           this.setState({
             contact: null,
           });
@@ -617,32 +615,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleCancelButtonClick: function() {
       this.props.selectTab("contacts");
     },
 
     render: function() {
       let cx = React.addons.classSet;
-      // XXX do we need the ref="" attributes below?
+      let phoneOrEmailRequired = !this.state.email && !this.state.tel;
+
       return (
         <div className="content-area contact-form">
           <header>{this.props.mode == "add"
                    ? mozL10n.get("add_contact_button")
                    : mozL10n.get("edit_contact_title")}</header>
           <label>{mozL10n.get("edit_contact_name_label")}</label>
           <input ref="name" required pattern="\s*\S.*" type="text"
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("name")} />
           <label>{mozL10n.get("edit_contact_email_label")}</label>
-          <input ref="email" required type="email"
+          <input ref="email" type="email" required={phoneOrEmailRequired}
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("email")} />
           <label>{mozL10n.get("new_contact_phone_placeholder")}</label>
-          <input ref="tel" type="tel"
+          <input ref="tel" type="tel" required={phoneOrEmailRequired}
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("tel")} />
           <ButtonGroup>
             <Button additionalClass="button-cancel"
                     caption={mozL10n.get("cancel_button")}
                     onClick={this.handleCancelButtonClick} />
             <Button additionalClass="button-accept"
                     caption={this.props.mode == "add"
--- a/browser/components/loop/test/desktop-local/contacts_test.js
+++ b/browser/components/loop/test/desktop-local/contacts_test.js
@@ -70,16 +70,90 @@ describe("loop.contacts", function() {
         it("should render 'add contact' button", function() {
           var view = TestUtils.renderIntoDocument(
             loop.contacts.ContactDetailsForm({mode: "add"}));
 
           var addButton = view.getDOMNode().querySelector(".button-accept");
           expect(addButton).to.not.equal(null);
           expect(addButton.textContent).to.eql(fakeAddContactButtonText);
         });
+        it("should have all fields required by default", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.required).to.equal(true);
+          expect(emailInput.required).to.equal(true);
+          expect(telInput.required).to.equal(true);
+        });
+        it("should have email and tel required after a name is input", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Jenny"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.required).to.equal(true);
+          expect(emailInput.required).to.equal(true);
+          expect(telInput.required).to.equal(true);
+        });
+        it("should allow a contact with only a name and a phone number", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Jenny"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          TestUtils.Simulate.change(telInput, {target: {value: "867-5309"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.checkValidity()).to.equal(true, "nameInput");
+          expect(emailInput.required).to.equal(false, "emailInput");
+          expect(telInput.checkValidity()).to.equal(true, "telInput");
+        });
+        it("should allow a contact with only a name and email", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Example"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          TestUtils.Simulate.change(emailInput, {target: {value: "test@example.com"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+
+          expect(nameInput.checkValidity()).to.equal(true);
+          expect(emailInput.checkValidity()).to.equal(true);
+          expect(telInput.required).to.equal(false);
+        });
+        it("should not allow a contact with only a name", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Example"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+
+          expect(nameInput.checkValidity()).to.equal(true);
+          expect(emailInput.checkValidity()).to.equal(false);
+          expect(telInput.checkValidity()).to.equal(false);
+        });
+        it("should not allow a contact without name", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          TestUtils.Simulate.change(emailInput, {target: {value: "test@example.com"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          TestUtils.Simulate.change(telInput, {target: {value: "867-5309"}});
+
+          expect(nameInput.checkValidity()).to.equal(false);
+          expect(emailInput.checkValidity()).to.equal(true);
+          expect(telInput.checkValidity()).to.equal(true);
+        });
       });
       describe("edit mode", function() {
         it("should render 'edit' header", function() {
           var view = TestUtils.renderIntoDocument(
             loop.contacts.ContactDetailsForm({mode: "edit"}));
 
           var header = view.getDOMNode().querySelector("header");
           expect(header).to.not.equal(null);
@@ -132,35 +206,28 @@ describe("loop.contacts", function() {
       expect(obj.value).to.eql(correctValue);
     });
   });
 
   describe("_setPreferred", function() {
     it("should not set the value on the object if the new value is empty," +
        " it didn't exist before, and it is optional", function() {
           var contact = {};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "", optional: true});
+          loop.contacts._setPreferred(contact, "fakeField", "");
 
           expect(contact).to.not.have.property("fakeField");
        });
     it("should clear the value on the object if the new value is empty," +
        " it existed before, and it is optional", function() {
           var contact = {fakeField: [{value: "foobar"}]};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "", optional: true});
+          loop.contacts._setPreferred(contact, "fakeField", "");
 
           expect(contact["fakeField"][0].value).to.eql("");
        });
     it("should set the value on the object if the new value is empty," +
        " and it did not exist before", function() {
-          var contact = {};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: ""});
-
-          expect(contact).to.have.property("fakeField");
-       });
-    it("should set the value on the object if the new value is empty," +
-       " and it did not exist before", function() {
           var contact = {fakeField: [{value: "foobar"}]};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "barbaz"});
+          loop.contacts._setPreferred(contact, "fakeField", "barbaz");
 
           expect(contact["fakeField"][0].value).to.eql("barbaz");
        });
   });
 });