Bug 1402963 - Part 2: Merge the credit card record into existing data. r=lchang draft
authorsteveck-chung <schung@mozilla.com>
Wed, 25 Oct 2017 17:46:56 +0800
changeset 688556 3aa3c9fb2f1b2c514f8597c31f3d5e5e0dc5e0de
parent 687551 59d62efdb761eff518561985d14bb040ff630d8e
child 738095 5bdb01a2e5812e01932659feb9bbcded76468e66
push id86778
push userbmo:schung@mozilla.com
push dateMon, 30 Oct 2017 08:04:55 +0000
reviewerslchang
bugs1402963
milestone58.0a1
Bug 1402963 - Part 2: Merge the credit card record into existing data. r=lchang MozReview-Commit-ID: 3Hkqvo2rK9R
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
browser/extensions/formautofill/test/browser/head.js
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -473,17 +473,29 @@ FormAutofillParent.prototype = {
 
     // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
     // APIs are refactored to be async functions (bug 1399367).
     if (!await MasterPassword.ensureLoggedIn()) {
       log.warn("User canceled master password entry");
       return;
     }
 
-    this.profileStorage.creditCards.add(creditCard.record);
+    // TODO: Autofill(with guid) case should show update doorhanger with update/create new.
+    // It'll be implemented in bug 1403881 and only avoid mergering for now.
+    if (creditCard.guid) {
+      this.profileStorage.creditCards.add(creditCard.record);
+      return;
+    }
+
+    let changedGUIDs = this.profileStorage.creditCards.mergeToStorage(creditCard.record);
+    if (!changedGUIDs.length) {
+      this.profileStorage.creditCards.add(creditCard.record);
+    } else {
+      changedGUIDs.forEach(guid => this.profileStorage.creditCards.notifyUsed(guid));
+    }
   },
 
   _onFormSubmit(data, target) {
     let {profile: {address, creditCard}, timeStartedFillingMS} = data;
 
     if (address) {
       this._onAddressSubmit(address, target, timeStartedFillingMS);
     }
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -1156,30 +1156,45 @@ class AutofillRecords {
     this._store.data[this._collectionName] = [];
     // test-only, so there's no good reason to request a save!
   }
 
   _stripComputedFields(record) {
     this.VALID_COMPUTED_FIELDS.forEach(field => delete record[field]);
   }
 
+  /**
+   * Merge the record if storage has multiple mergeable records.
+   * @param {Object} targetRecord
+   *        The record for merge.
+   * @returns {Array.<string>}
+   *          Return an array of the merged GUID string.
+   */
+  mergeToStorage(targetRecord) {
+    let mergedGUIDs = [];
+    for (let record of this.data) {
+      if (!record.deleted && this.mergeIfPossible(record.guid, targetRecord)) {
+        mergedGUIDs.push(record.guid);
+      }
+    }
+    this.log.debug("Existing records matching and merging count is", mergedGUIDs.length);
+    return mergedGUIDs;
+  }
+
   // An interface to be inherited.
   _recordReadProcessor(record) {}
 
   // An interface to be inherited.
   _computeFields(record) {}
 
   // An interface to be inherited.
   _normalizeFields(record) {}
 
   // An interface to be inherited.
   mergeIfPossible(guid, record) {}
-
-  // An interface to be inherited.
-  mergeToStorage(targetRecord) {}
 }
 
 class Addresses extends AutofillRecords {
   constructor(store) {
     super(store, "addresses", VALID_ADDRESS_FIELDS, VALID_ADDRESS_COMPUTED_FIELDS, ADDRESS_SCHEMA_VERSION);
   }
 
   _recordReadProcessor(address) {
@@ -1570,16 +1585,77 @@ class CreditCards extends AutofillRecord
         }
         if (!targetCreditCard[field]) {
           return !creditCard[field];
         }
         return targetCreditCard[field] == creditCard[field];
       });
     });
   }
