From 525c98bf3d75b73c216f5b50113ea2eb6be687b7 Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Wed, 27 Dec 2017 14:12:15 -0800
Subject: [PATCH] Reset state flags on loadData calls

In some cases, bound values were setting the flags tracking changes in
gr-account-info. Resetting the flags when loadData is called ensures no
false positives occur.

In addition, upgrades the _usernameChanged logic to compare to the
existing account username value, as opposed to just setting the flag to
true on any modification.

Bug: Issue 7893
Bug: Issue 8287
Change-Id: I75a3cda93bb065a7b9640d4b40905a5e21042bb5
(cherry picked from commit 5cb34e7fa3896a8290023762729690cf15b17ef7)
---
 .../gr-account-info/gr-account-info.js        | 23 ++++++++-----------
 .../gr-account-info/gr-account-info_test.html | 18 +++++++++++++++
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
index a698c71499..3cec65a594 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
@@ -41,18 +41,9 @@
             '_hasUsernameChange, _hasStatusChange)',
       },
 
-      _hasNameChange: {
-        type: Boolean,
-        value: false,
-      },
-      _hasUsernameChange: {
-        type: Boolean,
-        value: false,
-      },
-      _hasStatusChange: {
-        type: Boolean,
-        value: false,
-      },
+      _hasNameChange: Boolean,
+      _hasUsernameChange: Boolean,
+      _hasStatusChange: Boolean,
       _loading: {
         type: Boolean,
         value: false,
@@ -85,6 +76,9 @@
       }));
 
       promises.push(this.$.restAPI.getAccount().then(account => {
+        this._hasNameChange = false;
+        this._hasUsernameChange = false;
+        this._hasStatusChange = false;
         // Provide predefined value for username to trigger computation of
         // username mutability.
         account.username = account.username || '';
@@ -154,8 +148,9 @@
     },
 
     _usernameChanged() {
-      if (this._loading) { return; }
-      this._hasUsernameChange = true;
+      if (this._loading || !this._account) { return; }
+      this._hasUsernameChange =
+          (this._account.username || '') !== (this._username || '');
     },
 
     _nameChanged() {
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html
index d27d15393a..82997a5ec3 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html
@@ -311,5 +311,23 @@ limitations under the License.
         });
       });
     });
+
+    test('_usernameChanged compares usernames with loose equality', () => {
+      element._account = {};
+      element._username = '';
+      element._hasUsernameChange = false;
+      element._loading = false;
+      // _usernameChanged is an observer, but call it here after setting
+      // _hasUsernameChange in the test to force recomputation.
+      element._usernameChanged();
+      flushAsynchronousOperations();
+
+      assert.isFalse(element._hasUsernameChange);
+
+      element.set('_username', 'test');
+      flushAsynchronousOperations();
+
+      assert.isTrue(element._hasUsernameChange);
+    });
   });
 </script>