[BUGFIX] various bugfixes (#26)

* [WIP] fixes * [TASK] ci fix * [TASK] add php-version 8.2 to github workflow * [WIP] intercept errors when API connection fails, logging, messages * [TASK] bump version
pixelant · Nov 23, 2023 · cf03df6 · cf03df6
1 parent 0869a16
commit cf03df6
Show file tree

Hide file tree

Showing 18 changed files with 307 additions and 103 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -21,6 +21,7 @@ jobs:
       matrix:
         php-version:
           - 8.1
+          - 8.2
   typoscript-lint:
     name: "TypoScript linter"
     runs-on: ubuntu-20.04
@@ -80,6 +81,7 @@ jobs:
           - "php:sniff"
         php-version:
           - 8.1
+          - 8.2
   xliff-lint:
     name: "Xliff linter"
     runs-on: ubuntu-20.04
@@ -115,12 +117,12 @@ jobs:
         run: |
           composer require --no-progress typo3/minimal:"$TYPO3"
           composer show
-      - if: "matrix.composer-dependencies == 'lowest'"
+      - if: matrix.composer-dependencies == 'lowest'
         name: "Install lowest dependencies with composer"
         run: |
           composer update --no-ansi --no-interaction --no-progress --with-dependencies --prefer-lowest
           composer show
-      - if: "matrix.composer-dependencies == 'highest'"
+      - if: matrix.composer-dependencies == 'highest'
         name: "Install highest dependencies with composer"
         run: |
           composer update --no-ansi --no-interaction --no-progress --with-dependencies
@@ -135,5 +137,6 @@ jobs:
           - lowest
         php-version:
           - 8.1
+          - 8.2
         typo3-version:
           - ^12.4
diff --git a/Classes/Command/UpdateQbankFileStatusCommand.php b/Classes/Command/UpdateQbankFileStatusCommand.php
@@ -115,6 +115,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
                     continue;
                 }
+            } catch (\Throwable $th) {
+                $io->writeln(
+                    sprintf(
+                        'QBank file [%s] was not found: "%s"',
+                        $file['tx_qbank_id'],
+                        $th->getMessage()
+                    )
+                );
+                break;
             }
 
             $remoteUpdate = (int)$media->getUpdated()->getTimestamp();

diff --git a/Classes/Controller/ManagementController.php b/Classes/Controller/ManagementController.php
@@ -33,6 +33,7 @@
 use TYPO3\CMS\Core\Imaging\Icon;
 use TYPO3\CMS\Core\Imaging\IconFactory;
 use TYPO3\CMS\Core\Localization\LanguageService;
+use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\Messaging\AbstractMessage;
 use TYPO3\CMS\Core\Page\JavaScriptModuleInstruction;
 use TYPO3\CMS\Core\Page\PageRenderer;
