Fix PublishPress Revisions filter corrupting field names on upload (WP-1026)

composer.json

@@ -1,7 +1,7 @@
 {
   "name": "smartling/wordpress-connector",
   "license": "GPL-2.0-or-later",
-  "version": "5.8.1",
+  "version": "5.8.2",
   "description": "",
   "type": "wordpress-plugin",
   "repositories": [

inc/Smartling/Base/ExportedAPI.php

@@ -131,6 +131,16 @@ interface ExportedAPI
      */
     public const FILTER_SMARTLING_METADATA_FIELD_PROCESS = 'smartling_metadata_string_process';
 
+    /**
+     * Filter to modify a field of the target entity (for example a post) before it is created from the source entity.
+     * Unlike {@see FILTER_SMARTLING_METADATA_FIELD_PROCESS}, the value is the first argument, so the value returned by
+     * a callback is what the next callback and the target entity receive.
+     * @param mixed field value
+     * @param string entity field name
+     * @param SubmissionEntity
+     */
+    public const FILTER_SMARTLING_TARGET_ENTITY_FIELD = 'smartling_target_entity_field';
+
     /**
      * Filter to modify meta value on translation
      * @param SubmissionEntity

inc/Smartling/Base/SmartlingCoreTrait.php

@@ -70,7 +70,8 @@ protected function prepareTargetEntity(SubmissionEntity $submission): Submission
 
         if ($targetContent instanceof EntityAbstract) {
             foreach ($filteredData['entity'] ?? [] as $k => $v) {
-                $targetContent->{$k} = $this->wpProxy->apply_filters(ExportedAPI::FILTER_SMARTLING_METADATA_FIELD_PROCESS, $k, $v, $submission);
+                $processed = $this->wpProxy->apply_filters(ExportedAPI::FILTER_SMARTLING_METADATA_FIELD_PROCESS, $k, $v, $submission);
+                $targetContent->{$k} = $this->wpProxy->apply_filters(ExportedAPI::FILTER_SMARTLING_TARGET_ENTITY_FIELD, $processed, $k, $submission);
             }
         }
 

inc/Smartling/Extensions/PublishPressRevisions.php

@@ -73,7 +73,7 @@ public function register(): void
         $this->wpProxy->add_action('before_delete_post', [$this, 'moveSubmissions'], 5);
         $this->wpProxy->add_action('revision_applied', [$this, 'resumeChangeDetection']);
         $this->wpProxy->add_action('shutdown', [$this, 'resumeAllChangeDetection']);
-        $this->wpProxy->add_filter(ExportedAPI::FILTER_SMARTLING_METADATA_FIELD_PROCESS, [$this, 'sanitizeTargetField'], 5, 3);
+        $this->wpProxy->add_filter(ExportedAPI::FILTER_SMARTLING_TARGET_ENTITY_FIELD, [$this, 'sanitizeTargetField'], 10, 3);
     }
 
     /**
@@ -228,7 +228,7 @@ private function unlink(SubmissionEntity $submission): void
     /**
      * The target of a revision must be an ordinary post: no revision status and no parent (the original is the parent).
      */
-    public function sanitizeTargetField(mixed $name, mixed $value, mixed $submission = null)
+    public function sanitizeTargetField(mixed $value, mixed $name, mixed $submission = null): mixed
     {
         if ($name === 'post_mime_type' && in_array($value, self::REVISION_STATUSES, true)) {
             return '';

inc/Smartling/Jobs/UploadJob.php

@@ -12,6 +12,7 @@
 use Smartling\Models\UploadQueueItem;
 use Smartling\Settings\SettingsManager;
 use Smartling\Submissions\SubmissionManager;
+use Smartling\Vendor\Psr\Log\LogLevel;
 
 class UploadJob extends JobAbstract
 {
@@ -128,18 +129,35 @@ private function processUploadQueue(int $blogId): void
             try {
                 $this->wpProxy->do_action(ExportedAPI::ACTION_SMARTLING_SEND_FOR_TRANSLATION, $item);
             } catch (\Throwable $e) {
-                $this->failItem($item, 'Failing', $e->getMessage());
+                $this->failItem(
+                    $item,
+                    'Failing',
+                    $e->getMessage(),
+                    sprintf('%s: %s in %s:%d', get_class($e), $e->getMessage(), $e->getFile(), $e->getLine()),
+                    LogLevel::ERROR,
+                );
             }
             $this->uploadQueueManager->complete($item);
             $this->placeLockFlag(true);
         }
     }
 
-    private function failItem(UploadQueueItem $item, string $logVerb, string $errorMessage, ?string $logMessage = null): void
-    {
+    private function failItem(
+        UploadQueueItem $item,
+        string $logVerb,
+        string $errorMessage,
+        ?string $logMessage = null,
+        string $logLevel = LogLevel::NOTICE,
+    ): void {
         $logMessage ??= $errorMessage;
         foreach ($item->getSubmissions() as $submission) {
-            $this->getLogger()->notice("$logVerb submissionId={$submission->getId()}: $logMessage");
+            $message = "$logVerb submissionId={$submission->getId()}: $logMessage";
+            // named methods, not log(): LevelLogger adds its string context only in those
+            if ($logLevel === LogLevel::ERROR) {
+                $this->getLogger()->error($message);
+            } else {
+                $this->getLogger()->notice($message);
+            }
             $this->submissionManager->setErrorMessage($submission, $errorMessage);
         }
     }

readme.txt

@@ -4,7 +4,7 @@ Tags: translation, localization, multilingual, internationalization, smartling
 Requires at least: 5.5
 Tested up to: 7.0
 Requires PHP: 8.0
-Stable tag: 5.8.1
+Stable tag: 5.8.2
 License: GPLv2 or later
 
 Translate content in WordPress quickly and seamlessly with Smartling, the industry-leading Translation Management System.
@@ -62,6 +62,9 @@ Additional information on the Smartling Connector for WordPress can be found [he
 3. Track translation status within WordPress from the Submissions Board. View overall progress of submitted translation requests as well as resend updated content.
 
 == Changelog ==
+= 5.8.2 =
+* Fixed uploads failing silently, leaving the Smartling job empty, when PublishPress Revisions is active and a post has array-valued metadata
+
 = 5.8.1 =
 * Added profile selector for sites with multiple active profiles.
 * Improved Elementor query support: term and post IDs used by Posts, Loop Grid and Loop Carousel queries are now detected and replaced with the translated ones, only for the query mode the widget actually uses. Query terms are stored by Elementor Pro as term taxonomy IDs and are converted accordingly. Excluded terms and posts are replaced only if they are already translated when the page translation is downloaded; if they are translated later, download the page again to update the query.

smartling-connector.php

@@ -11,7 +11,7 @@
  * Plugin Name:       Smartling Connector
  * Plugin URI:        https://www.smartling.com/products/automate/integrations/wordpress/
  * Description:       Integrate your WordPress site with Smartling to upload your content and download translations.
- * Version:           5.8.1
+ * Version:           5.8.2
  * Author:            Smartling
  * Author URI:        https://www.smartling.com
  * License:           GPL-2.0+

tests/Smartling/Base/SmartlingCoreTraitTest.php

@@ -27,7 +27,9 @@ class SmartlingCoreTraitTest extends TestCase
     public function testPrepareTargetContentClonedGuid()
     {
         $wpProxy = $this->createMock(WordpressFunctionProxyHelper::class);
-        $wpProxy->method('apply_filters')->willReturnArgument(2);
+        $wpProxy->method('apply_filters')->willReturnCallback(
+            static fn(string $tag, mixed ...$args) => $tag === ExportedAPI::FILTER_SMARTLING_TARGET_ENTITY_FIELD ? $args[0] : $args[1],
+        );
 
         $x = new SmartlingCore(
             $this->createMock(AcfDynamicSupport::class),
@@ -72,4 +74,67 @@ public function testPrepareTargetContentClonedGuid()
 
         $x->prepareTargetContent($submission);
     }
+
+    public function testPrepareTargetContentAppliesTargetEntityFieldFilter()
+    {
+        $calls = [];
+        $wpProxy = $this->createMock(WordpressFunctionProxyHelper::class);
+        $wpProxy->method('apply_filters')->willReturnCallback(
+            function (string $tag, mixed ...$args) use (&$calls) {
+                if ($tag === ExportedAPI::FILTER_SMARTLING_TARGET_ENTITY_FIELD) {
+                    $calls[] = $args;
+
+                    return $args[1] === 'post_mime_type' ? 'sanitized' : $args[0];
+                }
+
+                return $args[1];
+            },
+        );
+
+        $x = new SmartlingCore(
+            $this->createMock(AcfDynamicSupport::class),
+            $this->createMock(ExternalContentManager::class),
+            $this->createMock(FileUriHelper::class),
+            $this->createMock(GutenbergBlockHelper::class),
+            $this->createMock(PostContentHelper::class),
+            $this->createMock(UploadQueueManager::class),
+            $this->createMock(XmlHelper::class),
+            $this->createMock(TestRunHelper::class),
+            $wpProxy,
+        );
+
+        $entity = new PostEntityStd();
+        $entity->post_mime_type = 'draft-revision';
+        $entity->post_title = 'title';
+
+        $contentHelper = $this->createMock(ContentHelper::class);
+        $contentHelper->method('readSourceContent')->willReturn($entity);
+        $contentHelper->expects($this->once())->method('writeTargetContent')
+            ->willReturnCallback(function (SubmissionEntity $_, Entity $entity) {
+                $this->assertSame('sanitized', $entity->post_mime_type);
+                $this->assertSame('title', $entity->post_title);
+
+                return $entity;
+            });
+
+        $x->setContentHelper($contentHelper);
+        $x->setContentSerializationHelper($this->createMock(ContentSerializationHelper::class));
+        $x->setFieldsFilter($this->createMock(FieldsFilterHelper::class));
+        $x->setSettingsManager($this->createMock(SettingsManager::class));
+        $submissionManager = $this->createMock(SubmissionManager::class);
+        $submissionManager->method('storeEntity')->willReturnArgument(0);
+        $x->setSubmissionManager($submissionManager);
+        $x->setTranslationHelper($this->createMock(TranslationHelper::class));
+
+        $submission = $this->createMock(SubmissionEntity::class);
+        $submission->method('getTargetId')->willReturn(0);
+        $submission->method('isCloned')->willReturn(true);
+
+        $x->prepareTargetContent($submission);
+
+        $this->assertContains('post_mime_type', array_column($calls, 1));
+        foreach ($calls as $call) {
+            $this->assertSame($submission, $call[2]);
+        }
+    }
 }

tests/Smartling/Extensions/PublishPressRevisionsTest.php

@@ -3,6 +3,7 @@
 namespace Smartling\Tests\Extensions;
 
 use PHPUnit\Framework\TestCase;
+use Smartling\Base\ExportedAPI;
 use Smartling\DbAl\LocalizationPluginProxyInterface;
 use Smartling\Extensions\PublishPressRevisions;
 use Smartling\Helpers\ContentSerializationHelper;
@@ -205,10 +206,10 @@ public function testRevisionMimeTypeIsResetOnTarget(): void
     {
         $x = $this->x();
         foreach (PublishPressRevisions::REVISION_STATUSES as $status) {
-            $this->assertSame('', $x->sanitizeTargetField('post_mime_type', $status));
+            $this->assertSame('', $x->sanitizeTargetField($status, 'post_mime_type'));
         }
-        $this->assertSame('image/png', $x->sanitizeTargetField('post_mime_type', 'image/png'));
-        $this->assertSame('title', $x->sanitizeTargetField('post_title', 'title'));
+        $this->assertSame('image/png', $x->sanitizeTargetField('image/png', 'post_mime_type'));
+        $this->assertSame('title', $x->sanitizeTargetField('title', 'post_title'));
     }
 
     public function testParentIsResetOnlyForRevisions(): void
@@ -221,7 +222,109 @@ public function testParentIsResetOnlyForRevisions(): void
         $wp->method('getPostMeta')->willReturn('');
         $x = $this->x(null, null, $wp);
 
-        $this->assertSame(0, $x->sanitizeTargetField('post_parent', 10, $this->submission(20, 0)));
-        $this->assertSame(10, $x->sanitizeTargetField('post_parent', 10, $this->submission(30, 0)));
+        $this->assertSame(0, $x->sanitizeTargetField(10, 'post_parent', $this->submission(20, 0)));
+        $this->assertSame(10, $x->sanitizeTargetField(10, 'post_parent', $this->submission(30, 0)));
+    }
+
+    /**
+     * @return array<string, array<int, array<int, callable>>> tag => priority => callbacks
+     */
+    private function registerAndCaptureFilters(PublishPressRevisions $x, WordpressFunctionProxyHelper $wp): array
+    {
+        $captured = [];
+        $wp->method('add_filter')->willReturnCallback(function (string $tag, callable $callback, int $priority = 10) use (&$captured) {
+            $captured[$tag][$priority][] = $callback;
+
+            return true;
+        });
+        $x->register();
+
+        return $captured;
+    }
+
+    private function supportedWordpress(): WordpressFunctionProxyHelper
+    {
+        $wp = $this->createMock(WordpressFunctionProxyHelper::class);
+        $wp->method('get_current_blog_id')->willReturn(1);
+        $wp->method('get_plugins')->willReturn(['revisionary/revisionary.php' => ['Version' => '3.5.0']]);
+        $wp->method('is_plugin_active')->willReturn(true);
+
+        return $wp;
+    }
+
+    private function revisionsWithSupportedPlugin(WordpressFunctionProxyHelper $wp): PublishPressRevisions
+    {
+        $pluginHelper = $this->createMock(PluginHelper::class);
+        $pluginHelper->method('versionInRange')->willReturn(true);
+
+        return new PublishPressRevisions(
+            $pluginHelper,
+            $wp,
+            $this->createMock(LocalizationPluginProxyInterface::class),
+            $this->createMock(SubmissionManager::class),
+            $this->createMock(DetectChangesHelper::class),
+            $this->createMock(ContentSerializationHelper::class),
+        );
+    }
+
+    /**
+     * Mimics WordPress apply_filters(): callbacks run by priority, and the value returned by one callback replaces the
+     * first argument for the next one.
+     *
+     * @param array<int, array<int, callable>> $callbacks priority => callbacks
+     */
+    private function applyFilters(array $callbacks, mixed ...$args): mixed
+    {
+        ksort($callbacks);
+        $value = $args[0];
+        foreach ($callbacks as $priorityCallbacks) {
+            foreach ($priorityCallbacks as $callback) {
+                $value = $callback($value, ...array_slice($args, 1));
+            }
+        }
+
+        return $value;
+    }
+
+    public function testMetadataFieldNameIsNotReplacedByFieldValue(): void
+    {
+        $wp = $this->supportedWordpress();
+        $captured = $this->registerAndCaptureFilters($this->revisionsWithSupportedPlugin($wp), $wp);
+
+        $namesSeenByFieldProcessor = [];
+        // what MetaFieldProcessorManager::handlePostTranslationFields does: takes the name first, returns the value
+        $captured[ExportedAPI::FILTER_SMARTLING_METADATA_FIELD_PROCESS][10][] =
+            function (mixed $fieldName, mixed $fieldValue) use (&$namesSeenByFieldProcessor) {
+                $namesSeenByFieldProcessor[] = $fieldName;
+                // MetaFieldProcessorManager::getProcessor() accepts only string names
+                $this->assertIsString($fieldName);
+
+                return $fieldValue;
+            };
+
+        $arrayValue = [160 => '2026-10-23 19:39:17'];
+        $result = $this->applyFilters(
+            $captured[ExportedAPI::FILTER_SMARTLING_METADATA_FIELD_PROCESS],
+            '_rvy_updated_by',
+            $arrayValue,
+            $this->submission(20, 0),
+        );
+
+        $this->assertSame(['_rvy_updated_by'], $namesSeenByFieldProcessor);
+        $this->assertSame($arrayValue, $result);
+    }
+
+    public function testTargetFieldsAreSanitizedByDedicatedFilterWithValueAsFirstArgument(): void
+    {
+        $wp = $this->supportedWordpress();
+        $wp->method('get_post')->willReturn((object)['post_mime_type' => 'draft-revision']);
+        $wp->method('getPostMeta')->willReturn('');
+        $captured = $this->registerAndCaptureFilters($this->revisionsWithSupportedPlugin($wp), $wp);
+
+        $callbacks = $captured[ExportedAPI::FILTER_SMARTLING_TARGET_ENTITY_FIELD] ?? [];
+        $this->assertNotEmpty($callbacks, 'sanitizing of target fields must be registered on its own filter');
+        $this->assertSame('', $this->applyFilters($callbacks, 'draft-revision', 'post_mime_type', $this->submission(20, 0)));
+        $this->assertSame(0, $this->applyFilters($callbacks, 10, 'post_parent', $this->submission(20, 0)));
+        $this->assertSame('title', $this->applyFilters($callbacks, 'title', 'post_title', $this->submission(20, 0)));
     }
 }

tests/Smartling/Jobs/UploadJobTest.php

@@ -19,6 +19,7 @@
 use Smartling\Submissions\SubmissionEntity;
 use Smartling\Submissions\SubmissionManager;
 use Smartling\Tests\Mocks\WordpressFunctionsMockHelper;
+use Smartling\Vendor\Psr\Log\LoggerInterface;
 
 class UploadJobTest extends TestCase
 {
@@ -85,6 +86,45 @@ public function testUploadThrowableFromHookStillRemovesItemButRecordsError()
         })->run('');
     }
 
+    /**
+     * An exception thrown while sending content (for example a TypeError from a field processor) fails the
+     * submission, and the cause must reach the log at error level with the exception class and location, not only
+     * as a notice, so it is not mistaken for a routine skip.
+     */
+    public function testUploadThrowableIsLoggedAsErrorWithExceptionClassAndLocation()
+    {
+        $submission = $this->createMock(SubmissionEntity::class);
+        $submission->method('getId')->willReturn(1);
+        $submission->method('getFileUri')->willReturn('file.xml');
+        $submission->method('getSourceBlogId')->willReturn(1);
+        $item = $this->buildItem($submission);
+
+        $job = $this->buildJob($this->buildQueueManager($item), null, static function () {
+            throw new \TypeError('array given');
+        });
+
+        $logger = $this->createMock(LoggerInterface::class);
+        $errors = [];
+        $notices = [];
+        $logger->method('error')->willReturnCallback(function ($message) use (&$errors) {
+            $errors[] = (string)$message;
+        });
+        $logger->method('notice')->willReturnCallback(function ($message) use (&$notices) {
+            $notices[] = (string)$message;
+        });
+        $property = new \ReflectionProperty(\Smartling\Jobs\JobAbstract::class, 'logger');
+        $property->setValue($job, $logger);
+
+        $job->run('');
+
+        $this->assertCount(1, $errors);
+        $this->assertSame([], $notices, 'the failure must be logged once, not as both an error and a notice');
+        $this->assertStringContainsString('submissionId=1', $errors[0]);
+        $this->assertStringContainsString('TypeError', $errors[0]);
+        $this->assertStringContainsString('array given', $errors[0]);
+        $this->assertStringContainsString('UploadJobTest.php', $errors[0]);
+    }
+
     /**
      * A row is claimed (and its attempt counter incremented) by dequeue() before this
      * check runs. If the row isn't completed here, it stays claimed until the stale