Bug 1494551 - Sort the formautofill data by the last used time. r=MattN
authorJared Wein <jwein@mozilla.com>
Fri, 19 Oct 2018 17:00:31 +0000
changeset 490540 388806780facbc345aa5ce0e7cc6b6bc108b29e8
parent 490539 0e7248074acf71bb6553a68c53299f2fd1b35329
child 490541 140a56e90594382a0c24298b08a5a853e95c0b32
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersMattN
bugs1494551
milestone64.0a1
Bug 1494551 - Sort the formautofill data by the last used time. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D9174
browser/components/payments/content/paymentDialogWrapper.js
browser/components/payments/res/containers/payment-dialog.js
browser/components/payments/res/debugging.js
browser/components/payments/res/paymentRequest.js
browser/components/payments/test/browser/browser_change_shipping.js
browser/components/payments/test/mochitest/test_address_picker.html
browser/components/payments/test/mochitest/test_payment_method_picker.html
browser/extensions/formautofill/content/manageDialog.js
--- a/browser/components/payments/content/paymentDialogWrapper.js
+++ b/browser/components/payments/content/paymentDialogWrapper.js
@@ -66,17 +66,18 @@ class TempCollection {
   async update(guid, record, preserveOldProperties) {
     let recordToSave = Object.assign(preserveOldProperties ? this._data[guid] : {}, record);
     await this._formAutofillCollection.computeFields(recordToSave);
     return (this._data[guid] = recordToSave);
   }
 
   async add(record) {
     let guid = "temp-" + Math.abs(Math.random() * 0xffffffff|0);
-    let recordToSave = Object.assign({guid}, record);
+    let timeLastModified = Date.now();
+    let recordToSave = Object.assign({guid, timeLastModified}, record);
     await this._formAutofillCollection.computeFields(recordToSave);
     this._data[guid] = recordToSave;
     return guid;
   }
 
   getAll() {
     return this._data;
   }
@@ -497,32 +498,49 @@ var paymentDialogWrapper = {
     paymentSrv.respondPayment(showResponse);
     window.close();
   },
 
   async onPay({
     selectedPayerAddressGUID: payerGUID,
     selectedPaymentCardGUID: paymentCardGUID,
     selectedPaymentCardSecurityCode: cardSecurityCode,
+    selectedShippingAddressGUID: shippingGUID,
   }) {
     let methodData = await this._convertProfileBasicCardToPaymentMethodData(paymentCardGUID,
                                                                             cardSecurityCode);
 
     if (!methodData) {
       // TODO (Bug 1429265/Bug 1429205): Handle when a user hits cancel on the
       // Master Password dialog.
       Cu.reportError("Bug 1429265/Bug 1429205: User canceled master password entry");
       return;
     }
 
-    let {
-      payerName,
-      payerEmail,
-      payerPhone,
-    } = await this._convertProfileAddressToPayerData(payerGUID);
+    let payerName = "";
+    let payerEmail = "";
+    let payerPhone = "";
+    if (payerGUID) {
+      let payerData = await this._convertProfileAddressToPayerData(payerGUID);
+      payerName = payerData.payerName;
+      payerEmail = payerData.payerEmail;
+      payerPhone = payerData.payerPhone;
+    }
+
+    // Update the lastUsedTime for the payerAddress and paymentCard. Check if
+    // the record exists in formAutofillStorage because it may be temporary.
+    if (shippingGUID && await formAutofillStorage.addresses.get(shippingGUID)) {
+      formAutofillStorage.addresses.notifyUsed(shippingGUID);
+    }
+    if (payerGUID && await formAutofillStorage.addresses.get(payerGUID)) {
+      formAutofillStorage.addresses.notifyUsed(payerGUID);
+    }
+    if (await formAutofillStorage.creditCards.get(paymentCardGUID)) {
+      formAutofillStorage.creditCards.notifyUsed(paymentCardGUID);
+    }
 
     this.pay({
       methodName: "basic-card",
       methodData,
       payerName,
       payerEmail,
       payerPhone,
     });
--- a/browser/components/payments/res/containers/payment-dialog.js
+++ b/browser/components/payments/res/containers/payment-dialog.js
@@ -97,27 +97,40 @@ export default class PaymentDialog exten
     event.preventDefault();
   }
 
   cancelRequest() {
     paymentRequest.cancel();
   }
 
   pay() {
+    let state = this.requestStore.getState();
     let {
       selectedPayerAddress,
       selectedPaymentCard,
       selectedPaymentCardSecurityCode,
-    } = this.requestStore.getState();
+      selectedShippingAddress,
+    } = state;
 
-    paymentRequest.pay({
-      selectedPayerAddressGUID: selectedPayerAddress,
+    let data = {
       selectedPaymentCardGUID: selectedPaymentCard,
       selectedPaymentCardSecurityCode,
-    });
+    };
+
+    data.selectedShippingAddressGUID =
+      state.request.paymentOptions.requestShipping ?
+      selectedShippingAddress :
+      null;
+
+    data.selectedPayerAddressGUID =
+      this._isPayerRequested(state.request.paymentOptions) ?
+      selectedPayerAddress :
+      null;
+
+    paymentRequest.pay(data);
   }
 
   changeShippingAddress(shippingAddressGUID) {
     paymentRequest.changeShippingAddress({
       shippingAddressGUID,
     });
   }
 
