Merge pull request #12 from heavymetalavo/hotfix/asset-handling

hotfix/asset-handling

Author: joepagan

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Release Notes for AI Alt Text
 
+## 1.5.3 - 2025-05-09
+- Improve bulk action notice wording
+- Improved logic to not skip generating alt text for an asset where a job is in the queue but it has a failed status
+- Replacing `preSaveAsset` setting with `propagate` setting, `preSaveAsset` tried to resolve an issue where the same value could be saved over multiple sites. Could sometimes cause errors e.g. `Failed to pre-save asset: filename.png`, 
+- Replacing native `file_get_contents` function with `$assets->getContents` in animated gif test, which is more reliable across asset different platforms
+- Removing tests to check an asset's file extension which is not a reliable way to ascertain if the file will be accepted by the OpenAI API
+- Updating tests to check an asset's mime type to ascertain if an image transform to a different format is required before it is sent to OpenAI API
+- Added new test to check if resulting transform which will be sent to OpenAI is accepted mime type
+- Added new test to check if SVGs can be transformed to an accepted mime type
+
 ## 1.5.2 - 2025-05-06
 
 - Updating changelog formatting slightly to test supporting Craft's `Utilities → Updates` screen 

src/controllers/GenerateController.php

@@ -177,17 +177,15 @@ public function actionGenerateAssetsWithoutAltText(): Response
             if ($siteId) {
                 $siteName = $sites[0]->name;
                 Craft::$app->getSession()->setNotice(
-                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets in site {site} (out of {total} total assets without alt text).', [
+                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets in site {site}', [
                         'count' => $queuedCount,
-                        'total' => $totalCount,
                         'site' => $siteName
                     ])
                 );
             } else {
                 Craft::$app->getSession()->setNotice(
-                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets across all sites (out of {total} total assets without alt text).', [
+                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets across all sites', [
                         'count' => $queuedCount,
-                        'total' => $totalCount
                     ])
                 );
             }
@@ -306,17 +304,15 @@ public function actionGenerateAllAssets(): Response
             if ($siteId) {
                 $siteName = $sites[0]->name;
                 Craft::$app->getSession()->setNotice(
-                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets in site {site} (out of {total} total image assets).', [
+                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets in site {site}.', [
                         'count' => $queuedCount,
-                        'total' => $totalCount,
                         'site' => $siteName
                     ])
                 );
             } else {
                 Craft::$app->getSession()->setNotice(
-                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets across all sites (out of {total} total image assets).', [
+                    Craft::t('ai-alt-text', 'Queued alt text generation for {count} assets across all sites.', [
                         'count' => $queuedCount,
-                        'total' => $totalCount
                     ])
                 );
             }

src/models/Settings.php

@@ -48,9 +48,9 @@ class Settings extends Model
     public string $openAiImageInputDetailLevel = 'low';
 
     /**
-     * @var bool Whether to pre-save the asset if alt field is empty before saving a value to it, prevents same value being saved to each Site
+     * @var bool Whether the asset should be saved across all of its supported sites, if enabled it could save the same initial alt text value across all sites.
      */
