Address review feedback

1. Apply reducer pattern 2. Move autofill method from filtering to autofill Auditor: @bridiver
brave · Oct 11, 2016 · d5e9876 · d5e9876
1 parent d9a1bd7
commit d5e9876
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 115 deletions.
diff --git a/app/autofill.js b/app/autofill.js
@@ -2,13 +2,55 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+const electron = require('electron')
+const session = electron.session
 const appActions = require('../js/actions/appActions')
 
 module.exports.init = () => {
   process.on('personal-data-changed', (profileGuids, creditCardGuids) => {
     setImmediate(() => {
-      appActions.updateAutofillAddress(profileGuids)
-      appActions.updateAutofillCreditCard(creditCardGuids)
+      appActions.autofillDataChanged(profileGuids, creditCardGuids)
     })
   })
 }
+
+module.exports.addAutofillAddress = (detail, guid) => {
+  session.defaultSession.autofill.addProfile({
+    full_name: detail.name,
+    company_name: detail.organization,
+    street_address: detail.streetAddress,
+    city: detail.city,
+    state: detail.state,
+    postal_code: detail.postalCode,
+    country_code: detail.country,
+    phone: detail.phone,
+    email: detail.email,
+    guid: guid
+  })
+}
+
+module.exports.removeAutofillAddress = (guid) => {
+  session.defaultSession.autofill.removeProfile(guid)
+}
+
+module.exports.addAutofillCreditCard = (detail, guid) => {
+  session.defaultSession.autofill.addCreditCard({
+    name: detail.name,
+    card_number: detail.card,
+    expiration_month: detail.month,
+    expiration_year: detail.year,
+    guid: guid
+  })
+}
+
+module.exports.removeAutofillCreditCard = (guid) => {
+  session.defaultSession.autofill.removeCreditCard(guid)
+}
+
+module.exports.clearAutocompleteData = () => {
+  session.defaultSession.autofill.clearAutocompleteData()
+}
+
+module.exports.clearAutofillData = () => {
+  session.defaultSession.autofill.clearAutofillData()
+}
diff --git a/app/filtering.js b/app/filtering.js
@@ -645,53 +645,6 @@ module.exports.setDefaultZoomLevel = (zoom) => {
   }
 }
 