@@ -90,7 +91,24 @@ public function initializeAction(): void
      */
     protected function initializeView(): void
     {
+        $apiStatus = [];
+        try {
+            $checkStatus = $this->qbankService->fetchMediaProperties();
+        } catch (\Throwable $th) {
+            $apiStatus = [
+                'errorMessage' => $th->getMessage(),
+                'errorCode' => $th->getCode(),
+            ];
+            $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
+            $logger->error(
+                sprintf(
+                    'Failed to connect to QBank API: "%s"',
+                    $th->getMessage()
+                )
+            );
+        }
         $this->moduleTemplate->assignMultiple([
+            'apiStatus' => $apiStatus,
             'dateFormat' => $GLOBALS['TYPO3_CONF_VARS']['SYS']['ddmmyy'],
             'timeFormat' => $GLOBALS['TYPO3_CONF_VARS']['SYS']['hhmm'],
             'dateTimeFormat' =>
@@ -192,7 +210,11 @@ protected function overviewAction(): ResponseInterface
     {
         $this->generateDropdownMenu('overview');
         $this->generateButtons('overview');
-        $properties = $this->qbankService->fetchMediaProperties();
+        try {
+            $properties = $this->qbankService->fetchMediaProperties();
+        } catch (\Throwable $th) {
+            $properties = [];
+        }
         $this->moduleTemplate->assign('properties', $properties);
         return $this->moduleTemplate->renderResponse('Management/Overview');
     }
@@ -206,9 +228,14 @@ protected function mappingsAction(): ResponseInterface
         $this->generateButtons('mappings');
         $mappingRepository = GeneralUtility::makeInstance(MappingRepository::class);
         $mappings = $mappingRepository->findAll();
+        try {
+            $mediaProperties = $this->qbankService->fetchMediaProperties();
+        } catch (\Throwable $th) {
+            $mediaProperties = [];
+        }
         $this->moduleTemplate->assignMultiple([
             'mappings' => $mappings,
-            'mediaProperties' => $this->qbankService->fetchMediaProperties(),
+            'mediaProperties' => $mediaProperties,
             'fileProperties' => PropertyUtility::getFileProperties(),
         ]);
         return $this->moduleTemplate->renderResponse('Management/Mappings');

diff --git a/Classes/Hook/MediaUsageReporterDataHandlerHook.php b/Classes/Hook/MediaUsageReporterDataHandlerHook.php
@@ -5,7 +5,9 @@
 namespace Pixelant\Qbank\Hook;
 
 use Pixelant\Qbank\Service\QbankService;
+use Pixelant\Qbank\Utility\MessageUtility;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
+use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
 
@@ -26,9 +28,20 @@ public function processDatamap_afterAllOperations(DataHandler $dataHandler): voi
         foreach ($dataHandler->datamap['sys_file_reference'] ?? [] as $id => $record) {
             // Only process new records
             if (!MathUtility::canBeInterpretedAsInteger($id)) {
-                GeneralUtility::makeInstance(QbankService::class)->reportMediaUsageInFileReference(
-                    $dataHandler->substNEWwithIDs[$id]
-                );
+                try {
+                    GeneralUtility::makeInstance(QbankService::class)->reportMediaUsageInFileReference(
+                        $dataHandler->substNEWwithIDs[$id]
+                    );
+                } catch (\Throwable $th) {
+                    MessageUtility::enqueueMessage(
+                        sprintf(
+                            'Media usage was not reported to QBank: "%s"',
+                            $th->getMessage()
+                        ),
+                        'Connection to QBank API failed',
+                        ContextualFeedbackSeverity::ERROR
+                    );
+                }
             }
         }
     }
@@ -52,22 +65,33 @@ public function processCmdmap_preProcess(
         DataHandler $dataHandler
     ): void {
         if ($table === 'sys_file_reference') {
-            switch ($command) {
-                case 'move':
-                    /** @var QbankService $qbankService */
-                    $qbankService = GeneralUtility::makeInstance(QbankService::class);
-                    $qbankService->removeMediaUsageInFileReference($id);
-                    $qbankService->reportMediaUsageInFileReference($id);
+            try {
+                switch ($command) {
+                    case 'move':
+                        /** @var QbankService $qbankService */
+                        $qbankService = GeneralUtility::makeInstance(QbankService::class);
+                        $qbankService->removeMediaUsageInFileReference($id);
+                        $qbankService->reportMediaUsageInFileReference($id);
 
-                    break;
-                case 'delete':
-                    GeneralUtility::makeInstance(QbankService::class)->removeMediaUsageInFileReference($id);
+                        break;
+                    case 'delete':
+                        GeneralUtility::makeInstance(QbankService::class)->removeMediaUsageInFileReference($id);
 
-                    break;
-                case 'undelete':
-                    GeneralUtility::makeInstance(QbankService::class)->reportMediaUsageInFileReference($id);
+                        break;
+                    case 'undelete':
+                        GeneralUtility::makeInstance(QbankService::class)->reportMediaUsageInFileReference($id);
 
-                    break;
+                        break;
+                }
+            } catch (\Throwable $th) {
+                MessageUtility::enqueueMessage(
+                    sprintf(
+                        'Media usage was not reported to QBank: "%s"',
+                        $th->getMessage()
+                    ),
+                    'Connection to QBank API failed',
+                    ContextualFeedbackSeverity::ERROR
+                );
             }
         }
     }

diff --git a/Classes/Repository/AbstractRepository.php b/Classes/Repository/AbstractRepository.php
@@ -6,7 +6,9 @@
 
 use Pixelant\Qbank\Utility\QbankUtility;
 use QBNK\QBank\API\QBankApi;
+use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\SingletonInterface;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 class AbstractRepository implements SingletonInterface
 {
@@ -17,6 +19,16 @@ class AbstractRepository implements SingletonInterface
 
     public function __construct()
     {
-        $this->api = QbankUtility::getApi();
+        try {
+            $this->api = QbankUtility::getApi();
+        } catch (\Throwable $th) {
+            $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
+            $logger->error(
+                sprintf(
+                    'Failed to connect to QBank API: "%s"',
+                    $th->getMessage()
+                )
+            );
+        }
     }
 }
diff --git a/Classes/Repository/MediaUsageRepository.php b/Classes/Repository/MediaUsageRepository.php
@@ -81,7 +81,7 @@ public function findByQbankAndLocalId(int $qbankId, string $localId): array
 
         $result = [];
         foreach ($usages as $usage) {
-            if ($usage->getContext()['localID'] === $localId) {
+            if (($usage->getContext()['localID'] ?? '') === $localId) {
                 $result[] = $usage;
             }
         }
@@ -103,7 +103,7 @@ public function findByQbankId(int $qbankId): array
             return $this->api->media()->listUsages($qbankId);
         }
 
-        return $this->api->media()->listUsages($qbankId, $sessionSourceId);
+        return $this->api->media()->listUsages($qbankId, (string)$sessionSourceId);
     }
 
     /**

diff --git a/Classes/Service/QbankService.php b/Classes/Service/QbankService.php
@@ -20,6 +20,7 @@
 use Pixelant\Qbank\Service\Event\FilePropertyChangeEvent;
 use Pixelant\Qbank\Service\Event\FileReferenceUrlEvent;
 use Pixelant\Qbank\Service\Event\ResolvePageTitleEvent;
+use Pixelant\Qbank\Utility\MessageUtility;
 use Pixelant\Qbank\Utility\PropertyUtility;
 use Pixelant\Qbank\Utility\QbankUtility;
 use QBNK\QBank\API\Exception\RequestException;
@@ -37,6 +38,7 @@
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Site\SiteFinder;
+use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\StringUtility;
 
@@ -252,6 +254,17 @@ public function synchronizeMetadata(int $fileId): void
                     1625149218
                 );
             }
+        } catch (\Throwable $th) {
+            MessageUtility::enqueueMessage(
+                sprintf(
+                    'Could not synchronize metadata for file [%s]: "%s"',
+                    $fileId,
+                    $th->getMessage()
+                ),
+                'QBank',
+                ContextualFeedbackSeverity::ERROR
+            );
+            return;
         }
 
         $metaDataMappings = GeneralUtility::makeInstance(MappingRepository::class)->findAllAsKeyValuePairs(false);

diff --git a/Classes/Utility/MessageUtility.php b/Classes/Utility/MessageUtility.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pixelant\Qbank\Utility;
+
+use TYPO3\CMS\Core\Context\Context;
+use TYPO3\CMS\Core\Log\LogManager;
+use TYPO3\CMS\Core\Messaging\FlashMessage;
+use TYPO3\CMS\Core\Messaging\FlashMessageQueue;
+use TYPO3\CMS\Core\Messaging\FlashMessageService;
+use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
+/**
+ * Convenience methods relating to TYPO3 messaging.
+ */
+class MessageUtility
+{
+    public static function enqueueMessage(
+        string $message,
+        string $title = '',
+        ContextualFeedbackSeverity $severity = ContextualFeedbackSeverity::OK
+    ): void {
+        $context = GeneralUtility::makeInstance(Context::class);
+        $beUserId = $context->getPropertyFromAspect('backend.user', 'id');
+
+        if ($beUserId > 0) {
+            $flashMessageService = GeneralUtility::makeInstance(FlashMessageService::class);
+            $notificationQueue = $flashMessageService->getMessageQueueByIdentifier(
+                FlashMessageQueue::NOTIFICATION_QUEUE
+            );
+            $flashMessage = GeneralUtility::makeInstance(
+                FlashMessage::class,
+                $message,
+                $title,
+                $severity,
+                true
+            );
+            $notificationQueue->enqueue($flashMessage);
+        }
+
+        $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
+        $logger->error($message . PHP_EOL . $title);
+    }
+}
diff --git a/Classes/Utility/PropertyUtility.php b/Classes/Utility/PropertyUtility.php
@@ -17,8 +17,6 @@
  */
 class PropertyUtility
 {
-    protected static $filePropertyTypeConverterCache = [];
-
     /**
      * Returns the file properties array from $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['qbank']['fileProperties'].
      *
@@ -29,12 +27,10 @@ public static function getFileProperties(): array
         $properties = $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['qbank']['fileProperties'] ?? [];
 
         foreach ($properties as $key => &$property) {
-            if (is_callable($property['label'])) {
-                $property['label'] = $property['label']() ?? $key;
-            }
-
             if (!isset($property['label'])) {
-                $property['label'] = $key;
+                $property['label'] = $GLOBALS['TCA']['sys_file']['columns'][$key]['label']
+                    ?? $GLOBALS['TCA']['sys_file_metadata']['columns'][$key]['label']
+                    ?? $key;
             }
         }
 
@@ -50,10 +46,6 @@ public static function getFileProperties(): array
      */
     public static function getTypeConverterForFileProperty(string $filePropertyName): TypeConverterInterface
     {
-        if (isset(self::$filePropertyTypeConverterCache[$filePropertyName])) {
-            return self::$filePropertyTypeConverterCache[$filePropertyName];
-        }
-
         $propertyConfiguration = self::getFileProperties()[$filePropertyName] ?? null;
 
         if (!isset($propertyConfiguration['typeConverter'])) {

diff --git a/Resources/Private/Language/locallang_mod_qbank.xlf b/Resources/Private/Language/locallang_mod_qbank.xlf
@@ -21,6 +21,9 @@
 			<trans-unit id="target_property">
 				<source>TYPO3 property</source>
 			</trans-unit>
+			<trans-unit id="api_error">
+				<source>Connection to QBank API failed</source>
+			</trans-unit>
 		</body>
 	</file>
 </xliff>
diff --git a/Resources/Private/Partials/ApiStatus.html b/Resources/Private/Partials/ApiStatus.html
@@ -0,0 +1,10 @@
+<f:if condition="{apiStatus -> f:count()}">
+    <div role="alert" class="alert alert-danger alert-dismissible">
+        <div class="media">
+            <div class="media-body">
+                <h4 class="alert-title">{f:translate(key: 'LLL:EXT:qbank/Resources/Private/Language/locallang_mod_qbank.xlf:api_error')}</h4>
+                <p class="alert-message">{apiStatus.errorMessage}</p>
+            </div>
+        </div>
+    </div>
+</f:if>
diff --git a/Resources/Private/Templates/Management/List.html b/Resources/Private/Templates/Management/List.html
@@ -11,6 +11,8 @@
     <h1><f:translate key="be.list.title" /></h1>
     <p><f:translate key="be.list.description" /></p>
 
+    <f:render partial="ApiStatus" arguments="{apiStatus: apiStatus}" />
+
     <f:if condition="{qbankFiles -> f:count()}">
         <f:render section="table" arguments="{_all}" />
     </f:if>