+
+  /**
+   * Merge new credit card into the specified record if mergeable.
+   *
+   * @param  {string} guid
+   *         Indicates which credit card to merge.
+   * @param  {Object} creditCard
+   *         The new credit card used to merge into the old one.
+   * @returns {boolean}
+   *          Return true if credit card is merged into target with specific guid or false if not.
+   */
+  mergeIfPossible(guid, creditCard) {
+    this.log.debug("mergeIfPossible:", guid, creditCard);
+
+    // Query raw data for comparing the decrypted credit card number
+    let creditCardFound = this.get(guid, {rawData: true});
+    if (!creditCardFound) {
+      throw new Error("No matching credit card.");
+    }
+
+    let creditCardToMerge = this._clone(creditCard);
+    this._normalizeRecord(creditCardToMerge);
+
+    for (let field of this.VALID_FIELDS) {
+      let existingField = creditCardFound[field];
+      if (!creditCardToMerge[field]) {
+        creditCardToMerge[field] = existingField;
+      }
+
+      let incomingField = creditCardToMerge[field];
+      if (!!incomingField && !!existingField) {
+        if (incomingField != existingField) {
+          this.log.debug("Conflicts: field", field, "has different value.");
+          return false;
+        }
+      }
+    }
+
+    // Early return if the data is the same.
+    let exactlyMatch = this.VALID_FIELDS.every((field) =>
+      creditCardFound[field] === creditCardToMerge[field]
+    );
+    if (exactlyMatch) {
+      return true;
+    }
+
+    creditCardFound = this._findByGUID(guid);
+    this._computeFields(creditCardToMerge);
+    for (let field in creditCardToMerge) {
+      creditCardFound[field] = creditCardToMerge[field];
+    }
+
+    creditCardFound.timeLastModified = Date.now();
+
+    this._store.saveSoon();
+    let str = Cc["@mozilla.org/supports-string;1"]
+                 .createInstance(Ci.nsISupportsString);
+    str.data = guid;
+    Services.obs.notifyObservers(str, "formautofill-storage-changed", "merge");
+    return true;
+  }
 }
 
 function ProfileStorage(path) {
   this._path = path;
   this._initializePromise = null;
   this.INTERNAL_FIELDS = INTERNAL_FIELDS;
 }
 
--- a/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
+++ b/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
@@ -53,19 +53,23 @@ add_task(async function test_submit_cred
       await promiseShown;
       await clickDoorhangerButton(MAIN_BUTTON);
     }
   );
 
   let creditCards = await getCreditCards();
   is(creditCards.length, 1, "1 credit card in storage");
   is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  await removeAllRecords();
 });
 
 add_task(async function test_submit_untouched_creditCard_form() {
+  await saveCreditCard(TEST_CREDIT_CARD_1);
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "1 credit card in storage");
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       await openPopupOn(browser, "form #cc-name");
       await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
       await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser);
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
 
@@ -74,107 +78,119 @@ add_task(async function test_submit_unto
         form.querySelector("input[type=submit]").click();
       });
 
       await sleep(1000);
       is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
     }
   );
 
-  let creditCards = await getCreditCards();
-  is(creditCards.length, 1, "Still 1 credit card in storage");
-  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card");
+  await removeAllRecords();
 });
 
-add_task(async function test_submit_changed_creditCard_form() {
+add_task(async function test_submit_changed_subset_creditCard_form() {
+  await saveCreditCard(TEST_CREDIT_CARD_1);
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "1 credit card in storage");
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
-      await openPopupOn(browser, "form #cc-name");
-      await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
-      await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser);
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
 
         name.focus();
         await new Promise(resolve => setTimeout(resolve, 1000));
         name.setUserInput("");
+
+        form.querySelector("#cc-number").setUserInput("1234567812345678");
+        form.querySelector("#cc-exp-month").setUserInput("4");
+        form.querySelector("#cc-exp-year").setUserInput("2017");
         // Wait 1000ms before submission to make sure the input value applied
         await new Promise(resolve => setTimeout(resolve, 1000));
         form.querySelector("input[type=submit]").click();
       });
 
       await promiseShown;
-      await clickDoorhangerButton(SECONDARY_BUTTON);
+      await clickDoorhangerButton(MAIN_BUTTON);
     }
   );
 
-  let creditCards = await getCreditCards();
+  creditCards = await getCreditCards();
   is(creditCards.length, 1, "Still 1 credit card in storage");
-  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  is(creditCards[0]["cc-name"], TEST_CREDIT_CARD_1["cc-name"], "name field still exists");
+  await removeAllRecords();
 });
 
 add_task(async function test_submit_duplicate_creditCard_form() {
+  await saveCreditCard(TEST_CREDIT_CARD_1);
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "1 credit card in storage");
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
         name.focus();
 
-        name.setUserInput("User 1");
-        form.querySelector("#cc-number").setUserInput("1111222233334444");
-        form.querySelector("#cc-exp-month").setUserInput("12");
+        name.setUserInput("John Doe");
+        form.querySelector("#cc-number").setUserInput("1234567812345678");
+        form.querySelector("#cc-exp-month").setUserInput("4");
         form.querySelector("#cc-exp-year").setUserInput("2017");
 
         // Wait 1000ms before submission to make sure the input value applied
         await new Promise(resolve => setTimeout(resolve, 1000));
         form.querySelector("input[type=submit]").click();
       });
 
       await sleep(1000);
       is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
     }
   );
 
-  let creditCards = await getCreditCards();
+  creditCards = await getCreditCards();
   is(creditCards.length, 1, "Still 1 credit card in storage");
