Merge "Fix for modified flag in repo access config"

polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html

@@ -329,17 +329,8 @@ limitations under the License.
           name: 'ldap/tests tests'}});
         assert.equal(element._rules.length, 3);
         assert.equal(Object.keys(element._groupsWithRules).length, 3);
-        if (Polymer.Element) {
-          // Under Polymer 2 gr-rule-editor.js#_handleValueChange get's
-          // fully loaded before this change, thus `modified: true` get's managed
-          // to be added. Under Polymer 1 it was a mix hence why it was not
-          // added in time for when this test ran.
-          assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
-              {action: 'ALLOW', min: -2, max: 2, modified: true, added: true});
-        } else {
         assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
             {action: 'ALLOW', min: -2, max: 2, added: true});
-        }
         // New rule should be removed if cancel from editing.
         element.editing = false;
         assert.equal(element._rules.length, 2);

polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js

@@ -118,11 +118,20 @@
       this._setupValues(this.rule);
     },
 
+    attached() {
+      if (!this.rule) { return; } // Check needed for test purposes.
+      if (!this._originalRuleValues) {
+        // Observer _handleValueChange is called after the ready()
+        // method finishes. Original values must be set later to
+        // avoid set .modified flag to true
+        this._setOriginalRuleValues(this.rule.value);
+      }
+    },
+
     _setupValues(rule) {
       if (!rule.value) {
         this._setDefaultRuleValues();
       }
-      this._setOriginalRuleValues(rule.value);
     },
 
     _computeForce(permission, action) {

polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html

@@ -202,7 +202,7 @@ limitations under the License.
     });
 
     suite('already existing generic rule', () => {
-      setup(() => {
+      setup(done => {
         element.group = 'Group Name';
         element.permission = 'submit';
         element.rule = {
@@ -218,6 +218,10 @@ limitations under the License.
         // by the parent element.
         element._setupValues(element.rule);
         flushAsynchronousOperations();
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -313,7 +317,7 @@ limitations under the License.
     });
 
     suite('new edit rule', () => {
-      setup(() => {
+      setup(done => {
         element.group = 'Group Name';
         element.permission = 'editTopicName';
         element.rule = {
@@ -323,6 +327,10 @@ limitations under the License.
         element._setupValues(element.rule);
         flushAsynchronousOperations();
         element.rule.value.added = true;
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -362,7 +370,7 @@ limitations under the License.
     });
 
     suite('already existing rule with labels', () => {
-      setup(() => {
+      setup(done => {
         element.label = {values: [
           {value: -2, text: 'This shall not be merged'},
           {value: -1, text: 'I would prefer this is not merged as is'},
@@ -384,6 +392,10 @@ limitations under the License.
         element.section = 'refs/*';
         element._setupValues(element.rule);
         flushAsynchronousOperations();
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -416,7 +428,7 @@ limitations under the License.
     });
 
     suite('new rule with labels', () => {
-      setup(() => {
+      setup(done => {
         sandbox.spy(element, '_setDefaultRuleValues');
         element.label = {values: [
           {value: -2, text: 'This shall not be merged'},
@@ -434,6 +446,10 @@ limitations under the License.
         element._setupValues(element.rule);
         flushAsynchronousOperations();
         element.rule.value.added = true;
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -474,7 +490,7 @@ limitations under the License.
     });
 
     suite('already existing push rule', () => {
-      setup(() => {
+      setup(done => {
         element.group = 'Group Name';
         element.permission = 'push';
         element.rule = {
@@ -487,6 +503,10 @@ limitations under the License.
         element.section = 'refs/*';
         element._setupValues(element.rule);
         flushAsynchronousOperations();
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -515,7 +535,7 @@ limitations under the License.
     });
 
     suite('new push rule', () => {
-      setup(() => {
+      setup(done => {
         element.group = 'Group Name';
         element.permission = 'push';
         element.rule = {
@@ -525,6 +545,10 @@ limitations under the License.
         element._setupValues(element.rule);
         flushAsynchronousOperations();
         element.rule.value.added = true;
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {
@@ -555,7 +579,7 @@ limitations under the License.
     });
 
     suite('already existing edit rule', () => {
-      setup(() => {
+      setup(done => {
         element.group = 'Group Name';
         element.permission = 'editTopicName';
         element.rule = {
@@ -568,6 +592,10 @@ limitations under the License.
         element.section = 'refs/*';
         element._setupValues(element.rule);
         flushAsynchronousOperations();
+        flush(() => {
+          element.attached();
+          done();
+        });
       });
 
       test('_ruleValues and _originalRuleValues are set correctly', () => {

polygerrit-ui/app/elements/shared/gr-select/gr-select.js

@@ -66,7 +66,7 @@
 
     ready() {
       // If not set via the property, set bind-value to the element value.
-      if (this.bindValue == undefined) {
+      if (this.bindValue == undefined && this.nativeSelect.options.length > 0) {
         this.bindValue = this.nativeSelect.value;
       }
     },

polygerrit-ui/app/elements/shared/gr-select/gr-select_test.html

@@ -40,6 +40,15 @@ limitations under the License.
   </template>
 </test-fixture>
 
+<test-fixture id="noOptions">
+  <template>
+    <gr-select>
+      <select>
+      </select>
+    </gr-select>
+  </template>
+</test-fixture>
+
 <script>
   suite('gr-select tests', () => {
     let element;
@@ -48,6 +57,10 @@ limitations under the License.
       element = fixture('basic');
     });
 
+    test('bindValue must be set to the first option value', () => {
+      assert.equal(element.bindValue, '1');
+    });
+
     test('value of 0 should still trigger value updates', () => {
       element.bindValue = 0;
       assert.equal(element.nativeSelect.value, 0);
@@ -90,4 +103,16 @@ limitations under the License.
       assert.isTrue(changeStub.called);
     });
   });
+
+  suite('gr-select no options tests', () => {
+    let element;
+
+    setup(() => {
+      element = fixture('noOptions');
+    });
+
+    test('bindValue must not be changed', () => {
+      assert.isUndefined(element.bindValue);
+    });
+  });
 </script>