--- a/browser/components/payments/res/debugging.js
+++ b/browser/components/payments/res/debugging.js
@@ -203,130 +203,139 @@ let ADDRESSES_1 = {
     "email": "foo@bar.com",
     "family-name": "Smith",
     "given-name": "John",
     "guid": "48bnds6854t",
     "name": "John Smith",
     "postal-code": "90210",
     "street-address": "123 Sesame Street,\nApt 40",
     "tel": "+1 519 555-5555",
+    timeLastUsed: 50000,
   },
   "68gjdh354j": {
     "additional-name": "Z.",
     "address-level1": "CA",
     "address-level2": "Mountain View",
     "country": "US",
     "family-name": "Doe",
     "given-name": "Jane",
     "guid": "68gjdh354j",
     "name": "Jane Z. Doe",
     "postal-code": "94041",
     "street-address": "P.O. Box 123",
     "tel": "+1 650 555-5555",
+    timeLastUsed: 30000,
   },
   "abcde12345": {
     "address-level2": "Mountain View",
     "country": "US",
     "family-name": "Fields",
     "given-name": "Mrs.",
     "guid": "abcde12345",
     "name": "Mrs. Fields",
+    timeLastUsed: 70000,
   },
   "german1": {
     "additional-name": "Y.",
     "address-level1": "",
     "address-level2": "Berlin",
     "country": "DE",
     "email": "de@example.com",
     "family-name": "Mouse",
     "given-name": "Anon",
     "guid": "german1",
     "name": "Anon Y. Mouse",
     "organization": "Mozilla",
     "postal-code": "10997",
     "street-address": "Schlesische Str. 27",
     "tel": "+49 30 983333002",
+    timeLastUsed: 10000,
   },
   "missing-country": {
     "address-level1": "ON",
     "address-level2": "Toronto",
     "family-name": "Bogard",
     "given-name": "Kristin",
     "guid": "missing-country",
     "name": "Kristin Bogard",
     "postal-code": "H0H 0H0",
     "street-address": "123 Yonge Street\nSuite 2300",
     "tel": "+1 416 555-5555",
+    timeLastUsed: 90000,
   },
 };
 
 let DUPED_ADDRESSES = {
   "a9e830667189": {
     "street-address": "Unit 1\n1505 Northeast Kentucky Industrial Parkway \n",
     "address-level2": "Greenup",
     "address-level1": "KY",
     "postal-code": "41144",
     "country": "US",
     "email": "bob@example.com",
     "family-name": "Smith",
     "given-name": "Bob",
     "guid": "a9e830667189",
     "tel": "+19871234567",
     "name": "Bob Smith",
+    timeLastUsed: 10001,
   },
   "72a15aed206d": {
     "street-address": "1 New St",
     "address-level2": "York",
     "address-level1": "SC",
     "postal-code": "29745",
     "country": "US",
     "given-name": "Mary Sue",
     "guid": "72a15aed206d",
     "tel": "+19871234567",
     "name": "Mary Sue",
     "address-line1": "1 New St",
+    timeLastUsed: 10009,
   },
   "2b4dce0fbc1f": {
     "street-address": "123 Park St",
     "address-level2": "Springfield",
     "address-level1": "OR",
     "postal-code": "97403",
     "country": "US",
     "email": "rita@foo.com",
     "family-name": "Foo",
     "given-name": "Rita",
     "guid": "2b4dce0fbc1f",
     "name": "Rita Foo",
     "address-line1": "123 Park St",
+    timeLastUsed: 10005,
   },
   "46b2635a5b26": {
     "street-address": "432 Another St",
     "address-level2": "Springfield",
     "address-level1": "OR",
     "postal-code": "97402",
     "country": "US",
     "email": "rita@foo.com",
     "family-name": "Foo",
     "given-name": "Rita",
     "guid": "46b2635a5b26",
     "name": "Rita Foo",
     "address-line1": "432 Another St",
+    timeLastUsed: 10003,
   },
 };
 
 let BASIC_CARDS_1 = {
   "53f9d009aed2": {
     billingAddressGUID: "68gjdh354j",
     methodName: "basic-card",
     "cc-number": "************5461",
     "guid": "53f9d009aed2",
     "version": 1,
     "timeCreated": 1505240896213,
     "timeLastModified": 1515609524588,
-    "timeLastUsed": 0,
+    "timeLastUsed": 10000,
     "timesUsed": 0,
     "cc-name": "John Smith",
     "cc-exp-month": 6,
     "cc-exp-year": 2024,
     "cc-type": "visa",
     "cc-given-name": "John",
     "cc-additional-name": "",
     "cc-family-name": "Smith",
@@ -334,17 +343,17 @@ let BASIC_CARDS_1 = {
   },
   "9h5d4h6f4d1s": {
     methodName: "basic-card",
     "cc-number": "************0954",
     "guid": "9h5d4h6f4d1s",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 50000,
     "timesUsed": 0,
     "cc-name": "Jane Doe",
     "cc-exp-month": 5,
     "cc-exp-year": 2023,
     "cc-type": "mastercard",
     "cc-given-name": "Jane",
     "cc-additional-name": "",
     "cc-family-name": "Doe",
@@ -352,33 +361,33 @@ let BASIC_CARDS_1 = {
   },
   "123456789abc": {
     methodName: "basic-card",
     "cc-number": "************1234",
     "guid": "123456789abc",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 90000,
     "timesUsed": 0,
     "cc-name": "Jane Fields",
     "cc-given-name": "Jane",
     "cc-additional-name": "",
     "cc-family-name": "Fields",
     "cc-type": "discover",
   },
   "amex-card": {
     methodName: "basic-card",
     billingAddressGUID: "68gjdh354j",
     "cc-number": "************1941",
     "guid": "amex-card",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 70000,
     "timesUsed": 0,
     "cc-name": "Capt America",
     "cc-given-name": "Capt",
     "cc-additional-name": "",
     "cc-family-name": "America",
     "cc-type": "amex",
     "cc-exp-month": 6,
     "cc-exp-year": 2023,
@@ -386,17 +395,17 @@ let BASIC_CARDS_1 = {
   },
   "missing-cc-name": {
     methodName: "basic-card",
     "cc-number": "************8563",
     "guid": "missing-cc-name",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 30000,
     "timesUsed": 0,
     "cc-exp-month": 8,
     "cc-exp-year": 2024,
     "cc-exp": "2024-08",
   },
 };
 
 let buttonActions = {
--- a/browser/components/payments/res/paymentRequest.js
+++ b/browser/components/payments/res/paymentRequest.js
@@ -268,22 +268,35 @@ var paymentRequest = {
     return state.request.paymentDetails.totalItem;
   },
 
   onPaymentRequestUnload() {
     // remove listeners that may be used multiple times here
     window.removeEventListener("paymentChromeToContent", this);
   },
 
+  _sortObjectsByTimeLastUsed(objects) {
+    let sortedValues = Object.values(objects).sort((a, b) => {
+      let aLastUsed = a.timeLastUsed || a.timeLastModified;
+      let bLastUsed = b.timeLastUsed || b.timeLastModified;
+      return bLastUsed - aLastUsed;
+    });
+    let sortedObjects = {};
+    for (let obj of sortedValues) {
+      sortedObjects[obj.guid] = obj;
+    }
+    return sortedObjects;
+  },
+
   getAddresses(state) {
     let addresses = Object.assign({}, state.savedAddresses, state.tempAddresses);
-    return addresses;
+    return this._sortObjectsByTimeLastUsed(addresses);
   },
 
   getBasicCards(state) {
     let cards = Object.assign({}, state.savedBasicCards, state.tempBasicCards);
-    return cards;
+    return this._sortObjectsByTimeLastUsed(cards);
   },
 };
 
 paymentRequest.init();
 
 export default paymentRequest;
--- a/browser/components/payments/test/browser/browser_change_shipping.js
+++ b/browser/components/payments/test/browser/browser_change_shipping.js
@@ -326,17 +326,19 @@ add_task(async function test_address_edi
 
     addressOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("initial addressOptions: " + JSON.stringify(addressOptions));
     selectedIndex = addressOptions.selectedOptionIndex;
     let selectedAddressGuid = addressOptions.options[selectedIndex].guid;
     let selectedAddress = await formAutofillStorage.addresses.get(selectedAddressGuid);
 
-    is(selectedIndex, 0, "First address should be selected");
+    // US address is inserted first, then German address, so German address
+    // has more recent timeLastModified and will appear at the top of the list.
+    is(selectedIndex, 1, "Second address should be selected");
     ok(selectedAddress, "Selected address does exist in the address collection");
     is(selectedAddress.country, "US", "Expected initial country value");
 
     info("Updating the address directly in the store");
     await formAutofillStorage.addresses.update(selectedAddress.guid, {
       country: "CA",
     }, true);
 
@@ -344,17 +346,17 @@ add_task(async function test_address_edi
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("updated addressOptions: " + JSON.stringify(addressOptions));
     selectedIndex = addressOptions.selectedOptionIndex;
     let newSelectedAddressGuid = addressOptions.options[selectedIndex].guid;
 
     is(newSelectedAddressGuid, selectedAddressGuid, "Selected guid hasnt changed");
     selectedAddress = await formAutofillStorage.addresses.get(selectedAddressGuid);
 
-    is(selectedIndex, 0, "First address should be selected");
+    is(selectedIndex, 1, "Second address should be selected");
     is(selectedAddress.country, "CA", "Expected changed country value");
 
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 
   await cleanupFormAutofillStorage();
 });
@@ -378,17 +380,19 @@ add_task(async function test_address_rem
     await selectPaymentDialogShippingAddressByCountry(frame, "US");
 
     let addressOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("initial addressOptions: " + JSON.stringify(addressOptions));
     let selectedIndex = addressOptions.selectedOptionIndex;
     let selectedAddressGuid = addressOptions.options[selectedIndex].guid;
 
-    is(selectedIndex, 0, "First address should be selected");
+    // US address is inserted first, then German address, so German address
+    // has more recent timeLastModified and will appear at the top of the list.
+    is(selectedIndex, 1, "Second address should be selected");
     is(addressOptions.options.length, 2, "Should be 2 address options initially");
 
     info("Remove the selected address from the store");
     await formAutofillStorage.addresses.remove(selectedAddressGuid);
 
     await ContentTask.spawn(browser, {
       eventName: "shippingaddresschange",
     }, PTU.ContentTasks.promisePaymentRequestEvent);
--- a/browser/components/payments/test/mochitest/test_address_picker.html
+++ b/browser/components/payments/test/mochitest/test_address_picker.html
@@ -52,41 +52,44 @@ add_task(async function test_initialSet(
         "address-level1": "MI",
         "address-level2": "Some City",
         "country": "US",
         "guid": "48bnds6854t",
         "name": "Mr. Foo",
         "postal-code": "90210",
         "street-address": "123 Sesame Street,\nApt 40",
         "tel": "+1 519 555-5555",
+        timeLastUsed: 200,
       },
       "68gjdh354j": {
         "address-level1": "CA",
         "address-level2": "Mountain View",
         "country": "US",
         "guid": "68gjdh354j",
         "name": "Mrs. Bar",
         "postal-code": "94041",
         "street-address": "P.O. Box 123",
         "tel": "+1 650 555-5555",
+        timeLastUsed: 300,
       },
       "abcde12345": {
         "address-level2": "Mountain View",
         "country": "US",
         "guid": "abcde12345",
         "name": "Mrs. Fields",
+        timeLastUsed: 100,
       },
     },
   });
   await asyncElementRendered();
   let options = picker1.dropdown.popupBox.children;
   is(options.length, 3, "Check dropdown has all addresses");
-  ok(options[0].textContent.includes("Mr. Foo"), "Check first address");
-  ok(options[1].textContent.includes("Mrs. Bar"), "Check second address");
-  ok(options[2].textContent.includes("Mrs. Fields"), "Check third address");
+  ok(options[0].textContent.includes("Mrs. Bar"), "Check first address based on timeLastUsed");
+  ok(options[1].textContent.includes("Mr. Foo"), "Check second address based on timeLastUsed");
+  ok(options[2].textContent.includes("Mrs. Fields"), "Check third address based on timeLastUsed");
 });
 
 add_task(async function test_update() {
   picker1.requestStore.setState({
     savedAddresses: {
       "48bnds6854t": {
         // Same GUID, different values to trigger an update
         "address-level1": "MI-edit",
--- a/browser/components/payments/test/mochitest/test_payment_method_picker.html
+++ b/browser/components/payments/test/mochitest/test_payment_method_picker.html
@@ -39,56 +39,59 @@ let picker1 = document.getElementById("p
 add_task(async function test_empty() {
   ok(picker1, "Check picker1 exists");
   let {savedBasicCards} = picker1.requestStore.getState();
   is(Object.keys(savedBasicCards).length, 0, "Check empty initial state");
   is(picker1.dropdown.popupBox.children.length, 0, "Check dropdown is empty");
 });
 
 add_task(async function test_initialSet() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       "48bnds6854t": {
         "cc-exp": "2017-02",
         "cc-exp-month": 2,
         "cc-exp-year": 2017,
         "cc-name": "John Doe",
         "cc-number": "************9999",
         "cc-type": "mastercard",
         "guid": "48bnds6854t",
+        timeLastUsed: 300,
       },
       "68gjdh354j": {
         "cc-exp": "2017-08",
         "cc-exp-month": 8,
         "cc-exp-year": 2017,
         "cc-name": "J Smith",
         "cc-number": "***********1234",
         "cc-type": "visa",
         "guid": "68gjdh354j",
+        timeLastUsed: 100,
       },
       "123456789abc": {
         "cc-name": "Jane Fields",
         "cc-given-name": "Jane",
         "cc-additional-name": "",
         "cc-family-name": "Fields",
         "cc-number": "************9876",
         "guid": "123456789abc",
+        timeLastUsed: 200,
       },
     },
   });
   await asyncElementRendered();
   let options = picker1.dropdown.popupBox.children;
   is(options.length, 3, "Check dropdown has all three cards");
-  ok(options[0].textContent.includes("John Doe"), "Check first card");
-  ok(options[1].textContent.includes("J Smith"), "Check second card");
-  ok(options[2].textContent.includes("Jane Fields"), "Check third card");
+  ok(options[0].textContent.includes("John Doe"), "Check first card based on timeLastUsed");
+  ok(options[1].textContent.includes("Jane Fields"), "Check second card based on timeLastUsed");
+  ok(options[2].textContent.includes("J Smith"), "Check third card based on timeLastUsed");
 });
 
 add_task(async function test_update() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       "48bnds6854t": {
         // Same GUID, different values to trigger an update
         "cc-exp": "2017-09",
         "cc-exp-month": 9,
         "cc-exp-year": 2017,
         // cc-name was cleared which means it's not returned
         "cc-number": "************9876",
@@ -178,17 +181,17 @@ add_task(async function test_change_sele
   picker1.securityCodeInput.querySelector("input").focus();
   sendString("836");
   sendKey("Tab");
   let state = await stateChangePromise;
   ok(state.selectedPaymentCardSecurityCode, "836", "Check security code in state");
 });
 
 add_task(async function test_delete() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       // 48bnds6854t was deleted
       "68gjdh354j": {
         "cc-exp": "2017-08",
         "cc-exp-month": 8,
         "cc-exp-year": 2017,
         "cc-name": "J Smith",
         "cc-number": "***********1234",
--- a/browser/extensions/formautofill/content/manageDialog.js
+++ b/browser/extensions/formautofill/content/manageDialog.js
@@ -99,18 +99,22 @@ class ManageRecords {
 
     // For testing only: Notify when records are loaded
     this._elements.records.dispatchEvent(new CustomEvent("RecordsLoaded"));
   }
 
   async _loadRecords() {
     let storage = await this.getStorage();
     let records = await storage.getAll();
-    // Sort by last modified time starting with most recent
-    records.sort((a, b) => b.timeLastModified - a.timeLastModified);
+    // Sort by last used time starting with most recent
+    records.sort((a, b) => {
+      let aLastUsed = a.timeLastUsed || a.timeLastModified;
+      let bLastUsed = b.timeLastUsed || b.timeLastModified;
+      return bLastUsed - aLastUsed;
+    });
     await this.renderRecordElements(records);
     this.updateButtonsStates(this._selectedOptions.length);
   }
 
   /**
    * Render the records onto the page while maintaining selected options if
    * they still exist.
    *