fix: enforce one composer action at a time

flarum · Dec 9, 2023 · 91990d5 · 91990d5
1 parent c9d2763
commit 91990d5
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 94 deletions.
diff --git a/extensions/package-manager/js/src/admin/components/Installer.tsx b/extensions/package-manager/js/src/admin/components/Installer.tsx
@@ -3,11 +3,6 @@ import app from 'flarum/admin/app';
 import Component, { ComponentAttrs } from 'flarum/common/Component';
 import Button from 'flarum/common/components/Button';
 import Stream from 'flarum/common/utils/Stream';
-import LoadingModal from 'flarum/admin/components/LoadingModal';
-
-import errorHandler from '../utils/errorHandler';
-import jumpToQueue from '../utils/jumpToQueue';
-import { AsyncBackendResponse } from '../shims';
 
 export interface InstallerAttrs extends ComponentAttrs {}
 
@@ -38,7 +33,7 @@ export default class Installer extends Component<InstallerAttrs> {
             icon="fas fa-download"
             onclick={this.onsubmit.bind(this)}
             loading={app.packageManager.control.isLoading('extension-install')}
-            disabled={app.packageManager.control.isLoadingOtherThan('extension-install')}
+            disabled={app.packageManager.control.isLoading()}
           >
             {app.translator.trans('flarum-package-manager.admin.extensions.proceed')}
           </Button>
@@ -54,35 +49,6 @@ export default class Installer extends Component<InstallerAttrs> {
   }
 
   onsubmit(): void {
-    app.packageManager.control.setLoading('extension-install');
-    app.modal.show(LoadingModal);
-
-    app
-      .request<AsyncBackendResponse & { id: number }>({
-        method: 'POST',
-        url: `${app.forum.attribute('apiUrl')}/package-manager/extensions`,
-        body: {
-          data: this.data(),
-        },
-      })
-      .then((response) => {
-        if (response.processing) {
-          jumpToQueue();
-        } else {
-          const extensionId = response.id;
-          app.alerts.show(
-            { type: 'success' },
-            app.translator.trans('flarum-package-manager.admin.extensions.successful_install', { extension: extensionId })
-          );
-          window.location.href = `${app.forum.attribute('adminUrl')}#/extension/${extensionId}`;
-          window.location.reload();
-        }
-      })
-      .catch(errorHandler)
-      .finally(() => {
-        app.packageManager.control.setLoading(null);
-        app.modal.close();
-        m.redraw();
-      });
+    app.packageManager.control.requirePackage(this.data());
   }
 }
diff --git a/extensions/package-manager/js/src/admin/components/MajorUpdater.tsx b/extensions/package-manager/js/src/admin/components/MajorUpdater.tsx
@@ -3,17 +3,12 @@ import app from 'flarum/admin/app';
 import Component, { ComponentAttrs } from 'flarum/common/Component';
 import Button from 'flarum/common/components/Button';
 import Tooltip from 'flarum/common/components/Tooltip';
-import LoadingModal from 'flarum/admin/components/LoadingModal';
 import Alert from 'flarum/common/components/Alert';
-import RequestError from 'flarum/common/utils/RequestError';
 
 import { UpdatedPackage, UpdateState } from '../states/ControlSectionState';
-import errorHandler from '../utils/errorHandler';
 import WhyNotModal from './WhyNotModal';
 import ExtensionItem from './ExtensionItem';
