From ddb661ed37deada5ca87a0c12c6f268eead5f0bf Mon Sep 17 00:00:00 2001
From: Juan Tejada <juans.tejada@gmail.com>
Date: Tue, 16 Feb 2016 18:03:22 -0800
Subject: [PATCH] fix(account-prefs): Update account save logic to prevent
 incorrect saves

- This fixes #1354 and #1235
- This issue was caused because the account details preferences
  component was keeping as state all of the account object fields. `setState` works
  like `extend`, so when the account changed, state was set to the new set of account fields,
  but the old values were only removed if they were overriden, and remained the
  same if the field did not exist in the new state object.
  Specifically, when a new account was added, setState was called with
  `{..., defaultAlias: undefined}` which did /not/ remove the  the defaultAlias
  from the previous state.
- Switched to managing state with a top level key `account`
---
 .../lib/tabs/preferences-account-details.jsx  | 44 +++++++++----------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/internal_packages/preferences/lib/tabs/preferences-account-details.jsx b/internal_packages/preferences/lib/tabs/preferences-account-details.jsx
index 6354afa6c..dc7027f03 100644
--- a/internal_packages/preferences/lib/tabs/preferences-account-details.jsx
+++ b/internal_packages/preferences/lib/tabs/preferences-account-details.jsx
@@ -12,11 +12,11 @@ class PreferencesAccountDetails extends Component {
 
   constructor(props) {
     super(props);
-    this.state = _.clone(props.account);
+    this.state = {account: _.clone(props.account)};
   }
 
   componentWillReceiveProps(nextProps) {
-    this.setState(_.clone(nextProps.account));
+    this.setState({account: _.clone(nextProps.account)});
   }
 
   componentWillUnmount() {
@@ -51,30 +51,32 @@ class PreferencesAccountDetails extends Component {
     return `${name} <${email}>`;
   }
 
-  _updatedDefaultAlias(originalAlias, newAlias, defaultAlias) {
-    if (originalAlias === defaultAlias) {
-      return newAlias;
-    }
-    return defaultAlias;
-  }
-
   _saveChanges = ()=> {
-    this.props.onAccountUpdated(this.props.account, this.state);
+    this.props.onAccountUpdated(this.props.account, this.state.account);
+  };
+
+  _setState = (updates, callback = ()=>{})=> {
+    const updated = _.extend({}, this.state.account, updates);
+    this.setState({account: updated}, callback);
+  };
+
+  _setStateAndSave = (updates)=> {
+    this._setState(updates, ()=> {
+      this._saveChanges();
+    });
   };
 
 
   // Handlers
 
   _onAccountLabelUpdated = (event)=> {
-    this.setState({label: event.target.value});
+    this._setState({label: event.target.value});
   };
 
   _onAccountAliasCreated = (newAlias)=> {
     const coercedAlias = this._makeAlias(newAlias);
     const aliases = this.state.aliases.concat([coercedAlias]);
-    this.setState({aliases}, ()=> {
-      this._saveChanges();
-    });
+    this._setStateAndSave({aliases})
   };
 
   _onAccountAliasUpdated = (newAlias, alias, idx)=> {
@@ -85,9 +87,7 @@ class PreferencesAccountDetails extends Component {
       defaultAlias = coercedAlias;
     }
     aliases[idx] = coercedAlias;
-    this.setState({aliases, defaultAlias}, ()=> {
-      this._saveChanges();
-    });
+    this._setStateAndSave({aliases, defaultAlias});
   };
 
   _onAccountAliasRemoved = (alias, idx)=> {
@@ -97,16 +97,12 @@ class PreferencesAccountDetails extends Component {
       defaultAlias = null;
     }
     aliases.splice(idx, 1);
-    this.setState({aliases, defaultAlias}, ()=> {
-      this._saveChanges();
-    });
+    this._setStateAndSave({aliases, defaultAlias});
   };
 
   _onDefaultAliasSelected = (event)=> {
     const defaultAlias = event.target.value === 'None' ? null : event.target.value;
-    this.setState({defaultAlias}, ()=> {
-      this._saveChanges();
-    });
+    this._setStateAndSave({defaultAlias});
   };
 
 
@@ -129,7 +125,7 @@ class PreferencesAccountDetails extends Component {
   }
 
   render() {
-    const account = this.state;
+    const {account} = this.state;
     const aliasPlaceholder = this._makeAlias(
       `alias@${account.emailAddress.split('@')[1]}`
     );