fix: address comments

adobe · Feb 29, 2024 · 2b42e57 · 2b42e57
1 parent 8865286
commit 2b42e57
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 12 deletions.
diff --git a/src/extension/_locales/en/messages.json b/src/extension/_locales/en/messages.json
@@ -610,7 +610,7 @@
     "not_authorized": {
       "message": "Not authorized"
     },
-    "please_login": {
-      "message": "Please login..."
+    "login_wait": {
+      "message": "Please sign in..."
     }
 }
diff --git a/src/extension/app/components/plugin/plugin-action-bar.js b/src/extension/app/components/plugin/plugin-action-bar.js
@@ -91,7 +91,8 @@ export class PluginActionBar extends MobxLitElement {
     `;
   }
 
-  onPluginButtonClick(evt, plugin) {
+  async onPluginButtonClick(evt, plugin) {
+    await appStore.validateSession();
     appStore.fireEvent(EXTERNAL_EVENTS.PLUGIN_USED, {
       id: plugin.id,
     });
@@ -115,7 +116,7 @@ export class PluginActionBar extends MobxLitElement {
 
   /**
    * Render the core and custom plugins
-   * @returns {(TemplateResult|string)[]|string} An array of Lit-html templates or strings, or a single empty string.
+   * @returns {(TemplateResult|string)|string} An array of Lit-html templates or strings, or a single empty string.
    */
   renderPlugins() {
     const corePlugins = Object.values(appStore.corePlugins)?.map((plugin) => (plugin.condition(appStore) ? this.createCorePlugin(plugin) : ''));
@@ -141,16 +142,25 @@ export class PluginActionBar extends MobxLitElement {
     const userPlugins = Object.values(customPlugins).map((plugin) => {
       if (plugin.children) {
         return html`
-          <action-bar-picker class=${`plugin-container ${plugin.id}`} label=${plugin.button.text} @change=${(e) => this.onChange(e, plugin)} placement="top">
-            ${Object.values(plugin.children).map((childPlugin) => (childPlugin.condition(appStore)
+          <action-bar-picker 
+            class=${`plugin-container ${plugin.id}`} 
+            label=${plugin.button.text} 
+            @change=${(e) => this.onChange(e, plugin)} 
+            placement="top"
+          >
+              ${Object.values(plugin.children).map((childPlugin) => (childPlugin.condition(appStore)
                 ? html`<sp-menu-item value=${childPlugin.id}>${childPlugin.button.text}</sp-menu-item>`
                 : ''))}
           </action-bar-picker>
         `;
       }
 
       return plugin.condition(appStore) ? html`
-              <sp-action-button class=${plugin.id} quiet @click=${(evt) => this.onPluginButtonClick(evt, plugin)}>
+              <sp-action-button 
+                class=${plugin.id} 
+                quiet 
+                @click=${(evt) => this.onPluginButtonClick(evt, plugin)}
+                >
                 ${plugin.button.text || plugin.id}
               </sp-action-button>
             ` : '';

diff --git a/src/extension/app/plugins/preview/preview.js b/src/extension/app/plugins/preview/preview.js
@@ -45,7 +45,6 @@ export function createPreviewPlugin(appStore) {
       text: appStore.i18n('preview'),
       action: async () => {
         const { status, location } = appStore;
-        await appStore.validateSession();
         if (status.edit && status.edit.sourceLocation
             && status.edit.sourceLocation.startsWith('onedrive:')
             && !location.pathname.startsWith('/:x:/')) {

diff --git a/src/extension/app/plugins/publish/publish.js b/src/extension/app/plugins/publish/publish.js
@@ -37,7 +37,6 @@ export function createPublishPlugin(appStore) {
         const { location } = appStore;
         const path = location.pathname;
         appStore.showWait();
-        await appStore.validateSession();
         const res = await appStore.publish(path);
         if (res.ok) {
           appStore.hideWait();

diff --git a/src/extension/app/plugins/reload/reload.js b/src/extension/app/plugins/reload/reload.js
@@ -36,7 +36,6 @@ export function createReloadPlugin(appStore) {
       text: i18n(appStore.languageDict, 'reload'),
       action: async (evt) => {
         appStore.showWait();
-        await appStore.validateSession();
         try {
           const resp = await appStore.update();
           if (!resp.ok && resp.status >= 400) {

diff --git a/src/extension/app/store/app.js b/src/extension/app/store/app.js
@@ -507,6 +507,7 @@ export class AppStore {
    * @param {string} url The URL to open
    * @returns {Window} The window object
    */
+  // istanbul ignore next 3
   openPage(url) {
     return window.open(url);
   }
@@ -929,7 +930,7 @@ export class AppStore {
    * @param {boolean} selectAccount <code>true</code> to allow user to select account (optional)
    */
   login(selectAccount) {
-    this.showWait(this.i18n('please_login'));
+    this.showWait(this.i18n('login_wait'));
     const loginUrl = getAdminUrl(this.siteStore, 'login');
     let extensionId = window.chrome?.runtime?.id;
     // istanbul ignore next 3

diff --git a/test/app/components/plugin/plugin-action-bar.test.js b/test/app/components/plugin/plugin-action-bar.test.js
@@ -41,6 +41,7 @@ import {
 } from '../../../mocks/environment.js';
 import { EXTERNAL_EVENTS } from '../../../../src/extension/app/constants.js';
 import { pluginFactory } from '../../../../src/extension/app/plugins/plugin-factory.js';
+import { appStore } from '../../../../src/extension/app/store/app.js';
 
 // @ts-ignore
 window.chrome = chromeMock;
@@ -244,7 +245,7 @@ describe('Plugin action bar', () => {
     it('core plugin clicked', async () => {
       window.hlx = {};
 
-      const actionFunction = async () => {};
+      const actionFunction = async () => Promise.resolve();
 
       // Create a spy for the action function
       const actionSpy = sinon.spy(actionFunction);
@@ -274,6 +275,7 @@ describe('Plugin action bar', () => {
       const publishButton = recursiveQuery(sidekick, '.publish');
       publishButton.click();
 
+      await waitUntil(() => actionSpy.calledOnce);
       expect(actionSpy.calledOnce).to.be.true;
 
       stub.restore();
@@ -318,6 +320,7 @@ describe('Plugin action bar', () => {
       mockFetchConfigWithPluginsJSONSuccess();
       mockEditorAdminEnvironment(document, 'editor');
 
+      const validateStub = sinon.stub(appStore, 'validateSession').resolves();
       const openStub = sinon.stub(window, 'open');
       const pluginUsedEventSpy = sinon.spy();
 
@@ -333,10 +336,12 @@ describe('Plugin action bar', () => {
       const libraryPlugin = recursiveQuery(sidekick, '.library');
       libraryPlugin.click();
 
+      await waitUntil(() => openStub.calledOnce);
       expect(openStub.calledOnce).to.be.true;
       expect(pluginUsedEventSpy.calledOnce).to.be.true;
 
       openStub.restore();
+      validateStub.restore();
     });
   });