From d19e3fcaecd38cfc470dd9c441efc0abed99b0a1 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Thu, 1 Oct 2026 20:08:39 +0200 Subject: [PATCH 01/11] Store configuration profile on submission and use it for delivery (WP-1021) Record the profile used at translation request time on the submission and resolve it from there for upload, download and FTS, so switching the active profile no longer breaks automated delivery. Submissions without a stored profile fall back to the active profile of the source blog. Co-Authored-By: Claude Sonnet 5.5 --- inc/Smartling/ApiWrapper.php | 2 +- .../Base/SmartlingCoreAttachments.php | 2 +- .../Base/SmartlingCoreDownloadTrait.php | 8 +- .../Base/SmartlingCoreUploadTrait.php | 6 +- .../DbAl/Migrations/Migration261001.php | 41 +++++ inc/Smartling/DbAl/UploadQueueManager.php | 2 +- inc/Smartling/FTS/FtsApiWrapper.php | 2 +- inc/Smartling/FTS/FtsService.php | 6 +- inc/Smartling/Helpers/DetectChangesHelper.php | 2 +- inc/Smartling/Helpers/FieldsFilterHelper.php | 10 +- .../SubstringProcessorHelperAbstract.php | 2 +- inc/Smartling/Helpers/XmlHelper.php | 2 +- inc/Smartling/Jobs/LastModifiedCheckJob.php | 6 +- inc/Smartling/Jobs/UploadJob.php | 7 +- inc/Smartling/Settings/SettingsManager.php | 20 +++ .../Submissions/SubmissionEntity.php | 16 ++ .../Submissions/SubmissionManager.php | 18 +- .../PostBasedWidgetControllerStd.php | 2 +- .../WP/Table/SubmissionTableWidget.php | 2 +- inc/config/migrations.yml | 4 + inc/config/services.yml | 1 + .../includes/bootstrap-local.php | 43 +++++ .../includes/local-wp-tests-config.php | 40 +++++ .../tests/FtsIntegrationTest.php | 167 ++++++++++++++++++ .../ContentRelationsDiscoveryServiceTest.php | 1 + tests/Smartling/Base/SmartlingCoreTest.php | 2 +- .../Base/SmartlingCoreUploadTraitTest.php | 6 +- tests/Smartling/FTS/FtsApiWrapperTest.php | 8 +- tests/Smartling/FTS/FtsServiceTest.php | 4 +- tests/Smartling/Jobs/UploadJobTest.php | 4 +- .../Settings/SettingsManagerTest.php | 46 +++++ .../Submissions/SubmissionManagerTest.php | 66 ++++++- tests/Traits/SubmissionManagerMock.php | 4 +- tests/playwright/bulk-submit.spec.js | 103 +++++++++++ tests/setup-local-test-db.sh | 147 +++++++++++++++ 35 files changed, 756 insertions(+), 46 deletions(-) create mode 100644 inc/Smartling/DbAl/Migrations/Migration261001.php create mode 100644 tests/IntegrationTests/includes/bootstrap-local.php create mode 100644 tests/IntegrationTests/includes/local-wp-tests-config.php create mode 100644 tests/IntegrationTests/tests/FtsIntegrationTest.php create mode 100644 tests/playwright/bulk-submit.spec.js create mode 100755 tests/setup-local-test-db.sh diff --git a/inc/Smartling/ApiWrapper.php b/inc/Smartling/ApiWrapper.php index 2fd6ba5cb..0d09b2b7f 100644 --- a/inc/Smartling/ApiWrapper.php +++ b/inc/Smartling/ApiWrapper.php @@ -68,7 +68,7 @@ public function __construct(SettingsManager $manager, string $pluginName, string */ private function getConfigurationProfile(SubmissionEntity $submission): ConfigurationProfileEntity { - $profile = $this->settings->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settings->getProfileBySubmission($submission); LogContextMixinHelper::addToContext('projectId', $profile->getProjectId()); if (TestRunHelper::isTestRunBlog($submission->getTargetBlogId())) { diff --git a/inc/Smartling/Base/SmartlingCoreAttachments.php b/inc/Smartling/Base/SmartlingCoreAttachments.php index eeac14bd8..812aa399f 100644 --- a/inc/Smartling/Base/SmartlingCoreAttachments.php +++ b/inc/Smartling/Base/SmartlingCoreAttachments.php @@ -35,7 +35,7 @@ public function syncAttachment(SubmissionEntity $submission): void $submission->getTargetId(), ]) ); - $profile = $this->getSettingsManager()->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->getSettingsManager()->getProfileBySubmission($submission); if (1 === $profile->getAlwaysSyncImagesOnUpload() || ($submission->getStatus() === SubmissionEntity::SUBMISSION_STATUS_NEW && !$targetFileExists)) { $this->syncMediaFile($submission); } diff --git a/inc/Smartling/Base/SmartlingCoreDownloadTrait.php b/inc/Smartling/Base/SmartlingCoreDownloadTrait.php index 69a27d5a4..bf0e2ec36 100644 --- a/inc/Smartling/Base/SmartlingCoreDownloadTrait.php +++ b/inc/Smartling/Base/SmartlingCoreDownloadTrait.php @@ -50,7 +50,7 @@ public function downloadTranslationBySubmission(SubmissionEntity $entity): void LiveNotificationController::pushNotification( $this ->getSettingsManager() - ->getSingleSettingsProfile($entity->getSourceBlogId()) + ->getProfileBySubmission($entity) ->getProjectId(), LiveNotificationController::getContentId($entity), LiveNotificationController::SEVERITY_SUCCESS, @@ -65,7 +65,7 @@ public function downloadTranslationBySubmission(SubmissionEntity $entity): void LiveNotificationController::pushNotification( $this ->getSettingsManager() - ->getSingleSettingsProfile($entity->getSourceBlogId()) + ->getProfileBySubmission($entity) ->getProjectId(), LiveNotificationController::getContentId($entity), LiveNotificationController::SEVERITY_SUCCESS, @@ -78,7 +78,7 @@ public function downloadTranslationBySubmission(SubmissionEntity $entity): void LiveNotificationController::pushNotification( $this ->getSettingsManager() - ->getSingleSettingsProfile($entity->getSourceBlogId()) + ->getProfileBySubmission($entity) ->getProjectId(), LiveNotificationController::getContentId($entity), LiveNotificationController::SEVERITY_SUCCESS, @@ -100,7 +100,7 @@ public function downloadTranslationBySubmission(SubmissionEntity $entity): void LiveNotificationController::pushNotification( $this ->getSettingsManager() - ->getSingleSettingsProfile($entity->getSourceBlogId()) + ->getProfileBySubmission($entity) ->getProjectId(), LiveNotificationController::getContentId($entity), LiveNotificationController::SEVERITY_ERROR, diff --git a/inc/Smartling/Base/SmartlingCoreUploadTrait.php b/inc/Smartling/Base/SmartlingCoreUploadTrait.php index 832a7bea3..8b3936409 100644 --- a/inc/Smartling/Base/SmartlingCoreUploadTrait.php +++ b/inc/Smartling/Base/SmartlingCoreUploadTrait.php @@ -259,7 +259,7 @@ public function applyXML(SubmissionEntity $submission, string $xml, XmlHelper $x $targetContent = $targetContent->fromArray($translation['entity']); } $configurationProfile = $this->getSettingsManager() - ->getSingleSettingsProfile($submission->getSourceBlogId()); + ->getProfileBySubmission($submission); $percentage = $submission->getCompletionPercentage(); $this->getLogger()->debug(vsprintf('Current percentage is %s', [$percentage])); @@ -424,7 +424,7 @@ public function bulkSubmit(UploadQueueItem $item): void } $submission = $item->getSubmissions()[0]; $locales = $item->getSmartlingLocales()->getList(); - $profile = $this->getSettingsManager()->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->getSettingsManager()->getProfileBySubmission($submission); try { $xml = $this->getXMLFiltered($submission); if ($xml === '') { @@ -530,7 +530,7 @@ public function sendForTranslation(UploadQueueItem $item): void return; } - $configurationProfile = $this->getSettingsManager()->getSingleSettingsProfile($item->getSubmissions()[0]->getSourceBlogId()); + $configurationProfile = $this->getSettingsManager()->getProfileBySubmission($item->getSubmissions()[0]); // Clone attachment submission instead of uploading it, if "Clone attachment" // option is enabled in configuration profile. diff --git a/inc/Smartling/DbAl/Migrations/Migration261001.php b/inc/Smartling/DbAl/Migrations/Migration261001.php new file mode 100644 index 000000000..8a56315cd --- /dev/null +++ b/inc/Smartling/DbAl/Migrations/Migration261001.php @@ -0,0 +1,41 @@ +completeTableName(SubmissionEntity::getTableName()); + + // Migration240315 may already have created the table with the current field definitions. + $existingColumns = $db->getColumnArray("SHOW COLUMNS FROM `$tableName`"); + if (in_array(SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID, $existingColumns, true)) { + return []; + } + + return [ + sprintf( + 'ALTER TABLE `%s` ADD COLUMN `%s` %s', + $tableName, + SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID, + SubmissionEntity::getFieldDefinitions()[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] + ), + ]; + } +} diff --git a/inc/Smartling/DbAl/UploadQueueManager.php b/inc/Smartling/DbAl/UploadQueueManager.php index ad5a0ac16..69bf5f8c5 100644 --- a/inc/Smartling/DbAl/UploadQueueManager.php +++ b/inc/Smartling/DbAl/UploadQueueManager.php @@ -272,7 +272,7 @@ public function purge(): void } if (!array_key_exists($submission->getSourceBlogId(), $profiles)) { try { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); } catch (SmartlingDbException) { $profile = null; } diff --git a/inc/Smartling/FTS/FtsApiWrapper.php b/inc/Smartling/FTS/FtsApiWrapper.php index 90ee934e9..1fe1c74d6 100644 --- a/inc/Smartling/FTS/FtsApiWrapper.php +++ b/inc/Smartling/FTS/FtsApiWrapper.php @@ -32,7 +32,7 @@ public function __construct( */ private function getConfigurationProfile(SubmissionEntity $submission): ConfigurationProfileEntity { - return $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + return $this->settingsManager->getProfileBySubmission($submission); } private function getFileTranslationsApi(ConfigurationProfileEntity $profile): FileTranslationsApiExtended diff --git a/inc/Smartling/FTS/FtsService.php b/inc/Smartling/FTS/FtsService.php index 5b6f9b2de..3df711de7 100644 --- a/inc/Smartling/FTS/FtsService.php +++ b/inc/Smartling/FTS/FtsService.php @@ -114,7 +114,7 @@ public function requestInstantTranslationBatch(array $submissions): array try { $fileUid = $this->uploadFile($firstSubmission); - $profile = $this->settingsManager->getSingleSettingsProfile($firstSubmission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($firstSubmission); $sourceLocale = $this->apiWrapper->getSourceLocale($profile); $targetLocales = []; @@ -304,7 +304,7 @@ private function submitFile(SubmissionEntity $submission, string $fileUid): stri { $this->getLogger()->debug("Submitting file for instant translation, submissionId={$submission->getId()}, fileUid=$fileUid"); - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); $sourceLocale = $this->apiWrapper->getSourceLocale($profile); $targetLocale = $profile->getSmartlingLocale($submission->getTargetBlogId()); @@ -398,7 +398,7 @@ private function downloadAndApply(SubmissionEntity $submission, string $fileUid, { $this->getLogger()->info("Downloading and applying translation, submissionId={$submission->getId()}, fileUid=$fileUid, mtUid=$mtUid"); - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); $targetLocale = $profile->getSmartlingLocale($submission->getTargetBlogId()); if (empty($targetLocale)) { diff --git a/inc/Smartling/Helpers/DetectChangesHelper.php b/inc/Smartling/Helpers/DetectChangesHelper.php index c13644a1e..5f374e94e 100644 --- a/inc/Smartling/Helpers/DetectChangesHelper.php +++ b/inc/Smartling/Helpers/DetectChangesHelper.php @@ -96,7 +96,7 @@ private function update(SubmissionEntity $submission, bool $needUpdateStatus, st LiveNotificationController::pushNotification( $this->settingsManager - ->getSingleSettingsProfile($submission->getSourceBlogId()) + ->getProfileBySubmission($submission) ->getProjectId(), LiveNotificationController::getContentId($submission), LiveNotificationController::SEVERITY_WARNING, diff --git a/inc/Smartling/Helpers/FieldsFilterHelper.php b/inc/Smartling/Helpers/FieldsFilterHelper.php index ac8162bf6..360ff13a6 100644 --- a/inc/Smartling/Helpers/FieldsFilterHelper.php +++ b/inc/Smartling/Helpers/FieldsFilterHelper.php @@ -111,7 +111,7 @@ public function removeIgnoringFields(SubmissionEntity $submission, array $data): $this->prepareSourceData($data) ), $this->contentSerializationHelper->prepareFieldProcessorValues($submission)['ignore'], - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp()), + $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp()), ); } @@ -135,11 +135,11 @@ public function processStringsBeforeEncoding( $this->removeFields( $this->flattenArray($data), $settings['ignore'], - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp(), + $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), ) ), $strategy, - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp(), + $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), $settings, ); } @@ -180,11 +180,11 @@ private function filterArray(array $array, SubmissionEntity $submission, string $this->removeFields( $array, $settings['ignore'], - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp(), + $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), ), ), $strategy, - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp(), + $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), $this->contentSerializationHelper->prepareFieldProcessorValues($submission), ); } diff --git a/inc/Smartling/Helpers/SubstringProcessorHelperAbstract.php b/inc/Smartling/Helpers/SubstringProcessorHelperAbstract.php index fd5903a0f..baada768e 100644 --- a/inc/Smartling/Helpers/SubstringProcessorHelperAbstract.php +++ b/inc/Smartling/Helpers/SubstringProcessorHelperAbstract.php @@ -256,7 +256,7 @@ private function passProfileFilters(array $attributes) $fFilter = $this->getFieldsFilter(); $settings = $this->contentSerializationHelper->prepareFieldProcessorValues($submission); - $removeAsRegExp = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId())->getFilterFieldNameRegExp(); + $removeAsRegExp = $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(); $attributes = $fFilter->removeFields($attributes, $settings['ignore'], $removeAsRegExp); $attributes = $fFilter->removeFields($attributes, $settings['copy']['name'], $removeAsRegExp); diff --git a/inc/Smartling/Helpers/XmlHelper.php b/inc/Smartling/Helpers/XmlHelper.php index 108856d7b..40fa157f7 100644 --- a/inc/Smartling/Helpers/XmlHelper.php +++ b/inc/Smartling/Helpers/XmlHelper.php @@ -90,7 +90,7 @@ public function xmlEncode(array $source, SubmissionEntity $submission, array $or { $this->getLogger()->debug(sprintf('Started creating XML for fields: %s', base64_encode(var_export($source, true)))); try { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); } catch (SmartlingDbException) { $profile = null; } diff --git a/inc/Smartling/Jobs/LastModifiedCheckJob.php b/inc/Smartling/Jobs/LastModifiedCheckJob.php index 5a9902e95..5989c16d6 100644 --- a/inc/Smartling/Jobs/LastModifiedCheckJob.php +++ b/inc/Smartling/Jobs/LastModifiedCheckJob.php @@ -194,7 +194,7 @@ private function lastModifiedCheck(string $queueName, bool $failMissing): void protected function processDownloadOnChange(array $submissions): void { foreach ($submissions as $submission) { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); if (ConfigurationProfileEntity::TRANSLATION_DOWNLOAD_MODE_PROGRESS_CHANGES === $profile->getDownloadOnChange()) { $this->getLogger() @@ -238,7 +238,7 @@ public function statusCheck(array $submissions): void $submissions = $this->submissionManager->storeSubmissions($statusCheckResult); foreach ($submissions as $submission) { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); if ($profile->getDownloadOnChange() !== ConfigurationProfileEntity::TRANSLATION_DOWNLOAD_MODE_MANUAL) { $this->checkEntityForDownload($submission); } @@ -273,7 +273,7 @@ public function getSmartlingLocaleIdBySubmission(SubmissionEntity $submission): { return $this->settingsManager ->getSmartlingLocaleIdBySettingsProfile( - $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()), + $this->settingsManager->getProfileBySubmission($submission), $submission->getTargetBlogId() ); } diff --git a/inc/Smartling/Jobs/UploadJob.php b/inc/Smartling/Jobs/UploadJob.php index 5a1a93398..570aa9321 100644 --- a/inc/Smartling/Jobs/UploadJob.php +++ b/inc/Smartling/Jobs/UploadJob.php @@ -77,16 +77,17 @@ private function processUploadQueue(int $blogId): void $submission->setFileUri($this->fileUriHelper->generateFileUri($submission)); $this->submissionManager->storeEntity($submission); } - if (!array_key_exists($submission->getSourceBlogId(), $profiles)) { + $profileKey = $submission->getConfigurationProfileId() ?? "blog{$submission->getSourceBlogId()}"; + if (!array_key_exists($profileKey, $profiles)) { try { - $profiles[$submission->getSourceBlogId()] = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profiles[$profileKey] = $this->settingsManager->getProfileBySubmission($submission); } catch (SmartlingDbException) { $this->failItem($item, 'Skipping upload of', "No active profile found for blogId={$submission->getSourceBlogId()}"); $this->uploadQueueManager->complete($item); continue; } } - $profile = $profiles[$submission->getSourceBlogId()]; + $profile = $profiles[$profileKey]; if ($item->getBatchUid() === '') { try { $item = $item->setBatchUid($this->api->getOrCreateJobInfoForDailyBucketJob($profile, [$submission->getFileUri()])->getBatchUid()); diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index beb5cd854..47a0e7315 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -119,6 +119,26 @@ public function getSingleSettingsProfile(int $mainBlogId): ConfigurationProfileE throw new SmartlingDbException($message); } + /** + * Returns the profile the submission was requested with, so delivery doesn't depend on which profile is active now. + * Falls back to the active profile of the source blog for submissions without a stored (or an existing) profile. + * + * @throws SmartlingDbException + */ + public function getProfileBySubmission(SubmissionEntity $submission): ConfigurationProfileEntity + { + $profileId = $submission->getConfigurationProfileId(); + if ($profileId !== null) { + $profile = ArrayHelper::first($this->getEntityById($profileId)); + if ($profile instanceof ConfigurationProfileEntity) { + return $profile; + } + $this->getLogger()->warning("Profile id=$profileId stored for submission id={$submission->getId()} not found, using active profile of source blog"); + } + + return $this->getSingleSettingsProfile($submission->getSourceBlogId()); + } + /** * @return int[] * @throws SmartlingDbException diff --git a/inc/Smartling/Submissions/SubmissionEntity.php b/inc/Smartling/Submissions/SubmissionEntity.php index 766d63cc6..dcc6ce5a6 100644 --- a/inc/Smartling/Submissions/SubmissionEntity.php +++ b/inc/Smartling/Submissions/SubmissionEntity.php @@ -83,6 +83,7 @@ class SubmissionEntity extends SmartlingEntityAbstract implements Submission public const FIELD_LAST_ERROR = 'last_error'; public const FIELD_LOCKED_FIELDS = 'locked_fields'; public const FIELD_CREATED_AT = 'created_at'; + public const FIELD_CONFIGURATION_PROFILE_ID = 'configuration_profile_id'; public const VIRTUAL_FIELD_JOB_LINK = 'job_link'; @@ -117,6 +118,7 @@ public static function getFieldDefinitions(): array static::FIELD_LAST_ERROR => static::DB_TYPE_STRING_TEXT, static::FIELD_LOCKED_FIELDS => 'TEXT NULL', static::FIELD_CREATED_AT => static::DB_TYPE_DATETIME, + static::FIELD_CONFIGURATION_PROFILE_ID => 'INT(20) UNSIGNED NULL', ]; } @@ -515,6 +517,20 @@ public function setSubmitter(string $submitter): SubmissionEntity return $this; } + public function getConfigurationProfileId(): ?int + { + $value = $this->stateFields[static::FIELD_CONFIGURATION_PROFILE_ID]; + + return $value === null ? null : (int)$value; + } + + public function setConfigurationProfileId(?int $configurationProfileId): self + { + $this->stateFields[static::FIELD_CONFIGURATION_PROFILE_ID] = $configurationProfileId; + + return $this; + } + public function getCreatedAt(): ?string { return $this->stateFields[static::FIELD_CREATED_AT]; diff --git a/inc/Smartling/Submissions/SubmissionManager.php b/inc/Smartling/Submissions/SubmissionManager.php index f29278252..c8bdf4185 100644 --- a/inc/Smartling/Submissions/SubmissionManager.php +++ b/inc/Smartling/Submissions/SubmissionManager.php @@ -5,6 +5,7 @@ use Smartling\DbAl\EntityManagerAbstract; use Smartling\DbAl\LocalizationPluginProxyInterface; use Smartling\DbAl\SmartlingToCMSDatabaseAccessWrapperInterface; +use Smartling\Exception\SmartlingDbException; use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\DateTimeHelper; @@ -20,6 +21,7 @@ use Smartling\Jobs\SubmissionJobEntity; use Smartling\Jobs\SubmissionsJobsManager; use Smartling\Models\DuplicateSubmissionDetails; +use Smartling\Settings\SettingsManager; class SubmissionManager extends EntityManagerAbstract { @@ -39,7 +41,7 @@ public function getDefaultSubmissionStatus(): string return SubmissionEntity::SUBMISSION_STATUS_IN_PROGRESS; } - public function __construct(SmartlingToCMSDatabaseAccessWrapperInterface $dbal, int $pageSize, JobManager $jobManager, LocalizationPluginProxyInterface $localizationPluginProxy, SiteHelper $siteHelper, SubmissionsJobsManager $submissionsJobsManager) + public function __construct(SmartlingToCMSDatabaseAccessWrapperInterface $dbal, int $pageSize, JobManager $jobManager, LocalizationPluginProxyInterface $localizationPluginProxy, SiteHelper $siteHelper, SubmissionsJobsManager $submissionsJobsManager, private SettingsManager $settingsManager) { parent::__construct($dbal, $pageSize, $siteHelper, $localizationPluginProxy); $this->jobManager = $jobManager; @@ -484,10 +486,24 @@ public function getSubmissionEntity( $entity->setSourceTitle('no title'); $entity->setCreatedAt(DateTimeHelper::nowAsString()); } + $this->stampConfigurationProfile($entity); return $entity; } + /** + * Remembers the profile used for this translation request, so that delivery uses the same profile + * even if the active profile has been switched in the meantime. + */ + private function stampConfigurationProfile(SubmissionEntity $entity): void + { + try { + $entity->setConfigurationProfileId($this->settingsManager->getSingleSettingsProfile($entity->getSourceBlogId())->getId()); + } catch (SmartlingDbException) { + $this->getLogger()->debug("No active profile for source blog {$entity->getSourceBlogId()}, configuration profile not stored for submission"); + } + } + /** * @param SubmissionEntity[] $submissions * diff --git a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php index 438961d3d..6a6731f36 100644 --- a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php +++ b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php @@ -138,7 +138,7 @@ public function ajaxDownloadHandler(): void if ($submission !== null) { $submissions[] = $submission; if ($profile === null) { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); } $logSubmissions[] = [ 'sourceBlogId' => $submission->getSourceBlogId(), diff --git a/inc/Smartling/WP/Table/SubmissionTableWidget.php b/inc/Smartling/WP/Table/SubmissionTableWidget.php index a8175b511..fe3d36b9a 100644 --- a/inc/Smartling/WP/Table/SubmissionTableWidget.php +++ b/inc/Smartling/WP/Table/SubmissionTableWidget.php @@ -216,7 +216,7 @@ public function processBulkAction(): void $profile = null; foreach ($submissions as $submission) { if ($profile === null) { - $profile = $this->settingsManager->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->settingsManager->getProfileBySubmission($submission); } $logSubmissions[] = [ 'sourceBlogId' => $submission->getSourceBlogId(), diff --git a/inc/config/migrations.yml b/inc/config/migrations.yml index a536f8102..00c7420cf 100644 --- a/inc/config/migrations.yml +++ b/inc/config/migrations.yml @@ -73,6 +73,9 @@ services: migration.260825: class: Smartling\DbAl\Migrations\Migration260825 + migration.261001: + class: Smartling\DbAl\Migrations\Migration261001 + manager.db.migrations: class: Smartling\DbAl\Migrations\DbMigrationManager calls: @@ -100,3 +103,4 @@ services: - [ "registerMigration", [ "@migration.220701" ]] - [ "registerMigration", [ "@migration.240315" ]] - [ "registerMigration", [ "@migration.260825" ]] + - [ "registerMigration", [ "@migration.261001" ]] diff --git a/inc/config/services.yml b/inc/config/services.yml index 3d51a7123..9aa4ab025 100644 --- a/inc/config/services.yml +++ b/inc/config/services.yml @@ -300,6 +300,7 @@ services: - "@multilang.proxy" - "@site.helper" - "@manager.submissions.jobs" + - "@manager.settings" manager.submissions.jobs: class: Smartling\Jobs\SubmissionsJobsManager diff --git a/tests/IntegrationTests/includes/bootstrap-local.php b/tests/IntegrationTests/includes/bootstrap-local.php new file mode 100644 index 000000000..52c66612b --- /dev/null +++ b/tests/IntegrationTests/includes/bootstrap-local.php @@ -0,0 +1,43 @@ +ftsService = $this->get('fts.service'); + } + + /** + * Full FTS workflow: upload, translate, poll, download, apply. + * Verifies that a post is translated and the translated content appears in the target blog. + */ + public function testFullFtsWorkflow(): void + { + $sourceContent = 'Hello world. This is a test post for instant translation.'; + $postId = $this->createPost('post', 'FTS Integration Test Post', $sourceContent); + $this->assertGreaterThan(0, $postId, 'Post creation failed'); + + $submission = $this->createSubmission('post', $postId, 1, 2); + $submission = $this->getSubmissionManager()->storeEntity($submission); + $this->assertNotNull($submission->getId(), 'Submission store failed'); + + // Initiate non-blocking FTS request + $result = $this->ftsService->requestInstantTranslationBatch([$submission]); + $this->assertTrue($result['success'], 'FTS batch request failed: ' . ($result['message'] ?? 'unknown error')); + $this->assertArrayHasKey('fileUid', $result); + $this->assertArrayHasKey('mtUid', $result); + $this->assertNotEmpty($result['fileUid']); + $this->assertNotEmpty($result['mtUid']); + + // Re-fetch submission to verify fileUid:mtUid was stored + $submission = $this->getSubmissionById($submission->getId()); + $this->assertNotNull($submission, 'Could not re-fetch submission'); + $this->assertNotEmpty($submission->getFileUri(), 'fileUid:mtUid was not stored in submission.file_uri'); + $this->assertStringContainsString(':', $submission->getFileUri(), 'file_uri should be in fileUid:mtUid format'); + + // Poll until completed or timeout + $finalStatus = $this->pollUntilDone($submission); + + $this->assertEquals('completed', $finalStatus['status'], + 'FTS translation did not complete within ' . self::POLL_TIMEOUT_SECONDS . ' seconds. ' . + 'Last status: ' . ($finalStatus['status'] ?? 'unknown') . '. ' . + 'Message: ' . ($finalStatus['message'] ?? '') + ); + + // Verify submission was marked as completed in the database + $completedSubmission = $this->getSubmissionById($submission->getId()); + $this->assertNotNull($completedSubmission); + $this->assertEquals( + SubmissionEntity::SUBMISSION_STATUS_COMPLETED, + $completedSubmission->getStatus(), + 'Submission status was not updated to COMPLETED' + ); + $this->assertGreaterThan(0, $completedSubmission->getTargetId(), + 'Target post was not created in the target blog' + ); + + // Verify the translated content exists in the target blog + $targetPost = $this->getTargetPost($this->getSiteHelper(), $completedSubmission); + $this->assertNotNull($targetPost, 'Target post not found in target blog'); + $this->assertNotEmpty($targetPost->post_content, 'Translated post content is empty'); + } + + /** + * Verifies that requestInstantTranslationBatch enforces the same-source constraint. + */ + public function testBatchRejectsSubmissionsFromDifferentSources(): void + { + $postId1 = $this->createPost('post', 'Source Post 1', 'Content 1'); + $postId2 = $this->createPost('post', 'Source Post 2', 'Content 2'); + + $submission1 = $this->getSubmissionManager()->storeEntity( + $this->createSubmission('post', $postId1, 1, 2) + ); + $submission2 = $this->getSubmissionManager()->storeEntity( + $this->createSubmission('post', $postId2, 1, 2) + ); + + // Two different source posts: should be rejected by the batch method + $result = $this->ftsService->requestInstantTranslationBatch([$submission1, $submission2]); + + $this->assertFalse($result['success']); + $this->assertStringContainsString('Same source', $result['message']); + } + + /** + * Verifies that checkAndApplyTranslation returns an error for a submission + * that has no fileUid:mtUid stored. + */ + public function testCheckStatusFailsWithoutFileUri(): void + { + $postId = $this->createPost('post', 'Post Without FTS', 'Some content'); + $submission = $this->getSubmissionManager()->storeEntity( + $this->createSubmission('post', $postId, 1, 2) + ); + + // file_uri is empty at this point (FTS not requested) + $result = $this->ftsService->checkAndApplyTranslation($submission); + + $this->assertEquals('error', $result['status']); + $this->assertArrayHasKey('message', $result); + } + + /** + * Polls FTS status until completed/failed/error or timeout. + */ + private function pollUntilDone(SubmissionEntity $submission): array + { + $start = time(); + $lastResult = ['status' => 'unknown']; + + $this->getLogger()->info(sprintf( + 'FtsIntegrationTest: Starting poll for submission %d (fileUri=%s)', + $submission->getId(), + $submission->getFileUri() + )); + + while ((time() - $start) < self::POLL_TIMEOUT_SECONDS) { + $result = $this->ftsService->checkAndApplyTranslation($submission); + $lastResult = $result; + + $this->getLogger()->info(sprintf( + 'FtsIntegrationTest: Poll result for submission %d: status=%s', + $submission->getId(), + $result['status'] + )); + + if (in_array($result['status'], ['completed', 'failed', 'error'], true)) { + return $result; + } + + sleep(self::POLL_SLEEP_SECONDS); + } + + $this->getLogger()->warning(sprintf( + 'FtsIntegrationTest: Polling timed out after %d seconds for submission %d', + self::POLL_TIMEOUT_SECONDS, + $submission->getId() + )); + + return array_merge($lastResult, ['status' => 'timeout']); + } +} diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index a480947fc..eb3767ac0 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -466,6 +466,7 @@ public function testJobInfoGetsStoredOnNewSubmissions() $this->createMock(LocalizationPluginProxyInterface::class), $this->createMock(SiteHelper::class), $submissionsJobsManager, + $this->createMock(SettingsManager::class), ])->onlyMethods(['find'])->getMock(); $submissionManager->method('find')->willReturn([]); diff --git a/tests/Smartling/Base/SmartlingCoreTest.php b/tests/Smartling/Base/SmartlingCoreTest.php index d71948331..4050f1a89 100644 --- a/tests/Smartling/Base/SmartlingCoreTest.php +++ b/tests/Smartling/Base/SmartlingCoreTest.php @@ -474,7 +474,7 @@ private function buildCoreForSendForTranslation( ?SubmissionManager $submissionManager = null, ): SmartlingCore|\PHPUnit\Framework\MockObject\MockObject { $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('getProfileBySubmission')->willReturn($profile); $submissionManager ??= $this->createMock(SubmissionManager::class); diff --git a/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php b/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php index f0ad36b4c..5b5e88446 100644 --- a/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php +++ b/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php @@ -105,7 +105,7 @@ public function testApplyXmlNoCleanMetadata() $fieldsFilterHelper->method('applyTranslatedValues')->willReturnArgument(2); $settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); - $settingsManager->method('getSingleSettingsProfile')->willReturn($this->createMock(ConfigurationProfileEntity::class)); + $settingsManager->method('getProfileBySubmission')->willReturn($this->createMock(ConfigurationProfileEntity::class)); $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); $submissionManager->method('storeEntity')->willReturnArgument(0); @@ -143,7 +143,7 @@ public function testApplyXmlCleanMetadata() $profile->method('getFilterSkipArray')->willReturn(['excluded']); $settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('getProfileBySubmission')->willReturn($profile); $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); $submissionManager->method('storeEntity')->willReturnArgument(0); @@ -297,7 +297,7 @@ public function testApplyXmlLockedBlocksById() $profile = $this->getMockBuilder(ConfigurationProfileEntity::class)->disableOriginalConstructor()->getMock(); $settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('getProfileBySubmission')->willReturn($profile); $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); $submissionManager->method('storeEntity')->willReturnArgument(0); diff --git a/tests/Smartling/FTS/FtsApiWrapperTest.php b/tests/Smartling/FTS/FtsApiWrapperTest.php index 4ceaf494b..e4c478035 100644 --- a/tests/Smartling/FTS/FtsApiWrapperTest.php +++ b/tests/Smartling/FTS/FtsApiWrapperTest.php @@ -35,7 +35,7 @@ public function testUploadFileRequiresConfiguration(): void $submission->method('getSourceBlogId')->willReturn(1); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willThrowException(new SmartlingDbException('No profile found')); $this->ftsApiWrapper->uploadFile( @@ -53,7 +53,7 @@ public function testSubmitForInstantTranslationRequiresConfiguration(): void $submission->method('getSourceBlogId')->willReturn(1); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willThrowException(new SmartlingDbException('No profile found')); $this->ftsApiWrapper->submitForInstantTranslation( @@ -72,7 +72,7 @@ public function testPollTranslationStatusRequiresConfiguration(): void $submission->method('getSourceBlogId')->willReturn(1); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willThrowException(new SmartlingDbException('No profile found')); $this->ftsApiWrapper->pollTranslationStatus( @@ -90,7 +90,7 @@ public function testDownloadTranslatedFileRequiresConfiguration(): void $submission->method('getSourceBlogId')->willReturn(1); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willThrowException(new SmartlingDbException('No profile found')); $this->ftsApiWrapper->downloadTranslatedFile( diff --git a/tests/Smartling/FTS/FtsServiceTest.php b/tests/Smartling/FTS/FtsServiceTest.php index 50493c391..8db1a4568 100644 --- a/tests/Smartling/FTS/FtsServiceTest.php +++ b/tests/Smartling/FTS/FtsServiceTest.php @@ -173,7 +173,7 @@ public function testCheckAndApplyTranslationWithCompletedState(): void $profile->method('getSmartlingLocale')->willReturn('de-DE'); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willReturn($profile); $this->ftsApiWrapper @@ -329,7 +329,7 @@ public function testCheckAndApplyTranslationWithDownloadException(): void $profile->method('getSmartlingLocale')->willReturn('de-DE'); $this->settingsManager - ->method('getSingleSettingsProfile') + ->method('getProfileBySubmission') ->willReturn($profile); $this->ftsApiWrapper diff --git a/tests/Smartling/Jobs/UploadJobTest.php b/tests/Smartling/Jobs/UploadJobTest.php index 86ffbbaaf..273e57a18 100644 --- a/tests/Smartling/Jobs/UploadJobTest.php +++ b/tests/Smartling/Jobs/UploadJobTest.php @@ -255,9 +255,9 @@ private function buildJob( ): UploadJob { $settingsManager = $this->createMock(SettingsManager::class); if ($onGetSingleSettingsProfile !== null) { - $settingsManager->method('getSingleSettingsProfile')->willReturnCallback($onGetSingleSettingsProfile); + $settingsManager->method('getProfileBySubmission')->willReturnCallback($onGetSingleSettingsProfile); } else { - $settingsManager->method('getSingleSettingsProfile') + $settingsManager->method('getProfileBySubmission') ->willReturn($this->createMock(ConfigurationProfileEntity::class)); } $settingsManager->method('getActiveProfile') diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index 59a12be8c..14e6b3793 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -10,6 +10,7 @@ use Smartling\Settings\ConfigurationProfileEntity; use Smartling\Settings\SettingsManager; use Smartling\Settings\TargetLocale; +use Smartling\Submissions\SubmissionEntity; use Smartling\Tests\Traits\SettingsManagerMock; class SettingsManagerTest extends TestCase @@ -87,6 +88,51 @@ public function testGetProfileTargetBlogIdsByMainBlogIdWithConfigException() $mock->getProfileTargetBlogIdsByMainBlogId(5); } + private function profileWithId(int $id): ConfigurationProfileEntity + { + $profile = new ConfigurationProfileEntity(); + $profile->setId($id); + + return $profile; + } + + public function testGetProfileBySubmissionUsesStoredProfile() + { + $stored = $this->profileWithId(7); + $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById']); + $mock->expects(self::once())->method('getEntityById')->with(7)->willReturn([$stored]); + $mock->expects(self::never())->method('getSingleSettingsProfile'); + + $submission = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(7); + + self::assertSame($stored, $mock->getProfileBySubmission($submission)); + } + + public function testGetProfileBySubmissionFallsBackWithoutStoredProfile() + { + $active = $this->profileWithId(3); + $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById']); + $mock->expects(self::never())->method('getEntityById'); + $mock->expects(self::once())->method('getSingleSettingsProfile')->with(1)->willReturn($active); + + $submission = (new SubmissionEntity())->setSourceBlogId(1); + + self::assertSame($active, $mock->getProfileBySubmission($submission)); + } + + public function testGetProfileBySubmissionFallsBackWhenStoredProfileWasDeleted() + { + $active = $this->profileWithId(3); + $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById', 'getLogger']); + $mock->method('getLogger')->willReturn(new NullLogger()); + $mock->expects(self::once())->method('getEntityById')->with(7)->willReturn([]); + $mock->expects(self::once())->method('getSingleSettingsProfile')->with(1)->willReturn($active); + + $submission = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(7); + + self::assertSame($active, $mock->getProfileBySubmission($submission)); + } + public function testGetEntitiesQueries() { $db = $this->createMock(SmartlingToCMSDatabaseAccessWrapperInterface::class); diff --git a/tests/Smartling/Submissions/SubmissionManagerTest.php b/tests/Smartling/Submissions/SubmissionManagerTest.php index 21d61e496..325b9849c 100644 --- a/tests/Smartling/Submissions/SubmissionManagerTest.php +++ b/tests/Smartling/Submissions/SubmissionManagerTest.php @@ -14,6 +14,9 @@ use Smartling\Jobs\JobManager; use Smartling\Jobs\SubmissionsJobsManager; use Smartling\Submissions\SubmissionEntity; +use Smartling\Exception\SmartlingDbException; +use Smartling\Settings\ConfigurationProfileEntity; +use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionManager; class SubmissionManagerTest extends TestCase @@ -132,6 +135,64 @@ public function testStoreEntityQuery() $x->storeEntity($entity); } + private function profileWithId(int $id): ConfigurationProfileEntity + { + $profile = new ConfigurationProfileEntity(); + $profile->setId($id); + + return $profile; + } + + private function getManagerForProfileStamping(SettingsManager $settingsManager, array $found): SubmissionManager + { + $x = $this->getMockBuilder(SubmissionManager::class)->setConstructorArgs([ + $this->db, + 20, + $this->createMock(JobManager::class), + $this->createMock(LocalizationPluginProxyInterface::class), + $this->createMock(SiteHelper::class), + $this->createMock(SubmissionsJobsManager::class), + $settingsManager, + ])->onlyMethods(['find', 'getLogger'])->getMock(); + $x->method('getLogger')->willReturn(new \Smartling\Vendor\Psr\Log\NullLogger()); + $x->method('find')->willReturn($found); + + return $x; + } + + public function testGetSubmissionEntityStoresProfileOnNewSubmission() + { + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->expects($this->once())->method('getSingleSettingsProfile')->with(1) + ->willReturn($this->profileWithId(9)); + + $entity = $this->getManagerForProfileStamping($settingsManager, [])->getSubmissionEntity('post', 1, 5, 2); + + $this->assertSame(9, $entity->getConfigurationProfileId()); + } + + public function testGetSubmissionEntityRefreshesProfileOnExistingSubmission() + { + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getSingleSettingsProfile')->willReturn($this->profileWithId(9)); + $existing = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(4); + + $entity = $this->getManagerForProfileStamping($settingsManager, [$existing])->getSubmissionEntity('post', 1, 5, 2); + + $this->assertSame(9, $entity->getConfigurationProfileId()); + } + + public function testGetSubmissionEntityKeepsProfileWhenNoActiveProfile() + { + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getSingleSettingsProfile')->willThrowException(new SmartlingDbException('none')); + $existing = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(4); + + $entity = $this->getManagerForProfileStamping($settingsManager, [$existing])->getSubmissionEntity('post', 1, 5, 2); + + $this->assertSame(4, $entity->getConfigurationProfileId()); + } + public function testUpdateEntityQuery() { $title = 'Test'; @@ -139,7 +200,7 @@ public function testUpdateEntityQuery() $submissionId = 17; $db = $this->db; $db->expects($this->once())->method('query') - ->with("UPDATE `wp_smartling_submissions` SET `source_title` = '$title', `source_blog_id` = '$sourceBlogId', `source_content_hash` = '', `content_type` = '', `source_id` = '', `file_uri` = '', `target_locale` = '', `target_blog_id` = '', `target_id` = '', `submitter` = '', `submission_date` = '', `applied_date` = '', `approved_string_count` = '', `completed_string_count` = '', `excluded_string_count` = '', `total_string_count` = '', `word_count` = '', `status` = '', `is_locked` = '', `is_cloned` = '', `last_modified` = '', `outdated` = '', `last_error` = '', `locked_fields` = '', `created_at` = '' WHERE ( `id` = '$submissionId' ) LIMIT 1") + ->with("UPDATE `wp_smartling_submissions` SET `source_title` = '$title', `source_blog_id` = '$sourceBlogId', `source_content_hash` = '', `content_type` = '', `source_id` = '', `file_uri` = '', `target_locale` = '', `target_blog_id` = '', `target_id` = '', `submitter` = '', `submission_date` = '', `applied_date` = '', `approved_string_count` = '', `completed_string_count` = '', `excluded_string_count` = '', `total_string_count` = '', `word_count` = '', `status` = '', `is_locked` = '', `is_cloned` = '', `last_modified` = '', `outdated` = '', `last_error` = '', `locked_fields` = '', `created_at` = '', `configuration_profile_id` = '' WHERE ( `id` = '$submissionId' ) LIMIT 1") ->willReturn(true); $x = $this->subject; $x->method('getDbal')->willReturn($db); @@ -172,9 +233,10 @@ public function testDeleteEntityQuery() $this->createMock(LocalizationPluginProxyInterface::class), $this->createMock(SiteHelper::class), $submissionsJobsManager, + $this->createMock(SettingsManager::class), ])->onlyMethods(['fetchData', 'getLogger'])->getMock(); $x->method('getLogger')->willReturn(new NullLogger()); - $x->expects($this->once())->method('fetchData')->with("SELECT s.id, s.source_title, s.source_blog_id, s.source_content_hash, s.content_type, s.source_id, s.file_uri, s.target_locale, s.target_blog_id, s.target_id, s.submitter, s.submission_date, s.applied_date, s.approved_string_count, s.completed_string_count, s.excluded_string_count, s.total_string_count, s.word_count, s.status, s.is_locked, s.is_cloned, s.last_modified, s.outdated, s.last_error, s.locked_fields, s.created_at, j.job_name, j.job_uid, j.project_uid, j.created, j.modified FROM wp_smartling_submissions AS s\n LEFT JOIN wp_smartling_submissions_jobs AS sj ON s.id = sj.submission_id\n LEFT JOIN wp_smartling_jobs AS j ON sj.job_id = j.id WHERE ( ( s.id IN('$submissionId') ) )")->willReturn([$entity]); + $x->expects($this->once())->method('fetchData')->with("SELECT s.id, s.source_title, s.source_blog_id, s.source_content_hash, s.content_type, s.source_id, s.file_uri, s.target_locale, s.target_blog_id, s.target_id, s.submitter, s.submission_date, s.applied_date, s.approved_string_count, s.completed_string_count, s.excluded_string_count, s.total_string_count, s.word_count, s.status, s.is_locked, s.is_cloned, s.last_modified, s.outdated, s.last_error, s.locked_fields, s.created_at, s.configuration_profile_id, j.job_name, j.job_uid, j.project_uid, j.created, j.modified FROM wp_smartling_submissions AS s\n LEFT JOIN wp_smartling_submissions_jobs AS sj ON s.id = sj.submission_id\n LEFT JOIN wp_smartling_jobs AS j ON sj.job_id = j.id WHERE ( ( s.id IN('$submissionId') ) )")->willReturn([$entity]); $x->delete($entity); } diff --git a/tests/Traits/SubmissionManagerMock.php b/tests/Traits/SubmissionManagerMock.php index b23edf335..3ebea3cf4 100644 --- a/tests/Traits/SubmissionManagerMock.php +++ b/tests/Traits/SubmissionManagerMock.php @@ -8,6 +8,7 @@ use Smartling\Helpers\SiteHelper; use Smartling\Jobs\JobManager; use Smartling\Jobs\SubmissionsJobsManager; +use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionManager; trait SubmissionManagerMock @@ -33,7 +34,8 @@ private function mockSubmissionManager(SmartlingToCMSDatabaseAccessWrapperInterf $this->createMock(JobManager::class), $this->createMock(LocalizationPluginProxyInterface::class), $this->createMock(SiteHelper::class), - $this->createMock(SubmissionsJobsManager::class) + $this->createMock(SubmissionsJobsManager::class), + $this->createMock(SettingsManager::class), ]) ->getMock(); } diff --git a/tests/playwright/bulk-submit.spec.js b/tests/playwright/bulk-submit.spec.js new file mode 100644 index 000000000..26beb58ce --- /dev/null +++ b/tests/playwright/bulk-submit.spec.js @@ -0,0 +1,103 @@ +/** + * Integration test for the bulk-submit "create submissions" flow. + * + * Regression coverage: bulk submit used to always fail with + * {"status":"FAILED","response":{"key":"content.submission.failed", + * "message":"Source content id is empty, please save content prior to uploading"}} + * because the bulk-submit UI sends an empty `source.id` array (the selected + * content ids travel in `ids` instead), while + * UserTranslationRequest::fromArray() unconditionally required `source.id[0]` + * before ever looking at `ids`/isBulk(). See UserTranslationRequest::fromArray() + * and UserCloneRequest::getSourceId(). + * + * This test drives the real bulk-submit page end to end (select a content row, + * create a job, submit) and inspects the actual admin-ajax.php network traffic, + * rather than asserting on rendered UI text — the front end swallows the + * server's error details (jQuery rejects on the AJAX call's HTTP 400 and the + * catch handler falls back to a generic message), so the network response body + * is the only place the regression signature is visible. + */ +const { test, expect } = require('@playwright/test'); + +// Real calls to the Smartling API (job creation, batch creation) plus a +// possibly-cold React mount (see job-wizard.spec.js) can comfortably exceed +// the default 120 s test timeout. +test.setTimeout(240000); + +test.describe('Bulk submit — create submissions', () => { + test('submitting a bulk selection does not fail with "Source content id is empty"', async ({ page }) => { + await page.goto('/wp-admin/admin.php?page=smartling-bulk-submit', { waitUntil: 'commit' }); + await page.waitForSelector('#smartling-app', { state: 'attached', timeout: 90000 }); + + const app = page.locator('#smartling-app'); + + // Wait for the React job wizard to mount (tabs rendered) and for the + // bulk-submit table rows (with their per-row checkboxes) to be present. + await page.waitForFunction( + () => { + const el = document.getElementById('smartling-app'); + return el && ( + el.querySelector('[role="tablist"]') !== null || + el.querySelector('.components-tab-panel__tabs') !== null + ); + }, + null, + { timeout: 90000 }, + ); + const rowCheckbox = page.locator('input.bulkaction[type="checkbox"]').first(); + await expect(rowCheckbox, 'Bulk submit table must list at least one content row to select').toBeVisible({ timeout: 30000 }); + + // Checkbox id is "{contentId}-{contentType}" (see BulkSubmitTableWidget::column_cb()). + const checkboxId = await rowCheckbox.getAttribute('id'); + const [expectedContentId] = checkboxId.split('-'); + await rowCheckbox.check(); + + // Default tab is "New Job" — fill the required Name field. + await app.getByLabel('Name', { exact: true }).fill(`Playwright bulk submit ${Date.now()}`); + + // Select a target locale (submit button stays disabled without one). + const targetLocalesFieldset = app.locator('fieldset', { hasText: 'Target Locales' }); + const localeCheckbox = targetLocalesFieldset.locator('input[type="checkbox"]').first(); + await expect(localeCheckbox, 'Profile must have at least one enabled target locale').toBeVisible({ timeout: 15000 }); + await localeCheckbox.check(); + + const isCreateSubmissions = (url, body) => + url.includes('admin-ajax.php') && url.includes('action=smartling-create-submissions'); + const isCreateJob = (url, body) => + url.includes('admin-ajax.php') && (body || '').includes('innerAction=create-job'); + + const [jobResponse, submissionResponse] = await Promise.all([ + page.waitForResponse((r) => isCreateJob(r.url(), r.request().postData()), { timeout: 120000 }), + page.waitForResponse((r) => isCreateSubmissions(r.url(), r.request().postData()), { timeout: 120000 }), + app.getByRole('button', { name: 'Create Job' }).click(), + ]); + + const jobBody = await jobResponse.json(); + expect(jobBody.status, `Job creation failed: ${JSON.stringify(jobBody)}`).toBe(200); + + const submissionRequestBody = submissionResponse.request().postData() || ''; + const submissionParams = new URLSearchParams(submissionRequestBody); + expect( + submissionParams.getAll('ids[]'), + 'Bulk submit must send the selected content id via `ids[]`', + ).toContain(expectedContentId); + expect( + submissionParams.has('source[id][]'), + 'Bulk submit must NOT send a populated source.id — bulk content ids travel in `ids` only', + ).toBe(false); + + const submissionBody = await submissionResponse.json(); + + // The exact regression: bulk submit must never fail because source + // content id is considered empty. + if (submissionBody.status === 'FAILED') { + expect( + submissionBody.response?.message, + `Regression: bulk submit failed with the "empty source id" error: ${JSON.stringify(submissionBody)}`, + ).not.toContain('Source content id is empty'); + } + + expect(submissionResponse.status(), `Unexpected AJAX status, body: ${JSON.stringify(submissionBody)}`).toBe(200); + expect(submissionBody.status, `Unexpected response body: ${JSON.stringify(submissionBody)}`).toBe('SUCCESS'); + }); +}); diff --git a/tests/setup-local-test-db.sh b/tests/setup-local-test-db.sh new file mode 100755 index 000000000..9f7500b1b --- /dev/null +++ b/tests/setup-local-test-db.sh @@ -0,0 +1,147 @@ +#!/usr/bin/env bash +# +# Setup script for local integration test database. +# +# Run once (or when you want a clean slate). Creates the wordpress_test database +# by importing the production wordpress database schema and data, then renames +# all tables from the production prefix (wp_) to the test prefix (wptests_). +# +# Also creates tests/wp-test-install/ — a minimal WordPress directory with its +# own wp-config.php pointing to the test database, used by wp-cli during tests +# (for cron execution and db:query calls). +# +# Usage: +# cp tests/.env.local.example tests/.env.local +# # Fill in your values in tests/.env.local +# bash tests/setup-local-test-db.sh + +set -e + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +PROJECT_DIR="$(dirname "$SCRIPT_DIR")" +ENV_FILE="$SCRIPT_DIR/.env.local" + +if [ ! -f "$ENV_FILE" ]; then + echo "ERROR: $ENV_FILE not found." + echo "Copy tests/.env.local.example to tests/.env.local and fill in your values." + exit 1 +fi + +# Load env vars +set -a +source "$ENV_FILE" +set +a + +# Defaults +WP_DB_USER="${WP_DB_USER:-root}" +WP_DB_PASS="${WP_DB_PASS:-}" +WP_DB_HOST="${WP_DB_HOST:-127.0.0.1}" +WP_DB_NAME="${WP_DB_NAME:-wordpress_test}" +WP_DB_TABLE_PREFIX="${WP_DB_TABLE_PREFIX:-wptests_}" +WP_INSTALL_DIR="${WP_INSTALL_DIR:-/opt/homebrew/var/www}" +WPCLI_PATH="${WPCLI_PATH:-$SCRIPT_DIR/wp-test-install}" +SOURCE_DB="${SOURCE_DB:-wordpress}" +SOURCE_PREFIX="${SOURCE_PREFIX:-wp_}" + +MYSQL_ARGS="-u $WP_DB_USER -h $WP_DB_HOST" +if [ -n "$WP_DB_PASS" ]; then + MYSQL_ARGS="$MYSQL_ARGS -p$WP_DB_PASS" +fi + +echo "=== Step 1: Create test database '$WP_DB_NAME' ===" +mysql $MYSQL_ARGS -e "DROP DATABASE IF EXISTS \`$WP_DB_NAME\`; CREATE DATABASE \`$WP_DB_NAME\` CHARACTER SET utf8 COLLATE utf8_unicode_ci;" + +echo "=== Step 2: Import production schema and data from '$SOURCE_DB' ===" +# --set-gtid-purged=OFF: required for MySQL servers with GTID mode enabled (MySQL 8+) +# --single-transaction: consistent snapshot without locking tables +mysqldump $MYSQL_ARGS --set-gtid-purged=OFF --single-transaction "$SOURCE_DB" | mysql $MYSQL_ARGS "$WP_DB_NAME" + +echo "=== Step 3: Rename tables from '${SOURCE_PREFIX}' prefix to '${WP_DB_TABLE_PREFIX}' prefix ===" +# Build and execute RENAME TABLE statements dynamically +RENAME_SQL=$(mysql $MYSQL_ARGS -N "$WP_DB_NAME" -e " + SELECT CONCAT('RENAME TABLE \`', TABLE_NAME, '\` TO \`', REPLACE(TABLE_NAME, '${SOURCE_PREFIX}', '${WP_DB_TABLE_PREFIX}'), '\`;') + FROM information_schema.TABLES + WHERE TABLE_SCHEMA = '${WP_DB_NAME}' + ORDER BY TABLE_NAME; +") + +if [ -z "$RENAME_SQL" ]; then + echo "ERROR: No tables found to rename in $WP_DB_NAME. Import may have failed." + exit 1 +fi + +echo "$RENAME_SQL" | mysql $MYSQL_ARGS "$WP_DB_NAME" + +echo "=== Step 4: Verify tables ===" +TABLE_COUNT=$(mysql $MYSQL_ARGS -N "$WP_DB_NAME" -e "SELECT COUNT(*) FROM information_schema.TABLES WHERE TABLE_SCHEMA = '$WP_DB_NAME';") +echo "Tables in $WP_DB_NAME: $TABLE_COUNT" + +echo "=== Step 5: Create test WordPress install dir at '$WPCLI_PATH' ===" +mkdir -p "$WPCLI_PATH" + +# Create wp-config.php pointing to the test database +cat > "$WPCLI_PATH/wp-config.php" << WPCONFIG + Date: Thu, 1 Oct 2026 22:23:52 +0200 Subject: [PATCH 02/11] Store configuration profile when existing submissions are uploaded (WP-1021) Queued uploads load existing submissions by id and never went through SubmissionManager::getSubmissionEntity(), so configuration_profile_id stayed null. Stamp and persist the profile in UploadJob and prepareUpload. Co-Authored-By: Claude Sonnet 5.5 --- .../Base/SmartlingCoreUploadTrait.php | 2 ++ inc/Smartling/Jobs/UploadJob.php | 7 ++++++ .../Submissions/SubmissionManager.php | 2 +- tests/Smartling/Jobs/UploadJobTest.php | 25 +++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/inc/Smartling/Base/SmartlingCoreUploadTrait.php b/inc/Smartling/Base/SmartlingCoreUploadTrait.php index 8b3936409..b1e89ca41 100644 --- a/inc/Smartling/Base/SmartlingCoreUploadTrait.php +++ b/inc/Smartling/Base/SmartlingCoreUploadTrait.php @@ -61,6 +61,8 @@ protected function getFunctionProxyHelper(): WordpressFunctionProxyHelper public function prepareUpload(SubmissionEntity $submission): SubmissionEntity { + $this->getSubmissionManager()->stampConfigurationProfile($submission); + return $this->renewContentHash( $this->createTargetContent( $this->setFileUriIfNullId($submission) diff --git a/inc/Smartling/Jobs/UploadJob.php b/inc/Smartling/Jobs/UploadJob.php index 570aa9321..4c1eef2e1 100644 --- a/inc/Smartling/Jobs/UploadJob.php +++ b/inc/Smartling/Jobs/UploadJob.php @@ -77,6 +77,13 @@ private function processUploadQueue(int $blogId): void $submission->setFileUri($this->fileUriHelper->generateFileUri($submission)); $this->submissionManager->storeEntity($submission); } + // Existing submissions are loaded from the queue by id, so they never pass through + // SubmissionManager::getSubmissionEntity(): remember the profile used for this upload here. + $previousProfileId = $submission->getConfigurationProfileId(); + $this->submissionManager->stampConfigurationProfile($submission); + if ($submission->getConfigurationProfileId() !== $previousProfileId) { + $this->submissionManager->storeEntity($submission); + } $profileKey = $submission->getConfigurationProfileId() ?? "blog{$submission->getSourceBlogId()}"; if (!array_key_exists($profileKey, $profiles)) { try { diff --git a/inc/Smartling/Submissions/SubmissionManager.php b/inc/Smartling/Submissions/SubmissionManager.php index c8bdf4185..426aa6b77 100644 --- a/inc/Smartling/Submissions/SubmissionManager.php +++ b/inc/Smartling/Submissions/SubmissionManager.php @@ -495,7 +495,7 @@ public function getSubmissionEntity( * Remembers the profile used for this translation request, so that delivery uses the same profile * even if the active profile has been switched in the meantime. */ - private function stampConfigurationProfile(SubmissionEntity $entity): void + public function stampConfigurationProfile(SubmissionEntity $entity): void { try { $entity->setConfigurationProfileId($this->settingsManager->getSingleSettingsProfile($entity->getSourceBlogId())->getId()); diff --git a/tests/Smartling/Jobs/UploadJobTest.php b/tests/Smartling/Jobs/UploadJobTest.php index 273e57a18..642c008a6 100644 --- a/tests/Smartling/Jobs/UploadJobTest.php +++ b/tests/Smartling/Jobs/UploadJobTest.php @@ -125,6 +125,31 @@ public function testClonedSubmissionIsSkippedAndCompletesQueueItemWithoutUploadi $this->assertFalse($uploaded, 'Cloned submissions must not be uploaded'); } + /** + * Existing submissions are loaded from the queue by id and never pass through + * SubmissionManager::getSubmissionEntity(), so the profile has to be stored on upload. + */ + public function testStoresConfigurationProfileOfExistingSubmissionOnUpload() + { + $submission = new SubmissionEntity(); + $submission->setId(1); + $submission->setFileUri('file.xml'); + $submission->setSourceBlogId(1); + $item = $this->buildItem($submission); + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->method('stampConfigurationProfile')->willReturnCallback( + static function (SubmissionEntity $submission) { + $submission->setConfigurationProfileId(5); + }, + ); + $submissionManager->expects($this->once())->method('storeEntity')->with($submission); + + $this->buildJob($this->buildQueueManager($item), $submissionManager)->run(''); + + $this->assertSame(5, $submission->getConfigurationProfileId()); + } + /** * A queue item groups submissions for the same content across multiple target * locales; only the first one is used to look up the profile/batch job. If either From b8e4092983dcff96075b701034ef32a607cc01a6 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 12:20:16 +0200 Subject: [PATCH 03/11] address review comments on #636 (WP-1021) - fix UploadQueueManager::purge() caching profiles by source blog id instead of configuration profile id, which could cancel batch files against the wrong project when submissions from the same blog were stamped with different profiles - log a warning in SettingsManager::getProfileBySubmission() when a stored profile's source blog no longer matches the submission's source blog, so a repurposed profile is diagnosable - add DB_TYPE_U_BIGINT_NULL and use it for the new configuration profile id column instead of a raw literal - resolve the profile once per method in FieldsFilterHelper instead of calling getProfileBySubmission() repeatedly for the same submission - document tests/setup-local-test-db.sh as a personal/local convenience script and require WP_INSTALL_DIR instead of silently defaulting to a Homebrew path - remove inc/.DS_Store and inc/composer-backups.zip, rewritten out of branch history entirely rather than just deleted in a new commit, and ignore both patterns going forward --- .gitignore | 4 ++ .../Base/SmartlingEntityAbstract.php | 1 + inc/Smartling/DbAl/UploadQueueManager.php | 7 +- inc/Smartling/Helpers/FieldsFilterHelper.php | 11 +-- inc/Smartling/Settings/SettingsManager.php | 3 + .../Submissions/SubmissionEntity.php | 2 +- .../Smartling/DbAl/UploadQueueManagerTest.php | 67 +++++++++++++++++++ .../Settings/SettingsManagerTest.php | 20 ++++++ tests/setup-local-test-db.sh | 11 ++- 9 files changed, 117 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 6c9b19445..2bfece247 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,10 @@ tests/IntegrationTests/src #IDE .idea +# macOS +.DS_Store +*-backups.zip + # Playwright E2E .env.playwright tests/playwright/.auth/ diff --git a/inc/Smartling/Base/SmartlingEntityAbstract.php b/inc/Smartling/Base/SmartlingEntityAbstract.php index 270bd5897..4d714ff4a 100644 --- a/inc/Smartling/Base/SmartlingEntityAbstract.php +++ b/inc/Smartling/Base/SmartlingEntityAbstract.php @@ -13,6 +13,7 @@ abstract class SmartlingEntityAbstract implements SmartlingTableDefinitionInterf public const DB_TYPE_DEFAULT_EMPTYSTRING = 'DEFAULT \'\''; public const DB_TYPE_U_BIGINT = 'INT(20) UNSIGNED NOT NULL'; // BIGINT alias of INT(20) + public const DB_TYPE_U_BIGINT_NULL = 'INT(20) UNSIGNED NULL'; public const DB_TYPE_DATETIME = 'DATETIME NOT NULL DEFAULT \'0000-00-00 00:00:00\''; public const DB_TYPE_DATETIME_NULL = 'DATETIME NULL DEFAULT NULL'; public const DB_TYPE_STRING_STANDARD = 'VARCHAR(255) NOT NULL'; diff --git a/inc/Smartling/DbAl/UploadQueueManager.php b/inc/Smartling/DbAl/UploadQueueManager.php index 69bf5f8c5..d833623a4 100644 --- a/inc/Smartling/DbAl/UploadQueueManager.php +++ b/inc/Smartling/DbAl/UploadQueueManager.php @@ -270,15 +270,16 @@ public function purge(): void if ($submission === null) { continue; } - if (!array_key_exists($submission->getSourceBlogId(), $profiles)) { + $profileKey = $submission->getConfigurationProfileId() ?? "blog{$submission->getSourceBlogId()}"; + if (!array_key_exists($profileKey, $profiles)) { try { $profile = $this->settingsManager->getProfileBySubmission($submission); } catch (SmartlingDbException) { $profile = null; } - $profiles[$submission->getSourceBlogId()] = $profile; + $profiles[$profileKey] = $profile; } - $profile = $profiles[$submission->getSourceBlogId()]; + $profile = $profiles[$profileKey]; if (!$profile instanceof ConfigurationProfileEntity) { continue; } diff --git a/inc/Smartling/Helpers/FieldsFilterHelper.php b/inc/Smartling/Helpers/FieldsFilterHelper.php index 360ff13a6..2ca96789e 100644 --- a/inc/Smartling/Helpers/FieldsFilterHelper.php +++ b/inc/Smartling/Helpers/FieldsFilterHelper.php @@ -128,6 +128,7 @@ public function processStringsBeforeEncoding( } $settings = $this->contentSerializationHelper->prepareFieldProcessorValues($submission); + $filterFieldNameRegExp = $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(); return $this->passConnectionProfileFilters( $this->passFieldProcessorsBeforeSendFilters( @@ -135,11 +136,11 @@ public function processStringsBeforeEncoding( $this->removeFields( $this->flattenArray($data), $settings['ignore'], - $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), + $filterFieldNameRegExp, ) ), $strategy, - $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), + $filterFieldNameRegExp, $settings, ); } @@ -174,17 +175,19 @@ public function applyTranslatedValues(SubmissionEntity $submission, array $origi private function filterArray(array $array, SubmissionEntity $submission, string $strategy, array $settings): array { + $filterFieldNameRegExp = $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(); + return $this->passConnectionProfileFilters( $this->passFieldProcessorsFilters( $submission, $this->removeFields( $array, $settings['ignore'], - $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), + $filterFieldNameRegExp, ), ), $strategy, - $this->settingsManager->getProfileBySubmission($submission)->getFilterFieldNameRegExp(), + $filterFieldNameRegExp, $this->contentSerializationHelper->prepareFieldProcessorValues($submission), ); } diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index 47a0e7315..a8599315a 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -131,6 +131,9 @@ public function getProfileBySubmission(SubmissionEntity $submission): Configurat if ($profileId !== null) { $profile = ArrayHelper::first($this->getEntityById($profileId)); if ($profile instanceof ConfigurationProfileEntity) { + if ($profile->getSourceLocale()->getBlogId() !== $submission->getSourceBlogId()) { + $this->getLogger()->warning("Profile id=$profileId stored for submission id={$submission->getId()} has source blog {$profile->getSourceLocale()->getBlogId()}, but submission source blog is {$submission->getSourceBlogId()}, profile may have been repurposed since the submission was stamped"); + } return $profile; } $this->getLogger()->warning("Profile id=$profileId stored for submission id={$submission->getId()} not found, using active profile of source blog"); diff --git a/inc/Smartling/Submissions/SubmissionEntity.php b/inc/Smartling/Submissions/SubmissionEntity.php index dcc6ce5a6..1e3220d88 100644 --- a/inc/Smartling/Submissions/SubmissionEntity.php +++ b/inc/Smartling/Submissions/SubmissionEntity.php @@ -118,7 +118,7 @@ public static function getFieldDefinitions(): array static::FIELD_LAST_ERROR => static::DB_TYPE_STRING_TEXT, static::FIELD_LOCKED_FIELDS => 'TEXT NULL', static::FIELD_CREATED_AT => static::DB_TYPE_DATETIME, - static::FIELD_CONFIGURATION_PROFILE_ID => 'INT(20) UNSIGNED NULL', + static::FIELD_CONFIGURATION_PROFILE_ID => static::DB_TYPE_U_BIGINT_NULL, ]; } diff --git a/tests/Smartling/DbAl/UploadQueueManagerTest.php b/tests/Smartling/DbAl/UploadQueueManagerTest.php index b4481dc83..54f9590a1 100644 --- a/tests/Smartling/DbAl/UploadQueueManagerTest.php +++ b/tests/Smartling/DbAl/UploadQueueManagerTest.php @@ -8,6 +8,7 @@ use Smartling\Exception\SmartlingDbException; use Smartling\Models\IntegerIterator; use Smartling\Models\UploadQueueEntity; +use Smartling\Settings\ConfigurationProfileEntity; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; use Smartling\Submissions\SubmissionManager; @@ -112,6 +113,72 @@ public function query() {} ))->enqueue(new IntegerIterator([1, 2, 3, 4, 7]), ''); // Submission with id 7 does not exist, and should not be stored } + public function testPurgeUsesDistinctProfilePerSubmissionSharingSourceBlog() + { + $profileA = $this->createMock(ConfigurationProfileEntity::class); + $profileB = $this->createMock(ConfigurationProfileEntity::class); + + $submission1 = $this->createMock(SubmissionEntity::class); + $submission1->method('getId')->willReturn(1); + $submission1->method('getSourceBlogId')->willReturn(1); + $submission1->method('getConfigurationProfileId')->willReturn(100); + $submission1->method('getFileUri')->willReturn('file1.xml'); + + $submission2 = $this->createMock(SubmissionEntity::class); + $submission2->method('getId')->willReturn(2); + $submission2->method('getSourceBlogId')->willReturn(1); // same source blog as submission1 + $submission2->method('getConfigurationProfileId')->willReturn(200); // different profile + $submission2->method('getFileUri')->willReturn('file2.xml'); + + $stored = [$submission1, $submission2]; + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->method('getEntityById')->willReturnCallback(function ($id) use ($stored) { + foreach ($stored as $submission) { + if ($submission->getId() === $id) { + return $submission; + } + } + return null; + }); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getProfileBySubmission')->willReturnCallback( + function (SubmissionEntity $submission) use ($submission1, $profileA, $profileB) { + return $submission === $submission1 ? $profileA : $profileB; + }, + ); + + $this->mockDbAl(); + $db = $this->getMockBuilder(DB::class) + ->setConstructorArgs([new class { + public string $base_prefix = ''; + public function getResultsArray() {} + public function query() {} + }]) + ->onlyMethods(['getResultsArray', 'query']) + ->getMock(); + $db->method('getResultsArray')->willReturn([ + ['id' => 1, 'batch_uid' => 'batch-1', 'submission_ids' => '1,2'], + ]); + + $cancelledWith = []; + $apiWrapper = $this->createMock(ApiWrapperInterface::class); + $apiWrapper->method('cancelBatchFile')->willReturnCallback( + function (ConfigurationProfileEntity $profile, string $batchUid, string $fileUri) use (&$cancelledWith) { + $cancelledWith[] = [$profile, $batchUid, $fileUri]; + }, + ); + + (new UploadQueueManager($apiWrapper, $settingsManager, $db, $submissionManager))->purge(); + + $this->assertCount(2, $cancelledWith, 'Expected cancelBatchFile to be called once per submission'); + $this->assertSame($profileA, $cancelledWith[0][0], 'Submission 1 must be cancelled against its own stamped profile'); + $this->assertSame('file1.xml', $cancelledWith[0][2]); + $this->assertSame($profileB, $cancelledWith[1][0], 'Submission 2 must be cancelled against its own stamped profile, not submission 1\'s cached one'); + $this->assertSame('file2.xml', $cancelledWith[1][2]); + } + public function testDequeue() { $submission1 = $this->createMock(SubmissionEntity::class); diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index 14e6b3793..b0f99e5a9 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -8,6 +8,7 @@ use Smartling\Exception\SmartlingConfigException; use Smartling\Exception\SmartlingDbException; use Smartling\Settings\ConfigurationProfileEntity; +use Smartling\Settings\Locale; use Smartling\Settings\SettingsManager; use Smartling\Settings\TargetLocale; use Smartling\Submissions\SubmissionEntity; @@ -99,6 +100,9 @@ private function profileWithId(int $id): ConfigurationProfileEntity public function testGetProfileBySubmissionUsesStoredProfile() { $stored = $this->profileWithId(7); + $sourceLocale = new Locale(); + $sourceLocale->setBlogId(1); + $stored->setSourceLocale($sourceLocale); $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById']); $mock->expects(self::once())->method('getEntityById')->with(7)->willReturn([$stored]); $mock->expects(self::never())->method('getSingleSettingsProfile'); @@ -108,6 +112,22 @@ public function testGetProfileBySubmissionUsesStoredProfile() self::assertSame($stored, $mock->getProfileBySubmission($submission)); } + public function testGetProfileBySubmissionLogsWarningWhenStoredProfileBlogMismatches() + { + $stored = $this->profileWithId(7); + $sourceLocale = new Locale(); + $sourceLocale->setBlogId(2); + $stored->setSourceLocale($sourceLocale); + $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById', 'getLogger']); + $mock->method('getLogger')->willReturn(new NullLogger()); + $mock->expects(self::once())->method('getEntityById')->with(7)->willReturn([$stored]); + $mock->expects(self::never())->method('getSingleSettingsProfile'); + + $submission = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(7); + + self::assertSame($stored, $mock->getProfileBySubmission($submission)); + } + public function testGetProfileBySubmissionFallsBackWithoutStoredProfile() { $active = $this->profileWithId(3); diff --git a/tests/setup-local-test-db.sh b/tests/setup-local-test-db.sh index 9f7500b1b..3af90ff99 100755 --- a/tests/setup-local-test-db.sh +++ b/tests/setup-local-test-db.sh @@ -14,6 +14,11 @@ # cp tests/.env.local.example tests/.env.local # # Fill in your values in tests/.env.local # bash tests/setup-local-test-db.sh +# +# This is a personal convenience script for local development, not shared tooling: +# it dumps a developer's own "production" WordPress database into the test database. +# All paths are read from tests/.env.local; there is no expectation it works unmodified +# on another contributor's machine. set -e @@ -38,7 +43,11 @@ WP_DB_PASS="${WP_DB_PASS:-}" WP_DB_HOST="${WP_DB_HOST:-127.0.0.1}" WP_DB_NAME="${WP_DB_NAME:-wordpress_test}" WP_DB_TABLE_PREFIX="${WP_DB_TABLE_PREFIX:-wptests_}" -WP_INSTALL_DIR="${WP_INSTALL_DIR:-/opt/homebrew/var/www}" +if [ -z "$WP_INSTALL_DIR" ]; then + echo "ERROR: WP_INSTALL_DIR not set in $ENV_FILE." + echo "See tests/.env.local.example for the expected value." + exit 1 +fi WPCLI_PATH="${WPCLI_PATH:-$SCRIPT_DIR/wp-test-install}" SOURCE_DB="${SOURCE_DB:-wordpress}" SOURCE_PREFIX="${SOURCE_PREFIX:-wp_}" From 06bdd03faaa040e5497a457e8fcc4a88dde49cc2 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 12:21:41 +0200 Subject: [PATCH 04/11] clarify Migration261001 docblock on profile backfill timing (WP-1021) --- inc/Smartling/DbAl/Migrations/Migration261001.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/inc/Smartling/DbAl/Migrations/Migration261001.php b/inc/Smartling/DbAl/Migrations/Migration261001.php index 8a56315cd..ca786a723 100644 --- a/inc/Smartling/DbAl/Migrations/Migration261001.php +++ b/inc/Smartling/DbAl/Migrations/Migration261001.php @@ -8,8 +8,10 @@ /** * Stores the configuration profile a submission was requested with. * - * Existing rows are left NULL on purpose: the profile active today is not necessarily the - * one they were uploaded with, so they keep resolving the profile by source blog. + * Existing rows are left NULL: until stamped, they keep resolving the profile by source + * blog. SubmissionManager::getSubmissionEntity() backfills the column with the currently + * active profile the first time a pre-existing submission is (re)submitted for translation, + * so an in-flight row stays NULL only until it is next touched by an upload/resubmit flow. */ class Migration261001 implements SmartlingDbMigrationInterface { From 908058aa70fa449538ef3a2e9d21068b8cb7b34a Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 13:15:45 +0200 Subject: [PATCH 05/11] fix FtsIntegrationTest::testFullFtsWorkflow failures (WP-1021) - SmartlingUnitTestCaseAbstract::getLogger() was type-hinted to the real Psr\Log\LoggerInterface, but MonologWrapper::getLogger() returns LevelLogger, which implements the scoped Smartling\Vendor\Psr\Log\LoggerInterface. Calling it threw a TypeError; nothing had called it before this test did. - testFullFtsWorkflow exercises the real upload/serialization pipeline (ContentHelper, FieldsFilterHelper's metadata filters) without registering WordPress hooks first, so FILTER_SMARTLING_METADATA_PROCESS_BEFORE_TRANSLATION had no listener and apply_filters() returned its "value" argument (the submission) unchanged, corrupting serialized field values. Added $this->loadBuiltInFilters(), matching the pattern already used by RelationsTest for the same reason. Also applied the already-committed Migration261001 to the local integration test database (tests/setup-local-test-db.sh predates this migration); no code change needed for that part. --- tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php | 2 +- tests/IntegrationTests/tests/FtsIntegrationTest.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php index a8c6c6341..cee4fa312 100644 --- a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php +++ b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php @@ -2,7 +2,6 @@ namespace Smartling\Tests\IntegrationTests; -use Psr\Log\LoggerInterface; use Smartling\ApiWrapperInterface; use Smartling\Bootstrap; use Smartling\ContentTypes\CustomPostType; @@ -31,6 +30,7 @@ use Smartling\Submissions\SubmissionEntity; use Smartling\Submissions\SubmissionManager; use Smartling\Tuner\MediaAttachmentRulesManager; +use Smartling\Vendor\Psr\Log\LoggerInterface; use Smartling\Vendor\Symfony\Component\DependencyInjection\ContainerBuilder; abstract class SmartlingUnitTestCaseAbstract extends WP_UnitTestCase diff --git a/tests/IntegrationTests/tests/FtsIntegrationTest.php b/tests/IntegrationTests/tests/FtsIntegrationTest.php index 25db59456..2474cb59b 100644 --- a/tests/IntegrationTests/tests/FtsIntegrationTest.php +++ b/tests/IntegrationTests/tests/FtsIntegrationTest.php @@ -36,6 +36,8 @@ public function setUp(): void */ public function testFullFtsWorkflow(): void { + $this->loadBuiltInFilters(); + $sourceContent = 'Hello world. This is a test post for instant translation.'; $postId = $this->createPost('post', 'FTS Integration Test Post', $sourceContent); $this->assertGreaterThan(0, $postId, 'Post creation failed'); From 537700eaae5ee8a03a4e5920a0317345ad4a45b6 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 14:22:15 +0200 Subject: [PATCH 06/11] fix SmartlingCoreTest leaking real WP hooks into global state (WP-1021) SmartlingCoreTest::setUp() constructed SmartlingCore with a real WordpressFunctionProxyHelper. SmartlingCore::__construct() registers several add_action/add_filter hooks bound to $this. Under the unit-only test run (bootstrap_units.php) this is harmless because add_filter is a mocked no-op function, but when the full suite runs under the real WP bootstrap (as CI's Buildplan/test.sh does: both the "plugin test" and "integration test" suites in one `phpunit -c tests/phpunit.xml` process, with no --bootstrap override), these become genuine WordPress hook registrations that persist in global state for the rest of the process, one per test method, each bound to a SmartlingCore instance that never went through the DI container's setContentHelper()/etc. calls. FtsIntegrationTest::testFullFtsWorkflow is the first test to actually fire FILTER_SMARTLING_PREPARE_TARGET_CONTENT in that combined run. The stale SmartlingCoreTest callbacks run first and call getContentHelper() on their own un-wired instance, throwing "Typed property ContentHelper:: $ioFactory must not be accessed before initialization" before the real, properly-wired entrypoint singleton's callback ever runs - reproduced locally with `phpunit -c tests/phpunit.xml` (both suites, no bootstrap override) and confirmed fixed by this change. Mock only add_action/add_filter (matching the existing pattern in SmartlingCoreTraitTest, which already avoids this) so the hooks SmartlingCore's constructor registers become no-ops, while every other proxy method still delegates to the real/mocked WP functions the rest of this test file's assertions rely on. --- tests/Smartling/Base/SmartlingCoreTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/Smartling/Base/SmartlingCoreTest.php b/tests/Smartling/Base/SmartlingCoreTest.php index 4050f1a89..a935f90c8 100644 --- a/tests/Smartling/Base/SmartlingCoreTest.php +++ b/tests/Smartling/Base/SmartlingCoreTest.php @@ -52,7 +52,13 @@ class SmartlingCoreTest extends TestCase protected function setUp(): void { WordpressFunctionsMockHelper::injectFunctionsMocks(); - $wpProxy = new WordpressFunctionProxyHelper(); + // add_action/add_filter are stubbed out: SmartlingCore::__construct() registers real + // WordPress hooks bound to $this, and under a real WP bootstrap (as used when this + // suite runs alongside the integration tests) those hooks leak into global state for + // the rest of the process, firing against this un-DI-wired instance in later tests. + $wpProxy = $this->getMockBuilder(WordpressFunctionProxyHelper::class) + ->onlyMethods(['add_action', 'add_filter']) + ->getMock(); $acf = $this->createMock(AcfDynamicSupport::class); $gutenbergBlockHelper = new GutenbergBlockHelper( $acf, From f7b967fcd0f38a3d52be37e2e47b3fa8c0765892 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 15:53:24 +0200 Subject: [PATCH 07/11] load plugin hooks before WP_UnitTestCase snapshots \$wp_filter (WP-1021) Root cause of the remaining FtsIntegrationTest::testFullFtsWorkflow failure ("Target post was not created"): smartling-connector.php only hooks Bootstrap::load() onto 'plugins_loaded' when is_admin() || DOING_CRON, neither of which is true under the PHPUnit CLI bootstrap, so it never runs there on its own - a test has to call it explicitly (as RelationsTest and FtsIntegrationTest already do via loadBuiltInFilters()). That explicit call is not enough on its own, though. WP_UnitTestCase::setUp() snapshots $wp_filter the first time any test runs and tearDown() restores that exact snapshot after every test (_backup_hooks()/_restore_hooks()), unconditionally wiping anything a test registered. SmartlingCore registers several hooks (including FILTER_SMARTLING_PREPARE_TARGET_CONTENT) in its own constructor, and since it's a container-cached singleton, that constructor - and the add_filter() calls inside it - only ever runs once for the whole process. Whichever test happens to trigger that first construction gets working hooks for its own duration, then loses them at its own tearDown; every later test (including this PR's new FtsIntegrationTest, which never got the lucky first slot once the earlier SmartlingCoreTest fix removed it as a construction trigger) is left with zero listeners on a hook its own filter-chain code depends on, with apply_filters() silently returning the submission unchanged and no target ever created. Loading the plugin here, before wp-settings.php's plugin loading has even returned control to the test runner and before any test's setUp() takes that first $wp_filter snapshot, means the snapshot itself already includes every hook the plugin registers, so later tests calling loadBuiltInFilters() again is harmless (idempotent) rather than their only chance at a working hook. Verified by reproducing the exact failure with `phpunit -c tests/phpunit.xml` (both testsuites, real WP bootstrap, no --testsuite/--bootstrap override - matching Buildplan/test.sh), adding temporary instrumentation to confirm zero listeners were registered on FILTER_SMARTLING_PREPARE_TARGET_CONTENT at the moment of failure, and confirming the fix resolves it across both a dirty and a freshly rebuilt local test database. --- tests/IntegrationTests/includes/bootstrap.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/IntegrationTests/includes/bootstrap.php b/tests/IntegrationTests/includes/bootstrap.php index f624ad265..9d7904ebf 100644 --- a/tests/IntegrationTests/includes/bootstrap.php +++ b/tests/IntegrationTests/includes/bootstrap.php @@ -27,3 +27,16 @@ tests_add_filter('wp_die_handler', '_wp_die_handler_filter'); require_once ABSPATH . '/wp-settings.php'; require_once __DIR__ . "/../../../inc/autoload.php"; + +/* + * smartling-connector.php only hooks Bootstrap::load() onto 'plugins_loaded' when + * is_admin() || DOING_CRON, neither of which is true in this CLI/PHPUnit context, so it + * never runs on its own here. Load it once, now, before any test's setUp() runs: the WP + * core test base class (WP_UnitTestCase::setUp()) snapshots $wp_filter on the very first + * test and restores that snapshot after every test (_backup_hooks()/_restore_hooks()), so + * any hook this registers after that point (e.g. from a test calling this again, or from a + * service lazily constructed mid-test) gets wiped at that test's tearDown and never comes + * back once the owning singleton is already cached. Loading here means the snapshot itself + * already includes every hook the plugin registers, so they survive for the whole run. + */ +(new Smartling\Bootstrap())->load(); From f9696088294f0a6927f8260cb43ac4d951989d34 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 16:18:45 +0200 Subject: [PATCH 08/11] fix wpCliExec() targeting the wrong WordPress install locally (WP-1021) wpCliExec() passed WP_INSTALL_DIR to wp-cli's --path, which is the developer's own real WordPress install (tests/.env.local.example's documented default), not the dedicated test install created by tests/setup-local-test-db.sh (WPCLI_PATH, whose wp-config.php points at the isolated test database). Every integration test that drives a translation through the real cron queue (runCronTask() -> wpCliExec('cron', 'event', 'run ...')) was running that cron event against the wrong database, so the job never saw the queued submission and silently did nothing. This only affects local (non-Docker) runs: Buildplan/test.sh and docker-init.sh never set WPCLI_PATH, so getWPcliPathEnv() falls back to WP_INSTALL_DIR there, unchanged. Discovered while trying to reproduce a CI-reported MetadataPartialVsFullTest failure locally: that test uses the same cron path and was failing with an unrelated, local-only symptom (target content never created) until this fix. --- .../SmartlingUnitTestCaseAbstract.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php index cee4fa312..458f82d1a 100644 --- a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php +++ b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php @@ -138,6 +138,18 @@ private static function getWPInstallDirEnv(): string return getenv('WP_INSTALL_DIR'); } + /** + * wp-cli needs a WordPress install whose wp-config.php points at the test database. + * WPCLI_PATH is that dedicated install (see tests/setup-local-test-db.sh); fall back to + * WP_INSTALL_DIR for environments that don't set it. + */ + private static function getWPcliPathEnv(): string + { + $path = getenv('WPCLI_PATH'); + + return $path !== false && $path !== '' ? $path : self::getWPInstallDirEnv(); + } + public function getApiWrapper(): ApiWrapperInterface { return $this->get('api.wrapper.with.retries'); @@ -208,7 +220,7 @@ protected function forceSubmissionDownload(SubmissionEntity $submission): void protected static function wpCliExec(string $command, string $subCommand, string $parameters): void { - shell_exec(sprintf('%s %s %s %s --path=%s', self::getWPcliEnv(), $command, $subCommand, $parameters, self::getWPInstallDirEnv())); + shell_exec(sprintf('%s %s %s %s --path=%s', self::getWPcliEnv(), $command, $subCommand, $parameters, self::getWPcliPathEnv())); } protected function getContainer(): ContainerBuilder From 32c79eb0007eff7c5192f26c2cb44997547ebcee Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 18:28:37 +0200 Subject: [PATCH 09/11] clear RuntimeCacheHelper between integration tests (WP-1021) Root cause of MetadataPartialVsFullTest::testTranslatePostWithMetadata failing with "array doesn't have key 'meta_a'": confirmed via bisection this is a genuine regression (passes cleanly on master under identical conditions), introduced by WP-1021's first commit. SmartlingUnitTestCaseAbstract::cleanUpTables() truncates the posts and smartling_submissions tables before every test, so every test's own post/submission fixtures restart from auto-increment id 1. ContentHelper::readSourceMetadata() caches by "{contentType}-{sourceBlogId}-{sourceId}" in RuntimeCacheHelper, a process-wide singleton that persists for the whole PHPUnit run and is never cleared between tests. So whichever earlier test's "submission 1 / post 1" first triggers an upload (populating the cache with *its* metadata) silently serves that stale entry to every later test that also lands on id 1 - which, thanks to the truncation, is effectively every integration test exercising this path. Confirmed by instrumenting readSourceMetadata() to log its cache key, hit/miss state and call stack: MetadataPartialVsFullTest's own submission is id=1/sourceId=1, and an earlier test's upload (SmartlingCore::prepareTargetEntity -> readSourceContentWithMetadataAsArray) had already populated that exact cache key first. WP-1021 doesn't touch this caching path at all; it just happened to shift which test first reaches that code for id 1, surfacing a pre-existing cross-test contamination bug in the harness. Clearing the cache alongside the table truncation in setUp() fixes it without touching any production caching behavior (WP admin requests build a fresh container, and therefore a fresh RuntimeCacheHelper, every time). --- inc/Smartling/Helpers/RuntimeCacheHelper.php | 5 +++++ tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/inc/Smartling/Helpers/RuntimeCacheHelper.php b/inc/Smartling/Helpers/RuntimeCacheHelper.php index a824a5eb6..8fd73d760 100644 --- a/inc/Smartling/Helpers/RuntimeCacheHelper.php +++ b/inc/Smartling/Helpers/RuntimeCacheHelper.php @@ -54,4 +54,9 @@ public function set($key, $value, $scope = self::DEFAULT_SCOPE) { $this->storage[$scope][$key] = $value; } + + public function clear(): void + { + $this->storage = []; + } } \ No newline at end of file diff --git a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php index 458f82d1a..e26946e25 100644 --- a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php +++ b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php @@ -10,6 +10,7 @@ use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\ContentHelper; use Smartling\Helpers\GutenbergBlockHelper; +use Smartling\Helpers\RuntimeCacheHelper; use Smartling\Helpers\SiteHelper; use Smartling\Helpers\TranslationHelper; use Smartling\Jobs\DownloadTranslationJob; @@ -122,6 +123,14 @@ public function setUp(): void { parent::setUp(); $this->cleanUpTables(); + /* + * cleanUpTables() truncates posts/submissions, so every test's fixtures restart from + * auto-increment id 1. ContentHelper's RuntimeCacheHelper is a process-wide singleton + * keyed by contentType-sourceBlogId-sourceId, so without this, one test's "submission + * 1 / post 1" can serve cached (stale) metadata to every later test that also lands on + * id 1 - which is effectively all of them. + */ + RuntimeCacheHelper::getInstance()->clear(); $this->registerPostTypes(); $this->ensureProfileExists(); } From 412cba1ee38c3c32e2bfd705eff4af0990240f26 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 19:17:16 +0200 Subject: [PATCH 10/11] truncate smartling_upload_queue between integration tests (WP-1021) SmartlingUnitTestCaseAbstract::cleanUpTables() truncates 'smartling_queue' (the legacy Queue class's download queue table) but never UploadQueueEntity's own table, smartling_upload_queue. Since posts and smartling_submissions ARE truncated every test (resetting their auto-increment ids back to 1), a stale upload_queue row left over from an earlier test's incomplete upload references a submission id that gets silently reused by whatever later test next creates submission #1 - and UploadQueueManager::dequeue() resolves queue rows by id, so it finds and reprocesses a submission that has nothing to do with the row's original test. Found this while investigating target content not being created for MetadataPartialVsFullTest/SubmissionUploadTest after real upload/download cycles: instrumented UploadJob::processUploadQueue() and saw the upload queue length balloon to 32 by the time that test ran, with the same submission id being repeatedly reclaimed across multiple cron invocations. Confirmed via the same instrumentation that queue length returns to 1 per invocation once this table is included. --- tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php index e26946e25..06e6ee714 100644 --- a/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php +++ b/tests/IntegrationTests/SmartlingUnitTestCaseAbstract.php @@ -107,6 +107,7 @@ protected function cleanUpTables() 'smartling_submissions', JobEntity::getTableName(), SubmissionJobEntity::getTableName(), + UploadQueueEntity::getTableName(), ]; $tablePrefix = getenv('WP_DB_TABLE_PREFIX'); From d78346efb7c4e1138523af87b8a58efd39471ec0 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Mon, 5 Oct 2026 20:50:28 +0200 Subject: [PATCH 11/11] address second round of review comments on #636 (WP-1021) sl-mmuradov found several places where the profile stamped at request time (the whole point of WP-1021) got silently discarded or inconsistently applied downstream: - UploadJob::processUploadQueue() and SmartlingCoreUploadTrait::prepareUpload() unconditionally re-stamped on every run, overwriting the profile active when the batch was created with whatever is active when the cron job/upload happens to fire. Both now only stamp when getConfigurationProfileId() is still null. UploadJob now does this for every submission in the queue item, not just the first - the others previously stayed unstamped until sendForTranslation touched them individually, with no stamp at all if that failed first. - prepareUpload() runs on every getXMLFiltered() call, including read-only content fetches (ContentProvider::getContent()), so the unconditional re-stamp also meant simply fetching a submission's XML silently rebound it to whatever profile is currently active. - ContentRelationsDiscoveryService::createSubmissions()/bulkUpload() build submissions directly (existing-submission reuse, and submissionFactory->fromArray()) without ever going through SubmissionManager::getSubmissionEntity(), so they never got stamped at all. Both paths now stamp explicitly with the profile the request's batch is being created under, at the moment of the request - not deferred to whichever downstream code first happens to touch the submission. - SettingsManager::getSmartlingLocaleBySubmission() still resolved the *active* profile (getSingleSettingsProfile) even though ApiWrapper::getConfigurationProfile() already resolves the *stamped* one (getProfileBySubmission) for credentials/project. After a profile switch this could send the right project with the wrong (or a nonexistent) locale. Now uses getProfileBySubmission() too, falling back to the active profile exactly like getConfigurationProfile() does. - ContentSerializationHelper::prepareFieldProcessorValues() resolved ignore/copy/SEO filter lists via findEntityByMainLocale() (always the active profile), while FieldsFilterHelper's regexp-mode flag already came from the stamped profile. After a profile switch this mixed filter settings from two different profiles in one upload/apply. Now uses getProfileBySubmission(), keeping the empty-filter fallback for SmartlingDbException. Added unit test coverage for each of these - previously prepareUpload(), getSmartlingLocaleBySubmission(), and prepareFieldProcessorValues() had none at all, and the enqueue-time-stamping paths in ContentRelationsDiscoveryService were only ever exercised against a fully-mocked SubmissionManager. --- .../Base/SmartlingCoreUploadTrait.php | 9 +- .../Helpers/ContentSerializationHelper.php | 9 +- inc/Smartling/Jobs/UploadJob.php | 18 ++- .../ContentRelationsDiscoveryService.php | 14 ++ inc/Smartling/Settings/SettingsManager.php | 2 +- .../ContentRelationsDiscoveryServiceTest.php | 125 ++++++++++++++++++ .../Base/SmartlingCoreUploadTraitTest.php | 82 ++++++++++++ .../ContentSerializationHelperTest.php | 39 ++++++ tests/Smartling/Jobs/UploadJobTest.php | 63 +++++++++ .../Settings/SettingsManagerTest.php | 25 ++++ 10 files changed, 375 insertions(+), 11 deletions(-) diff --git a/inc/Smartling/Base/SmartlingCoreUploadTrait.php b/inc/Smartling/Base/SmartlingCoreUploadTrait.php index b1e89ca41..22b150050 100644 --- a/inc/Smartling/Base/SmartlingCoreUploadTrait.php +++ b/inc/Smartling/Base/SmartlingCoreUploadTrait.php @@ -61,7 +61,14 @@ protected function getFunctionProxyHelper(): WordpressFunctionProxyHelper public function prepareUpload(SubmissionEntity $submission): SubmissionEntity { - $this->getSubmissionManager()->stampConfigurationProfile($submission); + // Only stamp if nothing stamped it yet (e.g. a creation path that bypasses + // SubmissionManager::getSubmissionEntity()). This method also runs on every + // getXMLFiltered() call, including read-only content fetches (ContentProvider), + // so re-stamping unconditionally would silently rebind an in-flight submission to + // whatever profile is currently active, not the one it was requested under. + if ($submission->getConfigurationProfileId() === null) { + $this->getSubmissionManager()->stampConfigurationProfile($submission); + } return $this->renewContentHash( $this->createTargetContent( diff --git a/inc/Smartling/Helpers/ContentSerializationHelper.php b/inc/Smartling/Helpers/ContentSerializationHelper.php index 18a1ea554..a265922de 100644 --- a/inc/Smartling/Helpers/ContentSerializationHelper.php +++ b/inc/Smartling/Helpers/ContentSerializationHelper.php @@ -2,6 +2,7 @@ namespace Smartling\Helpers; +use Smartling\Exception\SmartlingDbException; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; @@ -87,8 +88,6 @@ private function collectSubmissionSourceContent(SubmissionEntity $submission): a public function prepareFieldProcessorValues(SubmissionEntity $submission): array { - $profiles = $this->settingsManager->findEntityByMainLocale($submission->getSourceBlogId()); - $filter = [ 'ignore' => [], 'key' => [ @@ -100,8 +99,8 @@ public function prepareFieldProcessorValues(SubmissionEntity $submission): array ], ]; - if (0 < count($profiles)) { - $profile = ArrayHelper::first($profiles); + try { + $profile = $this->settingsManager->getProfileBySubmission($submission); $filter['ignore'] = $profile->getFilterSkipArray(); $filter['key']['seo'] = array_map('trim', explode(PHP_EOL, $profile->getFilterFlagSeo())); @@ -109,6 +108,8 @@ public function prepareFieldProcessorValues(SubmissionEntity $submission): array $filter['copy']['regexp'] = array_map('trim', explode(PHP_EOL, $profile->getFilterCopyByFieldValueRegex())); LogContextMixinHelper::addToContext('projectId', $profile->getProjectId()); + } catch (SmartlingDbException) { + // no profile available (stored or active) for this submission's source blog; leave filter at defaults } return $filter; diff --git a/inc/Smartling/Jobs/UploadJob.php b/inc/Smartling/Jobs/UploadJob.php index 4c1eef2e1..3f1ed6000 100644 --- a/inc/Smartling/Jobs/UploadJob.php +++ b/inc/Smartling/Jobs/UploadJob.php @@ -78,11 +78,19 @@ private function processUploadQueue(int $blogId): void $this->submissionManager->storeEntity($submission); } // Existing submissions are loaded from the queue by id, so they never pass through - // SubmissionManager::getSubmissionEntity(): remember the profile used for this upload here. - $previousProfileId = $submission->getConfigurationProfileId(); - $this->submissionManager->stampConfigurationProfile($submission); - if ($submission->getConfigurationProfileId() !== $previousProfileId) { - $this->submissionManager->storeEntity($submission); + // SubmissionManager::getSubmissionEntity(): stamp the profile that was active when + // translation was requested, for any submission in this batch that never got one + // (e.g. created via a path that bypasses getSubmissionEntity()). Never overwrite an + // existing stamp here - the batch was already created under that profile, and + // re-stamping to whatever is active when this cron job happens to run would send + // the upload with the wrong project's credentials against that batch. + foreach ($item->getSubmissions() as $itemSubmission) { + if ($itemSubmission->getConfigurationProfileId() === null) { + $this->submissionManager->stampConfigurationProfile($itemSubmission); + if ($itemSubmission->getConfigurationProfileId() !== null) { + $this->submissionManager->storeEntity($itemSubmission); + } + } } $profileKey = $submission->getConfigurationProfileId() ?? "blog{$submission->getSourceBlogId()}"; if (!array_key_exists($profileKey, $profiles)) { diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index 663ee8db2..07ca71fc1 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -106,6 +106,11 @@ public function bulkUpload( $submission->setJobInfo($jobInfo); $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); + // Bulk-submitting is an explicit new translation request: (re)stamp with the + // profile this request's batch is being created under, for both a found + // existing submission (which bypasses getSubmissionEntity() above) and a new + // one (where this just confirms what getSubmissionEntity() already stamped). + $submission->setConfigurationProfileId($profile->getId()); $submission = $this->submissionManager->storeEntity($submission); $queueIds[] = $submission->getId(); $this->logSubmissionCreated($submission, 'Bulk upload request', $jobInfo); @@ -225,6 +230,11 @@ public function createSubmissions(UserTranslationRequest $request): void } else { $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); + // Resubmitting an existing submission is an explicit new translation request, + // so (re)stamp it with the profile active right now, same as + // SubmissionManager::getSubmissionEntity() does - this path never goes through + // that method, so it would otherwise keep whatever profile (or none) it had. + $submission->setConfigurationProfileId($profile->getId()); $submission = $this->storeWithJobInfo($submission, $jobInfo, $request->getDescription()); $fileUris[] = $submission->getFileUri(); $queueIds[] = $submission->getId(); @@ -232,6 +242,10 @@ public function createSubmissions(UserTranslationRequest $request): void $submissionTemplateArray[SubmissionEntity::FIELD_STATUS] = SubmissionEntity::SUBMISSION_STATUS_NEW; $submissionTemplateArray[SubmissionEntity::FIELD_SUBMISSION_DATE] = DateTimeHelper::nowAsString(); + // New submissions built from this template bypass getSubmissionEntity() too; stamp + // them with the profile this request's batch is being created under (below), so + // UploadJob never has to guess at a profile for them later. + $submissionTemplateArray[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] = $profile->getId(); foreach ($sources as $source) { $submissionArray = array_merge($submissionTemplateArray, [ diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index a8599315a..5d79a55ba 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -92,7 +92,7 @@ public function getActiveProfiles(): array */ public function getSmartlingLocaleBySubmission(SubmissionEntity $submission): string { - $profile = $this->getSingleSettingsProfile($submission->getSourceBlogId()); + $profile = $this->getProfileBySubmission($submission); if (TestRunHelper::isTestRunBlog($submission->getTargetBlogId())) { if (count($profile->getTargetLocales()) === 0) { throw new SmartlingConfigException('Profile ' . $profile->getProfileName() . ' (' . $profile->getProjectId() . ') is expected to have at least one target locale for test run'); diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index eb3767ac0..ca8b89905 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -154,6 +154,65 @@ public function testCreateSubmissionsHandler() ])); } + /** + * The non-bulk path's "resubmit an existing submission" branch bypasses + * SubmissionManager::getSubmissionEntity() (and the profile stamp it does), so it + * must stamp the profile this request's batch is created under itself - this is an + * explicit new translation request, same as the bulk path. + */ + public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExistingSubmission() + { + $sourceBlogId = 1; + $sourceId = 48; + $contentType = 'post'; + $targetBlogId = 2; + $jobName = 'Job Name'; + $jobUid = 'abcdef123456'; + $profileId = 5; + + $profile = $this->createMock(ConfigurationProfileEntity::class); + $profile->method('getId')->willReturn($profileId); + + $apiWrapper = $this->createMock(ApiWrapper::class); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + + $siteHelper = $this->createMock(SiteHelper::class); + $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); + + $contentHelper = $this->createMock(ContentHelper::class); + $contentHelper->method('getSiteHelper')->willReturn($siteHelper); + + $submission = $this->createMock(SubmissionEntity::class); + $submission->method('getId')->willReturn(17); + $submission->expects(self::once())->method('setConfigurationProfileId')->with($profileId); + + $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); + $submissionManager->method('findOne')->willReturn($submission); + $submissionManager->method('storeEntity')->willReturnArgument(0); + + $wpProxy = $this->createMock(WordpressFunctionProxyHelper::class); + $wpProxy->method('get_current_blog_id')->willReturn($sourceBlogId); + + $x = $this->getContentRelationDiscoveryService($apiWrapper, $contentHelper, $settingsManager, $submissionManager, wpProxy: $wpProxy); + + $x->createSubmissions(UserTranslationRequest::fromArray([ + 'source' => ['contentType' => $contentType, 'id' => [$sourceId]], + 'job' => + [ + 'id' => $jobUid, + 'name' => $jobName, + 'description' => '', + 'dueDate' => '', + 'timeZone' => 'Europe/Kiev', + 'authorize' => 'true', + ], + 'targetBlogIds' => $targetBlogId, + 'relations' => [], + ])); + } + public function testCreateSubmissionsRelations() { $sourceBlogId = 1; @@ -305,6 +364,72 @@ public function testBulkSubmitHandler() ])); } + /** + * bulkUpload() finds an existing submission via findTargetBlogSubmission(), bypassing + * SubmissionManager::getSubmissionEntity() (and the profile stamp it does). A bulk + * submit is an explicit new translation request, so it must stamp the profile the + * batch is created under itself, or the submission can be delivered via whatever + * profile happens to be active once the cron job picks it up. + */ + public function testBulkSubmitHandlerStampsConfigurationProfile() + { + $sourceBlogId = 1; + $sourceIds = [48]; + $contentType = 'post'; + $targetBlogId = 2; + $jobName = 'Job Name'; + $jobUid = 'abcdef123456'; + $profileId = 5; + + $apiWrapper = $this->createMock(ApiWrapper::class); + + $profile = $this->createMock(ConfigurationProfileEntity::class); + $profile->method('getId')->willReturn($profileId); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + + $siteHelper = $this->createMock(SiteHelper::class); + $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); + + $contentHelper = $this->createMock(ContentHelper::class); + $contentHelper->method('getSiteHelper')->willReturn($siteHelper); + + $submission = $this->createMock(SubmissionEntity::class); + $submission->method('getId')->willReturn(48); + $submission->expects(self::once())->method('setConfigurationProfileId')->with($profileId); + + $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); + $submissionManager->method('findTargetBlogSubmission')->willReturn($submission); + $submissionManager->method('storeEntity')->willReturnArgument(0); + + $wpProxy = $this->createMock(WordpressFunctionProxyHelper::class); + $wpProxy->method('get_current_blog_id')->willReturn($sourceBlogId); + + $x = $this->getContentRelationDiscoveryService( + $apiWrapper, + $contentHelper, + $settingsManager, + $submissionManager, + wpProxy: $wpProxy, + ); + + $x->createSubmissions(UserTranslationRequest::fromArray([ + 'source' => ['contentType' => $contentType, 'id' => [0]], + 'job' => + [ + 'id' => $jobUid, + 'name' => $jobName, + 'description' => '', + 'dueDate' => '', + 'timeZone' => 'Europe/Kiev', + 'authorize' => 'true', + ], + 'targetBlogIds' => $targetBlogId, + 'ids' => $sourceIds, + ])); + } + public function testExistingMenuItemsGetSubmittedOnExistingMenuBulkSubmit() { $sourceBlogId = 1; diff --git a/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php b/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php index 5b5e88446..f8688a55a 100644 --- a/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php +++ b/tests/Smartling/Base/SmartlingCoreUploadTraitTest.php @@ -44,6 +44,7 @@ public function __construct( private TestRunHelper $testRunHelper, private UploadQueueManager $uploadQueueManager, private WordpressFunctionProxyHelper $wpProxy, + private ?ContentSerializationHelper $contentSerializationHelper = null, ) { } @@ -61,6 +62,11 @@ public function getContentHelper(): ContentHelper return $this->contentHelper; } + public function getContentSerializationHelper(): ContentSerializationHelper + { + return $this->contentSerializationHelper; + } + public function getFieldsFilter(): FieldsFilterHelper { return $this->fieldsFilterHelper; @@ -339,6 +345,7 @@ private function getSmartlingCoreUpload( ?SettingsManager $settingsManager = null, ?SubmissionManager $submissionManager = null, ?UploadQueueManager $uploadQueueManager = null, + ?ContentSerializationHelper $contentSerializationHelper = null, ) { if ($contentHelper === null) { $contentHelper = $this->createMock(ContentHelper::class); @@ -355,6 +362,9 @@ private function getSmartlingCoreUpload( if ($uploadQueueManager === null) { $uploadQueueManager = $this->createMock(UploadQueueManager::class); } + if ($contentSerializationHelper === null) { + $contentSerializationHelper = $this->createMock(ContentSerializationHelper::class); + } $externalContentManager = $this->createMock(ExternalContentManager::class); $externalContentManager->method('setExternalContent')->willReturnArgument(1); @@ -373,6 +383,78 @@ private function getSmartlingCoreUpload( $this->createMock(TestRunHelper::class), $uploadQueueManager, $wpProxy, + $contentSerializationHelper, + ); + } + + /** + * prepareUpload() runs on every getXMLFiltered() call, including read-only content + * fetches (ContentProvider::getContent()), not just explicit (re)submissions. It must + * not rebind an in-flight submission to whatever profile happens to be active now. + */ + public function testPrepareUploadDoesNotRestampAlreadyStampedSubmission() + { + $submission = new SubmissionEntity(); + $submission->setId(1); + $submission->setConfigurationProfileId(3); // profile A, stamped at request time + + $contentHelper = $this->createMock(ContentHelper::class); + $sourceContent = new PostEntityStd(); + $sourceContent->post_title = 'Test'; + $contentHelper->method('readSourceContent')->willReturn($sourceContent); + + $contentSerializationHelper = $this->createMock(ContentSerializationHelper::class); + $contentSerializationHelper->method('calculateHash')->willReturn('hash'); + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->expects(self::never())->method('stampConfigurationProfile'); + $submissionManager->method('storeEntity')->willReturnArgument(0); + + $x = $this->getSmartlingCoreUpload( + contentHelper: $contentHelper, + submissionManager: $submissionManager, + contentSerializationHelper: $contentSerializationHelper, ); + + $result = $x->prepareUpload($submission); + + self::assertSame(3, $result->getConfigurationProfileId(), 'Must keep the profile stamped at request time, not whatever is active now'); + } + + /** + * A submission that bypasses SubmissionManager::getSubmissionEntity() (e.g. created + * directly by ContentRelationsDiscoveryService) has no profile stamped yet; this is the + * one case prepareUpload() must still stamp. + */ + public function testPrepareUploadStampsUnstampedSubmission() + { + $submission = new SubmissionEntity(); + $submission->setId(1); + + $contentHelper = $this->createMock(ContentHelper::class); + $sourceContent = new PostEntityStd(); + $sourceContent->post_title = 'Test'; + $contentHelper->method('readSourceContent')->willReturn($sourceContent); + + $contentSerializationHelper = $this->createMock(ContentSerializationHelper::class); + $contentSerializationHelper->method('calculateHash')->willReturn('hash'); + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->expects(self::once())->method('stampConfigurationProfile')->willReturnCallback( + static function (SubmissionEntity $submission) { + $submission->setConfigurationProfileId(5); + }, + ); + $submissionManager->method('storeEntity')->willReturnArgument(0); + + $x = $this->getSmartlingCoreUpload( + contentHelper: $contentHelper, + submissionManager: $submissionManager, + contentSerializationHelper: $contentSerializationHelper, + ); + + $result = $x->prepareUpload($submission); + + self::assertSame(5, $result->getConfigurationProfileId()); } } diff --git a/tests/Smartling/Helpers/ContentSerializationHelperTest.php b/tests/Smartling/Helpers/ContentSerializationHelperTest.php index 5b64f5bd2..f6f2f927f 100644 --- a/tests/Smartling/Helpers/ContentSerializationHelperTest.php +++ b/tests/Smartling/Helpers/ContentSerializationHelperTest.php @@ -4,9 +4,11 @@ use PHPUnit\Framework\TestCase; use Smartling\DbAl\WordpressContentEntities\Entity; +use Smartling\Exception\SmartlingDbException; use Smartling\Helpers\ContentHelper; use Smartling\Helpers\ContentSerializationHelper; use Smartling\Helpers\RuntimeCacheHelper; +use Smartling\Settings\ConfigurationProfileEntity; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; @@ -51,4 +53,41 @@ public function testContentChangeChangesHash(): void $this->helper(['post_content' => 'b', 'post_status' => 'draft'])->calculateHash($this->submission(112)), ); } + + /** + * The filter name-regexp flag comes from the stamped profile (FieldsFilterHelper); the + * ignore/copy lists built here must come from the same profile, or a profile switch + * between request and delivery mixes filter settings from two different profiles. + */ + public function testPrepareFieldProcessorValuesUsesStampedProfile(): void + { + $profile = new ConfigurationProfileEntity(); + $profile->setFilterSkip("ignored_field"); + $profile->setFilterFlagSeo("seo_field"); + $profile->setFilterCopyByFieldName("copy_field"); + $profile->setFilterCopyByFieldValueRegex("copy_regex"); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->expects(self::once())->method('getProfileBySubmission')->willReturn($profile); + $settingsManager->expects(self::never())->method('findEntityByMainLocale'); + + $helper = new ContentSerializationHelper($this->createMock(ContentHelper::class), $settingsManager); + $filter = $helper->prepareFieldProcessorValues($this->submission(1)); + + self::assertSame(['ignored_field'], $filter['ignore']); + self::assertSame(['seo_field'], $filter['key']['seo']); + self::assertSame(['copy_field'], $filter['copy']['name']); + self::assertSame(['copy_regex'], $filter['copy']['regexp']); + } + + public function testPrepareFieldProcessorValuesFallsBackToEmptyFilterWithoutProfile(): void + { + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getProfileBySubmission')->willThrowException(new SmartlingDbException('no profile')); + + $helper = new ContentSerializationHelper($this->createMock(ContentHelper::class), $settingsManager); + $filter = $helper->prepareFieldProcessorValues($this->submission(1)); + + self::assertSame(['ignore' => [], 'key' => ['seo' => []], 'copy' => ['name' => [], 'regexp' => []]], $filter); + } } diff --git a/tests/Smartling/Jobs/UploadJobTest.php b/tests/Smartling/Jobs/UploadJobTest.php index 642c008a6..be9e408a0 100644 --- a/tests/Smartling/Jobs/UploadJobTest.php +++ b/tests/Smartling/Jobs/UploadJobTest.php @@ -150,6 +150,69 @@ static function (SubmissionEntity $submission) { $this->assertSame(5, $submission->getConfigurationProfileId()); } + /** + * The batch this upload is sent against was created under the profile active when the + * user requested translation. Re-stamping to whatever profile is active when this cron + * job happens to run would send the upload with the wrong project's credentials against + * that batch. Stored A, active B, run UploadJob -> still A. + */ + public function testDoesNotRestampAlreadyStampedSubmissionOnUpload() + { + $submission = new SubmissionEntity(); + $submission->setId(1); + $submission->setFileUri('file.xml'); + $submission->setSourceBlogId(1); + $submission->setConfigurationProfileId(3); // profile A, stamped at request time + $item = $this->buildItem($submission); + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->expects($this->never())->method('stampConfigurationProfile'); + $submissionManager->expects($this->never())->method('storeEntity'); + + $this->buildJob($this->buildQueueManager($item), $submissionManager)->run(''); + + $this->assertSame(3, $submission->getConfigurationProfileId(), 'Must keep the profile stamped at request time, not whatever is active now'); + } + + /** + * A queue item groups submissions for several target locales sharing one batch. If only + * the first submission gets stamped, the others would keep NULL profile ids until + * sendForTranslation stamps them individually later - and never get stamped at all if + * that fails first. Every submission in the item must be covered here. + */ + public function testStampsEverySubmissionInItemNotJustTheFirst() + { + $stampedAlready = new SubmissionEntity(); + $stampedAlready->setId(1); + $stampedAlready->setFileUri('file.xml'); + $stampedAlready->setSourceBlogId(1); + $stampedAlready->setConfigurationProfileId(3); + + $unstamped = new SubmissionEntity(); + $unstamped->setId(2); + $unstamped->setFileUri('file.xml'); + $unstamped->setSourceBlogId(1); + + $item = new UploadQueueItem( + [$stampedAlready, $unstamped], + 'batchUid', + new IntStringPairCollection([new IntStringPair(1, 'de-DE'), new IntStringPair(2, 'fr-FR')]), + 42, + ); + + $submissionManager = $this->createMock(SubmissionManager::class); + $submissionManager->method('stampConfigurationProfile')->willReturnCallback( + static function (SubmissionEntity $submission) { + $submission->setConfigurationProfileId(5); + }, + ); + + $this->buildJob($this->buildQueueManager($item), $submissionManager)->run(''); + + $this->assertSame(3, $stampedAlready->getConfigurationProfileId(), 'Must not touch a submission that already had a profile'); + $this->assertSame(5, $unstamped->getConfigurationProfileId(), 'Must stamp every unstamped submission in the item, not just the first'); + } + /** * A queue item groups submissions for the same content across multiple target * locales; only the first one is used to look up the profile/batch job. If either diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index b0f99e5a9..1fb5c201b 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -128,6 +128,31 @@ public function testGetProfileBySubmissionLogsWarningWhenStoredProfileBlogMismat self::assertSame($stored, $mock->getProfileBySubmission($submission)); } + /** + * Credentials/project now come from the stamped profile (getProfileBySubmission), so the + * locale must too - otherwise a profile switch between request and delivery can send the + * right project but the wrong (or a nonexistent) locale. + */ + public function testGetSmartlingLocaleBySubmissionUsesStoredProfileNotActiveOne() + { + $stored = $this->profileWithId(9); + $storedSourceLocale = new Locale(); + $storedSourceLocale->setBlogId(1); + $stored->setSourceLocale($storedSourceLocale); + $storedTargetLocale = new TargetLocale(); + $storedTargetLocale->setBlogId(2); + $storedTargetLocale->setSmartlingLocale('de-DE'); + $stored->setTargetLocales([$storedTargetLocale]); + + $mock = $this->createPartialMock(SettingsManager::class, ['getSingleSettingsProfile', 'getEntityById']); + $mock->expects(self::once())->method('getEntityById')->with(9)->willReturn([$stored]); + $mock->expects(self::never())->method('getSingleSettingsProfile'); + + $submission = (new SubmissionEntity())->setSourceBlogId(1)->setTargetBlogId(2)->setConfigurationProfileId(9); + + self::assertSame('de-DE', $mock->getSmartlingLocaleBySubmission($submission)); + } + public function testGetProfileBySubmissionFallsBackWithoutStoredProfile() { $active = $this->profileWithId(3);