-module.exports.addAutofillAddress = (detail, guid) => {
-  session.defaultSession.autofill.addProfile({
-    full_name: detail.name,
-    company_name: detail.organization,
-    street_address: detail.streetAddress,
-    city: detail.city,
-    state: detail.state,
-    postal_code: detail.postalCode,
-    country_code: detail.country,
-    phone: detail.phone,
-    email: detail.email,
-    guid: guid
-  })
-}
-
-module.exports.removeAutofillAddress = (guid) => {
-  session.defaultSession.autofill.removeProfile(guid)
-}
-
-module.exports.addAutofillCreditCard = (detail, guid) => {
-  session.defaultSession.autofill.addCreditCard({
-    name: detail.name,
-    card_number: detail.card,
-    expiration_month: detail.month,
-    expiration_year: detail.year,
-    guid: guid
-  })
-}
-
-module.exports.removeAutofillCreditCard = (guid) => {
-  session.defaultSession.autofill.removeCreditCard(guid)
-}
-
-module.exports.clearAutocompleteData = () => {
-  for (let partition in registeredSessions) {
-    let ses = registeredSessions[partition]
-    ses.autofill.clearAutocompleteData()
-  }
-}
-
-module.exports.clearAutofillData = () => {
-  for (let partition in registeredSessions) {
-    let ses = registeredSessions[partition]
-    ses.autofill.clearAutofillData()
-  }
-}
-
 module.exports.getMainFrameUrl = (details) => {
   if (details.resourceType === 'mainFrame') {
     return details.url

diff --git a/app/sessionStore.js b/app/sessionStore.js
@@ -24,6 +24,7 @@ const {tabFromFrame} = require('../js/state/frameStateUtil')
 const siteUtil = require('../js/state/siteUtil')
 const sessionStorageVersion = 1
 const filtering = require('./filtering')
+const autofill = require('./autofill')
 // const tabState = require('./common/state/tabState')
 
 const getSetting = require('../js/settings').getSetting
@@ -242,11 +243,11 @@ module.exports.cleanAppData = (data, isShutdown) => {
   }
   const clearAutocompleteData = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_AUTOCOMPLETE_DATA) === true
   if (clearAutocompleteData) {
-    filtering.clearAutocompleteData()
+    autofill.clearAutocompleteData()
   }
   const clearAutofillData = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_AUTOFILL_DATA) === true
   if (clearAutofillData) {
-    filtering.clearAutofillData()
+    autofill.clearAutofillData()
   }
   const clearSiteSettings = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_SITE_SETTINGS) === true
   if (clearSiteSettings) {

diff --git a/docs/appActions.md b/docs/appActions.md
@@ -381,16 +381,6 @@ Remove address data
 
 
 
-### updateAutofillAddress(guids) 
-
-Update guids of address
-
-**Parameters**
-
-**guids**: `Array`, the guid array to access entries in autofill DB
-
-
-
 ### addAutofillCreditCard(detail, originalDetail) 
 
 Add credit card data
@@ -413,13 +403,15 @@ Remove credit card data
 
 
 
-### updateAutofillCreditCard(guids) 
+### autofillDataChanged(addressGuids, creditCardGuids) 
 
-Update guids of credit card
+Autofill data changed
 
 **Parameters**
 
-**guids**: `Array`, the guid array to access entries in autofill DB
+**addressGuids**: `Array`, the guid array to access address entries in autofill DB
+
+**creditCardGuids**: `Array`, the guid array to access credit card entries in autofill DB
 
 
 

diff --git a/js/actions/appActions.js b/js/actions/appActions.js
@@ -441,17 +441,6 @@ const appActions = {
     })
   },
 
-  /**
-   * Update guids of address
-   * @param {Array} guids - the guid array to access entries in autofill DB
-   */
-  updateAutofillAddress: function (guids) {
-    AppDispatcher.dispatch({
-      actionType: AppConstants.APP_UPDATE_AUTOFILL_ADDRESS,
-      guids
-    })
-  },
-
   /**
    * Add credit card data
    * @param {object} detail - the credit card to add as per doc/state.md's autofillCreditCardDetail
@@ -477,13 +466,15 @@ const appActions = {
   },
 
   /**
-   * Update guids of credit card
-   * @param {Array} guids - the guid array to access entries in autofill DB
+   * Autofill data changed
+   * @param {Array} addressGuids - the guid array to access address entries in autofill DB
+   * @param {Array} creditCardGuids - the guid array to access credit card entries in autofill DB
    */
