Fix race in editing account info

Because modifying the name and status field in account-info requires two
separate API calls, a race was occurring server side that caused the DB
to reject the modification of the status field most of the time.

Before, the requests to set status and name were made every time. Now,
the requests are made synchronously, and are only made if the
corresponding property of the account object was actually modified.

Bug: Issue 5721
Change-Id: I38d2927cbd0998a0debc414ed09ca1f74e8354ff

polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js

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 25762d4..0e64cb2 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
@@ -32,9 +32,17 @@
       hasUnsavedChanges: {
         type: Boolean,
         notify: true,
-        value: false,
+        computed: '_computeHasUnsavedChanges(_hasNameChange, _hasStatusChange)',
       },
 
+      _hasNameChange: {
+        type: Boolean,
+        value: false,
+      },
+      _hasStatusChange: {
+        type: Boolean,
+        value: false,
+      },
       _loading: {
         type: Boolean,
         value: false,
@@ -48,7 +56,8 @@
     },
 
     observers: [
-      '_accountChanged(_account.*)',
+      '_nameChanged(_account.name)',
+      '_statusChanged(_account.status)',
     ],
 
     loadData: function() {
@@ -75,23 +84,46 @@
       }
 
       this._saving = true;
-      return Promise.all([
-        this.$.restAPI.setAccountName(this._account.name),
-        this.$.restAPI.setAccountStatus(this._account.status),
-      ]).then(function() {
-        this.hasUnsavedChanges = false;
-        this._saving = false;
-        this.fire('account-detail-update');
-      }.bind(this));
+      // Set only the fields that have changed.
+      // Must be done in sequence to avoid race conditions (@see Issue 5721)
+      return this._maybeSetName()
+          .then(this._maybeSetStatus.bind(this))
+          .then(function() {
+            this._hasNameChange = false;
+            this._hasStatusChange = false;
+            this._saving = false;
+            this.fire('account-detail-update');
+          }.bind(this));
+    },
+
+    _maybeSetName: function() {
+      return this._hasNameChange ?
+          this.$.restAPI.setAccountName(this._account.name) :
+          Promise.resolve();
+    },
+
+    _maybeSetStatus: function() {
+      return this._hasStatusChange ?
+          this.$.restAPI.setAccountStatus(this._account.status) :
+          Promise.resolve();
+    },
+
+    _computeHasUnsavedChanges: function(name, status) {
+      return name || status;
     },
 
     _computeMutable: function(config) {
       return config.auth.editable_account_fields.indexOf('FULL_NAME') !== -1;
     },
 
-    _accountChanged: function() {
+    _statusChanged: function() {
       if (this._loading) { return; }
-      this.hasUnsavedChanges = true;
+      this._hasStatusChange = true;
+    },
+
+    _nameChanged: function() {
+      if (this._loading) { return; }
+      this._hasNameChange = true;
     },
 
     _handleKeydown: function(e) {

polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html

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 5dbbf1f..ffff0f1 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
@@ -109,12 +109,14 @@
     });
 
     suite('account info edit', function() {
-      var accountChangedSpy;
+      var nameChangedSpy;
+      var statusChangedSpy;
       var nameStub;
       var statusStub;
 
       setup(function() {
-        accountChangedSpy = sandbox.spy(element, '_accountChanged');
+        nameChangedSpy = sandbox.spy(element, '_nameChanged');
+        statusChangedSpy = sandbox.spy(element, '_statusChanged');
         element.set('_serverConfig',
           {auth: {editable_account_fields: ['FULL_NAME']}});
 
@@ -130,12 +132,14 @@
 
         element.set('_account.name', 'new name');
 
-        assert.isTrue(accountChangedSpy.called);
+        assert.isTrue(nameChangedSpy.called);
+        assert.isFalse(statusChangedSpy.called);
         assert.isTrue(element.hasUnsavedChanges);
 
         MockInteractions.pressAndReleaseKeyOn(element.$.nameInput, 13);
 
         assert.isTrue(nameStub.called);
+        assert.isFalse(statusStub.called);
         nameStub.lastCall.returnValue.then(function() {
           assert.equal(nameStub.lastCall.args[0], 'new name');
           done();
@@ -148,16 +152,17 @@
 
         element.set('_account.status', 'new status');
 
-        assert.isTrue(accountChangedSpy.called);
+        assert.isFalse(nameChangedSpy.called);
+        assert.isTrue(statusChangedSpy.called);
         assert.isTrue(element.hasUnsavedChanges);
 
-        MockInteractions.pressAndReleaseKeyOn(element.$.statusInput, 13);
-        flushAsynchronousOperations();
-
-        assert.isTrue(statusStub.called);
-        statusStub.lastCall.returnValue.then(function() {
-          assert.equal(statusStub.lastCall.args[0], 'new status');
-          done();
+        element.save().then(function() {
+          assert.isTrue(statusStub.called);
+          assert.isFalse(nameStub.called);
+          statusStub.lastCall.returnValue.then(function() {
+            assert.equal(statusStub.lastCall.args[0], 'new status');
+            done();
+          });
         });
       });
     });