-import { AsyncBackendResponse } from '../shims';
-import jumpToQueue from '../utils/jumpToQueue';
-import classList from "flarum/common/utils/classList";
+import classList from 'flarum/common/utils/classList';
 
 export interface MajorUpdaterAttrs extends ComponentAttrs {
   coreUpdate: UpdatedPackage;
@@ -45,20 +40,15 @@ export default class MajorUpdater<T extends MajorUpdaterAttrs = MajorUpdaterAttr
         <p className="helpText">{app.translator.trans('flarum-package-manager.admin.major_updater.description')}</p>
         <div className="PackageManager-updaterControls">
           <Tooltip text={app.translator.trans('flarum-package-manager.admin.major_updater.dry_run_help')}>
-            <Button
-              className="Button"
-              icon="fas fa-vial"
-              onclick={this.update.bind(this, true)}
-              disabled={app.packageManager.control.isLoadingOtherThan('major-update-dry-run')}
-            >
+            <Button className="Button" icon="fas fa-vial" onclick={this.update.bind(this, true)} disabled={app.packageManager.control.isLoading()}>
               {app.translator.trans('flarum-package-manager.admin.major_updater.dry_run')}
             </Button>
           </Tooltip>
           <Button
             className="Button Button--danger"
             icon="fas fa-play"
             onclick={this.update.bind(this, false)}
-            disabled={app.packageManager.control.isLoadingOtherThan('major-update')}
+            disabled={app.packageManager.control.isLoading()}
           >
             {app.translator.trans('flarum-package-manager.admin.major_updater.update')}
           </Button>
@@ -100,34 +90,6 @@ export default class MajorUpdater<T extends MajorUpdaterAttrs = MajorUpdaterAttr
   }
 
   update(dryRun: boolean) {
-    app.packageManager.control.setLoading(dryRun ? 'major-update-dry-run' : 'major-update');
-    app.modal.show(LoadingModal);
-
-    app
-      .request<AsyncBackendResponse | null>({
-        method: 'POST',
-        url: `${app.forum.attribute('apiUrl')}/package-manager/major-update`,
-        body: {
-          data: { dryRun },
-        },
-      })
-      .then((response) => {
-        if (response?.processing) {
-          jumpToQueue();
-        } else {
-          app.alerts.show({ type: 'success' }, app.translator.trans('flarum-package-manager.admin.update_successful'));
-          window.location.reload();
-        }
-      })
-      .catch(errorHandler)
-      .catch((e: RequestError) => {
-        app.modal.close();
-        this.updateState.status = 'failure';
-        this.updateState.incompatibleExtensions = e.response?.errors?.pop()?.incompatible_extensions as string[];
-      })
-      .finally(() => {
-        app.packageManager.control.setLoading(null);
-        m.redraw();
-      });
+    app.packageManager.control.majorUpdate({ dryRun });
   }
 }
