Bug 1432952 - Add a billingAddressGUID property to saved credit cards. r=jaws,kitcambridge draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 10 Apr 2018 15:55:20 -0700
changeset 780011 3175d62bb3a46ca502f1d9aec1b305fb854e3fad
parent 779997 0528a414c2a86dad0623779abde5301d37337934
child 780012 e28eacb8c2c5d1e58c7e93ef71bd50226dfa08fd
push id105938
push usermozilla@noorenberghe.ca
push dateWed, 11 Apr 2018 02:32:24 +0000
reviewersjaws, kitcambridge
bugs1432952
milestone61.0a1
Bug 1432952 - Add a billingAddressGUID property to saved credit cards. r=jaws,kitcambridge test_reconcile.js already provides great test coverage for a new field (like billingAddressGUID) being added either locally or remotely. MozReview-Commit-ID: 5IGoH3zB1rz
browser/extensions/formautofill/FormAutofillStorage.jsm
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
--- a/browser/extensions/formautofill/FormAutofillStorage.jsm
+++ b/browser/extensions/formautofill/FormAutofillStorage.jsm
@@ -52,16 +52,19 @@
  *     }
  *   ],
  *   creditCards: [
  *     {
  *       guid,                 // 12 characters
  *       version,              // schema version in integer
  *
  *       // credit card fields
+ *       billingAddressGUID,   // An optional GUID of an autofill address record
+ *                                which may or may not exist locally.
+ *
  *       cc-name,
  *       cc-number,            // will be stored in masked format (************1234)
  *                             // (see details below)
  *       cc-exp-month,
  *       cc-exp-year,          // 2-digit year will be converted to 4 digits
  *                             // upon saving
  *
  *       // computed fields (These fields are computed based on the above fields
@@ -191,16 +194,17 @@ const TEL_COMPONENTS = [
 ];
 
 const VALID_ADDRESS_COMPUTED_FIELDS = [
   "name",
   "country-name",
 ].concat(STREET_ADDRESS_COMPONENTS, TEL_COMPONENTS);
 
 const VALID_CREDIT_CARD_FIELDS = [
+  "billingAddressGUID",
   "cc-name",
   "cc-number",
   "cc-exp-month",
   "cc-exp-year",
 ];
 
 const VALID_CREDIT_CARD_COMPUTED_FIELDS = [
   "cc-given-name",
@@ -1646,17 +1650,17 @@ class CreditCards extends AutofillRecord
         break;
       }
     }
 
     delete creditCard["cc-exp"];
   }
 
   /**
-   * Normailze the given record and retrun the first matched guid if storage has the same record.
+   * Normalize the given record and return the first matched guid if storage has the same record.
    * @param {Object} targetCreditCard
    *        The credit card for duplication checking.
    * @returns {string|null}
    *          Return the first guid if storage has the same credit card and null otherwise.
    */
   getDuplicateGuid(targetCreditCard) {
     let clonedTargetCreditCard = this._clone(targetCreditCard);
     this._normalizeRecord(clonedTargetCreditCard);
@@ -1708,17 +1712,17 @@ class CreditCards extends AutofillRecord
     for (let field of this.VALID_FIELDS) {
       let existingField = creditCardFound[field];
 
       // Make sure credit card field is existed and have value
       if (field == "cc-number" && (!existingField || !creditCardToMerge[field])) {
         return false;
       }
 
-      if (!creditCardToMerge[field]) {
+      if (!creditCardToMerge[field] && typeof(existingField) != "undefined") {
         creditCardToMerge[field] = existingField;
       }
 
       let incomingField = creditCardToMerge[field];
       if (incomingField && existingField) {
         if (incomingField != existingField) {
           this.log.debug("Conflicts: field", field, "has different value.");
           return false;
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -31,17 +31,24 @@ const TEST_CREDIT_CARD_3 = {
   "cc-exp-year": 2000,
 };
 
 const TEST_CREDIT_CARD_4 = {
   "cc-name": "Foo Bar",
   "cc-number": "9999888877776666",
 };
 
+const TEST_CREDIT_CARD_WITH_BILLING_ADDRESS = {
+  "cc-name": "J. Smith",
+  "cc-number": "4111111111111111",
+  billingAddressGUID: "9m6hf4gfr6ge",
+};
+
 const TEST_CREDIT_CARD_WITH_EMPTY_FIELD = {
+  billingAddressGUID: "",
   "cc-name": "",
   "cc-number": "1234123412341234",
   "cc-exp-month": 1,
 };
 
 const TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD = {
   "cc-given-name": "",
   "cc-additional-name": "",
@@ -102,16 +109,30 @@ const MERGE_TESTCASES = [
     expectedCreditCard: {
       "cc-name": "John Doe",
       "cc-number": "1234567812345678",
       "cc-exp-month": 4,
       "cc-exp-year": 2017,
     },
   },
   {
+    description: "Merge a superset with billingAddressGUID",
+    creditCardInStorage: {
+      "cc-number": "1234567812345678",
+    },
+    creditCardToMerge: {
+      "cc-number": "1234567812345678",
+      billingAddressGUID: "ijsnbhfr",
+    },
+    expectedCreditCard: {
+      "cc-number": "1234567812345678",
+      billingAddressGUID: "ijsnbhfr",
+    },
+  },
+  {
     description: "Merge a subset",
     creditCardInStorage: {
       "cc-name": "John Doe",
       "cc-number": "1234567812345678",
       "cc-exp-month": 4,
       "cc-exp-year": 2017,
     },
     creditCardToMerge: {
@@ -123,16 +144,31 @@ const MERGE_TESTCASES = [
       "cc-name": "John Doe",
       "cc-number": "1234567812345678",
       "cc-exp-month": 4,
       "cc-exp-year": 2017,
     },
     noNeedToUpdate: true,
   },
   {
+    description: "Merge a subset with billingAddressGUID",
+    creditCardInStorage: {
+      "cc-number": "1234567812345678",
+      billingAddressGUID: "8fhdb3ug6",
+    },
+    creditCardToMerge: {
+      "cc-number": "1234567812345678",
+    },
+    expectedCreditCard: {
+      billingAddressGUID: "8fhdb3ug6",
+      "cc-number": "1234567812345678",
+    },
+    noNeedToUpdate: true,
+  },
+  {
     description: "Merge an creditCard with partial overlaps",
     creditCardInStorage: {
       "cc-name": "John Doe",
       "cc-number": "1234567812345678",
     },
     creditCardToMerge: {
       "cc-number": "1234567812345678",
       "cc-exp-month": 4,
@@ -269,16 +305,17 @@ add_task(async function test_add() {
   Assert.equal(creditCards[0].timeLastUsed, 0);
   Assert.equal(creditCards[0].timesUsed, 0);
 
   // Empty string should be deleted before saving.
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_EMPTY_FIELD);
   let creditCard = profileStorage.creditCards._data[2];
   Assert.equal(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]);
   Assert.equal(creditCard["cc-name"], undefined);
+  Assert.equal(creditCard.billingAddressGUID, undefined);
 
   // Empty computed fields shouldn't cause any problem.
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD);
   creditCard = profileStorage.creditCards._data[3];
   Assert.equal(creditCard["cc-number"],
     profileStorage.creditCards._getMaskedCCNumber(TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD["cc-number"]));
 
   Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD),
@@ -286,16 +323,32 @@ add_task(async function test_add() {
 
   Assert.throws(() => profileStorage.creditCards.add({}),
     /Record contains no valid field\./);
 
   Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_EMPTY_AFTER_NORMALIZE),
     /Record contains no valid field\./);
 });
 
+add_task(async function test_addWithBillingAddress() {
+  let path = getTempFile(TEST_STORE_FILE_NAME).path;
+  let profileStorage = new FormAutofillStorage(path);
+  await profileStorage.initialize();
+
+  let creditCards = profileStorage.creditCards.getAll();
+
+  Assert.equal(creditCards.length, 0);
+
+  await profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_BILLING_ADDRESS);
+
+  creditCards = profileStorage.creditCards.getAll();
+  Assert.equal(creditCards.length, 1);
+  do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_WITH_BILLING_ADDRESS);
+});
+
 add_task(async function test_update() {
   // Test assumes that when an entry is saved a second time, it's last modified date will
   // be different from the first. With high values of precision reduction, we execute too
   // fast for that to be true.
   let timerPrecision = Preferences.get("privacy.reduceTimerPrecision");
   Preferences.set("privacy.reduceTimerPrecision", false);
 
   registerCleanupFunction(function() {
@@ -333,16 +386,17 @@ add_task(async function test_update() {
   Assert.notEqual(creditCard.timeLastModified, timeLastModified);
   do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_3);
 
   // Empty string should be deleted while updating.
   profileStorage.creditCards.update(profileStorage.creditCards._data[0].guid, TEST_CREDIT_CARD_WITH_EMPTY_FIELD);
   creditCard = profileStorage.creditCards._data[0];
   Assert.equal(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]);
   Assert.equal(creditCard["cc-name"], undefined);
+  Assert.equal(creditCard.billingAddressGUID, undefined);
 
   // Empty computed fields shouldn't cause any problem.
   profileStorage.creditCards.update(profileStorage.creditCards._data[0].guid, TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD, false);
   creditCard = profileStorage.creditCards._data[0];
   Assert.equal(creditCard["cc-number"],
     profileStorage.creditCards._getMaskedCCNumber(TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD["cc-number"]));
   profileStorage.creditCards.update(profileStorage.creditCards._data[1].guid, TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD, true);
   creditCard = profileStorage.creditCards._data[1];