-  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  is(creditCards[0]["cc-name"], TEST_CREDIT_CARD_1["cc-name"], "Verify the name field");
+  await removeAllRecords();
 });
 
 add_task(async function test_submit_unnormailzed_creditCard_form() {
+  await saveCreditCard(TEST_CREDIT_CARD_1);
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "1 credit card in storage");
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
         name.focus();
 
-        name.setUserInput("User 1");
-
-        form.querySelector("#cc-number").setUserInput("1111222233334444");
-        form.querySelector("#cc-exp-month").setUserInput("12");
+        name.setUserInput("John Doe");
+        form.querySelector("#cc-number").setUserInput("1234567812345678");
+        form.querySelector("#cc-exp-month").setUserInput("4");
         // Set unnormalized year
         form.querySelector("#cc-exp-year").setUserInput("17");
 
         // Wait 1000ms before submission to make sure the input value applied
         await new Promise(resolve => setTimeout(resolve, 1000));
         form.querySelector("input[type=submit]").click();
       });
 
       await sleep(1000);
       is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
     }
   );
 
-  let creditCards = await getCreditCards();
+  creditCards = await getCreditCards();
   is(creditCards.length, 1, "Still 1 credit card in storage");
   is(creditCards[0]["cc-exp-year"], "2017", "Verify the expiry year field");
+  await removeAllRecords();
 });
 
 add_task(async function test_submit_creditCard_never_save() {
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
       await ContentTask.spawn(browser, null, async function() {
@@ -195,17 +211,17 @@ add_task(async function test_submit_cred
       await promiseShown;
       await clickDoorhangerButton(MENU_BUTTON, 0);
     }
   );
 
   await sleep(1000);
   let creditCards = await getCreditCards();
   let creditCardPref = SpecialPowers.getBoolPref(ENABLED_AUTOFILL_CREDITCARDS_PREF);
-  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards.length, 0, "No credit card in storage");
   is(creditCardPref, false, "Credit card is disabled");
   SpecialPowers.clearUserPref(ENABLED_AUTOFILL_CREDITCARDS_PREF);
 });
 
 add_task(async function test_submit_creditCard_saved_with_mp_enabled() {
   LoginTestUtils.masterPassword.enable();
   // Login with the masterPassword in LoginTestUtils.
   let masterPasswordDialogShown = waitForMasterPasswordDialog(true);
@@ -231,20 +247,21 @@ add_task(async function test_submit_cred
       await promiseShown;
       await clickDoorhangerButton(MAIN_BUTTON);
       await masterPasswordDialogShown;
       await TestUtils.topicObserved("formautofill-storage-changed");
     }
   );
 
   let creditCards = await getCreditCards();
-  is(creditCards.length, 2, "2 credit cards in storage");
-  is(creditCards[1]["cc-name"], "User 0", "Verify the name field");
-  is(creditCards[1]["cc-number"], "************1234", "Verify the card number field");
+  is(creditCards.length, 1, "1 credit card in storage");
+  is(creditCards[0]["cc-name"], "User 0", "Verify the name field");
+  is(creditCards[0]["cc-number"], "************1234", "Verify the card number field");
   LoginTestUtils.masterPassword.disable();
+  await removeAllRecords();
 });
 
 add_task(async function test_submit_creditCard_saved_with_mp_enabled_but_canceled() {
   LoginTestUtils.masterPassword.enable();
   let masterPasswordDialogShown = waitForMasterPasswordDialog();
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
@@ -267,27 +284,26 @@ add_task(async function test_submit_cred
       await promiseShown;
       await clickDoorhangerButton(MAIN_BUTTON);
       await masterPasswordDialogShown;
     }
   );
 
   await sleep(1000);
   let creditCards = await getCreditCards();
-  is(creditCards.length, 2, "Still 2 credit cards in storage");
+  is(creditCards.length, 0, "No credit cards in storage");
   LoginTestUtils.masterPassword.disable();
 });
 
 add_task(async function test_submit_creditCard_unavailable_with_sync_account() {
   await SpecialPowers.pushPrefEnv({
     "set": [
       [SYNC_USERNAME_PREF, "foo@bar.com"],
     ],
   });
-
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
       is(SpecialPowers.getBoolPref(SYNC_CREDITCARDS_AVAILABLE_PREF), false,
          "creditCards sync should be unavailable by default");
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
@@ -357,30 +373,29 @@ add_task(async function test_submit_cred
       is(secondaryButton.disabled, true, "Not saving button should be disabled");
       is(menuButton.disabled, true, "Never saving menu button should be disabled");
       // Click the checkbox again to disable credit card sync.
       cb.click();
       is(SpecialPowers.getBoolPref(SYNC_CREDITCARDS_PREF), false,
          "creditCards sync should be disabled after unchecked");
       is(secondaryButton.disabled, false, "Not saving button should be enabled again");
       is(menuButton.disabled, false, "Never saving menu button should be enabled again");
-      await clickDoorhangerButton(MAIN_BUTTON);
+      await clickDoorhangerButton(SECONDARY_BUTTON);
     }
   );
 });
 
 add_task(async function test_submit_creditCard_with_synced_already() {
   await SpecialPowers.pushPrefEnv({
     "set": [
       [SYNC_CREDITCARDS_PREF, true],
       [SYNC_USERNAME_PREF, "foo@bar.com"],
       [SYNC_CREDITCARDS_AVAILABLE_PREF, true],
     ],
   });
-
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
         name.focus();
@@ -392,12 +407,45 @@ add_task(async function test_submit_cred
         // Wait 500ms before submission to make sure the input value applied
         await new Promise(resolve => setTimeout(resolve, 500));
         form.querySelector("input[type=submit]").click();
       });
 
       await promiseShown;
       let cb = getDoorhangerCheckbox();
       ok(cb.hidden, "Sync checkbox should be hidden");
+      await clickDoorhangerButton(SECONDARY_BUTTON);
+    }
+  );
+});
+
+add_task(async function test_submit_manual_mergeable_creditCard_form() {
+  await saveCreditCard(TEST_CREDIT_CARD_3);
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "1 credit card in storage");
+  await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
+    async function(browser) {
+      let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
+                                                       "popupshown");
+      await ContentTask.spawn(browser, null, async function() {
+        let form = content.document.getElementById("form");
+        let name = form.querySelector("#cc-name");
+        name.focus();
+
+        name.setUserInput("User 3");
+        form.querySelector("#cc-number").setUserInput("9999888877776666");
+        form.querySelector("#cc-exp-month").setUserInput("1");
+        form.querySelector("#cc-exp-year").setUserInput("2000");
+
+        // Wait 1000ms before submission to make sure the input value applied
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        form.querySelector("input[type=submit]").click();
+      });
+      await promiseShown;
       await clickDoorhangerButton(MAIN_BUTTON);
     }
   );