-  updateAutofillCreditCard: function (guids) {
+  autofillDataChanged: function (addressGuids, creditCardGuids) {
     AppDispatcher.dispatch({
-      actionType: AppConstants.APP_UPDATE_AUTOFILL_CREDIT_CARD,
-      guids
+      actionType: AppConstants.APP_AUTOFILL_DATA_CHANGED,
+      addressGuids,
+      creditCardGuids
     })
   },
 

diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js
@@ -40,10 +40,9 @@ const AppConstants = {
   APP_SET_DICTIONARY: _, /** @param {string} locale */
   APP_ADD_AUTOFILL_ADDRESS: _,
   APP_REMOVE_AUTOFILL_ADDRESS: _,
-  APP_UPDATE_AUTOFILL_ADDRESS: _,
   APP_ADD_AUTOFILL_CREDIT_CARD: _,
   APP_REMOVE_AUTOFILL_CREDIT_CARD: _,
-  APP_UPDATE_AUTOFILL_CREDIT_CARD: _,
+  APP_AUTOFILL_DATA_CHANGED: _,
   APP_SET_LOGIN_REQUIRED_DETAIL: _,
   APP_SET_LOGIN_RESPONSE_DETAIL: _,
   APP_WINDOW_BLURRED: _,

diff --git a/js/stores/appStore.js b/js/stores/appStore.js
@@ -32,6 +32,7 @@ const locale = require('../../app/locale')
 const path = require('path')
 const {channel} = require('../../app/channel')
 const os = require('os')
+const autofill = require('../../app/autofill')
 
 // state helpers
 const basicAuthState = require('../../app/common/state/basicAuthState')
@@ -606,12 +607,10 @@ const handleAppAction = (action) => {
         Filtering.clearStorageData()
       }
       if (action.clearDataDetail.get('autocompleteData')) {
-        const Filtering = require('../../app/filtering')
-        Filtering.clearAutocompleteData()
+        autofill.clearAutocompleteData()
       }
       if (action.clearDataDetail.get('autofillData')) {
-        const Filtering = require('../../app/filtering')
-        Filtering.clearAutofillData()
+        autofill.clearAutofillData()
       }
       if (action.clearDataDetail.get('savedSiteSettings')) {
         appState = appState.set('siteSettings', Immutable.Map())
@@ -631,39 +630,25 @@ const handleAppAction = (action) => {
         break
       }
     case AppConstants.APP_ADD_AUTOFILL_ADDRESS:
-      {
-        const Filtering = require('../../app/filtering')
-        Filtering.addAutofillAddress(action.detail.toJS(),
-          action.originalDetail.get('guid') === undefined ? '-1' : action.originalDetail.get('guid'))
-        break
-      }
+      autofill.addAutofillAddress(action.detail.toJS(),
+        action.originalDetail.get('guid') === undefined ? '-1' : action.originalDetail.get('guid'))
+      break
     case AppConstants.APP_REMOVE_AUTOFILL_ADDRESS:
-      {
-        const Filtering = require('../../app/filtering')
-        Filtering.removeAutofillAddress(action.detail.get('guid'))
-        break
-      }
-    case AppConstants.APP_UPDATE_AUTOFILL_ADDRESS:
-      appState = appState.setIn(['autofill', 'addresses', 'guid'], action.guids)
-      appState = appState.setIn(['autofill', 'addresses', 'timestamp'], new Date().getTime())
+      autofill.removeAutofillAddress(action.detail.get('guid'))
       break
     case AppConstants.APP_ADD_AUTOFILL_CREDIT_CARD:
-      {
-        const Filtering = require('../../app/filtering')
-
-        Filtering.addAutofillCreditCard(action.detail.toJS(),
-          action.originalDetail.get('guid') === undefined ? '-1' : action.originalDetail.get('guid'))
-        break
-      }
+      autofill.addAutofillCreditCard(action.detail.toJS(),
+        action.originalDetail.get('guid') === undefined ? '-1' : action.originalDetail.get('guid'))
+      break
     case AppConstants.APP_REMOVE_AUTOFILL_CREDIT_CARD:
-      {
-        const Filtering = require('../../app/filtering')
-        Filtering.removeAutofillCreditCard(action.detail.get('guid'))
-        break
-      }
-    case AppConstants.APP_UPDATE_AUTOFILL_CREDIT_CARD:
-      appState = appState.setIn(['autofill', 'creditCards', 'guid'], action.guids)
-      appState = appState.setIn(['autofill', 'creditCards', 'timestamp'], new Date().getTime())
+      autofill.removeAutofillCreditCard(action.detail.get('guid'))
+      break
+    case AppConstants.APP_AUTOFILL_DATA_CHANGED:
+      const date = new Date().getTime()
+      appState = appState.setIn(['autofill', 'addresses', 'guid'], action.addressGuids)
+      appState = appState.setIn(['autofill', 'addresses', 'timestamp'], date)
+      appState = appState.setIn(['autofill', 'creditCards', 'guid'], action.creditCardGuids)
+      appState = appState.setIn(['autofill', 'creditCards', 'timestamp'], date)
       break
     case AppConstants.APP_SET_LOGIN_REQUIRED_DETAIL:
       appState = basicAuthState.setLoginRequiredDetail(appState, action.tabId, action.detail)