diff --git a/extensions/package-manager/js/src/admin/components/Updater.tsx b/extensions/package-manager/js/src/admin/components/Updater.tsx
@@ -48,7 +48,7 @@ export default class Updater extends Component<IUpdaterAttrs> {
   availableUpdatesView() {
     const state = app.packageManager.control;
 
-    if (app.packageManager.control.isLoading()) {
+    if (app.packageManager.control.isLoading('check') || app.packageManager.control.isLoading('global-update')) {
       return (
         <div className="PackageManager-extensions">
           <LoadingIndicator />
@@ -101,7 +101,7 @@ export default class Updater extends Component<IUpdaterAttrs> {
         icon="fas fa-sync-alt"
         onclick={() => app.packageManager.control.checkForUpdates()}
         loading={app.packageManager.control.isLoading('check')}
-        disabled={app.packageManager.control.isLoadingOtherThan('check')}
+        disabled={app.packageManager.control.isLoading()}
       >
         {app.translator.trans('flarum-package-manager.admin.updater.check_for_updates')}
       </Button>,
@@ -115,7 +115,7 @@ export default class Updater extends Component<IUpdaterAttrs> {
         icon="fas fa-play"
         onclick={() => app.packageManager.control.updateGlobally()}
         loading={app.packageManager.control.isLoading('global-update')}
-        disabled={app.packageManager.control.isLoadingOtherThan('global-update')}
+        disabled={app.packageManager.control.isLoading()}
       >
         {app.translator.trans('flarum-package-manager.admin.updater.run_global_update')}
       </Button>

diff --git a/extensions/package-manager/js/src/admin/index.tsx b/extensions/package-manager/js/src/admin/index.tsx
@@ -36,6 +36,7 @@ app.initializers.add('flarum-package-manager', (app) => {
       default: false,
       type: 'boolean',
       disabled: app.data['flarum-package-manager.using_sync_queue'],
+      // @todo async to sync while setting is enabled
     })
     .registerSetting({
       setting: 'flarum-package-manager.task_retention_days',

diff --git a/extensions/package-manager/js/src/admin/states/ControlSectionState.ts b/extensions/package-manager/js/src/admin/states/ControlSectionState.ts
@@ -8,6 +8,7 @@ import errorHandler from '../utils/errorHandler';
 import jumpToQueue from '../utils/jumpToQueue';
 import { Extension } from 'flarum/admin/AdminApplication';
 import extractText from 'flarum/common/utils/extractText';
+import RequestError from 'flarum/common/utils/RequestError';
 
 export type UpdatedPackage = {
   name: string;
@@ -43,7 +44,7 @@ export type LastUpdateRun = {
   limitedPackages: () => string[];
 };
 
-export type LoadingTypes = UpdaterLoadingTypes | InstallerLoadingTypes | MajorUpdaterLoadingTypes;
+export type LoadingTypes = UpdaterLoadingTypes | InstallerLoadingTypes | MajorUpdaterLoadingTypes | 'queued-action';
 
 export type CoreUpdate = {
   package: UpdatedPackage;
@@ -79,14 +80,42 @@ export default class ControlSectionState {
     return (name && this.loading === name) || (!name && this.loading !== null);
   }
 
-  isLoadingOtherThan(name: LoadingTypes): boolean {
-    return this.loading !== null && this.loading !== name;
-  }
-
   setLoading(name: LoadingTypes): void {
     this.loading = name;
   }
 
+  requirePackage(data: any) {
+    app.packageManager.control.setLoading('extension-install');
+    app.modal.show(LoadingModal);
+
+    app
+      .request<AsyncBackendResponse & { id: number }>({
+        method: 'POST',
+        url: `${app.forum.attribute('apiUrl')}/package-manager/extensions`,
+        body: {
+          data,
+        },
+      })
+      .then((response) => {
+        if (response.processing) {
+          jumpToQueue();
+        } else {
+          const extensionId = response.id;
+          app.alerts.show(
+            { type: 'success' },
+            app.translator.trans('flarum-package-manager.admin.extensions.successful_install', { extension: extensionId })
+          );
+          window.location.href = `${app.forum.attribute('adminUrl')}#/extension/${extensionId}`;
+          window.location.reload();
+        }
+      })
+      .catch(errorHandler)
+      .finally(() => {
+        app.modal.close();
+        m.redraw();
+      });
+  }
+
   checkForUpdates() {
     this.setLoading('check');
 
@@ -102,12 +131,12 @@ export default class ControlSectionState {
           this.lastUpdateCheck = response as LastUpdateCheck;
           this.extensionUpdates = this.formatExtensionUpdates(response as LastUpdateCheck);
           this.coreUpdate = this.formatCoreUpdate(response as LastUpdateCheck);
+          this.setLoading(null);
           m.redraw();
         }
       })
       .catch(errorHandler)
       .finally(() => {
-        this.setLoading(null);
         m.redraw();
       });
   }
@@ -132,7 +161,6 @@ export default class ControlSectionState {
         })
         .catch(errorHandler)
         .finally(() => {
-          this.setLoading(null);
           app.modal.close();
           m.redraw();
         });
@@ -163,7 +191,6 @@ export default class ControlSectionState {
       })
       .catch(errorHandler)
       .finally(() => {
-        this.setLoading(null);
         app.modal.close();
         m.redraw();
       });
@@ -188,7 +215,6 @@ export default class ControlSectionState {
       })
       .catch(errorHandler)
       .finally(() => {
-        this.setLoading(null);
         app.modal.close();
         m.redraw();
       });
@@ -236,4 +262,36 @@ export default class ControlSectionState {
       },
     };
   }
+
+  majorUpdate({ dryRun }: { dryRun: boolean }) {
+    app.packageManager.control.setLoading(dryRun ? 'major-update-dry-run' : 'major-update');
+    app.modal.show(LoadingModal);
+    const updateState = this.lastUpdateRun.major;
+
+    app
+      .request<AsyncBackendResponse | null>({
+        method: 'POST',
+        url: `${app.forum.attribute('apiUrl')}/package-manager/major-update`,
+        body: {
+          data: { dryRun },
+        },
+      })
+      .then((response) => {
+        if (response?.processing) {
+          jumpToQueue();
+        } else {
+          app.alerts.show({ type: 'success' }, app.translator.trans('flarum-package-manager.admin.update_successful'));
+          window.location.reload();
+        }
+      })
+      .catch(errorHandler)
+      .catch((e: RequestError) => {
+        app.modal.close();
+        updateState.status = 'failure';
+        updateState.incompatibleExtensions = e.response?.errors?.pop()?.incompatible_extensions as string[];
+      })
+      .finally(() => {
+        m.redraw();
+      });
+  }
 }
diff --git a/extensions/package-manager/js/src/admin/states/QueueState.ts b/extensions/package-manager/js/src/admin/states/QueueState.ts
@@ -27,11 +27,13 @@ export default class QueueState {
       m.redraw();
 
       // Check if there is a pending or running task
-      const task = data?.find((task) => task.status() === 'pending' || task.status() === 'running');
+      const pendingTask = data?.find((task) => task.status() === 'pending' || task.status() === 'running');
 
-      if (task) {
+      if (pendingTask) {
         this.pollQueue(actionTaken);
       } else if (actionTaken) {
+        app.packageManager.control.setLoading(null);
+
         // Refresh the page
         window.location.reload();
       }

diff --git a/extensions/package-manager/js/src/admin/utils/errorHandler.ts b/extensions/package-manager/js/src/admin/utils/errorHandler.ts
@@ -1,12 +1,16 @@
 import app from 'flarum/admin/app';
 
 export default function (e: any) {
+  app.packageManager.control.setLoading(null);
+
   const error = e.response.errors[0];
 
   if (!['composer_command_failure', 'extension_already_installed', 'extension_not_installed'].includes(error.code)) {
     throw e;
   }
 
+  app.alerts.clear();
+
   switch (error.code) {
     case 'composer_command_failure':
       if (error.guessed_cause) {

diff --git a/extensions/package-manager/less/admin/ControlSection.less b/extensions/package-manager/less/admin/ControlSection.less
@@ -154,7 +154,7 @@
 }
 
 .PackageManager-installer .FormControl-container {
-  max-width: 400px;
+  max-width: 450px;
 
   .FormControl {
     width: 300px;

diff --git a/extensions/package-manager/src/Job/Dispatcher.php b/extensions/package-manager/src/Job/Dispatcher.php
@@ -68,13 +68,17 @@ public function dispatch(AbstractActionCommand $command): DispatcherResponse
     {
         $queueJobs = ($this->runSyncOverride === false) || ($this->runSyncOverride !== true && $this->settings->get('flarum-package-manager.queue_jobs'));
 
-        // @todo: check and skip if there is already a pending or running task.
+        // Skip if there is already a pending or running task.
+        if ($queueJobs && Task::query()->whereIn('status', [Task::PENDING, Task::RUNNING])->exists()) {
+            return new DispatcherResponse(true, null);
+        }
 
         if ($queueJobs && (! $this->queue instanceof SyncQueue)) {
             $task = Task::build($command->getOperationName(), $command->package ?? null);
 
             $command->task = $task;
 
+            // @todo: show guessed caused for queueing
             $this->queue->push(
                 new ComposerCommandJob($command, PHP_VERSION)
             );