+
+  creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards[0]["cc-name"], "User 3", "Verify the name field");
+  await removeAllRecords();
 });
--- a/browser/extensions/formautofill/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -1,17 +1,17 @@
 /* exported MANAGE_ADDRESSES_DIALOG_URL, MANAGE_CREDIT_CARDS_DIALOG_URL, EDIT_ADDRESS_DIALOG_URL, EDIT_CREDIT_CARD_DIALOG_URL,
             BASE_URL, TEST_ADDRESS_1, TEST_ADDRESS_2, TEST_ADDRESS_3, TEST_ADDRESS_4, TEST_ADDRESS_5,
             TEST_CREDIT_CARD_1, TEST_CREDIT_CARD_2, TEST_CREDIT_CARD_3, FORM_URL, CREDITCARD_FORM_URL,
             FTU_PREF, ENABLED_AUTOFILL_ADDRESSES_PREF, AUTOFILL_CREDITCARDS_AVAILABLE_PREF, ENABLED_AUTOFILL_CREDITCARDS_PREF,
             SYNC_USERNAME_PREF, SYNC_ADDRESSES_PREF, SYNC_CREDITCARDS_PREF, SYNC_CREDITCARDS_AVAILABLE_PREF,
             sleep, expectPopupOpen, openPopupOn, expectPopupClose, closePopup, clickDoorhangerButton,
             getAddresses, saveAddress, removeAddresses, saveCreditCard,
             getDisplayedPopupItems, getDoorhangerCheckbox, waitForMasterPasswordDialog,
-            getNotification, getDoorhangerButton */
+            getNotification, getDoorhangerButton, removeAllRecords */
 
 "use strict";
 
 Cu.import("resource://testing-common/LoginTestUtils.jsm", this);
 
 const MANAGE_ADDRESSES_DIALOG_URL = "chrome://formautofill/content/manageAddresses.xhtml";
 const MANAGE_CREDIT_CARDS_DIALOG_URL = "chrome://formautofill/content/manageCreditCards.xhtml";
 const EDIT_ADDRESS_DIALOG_URL = "chrome://formautofill/content/editAddress.xhtml";
@@ -272,19 +272,21 @@ function waitForMasterPasswordDialog(log
       dialog.ui.password1Textbox.value = LoginTestUtils.masterPassword.masterPassword;
       dialog.ui.button0.click();
     } else {
       dialog.ui.button1.click();
     }
   });
 }
 
-registerCleanupFunction(async function() {
+async function removeAllRecords() {
   let addresses = await getAddresses();
   if (addresses.length) {
     await removeAddresses(addresses.map(address => address.guid));
   }
 
   let creditCards = await getCreditCards();
   if (creditCards.length) {
     await removeCreditCards(creditCards.map(cc => cc.guid));
   }
-});
+}
+
+registerCleanupFunction(removeAllRecords);