-    public bool $preSaveAsset = true;
+    public bool $propagate = false;
 
     /**
      * @var bool Whether to save the translated result to each Site's Asset's translatable alt text field
@@ -74,7 +74,7 @@ public function defineRules(): array
             ['prompt', 'string'],
             ['openAiImageInputDetailLevel', 'string'],
             ['openAiImageInputDetailLevel', 'in', 'range' => ['low', 'high']],
-            ['preSaveAsset', 'boolean'],
+            ['propagate', 'boolean'],
             ['saveTranslatedResultsToEachSite', 'boolean'],
             ['generateForNewAssets', 'boolean'],
         ];

src/services/AiAltTextService.php

@@ -55,9 +55,10 @@ public function createJob(Asset $asset, $saveCurrentSiteOffQueue = false, $curre
             $hasExistingJob = false;
             foreach ($existingJobs as $job) {
                 // Only skip if both asset ID AND site ID match an existing job
-                if (isset($job['description']) && 
-                    str_contains($job['description'], "Asset: $asset->id") && 
-                    str_contains($job['description'], "Site: $assetSiteId")) {
+                if (isset($job['description'])
+                    && str_contains($job['description'], "Asset: $asset->id")
+                    && str_contains($job['description'], "Site: $assetSiteId")
+                    && $job['status'] !== 4) {
                     $hasExistingJob = true;
                     break;
                 }
@@ -143,23 +144,15 @@ public function generateAltText(Asset $asset, int $siteId = null, bool $forceReg
             throw new Exception('Asset must be an image');
         }
 
-        // Only pre-save if the alt is empty and we're not forcing regeneration
-        if (AiAltText::getInstance()->getSettings()->preSaveAsset && 
-            (empty($asset->alt) || $forceRegeneration)) {
-            $asset->alt = '';
-            if (!Craft::$app->elements->saveElement($asset)) {
-                throw new Exception('Failed to pre-save asset: ' . $asset->filename);
-            }
-        }
-
         $altText = $this->openAiService->generateAltText($asset, $siteId);
 
         if (empty($altText)) {
             throw new Exception('Empty alt text generated for asset: ' . $asset->filename);
         }
 
         $asset->alt = $altText;
-        if (!Craft::$app->elements->saveElement($asset, true)) {
+        $propagate = AiAltText::getInstance()->getSettings()->propagate;
+        if (!Craft::$app->elements->saveElement($asset, true, $propagate)) {
             throw new Exception('Failed to save alt text for asset: ' . $asset->filename);
         }
 

src/services/OpenAiService.php

@@ -154,21 +154,13 @@ private function isUrlAccessible(string $url): bool
      * Validates if the asset is an accepted image format and converts if needed
      *
      * @param Asset $asset The asset to validate
-     * @return bool Whether the asset is an accepted format or was successfully converted
+     * @return bool Whether the asset is an accepted mimetype or was successfully converted
      * @throws ImageTransformException
      */
     private function isValidImageFormat(Asset $asset): bool
     {
-        $extension = strtolower($asset->getExtension());
         $mimeType = strtolower($asset->getMimeType());
 
-        // Check for accepted extensions
-        $acceptedExtensions = ['png', 'jpg', 'jpeg', 'webp', 'gif'];
-        if (!in_array($extension, $acceptedExtensions)) {
-            Craft::warning('Asset has unsupported extension: ' . $extension . '. Will attempt to convert to JPEG.', __METHOD__);
-            return true; // We'll handle conversion in generateAltText
-        }
-
         // Check for accepted MIME types
         $acceptedMimeTypes = [
             'image/png',
@@ -184,14 +176,10 @@ private function isValidImageFormat(Asset $asset): bool
 
         // For GIFs, check if they're animated
         if ($extension === 'gif' && $mimeType === 'image/gif') {
-            $filePath = $asset->getPath();
-            if (file_exists($filePath)) {
-                $fileContents = file_get_contents($filePath);
-                // Check for multiple image frames in GIF
-                if (substr_count($fileContents, "\x21\xF9\x04") > 1) {
-                    Craft::warning('Animated GIF detected. Will attempt to convert first frame to JPEG.', __METHOD__);
-                    return true; // We'll handle conversion in generateAltText
-                }
+            $fileContents = $asset->getContents();
+            // Check for multiple image frames in GIF
+            if (substr_count($fileContents, "\x21\xF9\x04") > 1) {
+                return false;
             }
         }
 
@@ -228,9 +216,30 @@ public function generateAltText(Asset $asset, int $siteId = null): string
         $height = $asset->getHeight();
 
         // Check if format needs to be converted
-        $extension = strtolower($asset->getExtension());
-        $acceptedExtensions = ['png', 'jpg', 'jpeg', 'webp', 'gif'];
-        $needsFormatConversion = !in_array($extension, $acceptedExtensions);
+        $acceptedMimeTypes = [
+            'image/png',
+            'image/jpeg',
+            'image/webp',
+            'image/gif',
+        ];
+        $assetMimeType = strtolower($asset->getMimeType());
+
+        // Check if the asset is an animated gif which OpenAI API does not support
+        if ($assetMimeType === 'image/gif') {
+            $fileContents = $asset->getContents();
+            // Check for multiple image frames in GIF
+            if (substr_count($fileContents, "\x21\xF9\x04") > 1) {
+                throw new Exception('Animated GIF detected, this is not supported.');
+            }
+        }
+        
+        // decide if we need to transform svgs
+        if (!Craft::$app->getConfig()->getGeneral()->transformSvgs && $assetMimeType === 'image/svg+xml') {
+            throw new Exception('SVGs are not supported by the OpenAI API and transformSvgs is disabled.');
+        }
+
+        // decide if we need to transform the image to become a jpeg
+        $needsFormatConversion = !in_array($assetMimeType, $acceptedMimeTypes);
 
         // Set up transform parameters
         $transformParams = [];
@@ -247,9 +256,15 @@ public function generateAltText(Asset $asset, int $siteId = null): string
             $transformParams['mode'] = 'fit';
         }
 
-        // Get the image URL with transformation if needed
+        // Check mime type of the transform:
+        $transformMimeType = $asset->getMimeType($transformParams);
+        if (!in_array($transformMimeType, $acceptedMimeTypes)) {
+            throw new Exception("Asset transform has unsupported MIME type: $transformMimeType");
+        }
+
+        $assetTransform = $asset->setTransform($transformParams);
         // Make sure that we do not get a "generate transform" url, but a real url with true
-        $imageUrl = $asset->getUrl($transformParams, true);
+        $imageUrl = $assetTransform->getUrl($transformParams, true);
 
         // If we have a URL, check if it's accessible remotely
         if (!empty($imageUrl)) {
@@ -261,17 +276,11 @@ public function generateAltText(Asset $asset, int $siteId = null): string
 
         // If no public URL is available or URL is not accessible, try to get the file contents and encode as base64
         if (empty($imageUrl) || !$asset->getVolume()->getFs()->hasUrls) {
-            $assetContents = $asset->getContents();
-
-            // Get the MIME type
-            $mimeType = $asset->getMimeType();
-            if (empty($mimeType)) {
-                $mimeType = 'image/jpeg'; // Default to JPEG if MIME type is unknown
-            }
+            $assetContents = $assetTransform->getContents();
 
             // Encode as base64 and create data URI
             $base64Image = base64_encode($assetContents);
-            $imageUrl = "data:$mimeType;base64,$base64Image";
+            $imageUrl = "data:$transformMimeType;base64,$base64Image";
         }
 
         $detail = App::parseEnv(AiAltText::getInstance()->getSettings()->openAiImageInputDetailLevel) ?? 'low';

src/templates/_settings.twig

@@ -117,12 +117,12 @@
 }) }}
 
 {{ forms.lightswitchField({
-    label: 'Pre-save Asset'|t('aialttext'),
-    id: 'preSaveAsset',
-    name: 'preSaveAsset',
-    instructions: "If enabled, the plugin will pre-save the asset if alt field is empty before saving a value to it, prevents same initial value being saved to each Site"|t("aialttext"),
-    on: settings.preSaveAsset,
-    errors: settings.getErrors('preSaveAsset'),
+    label: 'propagate'|t('aialttext'),
+    id: 'propagate',
+    name: ' ',
+    instructions: "Whether the asset should be saved across all of its supported sites, if enabled it could save the same initial alt text value across all sites."|t("aialttext"),
+    on: settings.propagate,
+    errors: settings.getErrors('propagate'),
 }) }}
 
 {{ forms.lightswitchField({