Merge "Ensure Change and Reply plugin APIs have element set"

polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js

@@ -14,21 +14,49 @@
 (function(window) {
   'use strict';
 
+  /**
+   * Ensure GrChangeActionsInterface instance has access to gr-change-actions
+   * element and retrieve if the interface was created before element.
+   * @param {!GrChangeActionsInterface} api
+   */
+  function ensureEl(api) {
+    if (!api._el) {
+      const sharedApiElement = document.createElement('gr-js-api-interface');
+      setEl(api, sharedApiElement.getElement(
+          sharedApiElement.Element.CHANGE_ACTIONS));
+    }
+  }
+
+  /**
+   * Set gr-change-actions element to a GrChangeActionsInterface instance.
+   * @param {!GrChangeActionsInterface} api
+   * @param {!Element} el gr-change-actions
+   */
+  function setEl(api, el) {
+    if (!el) {
+      console.warn('changeActions() is not ready');
+      return;
+    }
+    api._el = el;
+    api.RevisionActions = el.RevisionActions;
+    api.ChangeActions = el.ChangeActions;
+    api.ActionType = el.ActionType;
+  }
+
   function GrChangeActionsInterface(plugin, el) {
     this.plugin = plugin;
-    this._el = el;
+    setEl(this, el);
-    this.RevisionActions = el.RevisionActions;
-    this.ChangeActions = el.ChangeActions;
-    this.ActionType = el.ActionType;
   }
 
   GrChangeActionsInterface.prototype.addPrimaryActionKey = function(key) {
+    ensureEl(this);
     if (this._el.primaryActionKeys.includes(key)) { return; }
 
     this._el.push('primaryActionKeys', key);
   };
 
   GrChangeActionsInterface.prototype.removePrimaryActionKey = function(key) {
+    ensureEl(this);
     this._el.primaryActionKeys = this._el.primaryActionKeys.filter(k => {
       return k !== key;
     });
@@ -36,45 +64,55 @@
 
   GrChangeActionsInterface.prototype.setActionOverflow = function(type, key,
       overflow) {
+    ensureEl(this);
     return this._el.setActionOverflow(type, key, overflow);
   };
 
   GrChangeActionsInterface.prototype.setActionPriority = function(type, key,
       priority) {
+    ensureEl(this);
     return this._el.setActionPriority(type, key, priority);
   };
 
   GrChangeActionsInterface.prototype.setActionHidden = function(type, key,
       hidden) {
+    ensureEl(this);
     return this._el.setActionHidden(type, key, hidden);
   };
 
   GrChangeActionsInterface.prototype.add = function(type, label) {
+    ensureEl(this);
     return this._el.addActionButton(type, label);
   };
 
   GrChangeActionsInterface.prototype.remove = function(key) {
+    ensureEl(this);
     return this._el.removeActionButton(key);
   };
 
   GrChangeActionsInterface.prototype.addTapListener = function(key, handler) {
+    ensureEl(this);
     this._el.addEventListener(key + '-tap', handler);
   };
 
   GrChangeActionsInterface.prototype.removeTapListener = function(key,
       handler) {
+    ensureEl(this);
     this._el.removeEventListener(key + '-tap', handler);
   };
 
   GrChangeActionsInterface.prototype.setLabel = function(key, text) {
+    ensureEl(this);
     this._el.setActionButtonProp(key, 'label', text);
   };
 
   GrChangeActionsInterface.prototype.setEnabled = function(key, enabled) {
+    ensureEl(this);
     this._el.setActionButtonProp(key, 'enabled', enabled);
   };
 
   GrChangeActionsInterface.prototype.getActionDetails = function(action) {
+    ensureEl(this);
     return this._el.getActionDetails(action) ||
       this._el.getActionDetails(this.plugin.getPluginName() + '~' + action);
   };

polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html

@@ -39,6 +39,7 @@ breaking changes to gr-change-actions won’t be noticed.
   suite('gr-js-api-interface tests', () => {
     let element;
     let changeActions;
+    let plugin;
 
     // Because deepEqual doesn’t behave in Safari.
     function assertArraysEqual(actual, expected) {
@@ -48,132 +49,152 @@ breaking changes to gr-change-actions won’t be noticed.
       }
     }
 
-    setup(() => {
+    suite('early init', () => {
-      element = fixture('basic');
+      setup(() => {
-      element.change = {};
+        Gerrit.install(p => { plugin = p; }, '0.1',
-      element._hasKnownChainState = false;
+            'http://test.com/plugins/testplugin/static/test.js');
-      let plugin;
+        changeActions = plugin.changeActions();
-      Gerrit.install(p => { plugin = p; }, '0.1',
+        element = fixture('basic');
-          'http://test.com/plugins/testplugin/static/test.js');
+      });
-      changeActions = plugin.changeActions();
-    });
 
-    teardown(() => {
+      teardown(() => {
-      changeActions = null;
+        changeActions = null;
-    });
+      });
 
-    test('property existence', () => {
+      test('does not throw', ()=> {
-      const properties = [
+        assert.doesNotThrow(() => {
-        'ActionType',
+          changeActions.add('change', 'foo');
-        'ChangeActions',
-        'RevisionActions',
-      ];
-      for (const p of properties) {
-        assertArraysEqual(changeActions[p], element[p]);
-      }
-    });
-
-    test('add/remove primary action keys', () => {
-      element.primaryActionKeys = [];
-      changeActions.addPrimaryActionKey('foo');
-      assertArraysEqual(element.primaryActionKeys, ['foo']);
-      changeActions.addPrimaryActionKey('foo');
-      assertArraysEqual(element.primaryActionKeys, ['foo']);
-      changeActions.addPrimaryActionKey('bar');
-      assertArraysEqual(element.primaryActionKeys, ['foo', 'bar']);
-      changeActions.removePrimaryActionKey('foo');
-      assertArraysEqual(element.primaryActionKeys, ['bar']);
-      changeActions.removePrimaryActionKey('baz');
-      assertArraysEqual(element.primaryActionKeys, ['bar']);
-      changeActions.removePrimaryActionKey('bar');
-      assertArraysEqual(element.primaryActionKeys, []);
-    });
-
-    test('action buttons', done => {
-      const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
-      const handler = sinon.spy();
-      changeActions.addTapListener(key, handler);
-      flush(() => {
-        MockInteractions.tap(element.$$('[data-action-key="' + key + '"]'));
-        assert(handler.calledOnce);
-        changeActions.removeTapListener(key, handler);
-        MockInteractions.tap(element.$$('[data-action-key="' + key + '"]'));
-        assert(handler.calledOnce);
-        changeActions.remove(key);
-        flush(() => {
-          assert.isNull(element.$$('[data-action-key="' + key + '"]'));
-          done();
         });
       });
     });
 
-    test('action button properties', done => {
+    suite('normal init', () => {
-      const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+      setup(() => {
-      flush(() => {
+        element = fixture('basic');
-        const button = element.$$('[data-action-key="' + key + '"]');
+        element.change = {};
-        assert.isOk(button);
+        element._hasKnownChainState = false;
-        assert.equal(button.getAttribute('data-label'), 'Bork!');
+        Gerrit.install(p => { plugin = p; }, '0.1',
-        assert.isNotOk(button.disabled);
+            'http://test.com/plugins/testplugin/static/test.js');
-        changeActions.setLabel(key, 'Yo');
+        changeActions = plugin.changeActions();
-        changeActions.setEnabled(key, false);
+      });
+
+      teardown(() => {
+        changeActions = null;
+      });
+
+      test('property existence', () => {
+        const properties = [
+          'ActionType',
+          'ChangeActions',
+          'RevisionActions',
+        ];
+        for (const p of properties) {
+          assertArraysEqual(changeActions[p], element[p]);
+        }
+      });
+
+      test('add/remove primary action keys', () => {
+        element.primaryActionKeys = [];
+        changeActions.addPrimaryActionKey('foo');
+        assertArraysEqual(element.primaryActionKeys, ['foo']);
+        changeActions.addPrimaryActionKey('foo');
+        assertArraysEqual(element.primaryActionKeys, ['foo']);
+        changeActions.addPrimaryActionKey('bar');
+        assertArraysEqual(element.primaryActionKeys, ['foo', 'bar']);
+        changeActions.removePrimaryActionKey('foo');
+        assertArraysEqual(element.primaryActionKeys, ['bar']);
+        changeActions.removePrimaryActionKey('baz');
+        assertArraysEqual(element.primaryActionKeys, ['bar']);
+        changeActions.removePrimaryActionKey('bar');
+        assertArraysEqual(element.primaryActionKeys, []);
+      });
+
+      test('action buttons', done => {
+        const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+        const handler = sinon.spy();
+        changeActions.addTapListener(key, handler);
         flush(() => {
-          assert.equal(button.getAttribute('data-label'), 'Yo');
+          MockInteractions.tap(element.$$('[data-action-key="' + key + '"]'));
-          assert.isTrue(button.disabled);
+          assert(handler.calledOnce);
-          done();
+          changeActions.removeTapListener(key, handler);
+          MockInteractions.tap(element.$$('[data-action-key="' + key + '"]'));
+          assert(handler.calledOnce);
+          changeActions.remove(key);
+          flush(() => {
+            assert.isNull(element.$$('[data-action-key="' + key + '"]'));
+            done();
+          });
         });
       });
-    });
 
-    test('hide action buttons', done => {
+      test('action button properties', done => {
-      const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+        const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
-      flush(() => {
-        const button = element.$$('[data-action-key="' + key + '"]');
-        assert.isOk(button);
-        assert.isFalse(button.hasAttribute('hidden'));
-        changeActions.setActionHidden(
-            changeActions.ActionType.REVISION, key, true);
         flush(() => {
           const button = element.$$('[data-action-key="' + key + '"]');
-          assert.isNotOk(button);
+          assert.isOk(button);
-          done();
+          assert.equal(button.getAttribute('data-label'), 'Bork!');
+          assert.isNotOk(button.disabled);
+          changeActions.setLabel(key, 'Yo');
+          changeActions.setEnabled(key, false);
+          flush(() => {
+            assert.equal(button.getAttribute('data-label'), 'Yo');
+            assert.isTrue(button.disabled);
+            done();
+          });
         });
       });
-    });
 
-    test('move action button to overflow', done => {
+      test('hide action buttons', done => {
-      const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+        const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
-      flush(() => {
-        assert.isTrue(element.$.moreActions.hidden);
-        assert.isOk(element.$$('[data-action-key="' + key + '"]'));
-        changeActions.setActionOverflow(
-            changeActions.ActionType.REVISION, key, true);
         flush(() => {
-          assert.isNotOk(element.$$('[data-action-key="' + key + '"]'));
+          const button = element.$$('[data-action-key="' + key + '"]');
-          assert.isFalse(element.$.moreActions.hidden);
+          assert.isOk(button);
-          assert.strictEqual(element.$.moreActions.items[0].name, 'Bork!');
+          assert.isFalse(button.hasAttribute('hidden'));
-          done();
+          changeActions.setActionHidden(
+              changeActions.ActionType.REVISION, key, true);
+          flush(() => {
+            const button = element.$$('[data-action-key="' + key + '"]');
+            assert.isNotOk(button);
+            done();
+          });
         });
       });
-    });
 
-    test('change actions priority', done => {
+      test('move action button to overflow', done => {
-      const key1 =
+        const key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+        flush(() => {
+          assert.isTrue(element.$.moreActions.hidden);
+          assert.isOk(element.$$('[data-action-key="' + key + '"]'));
+          changeActions.setActionOverflow(
+              changeActions.ActionType.REVISION, key, true);
+          flush(() => {
+            assert.isNotOk(element.$$('[data-action-key="' + key + '"]'));
+            assert.isFalse(element.$.moreActions.hidden);
+            assert.strictEqual(element.$.moreActions.items[0].name, 'Bork!');
+            done();
+          });
+        });
+      });
+
+      test('change actions priority', done => {
+        const key1 =
           changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
-      const key2 =
+        const key2 =
           changeActions.add(changeActions.ActionType.CHANGE, 'Squanch?');
-      flush(() => {
-        let buttons =
-            Polymer.dom(element.root).querySelectorAll('[data-action-key]');
-        assert.equal(buttons[0].getAttribute('data-action-key'), key1);
-        assert.equal(buttons[1].getAttribute('data-action-key'), key2);
-        changeActions.setActionPriority(
-            changeActions.ActionType.REVISION, key1, 10);
         flush(() => {
-          buttons =
+          let buttons =
+            Polymer.dom(element.root).querySelectorAll('[data-action-key]');
+          assert.equal(buttons[0].getAttribute('data-action-key'), key1);
+          assert.equal(buttons[1].getAttribute('data-action-key'), key2);
+          changeActions.setActionPriority(
+              changeActions.ActionType.REVISION, key1, 10);
+          flush(() => {
+            buttons =
               Polymer.dom(element.root).querySelectorAll('[data-action-key]');
-          assert.equal(buttons[0].getAttribute('data-action-key'), key2);
+            assert.equal(buttons[0].getAttribute('data-action-key'), key2);
-          assert.equal(buttons[1].getAttribute('data-action-key'), key1);
+            assert.equal(buttons[1].getAttribute('data-action-key'), key1);
-          done();
+            done();
+          });
         });
       });
     });

polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api.js

@@ -14,6 +14,19 @@
 (function(window) {
   'use strict';
 
+  /**
+   * Ensure GrChangeReplyInterface instance has access to gr-reply-dialog
+   * element and retrieve if the interface was created before element.
+   * @param {!GrChangeReplyInterfaceOld} api
+   */
+  function ensureEl(api) {
+    if (!api._el) {
+      const sharedApiElement = document.createElement('gr-js-api-interface');
+      api._el = sharedApiElement.getElement(
+          sharedApiElement.Element.REPLY_DIALOG);
+    }
+  }
+
   /**
    * @deprecated
    */
@@ -22,14 +35,17 @@
   }
 
   GrChangeReplyInterfaceOld.prototype.getLabelValue = function(label) {
+    ensureEl(this);
     return this._el.getLabelValue(label);
   };
 
   GrChangeReplyInterfaceOld.prototype.setLabelValue = function(label, value) {
+    ensureEl(this);
     this._el.setLabelValue(label, value);
   };
 
   GrChangeReplyInterfaceOld.prototype.send = function(opt_includeComments) {
+    ensureEl(this);
     return this._el.send(opt_includeComments);
   };
 

polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api_test.html

@@ -40,37 +40,72 @@ breaking changes to gr-reply-dialog won’t be noticed.
     let element;
     let sandbox;
     let changeReply;
+    let plugin;
 
     setup(() => {
+      sandbox = sinon.sandbox.create();
       stub('gr-rest-api-interface', {
         getConfig() { return Promise.resolve({}); },
         getAccount() { return Promise.resolve(null); },
       });
-      element = fixture('basic');
-      sandbox = sinon.sandbox.create();
-      let plugin;
-      Gerrit.install(p => { plugin = p; }, '0.1',
-          'http://test.com/plugins/testplugin/static/test.js');
-      changeReply = plugin.changeReply();
     });
 
     teardown(() => {
-      changeReply = null;
       sandbox.restore();
     });
 
-    test('calls', () => {
+    suite('early init', () => {
-      sandbox.stub(element, 'getLabelValue').returns('+123');
+      setup(() => {
-      assert.equal(changeReply.getLabelValue('My-Label'), '+123');
+        Gerrit.install(p => { plugin = p; }, '0.1',
+            'http://test.com/plugins/testplugin/static/test.js');
+        changeReply = plugin.changeReply();
+        element = fixture('basic');
+      });
 
-      sandbox.stub(element, 'setLabelValue');
+      teardown(() => {
-      changeReply.setLabelValue('My-Label', '+1337');
+        changeReply = null;
-      assert.isTrue(
+      });
-          element.setLabelValue.calledWithExactly('My-Label', '+1337'));
 
-      sandbox.stub(element, 'send');
+      test('works', () => {
-      changeReply.send(false);
+        sandbox.stub(element, 'getLabelValue').returns('+123');
-      assert.isTrue(element.send.calledWithExactly(false));
+        assert.equal(changeReply.getLabelValue('My-Label'), '+123');
+
+        sandbox.stub(element, 'setLabelValue');
+        changeReply.setLabelValue('My-Label', '+1337');
+        assert.isTrue(
+            element.setLabelValue.calledWithExactly('My-Label', '+1337'));
+
+        sandbox.stub(element, 'send');
+        changeReply.send(false);
+        assert.isTrue(element.send.calledWithExactly(false));
+      });
+    });
+
+    suite('normal init', () => {
+      setup(() => {
+        element = fixture('basic');
+        Gerrit.install(p => { plugin = p; }, '0.1',
+            'http://test.com/plugins/testplugin/static/test.js');
+        changeReply = plugin.changeReply();
+      });
+
+      teardown(() => {
+        changeReply = null;
+      });
+
+      test('works', () => {
+        sandbox.stub(element, 'getLabelValue').returns('+123');
+        assert.equal(changeReply.getLabelValue('My-Label'), '+123');
+
+        sandbox.stub(element, 'setLabelValue');
+        changeReply.setLabelValue('My-Label', '+1337');
+        assert.isTrue(
+            element.setLabelValue.calledWithExactly('My-Label', '+1337'));
+
+        sandbox.stub(element, 'send');
+        changeReply.send(false);
+        assert.isTrue(element.send.calledWithExactly(false));
+      });
     });
   });
 </script>